[FFmpeg-devel] [PATCH] ffmpeg should look up AVCodec by name
Aurelien Jacobs
aurel
Mon Oct 6 02:19:57 CEST 2008
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...
> 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.
> and i think ive seen a similar patch already from someone else ...
I don't remember about this.
> [...]
>
> > Index: libavcodec/utils.c
> > ===================================================================
> > --- libavcodec/utils.c (revision 15552)
> > +++ libavcodec/utils.c (working copy)
> > @@ -1033,6 +1033,8 @@ AVCodec *avcodec_find_decoder(enum CodecID id)
> > AVCodec *avcodec_find_decoder_by_name(const char *name)
> > {
> > AVCodec *p;
> > + if (!name)
> > + return NULL;
> > p = first_avcodec;
> > while (p) {
> > if (p->decode != NULL && strcmp(name,p->name) == 0)
>
> where and why is null passed?
In opt_input_file(), I call this function without checking if name is
NULL. I could do the check in opt_input_file() (3 times), but it felt
cleaner this way.
> and if approved this should be a seperate commit
OK.
> > @@ -1051,7 +1053,9 @@ void avcodec_string(char *buf, int buf_size, AVCod
> > int bitrate;
> > AVRational display_aspect_ratio;
> >
> > - if (encode)
> > + if (enc->codec)
> > + p = enc->codec;
> > + else if (encode)
> > p = avcodec_find_encoder(enc->codec_id);
> > else
> > p = avcodec_find_decoder(enc->codec_id);
>
> iam not sure if this always prints what is expected, i mean
> the input should be printed as vorbis or vorbis decoded by libvorbis
> not just libvorbis ...
I personally would expect the actual encoder/decoder name printed here
instead of the codec name, but maybe I'm wrong. Anyway, this is minor
issue. I can remove this from the patch. This can be fixed latter.
Aurel
More information about the ffmpeg-devel
mailing list