[Mesa-dev] [PATCH 1/3] vl: Only initialize vlc once
Christian König
deathsimple at vodafone.de
Tue Dec 20 05:16:44 PST 2011
On 20.12.2011 12:43, Maarten Lankhorst wrote:
> And add more sanity checks to stream. This shouldn't break things beyond those that aren't broken already.
>
> Signed-off-by: Maarten Lankhorst<m.b.lankhorst at gmail.com>
>
> ---
> And yes Andy, I mean that I haven't found a good video yet to fix the playback parts that are still broken..
Why do you want to change that anyway?
The search for start codes was especially split out of the VLC stuff,
because start codes start are byte aligned anyway and it doesn't make
much sense to search for them using the slower peekbits & fillbits
functions.
Se below for some further comments.
> src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c | 56 +++++++++++-------------
> src/gallium/auxiliary/vl/vl_vlc.h | 33 +++++++++-----
> 2 files changed, 47 insertions(+), 42 deletions(-)
>
> diff --git a/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c b/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c
> index 936cf2c..62d08d7 100644
> --- a/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c
> +++ b/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c
> @@ -787,7 +787,7 @@ entry:
> }
>
> static INLINE bool
> -decode_slice(struct vl_mpg12_bs *bs)
> +decode_slice(struct vl_mpg12_bs *bs, unsigned code)
> {
> struct pipe_mpeg12_macroblock mb;
> short dct_blocks[64*6];
> @@ -796,24 +796,29 @@ decode_slice(struct vl_mpg12_bs *bs)
>
> memset(&mb, 0, sizeof(mb));
> mb.base.codec = PIPE_VIDEO_CODEC_MPEG12;
> - mb.y = vl_vlc_get_uimsbf(&bs->vlc, 8) - 1;
> + mb.y = code - 1;
> mb.blocks = dct_blocks;
>
> reset_predictor(bs);
> + vl_vlc_fillbits(&bs->vlc);
> dct_scale = quant_scale[bs->desc.q_scale_type][vl_vlc_get_uimsbf(&bs->vlc, 5)];
>
> - if (vl_vlc_get_uimsbf(&bs->vlc, 1))
> + if (vl_vlc_get_uimsbf(&bs->vlc, 1)) {
> + vl_vlc_fillbits(&bs->vlc);
> while (vl_vlc_get_uimsbf(&bs->vlc, 9)& 1)
> vl_vlc_fillbits(&bs->vlc);
> + }
>
> vl_vlc_fillbits(&bs->vlc);
> + assert(vl_vlc_bits_left(&bs->vlc)> 23&& vl_vlc_peekbits(&bs->vlc, 23));
> do {
> int inc = 0;
>
> - while (vl_vlc_peekbits(&bs->vlc, 11) == 15) {
> - vl_vlc_eatbits(&bs->vlc, 11);
> - vl_vlc_fillbits(&bs->vlc);
> - }
> + if (bs->decoder->profile == PIPE_VIDEO_PROFILE_MPEG1)
> + while (vl_vlc_peekbits(&bs->vlc, 11) == 15) {
> + vl_vlc_eatbits(&bs->vlc, 11);
> + vl_vlc_fillbits(&bs->vlc);
> + }
>
> while (vl_vlc_peekbits(&bs->vlc, 11) == 8) {
> vl_vlc_eatbits(&bs->vlc, 11);
> @@ -959,32 +964,23 @@ void
> vl_mpg12_bs_decode(struct vl_mpg12_bs *bs, unsigned num_bytes, const uint8_t *buffer)
> {
> assert(bs);
> - assert(buffer&& num_bytes);
> -
> - while(num_bytes> 2) {
> - if (buffer[0] == 0x00&& buffer[1] == 0x00&& buffer[2] == 0x01&&
> - buffer[3]>= 0x01&& buffer[3]< 0xAF) {
> - unsigned consumed;
>
> - buffer += 3;
> - num_bytes -= 3;
> + vl_vlc_init(&bs->vlc, 1, num_bytes, (void const * const *)&buffer,&num_bytes);
>
> - vl_vlc_init(&bs->vlc, buffer, num_bytes);
> -
> - if (!decode_slice(bs))
> + while (vl_vlc_bits_left(&bs->vlc)>= 32) {
> + vl_vlc_fillbits(&bs->vlc);
> + if (vl_vlc_peekbits(&bs->vlc, 24) == 0x000001) {
> + unsigned code;
> + vl_vlc_eatbits(&bs->vlc, 24);
> + code = vl_vlc_get_uimsbf(&bs->vlc, 8);
> + if (!code || code> 0xaf)
> + continue;
> + if (!decode_slice(bs, code))
> return;
> -
> - consumed = num_bytes - vl_vlc_bits_left(&bs->vlc) / 8;
> -
> - /* crap, this is a bug we have consumed more bytes than left in the buffer */
> - assert(consumed<= num_bytes);
> -
> - num_bytes -= consumed;
> - buffer += consumed;
> -
> + vl_vlc_bitalign(&bs->vlc);
> } else {
> - ++buffer;
> - --num_bytes;
> + vl_vlc_eatbits(&bs->vlc, 8);
> + continue;
> }
> }
> }
> diff --git a/src/gallium/auxiliary/vl/vl_vlc.h b/src/gallium/auxiliary/vl/vl_vlc.h
> index dc4faed..a4ded70 100644
> --- a/src/gallium/auxiliary/vl/vl_vlc.h
> +++ b/src/gallium/auxiliary/vl/vl_vlc.h
> @@ -38,7 +38,7 @@
> struct vl_vlc
> {
> uint64_t buffer;
> - unsigned valid_bits;
> + uint32_t valid_bits;
> uint32_t *data;
> uint32_t *end;
> };
> @@ -74,11 +74,9 @@ vl_vlc_init_table(struct vl_vlc_entry *dst, unsigned dst_size, const struct vl_v
> static INLINE void
> vl_vlc_fillbits(struct vl_vlc *vlc)
> {
> - if (vlc->valid_bits< 32) {
> + if (vlc->valid_bits< 32&& vlc->data< vlc->end) {
> uint32_t value = *vlc->data;
The correct sollution would be to let fillbits return a boolean
signaling that the buffer(s) are depleted.
>
> - //assert(vlc->data<= vlc->end);
> -
> #ifndef PIPE_ARCH_BIG_ENDIAN
> value = util_bswap32(value);
> #endif
> @@ -90,10 +88,14 @@ vl_vlc_fillbits(struct vl_vlc *vlc)
> }
>
> static INLINE void
> -vl_vlc_init(struct vl_vlc *vlc, const uint8_t *data, unsigned len)
> +vl_vlc_init(struct vl_vlc *vlc,
> + const unsigned array_size, unsigned total_len,
> + const const void *const *datas, const unsigned *lens)
> {
> + const uint8_t *data = datas[0];
> assert(vlc);
> - assert(data&& len);
> + assert(array_size == 1); // TODO
> + assert(datas[0]&& lens[0]);
Adding an interface definition without an implementation usually only
makes sense when we could have more than one different implementation.
>
> vlc->buffer = 0;
> vlc->valid_bits = 0;
> @@ -102,11 +104,11 @@ vl_vlc_init(struct vl_vlc *vlc, const uint8_t *data, unsigned len)
> while (pointer_to_uintptr(data)& 3) {
> vlc->buffer |= (uint64_t)*data<< (56 - vlc->valid_bits);
> ++data;
> - --len;
> + --total_len;
> vlc->valid_bits += 8;
> }
> vlc->data = (uint32_t*)data;
> - vlc->end = (uint32_t*)(data + len);
> + vlc->end = (uint32_t*)(data + total_len);
>
> vl_vlc_fillbits(vlc);
> vl_vlc_fillbits(vlc);
> @@ -122,7 +124,7 @@ vl_vlc_bits_left(struct vl_vlc *vlc)
> static INLINE unsigned
> struct vl_vlc *vlc, unsigned num_bits)
> {
> - //assert(vlc->valid_bits>= num_bits);
> + assert(vlc->valid_bits>= num_bits || vl_vlc_bits_left(vlc)< num_bits);
Hui? bits_left is calculation the total number of bits left in the
buffer, while valid_bits is the number of currently loaded bits, so that
check is erroneous.
>
> return vlc->buffer>> (64 - num_bits);
> }
> @@ -130,7 +132,7 @@ vl_vlc_peekbits(struct vl_vlc *vlc, unsigned num_bits)
> static INLINE void
> vl_vlc_eatbits(struct vl_vlc *vlc, unsigned num_bits)
> {
> - //assert(vlc->valid_bits> num_bits);
> + assert(vlc->valid_bits>= num_bits);
Just commenting in all checks isn't such a good idea, since that affect
performance very badly, a define which enables all assertions at once at
the beginning of the file seems the better idea.
>
> vlc->buffer<<= num_bits;
> vlc->valid_bits -= num_bits;
> @@ -141,7 +143,7 @@ vl_vlc_get_uimsbf(struct vl_vlc *vlc, unsigned num_bits)
> {
> unsigned value;
>
> - //assert(vlc->valid_bits>= num_bits);
> + assert(vlc->valid_bits>= num_bits);
>
> value = vlc->buffer>> (64 - num_bits);
> vl_vlc_eatbits(vlc, num_bits);
> @@ -154,7 +156,7 @@ vl_vlc_get_simsbf(struct vl_vlc *vlc, unsigned num_bits)
> {
> signed value;
>
> - //assert(vlc->valid_bits>= num_bits);
> + assert(vlc->valid_bits>= num_bits);
>
> value = ((int64_t)vlc->buffer)>> (64 - num_bits);
> vl_vlc_eatbits(vlc, num_bits);
> @@ -167,7 +169,14 @@ vl_vlc_get_vlclbf(struct vl_vlc *vlc, const struct vl_vlc_entry *tbl, unsigned n
> {
> tbl += vl_vlc_peekbits(vlc, num_bits);
> vl_vlc_eatbits(vlc, tbl->length);
> + assert(tbl->length);
> return tbl->value;
> }
>
> +static INLINE void
> +vl_vlc_bitalign(struct vl_vlc *vlc)
> +{
> + vl_vlc_eatbits(vlc, vlc->valid_bits& 7);
> +}
> +
> #endif /* vl_vlc_h */
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
Christian.
More information about the mesa-dev
mailing list