[FFmpeg-devel] [PATCH v2 1/1] libavdevice/decklink: extend available actions on signal loss

[Michael Riedl]

On 5/14/24 21:39, Marton Balint wrote:
>
>
> On Tue, 14 May 2024, 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.
>>>
>>> As far as I see, you are overriding frameBytes for a repeated frame, that seems wrong. pkt.data (frameBytes) must be associated with the videoFrame which is passed to av_buffer_create() later on.
>>>
>>> Every AVFrame returned by the decklink device has an AVBuffer set up which
>>> keeps a reference to the original DeckLink frame. This allows the use of the DeckLink frame's raw buffer directly. But you cannot use the raw buffer of another DeckLink frame for which the AVBuffer of the AVFrame does not keep a reference.
>>
>> Thank you for your feedback!
>>
>> I took another look at the code and revisited the DeckLink documentation to ensure my understanding was correct. It seems that frameBytes is a pointer to the buffer of an IDeckLinkVideoFrame, and it remains valid as long as the videoFrame is not released.
>
> That is just it. You are releasing the repeated frame as soon as a valid frame comes in. The AVPacket data you previously returned will still point to the now released frameBytes. As I wrote above, the decklink frame corresponding to the returned frameBytes must be released in the destructor of the AVPacket buffer.

Took me a while to understand the issue, thank you for pointing that out. If I understand it correctly, the issue is fixed when the AVBuffer references the corresponding video frame that manages frameBytes. Does the change below look good to you?


diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index e10fd5d6569..a6f9c4e0b3c 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -734,6 +734,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
     BMDTimeValue frameDuration;
     int64_t wallclock = 0, abs_wallclock = 0;
     struct decklink_cctx *cctx = (struct decklink_cctx *) avctx->priv_data;
+    IDeckLinkVideoInputFrame *currentVideoFrame = videoFrame; // video frame that holds frameBytes
 
     if (ctx->autodetect) {
         if (videoFrame && !(videoFrame->GetFlags() & bmdFrameHasNoInputSource) &&
@@ -790,7 +791,8 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
                         *p++ = bars[(x * 8) / width];
                 }
             } else if (ctx->signal_loss_action == SIGNAL_LOSS_REPEAT) {
-                last_video_frame->GetBytes(&frameBytes);
+                currentVideoFrame = last_video_frame;
+                currentVideoFrame->GetBytes(&frameBytes);
             }
 
             if (!no_video) {
@@ -866,8 +868,8 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
         pkt.flags       |= AV_PKT_FLAG_KEY;
         pkt.stream_index = ctx->video_st->index;
         pkt.data         = (uint8_t *)frameBytes;
-        pkt.size         = videoFrame->GetRowBytes() *
-                           videoFrame->GetHeight();
+        pkt.size         = currentVideoFrame->GetRowBytes() *
+                           currentVideoFrame->GetHeight();
         //fprintf(stderr,"Video Frame size %d ts %d\n", pkt.size, pkt.pts);
 
         if (!no_video) {
@@ -943,9 +945,9 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
             }
         }
 
-        pkt.buf = av_buffer_create(pkt.data, pkt.size, decklink_object_free, videoFrame, 0);
+        pkt.buf = av_buffer_create(pkt.data, pkt.size, decklink_object_free, currentVideoFrame, 0);
         if (pkt.buf)
-            videoFrame->AddRef();
+            currentVideoFrame->AddRef();
 
         if (ff_decklink_packet_queue_put(&ctx->queue, &pkt) < 0) {
             ++ctx->dropped;