[FFmpeg-devel] [PATCH] ffmpeg should look up AVCodec by name

[Aurelien Jacobs]

Michael Niedermayer wrote:

> On Mon, Oct 06, 2008 at 10:58:20PM +0200, Aurelien Jacobs wrote:
> > Michael Niedermayer wrote:
> > 
> > > On Mon, Oct 06, 2008 at 02:19:57AM +0200, Aurelien Jacobs wrote:
> > > > Michael Niedermayer wrote:
> > > > 
> > > > > On Mon, Oct 06, 2008 at 01:22:09AM +0200, Aurelien Jacobs wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > Curently, when specifying a codec with -[asv]codec to ffmpeg, it is
> > > > > > first converted to a CODEC_ID_* and then the coresponding AVCodec
> > > > > > is picked up. Problem arise when we have several AVCodec handling the
> > > > > > same CODEC_ID_*. eg: -acodec libvorbis select the internal vorbis
> > > > > > encoder instead of the libvorbis one...
> > > > > > Attached patch ensure we pick up AVCodec using the codec name, when
> > > > > > available. This allows to actually select the libvorbis encoder for
> > > > > > example. This works for both input and output codecs. This also
> > > > > > correctly display the selected codec.
> > > > > > 
> > > > > > Aurel
> > > > > 
> > > > > > Index: ffmpeg.c
> > > > > > ===================================================================
> > > > > > --- ffmpeg.c	(revision 15552)
> > > > > > +++ ffmpeg.c	(working copy)
> > > > > > @@ -1915,7 +1915,9 @@ static int av_encode(AVFormatContext **output_file
> > > > > >      for(i=0;i<nb_ostreams;i++) {
> > > > > >          ost = ost_table[i];
> > > > > >          if (ost->encoding_needed) {
> > > > > > -            AVCodec *codec;
> > > > > > +            AVCodec *codec = ost->st->codec->codec;
> > > > > 
> > > > > > +            ost->st->codec->codec = NULL;
> > > > > 
> > > > > why?
> > > > 
> > > > Because avcodec_open() fails if it's not NULL.
> > > > I guess the reason is to prevent calling avcodec_open() twice for the
> > > > same AVCodecContext.
> > > > Maybe avcodec_open() could be modified to not check this...
> > > 
> > > I do like to keep double call detection ...
> > 
> > OK.
> > 
> > > > > besides the solution looks hackish
> > > > 
> > > > In a way, it is, because it misuse ost->st->codec->codec to convey
> > > > the AVcodec selection inside ffmpeg.c.
> > > > But I haven't found a better way to convey this simply. At the time
> > > > the audio_codec_name is parsed, the corresponding AVInputStream is
> > > > still not allocated/initialized, so it can't be used. And when
> > > > AVInputStream is allocated, audio_codec_name is not available
> > > > anymore. But still this information must be conveyed on a per stream
> > > > basis.
> > > 
> > > why dont you use a static array?
> > 
> > That's indeed another possibility...
> > Done that way.
> > 
> > > > > and i think ive seen a similar patch already from someone else ...
> > > > 
> > > > I don't remember about this.
> > > 
> > > somewhere in the thread with subject
> > > "[FFmpeg-devel] [PATCH] Codec lookup: do not use codec_id"
> > > i think
> > 
> > Got it. I didn't remembered about it, and despite this, my patch was
> > really quite similar to the one of this old thread...
> > One difference is that my patch also allows selecting input codecs
> > by name (not only output codecs).
> > 
> > Attached an updated patch (the libavcodec/utils.c hunks are meant to
> > be applied separately).
> 
> patch ok if it has been tested with raw, non raw, stream copy, no stream copy
> and of course regression tests pass

All verified. Patch applied.

Aurel