[FFmpeg-devel] [PATCH] start FLAC decoding mid-stream

[Justin Ruggles]

Michael Niedermayer wrote:
> On Sun, Dec 07, 2008 at 05:32:02PM -0500, Justin Ruggles wrote:
>> Justin Ruggles wrote:
>>> Hi,
>>>
>>> As mentioned in another thread, our FLAC decoder cannot start decoding
>>> mid-stream without having the STREAMINFO.  The FLAC format allows for
>>> doing this as long as each frame header supplies all the necessary info.
>>>
>>> Here is a patch to allow decoding starting anywhere in a FLAC stream.
>>>
>>> Thanks,
>>> Justin
>> Here is a slightly better version which only allocates buffers once if
>> starting mid-stream.
>>
>> -Justin
>>
> 
>> Index: libavcodec/flac.c
>> ===================================================================
>> --- libavcodec/flac.c	(revision 16030)
>> +++ libavcodec/flac.c	(working copy)
>> @@ -102,6 +102,11 @@ static av_cold int flac_decode_init(AVCodecContext
>>      FLACContext *s = avctx->priv_data;
>>      s->avctx = avctx;
>>  
>> +    s->min_blocksize = 16;
>> +    s->max_blocksize = 32768;
> 
>> +    s->samplerate = -1;
>> +    s->channels = -1;
>> +    s->bps = -1;
> 
> is 0 for these not good enough?

yeah, 0 is good enough. I'll change it.

> 
>>      if (avctx->extradata_size > 4) {
>>          /* initialize based on the demuxer-supplied streamdata header */
>>          if (avctx->extradata_size == FLAC_STREAMINFO_SIZE) {
>> @@ -131,7 +136,7 @@ static void allocate_buffers(FLACContext *s){
>>  
>>      assert(s->max_blocksize);
>>  
>> -    if(s->max_framesize == 0 && s->max_blocksize){
>> +    if(s->max_blocksize){
>>          s->max_framesize= (s->channels * s->bps * s->max_blocksize + 7)/ 8; //FIXME header overhead
>>      }
>>  
> 
> why?

After explaining it to myself several times (it's been a long day), it's
clear to me that the crux of the issue is that currently the STREAMINFO
can contain a 0 for max_framesize, indicating unknown, so that section
is a fallback.  We need to ALWAYS use the fallback when we don't have a
STREAMINFO, but max_framesize is set to non-zero before we get to this
point.  So now I think a better way to solve it might be to set
max_framesize to zero when allocate_buffers == 1 in decode_frame().
That would effectively mimic a 0 value in the STREAMINFO.

> 
>> @@ -495,12 +500,25 @@ static int decode_frame(FLACContext *s, int alloc_
>>  {
>>      int blocksize_code, sample_rate_code, sample_size_code, assignment, i, crc8;
>>      int decorrelation, bps, blocksize, samplerate;
>> +    int allocate_buffers = 0;
>>  
>>      blocksize_code = get_bits(&s->gb, 4);
>>  
>>      sample_rate_code = get_bits(&s->gb, 4);
>>  
>>      assignment = get_bits(&s->gb, 4); /* channel assignment */
>> +    if (s->channels < 0)
>> +    {
>> +        if (assignment < 8)
>> +        {
>> +            s->channels = assignment + 1;
>> +        }
>> +        else
>> +        {
>> +            s->channels = 2;
>> +        }
>> +        allocate_buffers = 1;
>> +    }
> 
> why is this under channels < 0 ?

The idea was that the number of channels shouldn't change mid-stream.
That is not currently supported, but I could fabricate a test stream and
try to support it.  Then again, I don't know if ffmpeg/ffplay would
support it either.  It is handled in AC3 by silently changing the mixing
within the decoder, but I don't think that would be desirable for a
lossless codec.

> 
> [...]
>> @@ -570,6 +605,12 @@ static int decode_frame(FLACContext *s, int alloc_
>>          av_log(s->avctx, AV_LOG_ERROR, "illegal sample rate code %d\n", sample_rate_code);
>>          return -1;
>>      }
>> +    if (s->samplerate < 0) {
>> +        s->avctx->sample_rate = samplerate;
> 
>> +    } else if (samplerate != s->avctx->sample_rate) {
>> +        av_log(s->avctx, AV_LOG_ERROR, "cannot change sample rate mid-stream\n");
>> +        return -1;
>> +    }
>>  
>>      skip_bits(&s->gb, 8);
>>      crc8 = av_crc(av_crc_get_table(AV_CRC_8_ATM), 0,
> 
> what is the problem with it changing?

Same deal as channels (and bps for that matter).  I can try to add
support for it.  It would be interesting to see how libFLAC handles it,
so I'll give it a shot.

> 
>> @@ -584,6 +625,9 @@ static int decode_frame(FLACContext *s, int alloc_
>>      s->bps          = bps;
>>      s->decorrelation= decorrelation;
>>  
>> +    if (allocate_buffers)
>> +        return 1;
>> +
>>  //    dump_headers(s->avctx, (FLACStreaminfo *)s);
>>  
>>      /* subframes */
>> @@ -658,11 +702,17 @@ static int flac_decode_frame(AVCodecContext *avctx
>>              goto end; // we may not have enough bits left to decode a frame, so try next time
>>          }
>>          skip_bits(&s->gb, 16);
>> -        if (decode_frame(s, alloc_data_size) < 0){
>> -            av_log(s->avctx, AV_LOG_ERROR, "decode_frame() failed\n");
>> +        tmp = decode_frame(s, alloc_data_size);
>> +        if (tmp) {
>>              s->bitstream_size=0;
>>              s->bitstream_index=0;
> 
>> +            if (tmp < 0) {
>> +            av_log(s->avctx, AV_LOG_ERROR, "decode_frame() failed\n");
> 
> one of these 2 lines is not correctly indented

Yeah, one of the lines is just moved, not modified, so I didn't change
the indentation... either way it would be fixed in a follow-up commit.
Plus that whole section is currently incorrectly indented, so I was
going to correct that too.

Thanks,
Justin