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

Nicolas George nicolas.george
Tue Jun 19 13:13:36 CEST 2007


Le decadi 30 prairial, an CCXV, Michael Niedermayer a ?crit?:
> why dont you replace the int *codec_id by a char * ?

I find more logical and efficient to do the as soon as possible rather than
having a call to avcodec_find_by_name each time the codec is used.

> this looks like an ugly hack

I agree with that, but the real hack was already there. More specifically:

- The current implementation uses the codec_id field of the AVCodecContext
  structure to store the CodecID of the codec it wants to use later.

- I want to use the codec field for the same purpose.

- avcodec_alloc_context sets the codec_id field to CODEC_ID_NONE and the
  codec field to NULL.

- For some reason, avcodec_open checks that the codec field is NULL, but not
  that the codec_id field is CODEC_ID_NONE.

I believe there is a small inconsistency there. The purpose of the check is:

# ------------------------------------------------------------------------
# r2251 | michaelni | 2003-09-10 10:20:14 +0200 (Wed, 10 Sep 2003) | 2 lines
# 
# detect avcodec_open() on an already opened AVCodecContext

I believe that the hack here is to use a field in the AVCodecContext
structure for something else than its purpose, but it is a harmless one.

> the != NULL is unneeded

As a rule, I only use "if(foo)" when foo is an integer variable whose sole
purpose is to hold a boolean. Everywhere else, I explicitely write "== 0" or
"== NULL". I believe it makes the intent of the code more apparent.

> and this change belongs to a seperate patch

Ok. I'll submit a new version when the problem above will be settled.
-------------- 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/20070619/6c28e506/attachment.pgp>



More information about the ffmpeg-devel mailing list