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

Christophe Fergeau cfergeau at redhat.com
Fri Nov 8 07:59:37 PST 2013


On Thu, Oct 31, 2013 at 12:13:44PM -0500, Jeremy White wrote:
> Signed-off-by: Jeremy White <jwhite at codeweavers.com>
> ---
>  client/audio_devices.h      |    8 ---
>  client/platform.h           |    6 ++-
>  client/playback_channel.cpp |   11 +++-
>  client/record_channel.cpp   |   24 +++++----
>  client/x11/platform.cpp     |   10 ++--
>  client/x11/playback.cpp     |   13 ++---
>  client/x11/playback.h       |    5 +-
>  client/x11/record.cpp       |   15 +++---
>  client/x11/record.h         |    7 ++-
>  server/snd_worker.c         |  117 ++++++++++++++++++++++++++++++++++---------
>  server/spice-server.syms    |    9 ++++
>  server/spice.h              |   15 ++++--
>  12 files changed, 170 insertions(+), 70 deletions(-)
> 
> diff --git a/client/audio_devices.h b/client/audio_devices.h
> index a1da1f7..111c366 100644
> --- a/client/audio_devices.h
> +++ b/client/audio_devices.h
> @@ -27,10 +27,6 @@ public:
>      virtual bool abort() = 0;
>      virtual void stop() = 0;
>      virtual uint32_t get_delay_ms() = 0;
> -
> -    enum {
> -        FRAME_SIZE = 256,
> -    };
>  };
>  
>  class WaveRecordAbstract {
> @@ -42,10 +38,6 @@ public:
>      virtual void start() = 0;
>      virtual void stop() = 0;
>      virtual bool abort() = 0;
> -
> -    enum {
> -        FRAME_SIZE = 256,
> -    };
>  };
>  
>  #endif
> diff --git a/client/platform.h b/client/platform.h
> index 913bcde..0a88f06 100644
> --- a/client/platform.h
> +++ b/client/platform.h
> @@ -68,10 +68,12 @@ public:
>      static WaveRecordAbstract* create_recorder(RecordClient& client,
>                                                 uint32_t sampels_per_sec,

I'd tend to fix the 'sampel' typo while changing these functions (it shows
up in several places in this patch).

>                                                 uint32_t bits_per_sample,
> -                                               uint32_t channels);
> +                                               uint32_t channels,
> +                                               uint32_t frame_size);
>      static WavePlaybackAbstract* create_player(uint32_t sampels_per_sec,
>                                                 uint32_t bits_per_sample,
> -                                               uint32_t channels);
> +                                               uint32_t channels,
> +                                               uint32_t frame_size);
>  
>      enum {
>          SCROLL_LOCK_MODIFIER_SHIFT,
> diff --git a/client/playback_channel.cpp b/client/playback_channel.cpp
> index 26a7c48..a13f508 100644
> --- a/client/playback_channel.cpp
> +++ b/client/playback_channel.cpp
> @@ -170,6 +170,8 @@ PlaybackChannel::PlaybackChannel(RedClient& client, uint32_t id)
>  
>      if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1))
>          set_capability(SPICE_PLAYBACK_CAP_CELT_0_5_1);
> +    if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS))
> +        set_capability(SPICE_PLAYBACK_CAP_OPUS);
>  }
>  
>  void PlaybackChannel::clear()
> @@ -266,9 +268,16 @@ void PlaybackChannel::handle_start(RedPeer::InMessage* message)
>          }
>          int bits_per_sample = 16;
>          int frame_size = SND_CODEC_MAX_FRAME_SIZE;
> +
> +        if (_mode != SPICE_AUDIO_DATA_MODE_RAW) {
> +            if (snd_codec_create(&_codec, _mode, start->frequency, FALSE, TRUE) != SND_CODEC_OK)
> +                THROW("create decoder");
> +            frame_size = snd_codec_frame_size(_codec);
> +        }
> +

With this hunk, we end up with 2 snd_codec_create calls in handle_start()

>          try {
>              _wave_player = Platform::create_player(start->frequency, bits_per_sample,
> -                                                   start->channels);
> +                                                   start->channels, frame_size);
>          } catch (...) {
>              LOG_WARN("create player failed");
>              //todo: support disconnecting single channel
> diff --git a/client/record_channel.cpp b/client/record_channel.cpp
> index 7b5bc3c..258801b 100644
> --- a/client/record_channel.cpp
> +++ b/client/record_channel.cpp
> @@ -89,6 +89,8 @@ RecordChannel::RecordChannel(RedClient& client, uint32_t id)
>  
>      if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1))
>          set_capability(SPICE_RECORD_CAP_CELT_0_5_1);
> +    if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS))
> +        set_capability(SPICE_RECORD_CAP_OPUS);
>  }
>  
>  RecordChannel::~RecordChannel(void)
> @@ -112,7 +114,10 @@ void RecordChannel::on_connect()
>      Message* message = new Message(SPICE_MSGC_RECORD_MODE);
>      SpiceMsgcRecordMode mode;
>      mode.time = get_mm_time();
> -    if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1) &&
> +    if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS) &&
> +      test_capability(SPICE_RECORD_CAP_OPUS))
> +          _mode = SPICE_AUDIO_DATA_MODE_OPUS;

I think we also need to check if the frequency is acceptable for Opus as
the server-side code will silently fallback to celt/raw if the frequency is
not good (I don't think there's a server -> client message for the
recording channel to set the compression to use).
This case can easily be tested with new spice-server/new client, but qemu
binary built against spice 0.12.4

> +    else if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1) &&
>        test_capability(SPICE_RECORD_CAP_CELT_0_5_1))
>            _mode = SPICE_AUDIO_DATA_MODE_CELT_0_5_1;
>        else
> @@ -153,22 +158,24 @@ void RecordChannel::handle_start(RedPeer::InMessage* message)
>          THROW("unexpected number of channels");
>      }
>  
> +    int frame_size = SND_CODEC_MAX_FRAME_SIZE;
> +    if (_mode != SPICE_AUDIO_DATA_MODE_RAW) {
> +        if (snd_codec_create(&_codec, _mode, start->frequency, TRUE, FALSE) != SND_CODEC_OK)
> +            THROW("create encoder failed");
> +        frame_size = snd_codec_frame_size(_codec);
> +    }
> +
>      int bits_per_sample = 16;
>      try {
>          _wave_recorder = Platform::create_recorder(*this, start->frequency,
>                                                     bits_per_sample,
> -                                                   start->channels);
> +                                                   start->channels,
> +                                                   frame_size);
>      } catch (...) {
>          LOG_WARN("create recorder failed");
>          return;
>      }
>  
> -    int frame_size = SND_CODEC_MAX_FRAME_SIZE;
> -    if (_mode != SPICE_AUDIO_DATA_MODE_RAW) {
> -        if (snd_codec_create(&_codec, _mode, start->frequency, TRUE, FALSE) != SND_CODEC_OK)
> -            THROW("create encoder failed");
> -        frame_size = snd_codec_frame_size(_codec);
> -    }
>      _frame_bytes = frame_size * bits_per_sample * start->channels / 8;
>  
>      send_start_mark();
> @@ -237,7 +244,6 @@ void RecordChannel::remove_event_source(EventSources::Trigger& event_source)
>  void RecordChannel::push_frame(uint8_t *frame)
>  {
>      RecordSamplesMessage *message;
> -    ASSERT(_frame_bytes == FRAME_SIZE * 4);
>      if (!(message = get_message())) {
>          DBG(0, "blocked");
>          return;
> diff --git a/client/x11/platform.cpp b/client/x11/platform.cpp
> index b8563b3..bb8704c 100644
> --- a/client/x11/platform.cpp
> +++ b/client/x11/platform.cpp
> @@ -3433,16 +3433,18 @@ void Platform::reset_cursor_pos()
>  WaveRecordAbstract* Platform::create_recorder(RecordClient& client,
>                                                uint32_t sampels_per_sec,
>                                                uint32_t bits_per_sample,
> -                                              uint32_t channels)
> +                                              uint32_t channels,
> +                                              uint32_t frame_size)
>  {
> -    return new WaveRecorder(client, sampels_per_sec, bits_per_sample, channels);
> +    return new WaveRecorder(client, sampels_per_sec, bits_per_sample, channels, frame_size);
>  }
>  
>  WavePlaybackAbstract* Platform::create_player(uint32_t sampels_per_sec,
>                                                uint32_t bits_per_sample,
> -                                              uint32_t channels)
> +                                              uint32_t channels,
> +                                              uint32_t frame_size)
>  {
> -    return new WavePlayer(sampels_per_sec, bits_per_sample, channels);
> +    return new WavePlayer(sampels_per_sec, bits_per_sample, channels, frame_size);
>  }
>  
>  void XPlatform::on_focus_in()
> diff --git a/client/x11/playback.cpp b/client/x11/playback.cpp
> index 5fa7e18..035d6de 100644
> --- a/client/x11/playback.cpp
> +++ b/client/x11/playback.cpp
> @@ -24,12 +24,12 @@
>  
>  #define REING_SIZE_MS 300
>  
> -WavePlayer::WavePlayer(uint32_t sampels_per_sec, uint32_t bits_per_sample, uint32_t channels)
> +WavePlayer::WavePlayer(uint32_t sampels_per_sec, uint32_t bits_per_sample, uint32_t channels, uint32_t frame_size)
>      : _pcm (NULL)
>      , _hw_params (NULL)
>      , _sw_params (NULL)
>  {
> -    if (!init(sampels_per_sec, bits_per_sample, channels)) {
> +    if (!init(sampels_per_sec, bits_per_sample, channels, frame_size)) {
>          cleanup();
>          THROW("failed");
>      }
> @@ -57,9 +57,9 @@ WavePlayer::~WavePlayer()
>  
>  bool WavePlayer::init(uint32_t sampels_per_sec,
>                        uint32_t bits_per_sample,
> -                      uint32_t channels)
> +                      uint32_t channels,
> +                      uint32_t frame_size)
>  {
> -    const int frame_size = WavePlaybackAbstract::FRAME_SIZE;
>      const char* pcm_device = "default";
>      snd_pcm_format_t format;
>      int err;
> @@ -75,6 +75,7 @@ bool WavePlayer::init(uint32_t sampels_per_sec,
>          return false;
>      }
>      _sampels_per_ms = sampels_per_sec / 1000;
> +    _frame_size = frame_size;
>  
>      if ((err = snd_pcm_open(&_pcm, pcm_device, SND_PCM_STREAM_PLAYBACK, SND_PCM_NONBLOCK)) < 0) {
>          LOG_ERROR("cannot open audio playback device %s %s", pcm_device, snd_strerror(err));
> @@ -183,14 +184,14 @@ bool WavePlayer::init(uint32_t sampels_per_sec,
>  
>  bool WavePlayer::write(uint8_t* frame)
>  {
> -    snd_pcm_sframes_t ret = snd_pcm_writei(_pcm, frame, WavePlaybackAbstract::FRAME_SIZE);
> +    snd_pcm_sframes_t ret = snd_pcm_writei(_pcm, frame, _frame_size);
>      if (ret < 0) {
>          if (ret == -EAGAIN) {
>              return false;
>          }
>          DBG(0, "err %s", snd_strerror(-ret));
>          if (snd_pcm_recover(_pcm, ret, 1) == 0) {
> -            snd_pcm_writei(_pcm, frame, WavePlaybackAbstract::FRAME_SIZE);
> +            snd_pcm_writei(_pcm, frame, _frame_size);
>          }
>      }
>      return true;
> diff --git a/client/x11/playback.h b/client/x11/playback.h
> index 27ef9ed..383f02d 100644
> --- a/client/x11/playback.h
> +++ b/client/x11/playback.h
> @@ -25,7 +25,7 @@
>  
>  class WavePlayer: public WavePlaybackAbstract {
>  public:
> -    WavePlayer(uint32_t sampels_per_sec, uint32_t bits_per_sample, uint32_t channels);
> +    WavePlayer(uint32_t sampels_per_sec, uint32_t bits_per_sample, uint32_t channels, uint32_t frame_size);
>      virtual ~WavePlayer();
>  
>      virtual bool write(uint8_t* frame);
> @@ -34,7 +34,7 @@ public:
>      virtual uint32_t get_delay_ms();
>  
>  private:
> -    bool init(uint32_t sampels_per_sec, uint32_t bits_per_sample, uint32_t channel);
> +    bool init(uint32_t sampels_per_sec, uint32_t bits_per_sample, uint32_t channel, uint32_t frame_size);
>      void cleanup();
>  
>  private:
> @@ -42,6 +42,7 @@ private:
>      snd_pcm_hw_params_t* _hw_params;
>      snd_pcm_sw_params_t* _sw_params;
>      uint32_t _sampels_per_ms;
> +    uint32_t _frame_size;
>  };
>  
>  #endif
> diff --git a/client/x11/record.cpp b/client/x11/record.cpp
> index 535a8c9..32c0af7 100644
> --- a/client/x11/record.cpp
> +++ b/client/x11/record.cpp
> @@ -48,18 +48,19 @@ void WaveRecorder::EventTrigger::on_event()
>  WaveRecorder::WaveRecorder(Platform::RecordClient& client,
>                             uint32_t sampels_per_sec,
>                             uint32_t bits_per_sample,
> -                           uint32_t channels)
> +                           uint32_t channels,
> +                           uint32_t frame_size)
>      : _client (client)
>      , _pcm (NULL)
>      , _hw_params (NULL)
>      , _sw_params (NULL)
>      , _sample_bytes (bits_per_sample * channels / 8)
> -    , _frame (new uint8_t[_sample_bytes * WaveRecordAbstract::FRAME_SIZE])
> +    , _frame (new uint8_t[_sample_bytes * frame_size])
>      , _frame_pos (_frame)
> -    , _frame_end (_frame + _sample_bytes * WaveRecordAbstract::FRAME_SIZE)
> +    , _frame_end (_frame + _sample_bytes * frame_size)
>      , _event_trigger (NULL)
>  {
> -    if (!init(sampels_per_sec, bits_per_sample, channels)) {
> +    if (!init(sampels_per_sec, bits_per_sample, channels, frame_size)) {
>          cleanup();
>          THROW("failed");
>      }
> @@ -93,13 +94,15 @@ void WaveRecorder::cleanup()
>  
>  bool WaveRecorder::init(uint32_t sampels_per_sec,
>                          uint32_t bits_per_sample,
> -                        uint32_t channels)
> +                        uint32_t channels,
> +                        uint32_t frame_size)
>  {
> -    const int frame_size = WaveRecordAbstract::FRAME_SIZE;
>      const char* pcm_device = "default";
>      snd_pcm_format_t format;
>      int err;
>  
> +    _frame_size = frame_size;
> +
>      switch (bits_per_sample) {
>      case 8:
>          format = SND_PCM_FORMAT_S8;
> diff --git a/client/x11/record.h b/client/x11/record.h
> index 9141096..9470791 100644
> --- a/client/x11/record.h
> +++ b/client/x11/record.h
> @@ -29,7 +29,8 @@ public:
>      WaveRecorder(Platform::RecordClient& client,
>                   uint32_t sampels_per_sec,
>                   uint32_t bits_per_sample,
> -                 uint32_t channels);
> +                 uint32_t channels,
> +                 uint32_t frame_size);
>      virtual ~WaveRecorder();
>  
>      virtual void start();
> @@ -39,7 +40,8 @@ public:
>  private:
>      bool init(uint32_t sampels_per_sec,
>                uint32_t bits_per_sample,
> -              uint32_t channels);
> +              uint32_t channels,
> +              uint32_t frame_size);
>      void cleanup();
>      void on_event();
>  
> @@ -49,6 +51,7 @@ private:
>      snd_pcm_hw_params_t* _hw_params;
>      snd_pcm_sw_params_t* _sw_params;
>      uint32_t _sample_bytes;
> +    uint32_t _frame_size;
>      uint8_t* _frame;
>      uint8_t* _frame_pos;
>      uint8_t* _frame_end;
> diff --git a/server/snd_worker.c b/server/snd_worker.c
> index 211dcf2..bdd998a 100644
> --- a/server/snd_worker.c
> +++ b/server/snd_worker.c
> @@ -156,12 +156,14 @@ struct SpicePlaybackState {
>      struct SndWorker worker;
>      SpicePlaybackInstance *sin;
>      SpiceVolumeState volume;
> +    uint32_t frequency;
>  };
>  
>  struct SpiceRecordState {
>      struct SndWorker worker;
>      SpiceRecordInstance *sin;
>      SpiceVolumeState volume;
> +    uint32_t frequency;
>  };
>  
>  typedef struct RecordChannel {
> @@ -370,14 +372,27 @@ static int snd_record_handle_message(SndChannel *channel, size_t size, uint32_t
>          return snd_record_handle_write((RecordChannel *)channel, size, message);
>      case SPICE_MSGC_RECORD_MODE: {
>          SpiceMsgcRecordMode *mode = (SpiceMsgcRecordMode *)message;
> -        record_channel->mode = mode->mode;
> +        SpiceRecordState *st = SPICE_CONTAINEROF(channel->worker, SpiceRecordState, worker);
>          record_channel->mode_time = mode->time;
> -        if (record_channel->mode != SPICE_AUDIO_DATA_MODE_RAW &&
> -             ! snd_codec_is_capable(record_channel->mode)) {
> -            spice_printerr("unsupported mode %d", record_channel->mode);
> +        if (mode->mode != SPICE_AUDIO_DATA_MODE_RAW)
> +        {
> +            if (snd_codec_is_capable(mode->mode)) {
> +                if (snd_codec_create(&record_channel->codec, mode->mode, st->frequency, FALSE, TRUE) == SND_CODEC_OK) {
> +                    record_channel->mode = mode->mode;
> +                } else {
> +                    spice_printerr("create decoder failed");
> +                }
> +            }
> +            else {
> +                spice_printerr("unsupported mode %d", record_channel->mode);
> +                return FALSE;
> +            }

Why did you move the recording codec creation from
on_new_record_channel() to here? You did not do that for the playback
channel, and more importantly, we lose the call to snd_desired_audio_mode()
which makes sure we don't try to use Opus when the frequency is 44100.

>          }
> +        else
> +            record_channel->mode = mode->mode;
>          break;
>      }
> +
>      case SPICE_MSGC_RECORD_START_MARK: {
>          SpiceMsgcRecordStartMark *mark = (SpiceMsgcRecordStartMark *)message;
>          record_channel->start_time = mark->time;
> @@ -615,6 +630,7 @@ static int snd_playback_send_latency(PlaybackChannel *playback_channel)
>  static int snd_playback_send_start(PlaybackChannel *playback_channel)
>  {
>      SndChannel *channel = (SndChannel *)playback_channel;
> +    SpicePlaybackState *st = SPICE_CONTAINEROF(channel->worker, SpicePlaybackState, worker);
>      SpiceMsgPlaybackStart start;
>  
>      if (!snd_reset_send_data(channel, SPICE_MSG_PLAYBACK_START)) {
> @@ -622,7 +638,7 @@ static int snd_playback_send_start(PlaybackChannel *playback_channel)
>      }
>  
>      start.channels = SPICE_INTERFACE_PLAYBACK_CHAN;
> -    start.frequency = SPICE_INTERFACE_PLAYBACK_FREQ;
> +    start.frequency = st->frequency;
>      spice_assert(SPICE_INTERFACE_PLAYBACK_FMT == SPICE_INTERFACE_AUDIO_FMT_S16);
>      start.format = SPICE_AUDIO_FMT_S16;
>      start.time = reds_get_mm_time();
> @@ -656,6 +672,7 @@ static int snd_playback_send_ctl(PlaybackChannel *playback_channel)
>  static int snd_record_send_start(RecordChannel *record_channel)
>  {
>      SndChannel *channel = (SndChannel *)record_channel;
> +    SpiceRecordState *st = SPICE_CONTAINEROF(channel->worker, SpiceRecordState, worker);
>      SpiceMsgRecordStart start;
>  
>      if (!snd_reset_send_data(channel, SPICE_MSG_RECORD_START)) {
> @@ -663,7 +680,7 @@ static int snd_record_send_start(RecordChannel *record_channel)
>      }
>  
>      start.channels = SPICE_INTERFACE_RECORD_CHAN;
> -    start.frequency = SPICE_INTERFACE_RECORD_FREQ;
> +    start.frequency = st->frequency;
>      spice_assert(SPICE_INTERFACE_RECORD_FMT == SPICE_INTERFACE_AUDIO_FMT_S16);
>      start.format = SPICE_AUDIO_FMT_S16;
>      spice_marshall_msg_record_start(channel->send_data.marshaller, &start);
> @@ -745,11 +762,13 @@ static int snd_playback_send_write(PlaybackChannel *playback_channel)
>  
>      if (playback_channel->mode == SPICE_AUDIO_DATA_MODE_RAW) {
>          spice_marshaller_add_ref(channel->send_data.marshaller,
> -                                 (uint8_t *)frame->samples, sizeof(frame->samples));
> +                                 (uint8_t *)frame->samples,
> +                                 snd_codec_frame_size(playback_channel->codec) * sizeof(frame->samples[0]));
>      }
>      else {
>          int n = sizeof(playback_channel->encode_buf);
> -        if (snd_codec_encode(playback_channel->codec, (uint8_t *) frame->samples, sizeof(frame->samples),
> +        if (snd_codec_encode(playback_channel->codec, (uint8_t *) frame->samples,
> +                                    snd_codec_frame_size(playback_channel->codec) * sizeof(frame->samples[0]),
>                                      playback_channel->encode_buf, &n) != SND_CODEC_OK) {
>              spice_printerr("encode failed");
>              snd_disconnect_channel(channel);
> @@ -1126,11 +1145,15 @@ void snd_set_playback_latency(RedClient *client, uint32_t latency)
>      }
>  }
>  
> -static int snd_desired_audio_mode(int client_can_celt)
> +static int snd_desired_audio_mode(int frequency, int client_can_celt, int client_can_opus)
>  {
>      if (! playback_compression)
>          return SPICE_AUDIO_DATA_MODE_RAW;
>  
> +    if (client_can_opus && snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS))
> +        if (frequency == 48000 || frequency == 24000 || frequency == 16000 || frequency == 12000 || frequency == 8000)
> +            return SPICE_AUDIO_DATA_MODE_OPUS;
> +
>      if (client_can_celt && snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1))
>          return SPICE_AUDIO_DATA_MODE_CELT_0_5_1;
>  
> @@ -1199,11 +1222,13 @@ static void snd_set_playback_peer(RedChannel *channel, RedClient *client, RedsSt
>  
>      int client_can_celt = red_channel_client_test_remote_cap(playback_channel->base.channel_client,
>                                            SPICE_PLAYBACK_CAP_CELT_0_5_1);
> -    int desired_mode = snd_desired_audio_mode(client_can_celt);
> +    int client_can_opus = red_channel_client_test_remote_cap(playback_channel->base.channel_client,
> +                                          SPICE_PLAYBACK_CAP_OPUS);
> +    int desired_mode = snd_desired_audio_mode(st->frequency, client_can_celt, client_can_opus);
>      playback_channel->mode = SPICE_AUDIO_DATA_MODE_RAW;
>      if (desired_mode != SPICE_AUDIO_DATA_MODE_RAW)
>      {
> -        if (snd_codec_create(&playback_channel->codec, desired_mode, SPICE_INTERFACE_PLAYBACK_FREQ, TRUE, FALSE) == SND_CODEC_OK) {
> +        if (snd_codec_create(&playback_channel->codec, desired_mode, st->frequency, TRUE, FALSE) == SND_CODEC_OK) {
>              playback_channel->mode = desired_mode;
>          } else {
>              spice_printerr("create encoder failed");
> @@ -1341,6 +1366,50 @@ SPICE_GNUC_VISIBLE uint32_t spice_server_record_get_samples(SpiceRecordInstance
>      return len;
>  }
>  
> +SPICE_GNUC_VISIBLE uint32_t spice_server_get_best_playback_rate(SpicePlaybackInstance *sin)
> +{
> +    int client_can_opus = TRUE;
> +    if (sin && sin->st->worker.connection)
> +    {
> +        SndChannel *channel = sin->st->worker.connection;
> +        PlaybackChannel *playback_channel = SPICE_CONTAINEROF(channel, PlaybackChannel, base);
> +        client_can_opus = red_channel_client_test_remote_cap(playback_channel->base.channel_client,
> +                                          SPICE_PLAYBACK_CAP_OPUS);
> +    }

Why are there some caps checks in here? It's called from qemu
line_out_init() which was called once at startup (when no client are
connected) in my testing.

> +
> +    if (client_can_opus && snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS))
> +        return SND_CODEC_OPUS_PLAYBACK_FREQ;
> +
> +    return SND_CODEC_CELT_PLAYBACK_FREQ;
> +}
> +
> +SPICE_GNUC_VISIBLE void spice_server_set_playback_rate(SpicePlaybackInstance *sin, uint32_t frequency)
> +{
> +    sin->st->frequency = frequency;
> +}
> +
> +SPICE_GNUC_VISIBLE uint32_t spice_server_get_best_record_rate(SpiceRecordInstance *sin)
> +{
> +    int client_can_opus = TRUE;
> +    if (sin && sin->st->worker.connection)
> +    {
> +        SndChannel *channel = sin->st->worker.connection;
> +        RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
> +        client_can_opus = red_channel_client_test_remote_cap(record_channel->base.channel_client,
> +                                          SPICE_RECORD_CAP_OPUS);
> +    }
> +
> +    if (client_can_opus && snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS))
> +        return SND_CODEC_OPUS_PLAYBACK_FREQ;
> +
> +    return SND_CODEC_CELT_PLAYBACK_FREQ;
> +}
> +
> +SPICE_GNUC_VISIBLE void spice_server_set_record_rate(SpiceRecordInstance *sin, uint32_t frequency)
> +{
> +    sin->st->frequency = frequency;
> +}

As we can't change playback/record rate separately, maybe the setter should
be spice_server_set_audio_rate()?

> +
>  static void on_new_record_channel(SndWorker *worker)
>  {
>      RecordChannel *record_channel = (RecordChannel *)worker->connection;
> @@ -1387,18 +1456,7 @@ static void snd_set_record_peer(RedChannel *channel, RedClient *client, RedsStre
>          return;
>      }
>  
> -    int client_can_celt = red_channel_client_test_remote_cap(record_channel->base.channel_client,
> -                                          SPICE_RECORD_CAP_CELT_0_5_1);
> -    int desired_mode = snd_desired_audio_mode(client_can_celt);
>      record_channel->mode = SPICE_AUDIO_DATA_MODE_RAW;
> -    if (desired_mode != SPICE_AUDIO_DATA_MODE_RAW)
> -    {
> -        if (snd_codec_create(&record_channel->codec, desired_mode, SPICE_INTERFACE_RECORD_FREQ, FALSE, TRUE) == SND_CODEC_OK) {
> -            record_channel->mode = desired_mode;
> -        } else {
> -            spice_printerr("create decoder failed");
> -        }
> -    }
>  
>      worker->connection = &record_channel->base;
>  
> @@ -1453,6 +1511,7 @@ void snd_attach_playback(SpicePlaybackInstance *sin)
>      sin->st = spice_new0(SpicePlaybackState, 1);
>      sin->st->sin = sin;
>      playback_worker = &sin->st->worker;
> +    sin->st->frequency = SND_CODEC_CELT_PLAYBACK_FREQ; /* Default to the legacy rate */
>  
>      // TODO: Make RedChannel base of worker? instead of assigning it to channel->data
>      channel = red_channel_create_dummy(sizeof(RedChannel), SPICE_CHANNEL_PLAYBACK, 0);
> @@ -1466,6 +1525,8 @@ void snd_attach_playback(SpicePlaybackInstance *sin)
>  
>      if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1))
>          red_channel_set_cap(channel, SPICE_PLAYBACK_CAP_CELT_0_5_1);
> +    if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS))
> +        red_channel_set_cap(channel, SPICE_PLAYBACK_CAP_OPUS);
>  
>      red_channel_set_cap(channel, SPICE_PLAYBACK_CAP_VOLUME);
>  
> @@ -1483,6 +1544,7 @@ void snd_attach_record(SpiceRecordInstance *sin)
>      sin->st = spice_new0(SpiceRecordState, 1);
>      sin->st->sin = sin;
>      record_worker = &sin->st->worker;
> +    sin->st->frequency = SND_CODEC_CELT_PLAYBACK_FREQ; /* Default to the legacy rate */
>  
>      // TODO: Make RedChannel base of worker? instead of assigning it to channel->data
>      channel = red_channel_create_dummy(sizeof(RedChannel), SPICE_CHANNEL_RECORD, 0);
> @@ -1495,6 +1557,8 @@ void snd_attach_record(SpiceRecordInstance *sin)
>      red_channel_set_data(channel, record_worker);
>      if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1))
>          red_channel_set_cap(channel, SPICE_RECORD_CAP_CELT_0_5_1);
> +    if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS))
> +        red_channel_set_cap(channel, SPICE_RECORD_CAP_OPUS);
>      red_channel_set_cap(channel, SPICE_RECORD_CAP_VOLUME);
>  
>      record_worker->base_channel = channel;
> @@ -1546,9 +1610,12 @@ void snd_set_playback_compression(int on)
>      for (; now; now = now->next) {
>          if (now->base_channel->type == SPICE_CHANNEL_PLAYBACK && now->connection) {
>              PlaybackChannel* playback = (PlaybackChannel*)now->connection;
> -            int desired_mode = snd_desired_audio_mode(
> -                    red_channel_client_test_remote_cap(now->connection->channel_client, SPICE_PLAYBACK_CAP_CELT_0_5_1)
> -                    );
> +            SpicePlaybackState *st = SPICE_CONTAINEROF(now, SpicePlaybackState, worker);
> +            int client_can_celt = red_channel_client_test_remote_cap(playback->base.channel_client,
> +                                    SPICE_PLAYBACK_CAP_CELT_0_5_1);
> +            int client_can_opus = red_channel_client_test_remote_cap(playback->base.channel_client,
> +                                    SPICE_PLAYBACK_CAP_OPUS);
> +            int desired_mode = snd_desired_audio_mode(st->frequency, client_can_opus, client_can_celt);
>              if (playback->mode != desired_mode) {
>                  playback->mode = desired_mode;
>                  snd_set_command(now->connection, SND_PLAYBACK_MODE_MASK);
> diff --git a/server/spice-server.syms b/server/spice-server.syms
> index 4f2dc37..0e8cf2e 100644
> --- a/server/spice-server.syms
> +++ b/server/spice-server.syms
> @@ -145,3 +145,12 @@ SPICE_SERVER_0.12.4 {
>  global:
>      spice_server_set_agent_file_xfer;
>  } SPICE_SERVER_0.12.3;
> +
> +SPICE_SERVER_0.12.5 {
> +global:
> +    spice_server_get_best_playback_rate;
> +    spice_server_set_playback_rate;
> +    spice_server_get_best_record_rate;
> +    spice_server_set_record_rate;
> +} SPICE_SERVER_0.12.4;
> +

This adds a blank line to the end of the file.

> diff --git a/server/spice.h b/server/spice.h
> index b645112..c648a1d 100644
> --- a/server/spice.h
> +++ b/server/spice.h
> @@ -24,7 +24,7 @@
>  #include <spice/vd_agent.h>
>  #include <spice/macros.h>
>  
> -#define SPICE_SERVER_VERSION 0x000c04 /* release 0.12.4 */
> +#define SPICE_SERVER_VERSION 0x000c05 /* release 0.12.5 */
>  
>  #ifdef SPICE_SERVER_INTERNAL
>  #undef SPICE_GNUC_DEPRECATED
> @@ -333,7 +333,7 @@ struct SpiceTabletInstance {
>  
>  #define SPICE_INTERFACE_PLAYBACK "playback"
>  #define SPICE_INTERFACE_PLAYBACK_MAJOR 1
> -#define SPICE_INTERFACE_PLAYBACK_MINOR 2
> +#define SPICE_INTERFACE_PLAYBACK_MINOR 3
>  typedef struct SpicePlaybackInterface SpicePlaybackInterface;
>  typedef struct SpicePlaybackInstance SpicePlaybackInstance;
>  typedef struct SpicePlaybackState SpicePlaybackState;
> @@ -342,7 +342,7 @@ enum {
>      SPICE_INTERFACE_AUDIO_FMT_S16 = 1,
>  };
>  
> -#define SPICE_INTERFACE_PLAYBACK_FREQ  44100
> +#define SPICE_INTERFACE_PLAYBACK_FREQ  48000
>  #define SPICE_INTERFACE_PLAYBACK_CHAN  2
>  #define SPICE_INTERFACE_PLAYBACK_FMT   SPICE_INTERFACE_AUDIO_FMT_S16
>  
> @@ -367,12 +367,12 @@ void spice_server_playback_set_mute(SpicePlaybackInstance *sin, uint8_t mute);
>  
>  #define SPICE_INTERFACE_RECORD "record"
>  #define SPICE_INTERFACE_RECORD_MAJOR 2
> -#define SPICE_INTERFACE_RECORD_MINOR 2
> +#define SPICE_INTERFACE_RECORD_MINOR 3
>  typedef struct SpiceRecordInterface SpiceRecordInterface;
>  typedef struct SpiceRecordInstance SpiceRecordInstance;
>  typedef struct SpiceRecordState SpiceRecordState;
>  
> -#define SPICE_INTERFACE_RECORD_FREQ  44100
> +#define SPICE_INTERFACE_RECORD_FREQ  48000
>  #define SPICE_INTERFACE_RECORD_CHAN  2
>  #define SPICE_INTERFACE_RECORD_FMT   SPICE_INTERFACE_AUDIO_FMT_S16
>  
> @@ -393,6 +393,11 @@ void spice_server_record_set_volume(SpiceRecordInstance *sin,
>                                      uint8_t nchannels, uint16_t *volume);
>  void spice_server_record_set_mute(SpiceRecordInstance *sin, uint8_t mute);
>  
> +uint32_t spice_server_get_best_playback_rate(SpicePlaybackInstance *sin);
> +void     spice_server_set_playback_rate(SpicePlaybackInstance *sin, uint32_t frequency);
> +uint32_t spice_server_get_best_record_rate(SpiceRecordInstance *sin);
> +void     spice_server_set_record_rate(SpiceRecordInstance *sin, uint32_t frequency);
> +
>  /* char device interfaces */
>  
>  #define SPICE_INTERFACE_CHAR_DEVICE "char_device"
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- 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/20131108/e8bd9c29/attachment-0001.pgp>


More information about the Spice-devel mailing list