[FFmpeg-devel] [PATCH 1/1] libavdevice/decklink: configurablity to set max queue size

[Marton Balint]

On Thu, 17 Aug 2017, Patagar, Ravindra wrote:

> Hi Marton,
>
> I have updated the patch as per your comments. Please find the updated patch attached.

Thanks. Here are some more comments:

> From a55d4b9b2efe919aedc9b4984c100abdca2e41ec Mon Sep 17 00:00:00 2001
> From: Ravindra <rpatagar at akamai.com>
> Date: Thu, 10 Aug 2017 11:59:30 +0530
> Subject: [PATCH 1/1] libavdevice/decklink: configurablity to set max queue
>  size
> 
> Signed-off-by: Ravindra Patagar <rpatagar at akamai.com>
> ---
>  doc/indevs.texi                 | 5 +++++
>  libavdevice/decklink_common.h   | 2 ++
>  libavdevice/decklink_common_c.h | 2 ++
>  libavdevice/decklink_dec.cpp    | 7 +++++--
>  libavdevice/decklink_dec_c.c    | 2 ++
>  libavdevice/version.h           | 2 +-
>  6 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/indevs.texi b/doc/indevs.texi
> index 09e3321..679f1fb 100644
> --- a/doc/indevs.texi
> +++ b/doc/indevs.texi
> @@ -289,6 +289,11 @@ Sets the audio packet timestamp source. Must be @samp{video}, @samp{audio},
>  If set to @samp{true}, color bars are drawn in the event of a signal loss.
>  Defaults to @samp{true}.
> 
> + at item queue_size
> +Sets maximum input buffer size. If the buffering reaches this value,

Sets maximum input buffer size in bytes.

> +incoming frames will be dropped.
> +Defaults to @samp{1073741824}.
> +
>  @end table
>
>  @subsection Examples
> diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
> index c12cf18..64cc722 100644
> --- a/libavdevice/decklink_common.h
> +++ b/libavdevice/decklink_common.h
> @@ -1,6 +1,7 @@
>  /*
>   * Blackmagic DeckLink common code
>   * Copyright (c) 2013-2014 Ramiro Polla, Luca Barbato, Deti Fliegl
> + * Copyright (c) 2017 Akamai Technologies, Inc.
>   *
>   * This file is part of FFmpeg.
>   *
> @@ -38,6 +39,7 @@ typedef struct AVPacketQueue {
>      pthread_mutex_t mutex;
>      pthread_cond_t cond;
>      AVFormatContext *avctx;
> +    int max_q_size;

Use int64_t instead, 1 GB default is getting near the 32 bit limit, some 
users might want more, especially with 4K content.

>  } AVPacketQueue;
>
>  struct decklink_ctx {
> diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_c.h
> index 72c5f9a..0ddd32a 100644
> --- a/libavdevice/decklink_common_c.h
> +++ b/libavdevice/decklink_common_c.h
> @@ -1,6 +1,7 @@
>  /*
>   * Blackmagic DeckLink common code
>   * Copyright (c) 2013-2014 Ramiro Polla
> + * Copyright (c) 2017 Akamai Technologies, Inc.
>   *
>   * This file is part of FFmpeg.
>   *
> @@ -48,6 +49,7 @@ struct decklink_cctx {
>      int video_input;
>      int draw_bars;
>      char *format_code;
> +    int queue_size;

Use int64_t instead here as well.

>  };
>
>  #endif /* AVDEVICE_DECKLINK_COMMON_C_H */
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index 72449a8..8e85c65 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -1,6 +1,7 @@
>  /*
>   * Blackmagic DeckLink input
>   * Copyright (c) 2013-2014 Luca Barbato, Deti Fliegl
> + * Copyright (c) 2017 Akamai Technologies, Inc.
>   *
>   * This file is part of FFmpeg.
>   *
> @@ -187,10 +188,12 @@ static uint8_t* teletext_data_unit_from_vanc_data(uint8_t *src, uint8_t *tgt, in
>
>  static void avpacket_queue_init(AVFormatContext *avctx, AVPacketQueue *q)
>  {
> +    struct decklink_cctx * ctx = (struct decklink_cctx *)avctx->priv_data;

Try to use whitespaces consistent to existing code. E.g.

struct decklink_cctx *ctx = ...

>      memset(q, 0, sizeof(AVPacketQueue));
>      pthread_mutex_init(&q->mutex, NULL);
>      pthread_cond_init(&q->cond, NULL);
>      q->avctx = avctx;
> +    q->max_q_size = ctx->queue_size;
>  }
>
>  static void avpacket_queue_flush(AVPacketQueue *q)
> @@ -230,8 +233,8 @@ static int avpacket_queue_put(AVPacketQueue *q, AVPacket *pkt)
>  {
>      AVPacketList *pkt1;
> 
> -    // Drop Packet if queue size is > 1GB
> -    if (avpacket_queue_size(q) >  1024 * 1024 * 1024 ) {
> +    // Drop Packet if queue size is > maximum queue size
> +    if (avpacket_queue_size(q) >  q->max_q_size ) {

There are some extra white spaces around q->max_q_size.

>          av_log(q->avctx, AV_LOG_WARNING,  "Decklink input buffer overrun!\n");
>          return -1;
>      }
> diff --git a/libavdevice/decklink_dec_c.c b/libavdevice/decklink_dec_c.c
> index 5b26d12..1c992ba 100644
> --- a/libavdevice/decklink_dec_c.c
> +++ b/libavdevice/decklink_dec_c.c
> @@ -1,6 +1,7 @@
>  /*
>   * Blackmagic DeckLink input
>   * Copyright (c) 2014 Deti Fliegl
> + * Copyright (c) 2017 Akamai Technologies, Inc.
>   *
>   * This file is part of FFmpeg.
>   *
> @@ -64,6 +65,7 @@ static const AVOption options[] = {
>      { "reference",     NULL,                                          0,  AV_OPT_TYPE_CONST, { .i64 = PTS_SRC_REFERENCE}, 0, 0, DEC, "pts_source"},
>      { "wallclock",     NULL,                                          0,  AV_OPT_TYPE_CONST, { .i64 = PTS_SRC_WALLCLOCK}, 0, 0, DEC, "pts_source"},
>      { "draw_bars",     "draw bars on signal loss" , OFFSET(draw_bars),    AV_OPT_TYPE_BOOL,  { .i64 = 1}, 0, 1, DEC },
> +    { "queue_size",    "Input queue buffer size",   OFFSET(queue_size),   AV_OPT_TYPE_INT,   { .i64 = (1024 * 1024 * 1024)}, 0, INT_MAX, DEC, "bytes"},

AV_OPT_TYPE_INT64 type, INT64_MAX maximum, and delete "bytes", because 
that parameter is only used for grouping named constants of an option. 
(The attribute name "unit" is a bit misleading, sorry.)

Regards,
Marton