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

From: Michael Riedl <michael.riedl@nativewaves.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH v2 1/1] libavdevice/decklink: extend available actions on signal loss
Date: Tue, 14 May 2024 11:42:44 +0200
Message-ID: <6c4c27be-ac9b-4351-be6a-b3dc347843f1@nativewaves.com> (raw)
In-Reply-To: <9e94a07b-21ac-50ad-1572-ac9327974b97@passwd.hu>

>
>> 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. To handle this, I add a reference to the DeckLink videoFrame to keep it valid and then release it (decreasing the reference counter) when it's no longer needed. Updating frameBytes multiple times should be okay since it just points to the raw frame buffer.

I ran some tests using Valgrind with and without the repeat option. I found a memory leak introduced by the patch because the destructor of decklink_input_callback is never called, which leaves one last video frame unreleased.

I'll work on fixing this memory leak and send an update soon. If you have any more comments or concerns, please let me know.

Best regards,
Michael Riedl

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".