[FFmpeg-devel] [PATCH] add tag/comment support to the raw flac demuxer

[Justin Ruggles]

Hi,

Michael Niedermayer wrote:
> On Thu, Dec 04, 2008 at 11:44:39AM -0800, Baptiste Coudurier wrote:
>> Michael Niedermayer wrote:
>>> On Thu, Dec 04, 2008 at 11:06:34AM -0800, Baptiste Coudurier wrote:
>>>> Hi,
>>>>
>>>> Michael Niedermayer wrote:
>>>>> On Mon, Dec 01, 2008 at 02:18:02PM -0800, Jim Radford wrote:
>>>>>> This patch adds support for parsing vorbis comments in plain flac
>>>>>> streams.  Only metadata packets are parsed leaving the frame data to
>>>>>> be parsed in raw 1024 byte chunks like before.
>>>>> this wont work with flac in any container short of raw flac.
>>>>> Thus IMO this is unacceptable
>>>>>
>>>> It's not like flac would be put in another container, except ogg and 
>>>> this would be a lot worse, and I personnally don't care about flac in ogg.
>>> flac in ogg is idiotic but isnt anyone recording music videos ... with
>>> lossless audio from a cd?
>> Yes, this is possible.
>>
>>> For that use case (and i do not know if anyone is actually using that use
>>> case) flac in some non ogg container would be a possible option
>> Wouldn't it be better to store the metadata at the container level in 
>> this case, because metadata would be related to video as well ? Just an 
>> idea.
> 
> true, still completely ignoring the headers does not seem ideal ...
> 
> 
>>>> I guess 90% of flac files are lossless cd audio ripping scene, and in 
>>>> this case this patch really adds a feature which can be removed later if 
>>>> someone volonteer to do something at codec level.
>>>>
>>>> This is IMHO acceptable, and I'd like it in svn.
>>> ill review it then
>> Thanks a lot, do we have another flac expert around here, by chance ? 
>> This would save you some time :>
> 
> maybe justin or you could review it :)

Ok, I'll chime in here.  I don't like the concept of the demuxer passing
the header through in the packet payload in this case.  The current FLAC
demuxer does this because it can't handle the header and the decoder
can.  If the demuxer is modified to handle the header, this becomes
unnecessary.  Just pass the needed data (STREAMINFO) to the decoder via
extradata.

Ideally, I would like to see shared functions to handle different types
of FLAC header blocks.  That way they could be easily reused by other
demuxers such as ogg and matroska.  Similarly, a generic function for
decoding the STREAMINFO header block could be used in lavc and shared by
the demuxer.  But only handling the vorbiscomment header block is at
least a good start.

-Justin