[FFmpeg-devel] [PATCH 5/5] src_movie: implement multiple outputs.

[Nicolas George]

Le sextidi 6 thermidor, an CCXX, Stefano Sabatini a écrit :
> A stream spec may contain a program_id or a stream_id, I don't know if
> they could contain the '+' character.

They are integers too, for the moment at least.

> I suppose for the moment we can keep '+' and de-escape the spec list
> in case of need (e.g. by using av_get_token() instead of av_strtok()).

We can change that if it actually becomes necessary, since it requires more
code (malloc checking).

> (Unrelated note, may be useful to match the language of the stream in
> case it is specified).

I agree. In fact, any kind of metadata matching would be nice.

> doxy may help
> and here
> and here too

Added.

> >         av_log(log, AV_LOG_WARNING, "Stream specifier \"%s\" %s\n", spec,
> >                already ? "matched only already used streams" :
> matched only by already used stream?

I believe we usually say that a stream specifier matches a stream (or a
regexp matches a string), not the other way around.

> nit++: drop the final dot, should be more consistent with most libav* messages

Done.

> Also since this is a PAWE, maybe: "Loop with several streams is currently unsupported"

Adopted.

> can be simplified with the newly added avfilter_get_buffer_ref_from_frame()

Indeed.

> *to* the requested output.

Fixed.

> >     if (!ret)
> >         ret = pkt->size;
> why this?

Some video decoders return 0 for success instead of the number of used
bytes.

> &movie->pkt == pkt?
> In this case is less confusing to use just pkt->size consistently.

Changed.

New version attached.


> Unrelated, and feel free to not reply, I wonder how we should specify
> the options for the codec/format (this is especially relevant for a
> sink movie with multiple inputs). We could specify the options using
> an AVDictionary -> string serializer (like implemented in a recent

(about that patch, I believe it has not yet been pushed)

> patch), but then we need to specify the stream/format to which the
> options apply.
> 
> A possibility would be to apply stream specifier to options, for
> example:
> movie=a=1:v=1:
> fopts='probesize=1M, cryptokey=0xdeadbeef':
> copts='codec:a0=rawaudio, sample_fmt:a=u8, codec:v0=rawvideo, pix_fmt:v0=rgb24'

Using stream specifiers would be a good idea, especially since this is
consistent with what ffmpeg does.

OTOH, requiring the user to know the difference between a format option and
a stream option is not very practical. Especially since that difference may
be tricky: pix_fmt, for example, is actually an option of the rawvideo
format, not a stream option.

The best course of action is IMHO to do like ffmpeg: take all options
together and be smart about what they apply too. Due to the way the API is
done, this may not be too complicated: if we have the options in a dict, I
believe this would work:

  avformat_input_file(..., &dict);
    /* takes the format options, leaves what it does not know */
  avformat_find_stream_info(..., &dict); /* idem */
  for (all selected streams) {
      dict2 = filter_by_stream_specifier(dict);
      avcodec_open2(..., dict2);
  }

An additional bonus of doing that is that it does not require a second round
of escaping: the options can be inserted flatly into the movie argument.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: src_movie.c
Type: text/x-csrc
Size: 20070 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120723/56a12be1/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120723/56a12be1/attachment.asc>