[FFmpeg-devel] [PATCH] flac demuxer
Michael Niedermayer
michaelni
Mon Apr 28 15:59:19 CEST 2008
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
> > * svn cp libavcodec/flac.c libavcodec/flacdec.c
> > * put common streaminfo parsing in lavc/flac.[c,h]
> >
> > Now exactly 34 bytes of extradata containing the streaminfo data will be
> > required by the decoder. The next step is making a FLAC parser so that
> > the decoder doesn't have to do that as well...
> >
> > This fixes issue#187.
>
> I've split the change into 7 patches. Hopefully this will be easier to
> review.
>
> -Justin
>
> diff --git a/libavcodec/flac.c b/libavcodec/flac.c
> index bebed03..b849e7f 100644
> --- a/libavcodec/flac.c
> +++ b/libavcodec/flac.c
> @@ -60,7 +60,7 @@ typedef struct FLACContext {
> GetBitContext gb;
>
> int min_blocksize, max_blocksize;
> - int min_framesize, max_framesize;
> + int max_framesize;
> int samplerate, channels;
> int blocksize/*, last_blocksize*/;
> int bps, curr_bps;
> @@ -120,7 +120,7 @@ static av_cold int flac_decode_init(AVCodecContext * avctx)
> static void dump_headers(FLACContext *s)
> {
> av_log(s->avctx, AV_LOG_DEBUG, " Blocksize: %d .. %d (%d)\n", s->min_blocksize, s->max_blocksize, s->blocksize);
> - av_log(s->avctx, AV_LOG_DEBUG, " Framesize: %d .. %d\n", s->min_framesize, s->max_framesize);
> + av_log(s->avctx, AV_LOG_DEBUG, " Max Framesize: %d\n", s->max_framesize);
> av_log(s->avctx, AV_LOG_DEBUG, " Samplerate: %d\n", s->samplerate);
> av_log(s->avctx, AV_LOG_DEBUG, " Channels: %d\n", s->channels);
> av_log(s->avctx, AV_LOG_DEBUG, " Bits: %d\n", s->bps);
> @@ -149,7 +149,7 @@ static void metadata_streaminfo(FLACContext *s)
> s->min_blocksize = get_bits(&s->gb, 16);
> s->max_blocksize = get_bits(&s->gb, 16);
>
> - s->min_framesize = get_bits_long(&s->gb, 24);
> + skip_bits(&s->gb, 24); /* skip min frame size */
> s->max_framesize = get_bits_long(&s->gb, 24);
>
> s->samplerate = get_bits_long(&s->gb, 20);
> --
> 1.5.3.5
>
this patch is ok
> diff --git a/libavcodec/flac.c b/libavcodec/flac.c
> index b849e7f..ed5b2d2 100644
> --- a/libavcodec/flac.c
> +++ b/libavcodec/flac.c
> @@ -40,13 +40,13 @@
> #include "bitstream.h"
> #include "golomb.h"
> #include "crc.h"
> +#include "flac.h"
>
> #undef NDEBUG
> #include <assert.h>
>
> #define MAX_CHANNELS 8
> #define MAX_BLOCKSIZE 65535
> -#define FLAC_STREAMINFO_SIZE 34
>
> enum decorrelation_type {
> INDEPENDENT,
> @@ -98,6 +98,36 @@ static void metadata_streaminfo(FLACContext *s);
> static void allocate_buffers(FLACContext *s);
> static int metadata_parse(FLACContext *s);
>
> +int ff_flac_parse_streaminfo(FLACStreaminfo *s, const uint8_t *buffer)
> +{
> + GetBitContext gbc;
> + init_get_bits(&gbc, buffer, FLAC_STREAMINFO_SIZE*8);
> +
> + s->min_block_size = get_bits(&gbc, 16);
> + s->max_block_size = get_bits(&gbc, 16);
> + if(s->max_block_size < 16)
> + return -1;
> +
> + skip_bits_long(&gbc, 24); // skip min frame size
> + s->max_frame_size = get_bits_long(&gbc, 24);
> +
> + s->sample_rate = get_bits_long(&gbc, 20);
> + if(s->sample_rate < 1 || s->sample_rate > 655350)
> + return -1;
> +
> + s->channels = get_bits(&gbc, 3) + 1;
> +
> + s->bps = get_bits(&gbc, 5) + 1;
> + if(s->bps < 8 || s->bps & 0x3)
> + return -1;
> +
> + s->total_samples = get_bits_long(&gbc, 36);
> +
> + // don't need to parse MD5 checksum
> +
> + return 0;
> +}
code duplication
[...]
> diff --git a/libavcodec/flac.c b/libavcodec/flac.c
> index ed5b2d2..4b51946 100644
> --- a/libavcodec/flac.c
> +++ b/libavcodec/flac.c
> @@ -59,11 +59,9 @@ typedef struct FLACContext {
> AVCodecContext *avctx;
> GetBitContext gb;
>
> - int min_blocksize, max_blocksize;
> - int max_framesize;
> - int samplerate, channels;
> + FLACStreaminfo info;
> int blocksize/*, last_blocksize*/;
> - int bps, curr_bps;
> + int curr_bps;
> enum decorrelation_type decorrelation;
>
> int32_t *decoded[MAX_CHANNELS];
> @@ -149,50 +147,43 @@ static av_cold int flac_decode_init(AVCodecContext * avctx)
>
> static void dump_headers(FLACContext *s)
> {
> - av_log(s->avctx, AV_LOG_DEBUG, " Blocksize: %d .. %d (%d)\n", s->min_blocksize, s->max_blocksize, s->blocksize);
> - av_log(s->avctx, AV_LOG_DEBUG, " Max Framesize: %d\n", s->max_framesize);
> - av_log(s->avctx, AV_LOG_DEBUG, " Samplerate: %d\n", s->samplerate);
> - av_log(s->avctx, AV_LOG_DEBUG, " Channels: %d\n", s->channels);
> - av_log(s->avctx, AV_LOG_DEBUG, " Bits: %d\n", s->bps);
> + av_log(s->avctx, AV_LOG_DEBUG, " Blocksize: %d .. %d (%d)\n", s->info.min_block_size, s->info.max_block_size, s->blocksize);
> + av_log(s->avctx, AV_LOG_DEBUG, " Max Framesize: %d\n", s->info.max_frame_size);
> + av_log(s->avctx, AV_LOG_DEBUG, " Samplerate: %d\n", s->info.sample_rate);
> + av_log(s->avctx, AV_LOG_DEBUG, " Channels: %d\n", s->info.channels);
> + av_log(s->avctx, AV_LOG_DEBUG, " Bits: %d\n", s->info.bps);
> }
>
> static void allocate_buffers(FLACContext *s){
> int i;
>
> - assert(s->max_blocksize);
> + assert(s->info.max_block_size);
>
> - if(s->max_framesize == 0 && s->max_blocksize){
> - s->max_framesize= (s->channels * s->bps * s->max_blocksize + 7)/ 8; //FIXME header overhead
> + if(s->info.max_frame_size == 0 && s->info.max_block_size){
> + s->info.max_frame_size= (s->info.channels * s->info.bps * s->info.max_block_size + 7)/ 8; //FIXME header overhead
> }
>
> - for (i = 0; i < s->channels; i++)
> + for (i = 0; i < s->info.channels; i++)
> {
> - s->decoded[i] = av_realloc(s->decoded[i], sizeof(int32_t)*s->max_blocksize);
> + s->decoded[i] = av_realloc(s->decoded[i], sizeof(int32_t)*s->info.max_block_size);
> }
>
> - s->bitstream= av_fast_realloc(s->bitstream, &s->allocated_bitstream_size, s->max_framesize);
> + s->bitstream= av_fast_realloc(s->bitstream, &s->allocated_bitstream_size, s->info.max_frame_size);
> }
i do not like having "info." added to half of all lines of code
[...]
> +/**
> + * Parse a list of metadata blocks.
> + */
> +static int metadata_parse(AVFormatContext *s)
> +{
> + uint8_t streaminfo_bytes[FLAC_STREAMINFO_SIZE];
> + int metadata_last, metadata_type, metadata_size;
> + uint8_t *block_data;
> + ByteIOContext *pb = s->pb;
> + AVStream *st = s->streams[0];
> + FLACDemuxContext *fc = st->priv_data;
> +
> + do {
> + metadata_type = get_byte(pb);
> + metadata_last = metadata_type >> 7;
> + metadata_type &= 0x7F;
> + metadata_size = get_be24(pb);
> +
> + av_log(s, AV_LOG_DEBUG,
> + " metadata block: flag = %d, type = %d, size = %d\n",
> + metadata_last, metadata_type, metadata_size);
> +
> + switch (metadata_type) {
> + case METADATA_TYPE_STREAMINFO:
> + if(metadata_size != FLAC_STREAMINFO_SIZE)
> + return -1;
> +
> + /* read streaminfo header */
> + get_buffer(pb, streaminfo_bytes, FLAC_STREAMINFO_SIZE);
> + if(ff_flac_parse_streaminfo(&fc->info, streaminfo_bytes))
> + return -1;
> +
> + /* copy streaminfo to codec extradata */
> + st->codec->extradata = av_mallocz(FLAC_STREAMINFO_SIZE + FF_INPUT_BUFFER_PADDING_SIZE);
> + if (!st->codec->extradata)
> + return AVERROR(ENOMEM);
> + memcpy(st->codec->extradata, streaminfo_bytes, FLAC_STREAMINFO_SIZE);
> + st->codec->extradata_size = FLAC_STREAMINFO_SIZE;
> +
> + /* set codec parameters */
> + st->codec->frame_size = fc->info.max_block_size;
> + st->codec->sample_rate = fc->info.sample_rate;
> + st->codec->channels = fc->info.channels;
> + st->codec->bits_per_sample = fc->info.bps;
> + break;
> +
> + case METADATA_TYPE_VORBIS_COMMENT:
> + block_data = av_mallocz(metadata_size + FF_INPUT_BUFFER_PADDING_SIZE);
> + if (!st->codec->extradata)
> + return AVERROR(ENOMEM);
> +
> + get_buffer(pb, block_data, metadata_size);
> + if(vorbis_comment(s, block_data, metadata_size)) {
> + av_freep(&block_data);
> + return -1;
> + }
> +
> + av_freep(&block_data);
> + break;
> +
> + default:
> + /* skip the metadata block */
> + if(metadata_type > METADATA_TYPE_PICTURE)
> + return -1;
> + url_fskip(pb, metadata_size);
> + }
> + } while (!metadata_last && !url_feof(pb));
> +
> + return 0;
> +}
> +
> /* flac read */
> static int flac_read_header(AVFormatContext *s,
> AVFormatParameters *ap)
[...]
> -static int metadata_parse(FLACContext *s)
> -{
> - int i, metadata_last, metadata_type, metadata_size, streaminfo_updated=0;
> -
> - if (show_bits_long(&s->gb, 32) == MKBETAG('f','L','a','C')) {
> - skip_bits(&s->gb, 32);
> -
> - av_log(s->avctx, AV_LOG_DEBUG, "STREAM HEADER\n");
> - do {
> - metadata_last = get_bits1(&s->gb);
> - metadata_type = get_bits(&s->gb, 7);
> - metadata_size = get_bits_long(&s->gb, 24);
> -
> - av_log(s->avctx, AV_LOG_DEBUG,
> - " metadata block: flag = %d, type = %d, size = %d\n",
> - metadata_last, metadata_type, metadata_size);
> - if (metadata_size) {
> - switch (metadata_type) {
> - case METADATA_TYPE_STREAMINFO:
> - metadata_streaminfo(s);
> - streaminfo_updated = 1;
> - break;
> -
> - default:
> - for (i=0; i<metadata_size; i++)
> - skip_bits(&s->gb, 8);
> - }
> - }
> - } while (!metadata_last);
> -
> - if (streaminfo_updated)
> - allocate_buffers(s);
> - return 1;
> - }
> - return 0;
> -}
> -
moving code cannot be split in a patch duplicating it and one removing the old
this is unacceptable. Its like using cvs add and remove instead of svn mv.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Republics decline into democracies and democracies degenerate into
despotisms. -- 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/20080428/59447bd6/attachment.pgp>
More information about the ffmpeg-devel
mailing list