I hope this message makes it to the list. I can't find a proper way to subscribe.
I have been reviewing the official VP8 documentation. I think I have found a few mistakes or areas that could use clarification. Where should I send them, or should I publish separate errata on my own website?
On Fri, May 21, 2010 at 9:30 PM, Mike Melanson <m...@multimedia.cx> wrote:
> I hope this message makes it to the list. I can't find a proper way to
> subscribe.
> I have been reviewing the official VP8 documentation. I think I have found a
> few mistakes or areas that could use clarification. Where should I send
> them, or should I publish separate errata on my own website?
> Thanks for reviewing and incoming corrections. We are seeing your
> message here, so your posting method is working. Please do send them
> here.
Great. The first 2 problems I notice:
Section 9.1 describes the frame header. There are 1-bit, 3-bit, 1-bit, and 19-bit fields to kick off each frame. My natural assumption is that these should be read from the bitstream in left->right order. Bad guess. In fact, the general frame header is best interpreted by parsing the first 3 bytes as laid out it the reference code:
Of byte 0, bit 0 is interframe, version is bits 3-1, show_frame is bit 4, and the first partition's length in bytes is comprised of bits 7-5 as well as the bytes 1 and 2.
Next, section 7.3: Actual implementation of the boolean decoder. I don't believe init_bool_decoder() is correct:
int i = 0;
d->value = 0; /* value = first 24 input bytes */
while( ++i <= 24)
d->value = (d->value << 8) | *start_partition++;
This will shift 24 bytes into a value that is only comprised of 4 bytes to begin with. 24 bits, perhaps?
On Sat, May 22, 2010 at 12:30 AM, Mike Melanson <m...@multimedia.cx> wrote:
> I hope this message makes it to the list. I can't find a proper way to
> subscribe.
Agree, a lot of people are having problems with the web interface.
Easiest way to subscribe is to mail
<listname>+subscr...@webmproject.org
> I have been reviewing the official VP8 documentation. I think I have found a
> few mistakes or areas that could use clarification. Where should I send
> them, or should I publish separate errata on my own website?
We're going to put the plaintext source that the spec PDF is generated
from into git RSN, which should make it a little easier to correct
these errata. codec-devel@ is probably the right list for discussion.
On Sat, May 22, 2010 at 12:52 AM, Mike Melanson <m...@multimedia.cx> wrote:
> Yaowu Xu wrote:
>> Mike,
>> Thanks for reviewing and incoming corrections. We are seeing your
>> message here, so your posting method is working. Please do send them
>> here.
> Great. The first 2 problems I notice:
> Section 9.1 describes the frame header. There are 1-bit, 3-bit, 1-bit, and
> 19-bit fields to kick off each frame. My natural assumption is that these
> should be read from the bitstream in left->right order. Bad guess. In fact,
> the general frame header is best interpreted by parsing the first 3 bytes as
> laid out it the reference code:
> Of byte 0, bit 0 is interframe, version is bits 3-1, show_frame is bit 4,
> and the first partition's length in bytes is comprised of bits 7-5 as well
> as the bytes 1 and 2.
The phrase "little endian" is usually what I have to whisper to myself
when looking at some of the strange packings in the bitstream. Agree
that this could be more clear in the doc.
> Next, section 7.3: Actual implementation of the boolean decoder. I don't
> believe init_bool_decoder() is correct:
> int i = 0;
> d->value = 0; /* value = first 24 input bytes */
> while( ++i <= 24)
> d->value = (d->value << 8) | *start_partition++;
> This will shift 24 bytes into a value that is only comprised of 4 bytes to
> begin with. 24 bits, perhaps?
> I'll post more as I spot them.
Thanks. As I said on webm-discuss, we'll be getting the spec into git soon.
How about """ The uncompressed data chunk comprises a common (for key frames and interframes) 24-bit frame tag that contains four fields, as follows: bit 0 A 1-bit frame type ( 0 for key frames, 1 for interframes). bits 3..1 A 3-bit version number ( 0 - 3 are defined as four different profiles with different decoding complexity; other values may be defined for future variants of the VP8 data format). bit 4 A 1-bit show_frame flag ( 0 when current frame is not for display, 1 when current frame is for display). bits 23..5 A 19-bit field containing the size of the first data partition in bytes.
This 24-bit tag is stored in 3 bytes, starting with the least significant 8 bits. """
LIkewise, the 'image dimensions' record can be described as a 32-bit record, with 4 subfields, also stored little-end first. So there's no need at all to include the (incompletely specified) 'swap2' function, along with that C code using it (which does non-portable unaligned reads, BTW).
And a picture, showing the field layout wouldn't hurt.
I would also add : - all elements described in the header (or anywhere, for that matter) need to have actual names so that they can be properly referred to later. 9.2 doesn't name things, and 9.3 has a typo in the name of 'update_mb_segmentat[i]on_map', so that it can't be found in a search. - how about an overview of the whole header, showing how the optional and variable parts are organized (again, picture is good).
In general, there is too much 'implementation' detail in the spec. This may be an attempt to help the implementor, but it's a slippery slope, and clutters the spec, making it harder to understand what's actually going on. For instance, the last pgph of chapter 6 complains about the non-portable nature of >> on negative numbers, and promises to call attention to these when they show up (a promise not kept, BTW). This is a compression spec, not a C tutorial! Just define the
>> to do what you want, and be done with it.
In fact, there is no need to mess around with different integer widths, just say that all variables are infinite range, define what << and >> ( and '/' if applicable) do to these (which is simpler than the C equivalents), and then use '&0xFF' for instance if you need to truncate things. It is up the spec to define the operations, and ultimately up to the implementor to figure out what the range of subexpressions are, although it certainly doesn't hurt to define some of these in the comments alongside.
And on the subject of commentary, take a look at any other established standard doc (I highly recommend the ETS 300 401 DAB spec, which is very well written, at least the sections 10..15 that I worked with). They are, in general, clearly divided into 'normative' and 'informative' sections. The 'normative' sections should be sufficient to form the spec, but may be too terse and hard to get the 'big picture' from; the informative parts are to help out with the big picture. The advantage of this is that you can gloss over some details in the 'informative' part (for clarity) without compromising the detailed accuracy of the 'normative' parts. In the current VP8 spec, the two types of content are mixed confusingly. For instance, the description of how arithmetic coding actually works in section 7 is great, and welcome, but does not need to be part of the spec, it should be marked as an 'informative' section. That way, if anything in there is not exactly in line with the rest of the spec, it's not a problem. In the current spec, the 'normative' info appears to start in 7.2 but the language is still informal; information is then redundantly restated in 7.3, so maybe 7.2 is purely informative? this needs to be clear (and bugs in 7.3 need fixing :-) ).
At the end of the day, it should be possible to give the spec to someone, and with sufficient diligence they should be able to create a correct decoder from the spec alone, without looking at the reference code (or even at reference compressed data). The current spec is quite far from that ideal.
> int i = 0;
> d->value = 0; /* value = first 24 input bytes */
> while( ++i <= 24)
> d->value = (d->value << 8) | *start_partition++;
> This will shift 24 bytes into a value that is only comprised of 4 bytes
> to begin with. 24 bits, perhaps?
Actually, that should be 2 bytes. The code in 'read_bool' treats
'value' as being a 16-bit number with 'value' (as per 7.2) in bits
[15...8]; the lower 8 bits contain '8-bit_count' additional bits to be
used later. So the init should load 2 bytes.
The comment alongside value ('contains at least 24...') appears to be
left over from some other bit unpacking implementation. It should say
"decoding value, left-justified in 16 bits'. The number of 'live' bits
actually ranges from 9 to 16.
- 'range' and 'value' variables could be uint8 and uint16.
Another thing, in the given code, the byte-loading criterion is
'greedy', it loads a new byte as soon as it has room for it, before it
needs to look at any bits in that byte. At the end of the decoding of
a partition, it may end up reading one more byte than needed. In other
standards, that can be a problem, but I think that partitions have
explicitly specified byte lengths in VP8, right? if there is no need
for the decoding to always end on a well-defined byte, there's not
really a problem in over-reading.
HI ,
I want to know the fact that,
1. will vp8 decoder supports I Macroblock in P frames,
2. if yes how the prediction is performed in p frames I
macroblock.
can any one explain about this.
muras K
On May 29, 7:03 am, gs_toronto <gsm...@alumni.uwaterloo.ca> wrote:
> > int i = 0;
> > d->value = 0; /* value = first 24 input bytes */
> > while( ++i <= 24)
> > d->value = (d->value << 8) | *start_partition++;
> > This will shift 24 bytes into a value that is only comprised of 4 bytes
> > to begin with. 24 bits, perhaps?
> Actually, that should be 2 bytes. The code in 'read_bool' treats
> 'value' as being a 16-bit number with 'value' (as per 7.2) in bits
> [15...8]; the lower 8 bits contain '8-bit_count' additional bits to be
> used later. So the init should load 2 bytes.
> The comment alongside value ('contains at least 24...') appears to be
> left over from some other bit unpacking implementation. It should say
> "decoding value, left-justified in 16 bits'. The number of 'live' bits
> actually ranges from 9 to 16.
> - 'range' and 'value' variables could be uint8 and uint16.
> Another thing, in the given code, the byte-loading criterion is
> 'greedy', it loads a new byte as soon as it has room for it, before it
> needs to look at any bits in that byte. At the end of the decoding of
> a partition, it may end up reading one more byte than needed. In other
> standards, that can be a problem, but I think that partitions have
> explicitly specified byte lengths in VP8, right? if there is no need
> for the decoding to always end on a well-defined byte, there's not
> really a problem in over-reading.
Hi Muras, Please find the answers below. 1. Yes, VP8 supports I Mbs in P Frames. 2. For P frames - One syntax element has to be decoded to get the type of macroblock. (Intra or Inter) - If it is intra coded - Find out the type of prediction Mode. - The Intra prediction supports - 4 modes for 16x16 - 10 modes for 4x4 - It uses the unfiltered samples of the previously decoded macroblocks of the same frame.
On Mon, Jun 21, 2010 at 6:30 PM, Muras <murasolisel...@gmail.com> wrote: > HI , > I want to know the fact that, > 1. will vp8 decoder supports I Macroblock in P frames, > 2. if yes how the prediction is performed in p frames I > macroblock.
> can any one explain about this.
> muras K
> On May 29, 7:03 am, gs_toronto <gsm...@alumni.uwaterloo.ca> wrote: > > > int i = 0; > > > d->value = 0; /* value = first 24 input bytes */ > > > while( ++i <= 24) > > > d->value = (d->value << 8) | *start_partition++;
> > > This will shift 24 bytes into a value that is only comprised of 4 bytes > > > to begin with. 24 bits, perhaps?
> > Actually, that should be 2 bytes. The code in 'read_bool' treats > > 'value' as being a 16-bit number with 'value' (as per 7.2) in bits > > [15...8]; the lower 8 bits contain '8-bit_count' additional bits to be > > used later. So the init should load 2 bytes. > > The comment alongside value ('contains at least 24...') appears to be > > left over from some other bit unpacking implementation. It should say > > "decoding value, left-justified in 16 bits'. The number of 'live' bits > > actually ranges from 9 to 16.
> > - 'range' and 'value' variables could be uint8 and uint16.
> > Another thing, in the given code, the byte-loading criterion is > > 'greedy', it loads a new byte as soon as it has room for it, before it > > needs to look at any bits in that byte. At the end of the decoding of > > a partition, it may end up reading one more byte than needed. In other > > standards, that can be a problem, but I think that partitions have > > explicitly specified byte lengths in VP8, right? if there is no need > > for the decoding to always end on a well-defined byte, there's not > > really a problem in over-reading.
> -- > You received this message because you are subscribed to the Google Groups > "WebM Discussion" group. > To post to this group, send email to webm-disc...@webmproject.org. > To unsubscribe from this group, send email to > webm-discuss+unsubscr...@webmproject.org<webm-discuss%2Bunsubscr...@webmproject.org> > . > For more options, visit this group at > http://groups.google.com/a/webmproject.org/group/webm-discuss/?hl=en.
Manjit Hota Lead Engineer System LSI Division Samsung India Software Operations Bagmane Tech Park Bangalore M : +91-9900110051 E : manjit.h...@gmail.com
