[FFmpeg-devel] [PATCH] lavf/segment: add reference_stream option

[Alexander Strasser]

Stefano Sabatini wrote:
> On date Thursday 2012-12-27 01:04:59 +0100, Alexander Strasser encoded:
> > Stefano Sabatini wrote:
> > > On date Wednesday 2012-12-26 22:40:58 +0100, Alexander Strasser encoded:
> > > > Stefano Sabatini wrote:
> > [...]
> > > > > Will fix it.
> > > > > 
> > > > > > Move somewhere else?
> > > > > 
> > > > > I'd prefer to keep it in context.
> > > > 
> > > >   What about this (sorry neither compiled nor tested):
> > > > 
> > > 
> > > > /* higher value higher priority, 
> > > >    types not explicitly initialized will never be selected */
> > > > static const int media_type_priority[AVMEDIA_TYPE_NB] = {
> > > >   [AVMEDIA_TYPE_VIDEO]      = 5,
> > > >   [AVMEDIA_TYPE_AUDIO]      = 4,
> > > >   [AVMEDIA_TYPE_SUBTITLE]   = 3,
> > > >   [AVMEDIA_TYPE_DATA]       = 2,
> > > >   [AVMEDIA_TYPE_ATTACHMENT] = 1
> > > > };
> > > 
> > > I prefer to keep a list for semantical consistency (it is also easier
> > > to update).
> > 
> 
> >   Not sure I really understand; also updating should not be too frequent
> > action. But you mostly work on this code, so if you like it better it
> > should be OK.
> 
> I mean that the simplest way to represent a priority list is ... a
> list, thus I prefer to keep it that way.

  I disagree. But it doesn't matter and we are digressing :)

[...]
> > > +        /* select first index for each type */
> > > +        for (i = 0; i < s->nb_streams; i++) {
> > > +            type = s->streams[i]->codec->codec_type;
> > > +            if (type_index_map[type] == -1)
> > > +                type_index_map[type] = i;
> > > +        }
> > 
> 
> >   Is it safe enough to assume type can never be out of range here?
> 
> Good point.
>  
[...]
> +        /* select first index for each type */
> +        for (i = 0; i < s->nb_streams; i++) {
> +            type = s->streams[i]->codec->codec_type;
> +            if ((unsigned)type < AVMEDIA_TYPE_NB && type_index_map[type] == -1)
> +                type_index_map[type] = i;
> +        }

  Should be fine if it does not produce a compiler warning.

  Sorry I say it just now I wanted to mention it before but forgot. It
would probably be a good idea to put the auto stream index selection into
a static helper function and let it return the selected index or -1 in
case no index was selected. Could be done in a following commit though.

  No more remarks from me. I think the patch is fine to push now.

[...]

  Alexander