Re: [FFmpeg-devel] [PATCH 2/4] lavc: add standalone cached bitstream reader

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Anton Khirnov:
> From: Alexandra Hájková <alexandra.khirnova@gmail.com>
> 
> The cached bitstream reader was originally written by Alexandra Hájková
> for Libav, with significant input from Kostya Shishkov and Luca Barbato.
> It was then committed to FFmpeg in ca079b09549, by merging it with the
> implementation of the current bitstream reader.
> 
> This merge makes the code of get_bits.h significantly harder to read,
> since it now contains two different bitstream readers interleaved with
>  #ifdefs. Additionally, the code was committed without proper authorship
> attribution.
> 
> This commit re-adds the cached bitstream reader as a standalone header,
> as it was originally developed. It will be made useful in following
> commits.
> 
> Integration by Anton Khirnov.
> 
> Signed-off-by: Anton Khirnov <anton@khirnov.net>
> ---
>  libavcodec/bitstream.h | 490 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 490 insertions(+)
>  create mode 100644 libavcodec/bitstream.h
> 
> diff --git a/libavcodec/bitstream.h b/libavcodec/bitstream.h
> new file mode 100644
> index 0000000000..8a710bcecc
> --- /dev/null
> +++ b/libavcodec/bitstream.h
> @@ -0,0 +1,490 @@
> +/*
> + * Copyright (c) 2016 Alexandra Hájková
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * bitstream reader API header.
> + */
> +
> +#ifndef AVCODEC_BITSTREAM_H
> +#define AVCODEC_BITSTREAM_H
> +
> +#include <stdint.h>
> +
> +#include "config.h"
> +
> +#include "libavutil/common.h"
> +#include "libavutil/intreadwrite.h"
> +#include "libavutil/log.h"
> +
> +#include "mathops.h"
> +#include "vlc.h"
> +
> +#ifndef UNCHECKED_BITSTREAM_READER
> +#define UNCHECKED_BITSTREAM_READER !CONFIG_SAFE_BITSTREAM_READER
> +#endif
> +
> +typedef struct BitstreamContext {
> +    uint64_t bits;      // stores bits read from the buffer
> +    const uint8_t *buffer, *buffer_end;
> +    const uint8_t *ptr; // pointer to the position inside a buffer
> +    unsigned bits_left; // number of bits left in bits field
> +    unsigned size_in_bits;
> +} BitstreamContext;
> +
> +static inline void refill_64(BitstreamContext *bc)
> +{
> +#if !UNCHECKED_BITSTREAM_READER
> +    if (bc->ptr >= bc->buffer_end)
> +        return;
> +#endif
> +
> +#ifdef BITSTREAM_READER_LE
> +    bc->bits       = AV_RL64(bc->ptr);
> +#else
> +    bc->bits       = AV_RB64(bc->ptr);
> +#endif
> +    bc->ptr       += 8;
> +    bc->bits_left  = 64;
> +}
> +
> +static inline void refill_32(BitstreamContext *bc)
> +{
> +#if !UNCHECKED_BITSTREAM_READER
> +    if (bc->ptr >= bc->buffer_end)
> +        return;
> +#endif
> +
> +#ifdef BITSTREAM_READER_LE
> +    bc->bits       = (uint64_t)AV_RL32(bc->ptr) << bc->bits_left | bc->bits;
> +#else
> +    bc->bits       = bc->bits | (uint64_t)AV_RB32(bc->ptr) << (32 - bc->bits_left);
> +#endif
> +    bc->ptr       += 4;
> +    bc->bits_left += 32;
> +}
> +
> +/**
> + * Initialize BitstreamContext.
> + * @param buffer bitstream buffer, must be AV_INPUT_BUFFER_PADDING_SIZE bytes
> + *        larger than the actual read bits because some optimized bitstream
> + *        readers read 32 or 64 bits at once and could read over the end
> + * @param bit_size the size of the buffer in bits
> + * @return 0 on success, AVERROR_INVALIDDATA if the buffer_size would overflow.
> + */
> +static inline int bitstream_init(BitstreamContext *bc, const uint8_t *buffer,
> +                                 unsigned int bit_size)
> +{
> +    unsigned int buffer_size;
> +
> +    if (bit_size > INT_MAX - 7 || !buffer) {
> +        bc->buffer    = NULL;
> +        bc->ptr       = NULL;
> +        bc->bits_left = 0;
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    buffer_size = (bit_size + 7) >> 3;
> +
> +    bc->buffer       = buffer;
> +    bc->buffer_end   = buffer + buffer_size;
> +    bc->ptr          = bc->buffer;
> +    bc->size_in_bits = bit_size;
> +    bc->bits_left    = 0;
> +    bc->bits         = 0;
> +
> +    refill_64(bc);
> +
> +    return 0;
> +}
> +
> +/**
> + * Initialize BitstreamContext.
> + * @param buffer bitstream buffer, must be AV_INPUT_BUFFER_PADDING_SIZE bytes
> + *        larger than the actual read bits because some optimized bitstream
> + *        readers read 32 or 64 bits at once and could read over the end
> + * @param byte_size the size of the buffer in bytes
> + * @return 0 on success, AVERROR_INVALIDDATA if the buffer_size would overflow
> + */
> +static inline int bitstream_init8(BitstreamContext *bc, const uint8_t *buffer,
> +                                  unsigned int byte_size)
> +{
> +    if (byte_size > INT_MAX / 8)
> +        return AVERROR_INVALIDDATA;
> +    return bitstream_init(bc, buffer, byte_size * 8);
> +}
> +
> +/**
> + * Return number of bits already read.
> + */
> +static inline int bitstream_tell(const BitstreamContext *bc)
> +{
> +    return (bc->ptr - bc->buffer) * 8 - bc->bits_left;
> +}
> +
> +/**
> + * Return buffer size in bits.
> + */
> +static inline int bitstream_tell_size(const BitstreamContext *bc)
> +{
> +    return bc->size_in_bits;
> +}
> +
> +/**
> + * Return the number of the bits left in a buffer.
> + */
> +static inline int bitstream_bits_left(const BitstreamContext *bc)
> +{
> +    return (bc->buffer - bc->ptr) * 8 + bc->size_in_bits + bc->bits_left;
> +}
> +
> +static inline uint64_t get_val(BitstreamContext *bc, unsigned int n)
> +{
> +    uint64_t ret;
> +
> +#ifdef BITSTREAM_READER_LE
> +    ret = bc->bits & ((UINT64_C(1) << n) - 1);
> +    bc->bits >>= n;
> +#else
> +    ret = bc->bits >> (64 - n);
> +    bc->bits <<= n;
> +#endif
> +    bc->bits_left -= n;
> +
> +    return ret;
> +}
> +
> +/**
> + * Return one bit from the buffer.
> + */
> +static inline unsigned int bitstream_read_bit(BitstreamContext *bc)
> +{
> +    if (!bc->bits_left)
> +        refill_64(bc);
> +
> +    return get_val(bc, 1);
> +}
> +
> +/**
> + * Return n bits from the buffer, n has to be in the 0-63 range.

get_bits64() can read up to 64 bits. This discrepancy won't lead to
immediate trouble as no caller calls it with 64 bits. But it's not nice.

> + */
> +static inline uint64_t bitstream_read_63(BitstreamContext *bc, unsigned int n)
> +{
> +    uint64_t ret = 0;
> +#ifdef BITSTREAM_READER_LE
> +    uint64_t left = 0;

uint64_t for something that is supposed to be 0..64 range.

> +#endif
> +
> +    if (!n)
> +        return 0;
> +
> +    if (n > bc->bits_left) {
> +        n -= bc->bits_left;
> +#ifdef BITSTREAM_READER_LE
> +        left = bc->bits_left;
> +#endif
> +        ret = get_val(bc, bc->bits_left);
> +        refill_64(bc);
> +    }
> +
> +#ifdef BITSTREAM_READER_LE
> +    ret = get_val(bc, n) << left | ret;
> +#else
> +    ret = get_val(bc, n) | ret << n;

If the above refill_64 fails due to eos, then get_val() is called
despite there being less than n bits left. get_bits64() does not suffer
from this.

> +#endif
> +
> +    return ret;
> +}
> +
> +/**
> + * Return n bits from the buffer, n has to be in the 0-32  range.

Why doesn't this whole patch(set) not use av_assert2s to check all these
restrictions?

> + */
> +static inline uint32_t bitstream_read(BitstreamContext *bc, unsigned int n)
> +{
> +    if (!n)
> +        return 0;

This makes this function a get_bitsz equivalent. Why is there no
get_bits() equivalent for the cases where it is known that n is not
zero, but where it is impossible for the compiler to know?

> +
> +    if (n > bc->bits_left) {
> +        refill_32(bc);
> +        if (bc->bits_left < 32)
> +            bc->bits_left = n;
> +    }
> +
> +    return get_val(bc, n);
> +}
> +
> +/**
> + * Return n bits from the buffer as a signed integer.
> + * n has to be in the 0-32 range.
> + */
> +static inline int32_t bitstream_read_signed(BitstreamContext *bc, unsigned int n)
> +{
> +    return sign_extend(bitstream_read(bc, n), n);
> +}
> +
> +static inline unsigned int show_val(BitstreamContext *bc, unsigned int n)
> +{
> +#ifdef BITSTREAM_READER_LE
> +    return bc->bits & ((UINT64_C(1) << n) - 1);
> +#else
> +    return bc->bits >> (64 - n);
> +#endif
> +}
> +
> +/**
> + * Return n bits from the buffer but do not change the buffer state.
> + * n has to be in the 0-32 range.
> + */
> +static inline unsigned int bitstream_peek(BitstreamContext *bc, unsigned int n)
> +{
> +    if (n > bc->bits_left)
> +        refill_32(bc);
> +
> +    return show_val(bc, n);
> +}
> +
> +/**
> + * Return n bits from the buffer as a signed integer,
> + * do not change the buffer state.
> + * n has to be in the 0-32 range.
> + */
> +static inline int bitstream_peek_signed(BitstreamContext *bc, unsigned int n)
> +{
> +    return sign_extend(bitstream_peek(bc, n), n);
> +}
> +
> +static inline void skip_remaining(BitstreamContext *bc, unsigned int n)
> +{
> +#ifdef BITSTREAM_READER_LE
> +    bc->bits >>= n;
> +#else
> +    bc->bits <<= n;
> +#endif
> +    bc->bits_left -= n;
> +}
> +
> +/**
> + * Skip n bits in the buffer.
> + */
> +static inline void bitstream_skip(BitstreamContext *bc, unsigned int n)
> +{
> +    if (n < bc->bits_left)
> +        skip_remaining(bc, n);
> +    else {
> +        n -= bc->bits_left;
> +        bc->bits      = 0;
> +        bc->bits_left = 0;
> +
> +        if (n >= 64) {
> +            unsigned int skip = n / 8;
> +
> +            n -= skip * 8;
> +            bc->ptr += skip;
> +        }
> +        refill_64(bc);
> +        if (n)
> +            skip_remaining(bc, n);
> +    }
> +}
> +
> +/**
> + * Seek to the given bit position.
> + */
> +static inline void bitstream_seek(BitstreamContext *bc, unsigned pos)
> +{
> +    bc->ptr       = bc->buffer;
> +    bc->bits      = 0;
> +    bc->bits_left = 0;
> +
> +    bitstream_skip(bc, pos);
> +}
> +
> +/**
> + * Skip bits to a byte boundary.
> + */
> +static inline const uint8_t *bitstream_align(BitstreamContext *bc)
> +{
> +    unsigned int n = -bitstream_tell(bc) & 7;
> +    if (n)
> +        bitstream_skip(bc, n);
> +    return bc->buffer + (bitstream_tell(bc) >> 3);
> +}
> +
> +/**
> + * Read MPEG-1 dc-style VLC (sign bit + mantissa with no MSB).
> + * If MSB not set it is negative.
> + * @param n length in bits
> + */
> +static inline int bitstream_read_xbits(BitstreamContext *bc, unsigned int n)
> +{
> +    int32_t cache = bitstream_peek(bc, 32);
> +    int sign = ~cache >> 31;
> +    skip_remaining(bc, n);
> +
> +    return ((((uint32_t)(sign ^ cache)) >> (32 - n)) ^ sign) - sign;
> +}
> +
> +/**
> + * Return decoded truncated unary code for the values 0, 1, 2.
> + */
> +static inline int bitstream_decode012(BitstreamContext *bc)
> +{
> +    if (!bitstream_read_bit(bc))
> +        return 0;
> +    else
> +        return bitstream_read_bit(bc) + 1;
> +}
> +
> +/**
> + * Return decoded truncated unary code for the values 2, 1, 0.
> + */
> +static inline int bitstream_decode210(BitstreamContext *bc)
> +{
> +    if (bitstream_read_bit(bc))
> +        return 0;
> +    else
> +        return 2 - bitstream_read_bit(bc);
> +}
> +
> +/* Read sign bit and flip the sign of the provided value accordingly. */
> +static inline int bitstream_apply_sign(BitstreamContext *bc, int val)
> +{
> +    int sign = bitstream_read_signed(bc, 1);
> +    return (val ^ sign) - sign;
> +}
> +
> +static inline int bitstream_skip_1stop_8data(BitstreamContext *s)
> +{
> +    if (bitstream_bits_left(s) <= 0)
> +        return AVERROR_INVALIDDATA;
> +
> +    while (bitstream_read(s, 1)) {
> +        bitstream_skip(s, 8);
> +        if (bitstream_bits_left(s) <= 0)
> +            return AVERROR_INVALIDDATA;
> +    }
> +
> +    return 0;
> +}
> +
> +/* Unwind the cache so a refill_32 can fill it again. */
> +static inline void bitstream_unwind(BitstreamContext *bc)
> +{
> +    int unwind = 4;
> +    int unwind_bits = unwind * 8;

I'm surprised that you used signed types here.

> +
> +    if (bc->bits_left < unwind_bits)
> +        return;
> +
> +    bc->bits      >>= unwind_bits;
> +    bc->bits      <<= unwind_bits;

The above won't work in LE. Best to call skip_remaining here. And you
need to templatize this function in 3/4.

> +    bc->bits_left  -= unwind_bits;
> +    bc->ptr        -= unwind;
> +}
> +
> +/* Unget up to 32 bits. */
> +static inline void bitstream_unget(BitstreamContext *bc, uint64_t value,
> +                                   size_t amount)

size_t is the natural type for the bytesize of objects, but not for
bitsizes. A plane unsigned would be more natural here.

> +{
> +    size_t cache_size = sizeof(bc->bits) * 8;
> +
> +    if (bc->bits_left + amount > cache_size)
> +        bitstream_unwind(bc);
> +
> +    bc->bits       = (bc->bits >> amount) | (value << (cache_size - amount));

This is big-endian only, too.

> +    bc->bits_left += amount;
> +}
> +
> +/**
> + * Return the LUT element for the given bitstream configuration.
> + */
> +static inline int set_idx(BitstreamContext *bc, int code, int *n, int *nb_bits,
> +                          const VLCElem *table)
> +{
> +    unsigned idx;
> +
> +    *nb_bits = -*n;
> +    idx = bitstream_peek(bc, *nb_bits) + code;
> +    *n = table[idx].len;
> +
> +    return table[idx].sym;
> +}
> +
> +/**
> + * Parse a vlc code.
> + * @param bits is the number of bits which will be read at once, must be
> + *             identical to nb_bits in init_vlc()
> + * @param max_depth is the number of times bits bits must be read to completely
> + *                  read the longest vlc code
> + *                  = (max_vlc_length + bits - 1) / bits
> + * If the vlc code is invalid and max_depth=1, then no bits will be removed.
> + * If the vlc code is invalid and max_depth>1, then the number of bits removed
> + * is undefined.
> + */
> +static inline int bitstream_read_vlc(BitstreamContext *bc, const VLCElem *table,
> +                                     int bits, int max_depth)
> +{
> +    int nb_bits;
> +    unsigned idx = bitstream_peek(bc, bits);
> +    int code     = table[idx].sym;
> +    int n        = table[idx].len;
> +
> +    if (max_depth > 1 && n < 0) {
> +        skip_remaining(bc, bits);
> +        code = set_idx(bc, code, &n, &nb_bits, table);
> +        if (max_depth > 2 && n < 0) {
> +            skip_remaining(bc, nb_bits);
> +            code = set_idx(bc, code, &n, &nb_bits, table);
> +        }
> +    }
> +    skip_remaining(bc, n);
> +
> +    return code;
> +}
> +
> +#define BITSTREAM_RL_VLC(level, run, bc, table, bits, max_depth) \
> +    do {                                                         \
> +        int n, nb_bits;                                          \
> +        unsigned int index = bitstream_peek(bc, bits);           \
> +        level = table[index].level;                              \
> +        n     = table[index].len;                                \
> +                                                                 \
> +        if (max_depth > 1 && n < 0) {                            \
> +            bitstream_skip(bc, bits);                            \
> +                                                                 \
> +            nb_bits = -n;                                        \
> +                                                                 \
> +            index = bitstream_peek(bc, nb_bits) + level;         \
> +            level = table[index].level;                          \
> +            n     = table[index].len;                            \
> +            if (max_depth > 2 && n < 0) {                        \
> +                bitstream_skip(bc, nb_bits);                     \
> +                nb_bits = -n;                                    \
> +                                                                 \
> +                index = bitstream_peek(bc, nb_bits) + level;     \
> +                level = table[index].level;                      \
> +                n     = table[index].len;                        \
> +            }                                                    \
> +        }                                                        \
> +        run = table[index].run;                                  \
> +        bitstream_skip(bc, n);                                   \
> +    } while (0)
> +
> +#endif /* AVCODEC_BITSTREAM_H */

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".