Re: [FFmpeg-devel] [PATCH] avformat/hls:use EXT-X-START instead of live_start_index if it's in playlist

[Steven Liu <lq@chinaffmpeg.org>]

> 在 2022年6月25日，13:24，Li Kai <wolfleekay@gmail.com> 写道：
> 
> Ok, I add allowed_x_start option, and the option default is 0. So current
What about use “prefer_x_start” for option name.

BTW, add option descriptions into document doc/demuxers.texi


+        } else if (av_strstart(line, "#EXT-X-START:", &ptr)) {
+            ret = ensure_playlist(c, &pls, url);
+            if (ret < 0) {
+                goto fail;
+            }
+            if (av_strstart(ptr, "TIME-OFFSET=", &p)) {
+                float offset = strtof(p, NULL);
+                pls->start_time_offset = offset * AV_TIME_BASE;
+                pls->time_offset_flag = 1;
+            } else {
+                goto fail;
+            }

should this logic goto fail? 
why don’t output a warning and continue try to use live_start_index? Or maybe there have some reason, are they?
> users will not be affected.
> please review again.
> 
> At last, Thanks your tips.
> 
> Steven Liu <lq@chinaffmpeg.org> 于2022年6月25日周六 12:51写道：
> 
>> 
>> 
>>> 在 2022年6月25日，12:32，少宇李 <wolfleekay@gmail.com> 写道：
>>> 
>>> Hi Steven,
>>> 
>>> Please review code.
>>> 
>>> I changed code, but I still have a question.
>>> If EXT-X-START tag in playlist and users set live_start_index, the
>>> live_start_index will not work
>>> for current codes, which makes maybe users confused.
>>> Is that OK?
>> for current codes, user cannot aware there have EXT-X-START tag in the
>> playlist, so i think we should reserve the old use method.
>> or maybe you can add an option for user to decide which way they can use
>> (either EXT-X-START or live_start_index).
>> Because there should have many people using live_start_index for 'time
>> shift’ by player (user control),
>> 
>>> 
>>> In other hand, if make live_start_index valid when EXT-X-START tag in
>>> playlist, maybe live_start_index needs a flag, or
>>> make live_start_index default value is INT_MIN to distinguish.
>>> becuase the live_start_index default value is -3, I have no idea to
>>> distinguish users action or
>>> default value when live_start_index=-3.
>> yes cannot distinguish users action, but user should can decided use
>> live_start_index or EXT-X-START.
>> 
>>> 
>>> May I need to set default value=INT_MIN as live_start_index default
>> value?
>>> And only when does not exist EXT-X-START
>>> tag in playlist, make live_start_index = -3 or users's configuration
>> value.
>>> I have no idea which is better~
>>> 
>>> Thanks a lot!
>>> 
>>> Steven Liu <lingjiujianke@gmail.com> 于2022年6月24日周五 14:21写道：
>>> 
>>>> 少宇李 <wolfleekay@gmail.com> 于2022年6月24日周五 11:58写道：
>>>>>> 
>>>>>> 
>>>>>> Hi, Steven
>>>>> 
>>>>> 
>>>>> I got it.
>>>>> 
>>>>> User could use "live_start_index" option even if #EXT-X-START in the
>>>> playlist.
>>>>> 
>>>>> I changed the priority about #EXT-X-START/live_start_index,
>>>>> 
>>>>> this is new bellow:
>>>>> 
>>>>> ---
>>>>> libavformat/hls.c | 69 +++++++++++++++++++++++++++++++++++++++++++----
>>>>> 1 file changed, 64 insertions(+), 5 deletions(-)
>>>>> 
>>>>> diff --git a/libavformat/hls.c b/libavformat/hls.c
>>>>> index b736f093a9..309471efce 100644
>>>>> --- a/libavformat/hls.c
>>>>> +++ b/libavformat/hls.c
>>>>> @@ -120,6 +120,8 @@ struct playlist {
>>>>>    enum PlaylistType type;
>>>>>    int64_t target_duration;
>>>>>    int64_t start_seq_no;
>>>>> +    int time_offset_flag;
>>>>> +    int64_t start_time_offset;
>>>>>    int n_segments;
>>>>>    struct segment **segments;
>>>>>    int needed;
>>>>> @@ -741,6 +743,7 @@ static int parse_playlist(HLSContext *c, const char
>>>> *url,
>>>>>    struct segment **prev_segments = NULL;
>>>>>    int prev_n_segments = 0;
>>>>>    int64_t prev_start_seq_no = -1;
>>>>> +    const char *p;
>>>>> 
>>>>>    if (is_http && !in && c->http_persistent && c->playlist_pb) {
>>>>>        in = c->playlist_pb;
>>>>> @@ -889,6 +892,18 @@ static int parse_playlist(HLSContext *c, const
>> char
>>>> *url,
>>>>>                cur_init_section->key = NULL;
>>>>>            }
>>>>> 
>>>>> +        } else if (av_strstart(line, "#EXT-X-START:", &ptr)) {
>>>>> +            ret = ensure_playlist(c, &pls, url);
>>>>> +            if (ret < 0) {
>>>>> +                goto fail;
>>>>> +            }
>>>>> +            if (av_strstart(ptr, "TIME-OFFSET=", &p)) {
>>>>> +                float offset = strtof(p, NULL);
>>>>> +                pls->start_time_offset = offset * AV_TIME_BASE;
>>>>> +                pls->time_offset_flag = 1;
>>>>> +            } else {
>>>>> +                goto fail;
>>>>> +            }
>>>>>        } else if (av_strstart(line, "#EXT-X-ENDLIST", &ptr)) {
>>>>>            if (pls)
>>>>>                pls->finished = 1;
>>>>> @@ -1721,10 +1736,54 @@ static int64_t select_cur_seq_no(HLSContext *c,
>>>> struct playlist *pls)
>>>>> 
>>>>>        /* If this is a live stream, start live_start_index segments
>>>> from the
>>>>>         * start or end */
>>>>> -        if (c->live_start_index < 0)
>>>>> -            return pls->start_seq_no + FFMAX(pls->n_segments +
>>>> c->live_start_index, 0);
>>>>> -        else
>>>>> -            return pls->start_seq_no + FFMIN(c->live_start_index,
>>>> pls->n_segments - 1);
>>>>> +        if (c->live_start_index != INT_MIN) {
>>>> Maybe you should not change current logic.
>>>> You can just add  about process EXT-X-START sence,.
>>>> For example:
>>>> compute the start_seq_no and save the value to a temp varible and
>>>> don't return immediately, check the third scence which get EXT-X-START
>>>> tag from playlist.
>>>> process the temp varible after start_seq_no compute from
>>>> live_start_index or EXT-X-START.
>>>> 
>>>>> +            if (c->live_start_index < 0)
>>>>> +                return pls->start_seq_no + FFMAX(pls->n_segments +
>>>>> +                                                c->live_start_index,
>> 0);
>>>>> +            else
>>>>> +                return pls->start_seq_no + FFMIN(c->live_start_index,
>>>>> +                                                pls->n_segments - 1);
>>>>> +        } else if (c->live_start_index == INT_MIN &&
>>>> !pls->time_offset_flag) {
>>>>> +            return pls->start_seq_no + FFMAX(pls->n_segments - 3, 0);
>>>>> +        } else {
>>>>> +            /* If playlist indicate a TIME-OFFSET and user not use
>>>> live_start_index,
>>>>> +             * need to recalculate seq_no */
>>>>> +            int i;
>>>>> +            int64_t start_timestamp;
>>>>> +            int64_t playlist_duration = 0;
>>>>> +            int64_t cur_timestamp = c->cur_timestamp == AV_NOPTS_VALUE
>>>> ? 0 :
>>>>> +                                    c->cur_timestamp;
>>>>> +
>>>>> +            for (i = 0; i < pls->n_segments; i++) {
>>>>> +                playlist_duration += pls->segments[i]->duration;
>>>>> +            }
>>>>> +            /* If the absolute value of TIME-OFFSET exceeds
>>>>> +             * the duration of the playlist, it indicates either the
>>>> end of the
>>>>> +             * playlist (if positive) or the beginning of the playlist
>>>> (if
>>>>> +             * negative). */
>>>>> +            if (pls->start_time_offset >=0 &&
>>>>> +                pls->start_time_offset > playlist_duration)
>>>>> +                start_timestamp = cur_timestamp + playlist_duration;
>>>>> +            else if (pls->start_time_offset >= 0 &&
>>>>> +                        pls->start_time_offset <= playlist_duration)
>>>>> +                start_timestamp = cur_timestamp +
>>>> pls->start_time_offset;
>>>>> +            else if (pls->start_time_offset < 0 &&
>>>>> +                        pls->start_time_offset < -playlist_duration)
>>>>> +                start_timestamp = cur_timestamp;
>>>>> +            else if (pls->start_time_offset < 0 &&
>>>>> +                        pls->start_time_offset > -playlist_duration)
>>>>> +                start_timestamp = cur_timestamp + playlist_duration +
>>>>> +                                    pls->start_time_offset;
>>>>> +            else
>>>>> +                start_timestamp = cur_timestamp;
>>>>> +
>>>>> +            find_timestamp_in_playlist(c, pls, start_timestamp,
>>>> &seq_no, NULL);
>>>>> +            av_log(c, AV_LOG_DEBUG, "start_timestamp: %" PRId64
>>>>> +                                    "cur_timestamp:%" PRId64
>>>>> +                                    "cur_seq_no:%" PRId64 "\n",
>>>>> +                                    start_timestamp, cur_timestamp,
>>>> seq_no);
>>>>> +            return seq_no;
>>>>> +        }
>>>>>    }
>>>>> 
>>>>>    /* Otherwise just start on the first segment. */
>>>>> @@ -2475,7 +2534,7 @@ static int hls_probe(const AVProbeData *p)
>>>>> #define FLAGS AV_OPT_FLAG_DECODING_PARAM
>>>>> static const AVOption hls_options[] = {
>>>>>    {"live_start_index", "segment index to start live streams at
>>>> (negative values are from the end)",
>>>>> -        OFFSET(live_start_index), AV_OPT_TYPE_INT, {.i64 = -3},
>>>> INT_MIN, INT_MAX, FLAGS},
>>>>> +        OFFSET(live_start_index), AV_OPT_TYPE_INT, {.i64 = INT_MIN},
>>>> INT_MIN, INT_MAX, FLAGS},
>>>> This is unnecessary， because the default -3 is ok and be using by
>>>> users from many years ago.
>>>> 
>>>>>    {"allowed_extensions", "List of file extensions that hls is allowed
>>>> to access",
>>>>>        OFFSET(allowed_extensions), AV_OPT_TYPE_STRING,
>>>>>        {.str =
>>>> 
>> "3gp,aac,avi,ac3,eac3,flac,mkv,m3u8,m4a,m4s,m4v,mpg,mov,mp2,mp3,mp4,mpeg,mpegts,ogg,ogv,oga,ts,vob,wav"},
>>>>> --
>>>>> 2.24.1 (Apple Git-126)
>>>>> 
>>>>> 
>>>>> Thanks Steven
>>>> 
>>> 
>> <0001-avformat-hls-use-EXT-X-START-instead-of-live_start_i.patch>_______________________________________________
>>> 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
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> 
> <v2-0001-avformat-hls-add-EXT-X-START-tag-support-by-allow.patch>_______________________________________________
> 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
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".