[FFmpeg-devel] [PATCH 2/3] lavc: add standalone cached bitstream reader
Anton Khirnov
anton at khirnov.net
Tue Jul 5 14:30:09 EEST 2022
Quoting Andreas Rheinhardt (2022-07-03 15:16:39)
> Anton Khirnov:
> > +
> > +#include "mathops.h"
>
> What exactly is mathops.h for? get_bits.h uses it for NEG_USR32 and
> NEG_SSR32, but you are not using it here.
sign_extend()?
> > +/**
> > + * Skip bits to a byte boundary.
> > + */
> > +static inline const uint8_t *bits_align(BitstreamContext *bc)
> > +{
> > + unsigned int n = -bits_tell(bc) & 7;
> > + if (n)
> > + bits_skip(bc, n);
>
> Is there a reason that I don't see that makes you not simply use
> bc->bits_left &= ~0xff?
I don't see how that is supposed to work.
>
> > + return bc->buffer + (bits_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 bits_read_xbits(BitstreamContext *bc, unsigned int n)
> > +{
> > + int32_t cache = bits_peek(bc, 32);
> > + int sign = ~cache >> 31;
> > + bits_priv_skip_remaining(bc, n);
>
> FYI: You are potentially skipping more bits here than you have.
Yes, you made that clear in your fake-caching patch. Do you require that
be fixed now? Or can this go in, then we apply some form of your patch?
> > +/* Read sign bit and flip the sign of the provided value accordingly. */
> > +static inline int bits_apply_sign(BitstreamContext *bc, int val)
> > +{
> > + int sign = bits_read_signed(bc, 1);
>
> Is there a reason you are not using bits_read_bit here?
I want a signed result.
--
Anton Khirnov
More information about the ffmpeg-devel
mailing list