[opus] [PATCH] Support for Ambisonics

[Jean-Marc Valin]

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 don't use something
that starts with an underscore.


> On Tue, Mar 20, 2018 at 8:38 AM Jean-Marc Valin <jmvalin at jmvalin.ca
> <mailto: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
>     > ope prefix like the other functions in the libopusenc library.
> 
>     I'd like to avoid using the ope_ prefix for functions that's aren't in
>     the public API. Right now there are other functions with a leading
>     underscore, so we'll have to fix them as well (not in this patch of
>     course). Maybe an "opeint_" prefix would do the job here (unless anyone
>     has a better idea)?
> 
>     > +int ope_encoder_deferred_init_with_mapping(OggOpusEnc *enc, int
>     > family, int streams,
>     >      int coupled_streams, const unsigned char *mapping) {
>     >    int ret;
>     >    int i;
>     >
>     > This code is allowing family 253 for a deferred init, but does not
>     > create a projection encoder in that case, so it looks like it will
>     > fail when writing the id header since it won't be able to get the
>     > demixing matrix.  It should probably not be allowing family 253 here.
> 
>     Actually, in the case of ope_encoder_deferred_init_with_mapping(), I
>     think it's probably best to just keep the existing code (not use
>     wrappers), since that function cannot be used by the projection encoder
>     at all.
> 
>             Jean-Marc
> 
> 
>     >  - Mark
>     >
>     >
>     > On Mon, Mar 19, 2018 at 3:00 PM, Drew Allen <bitllama at google.com
>     <mailto:bitllama at google.com>> wrote:
>     >> Hi Jean-Marc,
>     >>
>     >> I've modified my patches for libopus and libopusenc based on your
>     >> suggestions.
>     >>
>     >> Cheers,
>     >> Drew
>     >>
>     >> On Mon, Mar 19, 2018 at 2:05 PM Jean-Marc Valin
>     <jmvalin at jmvalin.ca <mailto:jmvalin at jmvalin.ca>> wrote:
>     >>>
>     >>> 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 that would return how large the
>     header
>     >>> *actually* is. Then you can use that value instead of
>     >>> MAX_PACKET_BUFFER_SIZE in init_stream()
>     >>> 2) I think the remaining if()s in ope_encoder_ctl() can also be
>     removed
>     >>> by adding another ctl() macro (like _oge_ctl()) with an extra
>     argument.
>     >>> In the case of OPUS_MULTISTREAM_GET_ENCODER_STATE_REQUEST, you can
>     >>> simply use _oge_ctl(enc->st,
>     >>> OPUS_MULTISTREAM_GET_ENCODER_STATE(stream_id, value))
>     >>> 3) On libopus itself, why "#define OPUS_HAVE_OPUS_PROJECTION_H 9000"
>     >>> instead of just "#define OPUS_HAVE_OPUS_PROJECTION_H"?
>     >>>
>     >>> Cheers,
>     >>>
>     >>>         Jean-Marc
>     >>>
>     >>> On 03/19/2018 02:53 PM, Drew Allen wrote:
>     >>>>
>     >>>> On Mon, Mar 19, 2018 at 11:52 AM Drew Allen
>     <bitllama at google.com <mailto:bitllama at google.com>
>     >>>> <mailto:bitllama at google.com <mailto:bitllama at google.com>>> wrote:
>     >>>>
>     >>>>     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
>     >>>>
>     >>>>
>     >>>>
>     >>>> _______________________________________________
>     >>>> opus mailing list
>     >>>> opus at xiph.org <mailto:opus at xiph.org>
>     >>>> http://lists.xiph.org/mailman/listinfo/opus
>     >>>>
>     >>
>     >>
>     >> _______________________________________________
>     >> opus mailing list
>     >> opus at xiph.org <mailto:opus at xiph.org>
>     >> http://lists.xiph.org/mailman/listinfo/opus
>     >>
>