diff mbox

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

Message ID B662926F-DC37-4FE1-AEB3-B332B1EE9B08@akamai.com
State New
Headers show

Commit Message

Patagar, Ravindra Aug. 14, 2017, 4:55 a.m. UTC 
Configurablity to set max queue size so that worst case latency can be controlled.
Signed-off-by: Ravindra Patagar <rpatagar@akamai.com>

---
libavdevice/decklink_common.h   | 2 ++
libavdevice/decklink_common_c.h | 2 ++
libavdevice/decklink_dec.cpp    | 7 +++++--
libavdevice/decklink_dec_c.c    | 2 ++
4 files changed, 11 insertions(+), 2 deletions(-)

--
1.9.1

Comments

Marton Balint Aug. 14, 2017, 7:08 p.m. UTC | #1 
On Mon, 14 Aug 2017, Patagar, Ravindra wrote:

>
> Configurablity to set max queue size so that worst case latency can be controlled.
> Signed-off-by: Ravindra Patagar <rpatagar@akamai.com>
> ---
> libavdevice/decklink_common.h   | 2 ++
> libavdevice/decklink_common_c.h | 2 ++
> libavdevice/decklink_dec.cpp    | 7 +++++--
> libavdevice/decklink_dec_c.c    | 2 ++
> 4 files changed, 11 insertions(+), 2 deletions(-)
>
> 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;
> } 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;
> };
>
> #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;
>     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 ) {
>         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..ea828be 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 },
> +    { "qbufsize",      "Input queue buffer size",   OFFSET(queue_size),    AV_OPT_TYPE_INT,   { .i64 = (1024 * 1024 * 1024)}, 0, INT_MAX, DEC, "bytes"},

I'd rather call the option "queue_size", shortened names should be fine 
for variables, but for option names we tend to use normal text.

Also please add documentation for it and increase libavdevice micro 
version.

Thanks,
Marton
Patagar, Ravindra Aug. 17, 2017, 10:38 a.m. UTC | #2 
Hi Marton,

I have updated the patch as per your comments. Please find the updated patch attached.

Regards,
Ravindra.


On 8/15/17, 12:38 AM, "Marton Balint" <cus@passwd.hu> wrote:

    
    On Mon, 14 Aug 2017, Patagar, Ravindra wrote:
    
    >

    > Configurablity to set max queue size so that worst case latency can be controlled.

    > Signed-off-by: Ravindra Patagar <rpatagar@akamai.com>

    > ---

    > libavdevice/decklink_common.h   | 2 ++

    > libavdevice/decklink_common_c.h | 2 ++

    > libavdevice/decklink_dec.cpp    | 7 +++++--

    > libavdevice/decklink_dec_c.c    | 2 ++

    > 4 files changed, 11 insertions(+), 2 deletions(-)

    >

    > 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;

    > } 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;

    > };

    >

    > #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;

    >     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 ) {

    >         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..ea828be 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 },

    > +    { "qbufsize",      "Input queue buffer size",   OFFSET(queue_size),    AV_OPT_TYPE_INT,   { .i64 = (1024 * 1024 * 1024)}, 0, INT_MAX, DEC, "bytes"},

    
    I'd rather call the option "queue_size", shortened names should be fine 
    for variables, but for option names we tend to use normal text.
    
    Also please add documentation for it and increase libavdevice micro 
    version.
    
    Thanks,
    Marton
    _______________________________________________
    ffmpeg-devel mailing list
    ffmpeg-devel@ffmpeg.org
    http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Marton Balint Aug. 17, 2017, 7:39 p.m. UTC | #3 
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@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@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}.
> 
> +@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
Patagar, Ravindra Aug. 18, 2017, 10:28 a.m. UTC | #4 
Hi Marton,

Thanks for the review. 
Please find the updated patch attached.

Regards,
Ravindra

On 8/18/17, 1:09 AM, "Marton Balint" <cus@passwd.hu> wrote:

    
    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@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@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}.

    > 

    > +@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
    _______________________________________________
    ffmpeg-devel mailing list
    ffmpeg-devel@ffmpeg.org
    http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Matthias Hunstock Aug. 18, 2017, 11:08 a.m. UTC | #5 
Am 14.08.2017 um 06:55 schrieb Patagar, Ravindra:
> + * Copyright (c) 2017 Akamai Technologies, Inc.

Isn't this kind of exaggerated for this little patch? Or is it company
policy to do it?

Beside that I'd prefer the person over the employer or at least both.

Matthias
Patagar, Ravindra Aug. 21, 2017, 9:03 a.m. UTC | #6 
Hi Matthias,
 
Thanks for that question. Yes, it is our company’s policy to add that copyright. It says, “All work submitted is copyright Akamai Technologies, Inc. released under the project’s license”. Since it is in the policy, I can’t add my name either.
But I would like to inform that we are planning to submit many more patches to this plugin, for support for Closed captions(CEA608), Digital program insertion (SCTE 104) etc., so that the overall changes will be much higher than this little patch.
Also I would like to inform that, as long as this project is active within our company, myself and colleagues in my team will actively support any inadvertent issues arising out of our patches.
 
Regards,
Ravindra

On 8/18/17, 4:38 PM, "Matthias Hunstock" <atze@fem.tu-ilmenau.de> wrote:

    Am 14.08.2017 um 06:55 schrieb Patagar, Ravindra:
    > + * Copyright (c) 2017 Akamai Technologies, Inc.

    
    Isn't this kind of exaggerated for this little patch? Or is it company
    policy to do it?
    
    Beside that I'd prefer the person over the employer or at least both.
    
    Matthias
    _______________________________________________
    ffmpeg-devel mailing list
    ffmpeg-devel@ffmpeg.org
    http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Matthias Hunstock Aug. 21, 2017, 9:35 p.m. UTC | #7 
Am 21.08.2017 um 11:03 schrieb Patagar, Ravindra:
> But I would like to inform that we are planning to submit many more patches to this plugin, for support for Closed captions(CEA608), Digital program insertion (SCTE 104) etc., so that the overall changes will be much higher than this little patch.


That sounds great! Thank you for the detailled answer.

Matthias
Marton Balint Aug. 22, 2017, 8:40 p.m. UTC | #8 
On Fri, 18 Aug 2017, Patagar, Ravindra wrote:

> Hi Marton,
>
> Thanks for the review.
> Please find the updated patch attached.

Thanks, applied.

Regards,
Marton
diff mbox

Patch

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;
} 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;
};

#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;
     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 ) {
         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..ea828be 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 },
+    { "qbufsize",      "Input queue buffer size",   OFFSET(queue_size),    AV_OPT_TYPE_INT,   { .i64 = (1024 * 1024 * 1024)}, 0, INT_MAX, DEC, "bytes"},
     { NULL },
};