[FFmpeg-devel] [PATCH v3] add put_bits_long to avoid undefined behaviour in put_bits

[Reimar Döffinger]

Hello,
I wanted to work on this, but...

On Sun, Sep 13, 2009 at 03:17:01PM +0200, matthieu castet wrote:
> Index: libavcodec/put_bits.h
> ===================================================================
> --- libavcodec/put_bits.h	(r?vision 19830)
> +++ libavcodec/put_bits.h	(copie de travail)
> @@ -143,7 +143,7 @@
>      int bit_left;
>  
>      //    printf("put_bits=%d %x\n", n, value);
> -    assert(n == 32 || value < (1U << n));
> +    assert(n <= 31 && value < (1U << n));
>  
>      bit_buf = s->bit_buf;
>      bit_left = s->bit_left;
> @@ -259,6 +259,22 @@
>      put_bits(pb, bits, val & ((1<<bits)-1));
>  }
>  
> +static inline void put_bits_long(PutBitContext *s, int n, unsigned int value)
> +{
> +    if(n <= 31) {
> +        put_bits(s, n, value);
> +    } else {
> +#ifdef ALT_BITSTREAM_READER_LE
> +        put_bits(s, 16, value & 0xffff);
> +        put_bits(s, 16, value >> 16);
> +#else
> +        put_bits(s, 16, value >> 16);
> +        put_bits(s, 16, value & 0xffff);
> +#endif
> +    }
> +
> +}

This only works for 0-32, and it is only used for exactly 32, so the
questions are
1) do we really need a put_bits_long, or can we just do a put_bits32?
2) if we add a put_bits_long, shouldn't it at least support up to 62
bits? The code would almost be the same, just the argument would be
uint64_t and the code would use
put_bits(s, 31, value & 0x7fffffff);
put_bits(s, n-31, value >> 31);
it would be a bit slower, but since it likely isn't speed-critical...