[Spice-devel] [spice-common opus support 2/5 (take 2)] Add support for the Opus codec.

Christophe Fergeau cfergeau at redhat.com
Tue Nov 5 08:30:42 PST 2013


On Thu, Oct 31, 2013 at 12:13:36PM -0500, Jeremy White wrote:
> Signed-off-by: Jeremy White <jwhite at codeweavers.com>
> ---
>  common/Makefile.am |    2 +
>  common/snd_codec.c |  130 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  common/snd_codec.h |   17 +++++--
>  configure.ac       |    9 ++++
>  spice.proto        |    1 +
>  spice1.proto       |    1 +
>  6 files changed, 151 insertions(+), 9 deletions(-)
> 
> diff --git a/common/Makefile.am b/common/Makefile.am
> index c79f596..73703fc 100644
> --- a/common/Makefile.am
> +++ b/common/Makefile.am
> @@ -84,6 +84,7 @@ AM_CPPFLAGS =				\
>  	$(GL_CFLAGS)			\
>  	$(PIXMAN_CFLAGS)		\
>  	$(CELT051_CFLAGS)		\
> +	$(OPUS_CFLAGS)		        \
>  	$(PROTOCOL_CFLAGS)		\
>  	$(SMARTCARD_CFLAGS)		\
>  	$(VISIBILITY_HIDDEN_CFLAGS)	\
> @@ -92,6 +93,7 @@ AM_CPPFLAGS =				\
>  	$(NULL)
>  
>  libspice_common_la_LIBADD =				\
> +	$(OPUS_LIBS)		                        \
>  	$(CELT051_LIBS)
>  
>  MARSHALLERS_DEPS =					\
> diff --git a/common/snd_codec.c b/common/snd_codec.c
> index 2683143..0e0f24b 100644
> --- a/common/snd_codec.c
> +++ b/common/snd_codec.c
> @@ -48,6 +48,11 @@ typedef struct
>      CELTEncoder *celt_encoder;
>      CELTDecoder *celt_decoder;
>  #endif
> +
> +#if HAVE_OPUS
> +    OpusEncoder *opus_encoder;
> +    OpusDecoder *opus_decoder;
> +#endif
>  } SndCodecInternal;
>  
>  
> @@ -80,7 +85,7 @@ static int snd_codec_create_celt051(SndCodecInternal *codec, int encode, int dec
>      int celt_error;
>  
>      codec->celt_mode = celt051_mode_create(codec->frequency,
> -                                           SND_CODEC_CELT_PLAYBACK_CHAN,
> +                                           SND_CODEC_PLAYBACK_CHAN,
>                                             SND_CODEC_CELT_FRAME_SIZE, &celt_error);
>      if (! codec->celt_mode)
>      {
> @@ -119,7 +124,7 @@ error:
>  static int snd_codec_encode_celt051(SndCodecInternal *codec, uint8_t *in_ptr, int in_size, uint8_t *out_ptr, int *out_size)
>  {
>      int n;
> -    if (in_size != SND_CODEC_CELT_FRAME_SIZE * SND_CODEC_CELT_PLAYBACK_CHAN * 2)
> +    if (in_size != SND_CODEC_CELT_FRAME_SIZE * SND_CODEC_PLAYBACK_CHAN * 2)
>          return SND_CODEC_INVALID_ENCODE_SIZE;
>      n = celt051_encode(codec->celt_encoder, (celt_int16_t *) in_ptr, NULL, out_ptr, *out_size);
>      if (n < 0) {
> @@ -138,12 +143,93 @@ static int snd_codec_decode_celt051(SndCodecInternal *codec, uint8_t *in_ptr, in
>          spice_printerr("celt051_decode failed %d\n", n);
>          return SND_CODEC_DECODE_FAILED;
>      }
> -    *out_size = SND_CODEC_CELT_FRAME_SIZE * SND_CODEC_CELT_PLAYBACK_CHAN * 2 /* 16 fmt */;
> +    *out_size = SND_CODEC_CELT_FRAME_SIZE * SND_CODEC_PLAYBACK_CHAN * 2 /* 16 fmt */;
>      return SND_CODEC_OK;
>  }
>  #endif
>  
>  
> +/* Opus support routines */
> +#if HAVE_OPUS
> +static void snd_codec_destroy_opus(SndCodecInternal *codec)
> +{
> +    if (codec->opus_decoder)
> +    {
> +        opus_decoder_destroy(codec->opus_decoder);
> +        codec->opus_decoder = NULL;
> +    }
> +
> +    if (codec->opus_encoder)
> +    {
> +        opus_encoder_destroy(codec->opus_encoder);
> +        codec->opus_encoder = NULL;
> +    }
> +
> +}
> +
> +static int snd_codec_create_opus(SndCodecInternal *codec, int encode, int decode)
> +{
> +    int opus_error;
> +
> +    if (encode)
> +    {
> +        codec->opus_encoder = opus_encoder_create(codec->frequency,
> +                                SND_CODEC_OPUS_PLAYBACK_CHAN,
> +                                OPUS_APPLICATION_AUDIO, &opus_error);
> +        if (! codec->opus_encoder)
> +        {
> +            spice_printerr("create opus encoder failed; error %d", opus_error);
> +            goto error;
> +        }
> +    }
> +
> +    if (decode)
> +    {
> +        codec->opus_decoder = opus_decoder_create(codec->frequency,
> +                                SND_CODEC_OPUS_PLAYBACK_CHAN, &opus_error);
> +        if (! codec->opus_decoder)
> +        {
> +            spice_printerr("create opus decoder failed; error %d", opus_error);
> +            goto error;
> +        }
> +    }
> +
> +    codec->mode = SPICE_AUDIO_DATA_MODE_OPUS;
> +    return SND_CODEC_OK;
> +
> +error:
> +    snd_codec_destroy_opus(codec);
> +    return SND_CODEC_UNAVAILABLE;
> +}
> +
> +static int snd_codec_encode_opus(SndCodecInternal *codec, uint8_t *in_ptr, int in_size, uint8_t *out_ptr, int *out_size)
> +{
> +    int n;
> +    if (in_size != SND_CODEC_OPUS_FRAME_SIZE * SND_CODEC_PLAYBACK_CHAN * 2)
> +        return SND_CODEC_INVALID_ENCODE_SIZE;
> +    n = opus_encode(codec->opus_encoder, (opus_int16 *) in_ptr, SND_CODEC_OPUS_FRAME_SIZE, out_ptr, *out_size);
> +    if (n < 0) {
> +        spice_printerr("opus_encode failed %d\n", n);
> +        return SND_CODEC_ENCODE_FAILED;
> +    }
> +    *out_size = n;
> +    return SND_CODEC_OK;
> +}
> +
> +static int snd_codec_decode_opus(SndCodecInternal *codec, uint8_t *in_ptr, int in_size, uint8_t *out_ptr, int *out_size)
> +{
> +    int n;
> +    n = opus_decode(codec->opus_decoder, in_ptr, in_size, (opus_int16 *) out_ptr,
> +                *out_size / SND_CODEC_OPUS_PLAYBACK_CHAN / 2, 0);
> +    if (n < 0) {
> +        spice_printerr("opus_decode failed %d\n", n);
> +        return SND_CODEC_DECODE_FAILED;
> +    }
> +    *out_size = n * SND_CODEC_OPUS_PLAYBACK_CHAN * 2 /* 16 fmt */;
> +    return SND_CODEC_OK;
> +}
> +#endif
> +
>  
>  /*----------------------------------------------------------------------------
>  **          PUBLIC INTERFACE
> @@ -160,9 +246,14 @@ int snd_codec_is_capable(int mode)
>  #if HAVE_CELT051
>      if (mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1)
>          return TRUE;
> -#else
> -    return FALSE;
>  #endif
> +
> +#if HAVE_OPUS
> +    if (mode == SPICE_AUDIO_DATA_MODE_OPUS)
> +        return TRUE;
> +#endif
> +
> +    return FALSE;
>  }
>  
>  /*
> @@ -191,6 +282,11 @@ int snd_codec_create(SndCodec *codec, int mode, int frequency, int encode, int d
>          rc = snd_codec_create_celt051(*c, encode, decode);
>  #endif
>  
> +#if HAVE_OPUS
> +    if (mode == SPICE_AUDIO_DATA_MODE_OPUS)
> +        rc = snd_codec_create_opus(*c, encode, decode);
> +#endif
> +
>      return rc;
>  }
>  
> @@ -208,6 +304,10 @@ void snd_codec_destroy(SndCodec *codec)
>      snd_codec_destroy_celt051(*c);
>  #endif
>  
> +#if HAVE_OPUS
> +    snd_codec_destroy_opus(*c);
> +#endif
> +
>      free(*c);
>      *c = NULL;
>  }
> @@ -225,6 +325,10 @@ int snd_codec_frame_size(SndCodec codec)
>      if (c && c->mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1)
>          return SND_CODEC_CELT_FRAME_SIZE;
>  #endif
> +#if HAVE_OPUS
> +    if (c && c->mode == SPICE_AUDIO_DATA_MODE_OPUS)
> +        return SND_CODEC_OPUS_FRAME_SIZE;
> +#endif
>      return SND_CODEC_MAX_FRAME_SIZE;
>  }
>  
> @@ -251,7 +355,18 @@ int snd_codec_encode(SndCodec codec, uint8_t *in_ptr, int in_size, uint8_t *out_
>      SndCodecInternal *c = (SndCodecInternal *) codec;
>  #if HAVE_CELT051
>      if (c && c->mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1)
> +    {
> +        /* The output buffer size in celt determines the compression,
> +            and so is essentially mandatory to use a certain value (47) */
> +        if (*out_size > SND_CODEC_CELT_COMPRESSED_FRAME_BYTES)
> +            *out_size = SND_CODEC_CELT_COMPRESSED_FRAME_BYTES;

I would have put this in the previous series I think. Shouldn't it be made
unconditionnally?

>          return snd_codec_encode_celt051(c, in_ptr, in_size, out_ptr, out_size);
> +    }
> +#endif
> +
> +#if HAVE_OPUS
> +    if (c && c->mode == SPICE_AUDIO_DATA_MODE_OPUS)
> +        return snd_codec_encode_opus(c, in_ptr, in_size, out_ptr, out_size);
>  #endif
>  
>      return SND_CODEC_ENCODER_UNAVAILABLE;
> @@ -281,5 +396,10 @@ int snd_codec_decode(SndCodec codec, uint8_t *in_ptr, int in_size, uint8_t *out_
>          return snd_codec_decode_celt051(c, in_ptr, in_size, out_ptr, out_size);
>  #endif
>  
> +#if HAVE_OPUS
> +    if (c && c->mode == SPICE_AUDIO_DATA_MODE_OPUS)
> +        return snd_codec_decode_opus(c, in_ptr, in_size, out_ptr, out_size);
> +#endif
> +
>      return SND_CODEC_DECODER_UNAVAILABLE;
>  }
> diff --git a/common/snd_codec.h b/common/snd_codec.h
> index 069356f..60d7865 100644
> --- a/common/snd_codec.h
> +++ b/common/snd_codec.h
> @@ -24,6 +24,10 @@
>  #include <celt051/celt.h>
>  #endif
>  
> +#if HAVE_OPUS
> +#include  <opus.h>
> +#endif
> +
>  /* Spice uses a very fixed protocol when transmitting CELT audio;
>     audio must be transmitted in frames of 256, and we must compress
>     data down to a fairly specific size (47, computation below).
> @@ -33,14 +37,19 @@
>  #define SND_CODEC_CELT_FRAME_SIZE       256
>  #define SND_CODEC_CELT_BIT_RATE         (64 * 1024)
>  #define SND_CODEC_CELT_PLAYBACK_FREQ    44100
> -#define SND_CODEC_CELT_PLAYBACK_CHAN    2
>  #define SND_CODEC_CELT_COMPRESSED_FRAME_BYTES (SND_CODEC_CELT_FRAME_SIZE * SND_CODEC_CELT_BIT_RATE / \
>                                          SND_CODEC_CELT_PLAYBACK_FREQ / 8)
>  
> +#define SND_CODEC_OPUS_FRAME_SIZE       480
> +#define SND_CODEC_OPUS_PLAYBACK_FREQ    48000
> +#define SND_CODEC_OPUS_PLAYBACK_CHAN    2

Why add this and rename  SND_CODEC_CELT_PLAYBACK_CHAN to something generic
at the same time?

> +#define SND_CODEC_OPUS_COMPRESSED_FRAME_BYTES 480
> +
> +#define SND_CODEC_PLAYBACK_CHAN         2
>  
> -#define SND_CODEC_MAX_FRAME_SIZE        SND_CODEC_CELT_FRAME_SIZE
> -#define SND_CODEC_MAX_FRAME_BYTES       (SND_CODEC_MAX_FRAME_SIZE * SND_CODEC_CELT_PLAYBACK_CHAN * 2 /* FMT_S16 */)
> -#define SND_CODEC_MAX_COMPRESSED_BYTES  SND_CODEC_CELT_COMPRESSED_FRAME_BYTES
> +#define SND_CODEC_MAX_FRAME_SIZE        (MAX(SND_CODEC_CELT_FRAME_SIZE, SND_CODEC_OPUS_FRAME_SIZE))
> +#define SND_CODEC_MAX_FRAME_BYTES       (SND_CODEC_MAX_FRAME_SIZE * SND_CODEC_PLAYBACK_CHAN * 2 /* FMT_S16 */)
> +#define SND_CODEC_MAX_COMPRESSED_BYTES  MAX(SND_CODEC_CELT_COMPRESSED_FRAME_BYTES, SND_CODEC_OPUS_COMPRESSED_FRAME_BYTES)
>  
>  #define SND_CODEC_OK                    0
>  #define SND_CODEC_UNAVAILABLE           1
> diff --git a/configure.ac b/configure.ac
> index 3443334..bb469c6 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -67,6 +67,15 @@ fi
>  AM_CONDITIONAL([HAVE_CELT051], [test "x$have_celt051" = "xyes"])
>  AM_COND_IF([HAVE_CELT051], AC_DEFINE([HAVE_CELT051], 1, [Define if we have celt051 codec]))
>  
> +PKG_CHECK_MODULES([OPUS], [opus >= 0.9.14], have_opus=yes, have_opus=no)

I'd require 1.0.0

> +
> +AM_CONDITIONAL([HAVE_OPUS], [test "x$have_opus" = "xyes"])
> +if test "x$have_opus" = "xyes" ; then
> +  AC_DEFINE([HAVE_OPUS], [1], [Define if we have OPUS])
> +  SPICE_REQUIRES+=" opus >= 0.9.14"
> +  opus_version=`pkg-config --modversion opus`

This is not used, is it?

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20131105/1c353810/attachment.pgp>


More information about the Spice-devel mailing list