[vorbis-dev] Specification change requests

[Monty]

Sorry for taking a few hours, I was running my 'what the fuck was I
thinking' filter set to 'high' just to make sure...  Naturally folks
will want to verify the corrections.  They've been propogated to the
online docs as well.

On Tue, Oct 15, 2002 at 08:50:18PM +0200, Tor-Einar Jarnbjo wrote:
> ###############
> #2002-09-23-001
> 
> Floor 1 / curve computation / step 1:
> 
> 23) vector [floor1_final_Y] element [i] = 
>   [predicted] - ([val] - [lowroom]) - 1
> 
> should be:
> 
> 23) vector [floor1_final_Y] element [i] = 
>   [predicted] - [val] + [hiroom] - 1
> 
> due to floor1.c, line 1019:
> 
> val = -1-(val-hiroom);

Correction committed.

> ###############
> #2002-09-23-002
> 
> Floor 1 / curve computation / step 1:
> 
> 25) vector [floor1_final_Y] element [i] = 
>  [predicted] - (([val] - 1) divided by  2 using integer division)
> 
> should be:
> 
> 25) vector [floor1_final_Y] element [i] = 
>  [predicted] - (([val] + 1) divided by  2 using integer division)
>  
> due to floor1.c, line 1023:
> 
> val= -((val+1)>>1);

Correction committed.

> ###############
> #2002-09-23-003
> 
> Floor 1 / curve computation / step 2:
> 
> Steps 11 and 12 should be performed outside and after the iteration
> started in step 4. As far as I can see, performing these two steps
> for each iteration makes no sense, as the values set in the vector
> are overwritten by the later iterations.
> 
> 11) if ( [hx] is less than [n] ) {
>   12) render_line( [hx], [hy], [n], [hy], [floor] )
> }

Correct, this was a bracketing error.

> It also does not make sense to use the expensive function render_line
> to set all remaining elements in the vector to the same value, so 
> a better algorithm for this might be something like:
> 
> if ( [hx] is less than [n] ) {
>    iterate [i] over the range [hx] + 1 ... [n] - 1 {
>       set vector [floor1_final_Y]' element [i] to [hy]
>    }
> )

In this case it's more important to be clear than fast.

> ###############
> #2002-09-23-004
> 
> Floor 1 / curve computation / step 2:
> 
> Are steps 13 and 14 necessary? It does not make sense to me for the
> encoder to produce an audio packet where the condition in step 13
> is met.
> 
> 13) if ( [hx] is greater than [n] ) {
>   14) truncate vector [floor] to [n] elements
> }

It does not make sense, but it's important to be well defined.  An
ecoder may be cofused, or the stream may be malicious and looking to
create an intentional overrun.

> ###############
> #2002-09-23-005
> 
> Floor 1 / curve computation / step 2:
> 
> It is confusing that these steps are using a different parameter
> order for calling the same function:
> 
> 8) render_line( [lx], [hx], [ly], [hy], [floor] )
> 12) render_line( [hx], [hy], [n], [hy], [floor] )
> 
> As well as the definitions of the functions render_line
> and render_point:
> 
> render_line(x0, y0, x1, y1, v)
> render_point(x0, x1, y0, y1, X)

...and by 'confusing' you meant 'utterly wrong not to mention
confusing'.  Fixed.

> ###############
> #2002-10-15-001
[...]
> If the flag is false, the value of vorbis_mapping_coupling_steps 
> and the length and content of the two vectors are left unspecified,
> although they are referenced anyway in the audio packet decode section under 
> "nonzero vector propagate".

You're correct, the default value of vorbis_mapping_coupling_steps
should be zero.  Fixed.

> ###############
> #2002-10-15-002
> 
> In the document "Ogg logical bitstream framing", Ogg pages are described
> as beginning with a capture of the form "OggS". I've come across some
> older Ogg files, which have other data between the valid Ogg pages,
> mostly
> starting with the three letters "TAG".

This is id3v2 information, eplicitly against spec, and we've informed
a number of tool authors that they're violating spec by allowing id3v2
tags in Ogg streams.  Mike went into more detail.

> If it is common practice to add
> data between the pages, this should at least be mentioned in the framing
> specification,

It is not.

> and a suggestion should be made how to find the beginning
> of the next Ogg page. Simply seeking forward until finding the OggS 
> pattern
> seems obvious, but this could lead to some trial and errors if the 
> pattern
> occurs in the extra data.

No, because a full capture with valid CRC, sequence, version and
serial number is needed for stream capture.  You'd need a perfect,
accidental match of at least 128 bits.

> ###############
> #2002-10-15-003
> 
> The function used to describe the window slope in "Ogg Vorbis I format 
> specification: introduction and description" is:
> 
> y=sin(2PI*sin^2(x/n))
> (here, I even don't understand the notation you use)
> 
> The function described in the source code comment (window.c, line 29)
> is:
> 
> y=sin(sin(x)*sin(x)*2pi)
> 
> And at last, the function actually implemented in libvorbis, 
> (window.c, lines 33-38) is:
> 
> y=sin(sin(x/n*PI/2)^2*PI/2)
> 
> In the implementation, this is the calcuation of the leading flank,
> where x is running from 0 (y=0) to n, where n is the end of the flank
> (y=1).

Corrected.

Monty
--- >8 ----
List archives:  http://www.xiph.org/archives/
Ogg project homepage: http://www.xiph.org/ogg/
To unsubscribe from this list, send a message to 'vorbis-dev-request at xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is needed.
Unsubscribe messages sent to the list will be ignored/filtered.