search for: opus_projection_encoder

...gt; at 0x100001330: test_creation_arguments (test_opus_projection.c:252) > > Additionally, the demixing_matrix appears to be used uninitialized > when order_plus_one is 1 (or greater than 4): > > Conditional jump or move depends on uninitialised value(s) > at 0x10006A6DF: opus_projection_encoder_init_impl > (opus_projection_encoder.c:125) > by 0x10006A9CA: opus_projection_ambisonics_encoder_init > (opus_projection_encoder.c:196) > by 0x10006AC7B: opus_projection_ambisonics_encoder_create > (opus_projection_encoder.c:261) > by 0x1000013D9: test_creation_argumen...

Hello all, Attached is the initial proposed implementation for ch.mapping 253/3, based on the IETF proposal: https://tools.ietf.org/html/draft-ietf-codec-ambisonics-03 A brief overview of the patch, as it is slightly lengthy: After discussion with Jean-Marc, we determined that ch.253/3 will need the demixing matrix as part of the encoder/decoder struct stack and thus will require a new

...(test_opus_projection.c:252) >>> >>> Additionally, the demixing_matrix appears to be used uninitialized >>> when order_plus_one is 1 (or greater than 4): >>> >>> Conditional jump or move depends on uninitialised value(s) >>> at 0x10006A6DF: opus_projection_encoder_init_impl >>> (opus_projection_encoder.c:125) >>> by 0x10006A9CA: opus_projection_ambisonics_encoder_init >>> (opus_projection_encoder.c:196) >>> by 0x10006AC7B: opus_projection_ambisonics_encoder_create >>> (opus_projection_encoder.c:261) >&g...

...s created by a stack allocation at 0x100001330: test_creation_arguments (test_opus_projection.c:252) Additionally, the demixing_matrix appears to be used uninitialized when order_plus_one is 1 (or greater than 4): Conditional jump or move depends on uninitialised value(s) at 0x10006A6DF: opus_projection_encoder_init_impl (opus_projection_encoder.c:125) by 0x10006A9CA: opus_projection_ambisonics_encoder_init (opus_projection_encoder.c:196) by 0x10006AC7B: opus_projection_ambisonics_encoder_create (opus_projection_encoder.c:261) by 0x1000013D9: test_creation_arguments (test_opus_projection.c:268...

...test_creation_arguments (test_opus_projection.c:252) >> >> Additionally, the demixing_matrix appears to be used uninitialized >> when order_plus_one is 1 (or greater than 4): >> >> Conditional jump or move depends on uninitialised value(s) >> at 0x10006A6DF: opus_projection_encoder_init_impl >> (opus_projection_encoder.c:125) >> by 0x10006A9CA: opus_projection_ambisonics_encoder_init >> (opus_projection_encoder.c:196) >> by 0x10006AC7B: opus_projection_ambisonics_encoder_create >> (opus_projection_encoder.c:261) >> by 0x1000013D...

...rename the function that validates the correct order before this is > called (It fails and hence the init() func fails if order_plus_one is < 2 or >> 4.) Where does it check for order_plus_one < 2 or > 4? The draft allows order 0 and also higher orders. The current check in src/opus_projection_encoder.c line 65 checks for "order_plus_one < 1 || order_plus_one > 15". I don't see a more restrictive check, and if there is one somewhere it doesn't prevent opus_projection_ambisonics_encoder_init() from accessing uninitialized memory. This can be seen by running test_opus_pro...

Hi Mark, Attached are corrections based on your comments. I will extend these to the patch I recently submitted to fix memory issues once that is resolved as well. Cheers, Drew On Sat, Nov 18, 2017 at 5:48 PM Mark Harris <mark.hsj at gmail.com> wrote: > Hi Drew, > > Some additional comments on your mapping family 253 changes: > > 1) mapping_matrix_get_data: The computed

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 >

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

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 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[]

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).

thanks for all your feedback. here's the revised patch: On Wed, Oct 11, 2017 at 2:20 PM Timothy B. Terriberry <tterribe at xiph.org> wrote: > Jean-Marc Valin wrote: > > I think you'll want something like: > > (opus_int16)((unsigned)demixing_matrix[2*i+1] << 8) > > (though you might want to check it too) > > FWIW, we use the construct > int s =

Here's another one. On Thu, Nov 2, 2017 at 9:54 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > Hi Drew, > > We're getting there... Some minor comments: > > 1) The public header file should not have an > #ifdef ENABLE_EXPERIMENTAL_AMBISONICS > since that would require the user code to define it. > > Done > 2) Why do you have #define

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[] that's not in opus_multistream_encoder.c. I assume it's due to a merge problem (that field was removed some time ago), but can you confirm/fix the issue? 2) I noticed your patch adds many OPUS_EXPORT

Hello all, Attached is an up-to-date patch for supporting channel mapping 253. Please advise and Thank you for your time. Cheers, Drew -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.xiph.org/pipermail/opus/attachments/20170919/055d9034/attachment-0001.html> -------------- next part -------------- A non-text attachment was scrubbed... Name:

Hi Drew, We're getting there... Some minor comments: 1) The public header file should not have an #ifdef ENABLE_EXPERIMENTAL_AMBISONICS since that would require the user code to define it. 2) Why do you have #define MAPPING_MATRIX_C ? 3) Looks like MAPPING_MATRIX_MAX_SIZE is not longer useful, right? 4) Even though it's not strictly necessary here, please add parentheses to the

Hi Drew, Thanks for the update. Your patch is now in master. Now, it would be good if you could think of a way to reduce the stack usage as we discussed. Cheers, Jean-Marc On 11/07/2017 04:28 PM, Drew Allen wrote: > Here's another patch. Cheers! > > On Mon, Nov 6, 2017 at 10:08 AM Drew Allen <bitllama at google.com > <mailto:bitllama at google.com>> wrote: >

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