[FFmpeg-devel] ALAC encoder is not bitperfect

[Justin Ruggles]

Baptiste Coudurier wrote:
> On 4/13/2009 1:59 PM, Baptiste Coudurier wrote:
>> On 4/13/2009 1:50 PM, Justin Ruggles wrote:
>>> Jai Menon wrote:
>>>> On 4/13/09, Brent Huisman <brenthuisman at gmail.com> wrote:
>>>>> Hey Jai,
>>>>>
>>>>>  I've used several different builds, including the new ffmpeg 0.5. Any
>>>>>  and all versions I tried exhibit this behaviour. Also any and all
>>>>>  source wave files I use have this. Have you tried bitcomparing
>>>>>  yourself?
>>>> Yes, and the output is bitexact as far as I have seen. Please specify
>>>> what exactly you are using to bitcompare. If its foobar2k bitcompare,
>>>> then sadly thats a bug in fb2k's mp4 demuxer and should be reported to
>>>> them. I think Justin posted something in this regard to HydrogenAudio.
>>>> FFmpeg'a alac encoder writes out the no. of samples in every frame
>>>> correctly which fb2k discards and instead pads the frame with zeroes.
>>>> Try checking the bitexactness using itunes or ffalac.
>>> The specific issue seems to be a combination of our mp4 muxer and how
>>> fb2k handles it, but ALAC decoders other than fb2k read the actual ALAC
>>> frames to determine the number of samples, while fb2k uses info from the
>>> mp4 container.
>>>
>>> Here is a quote on Hydrogenaudio from a user named Gregory S. Chudov:
>>>
>>> ** start quote **
>>> There are two places in mp4 container, where the length is stored.
>>>
>>> First place is in moov.mvhd chunk (movie header).
>>> iTunes encoder writes the approximate number of samples there.
>>> ffmpeg encoder writes the approximate length in milliseconds.
>>> This is not very reliable field and is ignored by fb2k.
>> Well mvhd is scaled according to global timescale which is 1000, and set
>> accordingly.
>> I guess sample rate could be used if the file has only one audio track.
>>
>> But in any case, track time scale is sample rate and duration is in
>> samples number.
>>
>>> Second place is moov.trak.mdia.minf.stbl.stts (sample table).
>>> This is where iTunes encoder stores the correct length. This is what
>>> fb2k uses.
>>> This table contains array of struct { int sample_count; int
>>> sample_duration }
>>> Total length is a sum of sample_count*sample_duration.
>>> Normally for iTunes-encoded file this table contains two entries.
>>> First entry with sample_duration=4096 and sample_count=total_samples/4096
>>> Second entry with sample_duration=total_samples%4096 and sample_count=1
>>> For ffmpeg, this table sadly contains only one entry, so the total
>>> sample length is rounded up to a multiple of 4096.
>>> ** end quote **
>> Is alac frame size different for the last sample ? If not, then it is
>> _wrong_ to set it differently.
>>
> 
> Well, it seems indeed alac last frame size is smaller, so in this case
> pkt->duration must be set differently and this will be taken into
> account by muxer.
> 
> Patch attached.
> 
> Btw can somebody explain why avctx->frame_size is changed back to its
> old value ? Because that's what caused the problem, if frame_size was
> kept to the new size, libavformat would have computed the duration
> correctly.
> 
> But of course setting pkt->duration is the correct solution.

Wouldn't pkt->duration be rewritten in av_interleaved_write_frame()
based on the value of enc->frame_size? or am I missing something?

-Justin