| Message ID | 20200817004944.62003-1-lq@chinaffmpeg.org |
|---|---|
| State | Superseded |
| 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 |
Hi Steven, > On Aug 17, 2020, at 8:49 AM, Steven Liu <lq@chinaffmpeg.org> wrote: > > output a warning message if the target duration of the segment. The sentence is incomplete. > 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. The warning message is very helpful, modify the target duration is less helpful IMHO. The generated files are broken and cannot be fixed. Let the user see what’s going wrong from the log messages or from the m3u8 files, and fix the usage problem. > > Signed-off-by: Steven Liu <lq@chinaffmpeg.org> > --- > libavformat/hlsenc.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > index cb31d6aed7..76d59f5f79 100644 > --- a/libavformat/hlsenc.c > +++ b/libavformat/hlsenc.c > @@ -2512,6 +2512,11 @@ 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, "duration < 0, maybe you splited a too short segment, " > + "the duration will set to 1 packet duration.\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; > -- > 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".
> On Aug 17, 2020, at 11:02 AM, Steven Liu <lingjiujianke@gmail.com> wrote: > > zhilizhao <quinkblack@foxmail.com <mailto:quinkblack@foxmail.com>> 于2020年8月17日周一 上午10:54写道： >> >> Hi Steven, >> >>> On Aug 17, 2020, at 8:49 AM, Steven Liu <lq@chinaffmpeg.org <mailto:lq@chinaffmpeg.org>> wrote: >>> >>> output a warning message if the target duration of the segment. >> >> The sentence is incomplete. > Ah ,yes, should > "Output a warning message if the target duration of the segment is negative." >> >>> 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. >> >> The warning message is very helpful, modify the target duration is less helpful >> IMHO. The generated files are broken and cannot be fixed. Let the user see what’s >> going wrong from the log messages or from the m3u8 files, and fix the usage >> problem. > What about return a suggestion and give an error message? > "duration < 0, maybe you splited a too short segment, you should split > an keyframe if too short.”? I suggest naming the option (“hls_time” and “hls_segment_size” maybe?) in the message so the user not only know what’s going wrong but also how to fix it. >> >>> >>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org> >>> --- >>> libavformat/hlsenc.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c >>> index cb31d6aed7..76d59f5f79 100644 >>> --- a/libavformat/hlsenc.c >>> +++ b/libavformat/hlsenc.c >>> @@ -2512,6 +2512,11 @@ 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, "duration < 0, maybe you splited a too short segment, " >>> + "the duration will set to 1 packet duration.\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; >>> -- >>> 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". >> >> _______________________________________________ >> 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". > > Thanks > Steven > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel> > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
> On Aug 17, 2020, at 11:18 AM, Steven Liu <lingjiujianke@gmail.com> wrote: > > zhilizhao <quinkblack@foxmail.com <mailto:quinkblack@foxmail.com>> 于2020年8月17日周一 上午11:15写道： >> >> >> >>> On Aug 17, 2020, at 11:02 AM, Steven Liu <lingjiujianke@gmail.com <mailto:lingjiujianke@gmail.com>> wrote: >>> >>> zhilizhao <quinkblack@foxmail.com <mailto:quinkblack@foxmail.com> <mailto:quinkblack@foxmail.com <mailto:quinkblack@foxmail.com>>> 于2020年8月17日周一 上午10:54写道： >>>> >>>> Hi Steven, >>>> >>>>> On Aug 17, 2020, at 8:49 AM, Steven Liu <lq@chinaffmpeg.org <mailto:lq@chinaffmpeg.org> <mailto:lq@chinaffmpeg.org <mailto:lq@chinaffmpeg.org>>> wrote: >>>>> >>>>> output a warning message if the target duration of the segment. >>>> >>>> The sentence is incomplete. >>> Ah ,yes, should >>> "Output a warning message if the target duration of the segment is negative." >>>> >>>>> 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. >>>> >>>> The warning message is very helpful, modify the target duration is less helpful >>>> IMHO. The generated files are broken and cannot be fixed. Let the user see what’s >>>> going wrong from the log messages or from the m3u8 files, and fix the usage >>>> problem. >>> What about return a suggestion and give an error message? >>> "duration < 0, maybe you splited a too short segment, you should split >>> an keyframe if too short.”? >> >> I suggest naming the option (“hls_time” and “hls_segment_size” maybe?) in the message > I cannot understand the mean "naming the option", > user using hls_time, and hls_flag split_by_time (this should split at > non-keyframe), > so maybe user use a wrong way when use the flag. I mean mention which option should be changed, something like: "try increase hls_time or disable split_by_time flag". > >> so the user not only know what’s going wrong but also how to fix it. >> >>>> >>>>> >>>>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org> >>>>> --- >>>>> libavformat/hlsenc.c | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c >>>>> index cb31d6aed7..76d59f5f79 100644 >>>>> --- a/libavformat/hlsenc.c >>>>> +++ b/libavformat/hlsenc.c >>>>> @@ -2512,6 +2512,11 @@ 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, "duration < 0, maybe you splited a too short segment, " >>>>> + "the duration will set to 1 packet duration.\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; >>>>> -- >>>>> 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". >>>> >>>> _______________________________________________ >>>> 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". >>> >>> Thanks >>> Steven >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> <mailto:ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>> >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel> <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>> >>> >>> To unsubscribe, visit link above, or email >>> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> <mailto:ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org>> with subject "unsubscribe". >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel> >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel> > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
zhilizhao <quinkblack@foxmail.com> 于2020年8月17日周一 上午10:54写道： > > Hi Steven, > > > On Aug 17, 2020, at 8:49 AM, Steven Liu <lq@chinaffmpeg.org> wrote: > > > > output a warning message if the target duration of the segment. > > The sentence is incomplete. Ah ,yes, should "Output a warning message if the target duration of the segment is negative." > > > 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. > > The warning message is very helpful, modify the target duration is less helpful > IMHO. The generated files are broken and cannot be fixed. Let the user see what’s > going wrong from the log messages or from the m3u8 files, and fix the usage > problem. What about return a suggestion and give an error message? "duration < 0, maybe you splited a too short segment, you should split an keyframe if too short."? > > > > > Signed-off-by: Steven Liu <lq@chinaffmpeg.org> > > --- > > libavformat/hlsenc.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > > index cb31d6aed7..76d59f5f79 100644 > > --- a/libavformat/hlsenc.c > > +++ b/libavformat/hlsenc.c > > @@ -2512,6 +2512,11 @@ 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, "duration < 0, maybe you splited a too short segment, " > > + "the duration will set to 1 packet duration.\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; > > -- > > 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". > > _______________________________________________ > 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". Thanks Steven
zhilizhao <quinkblack@foxmail.com> 于2020年8月17日周一 上午11:15写道： > > > > > On Aug 17, 2020, at 11:02 AM, Steven Liu <lingjiujianke@gmail.com> wrote: > > > > zhilizhao <quinkblack@foxmail.com <mailto:quinkblack@foxmail.com>> 于2020年8月17日周一 上午10:54写道： > >> > >> Hi Steven, > >> > >>> On Aug 17, 2020, at 8:49 AM, Steven Liu <lq@chinaffmpeg.org <mailto:lq@chinaffmpeg.org>> wrote: > >>> > >>> output a warning message if the target duration of the segment. > >> > >> The sentence is incomplete. > > Ah ,yes, should > > "Output a warning message if the target duration of the segment is negative." > >> > >>> 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. > >> > >> The warning message is very helpful, modify the target duration is less helpful > >> IMHO. The generated files are broken and cannot be fixed. Let the user see what’s > >> going wrong from the log messages or from the m3u8 files, and fix the usage > >> problem. > > What about return a suggestion and give an error message? > > "duration < 0, maybe you splited a too short segment, you should split > > an keyframe if too short.”? > > I suggest naming the option (“hls_time” and “hls_segment_size” maybe?) in the message I cannot understand the mean "naming the option", user using hls_time, and hls_flag split_by_time (this should split at non-keyframe), so maybe user use a wrong way when use the flag. > so the user not only know what’s going wrong but also how to fix it. > > >> > >>> > >>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org> > >>> --- > >>> libavformat/hlsenc.c | 5 +++++ > >>> 1 file changed, 5 insertions(+) > >>> > >>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > >>> index cb31d6aed7..76d59f5f79 100644 > >>> --- a/libavformat/hlsenc.c > >>> +++ b/libavformat/hlsenc.c > >>> @@ -2512,6 +2512,11 @@ 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, "duration < 0, maybe you splited a too short segment, " > >>> + "the duration will set to 1 packet duration.\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; > >>> -- > >>> 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". > >> > >> _______________________________________________ > >> 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". > > > > Thanks > > Steven > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel> > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org <mailto: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".
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index cb31d6aed7..76d59f5f79 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -2512,6 +2512,11 @@ 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, "duration < 0, maybe you splited a too short segment, " + "the duration will set to 1 packet duration.\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. 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 | 5 +++++ 1 file changed, 5 insertions(+)