[FFmpeg-devel] [PATCH] Added integer 32 bits support to wavpack

[Reimar Döffinger]

On Sat, May 02, 2009 at 02:56:02PM +0200, Reimar D?ffinger wrote:
> On Sat, May 02, 2009 at 12:02:00AM +0530, Jai Menon wrote:
> > On Fri, May 1, 2009 at 11:58 PM, Reimar D?ffinger
> > <Reimar.Doeffinger at gmx.de> wrote:
> > > On Fri, May 01, 2009 at 07:39:37PM +0300, Kostya wrote:
> > >> On Fri, May 01, 2009 at 06:08:37PM +0200, Laurent Aimar wrote:
> > >> > > I'd use s->crc_extra = AV_RL32(buf) and open bit buffer after that but this
> > >> > > looks fine too. Oh, and please add block size check.
> > >> > ?Using the bitstream reader here seems simpler to me, and additionnaly
> > >> > avoid the need of buffer size checks ;)
> > >>
> > >> It's not - if size <=4 (as WavPack source code says), it should be treated as
> > >> error, so check is needed anyway (not for bits reading though).
> > >
> > > Huh? Where does that idea that using the bitstream reader means you can
> > > skip the size checks come from?
> > > Is that why ALAC is so horribly broken in that regard?
> > 
> > Very likely. All input is done *solely* using get_bits, so adding
> > checks requires some hacks.
> 
> I don't know what you consider hacks, but attached patch e.g. fixes
> issue 758 (I sent it a long time ago to roundup and just tried again -
> it still seems to refuse any mail with a patch).
> The uncompressed case still needs to be fixed/optimized (and even the
> compressed case could be improved) but I can't see what would be so
> horrible about it that it would be better to have crashes.
> Actually I don't consider it bad enough to explain why it wasn't done
> in the first place.

> Index: libavcodec/alac.c
> ===================================================================
> --- libavcodec/alac.c	(revision 18377)
> +++ libavcodec/alac.c	(working copy)
> @@ -183,6 +183,9 @@
>          /* standard rice encoding */
>          int k; /* size of extra bits */
>  
> +        if (get_bits_count(&alac->gb) >= alac->gb.size_in_bits)
> +            break;
> +
>          /* read k, that is bits as is */
>          k = av_log2((history >> 9) + 3);
>          x= decode_scalar(&alac->gb, k, rice_kmodifier, readsamplesize);
> @@ -507,6 +510,8 @@
>              av_log(avctx, AV_LOG_ERROR, "FIXME: unimplemented, unhandling of wasted_bytes\n");
>  
>          for (chan = 0; chan < channels; chan++) {
> +            if (get_bits_count(&alac->gb) >= alac->gb.size_in_bits)
> +                break;
>              bastardized_rice_decompress(alac,
>                                          alac->predicterror_buffer[chan],
>                                          outputsamples,
> @@ -539,6 +544,8 @@
>          /* not compressed, easy case */
>          int i, chan;
>          for (i = 0; i < outputsamples; i++)
> +            if (get_bits_count(&alac->gb) >= alac->gb.size_in_bits)
> +                break;
>              for (chan = 0; chan < channels; chan++) {
>                  int32_t audiobits;

ALAC decoder is still without a single range check, is someone actually
maintaining it?
I not I intend to apply this soon, updated to use get_bits_left though.