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

[Stefano Sabatini]

On date Wednesday 2012-12-26 22:40:58 +0100, Alexander Strasser encoded:
> Hi Stefano,
> 
> Stefano Sabatini wrote:
> > On date Wednesday 2012-12-26 19:39:50 +0000, Paul B Mahol encoded:
> > > On 12/22/12, Stefano Sabatini <stefasab at gmail.com> wrote:
> [...]
> > > > diff --git a/libavformat/segment.c b/libavformat/segment.c
> > > > index dc9b9c8..b46de10 100644
> > > > --- a/libavformat/segment.c
> > > > +++ b/libavformat/segment.c
> [...]
> > > >
> > > > -    for (i = 0; i < s->nb_streams; i++)
> > > > -        seg->has_video +=
> > > > -            (s->streams[i]->codec->codec_type == AVMEDIA_TYPE_VIDEO);
> > > > +    seg->reference_stream_index = -1;
> > > > +    if (!strcmp(seg->reference_stream_specifier, "auto")) {
> > > > +        int first_id[AVMEDIA_TYPE_NB];
> > > > +
> > > > +        for (i = 0; i < AVMEDIA_TYPE_NB; i++)
> > > > +            first_id[i] = -1;
> > > > +
> > > > +        for (i = 0; i < s->nb_streams; i++) {
> > > > +            enum AVMediaType type_priority_list[] = {
> > > 
> > 
> > > static const?
> > 
> > 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).

> 
> int cur_reference_prio = 0;
> 
> for (i = 0; i < s->nb_streams; ++i) {
>    enum AVMediaType media_type = s->streams[i]->codec->codec_type; 
>    int prio;
> 
>    if (media_type < 0 || media_type >= AVMEDIA_TYPE_NB) {
>      continue;
>    }
>    prio = media_type_priority[media_type];
> 
>    if (prio > cur_reference_prio) {
>       seg->reference_stream_index = i;
>       cur_reference_prio = prio;
>    }
> }
> 
> > 
> > > Is this actually used?
> > 
> > See below.
> > 
> > > 
> > > > +                AVMEDIA_TYPE_VIDEO,
> > > > +                AVMEDIA_TYPE_AUDIO,
> > > > +                AVMEDIA_TYPE_SUBTITLE,
> > > > +                AVMEDIA_TYPE_DATA,
> > > > +                AVMEDIA_TYPE_ATTACHMENT
> > > > +            };
> > > > +            enum AVMediaType type = s->streams[i]->codec->codec_type;
> > > > +            first_id[type] = FFMAX(first_id[type], i);
> 
>   Would the FFMAX here usually work as expected?
> 
> > > > +
> > > > +            for (i = 0; i < FF_ARRAY_ELEMS(type_priority_list); i++)
> > > > +                if ((seg->reference_stream_index = first_id[i]) >= 0)
> > > > +                    break;
> > > > +        }
> > [...]

Updated with fixes and some cosmetics to make the code more readable.
-- 
FFmpeg = Fast and Fanciful Monstrous Pitiful Esoteric Gadget
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-lavf-segment-add-reference_stream-option.patch
Type: text/x-diff
Size: 6507 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121227/42e39ec6/attachment.bin>