[FFmpeg-devel] [PATCH 4/6] ffmpeg: don't skip packets before a keyframe was seen if a bsf with delay is used

[James Almer]

On 2/15/2022 9:03 AM, Anton Khirnov wrote:
> Quoting James Almer (2022-02-15 12:48:09)
>>
>>
>> On 2/15/2022 8:41 AM, Anton Khirnov wrote:
>>> Quoting James Almer (2022-02-14 23:41:54)
>>>> A keyframe could be buffered in the bsf and not be output until more packets
>>>> had been fed to it.
>>>>
>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>> ---
>>>>    fftools/ffmpeg.c | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>>>> index 6aa0986f02..48d9016b4c 100644
>>>> --- a/fftools/ffmpeg.c
>>>> +++ b/fftools/ffmpeg.c
>>>> @@ -2026,7 +2026,8 @@ static void do_streamcopy(InputStream *ist, OutputStream *ost, const AVPacket *p
>>>>        }
>>>>    
>>>>        if ((!ost->frame_number && !(pkt->flags & AV_PKT_FLAG_KEY)) &&
>>>> -        !ost->copy_initial_nonkeyframes)
>>>> +        !ost->copy_initial_nonkeyframes &&
>>>> +        !(ost->bsf_ctx && ost->bsf_ctx->filter->capabilities & AV_BSF_CAP_DELAY))
>>>>            return;
>>>
>>> Wouldn't it be simpler to add an OutputStream field that tracks whether
>>> we've seen a keyframe packet yet? No new API required.
>>
>> Probably. It would also only trigger when a keyframe was seen instead of
>> unconditionally for all delay flagged bsfs.
>>
>> I still think this new API is a good addition, either way. Only a
>> handful of bsfs buffer packets and require the caller to flush them
>> after sending NULL (av1_frame_merge, vp9_superframe, and setts after
>> this set) so library users could have all this time never signaled EOF
>> and never noticed anything wrong, much like it happened here.
>> The presence of this flag might help library users know they really need
>> to signal EOF.
> 
> I don't see where the advantage would be. The callers still need to have
> the flushing code, so might as well always call it.

Then we probably need to enforce it in the doxy, or at least strongly 
suggest it with a @note or @warning line to ensure you get complete output.
Right now it's optional, mentioned as "If you send a NULL packet, it 
will trigger EOF", meaning not doing so is still a valid scenario, and 
there's nothing letting the user know he's got packets stuck in the bsf 
even after receive_packet() returned EAGAIN if they don't.

> 
> The disadvantage for us is more complixity and we have to maintain the
> list of delay BSFs.
>