[FFmpeg-devel] [PATCH] Support for Ambisonics and OpusProjection* API.

Michael Niedermayer michael at niedermayer.cc
Wed Apr 11 04:10:29 EEST 2018


On Wed, Mar 28, 2018 at 09:59:00PM +0000, Drew Allen wrote:
> Hello all,
> 
> My name is Andrew Allen and I'm a contributor to Opus, supporting new
> channel mappings 2 and 3 for ambisonics compression. I've attached a patch
> to support the new OpusProjectionEncoder and OpusProjectionDecoder APIs for
> handling the new channel mapping 3 in OPUS.
> 
> Please let me know of any changes I should make or if there are any
> questions I can help answer.
> 
> Cheers,
> Drew

>  libopusdec.c |  160 ++++++++++++++++++++++++++++++------
>  libopusenc.c |  257 ++++++++++++++++++++++++++++++++++++++++++++++++++---------
>  opus.c       |   18 +---
>  3 files changed, 358 insertions(+), 77 deletions(-)
> 1f1fae1e93478880b50cd5432bb7479bc816f659  ffmpeg-Support-Ambisonics.patch
> From a897b4d9b1ebe9031b98a9e507c28355ef9a44ba Mon Sep 17 00:00:00 2001
> From: Andrew Allen <bitllama at google.com>
> Date: Wed, 28 Mar 2018 14:48:46 -0700
> Subject: [PATCH] Support for Ambisonics and OpusProjection* API.

> 
> ---
>  libavcodec/libopusdec.c | 160 ++++++++++++++++++++-----
>  libavcodec/libopusenc.c | 257 ++++++++++++++++++++++++++++++++++------
>  libavcodec/opus.c       |  18 ++-
>  3 files changed, 358 insertions(+), 77 deletions(-)
> 
> diff --git a/libavcodec/libopusdec.c b/libavcodec/libopusdec.c
> index 3d2ee5b61b..d4b5a459b9 100644
> --- a/libavcodec/libopusdec.c
> +++ b/libavcodec/libopusdec.c
> @@ -21,6 +21,9 @@
>  
>  #include <opus.h>
>  #include <opus_multistream.h>
> +#ifdef OPUS_HAVE_OPUS_PROJECTION_H
> +#include <opus_projection.h>
> +#endif
>  
>  #include "libavutil/internal.h"
>  #include "libavutil/intreadwrite.h"
> @@ -33,9 +36,93 @@
>  #include "mathops.h"
>  #include "libopus.h"
>  
> +typedef struct OpusGenericDecoder {
> +#ifdef OPUS_HAVE_OPUS_PROJECTION_H
> +    OpusProjectionDecoder *pr;
> +#endif
> +    OpusMSDecoder *ms;
> +} OpusGenericDecoder;
> +
> +static int libopus_generic_decoder_init(OpusGenericDecoder *st, int Fs,
> +                                        int channels, int nb_streams,
> +                                        int nb_coupled, uint8_t *mapping,
> +                                        uint8_t *dmatrix) {
> +    int err;
> +    if (dmatrix != NULL) {
> +#ifdef OPUS_HAVE_OPUS_PROJECTION_H
> +        opus_int32 size;
> +        size = 2 * channels * (nb_streams + nb_coupled);
> +        st->pr = opus_projection_decoder_create(Fs, channels, nb_streams,
> +            nb_coupled, dmatrix, size, &err);
> +#else
> +        err = OPUS_UNIMPLEMENTED;
> +#endif
> +        st->ms = NULL;
> +        return err;
> +    }
> +#ifdef OPUS_HAVE_OPUS_PROJECTION_H
> +    st->pr = NULL;
> +#endif
> +    st->ms = opus_multistream_decoder_create(Fs, channels, nb_streams,
> +        nb_coupled, mapping, &err);
> +    return err;
> +}
> +
> +static void libopus_generic_decoder_cleanup(OpusGenericDecoder *st)
> +{

> +#ifdef OPUS_HAVE_OPUS_PROJECTION_H
> +    if (st->pr) opus_projection_decoder_destroy(st->pr);
> +#endif
> +    if (st->ms) opus_multistream_decoder_destroy(st->ms);
> +}
> +
> +static int libopus_generic_decode(OpusGenericDecoder *st,
> +        const unsigned char *data, opus_int32 len, opus_int16 *pcm,
> +        int frame_size, int decode_fec) {
> +    int ret;
> +
> +#if defined(OPUS_HAVE_OPUS_PROJECTION_H)

inconsistent comapred to previous #ifdef


> +    if (st->pr){
> +        ret=opus_projection_decode(st->pr, data, len, pcm, frame_size,
> +            decode_fec);
> +        return ret;
> +    }
> +#endif
> +    ret=opus_multistream_decode(st->ms, data, len, pcm, frame_size,
> +        decode_fec);
> +    return ret;
> +}
> +
> +static int libopus_generic_decode_float(OpusGenericDecoder *st,
> +        const unsigned char *data, opus_int32 len, float *pcm, int frame_size,
> +        int decode_fec) {
> +    int ret;
> +
> +#if defined(OPUS_HAVE_OPUS_PROJECTION_H)
> +    if (st->pr){
> +        ret=opus_projection_decode_float(st->pr, data, len, pcm, frame_size,
> +            decode_fec);
> +        return ret;
> +    }
> +#endif
> +    ret=opus_multistream_decode_float(st->ms, data, len, pcm, frame_size,
> +        decode_fec);
> +    return ret;
> +}
> +
> +#ifdef OPUS_HAVE_OPUS_PROJECTION_H
> +# define libopus_generic_decoder_ctl(st, request) \
> +    ((st)->pr != NULL ? \
> +    opus_projection_decoder_ctl((st)->pr, request) : \
> +    opus_multistream_decoder_ctl((st)->ms, request))
> +#else
> +# define libopus_generic_decoder_ctl(st, request) \
> +    opus_multistream_decoder_ctl((st)->ms, request)
> +#endif
> +
>  struct libopus_context {
>      AVClass *class;
> -    OpusMSDecoder *dec;
> +    OpusGenericDecoder dec;
>      int pre_skip;
>  #ifndef OPUS_SET_GAIN
>      union { int i; double d; } gain;
> @@ -46,12 +133,17 @@ struct libopus_context {
>  };
>  
>  #define OPUS_HEAD_SIZE 19
> +#ifdef OPUS_HAVE_OPUS_PROJECTION_H
> +# define OPUS_MAX_CHANNELS 18
> +#else
> +# define OPUS_MAX_CHANNELS 8
> +#endif
>  
>  static av_cold int libopus_decode_init(AVCodecContext *avc)
>  {
>      struct libopus_context *opus = avc->priv_data;
>      int ret, channel_map = 0, gain_db = 0, nb_streams, nb_coupled;
> -    uint8_t mapping_arr[8] = { 0, 1 }, *mapping;
> +    uint8_t mapping_arr[OPUS_MAX_CHANNELS] = { 0, 1 }, *mapping, *dmatrix = NULL;
>  
>      avc->channels = avc->extradata_size >= 10 ? avc->extradata[9] : (avc->channels == 1) ? 1 : 2;
>      if (avc->channels <= 0) {

> @@ -74,7 +166,21 @@ static av_cold int libopus_decode_init(AVCodecContext *avc)
>          nb_coupled = avc->extradata[OPUS_HEAD_SIZE + 1];
>          if (nb_streams + nb_coupled != avc->channels)
>              av_log(avc, AV_LOG_WARNING, "Inconsistent channel mapping.\n");
> -        mapping = avc->extradata + OPUS_HEAD_SIZE + 2;
> +        if (channel_map == 3) {
> +            int ch;
> +            if (avc->extradata_size >= OPUS_HEAD_SIZE + 2 + 2 * avc->channels * (nb_streams + nb_coupled)) {
> +                dmatrix =avc->extradata + OPUS_HEAD_SIZE + 2;
> +            } else {
> +                av_log(avc, AV_LOG_ERROR,
> +                    "Demixing matrix not present.\n");
> +                return AVERROR(EINVAL);

AVERROR_INVALIDDATA, extradata is part of the input bitstream and thats
the error type we generally use for it
unless iam missing something here

[...]
> diff --git a/libavcodec/libopusenc.c b/libavcodec/libopusenc.c
> index 4ae81b0bb2..729d86d2d7 100644
> --- a/libavcodec/libopusenc.c
> +++ b/libavcodec/libopusenc.c
> @@ -21,15 +21,139 @@
>  
>  #include <opus.h>
>  #include <opus_multistream.h>
> +#ifdef OPUS_HAVE_OPUS_PROJECTION_H
> +#include <opus_projection.h>
> +#endif
>  
>  #include "libavutil/opt.h"
>  #include "avcodec.h"
>  #include "bytestream.h"
>  #include "internal.h"
>  #include "libopus.h"
> +#include "mathops.h"
>  #include "vorbis.h"
>  #include "audio_frame_queue.h"
>  
> +typedef struct OpusGenericEncoder {
> +#ifdef OPUS_HAVE_OPUS_PROJECTION_H
> +    OpusProjectionEncoder *pr;
> +#endif
> +    OpusMSEncoder *ms;
> +} OpusGenericEncoder;
> +
> +static int libopus_generic_encoder_surround_init(OpusGenericEncoder *st, int Fs,
> +                                                 int channels,
> +                                                 int mapping_family,
> +                                                 int *nb_streams,
> +                                                 int *nb_coupled,
> +                                                 unsigned char *stream_map,
> +                                                 int application)
> +{
> +    int ret;
> +    if (mapping_family == 3) {
> +#ifdef OPUS_HAVE_OPUS_PROJECTION_H
> +        int ci;
> +        st->pr = opus_projection_ambisonics_encoder_create(
> +            Fs, channels, mapping_family, nb_streams, nb_coupled,
> +            application, &ret);

> +        for (ci = 0; ci < channels; ci++) stream_map[ci] = ci;

this is easier to read with a \n


> +#else
> +        ret = OPUS_UNIMPLEMENTED;
> +#endif
> +        st->ms = NULL;
> +        return ret;

the NULL init may be unneeded. The struct is memset 0 at the begin
so unless there is a patch that could set this to something else
it can be assumed to be still NULL

I would also suggest to use a differnt context name from
"st", as that is very commonly used for AVStream so it could
confuse people

[...]
> +
>  typedef struct LibopusEncOpts {
>      int vbr;
>      int application;
> @@ -46,7 +170,7 @@ typedef struct LibopusEncOpts {
>  
>  typedef struct LibopusEncContext {
>      AVClass *class;
> -    OpusMSEncoder *enc;
> +    OpusGenericEncoder enc;
>      int stream_count;
>      uint8_t *samples;
>      LibopusEncOpts opts;
> @@ -85,28 +209,68 @@ static const uint8_t libavcodec_libopus_channel_map[8][8] = {
>  static void libopus_write_header(AVCodecContext *avctx, int stream_count,
>                                   int coupled_stream_count,
>                                   int mapping_family,
> -                                 const uint8_t *channel_mapping)
> +                                 const uint8_t *channel_mapping,
> +                                 OpusGenericEncoder *enc)
>  {
>      uint8_t *p   = avctx->extradata;
>      int channels = avctx->channels;
> +    int gain = 0;
>  
>      bytestream_put_buffer(&p, "OpusHead", 8);
>      bytestream_put_byte(&p, 1); /* Version */
>      bytestream_put_byte(&p, channels);
>      bytestream_put_le16(&p, avctx->initial_padding); /* Lookahead samples at 48kHz */
>      bytestream_put_le32(&p, avctx->sample_rate); /* Original sample rate */
> -    bytestream_put_le16(&p, 0); /* Gain of 0dB is recommended. */
> +    if (mapping_family == 3) {
> +#ifdef OPUS_HAVE_OPUS_PROJECTION_H
> +        int ret;
> +        ret = libopus_generic_encoder_ctl(enc,
> +                                          OPUS_PROJECTION_GET_DEMIXING_MATRIX_GAIN(&gain));
> +        if (ret != OPUS_OK) {
> +            av_log(avctx, AV_LOG_ERROR,
> +                   "Unable to write header, demixing matrix gain not found.\n");
> +            return;
> +        }
> +#endif
> +    }
> +    bytestream_put_le16(&p, gain); /* Gain of 0dB is recommended. */
>  
>      /* Channel mapping */
>      bytestream_put_byte(&p, mapping_family);
> -    if (mapping_family != 0) {
> +    if (mapping_family == 3) {
> +        int ret;
> +        int32_t size;
> +        size = 2 * channels * (stream_count + coupled_stream_count);
> +        bytestream_put_byte(&p, stream_count);
> +        bytestream_put_byte(&p, coupled_stream_count);
> +        bytestream_put_byte(&p, stream_count);
> +#ifdef OPUS_HAVE_OPUS_PROJECTION_H
> +        ret = libopus_generic_encoder_ctl(enc,
> +                                          OPUS_PROJECTION_GET_DEMIXING_MATRIX_SIZE(&size));
> +        if (ret != OPUS_OK) {
> +            av_log(avctx, AV_LOG_ERROR,
> +                   "Unable to write header, demixing matrix size not found.\n");
> +            return;
> +        }
> +        ret = libopus_generic_encoder_ctl(enc,
> +                                          OPUS_PROJECTION_GET_DEMIXING_MATRIX(p, size));
> +        if (ret != OPUS_OK) {
> +            av_log(avctx, AV_LOG_ERROR,
> +                   "Unable to write header, demixing matrix not found.\n");
> +            return;
> +        }
> +        (*(&p)) += size;

I think with the increasing complexity of writing extradata
it would make sense to before or after this patch switch to 
bytestream2_put* or something else that does check writes
against the output buffer bounds.
(this change should be in a seperate patch though)

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180411/1c39730e/attachment.sig>


More information about the ffmpeg-devel mailing list