| Message ID | 1516432032-4465-1-git-send-email-redmcg@redmandi.dyndns.org |
|---|---|
| State | Accepted |
| Commit | 4e3e8980b58fc22eb41c0e3cd3392bb4e6ca0184 |
| Headers | show |
Just making sure this patch hasn't been overlooked. To better explain the existing issue - the value of 'pls->first_seq_no' comes from the 'startNumber' attribute of the 'SegmentTemplate' element. The 'pls->timelines' array represents the elements within the 'SegmentTimeline' element. So for this example: <SegmentTemplate timescale="48000" initialization="init-stream$RepresentationID$.m4s" media="chunk-stream$RepresentationID$-$Number%05d$.m4s" startNumber="7"> <SegmentTimeline> <S t="2196480" d="480256" r="1" /> </SegmentTimeline> </SegmentTemplate> 'pls->first_seq_no' will be 7 but the 'pls->timelines' array will only have the one element - hence: pls->timelines[pls->first_seq_no] causes a segfault. This patch removes the use of 'pls->first_seq_no' within the 'pls->timelines' array On 20/01/18 18:07, Brendan McGrath wrote: > If first_seq_no is not within the bounds of timelines then a segfault > will occur. > > This patch removes the use of first_seq_no within the timelines array > > It also adds first_seq_no to the value returned by calc_next_seg_no_from_timelines > (which allows for different values of 'startNumber') > > Signed-off-by: Brendan McGrath <redmcg@redmandi.dyndns.org> > --- > > This can be reproduced by creating a dash file with the following command: > ffmpeg -i in.ts -f dash -window_size 2 out.mpd > > and then after about 10 seconds run ffprobe against it. > > The SegementTemplatei of out.mpd will look something like this: > <SegmentTemplate timescale="48000" initialization="init-stream$RepresentationID$.m4s" media="chunk-stream$RepresentationID$-$Number%05d$.m4s" startNumber="7"> > <SegmentTimeline> > <S t="2196480" d="480256" r="1" /> > </SegmentTimeline> > </SegmentTemplate> > > So the code was trying to access the 7th element within the timeline > array (which only had one element). > > The patch doesn't worry about checking for a negative value after deducting the 60 > seconds as calc_next_seg_no_from_timelines accepts a signed int and works correctly with > negative values. > > libavformat/dashdec.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c > index 2492f1d..cdb7ba5 100644 > --- a/libavformat/dashdec.c > +++ b/libavformat/dashdec.c > @@ -1083,15 +1083,12 @@ static int64_t calc_cur_seg_no(AVFormatContext *s, struct representation *pls) > if (pls->n_fragments) { > num = pls->first_seq_no; > } else if (pls->n_timelines) { > - start_time_offset = get_segment_start_time_based_on_timeline(pls, 0xFFFFFFFF) - pls->timelines[pls->first_seq_no]->starttime; // total duration of playlist > - if (start_time_offset < 60 * pls->fragment_timescale) > - start_time_offset = 0; > - else > - start_time_offset = start_time_offset - 60 * pls->fragment_timescale; > - > - num = calc_next_seg_no_from_timelines(pls, pls->timelines[pls->first_seq_no]->starttime + start_time_offset); > + start_time_offset = get_segment_start_time_based_on_timeline(pls, 0xFFFFFFFF) - 60 * pls->fragment_timescale; // 60 seconds before end > + num = calc_next_seg_no_from_timelines(pls, start_time_offset); > if (num == -1) > num = pls->first_seq_no; > + else > + num += pls->first_seq_no; > } else if (pls->fragment_duration){ > if (pls->presentation_timeoffset) { > num = pls->presentation_timeoffset * pls->fragment_timescale / pls->fragment_duration;
> 在 2018年1月28日，上午9:09，Brendan McGrath <redmcg@redmandi.dyndns.org> 写道： > > Just making sure this patch hasn't been overlooked. > > To better explain the existing issue - the value of 'pls->first_seq_no' comes from the 'startNumber' attribute of the 'SegmentTemplate' element. > > The 'pls->timelines' array represents the elements within the 'SegmentTimeline' element. > > So for this example: > <SegmentTemplate timescale="48000" initialization="init-stream$RepresentationID$.m4s" media="chunk-stream$RepresentationID$-$Number%05d$.m4s" startNumber="7"> > <SegmentTimeline> > <S t="2196480" d="480256" r="1" /> > </SegmentTimeline> > </SegmentTemplate> > > 'pls->first_seq_no' will be 7 but the 'pls->timelines' array will only have the one element - hence: > pls->timelines[pls->first_seq_no] > will apply it > causes a segfault. > > This patch removes the use of 'pls->first_seq_no' within the 'pls->timelines' array > >> On 20/01/18 18:07, Brendan McGrath wrote: >> If first_seq_no is not within the bounds of timelines then a segfault >> will occur. >> >> This patch removes the use of first_seq_no within the timelines array >> >> It also adds first_seq_no to the value returned by calc_next_seg_no_from_timelines >> (which allows for different values of 'startNumber') >> >> Signed-off-by: Brendan McGrath <redmcg@redmandi.dyndns.org> >> --- >> >> This can be reproduced by creating a dash file with the following command: >> ffmpeg -i in.ts -f dash -window_size 2 out.mpd >> >> and then after about 10 seconds run ffprobe against it. >> >> The SegementTemplatei of out.mpd will look something like this: >> <SegmentTemplate timescale="48000" initialization="init-stream$RepresentationID$.m4s" media="chunk-stream$RepresentationID$-$Number%05d$.m4s" startNumber="7"> >> <SegmentTimeline> >> <S t="2196480" d="480256" r="1" /> >> </SegmentTimeline> >> </SegmentTemplate> >> >> So the code was trying to access the 7th element within the timeline >> array (which only had one element). >> >> The patch doesn't worry about checking for a negative value after deducting the 60 >> seconds as calc_next_seg_no_from_timelines accepts a signed int and works correctly with >> negative values. >> >> libavformat/dashdec.c | 11 ++++------- >> 1 file changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c >> index 2492f1d..cdb7ba5 100644 >> --- a/libavformat/dashdec.c >> +++ b/libavformat/dashdec.c >> @@ -1083,15 +1083,12 @@ static int64_t calc_cur_seg_no(AVFormatContext *s, struct representation *pls) >> if (pls->n_fragments) { >> num = pls->first_seq_no; >> } else if (pls->n_timelines) { >> - start_time_offset = get_segment_start_time_based_on_timeline(pls, 0xFFFFFFFF) - pls->timelines[pls->first_seq_no]->starttime; // total duration of playlist >> - if (start_time_offset < 60 * pls->fragment_timescale) >> - start_time_offset = 0; >> - else >> - start_time_offset = start_time_offset - 60 * pls->fragment_timescale; >> - >> - num = calc_next_seg_no_from_timelines(pls, pls->timelines[pls->first_seq_no]->starttime + start_time_offset); >> + start_time_offset = get_segment_start_time_based_on_timeline(pls, 0xFFFFFFFF) - 60 * pls->fragment_timescale; // 60 seconds before end >> + num = calc_next_seg_no_from_timelines(pls, start_time_offset); >> if (num == -1) >> num = pls->first_seq_no; >> + else >> + num += pls->first_seq_no; >> } else if (pls->fragment_duration){ >> if (pls->presentation_timeoffset) { >> num = pls->presentation_timeoffset * pls->fragment_timescale / pls->fragment_duration; >
>>> >>> libavformat/dashdec.c | 11 ++++------- >>> 1 file changed, 4 insertions(+), 7 deletions(-) >>> >>> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c >>> index 2492f1d..cdb7ba5 100644 >>> --- a/libavformat/dashdec.c >>> +++ b/libavformat/dashdec.c >>> @@ -1083,15 +1083,12 @@ static int64_t calc_cur_seg_no(AVFormatContext *s, struct representation *pls) >>> if (pls->n_fragments) { >>> num = pls->first_seq_no; >>> } else if (pls->n_timelines) { >>> - start_time_offset = get_segment_start_time_based_on_timeline(pls, 0xFFFFFFFF) - pls->timelines[pls->first_seq_no]->starttime; // total duration of playlist >>> - if (start_time_offset < 60 * pls->fragment_timescale) >>> - start_time_offset = 0; >>> - else >>> - start_time_offset = start_time_offset - 60 * pls->fragment_timescale; >>> - >>> - num = calc_next_seg_no_from_timelines(pls, pls->timelines[pls->first_seq_no]->starttime + start_time_offset); >>> + start_time_offset = get_segment_start_time_based_on_timeline(pls, 0xFFFFFFFF) - 60 * pls->fragment_timescale; // 60 seconds before end >>> + num = calc_next_seg_no_from_timelines(pls, start_time_offset); >>> if (num == -1) >>> num = pls->first_seq_no; >>> + else >>> + num += pls->first_seq_no; >>> } else if (pls->fragment_duration){ >>> if (pls->presentation_timeoffset) { >>> num = pls->presentation_timeoffset * pls->fragment_timescale / pls->fragment_duration; >> > > > Pushed Thanks Steven
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c index 2492f1d..cdb7ba5 100644 --- a/libavformat/dashdec.c +++ b/libavformat/dashdec.c @@ -1083,15 +1083,12 @@ static int64_t calc_cur_seg_no(AVFormatContext *s, struct representation *pls) if (pls->n_fragments) { num = pls->first_seq_no; } else if (pls->n_timelines) { - start_time_offset = get_segment_start_time_based_on_timeline(pls, 0xFFFFFFFF) - pls->timelines[pls->first_seq_no]->starttime; // total duration of playlist - if (start_time_offset < 60 * pls->fragment_timescale) - start_time_offset = 0; - else - start_time_offset = start_time_offset - 60 * pls->fragment_timescale; - - num = calc_next_seg_no_from_timelines(pls, pls->timelines[pls->first_seq_no]->starttime + start_time_offset); + start_time_offset = get_segment_start_time_based_on_timeline(pls, 0xFFFFFFFF) - 60 * pls->fragment_timescale; // 60 seconds before end + num = calc_next_seg_no_from_timelines(pls, start_time_offset); if (num == -1) num = pls->first_seq_no; + else + num += pls->first_seq_no; } else if (pls->fragment_duration){ if (pls->presentation_timeoffset) { num = pls->presentation_timeoffset * pls->fragment_timescale / pls->fragment_duration;
If first_seq_no is not within the bounds of timelines then a segfault will occur. This patch removes the use of first_seq_no within the timelines array It also adds first_seq_no to the value returned by calc_next_seg_no_from_timelines (which allows for different values of 'startNumber') Signed-off-by: Brendan McGrath <redmcg@redmandi.dyndns.org> --- This can be reproduced by creating a dash file with the following command: ffmpeg -i in.ts -f dash -window_size 2 out.mpd and then after about 10 seconds run ffprobe against it. The SegementTemplatei of out.mpd will look something like this: <SegmentTemplate timescale="48000" initialization="init-stream$RepresentationID$.m4s" media="chunk-stream$RepresentationID$-$Number%05d$.m4s" startNumber="7"> <SegmentTimeline> <S t="2196480" d="480256" r="1" /> </SegmentTimeline> </SegmentTemplate> So the code was trying to access the 7th element within the timeline array (which only had one element). The patch doesn't worry about checking for a negative value after deducting the 60 seconds as calc_next_seg_no_from_timelines accepts a signed int and works correctly with negative values. libavformat/dashdec.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)