[FFmpeg-devel] [PATCH] flac encoding, issue 526

[Justin Ruggles]

Michael Niedermayer wrote:
> On Sat, Jul 19, 2008 at 02:07:37PM -0400, Justin Ruggles wrote:
>> Justin Ruggles wrote:
>>> Justin Ruggles wrote:
>>>> Mathieu Velten wrote:
>>>>> new patch attached
>>>>>
>>>>> if ((data == NULL) || (avctx->frame_size != s->frame.blocksize)) {
>>>>>      write_streaminfo(s, avctx->extradata);
>>>>> }
>>>>>
>>>>> this is also execute during the first frame, so the header stored in
>>>>> extradata will have the length of the first frame instead of 0 during
>>>>> the encoding.
>>>>> is it a problem ?
>>>> Yes, it's a problem.  Check for the last frame at the beginning of
>>>> flac_encode_frame().
>>>>
>>>> int last_frame = (data == NULL) || (avctx->frame_size != s->frame.blocksize)
>>>>
>>>> ...
>>>>
>>>> if(last_frame)
>>>>     write_streaminfo(...)
>>> Oh, I see what you mean.  You also need to set s->frame.blocksize in
>>> flac_encode_init().
>> A simple solution might be something like:
>>
>> 1. add full_blocksize to the FlacEncodeContext
>> 2. set avctx->frame_size and s->full_blocksize at flac_encode_init()
>> 3. use s->full_blocksize to write the block size to the header in
>> write_streaminfo()
>> 4. check the current avctx->frame_size against s->full_blocksize to
>> check for last frame.
> 
> hm
> what if there is just one frame in the whole file?
> add CODEC_CAP_DELAY and you should always get a NULL frame, thats easier
> to test for

It would still work (I tested it), but if CODEC_CAP_DELAY allows you to
always get a last NULL frame, then it certainly makes things simpler.
The full block size would still need to be saved in the context though,
unless write_streaminfo() is updated to add an option to only update the
number of samples.  That option might be better.

Having a blocksize of 4608 in the streaminfo header and then only having
a single frame of smaller size is still a valid FLAC file.  Our FLAC
decoder doesn't seem to be able to handle it though.

-Justin