[FFmpeg-devel] [PATCH] lavf/segment: add reference_stream option
Stefano Sabatini
stefasab at gmail.com
Fri Dec 28 11:34:01 CET 2012
On date Thursday 2012-12-27 22:27:39 +0100, Stefano Sabatini encoded:
> 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.
>
> >
> > > > 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;
> > > > }
> > > > }
> > > >
> > [...]
> > > - int ret, i;
> > > + int ret, i, j;
> > >
> > > seg->segment_count = 0;
> > > if (!seg->write_header_trailer)
> > > @@ -398,14 +400,57 @@ static int seg_write_header(AVFormatContext *s)
> > > if (seg->list_type == LIST_TYPE_EXT)
> > > av_log(s, AV_LOG_WARNING, "'ext' list type option is deprecated in favor of 'csv'\n");
> > >
> > > - 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")) {
> > > + /* select first index of type with highest priority */
> > > + int type_index_map[AVMEDIA_TYPE_NB];
> > > + static const enum AVMediaType type_priority_list[] = {
> > > + AVMEDIA_TYPE_VIDEO,
> > > + AVMEDIA_TYPE_AUDIO,
> > > + AVMEDIA_TYPE_SUBTITLE,
> > > + AVMEDIA_TYPE_DATA,
> > > + AVMEDIA_TYPE_ATTACHMENT
> > > + };
> > > + enum AVMediaType type;
> > > +
> > > + for (i = 0; i < AVMEDIA_TYPE_NB; i++)
> > > + type_index_map[i] = -1;
> > > +
> > > + /* 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.
>
> > > +
> > > + for (i = 0; i < FF_ARRAY_ELEMS(type_priority_list); j++) {
> >
> > How does the variable j fit into the picture?
>
> Fixed.
>
> >
> > > + type = type_priority_list[i];
> > > + if ((seg->reference_stream_index = type_index_map[type]) >= 0)
> > > + break;
> > > + }
> >
> > Despite above comments it looks better this time. It still feels
> > a bit complex to me for a rather non-primary task.
>
> I tested it this time, updated patch in attachment. I'm going to push
> this tomorrow if I read no more comments, thanks for the reviews.
Applied.
--
FFmpeg = Funny Fast Mastodontic Portentous Enlightened Gigant
More information about the ffmpeg-devel
mailing list