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

[Moritz Barsnick]

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