diff mbox series

[FFmpeg-devel] avformat/hls: Fixes overwriting existing #EXT-X-PROGRAM-DATE-TIME value in HLS playlist Bug ID: 8989

Message ID 20201126142126.64043-1-vignesh.ravichandran02@gmail.com
State New
Headers show
Series [FFmpeg-devel] avformat/hls: Fixes overwriting existing #EXT-X-PROGRAM-DATE-TIME value in HLS playlist Bug ID: 8989 | expand

Checks

Context Check Description
andriy/x86_make fail Make failed
andriy/PPC64_make warning Make failed

Commit Message

Vignesh Ravichandran Nov. 26, 2020, 2:21 p.m. UTC
This is is due to the following behavior in the current code:
1. The initial_prog_date_time gets set to the current local time
2. The existing playlist (.m3u8) file gets parsed and the segments present are added to the variant stream
3. The new segment is created and added
4. The existing segments and the new segment are written to the playlist file. The initial_prog_date_time from point 1 is used for calculating "#EXT-X-PROGRAM-DATE-TIME" for the segments, which results in incorrect "#EXT-X-PROGRAM-DATE-TIME" values for existing segments

The following approach fixes this bug:
1. Add a new variable "discont_program_date_time" of type double to HLSSegment struct
2. Store the "EXT-X-PROGRAM-DATE-TIME" value from the existing segments in this variable
3. When writing to playlist file if "discont_program_date_time" is set, then use that value for "EXT-X-PROGRAM-DATE-TIME" else use the value present in vs->initial_prog_date_time

Signed-off-by: Vignesh Ravichandran <vignesh.ravichandran02@gmail.com>
---
 libavformat/hlsenc.c | 83 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 80 insertions(+), 3 deletions(-)

Comments

Liu Steven Nov. 27, 2020, 1:39 a.m. UTC | #1
> 2020年11月26日 下午10:21,Vignesh Ravichandran <vignesh.ravichandran02@gmail.com> 写道:
> 
> This is is due to the following behavior in the current code:
> 1. The initial_prog_date_time gets set to the current local time
> 2. The existing playlist (.m3u8) file gets parsed and the segments present are added to the variant stream
> 3. The new segment is created and added
> 4. The existing segments and the new segment are written to the playlist file. The initial_prog_date_time from point 1 is used for calculating "#EXT-X-PROGRAM-DATE-TIME" for the segments, which results in incorrect "#EXT-X-PROGRAM-DATE-TIME" values for existing segments
> 
> The following approach fixes this bug:
> 1. Add a new variable "discont_program_date_time" of type double to HLSSegment struct
> 2. Store the "EXT-X-PROGRAM-DATE-TIME" value from the existing segments in this variable
> 3. When writing to playlist file if "discont_program_date_time" is set, then use that value for "EXT-X-PROGRAM-DATE-TIME" else use the value present in vs->initial_prog_date_time
> 
> Signed-off-by: Vignesh Ravichandran <vignesh.ravichandran02@gmail.com>
> ---
> libavformat/hlsenc.c | 83 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 80 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index cbfd8f7c0d..9346b2cfea 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -25,6 +25,10 @@
> #include <stdint.h>
> #if HAVE_UNISTD_H
> #include <unistd.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <limits.h>
> +#include <math.h>
> #endif
> 
> #if CONFIG_GCRYPT
> @@ -88,6 +92,7 @@ typedef struct HLSSegment {
>     char iv_string[KEYSIZE*2 + 1];
> 
>     struct HLSSegment *next;
> +    double discont_program_date_time;
> } HLSSegment;
> 
> typedef enum HLSFlags {
> @@ -1124,6 +1129,7 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls,
>     en->keyframe_size     = vs->video_keyframe_size;
>     en->next     = NULL;
>     en->discont  = 0;
> +    en->discont_program_date_time = 0;
> 
>     if (vs->discontinuity) {
>         en->discont = 1;
> @@ -1148,7 +1154,8 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls,
> 
>     if (hls->max_nb_segments && vs->nb_entries >= hls->max_nb_segments) {
>         en = vs->segments;
> -        vs->initial_prog_date_time += en->duration;
> +        if (!en->next->discont_program_date_time && !en->discont_program_date_time)
> +            vs->initial_prog_date_time += en->duration;
>         vs->segments = en->next;
>         if (en && hls->flags & HLS_DELETE_SEGMENTS &&
> #if FF_API_HLS_WRAP
> @@ -1173,6 +1180,44 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls,
>     return 0;
> }
> 
> +static int check_program_date_time(const char *prog_date_time) {
> +    char s[strlen(prog_date_time)], *sptr0, *sptr1, *sptr2, *sptr3;
> +    char *err = NULL;
> +    char *arr[6] = {NULL};
> +    int i = 0;
> +    av_strlcpy(s, prog_date_time, strlen(prog_date_time));
> +    char *p = strtok_r(strtok_r(s, "+", &sptr0), "T", &sptr1);
> +    while (p) {
> +        char *q = strtok_r(p, "-", &sptr2);
> +        while (q) {
> +            char *r = strtok_r(q, ":", &sptr3);
> +            while (r) {
> +                arr[i] = r;
> +                i++;
> +                r = strtok_r(NULL, ":", &sptr3);
> +            }
> +            q = strtok_r(NULL, "-", &sptr2);
> +        }
> +        p = strtok_r(NULL, "T", &sptr1);
> +    }
> +    
> +    for (i=0; i < 5; i++) {
> +        errno=0;
> +        long number = strtol(arr[i], &err, 10);
> +        if((arr[i] == err) || (errno == ERANGE && number == LONG_MIN) || (errno == ERANGE && number == LONG_MAX) 
> +            || (errno == EINVAL) || (errno != 0 && number == 0) || (errno == 0 && arr[i] && *err != 0))
What about use av_isdigit to check them?
> +            return AVERROR_INVALIDDATA;
> +    }
> +    
> +    errno=0;
> +    double number = strtod(arr[i], &err);
> +    if((arr[i] == err) || (errno == ERANGE && number == HUGE_VALF) || (errno == ERANGE && number == HUGE_VALL)
> +        || (errno == EINVAL) || (errno != 0 && number == 0) || (errno == 0 && arr[i] && *err != 0))
> +        return AVERROR_INVALIDDATA;
> +
> +    return 0;
> +}
> +
> static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs)
> {
>     HLSContext *hls = s->priv_data;
> @@ -1182,6 +1227,8 @@ static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs
>     char line[MAX_URL_SIZE];
>     const char *ptr;
>     const char *end;
> +    int is_discont_detected = 0;
> +    double discont_program_date_time = 0;
> 
>     if ((ret = ffio_open_whitelist(&in, url, AVIO_FLAG_READ,
>                                    &s->interrupt_callback, NULL,
> @@ -1211,6 +1258,7 @@ static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs
>         } else if (av_strstart(line, "#EXT-X-DISCONTINUITY", &ptr)) {
>             is_segment = 1;
>             vs->discontinuity = 1;
> +            is_discont_detected = 1;
>         } else if (av_strstart(line, "#EXTINF:", &ptr)) {
>             is_segment = 1;
>             vs->duration = atof(ptr);
> @@ -1236,7 +1284,31 @@ static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs
>                     av_strlcpy(vs->iv_string, ptr, sizeof(vs->iv_string));
>                 }
>             }
> +        } 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 (check_program_date_time(ptr) != 0) {
> +                av_log(hls, AV_LOG_VERBOSE, 
> +                       "Found invalid program date time %s when parsing playlist. "
> +                       "Current and subsequent existing segments will be ignored", ptr);
> +                ret = AVERROR_INVALIDDATA;
> +                goto fail;
> +            }
> 
> +            sscanf(ptr, "%d-%d-%dT%d:%d:%d.%lf", &y, &M, &d, &h, &m, &s, &ms);
> +            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);
> +            is_discont_detected = 0;
>         } else if (av_strstart(line, "#", NULL)) {
>             continue;
>         } else if (line[0]) {
> @@ -1250,8 +1322,9 @@ static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs
>                 is_segment = 0;
>                 new_start_pos = avio_tell(vs->avf->pb);
>                 vs->size = new_start_pos - vs->start_pos;
> -                vs->initial_prog_date_time -= vs->duration; // this is a previously existing segment
>                 ret = hls_append_segment(s, hls, vs, vs->duration, vs->start_pos, vs->size);
> +                vs->last_segment->discont_program_date_time = discont_program_date_time;
> +                discont_program_date_time += vs->duration;
>                 if (ret < 0)
>                     goto fail;
>                 vs->start_pos = new_start_pos;
> @@ -1572,7 +1645,11 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>         ret = ff_hls_write_file_entry(byterange_mode ? hls->m3u8_out : vs->out, en->discont, byterange_mode,
>                                       en->duration, hls->flags & HLS_ROUND_DURATIONS,
>                                       en->size, en->pos, hls->baseurl,
> -                                      en->filename, prog_date_time_p, en->keyframe_size, en->keyframe_pos, hls->flags & HLS_I_FRAMES_ONLY);
> +                                      en->filename,
> +                                      en->discont_program_date_time ? &en->discont_program_date_time : prog_date_time_p,
> +                                      en->keyframe_size, en->keyframe_pos, hls->flags & HLS_I_FRAMES_ONLY);
> +        if (en->discont_program_date_time)
> +            en->discont_program_date_time -= en->duration;
>         if (ret < 0) {
>             av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
>         }
> -- 
> 2.24.2 (Apple Git-127)
> 
> _______________________________________________
> 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 Liu
Vignesh Ravichandran Nov. 27, 2020, 6:36 a.m. UTC | #2
av_isdigit seems to only check if the integer is from 0 to 9. So values
above 9 would fail the check, for example: 2020 would fail the check. Also,
we will not be able to use av_isdigit for checking the double value.

On Fri, Nov 27, 2020 at 7:41 AM Steven Liu <lq@chinaffmpeg.org> wrote:

>
>
> > 2020年11月26日 下午10:21,Vignesh Ravichandran <
> vignesh.ravichandran02@gmail.com> 写道:
> >
> > This is is due to the following behavior in the current code:
> > 1. The initial_prog_date_time gets set to the current local time
> > 2. The existing playlist (.m3u8) file gets parsed and the segments
> present are added to the variant stream
> > 3. The new segment is created and added
> > 4. The existing segments and the new segment are written to the playlist
> file. The initial_prog_date_time from point 1 is used for calculating
> "#EXT-X-PROGRAM-DATE-TIME" for the segments, which results in incorrect
> "#EXT-X-PROGRAM-DATE-TIME" values for existing segments
> >
> > The following approach fixes this bug:
> > 1. Add a new variable "discont_program_date_time" of type double to
> HLSSegment struct
> > 2. Store the "EXT-X-PROGRAM-DATE-TIME" value from the existing segments
> in this variable
> > 3. When writing to playlist file if "discont_program_date_time" is set,
> then use that value for "EXT-X-PROGRAM-DATE-TIME" else use the value
> present in vs->initial_prog_date_time
> >
> > Signed-off-by: Vignesh Ravichandran <vignesh.ravichandran02@gmail.com>
> > ---
> > libavformat/hlsenc.c | 83 ++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 80 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > index cbfd8f7c0d..9346b2cfea 100644
> > --- a/libavformat/hlsenc.c
> > +++ b/libavformat/hlsenc.c
> > @@ -25,6 +25,10 @@
> > #include <stdint.h>
> > #if HAVE_UNISTD_H
> > #include <unistd.h>
> > +#include <string.h>
> > +#include <errno.h>
> > +#include <limits.h>
> > +#include <math.h>
> > #endif
> >
> > #if CONFIG_GCRYPT
> > @@ -88,6 +92,7 @@ typedef struct HLSSegment {
> >     char iv_string[KEYSIZE*2 + 1];
> >
> >     struct HLSSegment *next;
> > +    double discont_program_date_time;
> > } HLSSegment;
> >
> > typedef enum HLSFlags {
> > @@ -1124,6 +1129,7 @@ static int hls_append_segment(struct
> AVFormatContext *s, HLSContext *hls,
> >     en->keyframe_size     = vs->video_keyframe_size;
> >     en->next     = NULL;
> >     en->discont  = 0;
> > +    en->discont_program_date_time = 0;
> >
> >     if (vs->discontinuity) {
> >         en->discont = 1;
> > @@ -1148,7 +1154,8 @@ static int hls_append_segment(struct
> AVFormatContext *s, HLSContext *hls,
> >
> >     if (hls->max_nb_segments && vs->nb_entries >= hls->max_nb_segments) {
> >         en = vs->segments;
> > -        vs->initial_prog_date_time += en->duration;
> > +        if (!en->next->discont_program_date_time &&
> !en->discont_program_date_time)
> > +            vs->initial_prog_date_time += en->duration;
> >         vs->segments = en->next;
> >         if (en && hls->flags & HLS_DELETE_SEGMENTS &&
> > #if FF_API_HLS_WRAP
> > @@ -1173,6 +1180,44 @@ static int hls_append_segment(struct
> AVFormatContext *s, HLSContext *hls,
> >     return 0;
> > }
> >
> > +static int check_program_date_time(const char *prog_date_time) {
> > +    char s[strlen(prog_date_time)], *sptr0, *sptr1, *sptr2, *sptr3;
> > +    char *err = NULL;
> > +    char *arr[6] = {NULL};
> > +    int i = 0;
> > +    av_strlcpy(s, prog_date_time, strlen(prog_date_time));
> > +    char *p = strtok_r(strtok_r(s, "+", &sptr0), "T", &sptr1);
> > +    while (p) {
> > +        char *q = strtok_r(p, "-", &sptr2);
> > +        while (q) {
> > +            char *r = strtok_r(q, ":", &sptr3);
> > +            while (r) {
> > +                arr[i] = r;
> > +                i++;
> > +                r = strtok_r(NULL, ":", &sptr3);
> > +            }
> > +            q = strtok_r(NULL, "-", &sptr2);
> > +        }
> > +        p = strtok_r(NULL, "T", &sptr1);
> > +    }
> > +
> > +    for (i=0; i < 5; i++) {
> > +        errno=0;
> > +        long number = strtol(arr[i], &err, 10);
> > +        if((arr[i] == err) || (errno == ERANGE && number == LONG_MIN)
> || (errno == ERANGE && number == LONG_MAX)
> > +            || (errno == EINVAL) || (errno != 0 && number == 0) ||
> (errno == 0 && arr[i] && *err != 0))
> What about use av_isdigit to check them?
> > +            return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    errno=0;
> > +    double number = strtod(arr[i], &err);
> > +    if((arr[i] == err) || (errno == ERANGE && number == HUGE_VALF) ||
> (errno == ERANGE && number == HUGE_VALL)
> > +        || (errno == EINVAL) || (errno != 0 && number == 0) || (errno
> == 0 && arr[i] && *err != 0))
> > +        return AVERROR_INVALIDDATA;
> > +
> > +    return 0;
> > +}
> > +
> > static int parse_playlist(AVFormatContext *s, const char *url,
> VariantStream *vs)
> > {
> >     HLSContext *hls = s->priv_data;
> > @@ -1182,6 +1227,8 @@ static int parse_playlist(AVFormatContext *s,
> const char *url, VariantStream *vs
> >     char line[MAX_URL_SIZE];
> >     const char *ptr;
> >     const char *end;
> > +    int is_discont_detected = 0;
> > +    double discont_program_date_time = 0;
> >
> >     if ((ret = ffio_open_whitelist(&in, url, AVIO_FLAG_READ,
> >                                    &s->interrupt_callback, NULL,
> > @@ -1211,6 +1258,7 @@ static int parse_playlist(AVFormatContext *s,
> const char *url, VariantStream *vs
> >         } else if (av_strstart(line, "#EXT-X-DISCONTINUITY", &ptr)) {
> >             is_segment = 1;
> >             vs->discontinuity = 1;
> > +            is_discont_detected = 1;
> >         } else if (av_strstart(line, "#EXTINF:", &ptr)) {
> >             is_segment = 1;
> >             vs->duration = atof(ptr);
> > @@ -1236,7 +1284,31 @@ static int parse_playlist(AVFormatContext *s,
> const char *url, VariantStream *vs
> >                     av_strlcpy(vs->iv_string, ptr,
> sizeof(vs->iv_string));
> >                 }
> >             }
> > +        } 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 (check_program_date_time(ptr) != 0) {
> > +                av_log(hls, AV_LOG_VERBOSE,
> > +                       "Found invalid program date time %s when parsing
> playlist. "
> > +                       "Current and subsequent existing segments will
> be ignored", ptr);
> > +                ret = AVERROR_INVALIDDATA;
> > +                goto fail;
> > +            }
> >
> > +            sscanf(ptr, "%d-%d-%dT%d:%d:%d.%lf", &y, &M, &d, &h, &m,
> &s, &ms);
> > +            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);
> > +            is_discont_detected = 0;
> >         } else if (av_strstart(line, "#", NULL)) {
> >             continue;
> >         } else if (line[0]) {
> > @@ -1250,8 +1322,9 @@ static int parse_playlist(AVFormatContext *s,
> const char *url, VariantStream *vs
> >                 is_segment = 0;
> >                 new_start_pos = avio_tell(vs->avf->pb);
> >                 vs->size = new_start_pos - vs->start_pos;
> > -                vs->initial_prog_date_time -= vs->duration; // this is
> a previously existing segment
> >                 ret = hls_append_segment(s, hls, vs, vs->duration,
> vs->start_pos, vs->size);
> > +                vs->last_segment->discont_program_date_time =
> discont_program_date_time;
> > +                discont_program_date_time += vs->duration;
> >                 if (ret < 0)
> >                     goto fail;
> >                 vs->start_pos = new_start_pos;
> > @@ -1572,7 +1645,11 @@ static int hls_window(AVFormatContext *s, int
> last, VariantStream *vs)
> >         ret = ff_hls_write_file_entry(byterange_mode ? hls->m3u8_out :
> vs->out, en->discont, byterange_mode,
> >                                       en->duration, hls->flags &
> HLS_ROUND_DURATIONS,
> >                                       en->size, en->pos, hls->baseurl,
> > -                                      en->filename, prog_date_time_p,
> en->keyframe_size, en->keyframe_pos, hls->flags & HLS_I_FRAMES_ONLY);
> > +                                      en->filename,
> > +                                      en->discont_program_date_time ?
> &en->discont_program_date_time : prog_date_time_p,
> > +                                      en->keyframe_size,
> en->keyframe_pos, hls->flags & HLS_I_FRAMES_ONLY);
> > +        if (en->discont_program_date_time)
> > +            en->discont_program_date_time -= en->duration;
> >         if (ret < 0) {
> >             av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get
> error\n");
> >         }
> > --
> > 2.24.2 (Apple Git-127)
> >
> > _______________________________________________
> > 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 Liu
>
>
>
>
>
>
>
>
Liu Steven Nov. 27, 2020, 6:41 a.m. UTC | #3
> 2020年11月27日 下午2:36,Vignesh Ravichandran <vignesh.ravichandran02@gmail.com> 写道:
> 
> av_isdigit seems to only check if the integer is from 0 to 9. So values
> above 9 would fail the check, for example: 2020 would fail the check. Also,
> we will not be able to use av_isdigit for checking the double value.
> 
> On Fri, Nov 27, 2020 at 7:41 AM Steven Liu <lq@chinaffmpeg.org> wrote:
> 
>> 
>> 
>>> 2020年11月26日 下午10:21,Vignesh Ravichandran <
>> vignesh.ravichandran02@gmail.com> 写道:
>>> 
>>> This is is due to the following behavior in the current code:
>>> 1. The initial_prog_date_time gets set to the current local time
>>> 2. The existing playlist (.m3u8) file gets parsed and the segments
>> present are added to the variant stream
>>> 3. The new segment is created and added
>>> 4. The existing segments and the new segment are written to the playlist
>> file. The initial_prog_date_time from point 1 is used for calculating
>> "#EXT-X-PROGRAM-DATE-TIME" for the segments, which results in incorrect
>> "#EXT-X-PROGRAM-DATE-TIME" values for existing segments
>>> 
>>> The following approach fixes this bug:
>>> 1. Add a new variable "discont_program_date_time" of type double to
>> HLSSegment struct
>>> 2. Store the "EXT-X-PROGRAM-DATE-TIME" value from the existing segments
>> in this variable
>>> 3. When writing to playlist file if "discont_program_date_time" is set,
>> then use that value for "EXT-X-PROGRAM-DATE-TIME" else use the value
>> present in vs->initial_prog_date_time
>>> 
>>> Signed-off-by: Vignesh Ravichandran <vignesh.ravichandran02@gmail.com>
>>> ---
>>> libavformat/hlsenc.c | 83 ++++++++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 80 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>> index cbfd8f7c0d..9346b2cfea 100644
>>> --- a/libavformat/hlsenc.c
>>> +++ b/libavformat/hlsenc.c
>>> @@ -25,6 +25,10 @@
>>> #include <stdint.h>
>>> #if HAVE_UNISTD_H
>>> #include <unistd.h>
>>> +#include <string.h>
>>> +#include <errno.h>
>>> +#include <limits.h>
>>> +#include <math.h>
>>> #endif
>>> 
>>> #if CONFIG_GCRYPT
>>> @@ -88,6 +92,7 @@ typedef struct HLSSegment {
>>>    char iv_string[KEYSIZE*2 + 1];
>>> 
>>>    struct HLSSegment *next;
>>> +    double discont_program_date_time;
>>> } HLSSegment;
>>> 
>>> typedef enum HLSFlags {
>>> @@ -1124,6 +1129,7 @@ static int hls_append_segment(struct
>> AVFormatContext *s, HLSContext *hls,
>>>    en->keyframe_size     = vs->video_keyframe_size;
>>>    en->next     = NULL;
>>>    en->discont  = 0;
>>> +    en->discont_program_date_time = 0;
>>> 
>>>    if (vs->discontinuity) {
>>>        en->discont = 1;
>>> @@ -1148,7 +1154,8 @@ static int hls_append_segment(struct
>> AVFormatContext *s, HLSContext *hls,
>>> 
>>>    if (hls->max_nb_segments && vs->nb_entries >= hls->max_nb_segments) {
>>>        en = vs->segments;
>>> -        vs->initial_prog_date_time += en->duration;
>>> +        if (!en->next->discont_program_date_time &&
>> !en->discont_program_date_time)
>>> +            vs->initial_prog_date_time += en->duration;
>>>        vs->segments = en->next;
>>>        if (en && hls->flags & HLS_DELETE_SEGMENTS &&
>>> #if FF_API_HLS_WRAP
>>> @@ -1173,6 +1180,44 @@ static int hls_append_segment(struct
>> AVFormatContext *s, HLSContext *hls,
>>>    return 0;
>>> }
>>> 
>>> +static int check_program_date_time(const char *prog_date_time) {
>>> +    char s[strlen(prog_date_time)], *sptr0, *sptr1, *sptr2, *sptr3;
>>> +    char *err = NULL;
>>> +    char *arr[6] = {NULL};
>>> +    int i = 0;
>>> +    av_strlcpy(s, prog_date_time, strlen(prog_date_time));
>>> +    char *p = strtok_r(strtok_r(s, "+", &sptr0), "T", &sptr1);
>>> +    while (p) {
>>> +        char *q = strtok_r(p, "-", &sptr2);
>>> +        while (q) {
>>> +            char *r = strtok_r(q, ":", &sptr3);
>>> +            while (r) {
>>> +                arr[i] = r;
>>> +                i++;
>>> +                r = strtok_r(NULL, ":", &sptr3);
>>> +            }
>>> +            q = strtok_r(NULL, "-", &sptr2);
>>> +        }
>>> +        p = strtok_r(NULL, "T", &sptr1);
>>> +    }
>>> +
>>> +    for (i=0; i < 5; i++) {
>>> +        errno=0;
>>> +        long number = strtol(arr[i], &err, 10);
>>> +        if((arr[i] == err) || (errno == ERANGE && number == LONG_MIN)
>> || (errno == ERANGE && number == LONG_MAX)
>>> +            || (errno == EINVAL) || (errno != 0 && number == 0) ||
>> (errno == 0 && arr[i] && *err != 0))
>> What about use av_isdigit to check them?
>>> +            return AVERROR_INVALIDDATA;
>>> +    }
>>> +
>>> +    errno=0;
>>> +    double number = strtod(arr[i], &err);
>>> +    if((arr[i] == err) || (errno == ERANGE && number == HUGE_VALF) ||
>> (errno == ERANGE && number == HUGE_VALL)
>>> +        || (errno == EINVAL) || (errno != 0 && number == 0) || (errno
>> == 0 && arr[i] && *err != 0))
>>> +        return AVERROR_INVALIDDATA;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> static int parse_playlist(AVFormatContext *s, const char *url,
>> VariantStream *vs)
>>> {
>>>    HLSContext *hls = s->priv_data;
>>> @@ -1182,6 +1227,8 @@ static int parse_playlist(AVFormatContext *s,
>> const char *url, VariantStream *vs
>>>    char line[MAX_URL_SIZE];
>>>    const char *ptr;
>>>    const char *end;
>>> +    int is_discont_detected = 0;
>>> +    double discont_program_date_time = 0;
>>> 
>>>    if ((ret = ffio_open_whitelist(&in, url, AVIO_FLAG_READ,
>>>                                   &s->interrupt_callback, NULL,
>>> @@ -1211,6 +1258,7 @@ static int parse_playlist(AVFormatContext *s,
>> const char *url, VariantStream *vs
>>>        } else if (av_strstart(line, "#EXT-X-DISCONTINUITY", &ptr)) {
>>>            is_segment = 1;
>>>            vs->discontinuity = 1;
>>> +            is_discont_detected = 1;
>>>        } else if (av_strstart(line, "#EXTINF:", &ptr)) {
>>>            is_segment = 1;
>>>            vs->duration = atof(ptr);
>>> @@ -1236,7 +1284,31 @@ static int parse_playlist(AVFormatContext *s,
>> const char *url, VariantStream *vs
>>>                    av_strlcpy(vs->iv_string, ptr,
>> sizeof(vs->iv_string));
>>>                }
>>>            }
>>> +        } 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 (check_program_date_time(ptr) != 0) {
>>> +                av_log(hls, AV_LOG_VERBOSE,
>>> +                       "Found invalid program date time %s when parsing
>> playlist. "
>>> +                       "Current and subsequent existing segments will
>> be ignored", ptr);
>>> +                ret = AVERROR_INVALIDDATA;
>>> +                goto fail;
>>> +            }
>>> 
>>> +            sscanf(ptr, "%d-%d-%dT%d:%d:%d.%lf", &y, &M, &d, &h, &m,
>> &s, &ms);

Ping Andreas & Other guys,

	I cannot sure only check return value of sscanf is ok, or validate the string “ptr" before sscanf,
	sscanf can return the result,  so do we need validate the string “ptr”?

	
>>> +            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);
>>> +            is_discont_detected = 0;
>>>        } else if (av_strstart(line, "#", NULL)) {
>>>            continue;
>>>        } else if (line[0]) {
>>> @@ -1250,8 +1322,9 @@ static int parse_playlist(AVFormatContext *s,
>> const char *url, VariantStream *vs
>>>                is_segment = 0;
>>>                new_start_pos = avio_tell(vs->avf->pb);
>>>                vs->size = new_start_pos - vs->start_pos;
>>> -                vs->initial_prog_date_time -= vs->duration; // this is
>> a previously existing segment
>>>                ret = hls_append_segment(s, hls, vs, vs->duration,
>> vs->start_pos, vs->size);
>>> +                vs->last_segment->discont_program_date_time =
>> discont_program_date_time;
>>> +                discont_program_date_time += vs->duration;
>>>                if (ret < 0)
>>>                    goto fail;
>>>                vs->start_pos = new_start_pos;
>>> @@ -1572,7 +1645,11 @@ static int hls_window(AVFormatContext *s, int
>> last, VariantStream *vs)
>>>        ret = ff_hls_write_file_entry(byterange_mode ? hls->m3u8_out :
>> vs->out, en->discont, byterange_mode,
>>>                                      en->duration, hls->flags &
>> HLS_ROUND_DURATIONS,
>>>                                      en->size, en->pos, hls->baseurl,
>>> -                                      en->filename, prog_date_time_p,
>> en->keyframe_size, en->keyframe_pos, hls->flags & HLS_I_FRAMES_ONLY);
>>> +                                      en->filename,
>>> +                                      en->discont_program_date_time ?
>> &en->discont_program_date_time : prog_date_time_p,
>>> +                                      en->keyframe_size,
>> en->keyframe_pos, hls->flags & HLS_I_FRAMES_ONLY);
>>> +        if (en->discont_program_date_time)
>>> +            en->discont_program_date_time -= en->duration;
>>>        if (ret < 0) {
>>>            av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get
>> error\n");
>>>        }
>>> --
>>> 2.24.2 (Apple Git-127)
>>> 
>>> _______________________________________________
>>> 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 Liu
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
> _______________________________________________
> 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 Liu
Andriy Gelman Nov. 27, 2020, 6:50 a.m. UTC | #4
On Thu, 26. Nov 19:51, Vignesh Ravichandran wrote:
> This is is due to the following behavior in the current code:
> 1. The initial_prog_date_time gets set to the current local time
> 2. The existing playlist (.m3u8) file gets parsed and the segments present are added to the variant stream
> 3. The new segment is created and added
> 4. The existing segments and the new segment are written to the playlist file. The initial_prog_date_time from point 1 is used for calculating "#EXT-X-PROGRAM-DATE-TIME" for the segments, which results in incorrect "#EXT-X-PROGRAM-DATE-TIME" values for existing segments
> 
> The following approach fixes this bug:
> 1. Add a new variable "discont_program_date_time" of type double to HLSSegment struct
> 2. Store the "EXT-X-PROGRAM-DATE-TIME" value from the existing segments in this variable
> 3. When writing to playlist file if "discont_program_date_time" is set, then use that value for "EXT-X-PROGRAM-DATE-TIME" else use the value present in vs->initial_prog_date_time
> 
> Signed-off-by: Vignesh Ravichandran <vignesh.ravichandran02@gmail.com>
> ---
>  libavformat/hlsenc.c | 83 ++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 80 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index cbfd8f7c0d..9346b2cfea 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -25,6 +25,10 @@
>  #include <stdint.h>
>  #if HAVE_UNISTD_H
>  #include <unistd.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <limits.h>
> +#include <math.h>
>  #endif
>  
>  #if CONFIG_GCRYPT
> @@ -88,6 +92,7 @@ typedef struct HLSSegment {
>      char iv_string[KEYSIZE*2 + 1];
>  
>      struct HLSSegment *next;
> +    double discont_program_date_time;
>  } HLSSegment;
>  
>  typedef enum HLSFlags {
> @@ -1124,6 +1129,7 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls,
>      en->keyframe_size     = vs->video_keyframe_size;
>      en->next     = NULL;
>      en->discont  = 0;
> +    en->discont_program_date_time = 0;
>  
>      if (vs->discontinuity) {
>          en->discont = 1;
> @@ -1148,7 +1154,8 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls,
>  
>      if (hls->max_nb_segments && vs->nb_entries >= hls->max_nb_segments) {
>          en = vs->segments;
> -        vs->initial_prog_date_time += en->duration;
> +        if (!en->next->discont_program_date_time && !en->discont_program_date_time)
> +            vs->initial_prog_date_time += en->duration;
>          vs->segments = en->next;
>          if (en && hls->flags & HLS_DELETE_SEGMENTS &&
>  #if FF_API_HLS_WRAP
> @@ -1173,6 +1180,44 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls,
>      return 0;
>  }
>  
> +static int check_program_date_time(const char *prog_date_time) {
> +    char s[strlen(prog_date_time)], *sptr0, *sptr1, *sptr2, *sptr3;
> +    char *err = NULL;
> +    char *arr[6] = {NULL};
> +    int i = 0;
> +    av_strlcpy(s, prog_date_time, strlen(prog_date_time));
> +    char *p = strtok_r(strtok_r(s, "+", &sptr0), "T", &sptr1);
> +    while (p) {
> +        char *q = strtok_r(p, "-", &sptr2);
> +        while (q) {
> +            char *r = strtok_r(q, ":", &sptr3);
> +            while (r) {
> +                arr[i] = r;
> +                i++;
> +                r = strtok_r(NULL, ":", &sptr3);
> +            }
> +            q = strtok_r(NULL, "-", &sptr2);
> +        }
> +        p = strtok_r(NULL, "T", &sptr1);
> +    }
> +    
> +    for (i=0; i < 5; i++) {
> +        errno=0;
> +        long number = strtol(arr[i], &err, 10);
> +        if((arr[i] == err) || (errno == ERANGE && number == LONG_MIN) || (errno == ERANGE && number == LONG_MAX) 
> +            || (errno == EINVAL) || (errno != 0 && number == 0) || (errno == 0 && arr[i] && *err != 0))
> +            return AVERROR_INVALIDDATA;
> +    }
> +    
> +    errno=0;
> +    double number = strtod(arr[i], &err);
> +    if((arr[i] == err) || (errno == ERANGE && number == HUGE_VALF) || (errno == ERANGE && number == HUGE_VALL)
> +        || (errno == EINVAL) || (errno != 0 && number == 0) || (errno == 0 && arr[i] && *err != 0))
> +        return AVERROR_INVALIDDATA;
> +
> +    return 0;
> +}
> +
>  static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs)
>  {
>      HLSContext *hls = s->priv_data;
> @@ -1182,6 +1227,8 @@ static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs
>      char line[MAX_URL_SIZE];
>      const char *ptr;
>      const char *end;
> +    int is_discont_detected = 0;
> +    double discont_program_date_time = 0;
>  
>      if ((ret = ffio_open_whitelist(&in, url, AVIO_FLAG_READ,
>                                     &s->interrupt_callback, NULL,
> @@ -1211,6 +1258,7 @@ static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs
>          } else if (av_strstart(line, "#EXT-X-DISCONTINUITY", &ptr)) {
>              is_segment = 1;
>              vs->discontinuity = 1;
> +            is_discont_detected = 1;
>          } else if (av_strstart(line, "#EXTINF:", &ptr)) {
>              is_segment = 1;
>              vs->duration = atof(ptr);
> @@ -1236,7 +1284,31 @@ static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs
>                      av_strlcpy(vs->iv_string, ptr, sizeof(vs->iv_string));
>                  }
>              }
> +        } 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 (check_program_date_time(ptr) != 0) {
> +                av_log(hls, AV_LOG_VERBOSE, 
> +                       "Found invalid program date time %s when parsing playlist. "
> +                       "Current and subsequent existing segments will be ignored", ptr);
> +                ret = AVERROR_INVALIDDATA;
> +                goto fail;
> +            }
>  
> +            sscanf(ptr, "%d-%d-%dT%d:%d:%d.%lf", &y, &M, &d, &h, &m, &s, &ms);
> +            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);
> +            is_discont_detected = 0;
>          } else if (av_strstart(line, "#", NULL)) {
>              continue;
>          } else if (line[0]) {
> @@ -1250,8 +1322,9 @@ static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs
>                  is_segment = 0;
>                  new_start_pos = avio_tell(vs->avf->pb);
>                  vs->size = new_start_pos - vs->start_pos;
> -                vs->initial_prog_date_time -= vs->duration; // this is a previously existing segment
>                  ret = hls_append_segment(s, hls, vs, vs->duration, vs->start_pos, vs->size);
> +                vs->last_segment->discont_program_date_time = discont_program_date_time;
> +                discont_program_date_time += vs->duration;
>                  if (ret < 0)
>                      goto fail;
>                  vs->start_pos = new_start_pos;
> @@ -1572,7 +1645,11 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>          ret = ff_hls_write_file_entry(byterange_mode ? hls->m3u8_out : vs->out, en->discont, byterange_mode,
>                                        en->duration, hls->flags & HLS_ROUND_DURATIONS,
>                                        en->size, en->pos, hls->baseurl,
> -                                      en->filename, prog_date_time_p, en->keyframe_size, en->keyframe_pos, hls->flags & HLS_I_FRAMES_ONLY);
> +                                      en->filename,
> +                                      en->discont_program_date_time ? &en->discont_program_date_time : prog_date_time_p,
> +                                      en->keyframe_size, en->keyframe_pos, hls->flags & HLS_I_FRAMES_ONLY);
> +        if (en->discont_program_date_time)
> +            en->discont_program_date_time -= en->duration;
>          if (ret < 0) {
>              av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
>          }
> -- 
> 2.24.2 (Apple Git-127)
> 

In case you didn't see the email, this didn't build
https://patchwork.ffmpeg.org/check/21422/

libavformat/hlsenc.c: In function ‘check_program_date_time’:
libavformat/hlsenc.c:1184:5: error: ISO C90 forbids variable length array ‘s’ [-Werror=vla]
 1184 |     char s[strlen(prog_date_time)], *sptr0, *sptr1, *sptr2, *sptr3;
      |     ^~~~
Vignesh Ravichandran Dec. 1, 2020, 11:03 a.m. UTC | #5
Ping Steven, Andreas, Andriy and others,

I have pushed a patch with the sscanf check (
http://ffmpeg.org/pipermail/ffmpeg-devel/2020-December/273185.html) and
also verified that the build passes (
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20201201101040.28338-1-vignesh.ravichandran02@gmail.com/).
Could you please advise if this is acceptable?

Thanks,
Vignesh

On Fri, Nov 27, 2020 at 12:11 PM Steven Liu <lq@chinaffmpeg.org> wrote:

>
>
> > 2020年11月27日 下午2:36,Vignesh Ravichandran <
> vignesh.ravichandran02@gmail.com> 写道:
> >
> > av_isdigit seems to only check if the integer is from 0 to 9. So values
> > above 9 would fail the check, for example: 2020 would fail the check.
> Also,
> > we will not be able to use av_isdigit for checking the double value.
> >
> > On Fri, Nov 27, 2020 at 7:41 AM Steven Liu <lq@chinaffmpeg.org> wrote:
> >
> >>
> >>
> >>> 2020年11月26日 下午10:21,Vignesh Ravichandran <
> >> vignesh.ravichandran02@gmail.com> 写道:
> >>>
> >>> This is is due to the following behavior in the current code:
> >>> 1. The initial_prog_date_time gets set to the current local time
> >>> 2. The existing playlist (.m3u8) file gets parsed and the segments
> >> present are added to the variant stream
> >>> 3. The new segment is created and added
> >>> 4. The existing segments and the new segment are written to the
> playlist
> >> file. The initial_prog_date_time from point 1 is used for calculating
> >> "#EXT-X-PROGRAM-DATE-TIME" for the segments, which results in incorrect
> >> "#EXT-X-PROGRAM-DATE-TIME" values for existing segments
> >>>
> >>> The following approach fixes this bug:
> >>> 1. Add a new variable "discont_program_date_time" of type double to
> >> HLSSegment struct
> >>> 2. Store the "EXT-X-PROGRAM-DATE-TIME" value from the existing segments
> >> in this variable
> >>> 3. When writing to playlist file if "discont_program_date_time" is set,
> >> then use that value for "EXT-X-PROGRAM-DATE-TIME" else use the value
> >> present in vs->initial_prog_date_time
> >>>
> >>> Signed-off-by: Vignesh Ravichandran <vignesh.ravichandran02@gmail.com>
> >>> ---
> >>> libavformat/hlsenc.c | 83 ++++++++++++++++++++++++++++++++++++++++++--
> >>> 1 file changed, 80 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> >>> index cbfd8f7c0d..9346b2cfea 100644
> >>> --- a/libavformat/hlsenc.c
> >>> +++ b/libavformat/hlsenc.c
> >>> @@ -25,6 +25,10 @@
> >>> #include <stdint.h>
> >>> #if HAVE_UNISTD_H
> >>> #include <unistd.h>
> >>> +#include <string.h>
> >>> +#include <errno.h>
> >>> +#include <limits.h>
> >>> +#include <math.h>
> >>> #endif
> >>>
> >>> #if CONFIG_GCRYPT
> >>> @@ -88,6 +92,7 @@ typedef struct HLSSegment {
> >>>    char iv_string[KEYSIZE*2 + 1];
> >>>
> >>>    struct HLSSegment *next;
> >>> +    double discont_program_date_time;
> >>> } HLSSegment;
> >>>
> >>> typedef enum HLSFlags {
> >>> @@ -1124,6 +1129,7 @@ static int hls_append_segment(struct
> >> AVFormatContext *s, HLSContext *hls,
> >>>    en->keyframe_size     = vs->video_keyframe_size;
> >>>    en->next     = NULL;
> >>>    en->discont  = 0;
> >>> +    en->discont_program_date_time = 0;
> >>>
> >>>    if (vs->discontinuity) {
> >>>        en->discont = 1;
> >>> @@ -1148,7 +1154,8 @@ static int hls_append_segment(struct
> >> AVFormatContext *s, HLSContext *hls,
> >>>
> >>>    if (hls->max_nb_segments && vs->nb_entries >= hls->max_nb_segments)
> {
> >>>        en = vs->segments;
> >>> -        vs->initial_prog_date_time += en->duration;
> >>> +        if (!en->next->discont_program_date_time &&
> >> !en->discont_program_date_time)
> >>> +            vs->initial_prog_date_time += en->duration;
> >>>        vs->segments = en->next;
> >>>        if (en && hls->flags & HLS_DELETE_SEGMENTS &&
> >>> #if FF_API_HLS_WRAP
> >>> @@ -1173,6 +1180,44 @@ static int hls_append_segment(struct
> >> AVFormatContext *s, HLSContext *hls,
> >>>    return 0;
> >>> }
> >>>
> >>> +static int check_program_date_time(const char *prog_date_time) {
> >>> +    char s[strlen(prog_date_time)], *sptr0, *sptr1, *sptr2, *sptr3;
> >>> +    char *err = NULL;
> >>> +    char *arr[6] = {NULL};
> >>> +    int i = 0;
> >>> +    av_strlcpy(s, prog_date_time, strlen(prog_date_time));
> >>> +    char *p = strtok_r(strtok_r(s, "+", &sptr0), "T", &sptr1);
> >>> +    while (p) {
> >>> +        char *q = strtok_r(p, "-", &sptr2);
> >>> +        while (q) {
> >>> +            char *r = strtok_r(q, ":", &sptr3);
> >>> +            while (r) {
> >>> +                arr[i] = r;
> >>> +                i++;
> >>> +                r = strtok_r(NULL, ":", &sptr3);
> >>> +            }
> >>> +            q = strtok_r(NULL, "-", &sptr2);
> >>> +        }
> >>> +        p = strtok_r(NULL, "T", &sptr1);
> >>> +    }
> >>> +
> >>> +    for (i=0; i < 5; i++) {
> >>> +        errno=0;
> >>> +        long number = strtol(arr[i], &err, 10);
> >>> +        if((arr[i] == err) || (errno == ERANGE && number == LONG_MIN)
> >> || (errno == ERANGE && number == LONG_MAX)
> >>> +            || (errno == EINVAL) || (errno != 0 && number == 0) ||
> >> (errno == 0 && arr[i] && *err != 0))
> >> What about use av_isdigit to check them?
> >>> +            return AVERROR_INVALIDDATA;
> >>> +    }
> >>> +
> >>> +    errno=0;
> >>> +    double number = strtod(arr[i], &err);
> >>> +    if((arr[i] == err) || (errno == ERANGE && number == HUGE_VALF) ||
> >> (errno == ERANGE && number == HUGE_VALL)
> >>> +        || (errno == EINVAL) || (errno != 0 && number == 0) || (errno
> >> == 0 && arr[i] && *err != 0))
> >>> +        return AVERROR_INVALIDDATA;
> >>> +
> >>> +    return 0;
> >>> +}
> >>> +
> >>> static int parse_playlist(AVFormatContext *s, const char *url,
> >> VariantStream *vs)
> >>> {
> >>>    HLSContext *hls = s->priv_data;
> >>> @@ -1182,6 +1227,8 @@ static int parse_playlist(AVFormatContext *s,
> >> const char *url, VariantStream *vs
> >>>    char line[MAX_URL_SIZE];
> >>>    const char *ptr;
> >>>    const char *end;
> >>> +    int is_discont_detected = 0;
> >>> +    double discont_program_date_time = 0;
> >>>
> >>>    if ((ret = ffio_open_whitelist(&in, url, AVIO_FLAG_READ,
> >>>                                   &s->interrupt_callback, NULL,
> >>> @@ -1211,6 +1258,7 @@ static int parse_playlist(AVFormatContext *s,
> >> const char *url, VariantStream *vs
> >>>        } else if (av_strstart(line, "#EXT-X-DISCONTINUITY", &ptr)) {
> >>>            is_segment = 1;
> >>>            vs->discontinuity = 1;
> >>> +            is_discont_detected = 1;
> >>>        } else if (av_strstart(line, "#EXTINF:", &ptr)) {
> >>>            is_segment = 1;
> >>>            vs->duration = atof(ptr);
> >>> @@ -1236,7 +1284,31 @@ static int parse_playlist(AVFormatContext *s,
> >> const char *url, VariantStream *vs
> >>>                    av_strlcpy(vs->iv_string, ptr,
> >> sizeof(vs->iv_string));
> >>>                }
> >>>            }
> >>> +        } 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 (check_program_date_time(ptr) != 0) {
> >>> +                av_log(hls, AV_LOG_VERBOSE,
> >>> +                       "Found invalid program date time %s when
> parsing
> >> playlist. "
> >>> +                       "Current and subsequent existing segments will
> >> be ignored", ptr);
> >>> +                ret = AVERROR_INVALIDDATA;
> >>> +                goto fail;
> >>> +            }
> >>>
> >>> +            sscanf(ptr, "%d-%d-%dT%d:%d:%d.%lf", &y, &M, &d, &h, &m,
> >> &s, &ms);
>
> Ping Andreas & Other guys,
>
>         I cannot sure only check return value of sscanf is ok, or validate
> the string “ptr" before sscanf,
>         sscanf can return the result,  so do we need validate the string
> “ptr”?
>
>
> >>> +            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);
> >>> +            is_discont_detected = 0;
> >>>        } else if (av_strstart(line, "#", NULL)) {
> >>>            continue;
> >>>        } else if (line[0]) {
> >>> @@ -1250,8 +1322,9 @@ static int parse_playlist(AVFormatContext *s,
> >> const char *url, VariantStream *vs
> >>>                is_segment = 0;
> >>>                new_start_pos = avio_tell(vs->avf->pb);
> >>>                vs->size = new_start_pos - vs->start_pos;
> >>> -                vs->initial_prog_date_time -= vs->duration; // this is
> >> a previously existing segment
> >>>                ret = hls_append_segment(s, hls, vs, vs->duration,
> >> vs->start_pos, vs->size);
> >>> +                vs->last_segment->discont_program_date_time =
> >> discont_program_date_time;
> >>> +                discont_program_date_time += vs->duration;
> >>>                if (ret < 0)
> >>>                    goto fail;
> >>>                vs->start_pos = new_start_pos;
> >>> @@ -1572,7 +1645,11 @@ static int hls_window(AVFormatContext *s, int
> >> last, VariantStream *vs)
> >>>        ret = ff_hls_write_file_entry(byterange_mode ? hls->m3u8_out :
> >> vs->out, en->discont, byterange_mode,
> >>>                                      en->duration, hls->flags &
> >> HLS_ROUND_DURATIONS,
> >>>                                      en->size, en->pos, hls->baseurl,
> >>> -                                      en->filename, prog_date_time_p,
> >> en->keyframe_size, en->keyframe_pos, hls->flags & HLS_I_FRAMES_ONLY);
> >>> +                                      en->filename,
> >>> +                                      en->discont_program_date_time ?
> >> &en->discont_program_date_time : prog_date_time_p,
> >>> +                                      en->keyframe_size,
> >> en->keyframe_pos, hls->flags & HLS_I_FRAMES_ONLY);
> >>> +        if (en->discont_program_date_time)
> >>> +            en->discont_program_date_time -= en->duration;
> >>>        if (ret < 0) {
> >>>            av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get
> >> error\n");
> >>>        }
> >>> --
> >>> 2.24.2 (Apple Git-127)
> >>>
> >>> _______________________________________________
> >>> 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 Liu
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> > _______________________________________________
> > 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 Liu
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index cbfd8f7c0d..9346b2cfea 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -25,6 +25,10 @@ 
 #include <stdint.h>
 #if HAVE_UNISTD_H
 #include <unistd.h>
+#include <string.h>
+#include <errno.h>
+#include <limits.h>
+#include <math.h>
 #endif
 
 #if CONFIG_GCRYPT
@@ -88,6 +92,7 @@  typedef struct HLSSegment {
     char iv_string[KEYSIZE*2 + 1];
 
     struct HLSSegment *next;
+    double discont_program_date_time;
 } HLSSegment;
 
 typedef enum HLSFlags {
@@ -1124,6 +1129,7 @@  static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls,
     en->keyframe_size     = vs->video_keyframe_size;
     en->next     = NULL;
     en->discont  = 0;
+    en->discont_program_date_time = 0;
 
     if (vs->discontinuity) {
         en->discont = 1;
@@ -1148,7 +1154,8 @@  static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls,
 
     if (hls->max_nb_segments && vs->nb_entries >= hls->max_nb_segments) {
         en = vs->segments;
-        vs->initial_prog_date_time += en->duration;
+        if (!en->next->discont_program_date_time && !en->discont_program_date_time)
+            vs->initial_prog_date_time += en->duration;
         vs->segments = en->next;
         if (en && hls->flags & HLS_DELETE_SEGMENTS &&
 #if FF_API_HLS_WRAP
@@ -1173,6 +1180,44 @@  static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls,
     return 0;
 }
 
+static int check_program_date_time(const char *prog_date_time) {
+    char s[strlen(prog_date_time)], *sptr0, *sptr1, *sptr2, *sptr3;
+    char *err = NULL;
+    char *arr[6] = {NULL};
+    int i = 0;
+    av_strlcpy(s, prog_date_time, strlen(prog_date_time));
+    char *p = strtok_r(strtok_r(s, "+", &sptr0), "T", &sptr1);
+    while (p) {
+        char *q = strtok_r(p, "-", &sptr2);
+        while (q) {
+            char *r = strtok_r(q, ":", &sptr3);
+            while (r) {
+                arr[i] = r;
+                i++;
+                r = strtok_r(NULL, ":", &sptr3);
+            }
+            q = strtok_r(NULL, "-", &sptr2);
+        }
+        p = strtok_r(NULL, "T", &sptr1);
+    }
+    
+    for (i=0; i < 5; i++) {
+        errno=0;
+        long number = strtol(arr[i], &err, 10);
+        if((arr[i] == err) || (errno == ERANGE && number == LONG_MIN) || (errno == ERANGE && number == LONG_MAX) 
+            || (errno == EINVAL) || (errno != 0 && number == 0) || (errno == 0 && arr[i] && *err != 0))
+            return AVERROR_INVALIDDATA;
+    }
+    
+    errno=0;
+    double number = strtod(arr[i], &err);
+    if((arr[i] == err) || (errno == ERANGE && number == HUGE_VALF) || (errno == ERANGE && number == HUGE_VALL)
+        || (errno == EINVAL) || (errno != 0 && number == 0) || (errno == 0 && arr[i] && *err != 0))
+        return AVERROR_INVALIDDATA;
+
+    return 0;
+}
+
 static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs)
 {
     HLSContext *hls = s->priv_data;
@@ -1182,6 +1227,8 @@  static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs
     char line[MAX_URL_SIZE];
     const char *ptr;
     const char *end;
+    int is_discont_detected = 0;
+    double discont_program_date_time = 0;
 
     if ((ret = ffio_open_whitelist(&in, url, AVIO_FLAG_READ,
                                    &s->interrupt_callback, NULL,
@@ -1211,6 +1258,7 @@  static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs
         } else if (av_strstart(line, "#EXT-X-DISCONTINUITY", &ptr)) {
             is_segment = 1;
             vs->discontinuity = 1;
+            is_discont_detected = 1;
         } else if (av_strstart(line, "#EXTINF:", &ptr)) {
             is_segment = 1;
             vs->duration = atof(ptr);
@@ -1236,7 +1284,31 @@  static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs
                     av_strlcpy(vs->iv_string, ptr, sizeof(vs->iv_string));
                 }
             }
+        } 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 (check_program_date_time(ptr) != 0) {
+                av_log(hls, AV_LOG_VERBOSE, 
+                       "Found invalid program date time %s when parsing playlist. "
+                       "Current and subsequent existing segments will be ignored", ptr);
+                ret = AVERROR_INVALIDDATA;
+                goto fail;
+            }
 
+            sscanf(ptr, "%d-%d-%dT%d:%d:%d.%lf", &y, &M, &d, &h, &m, &s, &ms);
+            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);
+            is_discont_detected = 0;
         } else if (av_strstart(line, "#", NULL)) {
             continue;
         } else if (line[0]) {
@@ -1250,8 +1322,9 @@  static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs
                 is_segment = 0;
                 new_start_pos = avio_tell(vs->avf->pb);
                 vs->size = new_start_pos - vs->start_pos;
-                vs->initial_prog_date_time -= vs->duration; // this is a previously existing segment
                 ret = hls_append_segment(s, hls, vs, vs->duration, vs->start_pos, vs->size);
+                vs->last_segment->discont_program_date_time = discont_program_date_time;
+                discont_program_date_time += vs->duration;
                 if (ret < 0)
                     goto fail;
                 vs->start_pos = new_start_pos;
@@ -1572,7 +1645,11 @@  static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
         ret = ff_hls_write_file_entry(byterange_mode ? hls->m3u8_out : vs->out, en->discont, byterange_mode,
                                       en->duration, hls->flags & HLS_ROUND_DURATIONS,
                                       en->size, en->pos, hls->baseurl,
-                                      en->filename, prog_date_time_p, en->keyframe_size, en->keyframe_pos, hls->flags & HLS_I_FRAMES_ONLY);
+                                      en->filename,
+                                      en->discont_program_date_time ? &en->discont_program_date_time : prog_date_time_p,
+                                      en->keyframe_size, en->keyframe_pos, hls->flags & HLS_I_FRAMES_ONLY);
+        if (en->discont_program_date_time)
+            en->discont_program_date_time -= en->duration;
         if (ret < 0) {
             av_log(s, AV_LOG_WARNING, "ff_hls_write_file_entry get error\n");
         }