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

[Kostya]

On Sat, May 02, 2009 at 07:10:28PM +0200, Laurent Aimar wrote:
> On Sat, May 02, 2009, Reimar D?ffinger wrote:
> > On Sat, May 02, 2009 at 12:07:42AM +0200, Laurent Aimar wrote:
> > > On Fri, May 01, 2009, Reimar D?ffinger 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?
> > >  There is a CRC anyway, but I added the check to ensure detecting error as
> > > soon as possible (and to avoid the possibility of a false positive CRC in this
> > > particuliar case).
> > 
> > I can't see how the CRC would avoid crashes. And actually I am not 100%
> > sure the committed code avoids crashes if the extrabits buffer is too
> > small (though it is possible it can't be too small because the
> > compressed data comes after it).
>  I thought that the bitstream layer ensured that no read could go beyond the
> provided buffer.
>  Looking at get_bits.h it does not seems to, so I have attached a patch that
> check that the provided buffer is wide enought.

Why not check it once during initialization? Decoder always read
extra_bits*channels*samples+32, doesn't it?
 
> Regards,
> 
> -- 
> fenrir