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.
Regards,
--
fenrir
>From af1e5c3a989291a1e78b1f0dbeabf76f5e8bdfc6 Mon Sep 17 00:00:00 2001
From: Laurent Aimar <fen...@videolan.org>
Date: Fri, 9 Sep 2011 00:52:36 +0200
Subject: [PATCH] Prevent by default for overread in get_bits.h functions.
It can be disabled (at compilation time so without any performance loss)
for decoder that check for overreads by themselves by defining
UNCHECKED_BITSTREAM_READER before including get_bits.h
Checks for A32_BITSTREAM_READER are not yet implemented.
---
libavcodec/get_bits.h | 34 ++++++++++++++++++++++++++++++----
1 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
index d2ae345..687169b 100644
--- a/libavcodec/get_bits.h
+++ b/libavcodec/get_bits.h
@@ -35,6 +35,8 @@
#include "libavutil/log.h"
#include "mathops.h"
+/* You can define UNCHECKED_BITSTREAM_READER before including this file to
+ * avoid the penalty cost of checking for overread */
#if defined(ALT_BITSTREAM_READER_LE) && !defined(ALT_BITSTREAM_READER)
# define ALT_BITSTREAM_READER
#endif
@@ -127,18 +129,37 @@ for examples see get_bits, show_bits, skip_bits, get_vlc
# define OPEN_READER(name, gb) \
unsigned int name##_index = (gb)->index; \
+ unsigned int av_unused name##_size_in_bits = (gb)->size_in_bits; \
unsigned int av_unused name##_cache = 0
# define CLOSE_READER(name, gb) (gb)->index = name##_index
# ifdef ALT_BITSTREAM_READER_LE
-# define UPDATE_CACHE(name, gb) \
- name##_cache = AV_RL32(((const uint8_t *)(gb)->buffer)+(name##_index>>3)) >> (name##_index&0x07)
+# ifdef UNCHECKED_BITSTREAM_READER
+# define UPDATE_CACHE(name, gb) \
+ name##_cache = AV_RL32(((const uint8_t *)(gb)->buffer)+(name##_index>>3)) >> (name##_index&0x07)
+# else
+# define UPDATE_CACHE(name, gb) do { \
+ if (name##_index < name##_size_in_bits) \
+ name##_cache = AV_RL32(((const uint8_t *)(gb)->buffer)+(name##_index>>3)) >> (name##_index&0x07); \
+ else \
+ name##_cache = 0; \
+ } while (0)
+# endif
# define SKIP_CACHE(name, gb, num) name##_cache >>= (num)
# else
-# define UPDATE_CACHE(name, gb) \
- name##_cache = AV_RB32(((const uint8_t *)(gb)->buffer)+(name##_index>>3)) << (name##_index&0x07)
+# ifdef UNCHECKED_BITSTREAM_READER
+# define UPDATE_CACHE(name, gb) \
+ name##_cache = AV_RB32(((const uint8_t *)(gb)->buffer)+(name##_index>>3)) << (name##_index&0x07)
+# else
+# define UPDATE_CACHE(name, gb) do {\
+ if (name##_index < name##_size_in_bits) \
+ name##_cache = AV_RB32(((const uint8_t *)(gb)->buffer)+(name##_index>>3)) << (name##_index&0x07); \
+ else \
+ name##_cache = 0; \
+ } while (0)
+# endif
# define SKIP_CACHE(name, gb, num) name##_cache <<= (num)
# endif
@@ -303,7 +324,12 @@ static inline void skip_bits(GetBitContext *s, int n){
static inline unsigned int get_bits1(GetBitContext *s){
#ifdef ALT_BITSTREAM_READER
unsigned int index = s->index;
+#ifdef UNCHECKED_BITSTREAM_READER
uint8_t result = s->buffer[index>>3];
+#else
+ uint8_t result = index < s->size_in_bits ? s->buffer[index>>3] : 0;
+#endif
+
#ifdef ALT_BITSTREAM_READER_LE
result >>= index & 7;
result &= 1;
--
1.7.2.5
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
