[FFmpeg-devel] [PATCH] Limited timecode support for lavd/decklink
Dave Rice
dave at dericed.com
Fri May 25 22:41:06 EEST 2018
> On May 25, 2018, at 1:08 PM, Jonathan Morley <jmorley at pixsystem.com> wrote:
>
>> On May 25, 2018, at 8:32 AM, Marton Balint <cus at passwd.hu <mailto:cus at passwd.hu>> wrote:
>>
>> On Fri, 25 May 2018, Jonathan Morley wrote:
>>
>>> I apologize in advance for the formatting of this message. My mail
>>> configuration impeded my efforts to use ‘—send-email’.
>>
>> Your mail is corrupted by new lines, attach the .patch file if you cannot resolve this.
>
> Will do! Sorry about that. The mail server’s auth setup kept me from using ‘—send-email’ directly. I will send future patches as attachments.
Here is a copy of my adjustments for the patch so that I could apply it for testing. https://gist.github.com/dericed/af69d60a99ad235b17722a64ea9413ba <https://gist.github.com/dericed/af69d60a99ad235b17722a64ea9413ba>
>>> This commit did pass 'make fate’ though the use of these changes requires
>>> decklink hardware so I did not add any new fate tests. By default the new
>>> optional behavior is skipped for old behavior.
>>>
>>> ---
>>> libavdevice/decklink_common.cpp | 30 -----------------------------
>>> libavdevice/decklink_common.h | 42 ++++++++++++++++++++++++++++++
>>> +++++++++++
>>
>> Why is this huge chunk of function movement needed? If there is a good reason to do that, then split it into a separate patch, if there is not, then just get rid of it.
>
> The move here was so that I could make use of the cross platform safe DECKLINK string methods. I was only able to test with linux and build on mac, but I wanted to do my best to make a contribution that _could_ work on all platforms. I can break this into two patches if you prefer.
>
>>> libavdevice/decklink_common_c.h | 1 +
>>> libavdevice/decklink_dec.cpp | 19 +++++++++++++++++++
>>> libavdevice/decklink_dec_c.c | 9 +++++++++
>>> 5 files changed, 71 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.
>>> cpp
>>> index d8cced7c74..aab9d85b94 100644
>>> --- a/libavdevice/decklink_common.cpp
>>> +++ b/libavdevice/decklink_common.cpp
>>> @@ -77,36 +77,6 @@ static IDeckLinkIterator
>>> *decklink_create_iterator(AVFormatContext
>>> *avctx)
>>> return iter;
>>> }
>>>
>>> -#ifdef _WIN32
>>> -static char *dup_wchar_to_utf8(wchar_t *w)
>>> -{
>>> - char *s = NULL;
>>> - int l = WideCharToMultiByte(CP_UTF8, 0, w, -1, 0, 0, 0, 0);
>>> - s = (char *) av_malloc(l);
>>> - if (s)
>>> - WideCharToMultiByte(CP_UTF8, 0, w, -1, s, l, 0, 0);
>>> - return s;
>>> -}
>>> -#define DECKLINK_STR OLECHAR *
>>> -#define DECKLINK_STRDUP dup_wchar_to_utf8
>>> -#define DECKLINK_FREE(s) SysFreeString(s)
>>> -#elif defined(__APPLE__)
>>> -static char *dup_cfstring_to_utf8(CFStringRef w)
>>> -{
>>> - char s[256];
>>> - CFStringGetCString(w, s, 255, kCFStringEncodingUTF8);
>>> - return av_strdup(s);
>>> -}
>>> -#define DECKLINK_STR const __CFString *
>>> -#define DECKLINK_STRDUP dup_cfstring_to_utf8
>>> -#define DECKLINK_FREE(s) CFRelease(s)
>>> -#else
>>> -#define DECKLINK_STR const char *
>>> -#define DECKLINK_STRDUP av_strdup
>>> -/* free() is needed for a string returned by the DeckLink SDL. */
>>> -#define DECKLINK_FREE(s) free((void *) s)
>>> -#endif
>>> -
>>> HRESULT ff_decklink_get_display_name(IDeckLink *This, const char
>>> **displayName)
>>> {
>>> DECKLINK_STR tmpDisplayName;
>>> diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
>>> index 57ee7d1d68..8c5f8e9f06 100644
>>> --- a/libavdevice/decklink_common.h
>>> +++ b/libavdevice/decklink_common.h
>>> @@ -34,6 +34,36 @@
>>> #define DECKLINK_BOOL bool
>>> #endif
>>>
>>> +#ifdef _WIN32
>>> +static char *dup_wchar_to_utf8(wchar_t *w)
>>> +{
>>> + char *s = NULL;
>>> + int l = WideCharToMultiByte(CP_UTF8, 0, w, -1, 0, 0, 0, 0);
>>> + s = (char *) av_malloc(l);
>>> + if (s)
>>> + WideCharToMultiByte(CP_UTF8, 0, w, -1, s, l, 0, 0);
>>> + return s;
>>> +}
>>> +#define DECKLINK_STR OLECHAR *
>>> +#define DECKLINK_STRDUP dup_wchar_to_utf8
>>> +#define DECKLINK_FREE(s) SysFreeString(s)
>>> +#elif defined(__APPLE__)
>>> +static char *dup_cfstring_to_utf8(CFStringRef w)
>>> +{
>>> + char s[256];
>>> + CFStringGetCString(w, s, 255, kCFStringEncodingUTF8);
>>> + return av_strdup(s);
>>> +}
>>> +#define DECKLINK_STR const __CFString *
>>> +#define DECKLINK_STRDUP dup_cfstring_to_utf8
>>> +#define DECKLINK_FREE(s) CFRelease(s)
>>> +#else
>>> +#define DECKLINK_STR const char *
>>> +#define DECKLINK_STRDUP av_strdup
>>> +/* free() is needed for a string returned by the DeckLink SDL. */
>>> +#define DECKLINK_FREE(s) free((void *) s)
>>> +#endif
>>> +
>>> class decklink_output_callback;
>>> class decklink_input_callback;
>>>
>>> @@ -64,6 +94,7 @@ struct decklink_ctx {
>>> BMDDisplayMode bmd_mode;
>>> BMDVideoConnection video_input;
>>> BMDAudioConnection audio_input;
>>> + BMDTimecodeFormat tc_format;
>>> int bmd_width;
>>> int bmd_height;
>>> int bmd_field_dominance;
>>> @@ -140,6 +171,17 @@ static const BMDVideoConnection
>>> decklink_video_connection_map[] = {
>>> bmdVideoConnectionSVideo,
>>> };
>>>
>>> +static const BMDTimecodeFormat decklink_timecode_format_map[] = {
>>> + (BMDTimecodeFormat)0,
>>> + bmdTimecodeRP188VITC1,
>>> + bmdTimecodeRP188VITC2,
>>> + bmdTimecodeRP188LTC,
>>> + bmdTimecodeRP188Any,
>>> + bmdTimecodeVITC,
>>> + bmdTimecodeVITCField2,
>>> + bmdTimecodeSerial,
>>> +};
>>> +
>>> HRESULT ff_decklink_get_display_name(IDeckLink *This, const char
>>> **displayName);
>>> int ff_decklink_set_configs(AVFormatContext *avctx, decklink_direction_t
>>> direction);
>>> int ff_decklink_set_format(AVFormatContext *avctx, int width, int height,
>>> int tb_num, int tb_den, enum AVFieldOrder field_order, decklink_direction_t
>>> direction = DIRECTION_OUT, int num = 0);
>>> diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_
>>> c.h
>>> index 08e9f9bbd5..32a5d70ee1 100644
>>> --- a/libavdevice/decklink_common_c.h
>>> +++ b/libavdevice/decklink_common_c.h
>>> @@ -50,6 +50,7 @@ struct decklink_cctx {
>>> DecklinkPtsSource video_pts_source;
>>> int audio_input;
>>> int video_input;
>>> + int tc_format;
>>> int draw_bars;
>>> char *format_code;
>>> int raw_format;
>>> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
>>> index 510637676c..818235bfa6 100644
>>> --- a/libavdevice/decklink_dec.cpp
>>> +++ b/libavdevice/decklink_dec.cpp
>>> @@ -726,6 +726,23 @@ HRESULT decklink_input_callback::
>>> VideoInputFrameArrived(
>>> no_video = 0;
>>> }
>>>
>>> + // Handle Timecode (if requested)
>>> + if (ctx->tc_format && !(av_dict_get(ctx->video_st->metadata,
>>> "timecode", NULL, 0)) && !no_video) {
>>> + IDeckLinkTimecode *timecode;
>>> + if (videoFrame->GetTimecode(ctx->tc_format, &timecode) ==
>>> S_OK) {
>>> + DECKLINK_STR timecodeString = NULL;
>>> + timecode->GetString(&timecodeString);
>>> + const char* tc = DECKLINK_STRDUP(timecodeString);
>>> + if (!(av_dict_set(&ctx->video_st->metadata, "timecode",
>>> tc, 0)))
>>
>> Don't you need AV_DICT_DONT_STRDUP_VAL flag here?
>
> Yes. I agree.
>
>>> + av_log(avctx, AV_LOG_ERROR, "Unable to set
>>> timecode\n");
>>> + if (timecodeString)
>>> + DECKLINK_FREE(timecodeString);
>>> + timecode->Release();
>>> + } else {
>>> + av_log(avctx, AV_LOG_ERROR, "Unable to find timecode.\n");
>>> + }
>>> + }
>>
>> Maybe it makes sense to put this under the else branch of if (videoFrame->GetFlags() & bmdFrameHasNoInputSource), because if you have no input, you won't have any (valid) timecode…
>
> I can move this. The logic still checks for !no_video, but moving it should be more explicit.
>
> Sadly I no longer have access to the hardware I used when making these changes and cannot test this kind of change. We have moved on to a video-for-linux based solution with AJA hardware. (I am currently adding support for TC to the v4l2 ffmpeg reader).
I tested moving this function up a few lines into the else branch as Marton suggested and it worked well for me. I’m running this on a Mac. The timecode values when recording with this match what I get via Blackmagic Media Express, but the drop frame flag does not match. I’m testing with a vitc timecode source which is non-drop frame but the value shows in ffmpeg as drop frame, 07:09:56;19.
>>> +
>>> pkt.pts = get_pkt_pts(videoFrame, audioFrame, wallclock,
>>> abs_wallclock, ctx->video_pts_source, ctx->video_st->time_base,
>>> &initial_video_pts, cctx->copyts);
>>> pkt.dts = pkt.pts;
>>>
>>> @@ -939,6 +956,8 @@ av_cold int ff_decklink_read_header(AVFormatContext
>>> *avctx)
>>> ctx->teletext_lines = cctx->teletext_lines;
>>> ctx->preroll = cctx->preroll;
>>> ctx->duplex_mode = cctx->duplex_mode;
>>> + if (cctx->tc_format > 0 && (unsigned int)cctx->tc_format <
>>> FF_ARRAY_ELEMS(decklink_timecode_format_map))
>>> + ctx->tc_format = decklink_timecode_format_map[cctx->tc_format];
>>> if (cctx->video_input > 0 && (unsigned int)cctx->video_input <
>>> FF_ARRAY_ELEMS(decklink_video_connection_map))
>>> ctx->video_input = decklink_video_connection_map[cctx->video_input];
>>> if (cctx->audio_input > 0 && (unsigned int)cctx->audio_input <
>>> FF_ARRAY_ELEMS(decklink_audio_connection_map))
>>> diff --git a/libavdevice/decklink_dec_c.c b/libavdevice/decklink_dec_c.c
>>> index 47018dc681..6ab3819375 100644
>>> --- a/libavdevice/decklink_dec_c.c
>>> +++ b/libavdevice/decklink_dec_c.c
>>> @@ -48,6 +48,15 @@ static const AVOption options[] = {
>>> { "unset", NULL, 0,
>>> AV_OPT_TYPE_CONST, { .i64 = 0}, 0, 0, DEC, "duplex_mode"},
>>> { "half", NULL, 0,
>>> AV_OPT_TYPE_CONST, { .i64 = 1}, 0, 0, DEC, "duplex_mode"},
>>> { "full", NULL, 0,
>>> AV_OPT_TYPE_CONST, { .i64 = 2}, 0, 0, DEC, "duplex_mode"},
>>> + { "timecode_format", "timecode format", OFFSET(tc_format),
>>> AV_OPT_TYPE_INT, { .i64 = 0}, 0, 7, DEC, "tc_format"},
>>> + { "none", NULL, 0,
>>> AV_OPT_TYPE_CONST, { .i64 = 0}, 0, 0, DEC, "tc_format"},
>>> + { "rp188vitc", NULL, 0,
>>> AV_OPT_TYPE_CONST, { .i64 = 1}, 0, 0, DEC, "tc_format"},
>>> + { "rp188vitc2", NULL, 0,
>>> AV_OPT_TYPE_CONST, { .i64 = 2}, 0, 0, DEC, "tc_format"},
>>> + { "rp188ltc", NULL, 0,
>>> AV_OPT_TYPE_CONST, { .i64 = 3}, 0, 0, DEC, "tc_format"},
>>> + { "rp188any", NULL, 0,
>>> AV_OPT_TYPE_CONST, { .i64 = 4}, 0, 0, DEC, "tc_format"},
>>> + { "vitc", NULL, 0,
>>> AV_OPT_TYPE_CONST, { .i64 = 5}, 0, 0, DEC, "tc_format"},
>>> + { "vitc2", NULL, 0,
>>> AV_OPT_TYPE_CONST, { .i64 = 6}, 0, 0, DEC, "tc_format"},
>>> + { "serial", NULL, 0,
>>> AV_OPT_TYPE_CONST, { .i64 = 7}, 0, 0, DEC, "tc_format"},
>>> { "video_input", "video input", OFFSET(video_input),
>>> AV_OPT_TYPE_INT, { .i64 = 0}, 0, 6, DEC, "video_input"},
>>> { "unset", NULL, 0,
>>> AV_OPT_TYPE_CONST, { .i64 = 0}, 0, 0, DEC, "video_input"},
>>> { "sdi", NULL, 0,
>>> AV_OPT_TYPE_CONST, { .i64 = 1}, 0, 0, DEC, "video_input"},
>>> --
>>
>> Documentation update is missing.
>
> D’oh! Sloppy.
Thanks for this work!
Dave
More information about the ffmpeg-devel
mailing list