search for: opusmsencoder

...c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/opus_multistream_encoder.c b/src/opus_multistream_encoder.c index 6e87337..7163d14 100644 --- a/src/opus_multistream_encoder.c +++ b/src/opus_multistream_encoder.c @@ -98,7 +98,8 @@ static opus_val32 *ms_get_preemph_mem(OpusMSEncoder *st) else ptr += align(mono_size); } - return (opus_val32*)(ptr+st->layout.nb_channels*120*sizeof(opus_val32)); + /* void* cast avoids clang -Wcast-align warning */ + return (opus_val32*)(void*)(ptr+st->layout.nb_channels*120*sizeof(opus_val32)); } static opus_v...

...9) For endianness conversion, you shouldn't have to even detect it. I think something like this (untested) should work for both little-endian and big endian: for (i=0;i<...;i++) short_ptr[i] = char_ptr[2*i] + (char_ptr[2*i] << 8); 10) Please don't store pointers (MappingMatrix, OpusMSEncoder, ...) in the OpusPEncoder/OpusPDecoder structs. All the data should be in a single struct with no pointers (except to constant data) so that they can be shallow-copied. Cheers, Jean-Marc On 27/09/17 03:19 PM, Drew Allen wrote: > Hello all, > > Here is an updated patch for channel mapp...

...icant byte. If you cast to char, then you need the endianness, but if you do (my_short>>8), you always get the most significant byte. Similarly, (my_short&0xff) is always the least significant byte. No need for an endianness check. > 10) Please don't store pointers (MappingMatrix, OpusMSEncoder, ...) in > the OpusPEncoder/OpusPDecoder structs. All the data should be in a > single struct with no pointers (except to constant data) so that they > can be shallow-copied. > > *Done.* Well, I still see pointers in your OpusProjectionDecoder. Cheers, Jean-Marc > On Wed, Oc...

Hi Michael, Sorry, I ended up looking at an older version and got confused. The new one looks good and I only have a few nits: 1) I think "allocation mode" should be renamed to "mapping type" or something similar that to make it less confusing. 2) After discussing with Tim, the conclusion is that we should use mapping family 254 in the short term. We can switch to mapping

...set mapping_type=MAPPING_TYPE_AMBISONICS. From my >> understanding, it would not change the code behaviour, but would make it >> clearer what the new type is for (or did I miss something?). > Done, I think. The patch still doesn't change behavior, but it now > sets the member OpusMSEncoder.mapping_type to MAPPING_TYPE_AMBISONICS. >

...n and big endian: for (i=0;i<...;i++) short_ptr[i] = char_ptr[2*i] + (char_ptr[2*i] << 8); *Can you show how to go from short -> char as well? I'm alittle confused on how this can be done without knowing the endianness first.* 10) Please don't store pointers (MappingMatrix, OpusMSEncoder, ...) in the OpusPEncoder/OpusPDecoder structs. All the data should be in a single struct with no pointers (except to constant data) so that they can be shallow-copied. *Done.* On Wed, Oct 4, 2017 at 10:30 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > Hi Drew, > > Here's...

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

...nc library. + if (_oge_use_projection(h->channel_mapping)) + { + len=27+(h->channels*(h->nb_streams+h->nb_coupled)*2); + } + else + { + len=27+h->channels; + } The fixed part of the header is 19 bytes; where does the 27 come from here? +struct OpusGenericEncoder { + OpusMSEncoder *ms; +#ifdef OPUS_HAVE_OPUS_PROJECTION_H + OpusProjectionEncoder *pr; +#endif + int family; +}; opus_multistream.h needs to be included for OpusMSEncoder. opus_projection.h can't be relied upon to include it since opus_projection.h may not be available. +OpusGenericEncoder *_oge_surround_c...

...to char, > then you need the endianness, but if you do (my_short>>8), you always > get the most significant byte. Similarly, (my_short&0xff) is always the > least significant byte. No need for an endianness check. > > > 10) Please don't store pointers (MappingMatrix, OpusMSEncoder, ...) in > > the OpusPEncoder/OpusPDecoder structs. All the data should be in a > > single struct with no pointers (except to constant data) so that they > > can be shallow-copied. > > > > *Done.* > > Well, I still see pointers in your OpusProjectionDecoder. >...

...then you need the endianness, but if you do (my_short>>8), you always > get the most significant byte. Similarly, (my_short&0xff) is always the > least significant byte. No need for an endianness check. > > > 10) Please don't store pointers (MappingMatrix, OpusMSEncoder, ...) in > > the OpusPEncoder/OpusPDecoder structs. All the data should be in a > > single struct with no pointers (except to constant data) so that they > > can be shallow-copied. > > > > *Done.* > > Well, I still see pointers in your Op...

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 =

...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[] 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? > Done. Yes, this is a merge conflict. > > 2) I noticed your patch add...

...254 should already set mapping_type=MAPPING_TYPE_AMBISONICS. From my > understanding, it would not change the code behaviour, but would make it > clearer what the new type is for (or did I miss something?). Done, I think. The patch still doesn't change behavior, but it now sets the member OpusMSEncoder.mapping_type to MAPPING_TYPE_AMBISONICS. -------------- next part --------------

...;jmvalin at jmvalin.ca > > <mailto: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[] 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? > > > > Done. Yes, this is a me...

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 declarations. OPUS_EXPORT is only meant for func...

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:

...ct 30, 2017 at 5:48 PM Jean-Marc Valin <jmvalin at jmvalin.ca > <mailto: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[] 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? > > Done. Yes, this is a merge conflict.  > > >...

...gt; >     > > >     >     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), >...

...> 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 > > > > &g...

...e 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 pro...