[FFmpeg-devel,1/4] avformat/hlsenc: Add init_program_date_time so start time can be specified

---
 doc/muxers.texi      |  3 +++
 libavformat/hlsenc.c | 41 +++++++++++++++++++++++++----------------
 2 files changed, 28 insertions(+), 16 deletions(-)

On 27 Oct 2023, at 5:59, Dave Johansen wrote:

> ---
>  doc/muxers.texi      |  3 +++
>  libavformat/hlsenc.c | 41 +++++++++++++++++++++++++----------------
>  2 files changed, 28 insertions(+), 16 deletions(-)
>

Thanks for the revised patch!

> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index f6071484ff..87c19a5cb9 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -1086,6 +1086,9 @@ seeking. This flag should be used with the @code{hls_time} option.
>  @item program_date_time
>  Generate @code{EXT-X-PROGRAM-DATE-TIME} tags.
>
> +@item init_program_date_time
> +Time to start program date time at.
> +

Probably would help to mention the expected format here.

>  @item second_level_segment_index
>  Makes it possible to use segment indexes as %%d in hls_segment_filename expression
>  besides date/time values when strftime is on.
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 4ef84c05c1..5dfff6b2b6 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -212,6 +212,8 @@ typedef struct HLSContext {
>      int64_t recording_time;
>      int64_t max_seg_size; // every segment file max size
>
> +    char *init_program_date_time;
> +
>      char *baseurl;
>      char *vtt_format_options_str;
>      char *subtitle_filename;
> @@ -1192,6 +1194,25 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls,
>      return 0;
>  }
>
> +static double parse_iso8601(const char *ptr) {
> +    struct tm program_date_time;
> +    int y,M,d,h,m,s;
> +    double ms;
> +    if (sscanf(ptr, "%d-%d-%dT%d:%d:%d.%lf", &y, &M, &d, &h, &m, &s, &ms) != 7) {
> +        return -1;
> +    }
> +
> +    program_date_time.tm_year = y - 1900;
> +    program_date_time.tm_mon = M - 1;
> +    program_date_time.tm_mday = d;
> +    program_date_time.tm_hour = h;
> +    program_date_time.tm_min = m;
> +    program_date_time.tm_sec = s;
> +    program_date_time.tm_isdst = -1;
> +
> +    return mktime(&program_date_time) + (double)(ms / 1000);
> +}
> +
>  static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs)
>  {
>      HLSContext *hls = s->priv_data;
> @@ -1257,24 +1278,11 @@ static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs
>                  }
>              }
>          } else if (av_strstart(line, "#EXT-X-PROGRAM-DATE-TIME:", &ptr)) {
> -            struct tm program_date_time;
> -            int y,M,d,h,m,s;
> -            double ms;
> -            if (sscanf(ptr, "%d-%d-%dT%d:%d:%d.%lf", &y, &M, &d, &h, &m, &s, &ms) != 7) {
> +            discont_program_date_time = parse_iso8601(ptr);
> +            if (discont_program_date_time < 0) {
>                  ret = AVERROR_INVALIDDATA;
>                  goto fail;
>              }
> -
> -            program_date_time.tm_year = y - 1900;
> -            program_date_time.tm_mon = M - 1;
> -            program_date_time.tm_mday = d;
> -            program_date_time.tm_hour = h;
> -            program_date_time.tm_min = m;
> -            program_date_time.tm_sec = s;
> -            program_date_time.tm_isdst = -1;
> -
> -            discont_program_date_time = mktime(&program_date_time);
> -            discont_program_date_time += (double)(ms / 1000);
>          } else if (av_strstart(line, "#", NULL)) {
>              continue;
>          } else if (line[0]) {
> @@ -2867,7 +2875,7 @@ static int hls_init(AVFormatContext *s)
>      char *p = NULL;
>      int http_base_proto = ff_is_http_proto(s->url);
>      int fmp4_init_filename_len = strlen(hls->fmp4_init_filename) + 1;
> -    double initial_program_date_time = av_gettime() / 1000000.0;
> +    double initial_program_date_time = hls->init_program_date_time ? parse_iso8601(hls->init_program_date_time) : av_gettime() / 1000000.0;

As parse_iso8601 parsing user input can fail, it should properly report the error and fail. Especially given that it does not accept all variations
of ISO-8601 date/time IIUC.

It might be confusing if the user specifies a time, forgets the milliseconds and it will just silently not use the option at all?

>
>      if (hls->use_localtime) {
>          pattern = get_default_pattern_localtime_fmt(s);
> @@ -3141,6 +3149,7 @@ static const AVOption options[] = {
>      {"split_by_time", "split the hls segment by time which user set by hls_time", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_SPLIT_BY_TIME }, 0, UINT_MAX,   E, "flags"},
>      {"append_list", "append the new segments into old hls segment list", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_APPEND_LIST }, 0, UINT_MAX,   E, "flags"},
>      {"program_date_time", "add EXT-X-PROGRAM-DATE-TIME", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_PROGRAM_DATE_TIME }, 0, UINT_MAX,   E, "flags"},
> +    {"init_program_date_time", "Time to start program date time at", OFFSET(init_program_date_time), AV_OPT_TYPE_STRING, .flags = E},
>      {"second_level_segment_index", "include segment index in segment filenames when use_localtime", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_SECOND_LEVEL_SEGMENT_INDEX }, 0, UINT_MAX,   E, "flags"},
>      {"second_level_segment_duration", "include segment duration in segment filenames when use_localtime", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_SECOND_LEVEL_SEGMENT_DURATION }, 0, UINT_MAX,   E, "flags"},
>      {"second_level_segment_size", "include segment size in segment filenames when use_localtime", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_SECOND_LEVEL_SEGMENT_SIZE }, 0, UINT_MAX,   E, "flags"},
> -- 
> 2.39.2 (Apple Git-143)
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

On Fri, Oct 27, 2023 at 4:58 AM <epirat07@gmail.com> wrote:

> On 27 Oct 2023, at 5:59, Dave Johansen wrote:
> >  @item second_level_segment_index
> >  Makes it possible to use segment indexes as %%d in hls_segment_filename
> expression
> >  besides date/time values when strftime is on.
> > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > index 4ef84c05c1..5dfff6b2b6 100644
> > --- a/libavformat/hlsenc.c
> > +++ b/libavformat/hlsenc.c
> > @@ -212,6 +212,8 @@ typedef struct HLSContext {
> >      int64_t recording_time;
> >      int64_t max_seg_size; // every segment file max size
> >
> > +    char *init_program_date_time;
> > +
> >      char *baseurl;
> >      char *vtt_format_options_str;
> >      char *subtitle_filename;
> > @@ -1192,6 +1194,25 @@ static int hls_append_segment(struct
> AVFormatContext *s, HLSContext *hls,
> >      return 0;
> >  }
> >
> > +static double parse_iso8601(const char *ptr) {
> > +    struct tm program_date_time;
> > +    int y,M,d,h,m,s;
> > +    double ms;
> > +    if (sscanf(ptr, "%d-%d-%dT%d:%d:%d.%lf", &y, &M, &d, &h, &m, &s,
> &ms) != 7) {
> > +        return -1;
> > +    }
> > +
> > +    program_date_time.tm_year = y - 1900;
> > +    program_date_time.tm_mon = M - 1;
> > +    program_date_time.tm_mday = d;
> > +    program_date_time.tm_hour = h;
> > +    program_date_time.tm_min = m;
> > +    program_date_time.tm_sec = s;
> > +    program_date_time.tm_isdst = -1;
> > +
> > +    return mktime(&program_date_time) + (double)(ms / 1000);
> > +}
> > +
> >  static int parse_playlist(AVFormatContext *s, const char *url,
> VariantStream *vs)
> >  {
> >      HLSContext *hls = s->priv_data;
> > @@ -1257,24 +1278,11 @@ static int parse_playlist(AVFormatContext *s,
> const char *url, VariantStream *vs
> >                  }
> >              }
> >          } else if (av_strstart(line, "#EXT-X-PROGRAM-DATE-TIME:",
> &ptr)) {
> > -            struct tm program_date_time;
> > -            int y,M,d,h,m,s;
> > -            double ms;
> > -            if (sscanf(ptr, "%d-%d-%dT%d:%d:%d.%lf", &y, &M, &d, &h,
> &m, &s, &ms) != 7) {
> > +            discont_program_date_time = parse_iso8601(ptr);
> > +            if (discont_program_date_time < 0) {
> >                  ret = AVERROR_INVALIDDATA;
> >                  goto fail;
> >              }
> > -
> > -            program_date_time.tm_year = y - 1900;
> > -            program_date_time.tm_mon = M - 1;
> > -            program_date_time.tm_mday = d;
> > -            program_date_time.tm_hour = h;
> > -            program_date_time.tm_min = m;
> > -            program_date_time.tm_sec = s;
> > -            program_date_time.tm_isdst = -1;
> > -
> > -            discont_program_date_time = mktime(&program_date_time);
> > -            discont_program_date_time += (double)(ms / 1000);
> >          } else if (av_strstart(line, "#", NULL)) {
> >              continue;
> >          } else if (line[0]) {
> > @@ -2867,7 +2875,7 @@ static int hls_init(AVFormatContext *s)
> >      char *p = NULL;
> >      int http_base_proto = ff_is_http_proto(s->url);
> >      int fmp4_init_filename_len = strlen(hls->fmp4_init_filename) + 1;
> > -    double initial_program_date_time = av_gettime() / 1000000.0;
> > +    double initial_program_date_time = hls->init_program_date_time ?
> parse_iso8601(hls->init_program_date_time) : av_gettime() / 1000000.0;
>
> As parse_iso8601 parsing user input can fail, it should properly report
> the error and fail. Especially given that it does not accept all variations
> of ISO-8601 date/time IIUC.
>
> It might be confusing if the user specifies a time, forgets the
> milliseconds and it will just silently not use the option at all?
>

I added improved parsing and error reporting. I sent in the patch, but is
there a way for me to tie it to this patchset in the future?

On Fri, 27 Oct 2023, David Johansen wrote:

> On Fri, Oct 27, 2023 at 4:58 AM <epirat07@gmail.com> wrote:
>
>> On 27 Oct 2023, at 5:59, Dave Johansen wrote:
>> >  @item second_level_segment_index
>> >  Makes it possible to use segment indexes as %%d in hls_segment_filename
>> expression
>> >  besides date/time values when strftime is on.
>> > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> > index 4ef84c05c1..5dfff6b2b6 100644
>> > --- a/libavformat/hlsenc.c
>> > +++ b/libavformat/hlsenc.c
>> > @@ -212,6 +212,8 @@ typedef struct HLSContext {
>> >      int64_t recording_time;
>> >      int64_t max_seg_size; // every segment file max size
>> >
>> > +    char *init_program_date_time;
>> > +
>> >      char *baseurl;
>> >      char *vtt_format_options_str;
>> >      char *subtitle_filename;
>> > @@ -1192,6 +1194,25 @@ static int hls_append_segment(struct
>> AVFormatContext *s, HLSContext *hls,
>> >      return 0;
>> >  }
>> >
>> > +static double parse_iso8601(const char *ptr) {

Please use the existing function av_parse_time(). That can more completely 
handle ISO8601 and even more.

Regards,
Marton

On Sun, Nov 5, 2023 at 1:21 AM Marton Balint <cus@passwd.hu> wrote:

>
>
> On Fri, 27 Oct 2023, David Johansen wrote:
>
> > On Fri, Oct 27, 2023 at 4:58 AM <epirat07@gmail.com> wrote:
> >
> >> On 27 Oct 2023, at 5:59, Dave Johansen wrote:
> >> >  @item second_level_segment_index
> >> >  Makes it possible to use segment indexes as %%d in
> hls_segment_filename
> >> expression
> >> >  besides date/time values when strftime is on.
> >> > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> >> > index 4ef84c05c1..5dfff6b2b6 100644
> >> > --- a/libavformat/hlsenc.c
> >> > +++ b/libavformat/hlsenc.c
> >> > @@ -212,6 +212,8 @@ typedef struct HLSContext {
> >> >      int64_t recording_time;
> >> >      int64_t max_seg_size; // every segment file max size
> >> >
> >> > +    char *init_program_date_time;
> >> > +
> >> >      char *baseurl;
> >> >      char *vtt_format_options_str;
> >> >      char *subtitle_filename;
> >> > @@ -1192,6 +1194,25 @@ static int hls_append_segment(struct
> >> AVFormatContext *s, HLSContext *hls,
> >> >      return 0;
> >> >  }
> >> >
> >> > +static double parse_iso8601(const char *ptr) {
>
> Please use the existing function av_parse_time(). That can more completely
> handle ISO8601 and even more.
>

I made a patch for this an submitted it with --in-reply-to with the Message
ID from this email, but it doesn't appear that it linked it to the existing
patches I sent. What's the proper way that I should submit follow on
patches like this in the future?