search for: opus_projection_decoder

Hey Jean-Marc, Thanks again! To your first point, I was only trying to copy how _multistream_'s c files function in this way, possibly that's worth refactoring as well (as a separate patch). I made the other changes you suggested. Cheers, Drew On Thu, Nov 23, 2017 at 9:02 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > On 11/22/2017 06:23 PM, Drew Allen wrote: >

Hi Jean-Marc, Here's my responses: 1) I didn't change how multistream_decode_float works in the argument list... I noticed it changes it's arguments depending on whether FIXED_POINT is used. I copied this style for the projection API as well. If this isn't desired, we should make those changes separately. 2) See above. 3) I only zero out initially. For the matrix_multiply_out

got it. actually that patch i sent you has something wrong with the mapping_matrix_multiply_short_out... let me fix that and will send you another patch soon. Cheers, Drew On Thu, Nov 23, 2017 at 10:34 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > On 11/23/2017 01:28 PM, Drew Allen wrote: > > To your first point, I was only trying to copy how _multistream_'s c > >

...st_creation_arguments(). > opus_projection_ambisonics_encoder_create() is intentionally called > with invalid arguments (initially, channels=0), which causes an error > to be returned and streams and coupled_streams are left uninitialized. > These uninitialized values are passed to > opus_projection_decoder_create(), which does not check that the > arguments are not too large before using them to attempt allocation of > a ridiculous amount of memory. Results may vary depending on the > uninitialized values of streams and coupled_streams. Valgrind reports > several uses of uninitialized v...

Hi Jean-Marc, Attached is an updated patch. I had to include some of Mark's suggestions in order to get the tests to work correctly. I will still submit a separate patch for him for a few other concerns he had after this one clears. Cheers, Drew On Thu, Nov 23, 2017 at 10:42 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > Actually, there's also something wrong with the

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

...us_projection_ambisonics_encoder_create() is intentionally called >>> with invalid arguments (initially, channels=0), which causes an error >>> to be returned and streams and coupled_streams are left uninitialized. >>> These uninitialized values are passed to >>> opus_projection_decoder_create(), which does not check that the >>> arguments are not too large before using them to attempt allocation of >>> a ridiculous amount of memory. Results may vary depending on the >>> uninitialized values of streams and coupled_streams. Valgrind reports >>>...

On 11/22/2017 06:23 PM, Drew Allen wrote: > 1) I didn't change how multistream_decode_float works in the argument > list... I noticed it changes it's arguments depending on whether > FIXED_POINT is used. I copied this style for the projection API as well. > If this isn't desired, we should make those changes separately. > > 2) See above. Just noticed the code will

I've solely addressed this concern here. Cheers, Drew On Fri, Nov 24, 2017 at 9:15 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > Hi Drew, > > I noticed you reverted the > output[output_rows * i] = (tmp + 16384) >> 15; > from the previous patch. That's still good. What should have been > changed is the float version: > output[output_rows * i] =

Aha good point! Im travelling this weekend but will submit another patch Monday morning. Cheers, Drew On Fri, Nov 24, 2017 at 9:15 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > Hi Drew, > > I noticed you reverted the > output[output_rows * i] = (tmp + 16384) >> 15; > from the previous patch. That's still good. What should have been > changed is the float

On 11/23/2017 01:28 PM, Drew Allen wrote: > To your first point, I was only trying to copy how _multistream_'s c > files function in this way, possibly that's worth refactoring as well > (as a separate patch). Well, the opus_multistream_decode_* calls were correct before your patch. It's only the encode ones that have the issues (should be addressed separately). Jean-Marc

...st, an issue occurs in test_creation_arguments(). opus_projection_ambisonics_encoder_create() is intentionally called with invalid arguments (initially, channels=0), which causes an error to be returned and streams and coupled_streams are left uninitialized. These uninitialized values are passed to opus_projection_decoder_create(), which does not check that the arguments are not too large before using them to attempt allocation of a ridiculous amount of memory. Results may vary depending on the uninitialized values of streams and coupled_streams. Valgrind reports several uses of uninitialized values such as: Con...

Hello, Attached is a patch to resolve a memory issue using the Projection API when compiling using a psuedo-stack / limited memory. Please let me any ?s you might have. Cheers, Drew -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.xiph.org/pipermail/opus/attachments/20171120/84e36e1b/attachment-0001.html> -------------- next part --------------

Actually, there's also something wrong with the in_short() function. For floating point (#else case), you shouldn't need shifting since you're already doing the scaling through a float multiply. Jean-Marc On 11/23/2017 01:39 PM, Drew Allen wrote: > got it. actually that patch i sent you has something wrong with the > mapping_matrix_multiply_short_out... let me fix that and

...s(). >> opus_projection_ambisonics_encoder_create() is intentionally called >> with invalid arguments (initially, channels=0), which causes an error >> to be returned and streams and coupled_streams are left uninitialized. >> These uninitialized values are passed to >> opus_projection_decoder_create(), which does not check that the >> arguments are not too large before using them to attempt allocation of >> a ridiculous amount of memory. Results may vary depending on the >> uninitialized values of streams and coupled_streams. Valgrind reports >> several uses of...

Hi Drew, I noticed you reverted the output[output_rows * i] = (tmp + 16384) >> 15; from the previous patch. That's still good. What should have been changed is the float version: output[output_rows * i] = (1/32768.f) * ((tmp + 16384) >> 15); which should just be: output[output_rows * i] = (1/(32768.f*32768.f)) * tmp; since there's no point in doing integer rounding when you

I only had a quick look, but your patch looks good except for the: output[output_rows * i] = (1/(32768.f*128.f))*tmp; For floating point, you shouldn't do the >>7 either. Just remove the >>8 from the floating-point calculation of tmp so that all the scaling is done in float. Cheers, Jean-Marc On 11/27/2017 04:01 PM, Drew Allen wrote: > Hi Jean-Marc, > > Attached is

Hi Jean-Marc, Attached is an updated patch with your suggestions + additional corrections I caught over the weekend. Cheers, Drew On Fri, Nov 24, 2017 at 10:08 AM Drew Allen <bitllama at google.com> wrote: > Aha good point! Im travelling this weekend but will submit another patch > Monday morning. > > Cheers, > Drew > On Fri, Nov 24, 2017 at 9:15 AM Jean-Marc Valin

Made a few minor tweaks to your patch (attached). Can you confirm you're OK with those and I haven't missed anything? Cheers, Jean-Marc On 12/04/2017 06:34 PM, Drew Allen wrote: > I've solely addressed this concern here. > > Cheers, > Drew > > On Fri, Nov 24, 2017 at 9:15 AM Jean-Marc Valin <jmvalin at jmvalin.ca > <mailto:jmvalin at jmvalin.ca>>

Done! On Tue, Nov 28, 2017 at 12:12 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > I only had a quick look, but your patch looks good except for the: > output[output_rows * i] = (1/(32768.f*128.f))*tmp; > > For floating point, you shouldn't do the >>7 either. Just remove the >>8 > from the floating-point calculation of tmp so that all the scaling is