| Message ID | 20200817071146.73636-1-lq@chinaffmpeg.org |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/hlsenc: check the segment duration valid | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
> On Aug 17, 2020, at 4:05 PM, Nicolas George <george@nsup.org> wrote: > > Steven Liu (12020-08-17): >> Output a warning message if the target duration of the segment is negative. >> Suggest user increase the hls_time value, >> and modify the target duration to one packet duration, >> because there maybe have bframe and then split not by keyframe, >> and the segment is very very small. > > Fiddling randomly with durations seem wrong. It will probably result in > invalid seeking. Have you found the exact cause of the issue? > The split_by_time flag which enabled split segments at any position (non-key frame) is the root cause of the issue. The specification (RFC8216, 6.2.1) says explicitly: The server SHOULD attempt to divide the source media at points that support effective decode of individual Media Segments, e.g., on packet and key frame boundaries. > Preventing duration from being negative in the first place would be a > much better fix than adjusting it arbitrarily. > > Regards, > > -- > Nicolas George > > _______________________________________________ > 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 (12020-08-17): > Output a warning message if the target duration of the segment is negative. > Suggest user increase the hls_time value, > and modify the target duration to one packet duration, > because there maybe have bframe and then split not by keyframe, > and the segment is very very small. Fiddling randomly with durations seem wrong. It will probably result in invalid seeking. Have you found the exact cause of the issue? Preventing duration from being negative in the first place would be a much better fix than adjusting it arbitrarily. Regards,
zhilizhao <quinkblack@foxmail.com> 于2020年8月17日周一 下午5:06写道： > > > > > On Aug 17, 2020, at 4:05 PM, Nicolas George <george@nsup.org> wrote: > > > > Steven Liu (12020-08-17): > >> Output a warning message if the target duration of the segment is negative. > >> Suggest user increase the hls_time value, > >> and modify the target duration to one packet duration, > >> because there maybe have bframe and then split not by keyframe, > >> and the segment is very very small. > > > > Fiddling randomly with durations seem wrong. It will probably result in > > invalid seeking. Have you found the exact cause of the issue? > > > > The split_by_time flag which enabled split segments at any position (non-key > frame) is the root cause of the issue. The specification (RFC8216, 6.2.1) > says explicitly: > > The server SHOULD attempt to divide the source media at points that > support effective decode of individual Media Segments, e.g., on > packet and key frame boundaries. When hls muxer have beed support split_by_time flag add, the 8216 have not beed release, and there have some guys order this feature, for example me. And I saw lots of people are using this flag, so this maybe cannot be removed now. So I think give a warning message and modify it to one packet duration is ok. > > > Preventing duration from being negative in the first place would be a > > much better fix than adjusting it arbitrarily. > > > > Regards, > > > > -- > > Nicolas George > > > > _______________________________________________ > > 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". > > _______________________________________________ > 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".
Nicolas George <george@nsup.org> 于2020年8月17日周一 下午4:06写道： > > Steven Liu (12020-08-17): > > Output a warning message if the target duration of the segment is negative. > > Suggest user increase the hls_time value, > > and modify the target duration to one packet duration, > > because there maybe have bframe and then split not by keyframe, > > and the segment is very very small. > > Fiddling randomly with durations seem wrong. It will probably result in > invalid seeking. Have you found the exact cause of the issue? > > Preventing duration from being negative in the first place would be a > much better fix than adjusting it arbitrarily. As Zhili Zhao said, the problem is the user is using hls_time with too short duration and want split not by keyframe. I have been think about how to make the user know it don't support short than 2 frames and with bframes, but this is not the better way. I think give a suggestion and warning message is better. Feel free comment better ways. Thanks Steven
Steven Liu (12020-08-17): > When hls muxer have beed support split_by_time flag add, the 8216 have > not beed release, > and there have some guys order this feature, for example me. > And I saw lots of people are using this flag, so this maybe cannot be > removed now. > So I think give a warning message and modify it to one packet duration is ok. No, it is not ok until we understand properly the problem. Have you observed the timestamps and type of several packets around the issue, plus the places where the muxer chooses to split? If yes, then please show the result; even better include it in the commit message. If no, then your analysis of the issue is way too shallow to judge whether this is a proper fix. Regards,
Steven Liu (12020-08-17): > As Zhili Zhao said, the problem is the user is using hls_time with too > short duration and want split not by keyframe. > I have been think about how to make the user know it don't support > short than 2 frames and with bframes, > but this is not the better way. > I think give a suggestion and warning message is better. > Feel free comment better ways. See my other reply. Regards,
Nicolas George <george@nsup.org> 于2020年8月17日周一 下午5:24写道： > > Steven Liu (12020-08-17): > > When hls muxer have beed support split_by_time flag add, the 8216 have > > not beed release, > > and there have some guys order this feature, for example me. > > And I saw lots of people are using this flag, so this maybe cannot be > > removed now. > > So I think give a warning message and modify it to one packet duration is ok. > > No, it is not ok until we understand properly the problem. > > Have you observed the timestamps and type of several packets around the > issue, plus the places where the muxer chooses to split? Good point, Thanks. > > If yes, then please show the result; even better include it in the > commit message. > > If no, then your analysis of the issue is way too shallow to judge > whether this is a proper fix. > > Regards, New version will submit. Thanks Steven
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index cb31d6aed7..79ee39aa23 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -2512,6 +2512,12 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt) if (vs->start_pos || hls->segment_type != SEGMENT_TYPE_FMP4) { double cur_duration = (double)(pkt->pts - vs->end_pts) * st->time_base.num / st->time_base.den; + if (cur_duration < 0) { + av_log(s, AV_LOG_WARNING, "The target duration incorrect, maybe you splited a too short segment, " + "you should increase the hls_time, " + "now the duration will set to 1 packet duration, but maybe incorrect too.\n"); + cur_duration = vs->duration; + } ret = hls_append_segment(s, hls, vs, cur_duration, vs->start_pos, vs->size); vs->end_pts = pkt->pts; vs->duration = 0;
Output a warning message if the target duration of the segment is negative. Suggest user increase the hls_time value, and modify the target duration to one packet duration, because there maybe have bframe and then split not by keyframe, and the segment is very very small. Signed-off-by: Steven Liu <lq@chinaffmpeg.org> --- libavformat/hlsenc.c | 6 ++++++ 1 file changed, 6 insertions(+)