[opus] [PATCH] Support for Ambisonics
Jean-Marc Valin
jmvalin at jmvalin.ca
Thu Mar 22 06:19:21 UTC 2018
Thanks, the libopus and the libopusenc patches are now merged.
Cheers,
Jean-Marc
On 03/20/2018 12:36 PM, Drew Allen wrote:
> Attached is an updated patch based on Jean-Marc and Mark's comments. :)
>
> Cheers,
> Drew
>
> On Tue, Mar 20, 2018 at 9:20 AM Jean-Marc Valin <jmvalin at jmvalin.ca
> <mailto:jmvalin at jmvalin.ca>> wrote:
>
> 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>
> > <mailto: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>
> > <mailto: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>
> <mailto: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>>
> > >>>> <mailto: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>
> <mailto: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> <mailto:opus at xiph.org
> <mailto:opus at xiph.org>>
> > >> http://lists.xiph.org/mailman/listinfo/opus
> > >>
> >
>
More information about the opus
mailing list