Documentation corrections

[Mike Melanson]

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...
--
-Mike Melanson

[Yaowu Xu]

Mike,

Thanks for reviewing and incoming corrections. We are seeing your
message here, so your posting method is working. Please do send them
here.

cheers,
Yaowu

[Mike Melanson]

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:

pc->frame_type = (FRAME_TYPE)(data[0] & 1);
pc->version = (data[0] >> 1) & 7;
pc->show_frame = (data[0] >> 4) & 1;
first_partition_length_in_bytes =
(data[0] | (data[1] << 8) | (data[2] << 16)) >> 5;

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?

I'll post more as I spot them.

Thanks...
--
-Mike Melanson

[John Koleszar]

On Sat, May 22, 2010 at 12:30 AM, Mike Melanson <mi...@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>+subs...@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.

[John Koleszar]

+codec-devel, bcc webm-discuss

On Sat, May 22, 2010 at 12:52 AM, Mike Melanson <mi...@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:
>
>    pc->frame_type = (FRAME_TYPE)(data[0] & 1);
>    pc->version = (data[0] >> 1) & 7;
>    pc->show_frame = (data[0] >> 4) & 1;
>    first_partition_length_in_bytes =
>        (data[0] | (data[1] << 8) | (data[2] << 16)) >> 5;
>
> 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.

[gs_toronto]

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.

[gs_toronto]

> 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.

[Muras]

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

[Manjit Hota]

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.

For details you can go through the spec.

Thanks,
Manjit Hota

--
You received this message because you are subscribed to the Google Groups "WebM Discussion" group.
To post to this group, send email to webm-d...@webmproject.org.
To unsubscribe from this group, send email to webm-discuss...@webmproject.org.
For more options, visit this group at http://groups.google.com/a/webmproject.org/group/webm-discuss/?hl=en.

--
                          () ()
                         ////\\\
                        (@ @)
-------------oOO-------( _ )------Ooo---------------

Manjit Hota
Lead Engineer
System LSI Division
Samsung India Software Operations
Bagmane Tech Park
Bangalore
M : +91-9900110051
E : manji...@gmail.com

-----------------------------------------------------------
                    /__/      \__\
                    |   |        |   |
                     | |          | |
                  ooO         Ooo