[opus] [PATCH] Support for Ambisonics

Sun Sep 16 07:19:31 UTC 2018

Since the opusenc and opusinfo changes were independent I split them
up and landed the opusinfo changes (with updated mapping family
numbers).

 - Mark

On Thu, Sep 6, 2018 at 4:22 AM, Mark Harris <mark.hsj at gmail.com> wrote:
> 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