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

[Vitor Sessak]

Stefano Sabatini wrote:
> On date Sunday 2008-08-31 17:30:58 +0100, Robert Swain encoded:
>> 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. 
> 
> Just my 2 ?cent...
> 
> We could have a list of the format supported, with a name like
> AVCodecFormat
> or
> AVStreamFormat
> 
> unfortunately the term "format" creates confusion with the specific
> (and likely not-so-correct) use of term format done in libavformat.
> 
> Such structure could contain for example a name, long_name, fourcc,
> type, and a list of encoders and decoders supporting it (yes it could
> be more than one, think for example aac/libfaad, last time I tried to do:
> ffmpeg -acodec libfaad -i ~/test.mov -ar 22100 -vcodec libx264 -t 1.0 -y outfile.mp4
> I end up decoding with the native decoder...).

I understand the usefulness of your idea (one format, several decoders) 
but I don't see a good way of implementing it without adding another 
list of encoders/decoders (besides the list of formats) or anything else 
ugly.

> Also, maybe it would make sense to keep there the capabilities and the
> list of supported things (pix_fmts, framerates, sizes etc.).

I'm not sure if this last point is a good idea. Think of the case where 
have a format that our decoder support both stereo and 5.1 and our 
encoder support only stereo.

-Vitor