[FFmpeg-devel] [RFC] stream parsing

[Michael Niedermayer]

On Mon, Sep 27, 2010 at 11:23:44AM -0700, Baptiste Coudurier wrote:
> On 09/27/2010 07:53 AM, Benjamin Larsson wrote:
>> On 09/27/2010 02:04 PM, Michael Niedermayer wrote:
>>> On Mon, Sep 27, 2010 at 12:59:27PM +0200, Benjamin Larsson wrote:
>>>
>>>> On 09/27/2010 12:39 PM, Michael Niedermayer wrote:
>>>>
>>>>>>> Please tell me if this works:
>>>>>>> @@ -2000,6 +2000,8 @@
>>>>>>>       int val;
>>>>>>>       switch(enc->codec_type) {
>>>>>>>       case AVMEDIA_TYPE_AUDIO:
>>>>>>> +        if(!enc->channel_layout&&  (!enc->codec || (enc->codec->capabilities&  CODEC_CAP_CHANNEL_CONF)))
>>>>>>> +            return 0;
>>>>>>>           val = enc->sample_rate&&  enc->channels&&  enc->sample_fmt != SAMPLE_FMT_NONE;
>>>>>>>           if(!enc->frame_size&&
>>>>>>>              (enc->codec_id == CODEC_ID_VORBIS ||
>>>>>>>
>>>>>>> if not, please tell me why it fails, ill fix it
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> Well it works if the container does not populate the channel_layout
>>>>>> (riff based ones do that if the field is there). So in that case the
>>>>>>
>>>>>>
>>>>> and is it wrong in the riff case?
>>>>> (if its wrong this could be argued to be a bug in the riff demuxer)
>>>>>
>>>>>
>>>> Well I wanted the  CODEC_CAP_CHANNEL_CONF flag to mean that whatever
>>>> channel configuration info is stored in the container, it should be
>>>> overridden by the codec info (ie sample_rate, channels, channel_layout).
>>>> It has nothing to do with if the riff demuxer is buggy or not.
>>>>
>>> well it matters because this flag is not stored per demuxer but codec
>>> we only need a single case where the demuxer is more accurate for this to
>>> break down
>>> or to see it differntly, if demuxer information is inaccurate this is a demuxer
>>> thing, its not a codec feature that the demuxer is wrong, its a codec feature
>>> if this information is stored in the codec.
>>>
>>>
>>>
>>>> In the riff case if the is a wavefmt_extensible header and the stored
>>>> codec is pcm then there is no other info then in the container, ie we
>>>> have to use that. But if the stored codec is dca we should use the info
>>>> from the codec as it is more reliable. Your patch does not cover that
>>>> scenario. I think that we should cover this scenario. My original patch
>>>> does cover this scanario and the same technique is used for AAC.
>>>>
>>> we could put the code in riff under !(capabilities&  CODEC_CAP_CHANNEL_CONF)
>>>
>>> that said i dont mind if we set things to 0 in av_find_stream_info() if it
>>> works better but your implementation of that is too convoluted for my taste
>>>
>>
>>
>> Is this too convoluted also ? Missing is a comment describing what it
>> does and how it works.

this looks good


>
> Suggestion: couldn't we just force decoding of at least one audio frame ?

i think this might break some things, like mp3 in avi where the
samplerate from teh avi header is actually correct and the mp3 stream is
wrong. (i dont have a test file but i think the avi header should be preferred
here)
but we can try this if anyway if benjamins patch unexpectedly turns out to
have issues


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100928/b7eb01c5/attachment.pgp>