search for: mapping_matrix_get_size

...Hi Drew, > > Some additional comments on your mapping family 253 changes: > > 1) mapping_matrix_get_data: The computed pointer is not correct; > matrix should be cast to char * before the addition to avoid > incrementing in units of the structure size. > > Done. > 2) mapping_matrix_get_size: "rows * cols * sizeof(opus_int16)" may > not have the necessary alignment. There may be alignment padding > after the struct and also after the opus_int16 array and this function > is assumed to return a size that includes both. Also the > multiplication and return value sh...

Hi Drew, Some additional comments on your mapping family 253 changes: 1) mapping_matrix_get_data: The computed pointer is not correct; matrix should be cast to char * before the addition to avoid incrementing in units of the structure size. 2) mapping_matrix_get_size: "rows * cols * sizeof(opus_int16)" may not have the necessary alignment. There may be alignment padding after the struct and also after the opus_int16 array and this function is assumed to return a size that includes both. Also the multiplication and return value should use opus_int32...

...8 PM Mark Harris <mark.hsj at gmail.com> wrote: >> >> 5) opus_projection_decoder_init: demixing_matrix_size_in_bytes >> doesn't include the MappingMatrix struct or alignment but is used to >> determine the offset of the decoder; it should be obtained from >> mapping_matrix_get_size() and the field should be opus_int32. >> > This value is only meant to represent the "data" portion of the > mappingmatrix struct. Note that we include the struct size when computing > the multistream decoder pointer in get_multistream_decoder(). > I will make it opus_in...

...tional comments on your mapping family 253 changes: > >  1) mapping_matrix_get_data: The computed pointer is not correct; > matrix should be cast to char * before the addition to avoid > incrementing in units of the structure size. > > Done. >   > >  2) mapping_matrix_get_size: "rows * cols * sizeof(opus_int16)" may > not have the necessary alignment.  There may be alignment padding > after the struct and also after the opus_int16 array and this function > is assumed to return a size that includes both.  Also the > multiplication and...

...; >         >         >     > > >         >         >     > Done. > >         >         >     >   > >         >         >     > > >         >         >     >     4) I believe > mapping_matrix_get_size() > >         should be > >         >         returning > >         >         >     >     align(sizeof(MappingMatrix)) + > rows * cols * > >         >         sizeof(opus_int16) > >         >         ...

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 =

...the API (and in the include/ > directory), but I see several of these in mapping_matrix.h, which I > think is incorrect. > > Done. > 3) opus_projection_ambisonics_encoder_init() (which should be > OPUS_EXPORT) is missing from opus_projection.h > > Done. > 4) I believe mapping_matrix_get_size() should be returning > align(sizeof(MappingMatrix)) + rows * cols * sizeof(opus_int16) > rather than: > align(sizeof(MappingMatrix) + rows * cols * sizeof(opus_int16)) > to match what mapping_matrix_get_data() does > > Done. > 5) I'm not sure I understand the purpose of...

..., which I > > think is incorrect. > > > > Done. > > > > > > 3) opus_projection_ambisonics_encoder_init() (which should be > > OPUS_EXPORT) is missing from opus_projection.h > > > > Done. > > > > > > 4) I believe mapping_matrix_get_size() should be returning > > align(sizeof(MappingMatrix)) + rows * cols * sizeof(opus_int16) > > rather than: > > align(sizeof(MappingMatrix) + rows * cols * sizeof(opus_int16)) > > to match what mapping_matrix_get_data() does > > > > Done. > >...

...ations. OPUS_EXPORT is only meant for functions exposed to the API (and in the include/ directory), but I see several of these in mapping_matrix.h, which I think is incorrect. 3) opus_projection_ambisonics_encoder_init() (which should be OPUS_EXPORT) is missing from opus_projection.h 4) I believe mapping_matrix_get_size() should be returning align(sizeof(MappingMatrix)) + rows * cols * sizeof(opus_int16) rather than: align(sizeof(MappingMatrix) + rows * cols * sizeof(opus_int16)) to match what mapping_matrix_get_data() does 5) I'm not sure I understand the purpose of mapping_matrix_validate(). Can the user re...

...ory), but I see several of these in mapping_matrix.h, which I > think is incorrect. > > Done. >   > > 3) opus_projection_ambisonics_encoder_init() (which should be > OPUS_EXPORT) is missing from opus_projection.h > > Done. >   > > 4) I believe mapping_matrix_get_size() should be returning > align(sizeof(MappingMatrix)) + rows * cols * sizeof(opus_int16) > rather than: > align(sizeof(MappingMatrix) + rows * cols * sizeof(opus_int16)) > to match what mapping_matrix_get_data() does > > Done. >   > > 5) I'm not s...

...ction_ambisonics_encoder_init() (which > should be > >     >     OPUS_EXPORT) is missing from opus_projection.h > >     > > >     > Done. > >     >   > >     > > >     >     4) I believe mapping_matrix_get_size() should be > returning > >     >     align(sizeof(MappingMatrix)) + rows * cols * > sizeof(opus_int16) > >     >     rather than: > >     >     align(sizeof(MappingMatrix) + rows * cols * > sizeof(opus_int16)) >...

...OPUS_EXPORT) is missing from > opus_projection.h > > > > > > > > > > Done. > > > > > > > > > > > > > > > 4) I believe mapping_matrix_get_size() > > should be > > > returning > > > > > align(sizeof(MappingMatrix)) + rows * cols > * > > > sizeof(opus_int16) > > > > > rather than: > >...

...;         >     >     OPUS_EXPORT) is missing from opus_projection.h > >         >     > > >         >     > Done. > >         >     >   > >         >     > > >         >     >     4) I believe mapping_matrix_get_size() > should be > >         returning > >         >     >     align(sizeof(MappingMatrix)) + rows * cols * > >         sizeof(opus_int16) > >         >     >     rather than: > >         >     >     ali...

...from >> opus_projection.h >> > > > > >> > > > > Done. >> > > > > >> > > > > >> > > > > 4) I believe mapping_matrix_get_size() >> > should be >> > > returning >> > > > > align(sizeof(MappingMatrix)) + rows * >> cols * >> > > sizeof(opus_int16) >> > > > > rather...