[FFmpeg-devel] [PATCH] read header in FLAC demuxer

[Justin Ruggles]

Michael Niedermayer wrote:
> On Sat, Jan 31, 2009 at 10:47:53AM -0500, Justin Ruggles wrote:
>> M?ns Rullg?rd wrote:
>>> Justin Ruggles <justin.ruggles at gmail.com> writes:
>>>
>>>> M?ns Rullg?rd wrote:
>>>>> Michael Niedermayer <michaelni at gmx.at> writes:
>>>>>
>>>>>> On Fri, Jan 30, 2009 at 10:36:10PM -0500, Justin Ruggles wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Here is a patch to read the header in the raw FLAC demuxer.  Currently
>>>>>>> the header is passed through in the output AVPackets, which is not good
>>>>>>> and requires some ugliness on the decoder side.  After this patch is
>>>>>>> applied, the decoder can be changed so it doesn't have to handle the
>>>>>>> header except in the extradata.  The extradata handling in the decoder
>>>>>>> will also be simplified.
>>>>>> is it allowed to place the header in the stream instead of extradata in
>>>>>> flac in avi?
>>>>> If it is physically possible, someone will do it.
>>>> Should we really support everything that is physically possible?
>>> Of course not.
>>>
>>>> FLAC in avi and wav should not have the header in the stream.
>>> Flac in avi and wav simply shouldn't IMO.  In fact, avi shouldn't at
>>> all, and wav is only useful for PCM.
>>>
>>>>>> flac in mkv?
>>>>> No.
>>>>>
>>>>>> flac in * ?
>>>>> There's always Ogg...
>>>> Ogg doesn't use the raw FLAC header. It splits it up in its own funny
>>>> way, which we already support.
>>>>
>>>> Do we have any other format that is header+frames that we allow the
>>>> header to be in the stream?
>>> It's possible to do all sorts of funny things with MPEG-PS/TS.  Some
>>> variants of AAC encapsulation put (parts of) the header outside the
>>> main stream, some put it all inline.
>> But raw FLAC doesn't do any funny things... There is a header, which
>> contains metadata, information about the stream, seek table, etc... It
>> is always completely separate from the audio frames.  If we only handle
>> it in the parser or decoder, we lose the ability to use that information
>> in the demuxer, right?
> 
> i dont see why handling the seektable or others in the raw demuxer would
> affect if its passed on or not to the parser, of course theres no point
> on passing the seektable to the parser but still one could if one wanted

That is true. The issue is code duplication. It's either in lavf where
we can better utilize it, lavc where we can't, or both. It would require
messy workarounds to get it to work in one place with both and maybe
even unavoidable code duplication.  Passing the header would also mean
that a FLAC parser would have to handle the header as well.

> About the rest of the header i dont know how or where to handle it best
> but what you apparently propose is not adding any features its not making
> code simpler it just removes support for having the headers inline

This does add features. It reads the metadata and sets the stream
timebase and duration.

> If you want to add support for something iam very much in favor for that
> and if you explain how that might conflict with inline headers i also
> dont mind loosing support for them assuming such files are practically
> non existent but 
> Just removing it (or even worse in this patch duplicating it first)
> with no real explanation why is something that feels odd ...
> also moving code should never be split in duplication and removial

Ok, I can update the patch to remove support in the decoder at the same
time.

-Justin