[FFmpeg-devel] [PATCH] libavformat/dashdec: Support negative value of the @r attrbute of S in SegmentTimeline element

[sanilraut]

Thanks Moritz. I have re-submitted the patch.

Sanil

On Wed, Aug 8, 2018 at 1:04 AM, Moritz Barsnick <barsnick at gmx.net> wrote:

> On Mon, Aug 06, 2018 at 19:18:27 -0700, sanil wrote:
> > The following patch supports parsing negative value of the @r attribute
> of S in SegmentTimeline element.
> >
> > Example streams:
> > 1. http://dash.edgesuite.net/dash264/TestCases/1c/qualcomm/
> 1/MultiRate.mpd
> > 2. http://dash.edgesuite.net/dash264/TestCases/1c/qualcomm/
> 2/MultiRate.mpd
>
> I can confirm that the patch makes these streams (seem to) work, albeit
> with one warning message:
> [dash @ 0xba72680] Could not read complete fragment.
>
> But, for one, your patch doesn't apply to master anymore, due to recent
> changes. That's due to this change:
>
> > @@ -1952,7 +1964,7 @@ static int dash_read_header(AVFormatContext *s)
> >          ++stream_index;
> >      }
> >
> > -  c->is_init_section_common_audio = is_common_init_section_exist(c->audios,
> c->n_audios);
> > +    c->is_init_section_common_audio = is_common_init_section_exist(c->audios,
> c->n_audios);
> >
> >      for (i = 0; i < c->n_audios; i++) {
> >          struct representation *cur_audio = c->audios[i];
>
> While this indentation fix is correct, it doesn't belong into a
> functional fix patch. And it's not required anymore anyway, since
> commit 2f45378ba14417cbb4fc9494ba941cb06443c4f9.
>
> Furthermore:
>
> >          num = pls->first_seq_no + pls->n_timelines - 1;
> > -        for (i = 0; i < pls->n_timelines; i++) {
> > -            num += pls->timelines[i]->repeat;
> > +         for (i = 0; i < pls->n_timelines; i++) {
> > +            if (pls->timelines[i]->repeat == -1) {
>
> This also changes indentation, but incorrectly.
>
> > +                int length_of_each_segment =
> pls->timelines[i]->duration/pls->fragment_timescale;
> > +                num =  c->period_duration/length_of_each_segment;
>
> Operators are surrounded by spaces, e.g. "pls->timelines[i]->duration /
> pls->fragment_timescale".
>
> >          }
> > +
> >      } else if (c->is_live && pls->fragment_duration) {
>
> And here you're adding an empty line, which you also shouldn't do.
>
> Cheers,
> Moritz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>