[MPlayer-users] wrong length and bitrate with FLAC

[Reimar Döffinger]

On Sun, Nov 25, 2012 at 04:13:19PM +0100, Ilja Sekler wrote:
> Am 25.11.2012 12:59, schrieb Reimar Döffinger:
> 
> > On Fri, Nov 23, 2012 at 11:39:43AM +0100, Ilja Sekler wrote:
> > 
> >>>> <http://lists.mplayerhq.hu/pipermail/mplayer-users/2012-November/085621.html>,
> >>>> reverting the checkin from r35333 fixes the issue with long
> >>>> FLAC files.
> >>> 
> >>> That would be wrong, you just don't notice because now _your_
> >>> files are too short :-) (the issue would become visible for files
> >>> around 100 hours long).
> >> 
> >> It is IMHO quite safe to assume that no one would ever get FLAC
> >> files with such an incredible length :-) For real life cases, it
> >> just works right.
> > 
> > Yes, but there is a difference between "working right" and
> > "correct", the former one usually making the code a nightmare to
> > maintain in the long run.
> 
> I'm aware of it and very sorry if it sounded like neglecting flawless
> logic. The problem is that I mostly can't judge what is logically
> correct in C, but can easily see whether something is "working right"
> when using an application for my everyday computing needs.

In this case, the main reason is that coverity complained about the
code, and using stream_read_int24 to then discard everything but the
lowest 16 bits is senseless.

> Could you please shed a bit of light on how you calculated the lenght of
> a FLAC file sufficient to make things go wrong?

I can't fully parse that sentence.
So the thing is that the number of audio samples value in FLAC is 36
bits (i.e. 5 bytes but 4 bits used for something else).
The old code read this by reading 3 bytes and appending the next 2
bytes.
However of the first 3 bytes read it actually ended up discarding the
first byte, thus actually only using 32 of the 36 bits of the sample
count.
After my first "fix" that actually broke it, it would use all bytes
read, which is 5*8 = 40 bits. If those extra bits were not 0, things
would break.
The latest version will now instead read 1 byte, but discard the highest
4 bits, and then append the next 4 bytes, giving the desired 8 - 4 + 32
= 36 bits.