[FFmpeg-devel] [PATCH] Updated (v3) -- Add input mode autodetect to the decklink module.

Marton Balint cus at passwd.hu
Sat May 21 11:38:38 CEST 2016



On Thu, 12 May 2016, Felt, Patrick wrote:

> I hang my head in shame.  I neglected to notice that time wasn’t already included and so I had to modify the patch.  Apologies for the noise.
>
> -- Add input mode autodetect to the decklink module. Previously users had to supply the input format like this 'DeckLink Device at modenum'.  This patch allows users to either leave it off completely, or supply 0 or negative number to indicate autodetect is requested. Autodetect only works the first time so if the mode changes mid stream you'll die
>
> ---
> libavdevice/decklink_common.cpp | 110 ++++++++++++++++++++++++++++++++++++++++
> libavdevice/decklink_common.h   |   5 ++
> libavdevice/decklink_common_c.h |   1 +
> libavdevice/decklink_dec.cpp    |  86 +++++++++++++++++++++++++------
> libavdevice/decklink_dec_c.c    |   1 +
> 5 files changed, 188 insertions(+), 15 deletions(-)
>
> diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
> index ac7964c..1d51294 100644
> --- a/libavdevice/decklink_common.cpp
> +++ b/libavdevice/decklink_common.cpp
> @@ -98,6 +98,90 @@ HRESULT ff_decklink_get_display_name(IDeckLink *This, const char **displayName)
>     return hr;
> }
>
> +long ff_decklink_mode_to_bm(AVFormatContext *avctx,

Should be BMDDisplayMode, not long.

> +                               decklink_direction_t direction,
> +                               int ffmpeg_mode,
> +                               IDeckLinkDisplayMode **mode)

As far a I see you do not use **mode with a non-NULL arugment in your 
code, so you can get rid of it and its functionality.

> +{
> +    struct decklink_cctx *cctx = (struct decklink_cctx *) avctx->priv_data;

unnecessary space before avctx

> +    struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx;
> +    IDeckLinkDisplayModeIterator *itermode;
> +    IDeckLinkDisplayMode *internal_mode;
> +
> +    int result=0;

add breathing spaces before and after =.
Should be BMDDislpayMode, not int.

> +    HRESULT res;
> +
> +    if (direction == DIRECTION_IN) {
> +        res = ctx->dli->GetDisplayModeIterator (&itermode);

whitespace issue here as well.

> +    } else {
> +        res = ctx->dlo->GetDisplayModeIterator (&itermode);

and here

> +    }
> +
> +    if (res != S_OK) {
> +        av_log(avctx, AV_LOG_ERROR, "Could not get the mode iterator\n");
> +        return -1;

bmdModeUnkown

> +    }
> +
> +    while (itermode->Next(&internal_mode) == S_OK) {
> +        if (++result == ffmpeg_mode) {

You are misuising the result variable here for something that has 
nothing to do with the result of this function.

> +            break;
> +        }
> +
> +        internal_mode->Release();
> +    }
> +
> +    itermode->Release();
> +    if (internal_mode) {

What if there is no match for ffmpeg_mode? Is it documented in the 
Decklink SDK that internal_mode will be overwritten to NULL on the last 
iteration?

> +        result = internal_mode->GetDisplayMode();
> +        if (mode) {
> +            *mode = internal_mode;
> +        } else {
> +            internal_mode->Release();
> +        }
> +
> +        return result;
> +    }
> +
> +    return 0;

bmdModeUnknown

> +}
> +
> +int ff_decklink_mode_to_ffmpeg(AVFormatContext *avctx,
> +                               decklink_direction_t direction,
> +                               IDeckLinkDisplayMode **mode)

should use *mode, not **mode, because *mode is not overwritten in this 
function

> +{
> +    struct decklink_cctx *cctx = (struct decklink_cctx *) avctx->priv_data;

extra space

> +    struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx;
> +    IDeckLinkDisplayModeIterator *itermode;
> +    IDeckLinkDisplayMode *internal_mode;
> +    long bdm_mode_number = (*mode)->GetDisplayMode();

BMDDisplayMode, not long.

> +    int result=1, found=0;

missing breathing spaces

> +    HRESULT res;
> +
> +    if (direction == DIRECTION_IN) {
> +        res = ctx->dli->GetDisplayModeIterator (&itermode);

extra space

> +    } else {
> +        res = ctx->dlo->GetDisplayModeIterator (&itermode);

same here

> +    }
> +
> +    if (res != S_OK) {
> +        av_log(avctx, AV_LOG_ERROR, "Could not get the mode iterator\n");
> +        return -1;

AVERROR_EXTERNAL

> +    }
> +
> +    while (itermode->Next(&internal_mode) == S_OK) {
> +        if (internal_mode->GetDisplayMode() == bdm_mode_number) {
> +            internal_mode->Release();
> +            found = 1;
> +            break;
> +        }
> +        internal_mode->Release();
> +        result++;
> +    }
> +
> +    itermode->Release();
> +    return (found) ? result : -1;

unnecesary paranthesis

> +}
> +
> int ff_decklink_set_format(AVFormatContext *avctx,
>                                int width, int height,
>                                int tb_num, int tb_den,
> @@ -197,6 +281,29 @@ int ff_decklink_list_devices(AVFormatContext *avctx)
>     return 0;
> }
>
> +int ff_decklink_device_autodetect(AVFormatContext *avctx)

Probably worth remaining to somehting like 
ff_decklink_can_detect_input_format otherwise somebody may be 
under the impression that this function will do the autodetection.

> +{
> +    HRESULT res;
> +    struct decklink_cctx *cctx = (struct decklink_cctx *) avctx->priv_data;

extra space

> +    struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx;
> +    IDeckLinkAttributes *attrs = NULL;
> +    bool auto_detect;
> +
> +    res = ctx->dl->QueryInterface(IID_IDeckLinkAttributes, (void**)&attrs);
> +    if (res != S_OK) {
> +        av_log(avctx, AV_LOG_ERROR, "Could not get decklink attributes\n");
> +        return -1;

AVERROR_EXTERNAL

> +    }
> +
> +    res = attrs->GetFlag(BMDDeckLinkSupportsInputFormatDetection, &auto_detect);
> +    if (res != S_OK) {
> +        av_log(avctx, AV_LOG_ERROR, "Attribute fetch failed\n");
> +        return -1;

AVERROR_EXTERNAL

> +    }
> +
> +    return (auto_detect) ? 1 : 0;

Unnecessary paranthesis

> +}
> +
> int ff_decklink_list_formats(AVFormatContext *avctx, decklink_direction_t direction)
> {
>     struct decklink_cctx *cctx = (struct decklink_cctx *) avctx->priv_data;
> @@ -219,6 +326,9 @@ int ff_decklink_list_formats(AVFormatContext *avctx, decklink_direction_t direct
>
>     av_log(avctx, AV_LOG_INFO, "Supported formats for '%s':\n",
>                avctx->filename);
> +    if (ff_decklink_device_autodetect(avctx)) {
> +        av_log(avctx, AV_LOG_INFO, "\t-1\tAuto detection supported\n");

Maybe "Input format autodetection supported" is better not to confuse with 
input detection. (E.g. HDMI or SDI)

> +    }
>     while (itermode->Next(&mode) == S_OK) {
>         BMDTimeValue tb_num, tb_den;
>         mode->GetFrameRate(&tb_num, &tb_den);
> diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
> index dff4fc1..cbe8de2 100644
> --- a/libavdevice/decklink_common.h
> +++ b/libavdevice/decklink_common.h
> @@ -84,6 +84,8 @@ struct decklink_ctx {
>     sem_t semaphore;
>
>     int channels;
> +    int mode_num;
> +    int auto_detect;
> };
>
> typedef enum { DIRECTION_IN, DIRECTION_OUT} decklink_direction_t;
> @@ -105,5 +107,8 @@ int ff_decklink_set_format(AVFormatContext *avctx, int width, int height, int tb
> int ff_decklink_set_format(AVFormatContext *avctx, decklink_direction_t direction, int num);
> int ff_decklink_list_devices(AVFormatContext *avctx);
> int ff_decklink_list_formats(AVFormatContext *avctx, decklink_direction_t direction = DIRECTION_OUT);
> +int ff_decklink_device_autodetect(AVFormatContext *avctx);
> +int ff_decklink_mode_to_ffmpeg(AVFormatContext *avctx, decklink_direction_t direction, IDeckLinkDisplayMode **mode);
> +long ff_decklink_mode_to_bm(AVFormatContext *avctx, decklink_direction_t direction, int ffmpeg_mode, IDeckLinkDisplayMode **mode);

BMDDisplayMode

>
> #endif /* AVDEVICE_DECKLINK_COMMON_H */
> diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_c.h
> index 2b5d92f..0d365be 100644
> --- a/libavdevice/decklink_common_c.h
> +++ b/libavdevice/decklink_common_c.h
> @@ -34,6 +34,7 @@ struct decklink_cctx {
>     double preroll;
>     int v210;
>     int audio_channels;
> +    int autodetect_delay;
> };
>
> #endif /* AVDEVICE_DECKLINK_COMMON_C_H */
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index 1c305f3..7f33909 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -1,5 +1,5 @@
> /*
> - * Blackmagic DeckLink output
> + * Blackmagic DeckLink input
>  * Copyright (c) 2013-2014 Luca Barbato, Deti Fliegl
>  *
>  * This file is part of FFmpeg.
> @@ -32,6 +32,7 @@ extern "C" {
> #if CONFIG_LIBZVBI
> #include <libzvbi.h>
> #endif
> +#include "libavutil/time.h"
> }
>
> #include "decklink_common.h"
> @@ -244,6 +245,12 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>     BMDTimeValue frameTime;
>     BMDTimeValue frameDuration;
>
> +    /* if we don't have video, we are in the autodetect phase.  skip everything and let
> +     * autodetect do its magic */
> +    if (!ctx->video) {
> +        return S_OK;
> +    }
> +
>     ctx->frameCount++;
>
>     // Handle Video Frame
> @@ -393,6 +400,14 @@ HRESULT decklink_input_callback::VideoInputFormatChanged(
>     BMDVideoInputFormatChangedEvents events, IDeckLinkDisplayMode *mode,
>     BMDDetectedVideoInputFormatFlags)
> {
> +
> +    /* undo all the autodetect stuff so we can move on with life */
> +    ctx->dli->PauseStreams();
> +    ctx->dli->FlushStreams();
> +
> +    ctx->mode_num = ff_decklink_mode_to_ffmpeg(avctx, DIRECTION_IN, &mode);
> +    ctx->video = 1;

I would only do anything in this function, if ctx->auto_detect is set 
to 1, and I would also set ctx->auto_detect to 0, so you don't have to 
use a separate ->video variable for signalling a successful autodetection.

Also don't you want to StopStreams and DisableAudio/VideoInput here? 
Because you will be re-doing the whole initialization stuff later, and I 
am not sure you are supposed to call ff_decklink_set_format when the 
streams are already running.

> +
>     return S_OK;
> }
>
> @@ -435,14 +450,14 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
> {
>     struct decklink_cctx *cctx = (struct decklink_cctx *) avctx->priv_data;
>     struct decklink_ctx *ctx;
> -    IDeckLinkDisplayModeIterator *itermode;
>     IDeckLinkIterator *iter;
>     IDeckLink *dl = NULL;
>     AVStream *st;
>     HRESULT result;
>     char fname[1024];
>     char *tmp;
> -    int mode_num = 0;
> +    int auto_detect = 0;
> +    unsigned int autodetect_delay = cctx->autodetect_delay;

You don't have to set autodetect_delay here, you set it later.

>
>     ctx = (struct decklink_ctx *) av_mallocz(sizeof(struct decklink_ctx));
>     if (!ctx)
> @@ -486,7 +501,7 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
>     strcpy (fname, avctx->filename);
>     tmp=strchr (fname, '@');
>     if (tmp != NULL) {
> -        mode_num = atoi (tmp+1);
> +        ctx->mode_num = atoi (tmp+1);
>         *tmp = 0;
>     }
>
> @@ -510,34 +525,74 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
>
>     /* Get input device. */
>     if (ctx->dl->QueryInterface(IID_IDeckLinkInput, (void **) &ctx->dli) != S_OK) {
> -        av_log(avctx, AV_LOG_ERROR, "Could not open output device from '%s'\n",
> +        av_log(avctx, AV_LOG_ERROR, "Could not open input device from '%s'\n",
>                avctx->filename);
>         ctx->dl->Release();
>         return AVERROR(EIO);
>     }
>
> +    auto_detect = ff_decklink_device_autodetect(avctx);
> +
>     /* List supported formats. */
> -    if (ctx->list_formats) {
> +    if (ctx->list_formats || (ctx->mode_num <= 0 && !auto_detect)) {

This seems like an unrelated change, i'd drop it for now.

>         ff_decklink_list_formats(avctx, DIRECTION_IN);
>         ctx->dli->Release();
>         ctx->dl->Release();
>         return AVERROR_EXIT;
>     }
>
> -    if (ctx->dli->GetDisplayModeIterator(&itermode) != S_OK) {
> -        av_log(avctx, AV_LOG_ERROR, "Could not get Display Mode Iterator\n");
> -        ctx->dl->Release();
> -        return AVERROR(EIO);
> -    }
> +    if (ctx->mode_num <= 0 && auto_detect) {
> +        /* the user is wanting to auto detect the mode. we need to fake out the api and not start ffmpeg fully in this mode!*/
> +        ctx->auto_detect = 1;
> +        ctx->mode_num = 1; /* it is assumed that there is at least one mode supported on a decklink card. */
> +
> +        if (ff_decklink_set_format(avctx, DIRECTION_IN, ctx->mode_num) < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Could not set mode %d\n", ctx->mode_num);
> +            goto error;
> +        }
>
> -    if (mode_num > 0) {
> -        if (ff_decklink_set_format(avctx, DIRECTION_IN, mode_num) < 0) {
> -            av_log(avctx, AV_LOG_ERROR, "Could not set mode %d for %s\n", mode_num, fname);
> +        ctx->bmd_mode = ff_decklink_mode_to_bm(avctx, DIRECTION_IN, ctx->mode_num, NULL);
> +        result = ctx->dli->EnableVideoInput(ctx->bmd_mode,
> +                                            cctx->v210 ? bmdFormat10BitYUV : bmdFormat8BitYUV,
> +                                            bmdVideoInputFlagDefault | bmdVideoInputEnableFormatDetection);
> +        if (result != S_OK) {
> +            av_log(avctx, AV_LOG_ERROR, "Could not enable video in the pre-startup autodectect mode\n");
>             goto error;
>         }
> +
> +        result = ctx->dli->EnableAudioInput(bmdAudioSampleRate48kHz, bmdAudioSampleType16bitInteger, cctx->audio_channels);
> +        if (result != S_OK) {
> +            av_log(avctx, AV_LOG_ERROR, "Could not enable audio in the pre-startup autodetect mode\n");
> +            goto error;
> +        }

Are you sure you need audio? Maybe for auto detection, audio is not that 
important, also what if the first format does not have that many audio 
channels, as many the user wants...

> +
> +        result = decklink_start_input(avctx);
> +        if (result != S_OK) {
> +            av_log(avctx, AV_LOG_ERROR, "Could not start input in the pre-startup autodetect mode\n");
> +            goto error;
> +        }
> +
> +        /* we need to give the auto detect code some time to figure out what mode we are really in */
> +        autodetect_delay = cctx->autodetect_delay;
> +        while (!ctx->video) {
> +            if (autodetect_delay--) {
> +                /* this could indicate we are in the right mode.  let's assume so */
> +                break;
> +            }
> +            /* sleep for 1 second */
> +            av_usleep(100000);

That's actually 0.1 sec.

> +        }

You should rework this loop a bit. For starters, probably it is better if 
the user can set the maximum autodetection delay using milisecond 
precision. On the other hand, the sleep time between your checks if the 
autodetecton is completed can be a constant 0.01 second.

Also, as I mentioned earlier I suggest you don't use the ->video variable, 
only the ->auto_detect, so in order to detect a successful detection you 
will wait in the loop until ->auto_detect becomes 0.

>     }
>
> -    itermode->Release();
> +    /* regardless as to if we did autodetect or not, we should now be in a position to
> +     * continue on as before with all the right setup to ensure we get the right mode */
> +    ctx->video = 1;
> +    if (ctx->mode_num > 0) {
> +        if (ff_decklink_set_format(avctx, DIRECTION_IN, ctx->mode_num) < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Could not set mode %d for %s\n", ctx->mode_num, fname);
> +            goto error;
> +        }
> +    }
>
>     /* Setup streams. */
>     st = avformat_new_stream(avctx, NULL);
> @@ -618,6 +673,7 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
>
>     return 0;
>
> +

Unrelated change

> error:
>
>     ctx->dli->Release();
> diff --git a/libavdevice/decklink_dec_c.c b/libavdevice/decklink_dec_c.c
> index 40c21a7..3f83f8f 100644
> --- a/libavdevice/decklink_dec_c.c
> +++ b/libavdevice/decklink_dec_c.c
> @@ -36,6 +36,7 @@ static const AVOption options[] = {
>     { "standard",     NULL,                                           0,  AV_OPT_TYPE_CONST, { .i64 = 0x7fff9fffeLL}, 0, 0,    DEC, "teletext_lines"},
>     { "all",          NULL,                                           0,  AV_OPT_TYPE_CONST, { .i64 = 0x7ffffffffLL}, 0, 0,    DEC, "teletext_lines"},
>     { "channels",     "number of audio channels", OFFSET(audio_channels), AV_OPT_TYPE_INT , { .i64 = 2   }, 2, 16, DEC },
> +    { "autodetect_delay", "number of seconds to wait for autodetect to complete", OFFSET(autodetect_delay), AV_OPT_TYPE_INT , { .i64 = 5   }, 1, 30, DEC },
>     { NULL },
> };
>

Regards,
Marton


More information about the ffmpeg-devel mailing list