similar to: [PATCH] Support for Channel Mapping 253.

Hi Drew, Here's some comments on your patch (in no particular order): 1) I see that it's adding an #include of stdarg.h to opus_multistream.h Is that left over from the previous version? 2) Someone on this list might know better than I do on that one, but for the new _ctl_va_list() calls, I believe the convention is for va_start() and va_end() to appear in the caller rather than in in

Hi Drew, On 10/10/17 02:29 PM, Drew Allen wrote: > 2) Someone on this list might know better than I do on that one, but for > the new _ctl_va_list() calls, I believe the convention is for va_start() > and va_end() to appear in the caller rather than in in the va_list() > function itself. > > *My understanding is that it's impossible to pass ellipsis to another >

1. Fixed the va_end() thing. Thanks for the clarification. 2. I've redone the mapping matrix so it only either handles floats or int16 explicitly. please let me know if you'd like another functionality, but my thoughts on this were to keep the val16 handled entirely inside multistream API. 3. endianness fixed. thank you! 4. Removed those pointers, accidentally left them in but they were

Hi Jean-Marc, Thanks for the feedback. Attached are my comments and an updated patch. 1) I see that it's adding an #include of stdarg.h to opus_multistream.h Is that left over from the previous version? *That was a typo. Fixed.* 2) Someone on this list might know better than I do on that one, but for the new _ctl_va_list() calls, I believe the convention is for va_start() and va_end() to

Hi Drew, I think the libopusenc patch is better, but there's still a few issues left: 1) The static MAX_PACKET_BUFFER_SIZE value is still problematic because if you link libopusenc with a new version of libopus that supports higher order projection or just more projection channels for order 3, then you will overflow the buffer. I think what you'd want is a _ope_opus_header_get_size() call

Hi Drew, Thanks for addressing these issues. A few remaining things: 1) In this line: buf[i] = (opus_int16)demixing_matrix[2*i] + ((opus_int16)demixing_matrix[2*i+1] << 8); you might want to test it with undefined-behaviour sanitizer, but I believe the second term will be undefined for negative values (the value would be positive after the cast and only be negative because of the shift).

Sure, ill send that asap On Wed, Nov 8, 2017 at 4:44 PM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > Hi Drew, > > Your ambisonics patch is already merged. Can you send a patch that > applies to master? > > Jean-Marc > > On 11/08/2017 07:05 PM, Drew Allen wrote: > > Hey Jean-Marc, > > > > I found a bug regarding exporting the matrix that

Apologies! That patch will not work correctly with Opus 1.2. Will send an updated patch shortly!! On Mon, Mar 26, 2018 at 11:56 AM Drew Allen <bitllama at google.com> wrote: > Hello all! > > I've attached an updated patch for opusfile, based on formatting > suggestions I got for the opus and libopusenc patches. > > Cheers! > Drew > > On Thu, Mar 22, 2018 at

Thanks! 2 down, 2 to go. :D :D :D On Wed, Mar 21, 2018 at 11:19 PM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > Thanks, the libopus and the libopusenc patches are now merged. > > Cheers, > > Jean-Marc > > On 03/20/2018 12:36 PM, Drew Allen wrote: > > Attached is an updated patch based on Jean-Marc and Mark's comments. :) > > > >

Attached is an updated patch based on Jean-Marc and Mark's comments. :) Cheers, Drew On Tue, Mar 20, 2018 at 9:20 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > On 03/20/2018 11:51 AM, Drew Allen wrote: > > Just to confirm, I would use opeint_* for all the > > OpusGenericEncoder-related functions? > > Correct. Or you can also use oge_ if you like. Just

>>> Drew Allen <bitllama at google.com> schrieb am 10.10.2017 um 20:29 in Nachricht <CABQ9DctQ0+gBgUif7BBJpjjKR7_V_H5OC1JM47w50oaaLXL4Tg at mail.gmail.com>: > Hi Jean-Marc, > > Thanks for the feedback. Attached are my comments and an updated patch. > > 1) I see that it's adding an #include of stdarg.h to opus_multistream.h > Is that left over from the

apologies, this is the correct patch I meant to send: On Mon, Jun 12, 2017 at 9:19 AM Drew Allen <bitllama at google.com> wrote: > Thanks again Mark for taking a look and pointing out the issues that > previous patch had. I've addressed your concerns. The tests should no > longer give any warnings or errors regarding usage or c89. I've ensured the > memory issues have

Just to confirm, I would use opeint_* for all the OpusGenericEncoder-related functions? On Tue, Mar 20, 2018 at 8:38 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > Hi Mark, Drew, > > On 03/20/2018 02:40 AM, Mark Harris wrote: > > + int _oge_use_projection(int channel_mapping); > > > > These functions are part of libopusenc, so I'd expect them to have an

Hi Jean-Marc, Thanks so much for your review. Attached are my comments and an updated patch. On Mon, Oct 30, 2017 at 5:48 PM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > Hi Drew, > > I've had some time to dig more deeply into your patch. Here's some more > in-depth comments: > > 1) I note that your OpusMSEncoder struct in private.h adds a > subframe_mem[]

Thanks for looking over this, Mark! I'm travelling this week but when I'm back I'll address all of your concerns and send,out another patch. :) On Jun 7, 2017 9:56 AM, "Mark Harris" <mark.hsj at gmail.com> wrote: > On Tue, May 30, 2017 at 3:13 PM, Drew Allen <bitllama at google.com> wrote: > > Hello all, > > > > Attached is the initial

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 -------------- next part -------------- An HTML attachment was

Hi Drew, Some comments on the libopusenc patch: + int _oge_use_projection(int channel_mapping); These functions are part of libopusenc, so I'd expect them to have an ope prefix like the other functions in the libopusenc library. + if (_oge_use_projection(h->channel_mapping)) + { + len=27+(h->channels*(h->nb_streams+h->nb_coupled)*2); + } + else + { +

I think you just attached the wrong (previous) version of the patch. Jean-Marc On 11/28/2017 12:24 PM, Drew Allen wrote: > Done! > > On Tue, Nov 28, 2017 at 12:12 AM Jean-Marc Valin <jmvalin at jmvalin.ca > <mailto:jmvalin at jmvalin.ca>> wrote: > > I only had a quick look, but your patch looks good except for the: >   output[output_rows * i] =

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

Hello all, Here is an updated patch for channel mapping 253 support using the proposed "Projection" API. Matrix operations support and tests are included. Looking forward to your feedback. Cheers, Drew On Tue, Sep 19, 2017 at 9:04 AM Drew Allen <bitllama at google.com> wrote: > Hello all, > > Attached is an up-to-date patch for supporting channel mapping 253. Please