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

[Michael Niedermayer]

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
[...]
> 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

nothing for me to review left :)

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

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090224/8f28876d/attachment.pgp>