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

[Michael Niedermayer]

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
[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081007/a8634303/attachment.pgp>