[FFmpeg-devel] [PATCH] MPEG-4 ALS decoder MCC decoding bug (non-lossless output) (Fixes issue 2387)

[Thilo Borgmann]

Hi,

>> Just a little remark, if you would not have submitted a patch that successfully
>> decodes the sample you've uploaded, the original waveform would have been nice
>> to have, too.
> 
> Ok, I will do so next time. (Anyway you can always get back the original wave
> using the Reference Software decoder... ;))

... which is 100% bug free?

> In the future it might make more sense to duplicate the whole
> ALSBlockData structure per channel, since I think it would make
> the code shorter.

Maybe, patches welcome.

> 
> But for now, I attached a patch without those variables removed,
> and duplicated for all channels.

Review below:

> Index: libavcodec/alsdec.c
> ===================================================================
> --- libavcodec/alsdec.c	(revision 25831)
> +++ libavcodec/alsdec.c	(working copy)
> @@ -205,6 +205,11 @@ typedef struct {
>      uint8_t *bgmc_lut;              ///< pointer at lookup tables used for BGMC
>      int *bgmc_lut_status;           ///< pointer at lookup table status flags
used for BGMC
>      int ltp_lag_length;             ///< number of bits used for ltp lag value
> +    int *const_block;               ///< contains const_block flags for all
channels
> +    unsigned int *shift_lsbs;       ///< contains shift_lsbs flags for all
channels
> +    unsigned int *opt_order;        ///< contains opt_order flags for all
channels
> +    int32_t *const_val;             ///< contains sample values of constant
blocks for all channels

This is not what I wanted to say with my last remark. Storing the constant value
in ->raw_buffer like you did in the first place is ok!


> +    int *store_prev_samples;        ///< contains store_prev_samples flags
for all channels

Storing these per channel definitely removes a bug and complies to this patch,
so it is ok although I've no file in my test suite nor does your uploaded file
suffer from this buggy field. Ok.

All other changes ok.

Getting close!

-Thilo