diff mbox series

[FFmpeg-devel] avformat/hlsenc: check duration value correct before split segment

Message ID 20200817095337.89430-1-lq@chinaffmpeg.org
State New
Headers show
Series [FFmpeg-devel] avformat/hlsenc: check duration value correct before split segment
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Steven Liu Aug. 17, 2020, 9:53 a.m. UTC
fix ticket: 8847
check the timestamps of current packets minus last segment end_pts
if the duration is negative, plus the places where the muxer should split.

Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavformat/hlsenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

zhilizhao(赵志立) Aug. 16, 2020, 11:50 p.m. UTC | #1
> On Aug 17, 2020, at 5:53 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
> 
> fix ticket: 8847
> check the timestamps of current packets minus last segment end_pts
> if the duration is negative, plus the places where the muxer should split.
> 
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
> libavformat/hlsenc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index cb31d6aed7..3564c68732 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -2398,9 +2398,9 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>                 vs->duration = (double)(pkt->pts - vs->end_pts) * st->time_base.num / st->time_base.den;
>             }
>         }
> -
>     }
> 
> +    can_split = pkt->pts - vs->end_pts  < 0 ? 0 : 1;

If can_split is 0 already, it can be set to 1 here. How about

can_split &&= (pkt->pts - vs->end_pts > 0)

Segment duration should be positive in my opinion, not only non-negative.

>     if (vs->packets_written && can_split && av_compare_ts(pkt->pts - vs->start_pts, st->time_base,
>                                                           end_pts, AV_TIME_BASE_Q) >= 0) {
>         int64_t new_start_pos;
> -- 
> 2.25.0
> 
> 
> 
> _______________________________________________
> 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".
Steven Liu Aug. 17, 2020, 11:52 a.m. UTC | #2
zhilizhao <quinkblack@foxmail.com> 于2020年8月17日周一 下午7:50写道:
>
>
>
> > On Aug 17, 2020, at 5:53 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
> >
> > fix ticket: 8847
> > check the timestamps of current packets minus last segment end_pts
> > if the duration is negative, plus the places where the muxer should split.
> >
> > Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> > ---
> > libavformat/hlsenc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > index cb31d6aed7..3564c68732 100644
> > --- a/libavformat/hlsenc.c
> > +++ b/libavformat/hlsenc.c
> > @@ -2398,9 +2398,9 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
> >                 vs->duration = (double)(pkt->pts - vs->end_pts) * st->time_base.num / st->time_base.den;
> >             }
> >         }
> > -
> >     }
> >
> > +    can_split = pkt->pts - vs->end_pts  < 0 ? 0 : 1;
>
> If can_split is 0 already, it can be set to 1 here. How about
>
> can_split &&= (pkt->pts - vs->end_pts > 0)
>
> Segment duration should be positive in my opinion, not only non-negative.
Good advice, Thanks
>
> >     if (vs->packets_written && can_split && av_compare_ts(pkt->pts - vs->start_pts, st->time_base,
> >                                                           end_pts, AV_TIME_BASE_Q) >= 0) {
> >         int64_t new_start_pos;
> > --
> > 2.25.0
> >
> >
> >


Thanks
Steven
Steven Liu Aug. 17, 2020, 11:56 a.m. UTC | #3
Steven Liu <lingjiujianke@gmail.com> 于2020年8月17日周一 下午7:52写道:
>
> zhilizhao <quinkblack@foxmail.com> 于2020年8月17日周一 下午7:50写道:
> >
> >
> >
> > > On Aug 17, 2020, at 5:53 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
> > >
> > > fix ticket: 8847
> > > check the timestamps of current packets minus last segment end_pts
> > > if the duration is negative, plus the places where the muxer should split.
> > >
> > > Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> > > ---
> > > libavformat/hlsenc.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > > index cb31d6aed7..3564c68732 100644
> > > --- a/libavformat/hlsenc.c
> > > +++ b/libavformat/hlsenc.c
> > > @@ -2398,9 +2398,9 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
> > >                 vs->duration = (double)(pkt->pts - vs->end_pts) * st->time_base.num / st->time_base.den;
> > >             }
> > >         }
> > > -
> > >     }
> > >
> > > +    can_split = pkt->pts - vs->end_pts  < 0 ? 0 : 1;
> >
> > If can_split is 0 already, it can be set to 1 here. How about
> >
> > can_split &&= (pkt->pts - vs->end_pts > 0)
> >
> > Segment duration should be positive in my opinion, not only non-negative.
Hello Nicolas,
    Do you have other comments?

> Good advice, Thanks
> >
> > >     if (vs->packets_written && can_split && av_compare_ts(pkt->pts - vs->start_pts, st->time_base,
> > >                                                           end_pts, AV_TIME_BASE_Q) >= 0) {
> > >         int64_t new_start_pos;
> > > --
> > > 2.25.0
> > >
> > >
> > >
>
>
> Thanks
> Steven
Nicolas George Aug. 17, 2020, 12:35 p.m. UTC | #4
Steven Liu (12020-08-17):
> Hello Nicolas,
>     Do you have other comments?

This solution seems much better (except that &&= does not exist IIRC).
But I would be happier if there was an analysis of the problem in the
commit message.

Also, please leave other people who know the code better some time to
comment, since the fix was not straightforward.

Regards,
diff mbox series

Patch

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index cb31d6aed7..3564c68732 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -2398,9 +2398,9 @@  static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
                 vs->duration = (double)(pkt->pts - vs->end_pts) * st->time_base.num / st->time_base.den;
             }
         }
-
     }
 
+    can_split = pkt->pts - vs->end_pts  < 0 ? 0 : 1;
     if (vs->packets_written && can_split && av_compare_ts(pkt->pts - vs->start_pts, st->time_base,
                                                           end_pts, AV_TIME_BASE_Q) >= 0) {
         int64_t new_start_pos;