[FFmpeg-devel] [PATCH] Make a clear distinction between an unsupported codec and an unknown one

[Vitor Sessak]

Robert Swain wrote:
> 2008/8/31 Peter Ross <pross at xvid.org>:
>> On Sun, Aug 31, 2008 at 12:53:34PM +0200, Michael Niedermayer wrote:
>>> On Sun, Aug 31, 2008 at 06:56:48AM +0200, Vitor Sessak wrote:
>>>> See $subj. I got pretty confused by "Stream #0.1: Audio: 0x0000, 5512 Hz,
>>>> mono, s16" meaning that a codec is known, but unsupported...
>>>>
>>>> -Vitor
>>>> Index: libavcodec/utils.c
>>>> ===================================================================
>>>> --- libavcodec/utils.c      (revision 15050)
>>>> +++ libavcodec/utils.c      (working copy)
>>>> @@ -1091,8 +1091,12 @@
>>>>                       (enc->codec_tag >> 16) & 0xff,
>>>>                       (enc->codec_tag >> 24) & 0xff,
>>>>                        enc->codec_tag);
>>>> +        } else if (enc->codec_id) {
>>>> +            snprintf(buf1, sizeof(buf1), "unsuported (id 0x%04x)", enc->codec_id);
>>>> +        } else if (enc->codec_tag) {
>>>> +            snprintf(buf1, sizeof(buf1), "unknown (0x%04x)", enc->codec_tag);
>>>>          } else {
>>>> -            snprintf(buf1, sizeof(buf1), "0x%04x", enc->codec_tag);
>>>> +            snprintf(buf1, sizeof(buf1), "unknown");
>>>>          }
>>> I think this is inconsistant now
>>> Before it just printed the tag now it prints the tag when its
>>> printable, if not and codec_id is not 0 the id and if not and
>>> id is 0 and the tag is not 0 the tag and ...
>>> Printing the ID in addition to the tag and "NONE" instead of
>>> 0x0000 or so seems more consistent to me

IMHO, when we have a codec ID, the codec is already identified and there 
is no use printing the 4CC. Yes, I should move them the "if 
(enc->codec_id)" up.

>> An enhancement to the unsupported case would be to actually print
>> the name of the codec. e.g. i am always forgetting which 0x16X
>> twocc refers to wmav3.
> 
> Indeed. In my opinion it should print the codec name if possible and

Today we have:

AVCodec flac_decoder = {
     "flac",
     CODEC_TYPE_AUDIO,
     CODEC_ID_FLAC,
     sizeof(FLACContext),
     flac_decode_init,
     NULL,
     flac_decode_close,
     flac_decode_frame,
     CODEC_CAP_DELAY,
     .flush= flac_flush,
     .long_name= NULL_IF_CONFIG_SMALL("FLAC (Free Lossless Audio Codec)"),
};

AVCodec flac_encoder = {
     "flac",
     CODEC_TYPE_AUDIO,
     CODEC_ID_FLAC,
     sizeof(FlacEncodeContext),
     flac_encode_init,
     flac_encode_frame,
     flac_encode_close,
     NULL,
     .capabilities = CODEC_CAP_SMALL_LAST_FRAME,
     .sample_fmts = (enum SampleFormat[]){SAMPLE_FMT_S16,SAMPLE_FMT_NONE},
     .long_name = NULL_IF_CONFIG_SMALL("FLAC (Free Lossless Audio Codec)"),
};

enum CodecID {
     CODEC_ID_NONE,

     /* video codecs */
     CODEC_ID_MPEG1VIDEO,
[...]

     CODEC_ID_FLAC,
[...]
}

Note the duplication of both the short and the long name. I think that 
setting the codec long and short name in the same place of CodecID would 
remove duplication and resolve the issue of finding the name of 
unimplemented codecs...

-Vitor