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

[Nicolas George]

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?

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 185 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070705/9591abde/attachment.pgp>