[FFmpeg-devel] [PATCH] Codec lookup: do not use codec_id

[Michael Niedermayer]

Hi

On Thu, Jul 05, 2007 at 02:02:42PM +0200, Nicolas George wrote:
> Hi.
> 
> Le septidi 17 messidor, an CCXV, Michael Niedermayer a ?crit?:
> > try to set it to jpeg5000
> 
> It fails with "Unknown codec 'jpeg5000'", just as the current version does.
> I am sorry, maybe I am missing something obvious, but I still do not
> understand what is wrong.
> 
> On the other hand, while thinking about your remark, I found a bug in my
> version, so it can not be applied as is anyway. It appears that fixing it
> would actually remove the global AVCodec pointers, but I am not sure that
> this is what you are objecting, since it is quite indirect.
> 
> In the current implementation:
> 
> 1. As soon as the user gives a -[vas]codec option, the name given is
>    searched in the table of all codecs. If it is not found, ffmpeg dies with
>    "unknown codec". If it is found, its CodecID is stored in a global
>    integer (which, later, will be copied in structure fields).
> 
> 2. Whenever the CodecID value is necessary, it is directly available.
> 
> 3. Whenever the AVCodec structure is necessary, it is searched in the table
>    of all codecs.
> 
> 4. Whenever the codec name is necessary, the AVCodec structure is searched in
>    the table of all codecs, and the name is taken there.
> 
> There is a common agreement that using the CodecID is a bad thing, because
> the mapping is not one-to-one. Here is what I indented to do:
> 
> 1. Instead of storing the codec_id field, we store a pointer to the AVCodec.
> 
> 2. The CodecID is available as the codec_id field.
> 
> 3. We already have the AVCodec structure.
> 
> 4. The codec name is available in the structure.
> 
> The bug in that is that, at step 1, we do not know yet if we need an encoder
> or a decoder. It happens to work because encoders appears before decoders in
> the table of all codecs, and the decoders still go through the CodecID
> lookup. It actually works, but I would not count on it.
> 
> I think I will do the following:
> 
> 1.0. When the user gives a -[vas]codec option, store the given name in a
>      global char *.
> 
> 1.5. When the global char * is to be stored in a structure field, do the
>      lookup (because at this point, we know if we need an encoder or a
>      decoder), and store a pointer to the AVCodec structure.
> 
> 2, 3, 4 stay same as above (patched version).
> 
> If you had time to read until here, could you tell me if you agree?

your suggestion is better then the last patch
still pleas keep in mind that not every string has a corresponding AVCodec
and thus the code must not depend on the existance of a AVCodec unless
one is really actually used for encoding/decoding that is for stream copy
it has to work with neither encoder nor decoder
and yes its very well possibe that this doesnt work currently either

but it should be possible to override the input codec and do stream copy
even if decoder or encoder AVCodec is missing

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070705/580c8499/attachment.pgp>