On 20.12.2011 19:20, Maarten Lankhorst wrote:
Hey Christian,
On 12/20/2011 02:16 PM, Christian König wrote:
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.
Since 4 bytes are loaded at a time and bit aligning is fast enough, it's not that
much slower, the reason I want this is to be able to add support for when the
array size is> 1, currently and even with that patch its not handled, but if a
slice is split between multiple buffers you can at least pick up where you left..
Good argument, but we should complete the patch instead of leaving the
code in a condition where we just bail out with an assertion if the
application happens to not behave as we have planned.
for example with mplayer I have observed that the start code 0x00,0x00,0x01
and the actual value are in separate buffers for some codecs, and for wmv the
following is said:
* Some VC-1 advanced profile streams do not contain slice start codes; again,
* the container format must indicate where picture data begins and ends. In
* this case, pictures are assumed to be progressive and to contain a single
* slice. It is highly recommended that applications detect this condition,
* and add the missing start codes to the bitstream passed to VDPAU. However,
* VDPAU implementations must allow bitstreams with missing start codes, and
* act as if a 0x0000010D (frame) start code had been present.
Since mplayer can chop up the start code and the 0x0d, you'd need a parser to
handle this.
I planned on using vlc to detect the lack of start code.
Well, sounds like a plan to me.
The correct sollution would be to let fillbits return a boolean signaling that
the buffer(s) are depleted.
Maybe, I want to start filling from the next stream if this is the case.
Yeah, take a look at the attachment, that's how I implemented it, but
what todo if we don't have any more inputs?
Currently we load up to three extra bytes and then just continue to
pretend that the stream only contains zeros, well on a second though
that isn't so bad after all.
I played around with it a bit today and it seems that the current
behavior is already the best we can do about it.
Adding an interface definition without an implementation usually only
makes sense when we could have more than one different implementation.
As the TODO says, I want to add support for array_size> 1 still, also next patch
uses this interface, even if I didn't add the support for array_size> 1 yet. If I
don't continuously reset vlc it should work correctly even when a stream has multiple
buffers.
Take a look at the attached patch then, that should do it quite well.
- //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.
Yes, but if you are near the end of the stream it is possible that peekbits can
extend beyond it when doing vlc decoding, hence the second check.
This is a special case for peekbits that would otherwise crash unnecessarily
over DCT coefficients, any call that really consumes the bits will still fail.
Ah ok, now I understand your intentions. Well then check for "vlc->data
>= vlc->end" instead, since at the end of the stream even
"vl_vlc_bits_left(vlc)< num_bits" will fail ( at least if everything
goes right), and in the middle of a stream it will succeed in the case
when there is a missing fillbits!
On 20.12.2011 21:08, Lucas Stach wrote:
Hi all!
Just jumping in with regard to the assert.
Am Dienstag, den 20.12.2011, 19:20 +0100 schrieb Maarten Lankhorst:
[snip]
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.
Sure.
Please don't define your own semantic for this checks. Assert is only
used in debug builds and there it shouldn't matter how it affects
performance. In release builds assert is typically a no-op and therefore
optimized away, but it could also be used to help the compiler optimize
for the asserted conditions.
Asserts should be never deactivated with code defines.
Those functions are called a couple of million times a second, so even
if the assertion is only tested in debug builds it has quite an effect
on performance, sometimes even masquerading real performance problems.
So my practice was to only uncomment them when I work on that part of
the code. I don't want to change the Assert semantics in any way, just a
simple define enabling certain assertions only under certain conditions
should be sufficient.
Christian.
>From b19139fa4b22dc8eebb9b105dd29c7fee88816e5 Mon Sep 17 00:00:00 2001
From: Maarten Lankhorst <m.b.lankho...@gmail.com>
Date: Tue, 20 Dec 2011 12:43:23 +0100
Subject: [PATCH] vl: Only initialize vlc once
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
And add more sanity checks to stream. This shouldn't
break things beyond those that aren't broken already.
v2: also implement multiple inputs for the vlc functions
Signed-off-by: Maarten Lankhorst <m.b.lankho...@gmail.com>
Signed-off-by: Christian König <deathsim...@vodafone.de>
---
src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c | 50 +++++-------
src/gallium/auxiliary/vl/vl_vlc.h | 102 ++++++++++++++++++-----
2 files changed, 101 insertions(+), 51 deletions(-)
diff --git a/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c b/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c
index 936cf2c..cf75000 100644
--- a/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c
+++ b/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c
@@ -786,7 +786,7 @@ entry:
}
}
-static INLINE bool
+static INLINE void
decode_slice(struct vl_mpg12_bs *bs)
{
struct pipe_mpeg12_macroblock mb;
@@ -800,20 +800,25 @@ decode_slice(struct vl_mpg12_bs *bs)
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);
@@ -928,7 +933,6 @@ decode_slice(struct vl_mpg12_bs *bs)
mb.num_skipped_macroblocks = 0;
bs->decoder->decode_macroblock(bs->decoder, &mb.base, 1);
- return true;
}
void
@@ -959,32 +963,20 @@ 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;
+ vl_vlc_init(&bs->vlc, 1, (const void * const *)&buffer, &num_bytes);
+ while (vl_vlc_bits_left(&bs->vlc) > 32) {
+ uint32_t code = vl_vlc_peekbits(&bs->vlc, 32);
- buffer += 3;
- num_bytes -= 3;
-
- vl_vlc_init(&bs->vlc, buffer, num_bytes);
-
- if (!decode_slice(bs))
- 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;
+ if (code >= 0x101 && code <= 0x1AF) {
+ vl_vlc_eatbits(&bs->vlc, 24);
+ decode_slice(bs);
+ vl_vlc_bytealign(&bs->vlc);
} else {
- ++buffer;
- --num_bytes;
+ vl_vlc_eatbits(&bs->vlc, 8);
}
+
+ vl_vlc_fillbits(&bs->vlc);
}
}
diff --git a/src/gallium/auxiliary/vl/vl_vlc.h b/src/gallium/auxiliary/vl/vl_vlc.h
index dc4faed..f961b8b 100644
--- a/src/gallium/auxiliary/vl/vl_vlc.h
+++ b/src/gallium/auxiliary/vl/vl_vlc.h
@@ -35,12 +35,22 @@
#include "util/u_math.h"
#include "util/u_pointer.h"
+#if 0
+ #define VL_VLC_DEBUG_ASSERT(x) assert(x)
+#else
+ #define VL_VLC_DEBUG_ASSERT(x) do { } while(0)
+#endif
+
struct vl_vlc
{
uint64_t buffer;
unsigned valid_bits;
- uint32_t *data;
- uint32_t *end;
+ const uint32_t *data;
+ const uint32_t *end;
+
+ unsigned num_inputs;
+ const void *const *inputs;
+ const unsigned *sizes;
};
struct vl_vlc_entry
@@ -60,6 +70,9 @@ vl_vlc_init_table(struct vl_vlc_entry *dst, unsigned dst_size, const struct vl_v
{
unsigned i, bits = util_logbase2(dst_size);
+ assert(dst && dst_size);
+ assert(src && src_size);
+
for (i=0;i<dst_size;++i) {
dst[i].length = 0;
dst[i].value = 0;
@@ -72,12 +85,36 @@ vl_vlc_init_table(struct vl_vlc_entry *dst, unsigned dst_size, const struct vl_v
}
static INLINE void
+vl_vlc_next_input(struct vl_vlc *vlc)
+{
+ const uint8_t* data = vlc->inputs[0];
+ unsigned len = vlc->sizes[0];
+
+ VL_VLC_DEBUG_ASSERT(vlc);
+ VL_VLC_DEBUG_ASSERT(vlc->num_inputs);
+
+ /* align the data pointer */
+ while (pointer_to_uintptr(data) & 3) {
+ vlc->buffer |= (uint64_t)*data << (56 - vlc->valid_bits);
+ ++data;
+ --len;
+ vlc->valid_bits += 8;
+ }
+ vlc->data = (const uint32_t*)data;
+ vlc->end = (const uint32_t*)(data + len);
+
+ --vlc->num_inputs;
+ ++vlc->inputs;
+ ++vlc->sizes;
+}
+
+static INLINE void
vl_vlc_fillbits(struct vl_vlc *vlc)
{
- if (vlc->valid_bits < 32) {
- uint32_t value = *vlc->data;
+ VL_VLC_DEBUG_ASSERT(vlc);
- //assert(vlc->data <= vlc->end);
+ if (vlc->valid_bits < 32 && vlc->data < vlc->end) {
+ uint32_t value = *vlc->data;
#ifndef PIPE_ARCH_BIG_ENDIAN
value = util_bswap32(value);
@@ -86,28 +123,40 @@ vl_vlc_fillbits(struct vl_vlc *vlc)
vlc->buffer |= (uint64_t)value << (32 - vlc->valid_bits);
++vlc->data;
vlc->valid_bits += 32;
+
+ /* if this input is depleted, go to next one */
+ if (vlc->data >= vlc->end && vlc->num_inputs) {
+ unsigned bytes_overdue = ((uint8_t*)vlc->data)-((uint8_t*)vlc->end);
+
+ /* adjust valid bits */
+ vlc->valid_bits -= bytes_overdue * 8;
+
+ /* clear not valid bits */
+ vlc->buffer &= ~((1LL << (64 - vlc->valid_bits)) - 1);
+
+ /* go on to next input */
+ vl_vlc_next_input(vlc);
+
+ /* and retry to fill the buffer */
+ vl_vlc_fillbits(vlc);
+ }
}
}
static INLINE void
-vl_vlc_init(struct vl_vlc *vlc, const uint8_t *data, unsigned len)
+vl_vlc_init(struct vl_vlc *vlc, unsigned num_inputs,
+ const void *const *inputs, const unsigned *sizes)
{
assert(vlc);
- assert(data && len);
+ assert(num_inputs);
vlc->buffer = 0;
vlc->valid_bits = 0;
+ vlc->num_inputs = num_inputs;
+ vlc->inputs = inputs;
+ vlc->sizes = sizes;
- /* align the data pointer */
- while (pointer_to_uintptr(data) & 3) {
- vlc->buffer |= (uint64_t)*data << (56 - vlc->valid_bits);
- ++data;
- --len;
- vlc->valid_bits += 8;
- }
- vlc->data = (uint32_t*)data;
- vlc->end = (uint32_t*)(data + len);
-
+ vl_vlc_next_input(vlc);
vl_vlc_fillbits(vlc);
vl_vlc_fillbits(vlc);
}
@@ -115,22 +164,24 @@ vl_vlc_init(struct vl_vlc *vlc, const uint8_t *data, unsigned len)
static INLINE unsigned
vl_vlc_bits_left(struct vl_vlc *vlc)
{
+ unsigned i;
signed bytes_left = ((uint8_t*)vlc->end)-((uint8_t*)vlc->data);
+ for (i = 0; i < vlc->num_inputs; ++i)
+ bytes_left += vlc->sizes[i];
return bytes_left * 8 + vlc->valid_bits;
}
static INLINE unsigned
vl_vlc_peekbits(struct vl_vlc *vlc, unsigned num_bits)
{
- //assert(vlc->valid_bits >= num_bits);
-
+ VL_VLC_DEBUG_ASSERT(vlc->valid_bits >= num_bits || vlc->data >= vlc->end);
return vlc->buffer >> (64 - num_bits);
}
static INLINE void
vl_vlc_eatbits(struct vl_vlc *vlc, unsigned num_bits)
{
- //assert(vlc->valid_bits > num_bits);
+ VL_VLC_DEBUG_ASSERT(vlc->valid_bits >= num_bits);
vlc->buffer <<= num_bits;
vlc->valid_bits -= num_bits;
@@ -141,7 +192,7 @@ vl_vlc_get_uimsbf(struct vl_vlc *vlc, unsigned num_bits)
{
unsigned value;
- //assert(vlc->valid_bits >= num_bits);
+ VL_VLC_DEBUG_ASSERT(vlc->valid_bits >= num_bits);
value = vlc->buffer >> (64 - num_bits);
vl_vlc_eatbits(vlc, num_bits);
@@ -154,7 +205,7 @@ vl_vlc_get_simsbf(struct vl_vlc *vlc, unsigned num_bits)
{
signed value;
- //assert(vlc->valid_bits >= num_bits);
+ VL_VLC_DEBUG_ASSERT(vlc->valid_bits >= num_bits);
value = ((int64_t)vlc->buffer) >> (64 - num_bits);
vl_vlc_eatbits(vlc, num_bits);
@@ -167,7 +218,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);
+ VL_VLC_DEBUG_ASSERT(tbl->length);
return tbl->value;
}
+static INLINE void
+vl_vlc_bytealign(struct vl_vlc *vlc)
+{
+ vl_vlc_eatbits(vlc, vlc->valid_bits & 7);
+}
+
#endif /* vl_vlc_h */
--
1.7.5.4
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
