[FFmpeg-devel] [PATCH] flac demuxer

Justin Ruggles justinruggles
Sat May 3 15:26:29 CEST 2008


Michael Niedermayer wrote:
> On Sat, May 03, 2008 at 07:24:51AM -0400, Justin Ruggles wrote:
>> Michael Niedermayer wrote:
>>> On Fri, May 02, 2008 at 06:21:35PM -0400, Justin Ruggles wrote:
>>>> Michael Niedermayer wrote:
>>>>> On Mon, Apr 28, 2008 at 12:51:10AM -0400, Justin Ruggles wrote:
>>>>>> Justin Ruggles wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> This patch splits out the FLAC demuxer from raw.c.  It has added
>>>>>>> functionality to read the raw FLAC header, including all metadata.  The
>>>>>>> function to read the streaminfo header needs to be shared with the FLAC
>>>>>>> decoder, so it has been put in lavc.
>>>>>>>
>>>>>>> * svn cp libavformat/raw.c libavformat/flacdec.c
>>>> Here is a patch just to create a new flac demuxer file by copying the
>>>> relevant functions from raw.c.  Each function will be modified later to
>>>> be more specific to FLAC instead of the current generic raw demuxing.
>>> copying (code duplication), and later changing part of it really sounds
>>> ugly and wrong.
>>>
>>> If you want to split raw.c that surely can be done without any code
>>> clone and mutate, also spliting raw.c has nothing to to with flac.
>> I'm a bit confused. My goal is to put the flac demuxer in its own file
>> so I can add flac-specific header parsing (eventually seeking) without
>> cluttering up raw.c.  Would it be better to just write it from scratch
>> all at once, along with removal from raw.c instead of copying?
> 
> The problem is the code duplication not how you end up with code duplication.
> Explicitly copying code or implementing from scratch the same functions makes
> no difference.
> 
> You can add whatever functions are needed to raw.c. If you dislike this you
> have to split raw.c cleanly, copying whatever code you need into a new file
> so it exists twice is absolutely not ok.

ok. I think I'm starting to see what you're getting at. A lot of the
"skeleton" code of many of the demuxers is already the same. A proper
raw flac demuxer which does full header parsing and correct seeking
wouldn't really be much different except that there is already a
sub-optimal demuxer existing in raw.c.  What it comes down to is whether
the history should show an incremental rewrite (which is how I actually
did it), a totally new demuxer, or something in-between.

Essentially, raw FLAC is a raw container, but it also has a specific
header, including metadata and a seek table.  So the only function which
would have substantial duplicated code is raw_read_partial_packet().  In
fact the function could work just fine as-is, but it wouldn't be
necessarily be ideal to always read 1024 bytes for each packet if you
can determine the largest possible frame size from the header.  That
said, would it be better to just share that one function and reuse it in
a separate flac demuxer file?

Thanks,
Justin





More information about the ffmpeg-devel mailing list