| Message ID | 2685f0cc-78f7-4ef7-833a-a56223fa6236@nativewaves.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] libavdevice/decklink: extend available actions on signal loss | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On 9/25/23 13:58, Michael Riedl wrote: > Deprecate the option 'draw_bars' in favor of the new option 'signal_loss_action', > which controls the behavior when the input signal is not available > (including the behavior previously available through draw_bars). > The default behavior remains unchanged to be backwards compatible. > The new option is more flexible for extending now and in the future. > > The new value 'repeat' repeats the last video frame. > This is useful for very short dropouts and was not available before. > > Signed-off-by: Michael Riedl <michael.riedl@nativewaves.com> > --- > doc/indevs.texi | 16 ++++++++++++++++ > libavdevice/decklink_common.h | 1 + > libavdevice/decklink_common_c.h | 7 +++++++ > libavdevice/decklink_dec.cpp | 23 ++++++++++++++++++++++- > libavdevice/decklink_dec_c.c | 6 +++++- > 5 files changed, 51 insertions(+), 2 deletions(-) > > diff --git a/doc/indevs.texi b/doc/indevs.texi > index 863536a34d5..a985d58ce7f 100644 > --- a/doc/indevs.texi > +++ b/doc/indevs.texi > @@ -395,6 +395,22 @@ Defaults to @samp{audio}. > @item draw_bars > If set to @samp{true}, color bars are drawn in the event of a signal loss. > Defaults to @samp{true}. > +This option is deprecated, please use the @code{signal_loss_action} option. > + > +@item signal_loss_action > +Sets the action to take in the event of a signal loss. Accepts one of the > +following values: > + > +@table @option > +@item 1, none > +Do nothing on signal loss. This usually results in black frames. > +@item 2, bars > +Draw color bars on signal loss. Only supported for 8-bit input signals. > +@item 3, repeat > +Repeat the last video frame on signal loss. > +@end table > + > +Defaults to @samp{bars}. > > @item queue_size > Sets maximum input buffer size in bytes. If the buffering reaches this value, > diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h > index 34ab1b96700..be666212e6c 100644 > --- a/libavdevice/decklink_common.h > +++ b/libavdevice/decklink_common.h > @@ -146,6 +146,7 @@ struct decklink_ctx { > DecklinkPtsSource video_pts_source; > int draw_bars; > BMDPixelFormat raw_format; > + DecklinkSignalLossAction signal_loss_action; > > int frames_preroll; > int frames_buffer; > diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_c.h > index 9c55d891494..53d2c583e7e 100644 > --- a/libavdevice/decklink_common_c.h > +++ b/libavdevice/decklink_common_c.h > @@ -37,6 +37,12 @@ typedef enum DecklinkPtsSource { > PTS_SRC_NB > } DecklinkPtsSource; > > +typedef enum DecklinkSignalLossAction { > + SIGNAL_LOSS_NONE = 1, > + SIGNAL_LOSS_REPEAT = 2, > + SIGNAL_LOSS_BARS = 3 > +} DecklinkSignalLossAction; > + > struct decklink_cctx { > const AVClass *cclass; > > @@ -68,6 +74,7 @@ struct decklink_cctx { > int64_t timestamp_align; > int timing_offset; > int wait_for_tc; > + DecklinkSignalLossAction signal_loss_action; > }; > > #endif /* AVDEVICE_DECKLINK_COMMON_C_H */ > diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp > index 11640f72caa..f6095c72359 100644 > --- a/libavdevice/decklink_dec.cpp > +++ b/libavdevice/decklink_dec.cpp > @@ -593,6 +593,7 @@ private: > int no_video; > int64_t initial_video_pts; > int64_t initial_audio_pts; > + IDeckLinkVideoInputFrame* last_video_frame; > }; > > decklink_input_callback::decklink_input_callback(AVFormatContext *_avctx) : _refs(1) > @@ -602,10 +603,13 @@ decklink_input_callback::decklink_input_callback(AVFormatContext *_avctx) : _ref > ctx = (struct decklink_ctx *)cctx->ctx; > no_video = 0; > initial_audio_pts = initial_video_pts = AV_NOPTS_VALUE; > + last_video_frame = nullptr; > } > > decklink_input_callback::~decklink_input_callback() > { > + if (last_video_frame) > + last_video_frame->Release(); > } > > ULONG decklink_input_callback::AddRef(void) > @@ -773,7 +777,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived( > ctx->video_st->time_base.den); > > if (videoFrame->GetFlags() & bmdFrameHasNoInputSource) { > - if (ctx->draw_bars && videoFrame->GetPixelFormat() == bmdFormat8BitYUV) { > + if (ctx->signal_loss_action == SIGNAL_LOSS_BARS && videoFrame->GetPixelFormat() == bmdFormat8BitYUV) { > unsigned bars[8] = { > 0xEA80EA80, 0xD292D210, 0xA910A9A5, 0x90229035, > 0x6ADD6ACA, 0x51EF515A, 0x286D28EF, 0x10801080 }; > @@ -785,6 +789,8 @@ HRESULT decklink_input_callback::VideoInputFrameArrived( > for (int x = 0; x < width; x += 2) > *p++ = bars[(x * 8) / width]; > } > + } else if (ctx->signal_loss_action == SIGNAL_LOSS_REPEAT) { > + last_video_frame->GetBytes(&frameBytes); > } > > if (!no_video) { > @@ -793,6 +799,12 @@ HRESULT decklink_input_callback::VideoInputFrameArrived( > } > no_video = 1; > } else { > + if (ctx->signal_loss_action == SIGNAL_LOSS_REPEAT) { > + if (last_video_frame) > + last_video_frame->Release(); > + last_video_frame = videoFrame; > + last_video_frame->AddRef(); > + } > if (no_video) { > av_log(avctx, AV_LOG_WARNING, "Frame received (#%lu) - Input returned " > "- Frames dropped %u\n", ctx->frameCount, ++ctx->dropped); > @@ -1074,6 +1086,15 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx) > ctx->audio_pts_source = cctx->audio_pts_source; > ctx->video_pts_source = cctx->video_pts_source; > ctx->draw_bars = cctx->draw_bars; > + ctx->signal_loss_action = cctx->signal_loss_action; > + if (!ctx->draw_bars && ctx->signal_loss_action == SIGNAL_LOSS_BARS) { > + ctx->signal_loss_action = SIGNAL_LOSS_NONE; > + av_log(avctx, AV_LOG_WARNING, "Setting signal_loss_action to none because draw_bars is false\n"); > + } > + if (!ctx->draw_bars && ctx->signal_loss_action != SIGNAL_LOSS_NONE) { > + av_log(avctx, AV_LOG_ERROR, "options draw_bars and signal_loss_action are mutually exclusive\n"); > + return AVERROR(EINVAL); > + } > ctx->audio_depth = cctx->audio_depth; > if (cctx->raw_format > 0 && (unsigned int)cctx->raw_format < FF_ARRAY_ELEMS(decklink_raw_format_map)) > ctx->raw_format = decklink_raw_format_map[cctx->raw_format]; > diff --git a/libavdevice/decklink_dec_c.c b/libavdevice/decklink_dec_c.c > index 2159702c96b..7c0ef4f8c23 100644 > --- a/libavdevice/decklink_dec_c.c > +++ b/libavdevice/decklink_dec_c.c > @@ -94,12 +94,16 @@ 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"}, > { "abs_wallclock", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = PTS_SRC_ABS_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 }, > + { "draw_bars", "use option signal_loss_action instead" , OFFSET(draw_bars), AV_OPT_TYPE_BOOL, { .i64 = 1}, 0, 1, DEC | AV_OPT_FLAG_DEPRECATED }, > { "queue_size", "input queue buffer size", OFFSET(queue_size), AV_OPT_TYPE_INT64, { .i64 = (1024 * 1024 * 1024)}, 0, INT64_MAX, DEC }, > { "audio_depth", "audio bitdepth (16 or 32)", OFFSET(audio_depth), AV_OPT_TYPE_INT, { .i64 = 16}, 16, 32, DEC }, > { "decklink_copyts", "copy timestamps, do not remove the initial offset", OFFSET(copyts), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, DEC }, > { "timestamp_align", "capture start time alignment (in seconds)", OFFSET(timestamp_align), AV_OPT_TYPE_DURATION, { .i64 = 0 }, 0, INT_MAX, DEC }, > { "wait_for_tc", "drop frames till a frame with timecode is received. TC format must be set", OFFSET(wait_for_tc), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, DEC }, > + { "signal_loss_action", "action on signal loss", OFFSET(signal_loss_action), AV_OPT_TYPE_INT, { .i64 = SIGNAL_LOSS_BARS }, SIGNAL_LOSS_NONE, SIGNAL_LOSS_BARS, DEC, "signal_loss_action" }, > + { "none", "do not do anything (usually leads to black frames)", 0, AV_OPT_TYPE_CONST, { .i64 = SIGNAL_LOSS_NONE }, 0, 0, DEC, "signal_loss_action"}, > + { "bars", "draw color bars (only supported for 8-bit signals)", 0, AV_OPT_TYPE_CONST, { .i64 = SIGNAL_LOSS_BARS }, 0, 0, DEC, "signal_loss_action"}, > + { "repeat", "repeat the last video frame", 0, AV_OPT_TYPE_CONST, { .i64 = SIGNAL_LOSS_REPEAT }, 0, 0, DEC, "signal_loss_action"}, > { NULL }, > }; Ping
Hi Michael, I haven't tried your patch, but a quick review suggests that while you've declared the option as deprecated that it no longer works. Presumably somewhere in there should be a line of code that says something like "if (ctx->draw_bars == 0) then ctx->signal_loss_action = SIGNAL_LOSS_NONE" Even though the option is deprecated, it should still continue to work until it is completely removed. Devin
> Hi Michael, > > I haven't tried your patch, but a quick review suggests that while > you've declared the option as deprecated that it no longer works. > Presumably somewhere in there should be a line of code that says > something like "if (ctx->draw_bars == 0) then ctx->signal_loss_action > = SIGNAL_LOSS_NONE" > > Even though the option is deprecated, it should still continue to work > until it is completely removed. > > Devin Hi Devin, thank you for reviewing. I just tested the code again and can confirm that the old option draw_bars still works as intended (as describe in the table below). If the users explicitly uses "-draw_bars false" then we set signal_loss_action to SIGNAL_LOSS_NONE (if the option was not set by the user) and emit a deprecation warning to the user. If the users leaves draw_bars to the default value, the option signal_loss_action has priority. I think this is the best solution for now that keeps the old behavior and allows us to remove the option in the future. | draw_bars | signal_loss_action | new action | | -------------- | ------------------ | -------------- | | true (default) | bars (default) | bars | | true (default) | none | none | | true (default) | repeat | repeat | | false | bars (default) | none | | false | none | none | | false | repeat | conflict error |
Ping
On Tue, Nov 28, 2023 at 4:22 AM Michael Riedl <michael.riedl@nativewaves.com> wrote: > > Ping My apologies, I saw your remarks that this didn't change backward compatibility but failed to reply. I have no further issues with this patch. Thanks, Devin
On 11/28/23 16:48, Devin Heitmueller wrote: > My apologies, I saw your remarks that this didn't change backward > compatibility but failed to reply. I have no further issues with this > patch. > > Thanks, > > Devin Ping. Can this please be merged? Thanks.
Ping. Anything missing or wrong with this patch?
diff --git a/doc/indevs.texi b/doc/indevs.texi index 863536a34d5..a985d58ce7f 100644 --- a/doc/indevs.texi +++ b/doc/indevs.texi @@ -395,6 +395,22 @@ Defaults to @samp{audio}. @item draw_bars If set to @samp{true}, color bars are drawn in the event of a signal loss. Defaults to @samp{true}. +This option is deprecated, please use the @code{signal_loss_action} option. + +@item signal_loss_action +Sets the action to take in the event of a signal loss. Accepts one of the +following values: + +@table @option +@item 1, none +Do nothing on signal loss. This usually results in black frames. +@item 2, bars +Draw color bars on signal loss. Only supported for 8-bit input signals. +@item 3, repeat +Repeat the last video frame on signal loss. +@end table + +Defaults to @samp{bars}. @item queue_size Sets maximum input buffer size in bytes. If the buffering reaches this value, diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h index 34ab1b96700..be666212e6c 100644 --- a/libavdevice/decklink_common.h +++ b/libavdevice/decklink_common.h @@ -146,6 +146,7 @@ struct decklink_ctx { DecklinkPtsSource video_pts_source; int draw_bars; BMDPixelFormat raw_format; + DecklinkSignalLossAction signal_loss_action; int frames_preroll; int frames_buffer; diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_c.h index 9c55d891494..53d2c583e7e 100644 --- a/libavdevice/decklink_common_c.h +++ b/libavdevice/decklink_common_c.h @@ -37,6 +37,12 @@ typedef enum DecklinkPtsSource { PTS_SRC_NB } DecklinkPtsSource; +typedef enum DecklinkSignalLossAction { + SIGNAL_LOSS_NONE = 1, + SIGNAL_LOSS_REPEAT = 2, + SIGNAL_LOSS_BARS = 3 +} DecklinkSignalLossAction; + struct decklink_cctx { const AVClass *cclass; @@ -68,6 +74,7 @@ struct decklink_cctx { int64_t timestamp_align; int timing_offset; int wait_for_tc; + DecklinkSignalLossAction signal_loss_action; }; #endif /* AVDEVICE_DECKLINK_COMMON_C_H */ diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp index 11640f72caa..f6095c72359 100644 --- a/libavdevice/decklink_dec.cpp +++ b/libavdevice/decklink_dec.cpp @@ -593,6 +593,7 @@ private: int no_video; int64_t initial_video_pts; int64_t initial_audio_pts; + IDeckLinkVideoInputFrame* last_video_frame; }; decklink_input_callback::decklink_input_callback(AVFormatContext *_avctx) : _refs(1) @@ -602,10 +603,13 @@ decklink_input_callback::decklink_input_callback(AVFormatContext *_avctx) : _ref ctx = (struct decklink_ctx *)cctx->ctx; no_video = 0; initial_audio_pts = initial_video_pts = AV_NOPTS_VALUE; + last_video_frame = nullptr; } decklink_input_callback::~decklink_input_callback() { + if (last_video_frame) + last_video_frame->Release(); } ULONG decklink_input_callback::AddRef(void) @@ -773,7 +777,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived( ctx->video_st->time_base.den); if (videoFrame->GetFlags() & bmdFrameHasNoInputSource) { - if (ctx->draw_bars && videoFrame->GetPixelFormat() == bmdFormat8BitYUV) { + if (ctx->signal_loss_action == SIGNAL_LOSS_BARS && videoFrame->GetPixelFormat() == bmdFormat8BitYUV) { unsigned bars[8] = { 0xEA80EA80, 0xD292D210, 0xA910A9A5, 0x90229035, 0x6ADD6ACA, 0x51EF515A, 0x286D28EF, 0x10801080 }; @@ -785,6 +789,8 @@ HRESULT decklink_input_callback::VideoInputFrameArrived( for (int x = 0; x < width; x += 2) *p++ = bars[(x * 8) / width]; } + } else if (ctx->signal_loss_action == SIGNAL_LOSS_REPEAT) { + last_video_frame->GetBytes(&frameBytes); } if (!no_video) { @@ -793,6 +799,12 @@ HRESULT decklink_input_callback::VideoInputFrameArrived( } no_video = 1; } else { + if (ctx->signal_loss_action == SIGNAL_LOSS_REPEAT) { + if (last_video_frame) + last_video_frame->Release(); + last_video_frame = videoFrame; + last_video_frame->AddRef(); + } if (no_video) { av_log(avctx, AV_LOG_WARNING, "Frame received (#%lu) - Input returned " "- Frames dropped %u\n", ctx->frameCount, ++ctx->dropped); @@ -1074,6 +1086,15 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx) ctx->audio_pts_source = cctx->audio_pts_source; ctx->video_pts_source = cctx->video_pts_source; ctx->draw_bars = cctx->draw_bars; + ctx->signal_loss_action = cctx->signal_loss_action; + if (!ctx->draw_bars && ctx->signal_loss_action == SIGNAL_LOSS_BARS) { + ctx->signal_loss_action = SIGNAL_LOSS_NONE; + av_log(avctx, AV_LOG_WARNING, "Setting signal_loss_action to none because draw_bars is false\n"); + } + if (!ctx->draw_bars && ctx->signal_loss_action != SIGNAL_LOSS_NONE) { + av_log(avctx, AV_LOG_ERROR, "options draw_bars and signal_loss_action are mutually exclusive\n"); + return AVERROR(EINVAL); + } ctx->audio_depth = cctx->audio_depth; if (cctx->raw_format > 0 && (unsigned int)cctx->raw_format < FF_ARRAY_ELEMS(decklink_raw_format_map)) ctx->raw_format = decklink_raw_format_map[cctx->raw_format]; diff --git a/libavdevice/decklink_dec_c.c b/libavdevice/decklink_dec_c.c index 2159702c96b..7c0ef4f8c23 100644 --- a/libavdevice/decklink_dec_c.c +++ b/libavdevice/decklink_dec_c.c @@ -94,12 +94,16 @@ 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"}, { "abs_wallclock", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = PTS_SRC_ABS_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 }, + { "draw_bars", "use option signal_loss_action instead" , OFFSET(draw_bars), AV_OPT_TYPE_BOOL, { .i64 = 1}, 0, 1, DEC | AV_OPT_FLAG_DEPRECATED }, { "queue_size", "input queue buffer size", OFFSET(queue_size), AV_OPT_TYPE_INT64, { .i64 = (1024 * 1024 * 1024)}, 0, INT64_MAX, DEC }, { "audio_depth", "audio bitdepth (16 or 32)", OFFSET(audio_depth), AV_OPT_TYPE_INT, { .i64 = 16}, 16, 32, DEC }, { "decklink_copyts", "copy timestamps, do not remove the initial offset", OFFSET(copyts), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, DEC }, { "timestamp_align", "capture start time alignment (in seconds)", OFFSET(timestamp_align), AV_OPT_TYPE_DURATION, { .i64 = 0 }, 0, INT_MAX, DEC }, { "wait_for_tc", "drop frames till a frame with timecode is received. TC format must be set", OFFSET(wait_for_tc), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, DEC }, + { "signal_loss_action", "action on signal loss", OFFSET(signal_loss_action), AV_OPT_TYPE_INT, { .i64 = SIGNAL_LOSS_BARS }, SIGNAL_LOSS_NONE, SIGNAL_LOSS_BARS, DEC, "signal_loss_action" }, + { "none", "do not do anything (usually leads to black frames)", 0, AV_OPT_TYPE_CONST, { .i64 = SIGNAL_LOSS_NONE }, 0, 0, DEC, "signal_loss_action"}, + { "bars", "draw color bars (only supported for 8-bit signals)", 0, AV_OPT_TYPE_CONST, { .i64 = SIGNAL_LOSS_BARS }, 0, 0, DEC, "signal_loss_action"}, + { "repeat", "repeat the last video frame", 0, AV_OPT_TYPE_CONST, { .i64 = SIGNAL_LOSS_REPEAT }, 0, 0, DEC, "signal_loss_action"}, { NULL }, };
Deprecate the option 'draw_bars' in favor of the new option 'signal_loss_action', which controls the behavior when the input signal is not available (including the behavior previously available through draw_bars). The default behavior remains unchanged to be backwards compatible. The new option is more flexible for extending now and in the future. The new value 'repeat' repeats the last video frame. This is useful for very short dropouts and was not available before. Signed-off-by: Michael Riedl <michael.riedl@nativewaves.com> --- doc/indevs.texi | 16 ++++++++++++++++ libavdevice/decklink_common.h | 1 + libavdevice/decklink_common_c.h | 7 +++++++ libavdevice/decklink_dec.cpp | 23 ++++++++++++++++++++++- libavdevice/decklink_dec_c.c | 6 +++++- 5 files changed, 51 insertions(+), 2 deletions(-)