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

[Robert Swain]

2008/8/31 Vitor Sessak <vitor1001 at gmail.com>:
> 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.

Some people may be interested in the FOURCC even if they know the codec.

>>> 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...

I like this idea.

Rob