[FFmpeg-devel] [PATCH] fix raw FLAC muxer extradata handling

[Justin Ruggles]

Michael Niedermayer wrote:
> On Mon, Feb 23, 2009 at 08:40:18PM -0500, Justin Ruggles wrote:
>> Aurelien Jacobs wrote:
>>> On Sat, 21 Feb 2009 21:25:45 -0500
>>> Justin Ruggles <justin.ruggles at gmail.com> wrote:
>>>
>>>> Michael Niedermayer wrote:
>>>>> On Sat, Feb 21, 2009 at 04:27:54PM -0500, Justin Ruggles wrote:
>>>>> [...]
>>>>>> --- a/libavformat/raw.c
>>>>>> +++ b/libavformat/raw.c
>>>>>> @@ -31,12 +31,11 @@
>>>>>>  
>>>>>>  /* simple formats */
>>>>>>  #if CONFIG_FLAC_MUXER
>>>>>> -static int flac_write_header(struct AVFormatContext *s)
>>>>>> +int ff_flac_write_header(ByteIOContext *pb, AVCodecContext *codec)
>>>>>>  {
>>>>> i doubt a little that this belongs on raw.c/h
>>>> True, it would definitely make dependencies simpler if it were split it
>>>> into a separate file.  How about libavformat/flac.[ch]?
>>> Sounds good. Or you may also consider moving the whole flac muxer into
>>> a flacenc.c file.
>> [PATCH 1/6] Separate the raw FLAC muxer into its own file.
>> [PATCH 2/6] Use a shared function to validate FLAC extradata
>> [PATCH 3/6] cosmetics: line wrap and indentation
>> [PATCH 4/6] Add support for full header extradata to raw FLAC muxer
>> [PATCH 5/6] cosmetics: add a comment in flac_write_header().
>> [PATCH 6/6] Share the function to write a raw FLAC header and use in the
>> Matroska muxer.
>>
>> I have not attached patches 3, 4, and 5 since they have been approved
>> already.  Patches 4 and 5 are against libavformat/flacenc.c instead of
>> raw.c, but they are the same otherwise.
>>
>> In patch 1, I decided to split out the whole FLAC muxer into a new file.
>>  I copied raw_write_packet() since it's only 3 lines and might be
>> modified in the future for flac-specific stuff.  If this small amount of
>> duplication is not acceptable, I could make raw_write_packet() shared in
>> raw.c/h instead.  The patch doesn't reflect the fact that I would use
>> "svn copy"...I couldn't figure out how to get git to show that.
>>
>> Patch 2 is the same as before, but in flacenc.c/h.  I attached it
>> because it has not been completely approved yet.
>>
>> Patch 6 uses the shared function from flacenc.c/h, so the matroska muxer
>> does not need a dependency in configure.
>>
>> Thanks,
>> Justin
>>
> 
> [...]
> 
> 
>> diff --git a/libavcodec/flac.h b/libavcodec/flac.h
>> index 9a4f820..8af79f2 100644
>> --- a/libavcodec/flac.h
>> +++ b/libavcodec/flac.h
> [...]
>> diff --git a/libavcodec/flacdec.c b/libavcodec/flacdec.c
>> index d6e0c86..42310b2 100644
>> --- a/libavcodec/flacdec.c
>> +++ b/libavcodec/flacdec.c
> [...]
>> diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
>> index 093a07a..66950fa 100644
>> --- a/libavformat/flacenc.c
>> +++ b/libavformat/flacenc.c
> 
> maintained by you :)
> 
> 
> [...]
> 
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index e5e87f9..891b1b9 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
> [...]

OKed by Aurel.

>> diff --git a/libavformat/oggenc.c b/libavformat/oggenc.c
>> index 4fdae05..bbbee0a 100644
>> --- a/libavformat/oggenc.c
>> +++ b/libavformat/oggenc.c
> [...]
> 
> not maintained by me

OKed by Baptiste.

> nothing for me to review left :)

Patches applied.

Thanks,
Justin