| Message ID | 20200817095337.89430-1-lq@chinaffmpeg.org |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/hlsenc: check duration value correct before split segment | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
> 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".
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 <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
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 --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;
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(-)