[FFmpeg-devel] [PATCH 1/1] avdevice/decklink_dec: Autodetect the video input format

Marton Balint cus at passwd.hu
Mon Nov 6 02:49:00 EET 2017


On Fri, 27 Oct 2017, Jeyapal, Karthick wrote:

> Please find the patch attached.
>

Thanks, below some comments:

> From b18679b91a79f5e23a5ad23ae70f3862a34ddfb8 Mon Sep 17 00:00:00 2001
> From: Karthick J <kjeyapal at akamai.com>
> Date: Fri, 27 Oct 2017 12:00:23 +0530
> Subject: [PATCH 1/1] avdevice/decklink_dec: Autodetect the video input format
> 
> When -format_code is not specified autodetection will happen
> ---
>  doc/indevs.texi                 |  1 +
>  libavdevice/decklink_common.cpp | 33 ++++++++++---------
>  libavdevice/decklink_common.h   |  8 +++++
>  libavdevice/decklink_dec.cpp    | 70 +++++++++++++++++++++++++++++++++++------
>  4 files changed, 89 insertions(+), 23 deletions(-)
> 
> diff --git a/doc/indevs.texi b/doc/indevs.texi
> index d308bbf..74bfcc5 100644
> --- a/doc/indevs.texi
> +++ b/doc/indevs.texi
> @@ -238,6 +238,7 @@ This sets the input video format to the format given by the FourCC. To see
>  the supported values of your device(s) use @option{list_formats}.
>  Note that there is a FourCC @option{'pal '} that can also be used
>  as @option{pal} (3 letters).
> +Default behavior is autodetection of the input video format

... video format, if the hardware supports it.

>
>  @item bm_v210
>  This is a deprecated option, you can use @option{raw_format} instead.
> diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
> index 2bd63ac..757f419 100644
> --- a/libavdevice/decklink_common.cpp
> +++ b/libavdevice/decklink_common.cpp
> @@ -148,23 +148,10 @@ static DECKLINK_BOOL field_order_eq(enum AVFieldOrder field_order, BMDFieldDomin
>      return false;
>  }
> 
> -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, int num)
> -{
> +void ff_decklink_set_duplex_mode(AVFormatContext *avctx) {
>      struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
>      struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx;
> -    BMDDisplayModeSupport support;
> -    IDeckLinkDisplayModeIterator *itermode;
> -    IDeckLinkDisplayMode *mode;
> -    int i = 1;
>      HRESULT res;
> -
> -    av_log(avctx, AV_LOG_DEBUG, "Trying to find mode for frame size %dx%d, frame timing %d/%d, field order %d, direction %d, mode number %d, format code %s\n",
> -        width, height, tb_num, tb_den, field_order, direction, num, (cctx->format_code) ? cctx->format_code : "(unset)");
> -
>      if (ctx->duplex_mode) {
>          DECKLINK_BOOL duplex_supported = false;
> 
> @@ -181,6 +168,24 @@ int ff_decklink_set_format(AVFormatContext *avctx,
>              av_log(avctx, AV_LOG_WARNING, "Unable to set duplex mode, because it is not supported.\n");
>          }
>      }
> +}

You factorized this out, but keep in mind that decklink_enc might also use this
to set duplex mode. (even if there is no option to do that at the moment). Also
it would make sense to rename the function and also put the input selection
logic here, because even if you autodetect, you should select the input
(sdi/hdmi/etc) first.

This factorization should be a separate patch for easier review.

> +
> +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, int num)
> +{
> +    struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
> +    struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx;
> +    BMDDisplayModeSupport support;
> +    IDeckLinkDisplayModeIterator *itermode;
> +    IDeckLinkDisplayMode *mode;
> +    int i = 1;
> +    HRESULT res;
> +
> +    av_log(avctx, AV_LOG_DEBUG, "Trying to find mode for frame size %dx%d, frame timing %d/%d, field order %d, direction %d, mode number %d, format code %s\n",
> +        width, height, tb_num, tb_den, field_order, direction, num, (cctx->format_code) ? cctx->format_code : "(unset)");
>
>      if (direction == DIRECTION_IN) {
>          int ret;
> diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
> index b6acb01..f38fc14 100644
> --- a/libavdevice/decklink_common.h
> +++ b/libavdevice/decklink_common.h
> @@ -31,6 +31,12 @@
>  class decklink_output_callback;
>  class decklink_input_callback;
> 
> +typedef enum autodetect_state {
> +    AUTODETECT_RESET = 0,

Maybe something like AUTODETECT_INACTIVE is a better name?

> +    AUTODETECT_START,
> +    AUTODETECT_DONE,
> +} autodetect_state;
> +
>  typedef struct AVPacketQueue {
>      AVPacketList *first_pkt, *last_pkt;
>      int nb_packets;
> @@ -95,6 +101,7 @@ struct decklink_ctx {
>      pthread_mutex_t mutex;
>      pthread_cond_t cond;
>      int frames_buffer_available_spots;
> +    autodetect_state autodetect;
>
>      int channels;
>      int audio_depth;
> @@ -134,6 +141,7 @@ static const BMDVideoConnection decklink_video_connection_map[] = {
>  };
>
>  HRESULT ff_decklink_get_display_name(IDeckLink *This, const char **displayName);
> +void ff_decklink_set_duplex_mode(AVFormatContext *avctx);
>  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);
>  int ff_decklink_set_format(AVFormatContext *avctx, decklink_direction_t direction, int num);
>  int ff_decklink_list_devices(AVFormatContext *avctx, struct AVDeviceInfoList *device_list, int show_inputs, int show_outputs);
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index b4b9e02..374a30f 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -634,6 +634,9 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>      BMDTimeValue frameDuration;
>      int64_t wallclock = 0;
> 
> +    if (ctx->autodetect != AUTODETECT_RESET)
> +        return S_OK;
> +
>      ctx->frameCount++;
>      if (ctx->audio_pts_source == PTS_SRC_WALLCLOCK || ctx->video_pts_source == PTS_SRC_WALLCLOCK)
>          wallclock = av_gettime_relative();
> @@ -794,17 +797,51 @@ HRESULT decklink_input_callback::VideoInputFormatChanged(
>      BMDVideoInputFormatChangedEvents events, IDeckLinkDisplayMode *mode,
>      BMDDetectedVideoInputFormatFlags)
>  {
> +    pthread_mutex_lock(&ctx->mutex);
> +    ctx->bmd_width  = mode->GetWidth();
> +    ctx->bmd_height = mode->GetHeight();
> +    ctx->bmd_mode  = mode->GetDisplayMode();
> +    ctx->bmd_field_dominance = mode->GetFieldDominance();
> +    mode->GetFrameRate(&ctx->bmd_tb_num, &ctx->bmd_tb_den);
> +    ctx->autodetect = AUTODETECT_DONE;
> +    pthread_cond_signal(&ctx->cond);
> +    pthread_mutex_unlock(&ctx->mutex);

I'd prefer an approach which only stores bmd_mode and calls
ff_decklink_set_format in the main thread as common code with the
no-autodetection case.

>      return S_OK;
>  }
> 
> -static HRESULT decklink_start_input(AVFormatContext *avctx)
> -{
> -    struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
> +static int decklink_autodetect(struct decklink_cctx *cctx) {
>      struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx;
> +    timespec ts;
> +    int rc;
> +    int64_t t;
> +
> +    ctx->autodetect = AUTODETECT_START;
> +    if (ctx->dli->EnableVideoInput(bmdModeNTSC,
> +                                   (BMDPixelFormat) cctx->raw_format,

You can specify a fixed pixel format here (e.g. 8bit YUV), raw_format here is
not yet validated.

> +                                   bmdVideoInputEnableFormatDetection) != S_OK) {
> +        return -1;
> +    }
> 
> -    ctx->input_callback = new decklink_input_callback(avctx);
> -    ctx->dli->SetCallback(ctx->input_callback);
> -    return ctx->dli->StartStreams();
> +    if (ctx->dli->StartStreams() != S_OK) {
> +        return -1;
> +    }
> +
> +    t = av_gettime();
> +    ts = { .tv_sec  =  t / 1000000,
> +           .tv_nsec = (t % 1000000) * 1000 };
> +    ts.tv_sec += 1;
> +    rc = 0;
> +    pthread_mutex_lock(&ctx->mutex);
> +    while (ctx->autodetect == AUTODETECT_START && rc == 0)
> +        rc = pthread_cond_timedwait(&ctx->cond, &ctx->mutex, &ts);

pthread_cond_timedwait is not portable, it has no compatiblity wrapper 
for w32 threads as far as I know, and using av_gettime() instead of the 
monotonic clock is also ugly a bit, so in general I'd try to avoid it. 
Maybe it is enough if you wait for 25 received frames in VideoFrameArrived 
to be able to detect no signal?

Also, what happens if you specify the proper format as the default? Do you
still get a Format Changed event? So if you get a VideoFrame with bmdNoSignal
flag *not* set, maybe you should finish the autodetection phase?

> +    pthread_mutex_unlock(&ctx->mutex);
> +    if (rc != 0) {
> +        return -1;
> +    }
> +    ctx->dli->PauseStreams();
> +    ctx->dli->FlushStreams();
> +
> +    return 0;
>  }
>
>  extern "C" {
> @@ -916,6 +953,13 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
>          goto error;
>      }
> 
> +    avpacket_queue_init (avctx, &ctx->queue);
> +
> +    ctx->input_callback = new decklink_input_callback(avctx);
> +    ctx->dli->SetCallback(ctx->input_callback);
> +
> +    ff_decklink_set_duplex_mode(avctx);
> +
>      if (mode_num > 0 || cctx->format_code) {
>          if (ff_decklink_set_format(avctx, DIRECTION_IN, mode_num) < 0) {
>              av_log(avctx, AV_LOG_ERROR, "Could not set mode number %d or format code %s for %s\n",
> @@ -923,6 +967,16 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
>              ret = AVERROR(EIO);
>              goto error;
>          }
> +    } else {

If autodetection is not supported, you should not do that. Query using
IDeckLinkAttributes::GetFlag(BMDDeckLinkSupportsInputFormatDetection).

> +        if (decklink_autodetect(cctx) < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Cannot Autodetect input stream or No signal");
> +            ret = AVERROR(EIO);
> +            goto error;
> +        }
> +        av_log(avctx, AV_LOG_INFO, "Autodetected the input mode %dx%d @ %.2f%s fps\n",
> +            ctx->bmd_width, ctx->bmd_height, (float)ctx->bmd_tb_den/(float)ctx->bmd_tb_num,
> +            (ctx->bmd_field_dominance==bmdLowerFieldFirst || ctx->bmd_field_dominance==bmdUpperFieldFirst)?"(i)":"");
> +        ctx->autodetect = AUTODETECT_RESET;
>      }

and if no mode is specified, and autodetetion is not supported, you might as
well fail and inform the user. I think previously the code failed later 
when EnableVideoInput was called with 0 mode...

>
>  #if !CONFIG_LIBZVBI
> @@ -1050,9 +1104,7 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
>          goto error;
>      }
> 
> -    avpacket_queue_init (avctx, &ctx->queue);
> -
> -    if (decklink_start_input (avctx) != S_OK) {
> +    if (ctx->dli->StartStreams() != S_OK) {
>          av_log(avctx, AV_LOG_ERROR, "Cannot start input stream\n");
>          ret = AVERROR(EIO);
>          goto error;
> -- 
> 1.9.1
>

Regards,
Marton


More information about the ffmpeg-devel mailing list