[FFmpeg-devel] [PATCH] Bugfix: decoding 8- and 24-bit FLAC files

[Michael Niedermayer]

Hi

On Sun, Jun 10, 2007 at 03:48:56PM +0100, Michael Donaghy wrote:
> Attached is a patch to fix two problems relating to decoding of FLAC files 
> with 8 and 24 (and I believe 20, though am unable to test this) bit samples; 
> also attached is a 5-second example of each for verification (test by "ffplay 
> file.flac" for ffplay built with and without this patch).

iam not sure if all subscribers are happy with receiving large sample files
over ffmpeg-dev ...


> 
> The original problem is the macro "DECORRELATE", lines 683-692 after patching 
> of libavcodec/flac.c, for dealing with the various types of "joint stereo" 
> frames:
> *(samples++) = (left ) >> (16 - s->bps);\
> *(samples++) = (right) >> (16 - s->bps);\
> This is supposed to bitshift two s->bps bit decoded samples into 16-bit format 
> for returning; however, it fails when s->bps is > 16 as in this case the 
> values need to be bitshifted to the right. The equivalent code for dealing 
> with non-joint stereo frames is encapsulated by the shift_to_16_bits 
> function:
>  *(samples++) = shift_to_16_bits(s->decoded[i][j], s->bps);
> Rather than maintaining two separate code paths I felt it best to reuse this 
> function, hence the two lines above become:
> *(samples++) = shift_to_16_bits(left , s->bps);\
> *(samples++) = shift_to_16_bits(right, s->bps);\
> Having changed this a second, related bug became apparent: the aforementioned 
> shift_to_16_bits (lines 571-582 after patching, of the same file) does not 
> handle 8-bit samples correctly (wheras the original joint stereo code did); 
> in keeping with the surrounding code I added the following code to handle 
> this case.
>     } else if (bps == 8) {
>         return (data << 8);
> The user-visible result of these changes is that 24-bit (and, without 
> verification, 20-bit) FLAC files making use of joint stereo frames (i.e. 
> encoded with flac -m or flac -M), and 8-bit FLAC files making use of 
> non-joint stereo frames (i.e. all 8-bit FLAC files) can now be played 
> correctly, where they previously could not. FLAC joint stereo frames with 
> 12-bit samples, if such exist, can no longer be played correctly, but since 
> all stereo FLAC files include non-joint stereo frames, such files are 
> unusable with or without this patch; if playback of 12-bit FLAC files is 
> desired this could (post-patch) be easily accomplished by adding 
> another "else if" clause to shift_to_16_bits().

why not replace all the shifts and shift_to_16_bits by
(x<<8)>>(24-bps)
?

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070611/3ec4b43e/attachment.pgp>