[FFmpeg-cvslog] r11883 - trunk/libavcodec/vorbis.c

[Balatoni Denes]

Hi!

Sunday 10 February 2008 02:41-kor Loren Merritt ezt ?rta:
> On Sun, 10 Feb 2008, Balatoni Denes wrote:
> > Saturday 09 February 2008 14:53-kor lorenm ezt ?rta:
> >> Log:
> >> 30% faster ff_vorbis_floor1_render_list, 3% faster overall
> >>
> >>          if (flag[pos]) {
> >> -            render_line(lx, ly, list[pos].x, y_list[pos] * multiplier,
> >> out, samples); -            lx = list[pos].x;
> >> -            ly = y_list[pos] * multiplier;
> >> +            int x1 = list[pos].x;
> >> +            int y1 = y_list[pos] * multiplier;
> >> +            if (lx < samples)
> >> +                render_line(lx, ly, FFMIN(x1,samples), y1, out);
> >
> > However this change seems to be functionally different when x1>samples -
> > the steepness of the line in render_line  will be different, no? I
> > haven't checked in the spec whether "x1" can be greater than "samples",
> > though (probably it can be, as there has always been a check for this
> > kind of clipping in render_line and it's predecessors).
>
> In the vorbis spec, samples is not an argument to render_line. Afaict
> x1>samples isn't supposed to happen, the check is there only to prevent
> buffer overrun on invalid bitstreams.

I think x1>samples is allowed by the spec. While it's true that the 
render_line doesn't get samples as an argument, but on the other hand there 
is the last step in curve synthesis, which is:
 13) if ( [hx] is greater than [n] ) {

            14) truncate vector [floor] to [n] elements

     }


Also, the exact same bug (not checking n<samples) was noticed by Oded (see the 
mail on 2006. 09. 23. 16.44 by Oded "[Ffmpeg-devel] Bugs in Vorbis decoder", 
and it's answers), and acknowledged as a bug by me.

So I think the fix is not compliant with the vorbis spec.

> --Loren Merritt

bye
Denes