[FFmpeg-devel] [PATCH v2] avformat/hls Implement support for using AVSEEK_FLAG_BACKWARD when seeking

[Gustav Grusell]

On Fri, Jun 11, 2021 at 6:29 PM Lingjiang Fang <vacingfang at foxmail.com>
wrote:

> On Sat,  5 Jun 2021 22:42:26 +0200
> Gustav Grusell <gustav.grusell at gmail.com> wrote:
>
> > Before, seeking in hls streams would always seek to the next keyframe
> > after the given timestamp. With this fix, if AVSEEK_FLAG_BACKWARD is
> > set, seeking will be to the first keyframe of the segment containing
> > the given timestamp. This fixes #7485.
> >
> > Signed-off-by: Gustav Grusell <gustav.grusell at gmail.com>
> > ---
> >  libavformat/hls.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > index 8fc6924c90..fb8c9b071a 100644
> > --- a/libavformat/hls.c
> > +++ b/libavformat/hls.c
> > @@ -2197,6 +2197,15 @@ static int hls_read_packet(AVFormatContext *s,
> > AVPacket *pkt) break;
> >                      }
> >
> > +                    /* If AVSEEK_FLAG_BACKWARD set and not
> > AVSEEK_FLAG_ANY,
> > +                     * we return the first keyframe encountered */
> > +                    if (pls->seek_flags & AVSEEK_FLAG_BACKWARD &&
> > +                        !(pls->seek_flags & AVSEEK_FLAG_ANY) &&
> > +                        pls->pkt->flags & AV_PKT_FLAG_KEY) {
> taking account of the case of using flag backward and flag any together,
> a logic like this is better:
>
> if ( pls->seek_flags & AVSEEK_FLAG_BACKWARD &&
>     (pls->seek_flags & AVSEEK_FLAG_ANY ||
>      pls->pkt->flags & AV_PKT_FLAG_KEY)) {
>    ....
> }
>
>
Thanks for your feedback!
I'm not sure about this, I think it makes more sense if seeking with
AVSEEK_FLAG_BACKWARD and AVSEEK_FLAG_ANY behaves the same as when using
only AVSEEK_FLAG_ANY, that is to seek to as close as possible as the exact
timestamp, regardless if that is a keyframe or not. If I'm not mistaken,
with your proposed change, using both AVSEEK_FLAG_BACKWARD and
AVSEEK_FLAG_ANY would always seek to the first frame of the segment which
is always a keyframe, so behaviour would be the same as using only
AVSEEK_FLAG_BACKWARD.


> > +                        pls->seek_timestamp = AV_NOPTS_VALUE;
> > +                        break;
> > +                    }
> > +
> >                      tb = get_timebase(pls);
> >                      ts_diff = av_rescale_rnd(pls->pkt->dts,
> > AV_TIME_BASE, tb.den, AV_ROUND_DOWN) -
>
> BTW, when I used seek backward before, what I want is to get exactly
> packets of the specified seek time, not only the specified stream
> but also all streams I need. To achieve this, I will put this patch
> lines before this if block, like this:
>
> if (pls->seek_timestamp == AV_NOPTS_VALUE)
>     break;
>
> if ( pls->seek_flags & AVSEEK_FLAG_BACKWARD &&
>     (pls->seek_flags & AVSEEK_FLAG_ANY ||
>      pls->pkt->flags & AV_PKT_FLAG_KEY)) {
>    ....
> }
>
> if (pls->seek_stream_index < 0 ||
> ...
>
>
Not sure I'm following. If I'm not mistaken, the hls spec does not require
audio and video segments to be aligned. Given this, it seems to me that the
above proposed change could lead to incorrect seek for some streams when
using AVSEEK_FLAG_BACKWARDS, since we would get the first frame of the
segments for all streams, and for audio streams this might not match the
timestamp of the first segment of the video stream.

Cheers,
Gustav Grusell