[FFmpeg-devel] [PATCH] avformat/hls: Fixes overwriting existing #EXT-X-PROGRAM-DATE-TIME value in HLS playlist
Vignesh Ravichandran
vignesh.ravichandran02 at gmail.com
Thu Nov 26 16:58:31 EET 2020
Sounds good :D I have incorporated the changes and resubmitted the patch -
http://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272964.html
Please review.
Thanks,
Vignesh
On Thu, Nov 26, 2020 at 12:37 PM Steven Liu <lq at chinaffmpeg.org> wrote:
>
>
> > 2020年11月26日 下午12:43，Vignesh Ravichandran <
> vignesh.ravichandran02 at gmail.com> 写道：
> >
> > This check may not be enough. The below values would pass the check
> which should not be the case:
> > 20-11-26T08:34:00.000
> > 2020-11-26T8:34:00.000
> > 2020-11-26T08:60:00.000
> > 2020-11-26T08:60:0.000
> > 2020-13-26T08:34:00.000
> >
> > The regex check approach mentioned earlier checks:
> > 1. The value is in "YYYY-MM-DDThh:mm:ss.SSS" format (
> https://tools.ietf.org/html/draft-pantos-http-live-streaming-23#section-4.3.2.6
> )
> > 2. The values for YYYY, MM, DD, hh, mm, ss, SSS are valid. For example:
> the check would fail for MM = 00
>
> 1st split by T
> 2nd split by -
> 3rd split by :
> 4th the last value is double
>
> And check the value :D
>
> What about this?
> >
> > On Thu, Nov 26, 2020 at 8:00 AM Steven Liu <lq at chinaffmpeg.org> wrote:
> >
> >
> > > 2020年11月25日 下午9:57，Vignesh Ravichandran <
> vignesh.ravichandran02 at gmail.com> 写道：
> > >
> > > Sure, how about the snippet below:
> > >
> > > regex_t regex;
> > > regcomp(®ex,
> > >
> "([12][0-9]{3}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])T([01][1-9]|2[0-3]):([0-5][0-9]):([0-5][0-9]).([0-9]{3}))",
> > > REG_EXTENDED);
> > > if (regexec(®ex, ptr, 0, NULL, 0) == REG_NOMATCH) {
> > > ret = AVERROR_INVALIDDATA;
> > > goto fail;
> > > }
> >
> > Define a new function to check the value:
> > 1. Split the string by ‘-‘
> > 2. Check every value splited by ‘-‘
> > 3. If the value is int, that is correct result.
> > 4. Else not.
> > >
> > > On Wed, Nov 25, 2020 at 5:31 PM Steven Liu <lq at chinaffmpeg.org> wrote:
> > >
> > >>
> > >>
> > >>> 2020年11月25日 下午7:49，Vignesh Ravichandran <
> > >> vignesh.ravichandran02 at gmail.com> 写道：
> > >>>
> > >>> We could add a simple check like below to prevent it:
> > >>>
> > >>> if (sscanf(ptr, "%d-%d-%dT%d:%d:%d.%lf", &y, &M, &d, &h, &m, &s,
> &ms) !=
> > >> 7)
> > >>> {
> > >>> ret = AVERROR_INVALIDDATA;
> > >>> goto fail;
> > >>> }
> > >>>
> > >>> Please let me know.
> > >> Not sure this is better way, what about validate the string of the
> > >> "#EXT-X-PROGRAM-DATE-TIME” values？
> > >>>
> > >>>
> > >>> Thanks,
> > >>> Vignesh
> > >>>
> > >>> On Wed, Nov 25, 2020 at 11:29 AM Steven Liu <lq at chinaffmpeg.org>
> wrote:
> > >>>
> > >>>>
> > >>>>
> > >>>>> 2020年11月25日 下午1:16，Vignesh Ravichandran <
> > >>>> vignesh.ravichandran02 at gmail.com> 写道：
> > >>>>>
> > >>>>> Bug ID: 8989
> > >>>>>
> > >>>>> 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 | 35 +++++++++++++++++++++++++++++++----
> > >>>>> 1 file changed, 31 insertions(+), 4 deletions(-)
> > >>>>>
> > >>>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > >>>>> index cbfd8f7c0d..030a2d3b97 100644
> > >>>>> --- a/libavformat/hlsenc.c
> > >>>>> +++ b/libavformat/hlsenc.c
> > >>>>> @@ -88,6 +88,7 @@ typedef struct HLSSegment {
> > >>>>> char iv_string[KEYSIZE*2 + 1];
> > >>>>>
> > >>>>> struct HLSSegment *next;
> > >>>>> + double discont_program_date_time;
> > >>>>> } HLSSegment;
> > >>>>>
> > >>>>> typedef enum HLSFlags {
> > >>>>> @@ -1124,6 +1125,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 +1150,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
> > >>>>> @@ -1182,6 +1185,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 +1216,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 +1242,23 @@ 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)
> > >>>> && is_discont_detected) {
> > >>>>> + struct tm program_date_time;
> > >>>>> + int y,M,d,h,m,s;
> > >>>>> + double ms;
> > >>>>> + sscanf(ptr, "%d-%d-%dT%d:%d:%d.%lf", &y, &M, &d, &h,
> &m,
> > >>>> &s, &ms);
> > >>>> What will happen if I modify the value of “EXT-X-PROGRAM-DATE-TIME”
> by
> > >>>> hacker,
> > >>>> For example:
> > >>>> #EXT-X-PROGRAM-DATA-TIME: good-morning-ladies-and-gentlemen.
> > >>>>
> > >>>>> +
> > >>>>> + 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 +1272,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 +1595,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
> > >>
> > >>
> > >>
> > >> _______________________________________________
> > >> 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".
> > > _______________________________________________
> > > 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