[FFmpeg-devel] [PATCH] avformat/hls: Fixes overwriting existing #EXT-X-PROGRAM-DATE-TIME value in HLS playlist Bug ID: 8989
Steven Liu
lq at chinaffmpeg.org
Fri Nov 27 08:41:22 EET 2020
> 2020年11月27日 下午2:36，Vignesh Ravichandran <vignesh.ravichandran02 at 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 at chinaffmpeg.org> wrote:
>
>>
>>
>>> 2020年11月26日 下午10:21，Vignesh Ravichandran <
>> vignesh.ravichandran02 at 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 at 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 at ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>>
>> Thanks
>>
>> Steven Liu
>>
>>
>>
>>
>>
>>
>>
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
Thanks
Steven Liu
More information about the ffmpeg-devel
mailing list