Ok perfect—I’ll get you changes to opusenc and opusdec this week.<br><div class="gmail_quote"><div dir="ltr">On Sun, Sep 16, 2018 at 3:19 AM Mark Harris <<a href="mailto:mark.hsj@gmail.com">mark.hsj@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Since the opusenc and opusinfo changes were independent I split them<br>
up and landed the opusinfo changes (with updated mapping family<br>
numbers).<br>
<br>
 - Mark<br>
<br>
<br>
On Thu, Sep 6, 2018 at 4:22 AM, Mark Harris <<a href="mailto:mark.hsj@gmail.com" target="_blank">mark.hsj@gmail.com</a>> wrote:<br>
> Hi Drew,<br>
><br>
> Sorry for the delay.<br>
><br>
> FYI the patch that you attached is not your latest version.  This<br>
> thread that you replied to is an older thread; the latest version is<br>
> on a different thread.<br>
><br>
> The patch does not use the mapping family numbers used by libopus and<br>
> libopusenc; could you update it to use family 2 and 3 rather than 254<br>
> and 253?<br>
><br>
> The patch also appears to break encoding of surround files and<br>
> non-Ambisonic files with more than 8 channels.  It will attempt to use<br>
> mapping family 0 but that is only valid for mono and stereo.<br>
><br>
> In this patch, an additional use_permute argument is added to a number<br>
> of functions that indicates whether to perform channel permutation.<br>
> It would be preferable to simply add this as a field in the oe_enc_opt<br>
> structure, which is already an argument and already contains other<br>
> options like this.  When I brought this up in the past you made a<br>
> patch that did that, however your latest patches are still using the<br>
> additional argument.  Can you please drop the additional argument and<br>
> incorporate your earlier patch that uses the struct field?  That<br>
> should simplify this patch.<br>
><br>
> Requiring the user to specify internal Ogg Opus mapping family numbers<br>
> may be okay for a quick hack to experiment with ambisonics, but<br>
> doesn't seem like the best solution for adding official support for<br>
> ambisonics in opus-tools, which is intended for use by end users.<br>
> Ideally no additional options would be needed and opusenc would be<br>
> able to identify a WAV file containing Ambix-compatible ambisonic<br>
> channels and by default convert such a file to a compressed ambisonic<br>
> output file in the format described by draft-ietf-codec-ambisonics.<br>
> However it appears that there is unfortunately no agreed upon metadata<br>
> to identify such WAV files, and the user will need a way to manually<br>
> override the WAV file's metadata to indicate that the input channels<br>
> are ambisonic channels.<br>
><br>
> If the user will have to manually specify some command line option, it<br>
> seems like an option that explicitly overrides the meaning of the<br>
> input channels, like "--channels ambix" to specify that the input<br>
> channels are Ambix-compatible channels (ACN order, SN3D normalization)<br>
> would be preferable to overriding the mapping family number of the<br>
> output.  Besides being cryptic, setting a mapping family number<br>
> directly is more likely to mislead the user into thinking that the<br>
> option affects only the mapping family number that is written in the<br>
> output file and that the input file and encoding is otherwise<br>
> unaffected.  This is of course false; for example the normal<br>
> permutation of the input channels, coupling, and bitrate allocation<br>
> will not be performed if it is known that the input channels are<br>
> ambisonic channels.<br>
><br>
> An option that overrides the meaning of the input channels would also<br>
> allow a future version to support other conventions if needed, e.g.<br>
> "--channels fuma" to convert ambisonic channels from FuMa format, or<br>
> "--channels ambix,stereo" / "--channels stereo,ambix" to convert an<br>
> input file with non-diegetic stereo either after or before the<br>
> ambisonic channels, or to force a particular surround configuration,<br>
> or force discrete unpermuted channels that would otherwise be permuted<br>
> and treated as a surround configuration.<br>
><br>
> Such an option would also allow the existing --downmix-mono and<br>
> --downmix-stereo options to be supported for ambisonic input, in case<br>
> the user did not actually want it encoded as ambisonics.  (There is no<br>
> need to support these in the first version, however if not supported<br>
> it should report an error if these options are used with ambisonic<br>
> input, and should not downmix as if the channels were surround<br>
> channels.)  In theory those options could be used with an option that<br>
> specifies ambisonics input using a mapping family number, but that<br>
> would be especially confusing because nowhere would it actually write<br>
> that mapping family number; it would be used only to determine the<br>
> meaning of the input channels.<br>
><br>
> Can you explain in terms that an end user would understand how they<br>
> should choose between mapping family 2 and 3?  Ideally the user should<br>
> not have to know the difference or be made to choose between them.  Is<br>
> there a reason to not use mapping family 3 for order 1, 2, and 3, and<br>
> mapping family 2 otherwise?  If it is a user option the user should be<br>
> given some guidance.<br>
><br>
> The opusenc manual page should also document any other new options.<br>
><br>
> Finally, Google's IPR disclosure at<br>
> <a href="https://datatracker.ietf.org/ipr/3200/" rel="noreferrer" target="_blank">https://datatracker.ietf.org/ipr/3200/</a> discloses that Google has<br>
> applied for a patent related to this technology and will grant a<br>
> "Royalty-Free, Reasonable and Non-Discriminatory License to All<br>
> Implementers".  Since this would be an implementation of this<br>
> specification it is my understanding that Google expects the<br>
> implementation to be bound by the terms of its patent license.  For<br>
> Opus itself, the relevant disclosures include the full text of the<br>
> patent license directly in the IPR disclosure, and the Opus repository<br>
> contains a file that links to each of them:<br>
> <a href="https://gitlab.xiph.org/xiph/opus/raw/master/LICENSE_PLEASE_READ.txt" rel="noreferrer" target="_blank">https://gitlab.xiph.org/xiph/opus/raw/master/LICENSE_PLEASE_READ.txt</a><br>
> If the full text of the patent license can be added to Google's IPR<br>
> disclosure then a similar file can be created for opus-tools.<br>
> Alternatively the full text of the patent license could be included<br>
> directly in the file as part of the patch.<br>
><br>
> Thanks,<br>
>  - Mark<br>
><br>
><br>
> On Mon, Jul 30, 2018 at 11:40 AM, Andrew Allen <<a href="mailto:bitllama@google.com" target="_blank">bitllama@google.com</a>> wrote:<br>
>> Friendly ping for the opus-tools patch...<br>
>><br>
>><br>
>> ---------- Forwarded message ---------<br>
>> From: Drew Allen <<a href="mailto:bitllama@google.com" target="_blank">bitllama@google.com</a>><br>
>> Date: Mon, Mar 19, 2018 at 2:53 PM<br>
>> Subject: Re: [PATCH] Support for Ambisonics<br>
>> To: <a href="mailto:opus@xiph.org" target="_blank">opus@xiph.org</a> <<a href="mailto:opus@xiph.org" target="_blank">opus@xiph.org</a>><br>
>><br>
>><br>
>><br>
>> On Mon, Mar 19, 2018 at 11:52 AM Drew Allen <<a href="mailto:bitllama@google.com" target="_blank">bitllama@google.com</a>> wrote:<br>
>>><br>
>>> Hello all,<br>
>>><br>
>>> Sorry for the delay (got really sick last week).<br>
>>><br>
>>> Attached are updated patches for libopus, libopusenc, opusfile and<br>
>>> opus-tools.<br>
>>><br>
>>> Note that the patches for libopusenc, opusfile and opus-tools are<br>
>>> dependent on the patch for libopus.<br>
>>><br>
>>> Please let me know if you have any additional followup comments or<br>
>>> questions.<br>
>>><br>
>>> Cheers,<br>
>>> Drew<br>
</blockquote></div>