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

Reply via email to