[FFmpeg-devel] [PATCH] avformat/hlsenc: check the segment duration valid

[Steven Liu]

zhilizhao <quinkblack at foxmail.com> 于2020年8月17日周一 上午11:15写道：
>
>
>
> > On Aug 17, 2020, at 11:02 AM, Steven Liu <lingjiujianke at gmail.com> wrote:
> >
> > zhilizhao <quinkblack at foxmail.com <mailto:quinkblack at foxmail.com>> 于2020年8月17日周一 上午10:54写道：
> >>
> >> Hi Steven,
> >>
> >>> On Aug 17, 2020, at 8:49 AM, Steven Liu <lq at chinaffmpeg.org <mailto:lq at 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 at 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 at ffmpeg.org
> >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>
> >>> To unsubscribe, visit link above, or email
> >>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> >>
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel at ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> >
> > Thanks
> > Steven
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org <mailto:ffmpeg-devel at 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 at ffmpeg.org <mailto:ffmpeg-devel-request at ffmpeg.org> with subject "unsubscribe".
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".