diff mbox series

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

Message ID 20210514194225.412924-1-gustav.grusell@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel] avformat/hls Implement support for using AVSEEK_FLAG_BACKWARD when seeking | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Gustav Grusell May 14, 2021, 7:42 p.m. UTC 
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 #6850.
---
 libavformat/hls.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Gustav Grusell May 24, 2021, 11:06 a.m. UTC | #1 
Den fre 14 maj 2021 21:42Gustav Grusell <gustav.grusell@gmail.com> skrev:

> 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 #6850.
> ---
>  libavformat/hls.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 8fc6924c90..3f1fd2cb70 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -2199,10 +2199,15 @@ static int hls_read_packet(AVFormatContext *s,
> AVPacket *pkt)
>
>                      tb = get_timebase(pls);
>                      ts_diff = av_rescale_rnd(pls->pkt->dts, AV_TIME_BASE,
> -                                            tb.den, AV_ROUND_DOWN) -
> -                            pls->seek_timestamp;
> -                    if (ts_diff >= 0 && (pls->seek_flags  &
> AVSEEK_FLAG_ANY ||
> -                                        pls->pkt->flags &
> AV_PKT_FLAG_KEY)) {
> +                                             tb.den, AV_ROUND_DOWN) -
> +                              pls->seek_timestamp;
> +                    /* 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) ||
> +                        (ts_diff >= 0 && (pls->seek_flags  &
> AVSEEK_FLAG_ANY ||
> +                                          pls->pkt->flags &
> AV_PKT_FLAG_KEY))) {
>                          pls->seek_timestamp = AV_NOPTS_VALUE;
>                          break;
>                      }
>

Did anyone find time to look at this? Any comments?

Cheers,
Gustav

>
Lingjiang Fang May 25, 2021, 2:24 a.m. UTC | #2 
On Fri, 14 May 2021 21:42:25 +0200
Gustav Grusell <gustav.grusell@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 #6850. ---
>  libavformat/hls.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 8fc6924c90..3f1fd2cb70 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -2199,10 +2199,15 @@ static int hls_read_packet(AVFormatContext
> *s, AVPacket *pkt) 
>                      tb = get_timebase(pls);
>                      ts_diff = av_rescale_rnd(pls->pkt->dts,
> AV_TIME_BASE,
> -                                            tb.den, AV_ROUND_DOWN) -
> -                            pls->seek_timestamp;
> -                    if (ts_diff >= 0 && (pls->seek_flags  &
> AVSEEK_FLAG_ANY ||
> -                                        pls->pkt->flags &
> AV_PKT_FLAG_KEY)) {
> +                                             tb.den, AV_ROUND_DOWN) -
> +                              pls->seek_timestamp;
only format change, maybe better to revert if you don't mean to do so
:)

> +                    /* 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) ||
> +                        (ts_diff >= 0 && (pls->seek_flags  &
> AVSEEK_FLAG_ANY ||
> +                                          pls->pkt->flags &
> AV_PKT_FLAG_KEY))) { pls->seek_timestamp = AV_NOPTS_VALUE;
>                          break;
>                      }



Regards,
Lingjiang Fang
徐慧书 May 25, 2021, 8:05 a.m. UTC | #3 
On Sat, May 15, 2021 at 4:11 AM Gustav Grusell <gustav.grusell@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 #6850.
> ---
>  libavformat/hls.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 8fc6924c90..3f1fd2cb70 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -2199,10 +2199,15 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>
>                      tb = get_timebase(pls);
>                      ts_diff = av_rescale_rnd(pls->pkt->dts, AV_TIME_BASE,
> -                                            tb.den, AV_ROUND_DOWN) -
> -                            pls->seek_timestamp;
> -                    if (ts_diff >= 0 && (pls->seek_flags  & AVSEEK_FLAG_ANY ||
> -                                        pls->pkt->flags & AV_PKT_FLAG_KEY)) {
> +                                             tb.den, AV_ROUND_DOWN) -
> +                              pls->seek_timestamp;
> +                    /* 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) ||
> +                        (ts_diff >= 0 && (pls->seek_flags  & AVSEEK_FLAG_ANY ||
> +                                          pls->pkt->flags & AV_PKT_FLAG_KEY))) {
>                          pls->seek_timestamp = AV_NOPTS_VALUE;
>                          break;
>                      }



I think you can put your code above this line ( tb =
get_timebase(pls). )  to avoid invalid code execution and keep the
code aligned

Regards,
bevis
Gustav Grusell May 25, 2021, 12:07 p.m. UTC | #4 
On Tue, May 25, 2021 at 4:24 AM Lingjiang Fang <vacingfang@foxmail.com>
wrote:

>
> only format change, maybe better to revert if you don't mean to do so
> :)
>
>
Thanks, will fix.

On Tue, May 25, 2021 at 10:30 AM 徐慧书 <javashu2012@gmail.com> wrote:

> On Sat, May 15, 2021 at 4:11 AM Gustav Grusell <gustav.grusell@gmail.com>
> wrote:
>
> >
> >                      tb = get_timebase(pls);
> >                      ts_diff = av_rescale_rnd(pls->pkt->dts,
> AV_TIME_BASE,
> > -                                            tb.den, AV_ROUND_DOWN) -
> > -                            pls->seek_timestamp;
> > -                    if (ts_diff >= 0 && (pls->seek_flags  &
> AVSEEK_FLAG_ANY ||
> > -                                        pls->pkt->flags &
> AV_PKT_FLAG_KEY)) {
> > +                                             tb.den, AV_ROUND_DOWN) -
> > +                              pls->seek_timestamp;
> > +                    /* 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) ||
> > +                        (ts_diff >= 0 && (pls->seek_flags  &
> AVSEEK_FLAG_ANY ||
> > +                                          pls->pkt->flags &
> AV_PKT_FLAG_KEY))) {
> >                          pls->seek_timestamp = AV_NOPTS_VALUE;
> >                          break;
> >                      }
>
> I think you can put your code above this line ( tb =
> get_timebase(pls). )  to avoid invalid code execution and keep the
> code aligned
>
> Good suggestion, I will do so and provide a new patch.

Regards,
Gustav
diff mbox series

Patch

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 8fc6924c90..3f1fd2cb70 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -2199,10 +2199,15 @@  static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
 
                     tb = get_timebase(pls);
                     ts_diff = av_rescale_rnd(pls->pkt->dts, AV_TIME_BASE,
-                                            tb.den, AV_ROUND_DOWN) -
-                            pls->seek_timestamp;
-                    if (ts_diff >= 0 && (pls->seek_flags  & AVSEEK_FLAG_ANY ||
-                                        pls->pkt->flags & AV_PKT_FLAG_KEY)) {
+                                             tb.den, AV_ROUND_DOWN) -
+                              pls->seek_timestamp;
+                    /* 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) ||
+                        (ts_diff >= 0 && (pls->seek_flags  & AVSEEK_FLAG_ANY ||
+                                          pls->pkt->flags & AV_PKT_FLAG_KEY))) {
                         pls->seek_timestamp = AV_NOPTS_VALUE;
                         break;
                     }