[FFmpeg-devel] [RFC] stream parsing

[Baptiste Coudurier]

Hi guys,

On 9/26/10 3:36 PM, Michael Niedermayer wrote:
> On Sun, Sep 26, 2010 at 12:54:34PM +0200, Benjamin Larsson wrote:
>>
>>>
>>> this looks strange
>>> i would have expected a
>>> if((codec->capabilities&  CODEC_CAP_CHANNEL_CONF)&&  !channel_layout)
>>>      return 0;
>>>
>>> in has_codec_parameters()
>>
>>
>> There is no AVCodec available in has_codec_parameters() only an
>> AVCodecContext.
>>
>>>
>>> but maybe this doesnt work for some case?
>>
>> It wont work as it's the wrong struct.
>>
>>
>>>
>>> at least your "if we dont have all parameters remove these 3 parameters"
>>> is looking worse
>>>
>>
>> This is what the AAC codec uses, I'm not saying its correct I just took
>> something that worked. But IIRC something need to set these to 0 before
>> opening the decoder otherwise it will use the values the demuxer set.
>>
>> Anyway I redid the patch, it's abit more logical now but pretty much the
>> same.
>>
>> Btw in has_codec_parameters() there is a list of codecs with some
>> special casing, I think these codecs should have this new capability set.
>>
>> MvH
>> Benjamin Larsson
>
>>   libavcodec/avcodec.h |    5 +++++
>>   libavcodec/dca.c     |    1 +
>>   libavformat/utils.c  |   27 ++++++++++++++++++++-------
>>   3 files changed, 26 insertions(+), 7 deletions(-)
>> ab6630288f93c954106ef242caf7ab19da255387  codec_cap_channel_conf.diff
>> Index: libavcodec/avcodec.h
>> ===================================================================
>> --- libavcodec/avcodec.h	(revision 25194)
>> +++ libavcodec/avcodec.h	(working copy)
>> @@ -684,7 +684,12 @@
>>    * encoders
>>    */
>>   #define CODEC_CAP_EXPERIMENTAL     0x0200
>> +/**
>> + * Codec should fill in channel configuration and samplerate instead of container
>> + */
>> +#define CODEC_CAP_CHANNEL_CONF     0x0400
>>
>> +
>>   //The following defines may change, don't expect compatibility if you use them.
>>   #define MB_TYPE_INTRA4x4   0x0001
>>   #define MB_TYPE_INTRA16x16 0x0002 //FIXME H.264-specific
>> Index: libavcodec/dca.c
>> ===================================================================
>> --- libavcodec/dca.c	(revision 25194)
>> +++ libavcodec/dca.c	(working copy)
>> @@ -1500,4 +1500,5 @@
>>       .decode = dca_decode_frame,
>>       .close = dca_decode_end,
>>       .long_name = NULL_IF_CONFIG_SMALL("DCA (DTS Coherent Acoustics)"),
>> +    .capabilities = CODEC_CAP_CHANNEL_CONF,
>>   };
>> Index: libavformat/utils.c
>> ===================================================================
>> --- libavformat/utils.c	(revision 25194)
>> +++ libavformat/utils.c	(working copy)
>> @@ -1995,12 +1995,22 @@
>>   #endif
>>   }
>>
>> -static int has_codec_parameters(AVCodecContext *enc)
>> +static int has_codec_parameters(AVCodecContext *enc, AVCodec *codec)
>>   {
>>       int val;
>>       switch(enc->codec_type) {
>>       case AVMEDIA_TYPE_AUDIO:
>>           val = enc->sample_rate&&  enc->channels&&  enc->sample_fmt != SAMPLE_FMT_NONE;
>> +        /* Some containers report wrong information about channel configuration
>> +         * let the codecs handle it in those cases (dca for example)).
>> +         */
>> +        if (codec&&  codec->capabilities&  CODEC_CAP_CHANNEL_CONF) {
>> +            enc->sample_rate    = 0;
>> +            enc->channel_layout = 0;
>> +            enc->frame_size     = 0;
>> +            enc->channels       = 0;
>> +            return 0;
>> +        }
>
> av_find_stream_info() will return once all streams has_codec_parameters()
> return non zero
> this explicitly returns 0 unconditional on any parameters being available or
> not thus has_codec_parameters() will never return non zero for an affected
> stream, leading to exit only by bug and time/pos limits
>
> like i said the code should test if channel_layout has been set and
> return 0 if not it should not set any variable to 0 because this is unneeded
> that is unless it is actually needed but then id like to know why

I don't understand, why isn't the codec always overriding the container ?
After all that's what video decoders do with width/height.

In which case, the container would be right over the decoder ?

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org