Laurent Aimar <[email protected]> writes:

> On Mon, Dec 12, 2011 at 07:35:44AM -0800, Ronald S. Bultje wrote:
>> Hi guys,
>> 
>> the idea of this patch is very simple, and has existed in Chrome in a 
>> slightly
>> different form for quite a while: for each call to get_bits() and related
>> functions, check for overreads before advancing the frame pointer.
>> 
>> This protects against overreads in most decoders except those that use custom
>> bitreaders, like VP8 (which already does this), CABAC (which I'll do later) 
>> and
>> a few more. Speed depends on how many bits are read per frame, but ranges 
>> from
>> a 1-10% loss on low to ultra-high bitrate CAVLC H264 streams, to unnoticeable
>> losses of <0.1% on e.g. VC1 (because the DSP is less optimized, so the
>> bitreader overhead is relatively lower).
>> 
>> Design is that there's a configure option (currently default-off) to enable 
>> the
>> "safe" bitstream reader, and individual decoders that do bitstream checks
>> themselves can turn it off on a per-decoder basis.
>
>  I think that with your patch, get_bits_count() will never be negative and 
> this
> will creates issues with some decoders.
> You may want to read the following thread
>  "[PATCH] Checked get_bits.h functions to prevent overread" 
> (libav-devel/ffmpeg-devel)
> to see the associated discussions/arguments.
>
>  I attached the version of a similar patch that I came up with after taking
> thoses arguments into account. (I never created the configue part).
>
> Regards,
>
> -- 
> fenrir
>
>>From af1e5c3a989291a1e78b1f0dbeabf76f5e8bdfc6 Mon Sep 17 00:00:00 2001
> From: Laurent Aimar <[email protected]>
> 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(-)

This patch a) doesn't protect against extreme overflows, and b) probably
has more performance impact.

-- 
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to