On Thu, Sep 08, 2011 at 10:01:54AM +0200, Laurent Aimar wrote:
> On Thu, Sep 08, 2011 at 09:23:56AM +0200, Kostya Shishkov wrote:
> > On Wed, Sep 07, 2011 at 10:08:37PM +0200, Laurent Aimar wrote:
> > > See patch.
> >
> > Last two chunks (zeroing samples_left when end of data is reported) is fine,
> > first chunks are a bit suspicious since they hit corner cases. For example,
> > what happens if block ends with a run of zeroes? Probably some
> > get_bits_left()
> > checks should be for strictly negative, not negative or zero.
> There is always a following read.
Ah, point taken.
> > @@ -360,9 +367,13 @@ static int wv_get_value(WavpackFrameContext *ctx,
> > GetBitContext *gb, int channel
> > }
> > if(!c->error_limit){
> > ret = base + get_tail(gb, add);
> > + if (get_bits_left(gb) <= 0)
> > + goto error;
> > }else{
> > int mid = (base*2 + add + 1) >> 1;
> > while(add > c->error_limit){
> > + if(get_bits_left(gb) <= 0)
> > + goto error;
> > if(get_bits1(gb)){
> > add -= (mid - base);
> > base = mid;
> I assume only this part was being criticised about. In this case, the
> original code is:
>
> if(!c->error_limit){
> ret = base + get_tail(gb, add);
> }else{
> int mid = (base*2 + add + 1) >> 1;
> while(add > c->error_limit){
> if(get_bits1(gb)){
> add -= (mid - base);
> base = mid;
> }else
> add = mid - base - 1;
> mid = (base*2 + add + 1) >> 1;
> }
> ret = mid;
> }
> sign = get_bits1(gb);
> where a bit is always read at least after the code I changed,
> so <= is the right test IMHO.
Indeed, patch is OK then.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel