Ambisonics in an Ogg Opus Container
draft-ietf-codec-ambisonics-10

So, so far outside my area of expertise...

Thanks for everyone's work on this document. I have two minor comments.

---------------------------------------------------------------------------

§4:

Are implementors intended to assume that the elided matrix coefficients are all
0.0? If so, it's probably best to say so explicitly.

---------------------------------------------------------------------------

§6:

To be clear, these code points that are being reserved are for experimental
channel mappings in general, not just channel mappings that are specific to
ambisonics, right? If so, I would suggest calling this out explicitly.

I see that Section 5.2 attempts to Update RFC 7845 to allow for the larger
channel mapping information used by family 3, but I'm still a little
unclear about what behavior I should expect from a pure RFC 7845
implementation that receives a family 3 stream.  The actual mapping table
would be "too long", but would the implementation detect that, or just note
that it's an unrecognized family and generate silence? 

Section 1

I think we want to say "by adding itesm with values 2 and 3" to the
registry, since we add two entries and not a single superposed entry.

Section 3.1

While I can deduce this from the list of allowed numbers of channels,
noting that both ends of the range (0 and 14) are allowed values probably 
would add clarity.
   
Section 3.2

Figure 3 could perhaps make it more clear that C and K are not necessarily
equal.

The term "side information" is used without definition (and is not used in
RFC 7845).  Does this clause really add anything in comparison to if we
just say "The matrix MUST be provided in the channel mapping table portion
of the identification header, in place of a normal channel mapping table"?

Section 5.1

Family 255 is specified in Section 5.1.1.3 of RFC 7845, not 5.1.1.4.
(Also, the unqualified Section references should probably all be of the
form Section N of RFC 7845, for the benefift of the HTML linkification
tooling.)

Section 8

Sometimes I see a "Description" column that allows for in-registry
visibility that a range is for experimental usage.  I suppose it would not
be too hard to also modify the registry structure to add such a thing, if
you want.

Rich version of this review at:
https://mozphab-ietf.devsvcdev.mozaws.net/D3213



IMPORTANT
S 3.2.
>                                                        | Stream Count  |
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>        | Coupled Count | Demixing Matrix                               :
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>   
>          Figure 4: Channel Mapping Table for Channel Mapping Family 3

Are you saying I MUST use family 3?


S 5.2.
>         The remaining channel mapping families (2...254) are reserved.  A
>         demuxer implementation encountering a 'channel mapping family'
>         value that it does not recognize SHOULD NOT attempt to decode the
>         packets and SHOULD NOT use any information except for the first 19
>         octets of the ID header packet (Fig. 2) and the comment header
>         (Fig. 10).

What is the rationale for this change? Also, why are you doing it in
this document? This seems like it's going to be extremely hard for
implementors to find in future.

This is outside the DISCUSS criteria, but I would strongly recommend
you split this into a separate document.


COMMENTS
S 3.2.
>      whether or not there is a separate non-diegetic stereo stream.  This
>      corresponds to periphonic ambisonics from zeroth to fourteenth order
>      plus potentially two channels of non-diegetic stereo.  Explicitly the
>      allowed number of channels are 1, 3, 4, 6, 9, 11, 16, 18, 25, 27, 36,
>      38, 49, 51, 64, 66, 81, 83, 100, 102, 121, 123, 144, 146, 169, 171,
>      196, 198, 225, and 227.

This seems like the same graf as in 3.1, so perhaps merge these?


S 3.2.
>   
>                 Figure 3: Demixing in Channel Mapping Family 3
>   
>      The matrix MUST be provided as side information and MUST be stored in
>      the channel mapping table part of the identification header, c.f.
>      section 5.1.1 in [RFC7845].  The matrix replaces the need for a

I don't think you want "c.f." here, b/c "cf." means "compare". I
suspect you just want "see". Also, there's no period between c and f.


S 5.1.
>      Figure 6: Stereo Downmixing Matrix for Channel Mapping Family 2 and 3
>             - Ambisonic Channels Plus a Non-diegetic Stereo Stream
>   
>   5.  Updates to RFC 7845
>   
>   5.1.  Format of the Channel Mapping Table

What are the interop implications of this? I.e., is a file formatted
according to this document at all playable with an existing Ogg
decoder? If not, perhaps say so