| Message ID | 20220623150040.1981341-1-wolfleekay@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/hls:use EXT-X-START instead of live_start_index if it's in playlist | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/commit_msg_loongarch64 | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/commit_msg_x86 | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/commit_msg_armv7_RPi4 | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
<wolfleekay@gmail.com> 于2022年6月23日周四 23:01写道： > > From: Li Kai <wolfleekay@gmail.com> Hi Li, > > Refer to https://datatracker.ietf.org/doc/html/rfc8216#section-4.3.5.2 > > Signed-off-by: Li Kai <wolfleekay@gmail.com> > --- > libavformat/hls.c | 68 ++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 62 insertions(+), 6 deletions(-) > > diff --git a/libavformat/hls.c b/libavformat/hls.c > index 8204f55df3..96923c3eac 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; > + } This part is refine hls function, but i prefer reserved old process way bellow. Support both way of them should be ok. > } else if (av_strstart(line, "#EXT-X-ENDLIST", &ptr)) { > if (pls) > pls->finished = 1; > @@ -1719,12 +1734,53 @@ static int64_t select_cur_seq_no(HLSContext *c, struct playlist *pls) > * require us to download a segment to inspect its timestamps. */ > return c->cur_seq_no; > > - /* 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 (!pls->time_offset_flag) { > + /* If this is a live stream, start live_start_index segments from the > + * start or end */ > + if (c->live_start_index < 0) This could be set by user from command line. It's not only used from play list, also can be controled by user. > + 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 playlist indicate a TIME-OFFSET, 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. */ > -- > 2.25.1 > > _______________________________________________ > 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
> > > 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) { + 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}, {"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"},
少宇李 <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
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? 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. 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 >
> 在 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
Ok, I add allowed_x_start option, and the option default is 0. So current 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". >
Ok, I add allowed_x_start option, and the option default is 0. So current 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". >
> 在 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
OK, I add the option description in doc/demuxers.texi. About invalid EXT-X-START value, it's not played on Safari. So I make it error to handle. You opinion, it's better way. The latest patch fix it. On 2022/6/25 21:26, Steven Liu wrote: > > >> 在 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". Steven Liu <lq@chinaffmpeg.org> 于2022年6月25日周六 21:28写道： > > > > 在 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". >
Li Kai <wolfleekay@gmail.com> 于2022年6月25日周六 22:15写道： > > OK, I add the option description in doc/demuxers.texi. > > About invalid EXT-X-START value, it's not played on Safari. > So I make it error to handle. > > You opinion, it's better way. The latest patch fix it. > > > @@ -741,6 +744,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 +893,20 @@ static int parse_playlist(HLSContext *c, const char *url, cur_init_section->key = NULL; } + } else if (av_strstart(line, "#EXT-X-START:", &ptr)) { Can the "const char *p;" move to here? And maybe make a clarify name will better i think, e.g. const char *time_offset_value? + 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 { + av_log(c->ctx, AV_LOG_WARNING, "#EXT-X-START value is" + "invalid, it will be ignored"); + continue; + } } else if (av_strstart(line, "#EXT-X-ENDLIST", &ptr)) { if (pls) pls->finished = 1; Thanks Steven
OK, I got it. Thanks a lot. 在 2022/6/27 上午10:23, Steven Liu 写道: > Li Kai <wolfleekay@gmail.com> 于2022年6月25日周六 22:15写道： >> OK, I add the option description in doc/demuxers.texi. >> >> About invalid EXT-X-START value, it's not played on Safari. >> So I make it error to handle. >> >> You opinion, it's better way. The latest patch fix it. >> >> >> > @@ -741,6 +744,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 +893,20 @@ static int parse_playlist(HLSContext *c, const char *url, > cur_init_section->key = NULL; > } > > + } else if (av_strstart(line, "#EXT-X-START:", &ptr)) { > Can the "const char *p;" move to here? And maybe make a clarify name > will better i think, e.g. const char *time_offset_value? > > + 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 { > + av_log(c->ctx, AV_LOG_WARNING, "#EXT-X-START value is" > + "invalid, it will be ignored"); > + continue; > + } > } else if (av_strstart(line, "#EXT-X-ENDLIST", &ptr)) { > if (pls) > pls->finished = 1; > > > > Thanks > Steven From 9c812414bc21030182fda00e702ea6433c920c9d Mon Sep 17 00:00:00 2001 From: Li Kai <wolfleekay@gmail.com> Date: Thu, 23 Jun 2022 00:55:38 +0800 Subject: [PATCH v4] avformat/hls: add #EXT-X-START tag support by prefer_x_start opt Signed-off-by: Li Kai <wolfleekay@gmail.com> --- doc/demuxers.texi | 3 +++ libavformat/hls.c | 60 +++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/doc/demuxers.texi b/doc/demuxers.texi index c95a9ae594..2b6dd86c2a 100644 --- a/doc/demuxers.texi +++ b/doc/demuxers.texi @@ -373,6 +373,9 @@ It accepts the following options: @item live_start_index segment index to start live streams at (negative values are from the end). +@item prefer_x_start +prefer to use #EXT-X-START if it's in playlist instead of live_start_index. + @item allowed_extensions ',' separated list of file extensions that hls is allowed to access. diff --git a/libavformat/hls.c b/libavformat/hls.c index b736f093a9..8ea4ff4671 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; @@ -211,6 +213,7 @@ typedef struct HLSContext { int64_t cur_seq_no; int m3u8_hold_counters; int live_start_index; + int prefer_x_start; int first_packet; int64_t first_timestamp; int64_t cur_timestamp; @@ -889,6 +892,21 @@ 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; + } + const char *time_offset_value; + if (av_strstart(ptr, "TIME-OFFSET=", &time_offset_value)) { + float offset = strtof(time_offset_value, NULL); + pls->start_time_offset = offset * AV_TIME_BASE; + pls->time_offset_flag = 1; + } else { + av_log(c->ctx, AV_LOG_WARNING, "#EXT-X-START value is" + "invalid, it will be ignored"); + continue; + } } else if (av_strstart(line, "#EXT-X-ENDLIST", &ptr)) { if (pls) pls->finished = 1; @@ -1722,9 +1740,45 @@ 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); + seq_no = 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); + seq_no = pls->start_seq_no + FFMIN(c->live_start_index, + pls->n_segments - 1); + + /* If #EXT-X-START in playlist, need to recalculate */ + if (pls->time_offset_flag && c->prefer_x_start) { + int64_t start_timestamp; + int64_t playlist_duration = 0; + int64_t cur_timestamp = c->cur_timestamp == AV_NOPTS_VALUE ? 0 : + c->cur_timestamp; + + for (int 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); + } + return seq_no; } /* Otherwise just start on the first segment. */ @@ -2476,6 +2530,8 @@ static int hls_probe(const AVProbeData *p) 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}, + {"prefer_x_start", "prefer to use #EXT-X-START if it's in playlist instead of live_start_index", + OFFSET(prefer_x_start), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS}, {"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"},
Li Kai <wolfleekay@gmail.com> 于2022年6月27日周一 10:58写道： > > OK, I got it. > looks ok to me, waiting for other reviewers comments, and will apply this v4 patch if no more comments. Thanks Steven
Steven Liu <lingjiujianke@gmail.com> 于2022年6月27日周一 11:05写道： > > Li Kai <wolfleekay@gmail.com> 于2022年6月27日周一 10:58写道： > > > > OK, I got it. > > > looks ok to me, waiting for other reviewers comments, and will apply > this v4 patch if no more comments. Will apply soon. Thanks Steven
diff --git a/libavformat/hls.c b/libavformat/hls.c index 8204f55df3..96923c3eac 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; @@ -1719,12 +1734,53 @@ static int64_t select_cur_seq_no(HLSContext *c, struct playlist *pls) * require us to download a segment to inspect its timestamps. */ return c->cur_seq_no; - /* 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 (!pls->time_offset_flag) { + /* 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); + } else { + /* If playlist indicate a TIME-OFFSET, 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. */