Hi,

2011/12/15 Måns Rullgård <[email protected]>

> "Ronald S. Bultje" <[email protected]> writes:
> > @@ -144,7 +164,13 @@ for examples see get_bits, show_bits, skip_bits,
> get_vlc
> >  # endif
> >
> >  // FIXME name?
> > +#if UNCHECKED_BITSTREAM_READER
> >  #   define SKIP_COUNTER(name, gb, num) name##_index += (num)
> > +#else
> > +#   define SKIP_COUNTER(name, gb, num) name##_index = \
> > +               (gb)->size_in_bits_plus1 - name##_index < (num) ? \
> > +               (gb)->size_in_bits_plus1 : name##_index + (num)
> > +#endif
>
> Thinking more carefully about it, this addition is overflow-safe since
> the skip amount must be small and we require end padding.  Your old
> version with FFMIN(size, index + num) is thus safe here after all,
> should it result in better code.
>

It does actually (one mov less, so only an added cmp and cmova), and
lightly faster on CAVLC/H264 (0.72s -> 0.83s instead of 0.86s on the
super-high-bitrate sample), so we're keeping the FFMIN() then.

> @@ -230,7 +263,12 @@ static inline int get_bits_count(const GetBitContext
> *s) {
> >  static inline void skip_bits_long(GetBitContext *s, int n){
> >      OPEN_READER(re, s);
> >      re_bit_count += n;
> > +#if UNCHECKED_BITSTREAM_READER
> >      re_buffer_ptr += re_bit_count>>5;
> > +#else
> > +    re_buffer_ptr = (re_bit_count >> 5) < (const uint32_t *)
> s->buffer_end - re_buffer_ptr ?
> > +                    re_buffer_ptr + (re_bit_count >> 5) : (const
> uint32_t *) s->buffer_end;
> > +#endif
>
> This is again guaranteed to fall within the padding region, so whatever
> variant gives better code is OK to use here.
>

gcc messes up and does 4 more instructions. Fortunately it's not terribly
important, but I'll stick to this:

[..]
      re_buffer_ptr += re_bit_count>>5;
+ #if !UNCHECKED_BITSTREAM_READER
+     re_buffer_ptr = FFMIN(re_buffer_ptr, s->buffer_end);
+ #endif

New patch attached. Documentation not yet updated until we decide whether
to enable by default or not.

Ronald

Attachment: 0001-get_bits-introduce-safe-bitreading-to-prevent-overre.patch
Description: Binary data

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to