On Fri, Sep 9, 2011 at 12:03 PM, Laurent Aimar <fen...@elivagar.org> wrote:
> Hi,
>
> On Fri, Sep 09, 2011 at 08:16:31AM +0200, Laurent Aimar wrote:
>> On Fri, Sep 09, 2011 at 02:26:53AM +0200, Michael Niedermayer wrote:
>> > On Fri, Sep 09, 2011 at 02:05:19AM +0200, Laurent Aimar wrote:
>> > [...]
>> > > > > @@ -311,7 +331,12 @@ static inline unsigned int
>> > > > > get_bits1(GetBitContext *s){
>> > > > > result <<= index & 7;
>> > > > > result >>= 8 - 1;
>> > > > > #endif
>> > > > > +#ifndef UNCHECK_BITSTREAM_READER
>> > > > > + if (index < s->size_in_bits)
>> > > > > + index++;
>> > > > > +#else
>> > > > > index++;
>> > > > > +#endif
>> > > >
>> > > > i think this will break error detection of some files with some
>> > > > decoders because they detect it by an overread having happened
>> > > >
>> > > > also it might lead to infinite loops or other unexpected things
>> > > > as some decoders depend on them
>> > > > hitting zero padding after the buffer which is guranteed to lead to
>> > > > vlc decoding failures for them as they have no valid all 0 vlc code
>> > > I see. A simple way could be to simply add 8 * PADDING_SIZE to the
>> > > check.
>> > > I will add that locally.
>> >
>> > Iam not sure this is enough
>> > the problematic case iam thinking of is a decoder that works with
>> > slices, so the guranteed 0 padding would be farther away if the
>> > current slice is not the last. mpeg1/2 should be examples of this
>> > case
>> >
>> > maybe just returning 0 after size+something would work better
>> I wanted to avoid the check while loading the cache, but you're right,
>> it's probably the simplest solution to avoid the issue you mentionned.
>> I will provide a patch to do that instead.
>
> New patch attached and it's a bit simpler.
>
> Now out of bound index is checked when reading the data and the value 0 is
> returned in such cases. get_bits_count() will then return the real number
> of bits read.
>
> I still have an issue with mpegaudio though. I will try to fix it first and
> then I will do some benchmarks.
>
You also broke with the following fate tests:
make: *** [fate-cljr] Error 1
make: *** [fate-creatureshock-avs] Error 1
make: *** [fate-ea-tqi-adpcm] Error 1
make: *** [fate-indeo2] Error 1
make: *** [fate-lossless-tta] Error 136
make: *** [fate-motionpixels] Error 1
make: *** [fate-wc3movie-xan] Error 139
make: *** [fate-ac3-5.1] Error 1
make: *** [fate-mp3-float-conf-compl] Error 1
make: *** [fate-mp3-float-conf-si] Error 1
make: *** [fate-vsynth1-ffv1] Error 1
make: *** [fate-vsynth2-ffv1] Error 1
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
