[opus] [PATCH] Support for Ambisonics

[Mark Harris]

Hi Drew,

Sorry for the delay.

FYI the patch that you attached is not your latest version.  This
thread that you replied to is an older thread; the latest version is
on a different thread.

The patch does not use the mapping family numbers used by libopus and
libopusenc; could you update it to use family 2 and 3 rather than 254
and 253?

The patch also appears to break encoding of surround files and
non-Ambisonic files with more than 8 channels.  It will attempt to use
mapping family 0 but that is only valid for mono and stereo.

In this patch, an additional use_permute argument is added to a number
of functions that indicates whether to perform channel permutation.
It would be preferable to simply add this as a field in the oe_enc_opt
structure, which is already an argument and already contains other
options like this.  When I brought this up in the past you made a
patch that did that, however your latest patches are still using the
additional argument.  Can you please drop the additional argument and
incorporate your earlier patch that uses the struct field?  That
should simplify this patch.

Requiring the user to specify internal Ogg Opus mapping family numbers
may be okay for a quick hack to experiment with ambisonics, but
doesn't seem like the best solution for adding official support for
ambisonics in opus-tools, which is intended for use by end users.
Ideally no additional options would be needed and opusenc would be
able to identify a WAV file containing Ambix-compatible ambisonic
channels and by default convert such a file to a compressed ambisonic
output file in the format described by draft-ietf-codec-ambisonics.
However it appears that there is unfortunately no agreed upon metadata
to identify such WAV files, and the user will need a way to manually
override the WAV file's metadata to indicate that the input channels
are ambisonic channels.

If the user will have to manually specify some command line option, it
seems like an option that explicitly overrides the meaning of the
input channels, like "--channels ambix" to specify that the input
channels are Ambix-compatible channels (ACN order, SN3D normalization)
would be preferable to overriding the mapping family number of the
output.  Besides being cryptic, setting a mapping family number
directly is more likely to mislead the user into thinking that the
option affects only the mapping family number that is written in the
output file and that the input file and encoding is otherwise
unaffected.  This is of course false; for example the normal
permutation of the input channels, coupling, and bitrate allocation
will not be performed if it is known that the input channels are
ambisonic channels.

An option that overrides the meaning of the input channels would also
allow a future version to support other conventions if needed, e.g.
"--channels fuma" to convert ambisonic channels from FuMa format, or
"--channels ambix,stereo" / "--channels stereo,ambix" to convert an
input file with non-diegetic stereo either after or before the
ambisonic channels, or to force a particular surround configuration,
or force discrete unpermuted channels that would otherwise be permuted
and treated as a surround configuration.

Such an option would also allow the existing --downmix-mono and
--downmix-stereo options to be supported for ambisonic input, in case
the user did not actually want it encoded as ambisonics.  (There is no
need to support these in the first version, however if not supported
it should report an error if these options are used with ambisonic
input, and should not downmix as if the channels were surround
channels.)  In theory those options could be used with an option that
specifies ambisonics input using a mapping family number, but that
would be especially confusing because nowhere would it actually write
that mapping family number; it would be used only to determine the
meaning of the input channels.

Can you explain in terms that an end user would understand how they
should choose between mapping family 2 and 3?  Ideally the user should
not have to know the difference or be made to choose between them.  Is
there a reason to not use mapping family 3 for order 1, 2, and 3, and
mapping family 2 otherwise?  If it is a user option the user should be
given some guidance.

The opusenc manual page should also document any other new options.

Finally, Google's IPR disclosure at
https://datatracker.ietf.org/ipr/3200/ discloses that Google has
applied for a patent related to this technology and will grant a
"Royalty-Free, Reasonable and Non-Discriminatory License to All
Implementers".  Since this would be an implementation of this
specification it is my understanding that Google expects the
implementation to be bound by the terms of its patent license.  For
Opus itself, the relevant disclosures include the full text of the
patent license directly in the IPR disclosure, and the Opus repository
contains a file that links to each of them:
https://gitlab.xiph.org/xiph/opus/raw/master/LICENSE_PLEASE_READ.txt
If the full text of the patent license can be added to Google's IPR
disclosure then a similar file can be created for opus-tools.
Alternatively the full text of the patent license could be included
directly in the file as part of the patch.

Thanks,
 - Mark


On Mon, Jul 30, 2018 at 11:40 AM, Andrew Allen <bitllama at google.com> wrote:
> Friendly ping for the opus-tools patch...
>
>
> ---------- Forwarded message ---------
> From: Drew Allen <bitllama at google.com>
> Date: Mon, Mar 19, 2018 at 2:53 PM
> Subject: Re: [PATCH] Support for Ambisonics
> To: opus at xiph.org <opus at xiph.org>
>
>
>
> On Mon, Mar 19, 2018 at 11:52 AM Drew Allen <bitllama at google.com> wrote:
>>
>> Hello all,
>>
>> Sorry for the delay (got really sick last week).
>>
>> Attached are updated patches for libopus, libopusenc, opusfile and
>> opus-tools.
>>
>> Note that the patches for libopusenc, opusfile and opus-tools are
>> dependent on the patch for libopus.
>>
>> Please let me know if you have any additional followup comments or
>> questions.
>>
>> Cheers,
>> Drew