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

[Anton Khirnov]

Quoting James Almer (2022-02-15 13:12:51)
> 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 doxy for av_bsf_send_packet() already says:
If pkt is empty, it signals the end of the stream and will cause the
filter to output any packets it may have buffered internally.

But I can write something more explicit.

-- 
Anton Khirnov