wip/opus: single frequency (48000 if HAVE_OPUS, else 44100), still funky noises from server/test/test_playbackopus

13 files changed, 218 insertions, 157 deletions

diff --git a/client/audio_channels.h b/client/audio_channels.h
index 84e45cc2..25a8ffd7 100644
--- a/client/audio_channels.h
+++ b/client/audio_channels.h
@@ -51,6 +51,9 @@ private:
     void handle_stop(RedPeer::InMessage* message);
     void handle_raw_data(RedPeer::InMessage* message);
     void handle_celt_data(RedPeer::InMessage* message);
+#ifdef HAVE_OPUS
+    void handle_opus_data(RedPeer::InMessage* message);
+#endif
     void null_handler(RedPeer::InMessage* message);
     void disable();
 
@@ -101,6 +104,9 @@ private:
     void clear();
 
 private:
+    void create_celt_encoder(int frequency, int channels);
+    void create_opus_encoder(int frequency, int channels);
+
     WaveRecordAbstract* _wave_recorder;
     Mutex _messages_lock;
     std::list<RecordSamplesMessage *> _messages;
@@ -112,6 +118,7 @@ private:
 #endif
     uint32_t _frame_size;
     uint32_t _frame_bytes;
+    int frequency;
 
     static int data_mode;
 
diff --git a/client/audio_devices.h b/client/audio_devices.h
index 85478acf..190c64ec 100644
--- a/client/audio_devices.h
+++ b/client/audio_devices.h
@@ -18,6 +18,9 @@
 #ifndef _H_AUDIO_DEVICES
 #define _H_AUDIO_DEVICES
 
+// TODO - move to spice-protocol, or make part of the protocol?
+#define FRAME_SIZE 480
+
 class WavePlaybackAbstract {
 public:
     WavePlaybackAbstract() {}
@@ -27,10 +30,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 +41,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 a9a1715b..bf4e44e1 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,
                                                uint32_t bits_per_sample,
-                                               uint32_t channels);
+                                               uint32_t channels,
+                                               int frame_size);
     static WavePlaybackAbstract* create_player(uint32_t sampels_per_sec,
                                                uint32_t bits_per_sample,
-                                               uint32_t channels);
+                                               uint32_t channels,
+                                               int frame_size);
 
     enum {
         SCROLL_LOCK_MODIFIER_SHIFT,
diff --git a/client/playback_channel.cpp b/client/playback_channel.cpp
index f3e2b93e..9b520f7e 100644
--- a/client/playback_channel.cpp
+++ b/client/playback_channel.cpp
@@ -23,6 +23,10 @@
 #include "audio_channels.h"
 #include "audio_devices.h"
 
+#ifdef HAVE_OPUS
+//#undef HAVE_OPUS
+#endif
+
 //#define WAVE_CAPTURE
 #ifdef WAVE_CAPTURE
 
@@ -153,6 +157,9 @@ PlaybackChannel::PlaybackChannel(RedClient& client, uint32_t id)
     , _mode (SPICE_AUDIO_DATA_MODE_INVALID)
     , _celt_mode (NULL)
     , _celt_decoder (NULL)
+#ifdef HAVE_OPUS
+    , _opus_decoder (NULL)
+#endif
     , _playing (false)
 {
 #ifdef WAVE_CAPTURE
@@ -169,11 +176,9 @@ PlaybackChannel::PlaybackChannel(RedClient& client, uint32_t id)
 
     handler->set_handler(SPICE_MSG_PLAYBACK_MODE, &PlaybackChannel::handle_mode);
 
-#ifdef HAVE_CELT051
     set_capability(SPICE_PLAYBACK_CAP_CELT_0_5_1);
-#endif
-#ifdef HAVE_CELT
-    set_capability(SPICE_PLAYBACK_CAP_CELT);
+#ifdef HAVE_OPUS
+    set_capability(SPICE_PLAYBACK_CAP_OPUS);
 #endif
 }
 
@@ -227,6 +232,12 @@ void PlaybackChannel::set_data_handler()
         handler->set_handler(SPICE_MSG_PLAYBACK_DATA, &PlaybackChannel::handle_raw_data);
     } else if (_mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1) {
         handler->set_handler(SPICE_MSG_PLAYBACK_DATA, &PlaybackChannel::handle_celt_data);
+    } else if (_mode == SPICE_AUDIO_DATA_MODE_OPUS) {
+#ifdef HAVE_OPUS
+        handler->set_handler(SPICE_MSG_PLAYBACK_DATA, &PlaybackChannel::handle_opus_data);
+#else
+        THROW("opus not linked in, unhandled mode");
+#endif
     } else {
         THROW("invalid mode");
     }
@@ -234,13 +245,14 @@ void PlaybackChannel::set_data_handler()
 
 void PlaybackChannel::handle_mode(RedPeer::InMessage* message)
 {
-    SpiceMsgPlaybackMode* playbacke_mode = (SpiceMsgPlaybackMode*)message->data();
-    if (playbacke_mode->mode != SPICE_AUDIO_DATA_MODE_RAW &&
-        playbacke_mode->mode != SPICE_AUDIO_DATA_MODE_CELT_0_5_1) {
+    SpiceMsgPlaybackMode* playback_mode = (SpiceMsgPlaybackMode*)message->data();
+    if (playback_mode->mode != SPICE_AUDIO_DATA_MODE_RAW &&
+        playback_mode->mode != SPICE_AUDIO_DATA_MODE_CELT_0_5_1 &&
+        playback_mode->mode != SPICE_AUDIO_DATA_MODE_OPUS) {
         THROW("invalid mode");
     }
 
-    _mode = playbacke_mode->mode;
+    _mode = playback_mode->mode;
     if (_playing) {
         set_data_handler();
         return;
@@ -282,21 +294,11 @@ void PlaybackChannel::handle_start(RedPeer::InMessage* message)
             THROW("unexpected format");
         }
         int bits_per_sample = 16;
-        _frame_size = 256;
-        _frame_bytes = _frame_size * start->channels * bits_per_sample / 8;
-        try {
-            _wave_player = Platform::create_player(start->frequency, bits_per_sample,
-                                                   start->channels);
-        } catch (...) {
-            LOG_WARN("create player failed");
-            //todo: support disconnecting single channel
-            disable();
-            return;
-        }
 
         if (_mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1) {
-#ifdef HAVE_CELT051
             int celt_mode_err;
+            _frame_size = 256;
+            _frame_bytes = _frame_size * start->channels * bits_per_sample / 8;
             if (!(_celt_mode = celt051_mode_create(start->frequency, start->channels,
                                                    _frame_size, &celt_mode_err))) {
                 THROW("create celt mode failed %d", celt_mode_err);
@@ -305,25 +307,31 @@ void PlaybackChannel::handle_start(RedPeer::InMessage* message)
             if (!(_celt_decoder = celt051_decoder_create(_celt_mode))) {
                 THROW("create celt decoder");
             }
-#else
-            LOG_WARN("This version of celt is not linked!");
-#endif
         } else {
-#ifdef HAVE_CELT
-            int celt_err;
-            if (!(_celt_mode = celt_mode_create(start->frequency, _frame_size,
-                                                &celt_err))) {
-                THROW("create celt mode failed %s", celt_strerror(celt_err));
-            }
-
-            if (!(_celt_decoder = celt_decoder_create(_celt_mode, start->channels,
-                                                      &celt_err))) {
-                THROW("create celt decoder failed %s", celt_strerror(celt_err));
+#ifdef HAVE_OPUS
+            int opus_err;
+
+            _frame_size = 480;
+            _frame_bytes = _frame_size * start->channels * bits_per_sample / 8;
+            if (!(_opus_decoder = opus_decoder_create(start->frequency, start->channels,
+                                                      &opus_err))) {
+                THROW("create opus decoder failed, freq %d, channels %d, err %d",
+                         start->frequency, start->channels, opus_err);
             }
 #else
-            LOG_WARN("This version of celt is not linked!");
+            LOG_WARN("opus is not linked!");
 #endif
         }
+        try {
+            _wave_player = Platform::create_player(start->frequency, bits_per_sample,
+                                                   start->channels, _frame_size);
+        } catch (...) {
+            LOG_WARN("create player failed");
+            //todo: support disconnecting single channel
+            disable();
+            return;
+        }
+
     }
     _playing = true;
     _frame_count = 0;
@@ -355,9 +363,12 @@ void PlaybackChannel::handle_raw_data(RedPeer::InMessage* message)
     return;
 #endif
     if (size != _frame_bytes) {
+        char buf[100];
         //for now throw on unexpected size (based on current server imp).
         // will probably be replaced by supporting flexible data size in the player imp
-        THROW("unexpected frame size");
+        snprintf(buf, 100, "unexpected frame size, expected %10d != %10d",
+            _frame_bytes, size);
+        THROW(buf);
     }
     if ((_frame_count++ % 1000) == 0) {
         get_client().set_mm_time(packet->time - _wave_player->get_delay_ms());
@@ -365,32 +376,44 @@ void PlaybackChannel::handle_raw_data(RedPeer::InMessage* message)
     _wave_player->write(data);
 }
 
-void PlaybackChannel::handle_celt_data(RedPeer::InMessage* message)
+void PlaybackChannel::handle_opus_data(RedPeer::InMessage* message)
 {
-#if defined(HAVE_CELT051) || defined(HAVE_CELT)
     SpiceMsgPlaybackPacket* packet = (SpiceMsgPlaybackPacket*)message->data();
     uint8_t* data = packet->data;
     uint32_t size = packet->data_size;
-#ifdef HAVE_CELT051
-    celt_int16_t pcm[256 * 2];
-#else
-    celt_int16 pcm[256 * 2];
-#endif
+    opus_int16 pcm[FRAME_SIZE * 2];
     int result = CELT_OK;
 
-    if (_mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1) {
-#ifdef HAVE_CELT051
-        result = celt051_decode(_celt_decoder, data, size, pcm);
-#else
-        LOG_ERROR("This version of celt is not enabled!");
-#endif
-    } else {
-#ifdef HAVE_CELT
-        result = celt_decode(_celt_decoder, data, size, pcm, _frame_size);
-#else
-        LOG_ERROR("This version of celt is not linked!");
+    result = opus_decode(_opus_decoder, data, size, pcm, FRAME_SIZE,
+                         1 /* decode_fec 0/1 TODO */);
+
+    if (result < 0) {
+        THROW("opus decode failed");
+    }
+    if (result * 4 != _frame_bytes) {
+        THROW("opus decode returned unexpected number of bytes, %d != %d",
+            result, _frame_bytes);
+    }
+
+#ifdef WAVE_CAPTURE
+    put_wave_data(pcm, _frame_bytes);
+    return;
 #endif
+    if ((_frame_count++ % 1000) == 0) {
+        get_client().set_mm_time(packet->time - _wave_player->get_delay_ms());
     }
+    _wave_player->write((uint8_t *)pcm);
+}
+
+void PlaybackChannel::handle_celt_data(RedPeer::InMessage* message)
+{
+    SpiceMsgPlaybackPacket* packet = (SpiceMsgPlaybackPacket*)message->data();
+    uint8_t* data = packet->data;
+    uint32_t size = packet->data_size;
+    celt_int16_t pcm[256 * 2];
+    int result = CELT_OK;
+
+    result = celt051_decode(_celt_decoder, data, size, pcm);
 
     if (result != CELT_OK) {
         THROW("celt decode failed");
@@ -404,9 +427,6 @@ void PlaybackChannel::handle_celt_data(RedPeer::InMessage* message)
         get_client().set_mm_time(packet->time - _wave_player->get_delay_ms());
     }
     _wave_player->write((uint8_t *)pcm);
-#else // !defined(HAVE_CELT051) && !defined(HAVE_CELT)
-    LOG_WARN("celt is not enabled!");
-#endif
 }
 
 class PlaybackFactory: public ChannelFactory {
diff --git a/client/record_channel.cpp b/client/record_channel.cpp
index 36749789..c80a15a6 100644
--- a/client/record_channel.cpp
+++ b/client/record_channel.cpp
@@ -18,6 +18,7 @@
 #include <config.h>
 #endif
 
+#include <server/spice.h>
 #include "common.h"
 #include "red_client.h"
 #include "audio_channels.h"
@@ -60,15 +61,6 @@ void RecordSamplesMessage::release()
     _channel.release_message(this);
 }
 
-#ifdef HAVE_CELT051
-int RecordChannel::data_mode = SPICE_AUDIO_DATA_MODE_CELT_0_5_1;
-#elif defined(HAVE_CELT)
-int RecordChannel::data_mode = SPICE_AUDIO_DATA_MODE_CELT;
-#else
-int RecordChannel::data_mode = SPICE_AUDIO_DATA_MODE_RAW;
-#endif
-
-
 class RecordHandler: public MessageHandlerImp<RecordChannel, SPICE_CHANNEL_RECORD> {
 public:
     RecordHandler(RecordChannel& channel)
@@ -97,11 +89,9 @@ RecordChannel::RecordChannel(RedClient& client, uint32_t id)
 
     handler->set_handler(SPICE_MSG_RECORD_START, &RecordChannel::handle_start);
 
-#ifdef HAVE_CELT051
     set_capability(SPICE_RECORD_CAP_CELT_0_5_1);
-#endif
-#ifdef HAVE_CELT
-    set_capability(SPICE_RECORD_CAP_CELT);
+#ifdef HAVE_OPUS
+    set_capability(SPICE_RECORD_CAP_OPUS);
 #endif
 
 }
@@ -128,8 +118,13 @@ void RecordChannel::on_connect()
     SpiceMsgcRecordMode mode;
     mode.time = get_mm_time();
     mode.mode = _mode =
-      test_capability(SPICE_RECORD_CAP_CELT_0_5_1) ? RecordChannel::data_mode :
-                                                                      SPICE_AUDIO_DATA_MODE_RAW;
+        test_capability(SPICE_RECORD_CAP_OPUS) ?
+        SPICE_AUDIO_DATA_MODE_OPUS :
+        (
+            test_capability(SPICE_RECORD_CAP_CELT_0_5_1) ?
+            SPICE_AUDIO_DATA_MODE_CELT_0_5_1 :
+            SPICE_AUDIO_DATA_MODE_RAW
+        );
     _marshallers->msgc_record_mode(message->marshaller(), &mode);
     post_message(message);
 }
@@ -148,6 +143,28 @@ void RecordChannel::send_start_mark()
     post_message(message);
 }
 
+void RecordChannel::create_celt_encoder(int frequency, int channels)
+{
+    int celt_mode_err;
+    if (!(_celt_mode = celt051_mode_create(frequency, channels, _frame_size,
+                                           &celt_mode_err))) {
+        THROW("create celt mode failed %d", celt_mode_err);
+    }
+
+    if (!(_celt_encoder = celt051_encoder_create(_celt_mode))) {
+        THROW("create celt encoder failed");
+    }
+}
+
+void RecordChannel::create_opus_encoder(int frequency, int channels)
+{
+    int opus_err;
+
+    if (!(_opus_encoder = opus_encoder_create(frequency, channels, _frame_size, &opus_err))) {
+        THROW("create opus encoder failed");
+    }
+}
+
 void RecordChannel::handle_start(RedPeer::InMessage* message)
 {
     RecordHandler* handler = static_cast<RecordHandler*>(get_message_handler());
@@ -155,7 +172,7 @@ void RecordChannel::handle_start(RedPeer::InMessage* message)
 
     handler->set_handler(SPICE_MSG_RECORD_START, NULL);
     handler->set_handler(SPICE_MSG_RECORD_STOP, &RecordChannel::handle_stop);
-    ASSERT(!_wave_recorder && !_celt_mode && !_celt_encoder);
+    ASSERT(!_wave_recorder && !_celt_mode && !_celt_encoder && !_opus_encoder);
 
     // for now support only one setting
     if (start->format != SPICE_AUDIO_FMT_S16) {
@@ -163,25 +180,21 @@ void RecordChannel::handle_start(RedPeer::InMessage* message)
     }
 
     int bits_per_sample = 16;
+    _frame_bytes = _frame_size * bits_per_sample * start->channels / 8;
     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 = 256;
-    int celt_mode_err;
-    _frame_bytes = frame_size * bits_per_sample * start->channels / 8;
-    if (!(_celt_mode = celt051_mode_create(start->frequency, start->channels, frame_size,
-                                           &celt_mode_err))) {
-        THROW("create celt mode failed %d", celt_mode_err);
-    }
-
-    if (!(_celt_encoder = celt051_encoder_create(_celt_mode))) {
-        THROW("create celt encoder failed");
+    if (_mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1) {
+        create_celt_encoder(start->frequency, start->channels);
+    } else if (_mode == SPICE_AUDIO_DATA_MODE_OPUS) {
+        create_opus_encoder(start->frequency, start->channels);
     }
 
     send_start_mark();
@@ -261,9 +274,8 @@ void RecordChannel::remove_event_source(EventSources::Trigger& event_source)
     get_process_loop().remove_trigger(event_source);
 }
 
-#define FRAME_SIZE 256
-#define CELT_BIT_RATE (64 * 1024)
-#define CELT_COMPRESSED_FRAME_BYTES (FRAME_SIZE * CELT_BIT_RATE / 44100 / 8)
+#define BIT_RATE (64 * 1024)
+#define COMPRESSED_FRAME_BYTES (FRAME_SIZE * BIT_RATE / SPICE_INTERFACE_PLAYBACK_FREQ / 8)
 
 void RecordChannel::push_frame(uint8_t *frame)
 {
@@ -273,31 +285,27 @@ void RecordChannel::push_frame(uint8_t *frame)
         DBG(0, "blocked");
         return;
     }
-    uint8_t celt_buf[CELT_COMPRESSED_FRAME_BYTES]; // TODO - max of opus requiremend and celt
+    uint8_t buf[COMPRESSED_FRAME_BYTES];
     int n = -1;
 
     if (_mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1) {
-#ifdef HAVE_CELT051
-        n = celt051_encode(_celt_encoder, (celt_int16_t *)frame, NULL, celt_buf,
-                           CELT_COMPRESSED_FRAME_BYTES);
-#else
-        LOG_ERROR("This version of celt is not enabled!");
-#endif
+        n = celt051_encode(_celt_encoder, (celt_int16_t *)frame, NULL, buf,
+                           COMPRESSED_FRAME_BYTES);
         if (n < 0) {
             THROW("celt encode failed");
         }
-        frame = celt_buf;
+        frame = buf;
     } else if (_mode == SPICE_AUDIO_DATA_MODE_OPUS) {
 #ifdef HAVE_OPUS
-        n = opus_encode(_opus_encoder, (opus_int16 *)frame, FRAME_SIZE, celt_buf,
-                        CELT_COMPRESSED_FRAME_BYTES);
+        n = opus_encode(_opus_encoder, (opus_int16 *)frame, _frame_size, buf,
+                        COMPRESSED_FRAME_BYTES);
 #else
         LOG_ERROR("This version of celt is not enabled!");
 #endif
         if (n < 0) {
             THROW("celt encode failed");
         }
-        frame = celt_buf;
+        frame = buf;
     } else {
         n = _frame_bytes;
     }
diff --git a/client/x11/platform.cpp b/client/x11/platform.cpp
index 9094a64a..e49c95b1 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,
+                                              int 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,
+                                              int 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 e69294c8..d267d320 100644
--- a/client/x11/playback.cpp
+++ b/client/x11/playback.cpp
@@ -24,12 +24,14 @@
 
 #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,
+                       int frame_size)
     : _pcm (NULL)
     , _hw_params (NULL)
     , _sw_params (NULL)
+    , _frame_size (0)
 {
-    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 +59,9 @@ WavePlayer::~WavePlayer()
 
 bool WavePlayer::init(uint32_t sampels_per_sec,
                       uint32_t bits_per_sample,
-                      uint32_t channels)
+                      uint32_t channels,
+                      int frame_size)
 {
-    const int frame_size = WavePlaybackAbstract::FRAME_SIZE;
     const char* pcm_device = "default";
     snd_pcm_format_t format;
     int err;
@@ -75,6 +77,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 +186,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 d8efd7e4..3ddb6707 100644
--- a/client/x11/playback.h
+++ b/client/x11/playback.h
@@ -25,7 +25,8 @@
 
 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,
+               int frame_size);
     virtual ~WavePlayer();
 
     virtual bool write(uint8_t* frame);
@@ -34,7 +35,8 @@ 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,
+              int frame_size);
     void cleanup();
 
 private:
@@ -42,6 +44,7 @@ private:
     snd_pcm_hw_params_t* _hw_params;
     snd_pcm_sw_params_t* _sw_params;
     uint32_t _sampels_per_ms;
+    int _frame_size;
 };
 
 #endif
diff --git a/client/x11/record.cpp b/client/x11/record.cpp
index 017a94db..5c0a1b2a 100644
--- a/client/x11/record.cpp
+++ b/client/x11/record.cpp
@@ -48,18 +48,20 @@ 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,
+                           int 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)
+    , _frame_size (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,9 +95,9 @@ void WaveRecorder::cleanup()
 
 bool WaveRecorder::init(uint32_t sampels_per_sec,
                         uint32_t bits_per_sample,
-                        uint32_t channels)
+                        uint32_t channels,
+                        int frame_size)
 {
-    const int frame_size = WaveRecordAbstract::FRAME_SIZE;
     const char* pcm_device = "hw:0,0"; // "default" ???
     snd_pcm_format_t format;
     int err;
diff --git a/client/x11/record.h b/client/x11/record.h
index fde58c77..9f33299e 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,
+                 int 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,
+              int frame_size);
     void cleanup();
     void on_event();
 
@@ -52,6 +54,7 @@ private:
     uint8_t* _frame;
     uint8_t* _frame_pos;
     uint8_t* _frame_end;
+    int _frame_size;
     class EventTrigger;
     EventTrigger* _event_trigger;
 };
diff --git a/server/snd_worker.c b/server/snd_worker.c
index aba7bdd1..bb92834d 100644
--- a/server/snd_worker.c
+++ b/server/snd_worker.c
@@ -49,23 +49,10 @@
  * The passed frame_size must an opus frame size for the encoder's sampling rate.
  * For example, at 48kHz the permitted values are 120, 240, 480, or 960.
  */
-#define OPUS_FRAME_SIZE 480
-#define CELT_FRAME_SIZE 256
-#ifdef HAVE_OPUS
-// max?
-# if OPUS_FRAME_SIZE > CELT_FRAME_SIZE
-#define FRAME_SIZE OPUS_FRAME_SIZE
-# else
-#define FRAME_SIZE CELT_FRAME_SIZE
-# endif
-#else
-#define FRAME_SIZE CELT_FRAME_SIZE
-#endif
-#define PLAYBACK_BUF_SIZE (FRAME_SIZE * 4)
+#define FRAME_SIZE 480
 
 #define COMPRESSED_BIT_RATE (64 * 1024)
 #define COMPRESSED_FRAME_BYTES (FRAME_SIZE * COMPRESSED_BIT_RATE / SPICE_INTERFACE_PLAYBACK_FREQ / 8)
-#define COMPRESSED_FRAME_BYTES_OPUS (FRAME_SIZE * COMPRESSED_BIT_RATE / SPICE_INTERFACE_PLAYBACK_FREQ_OPUS / 8)
 
 #define RECORD_SAMPLES_SIZE (RECIVE_BUF_SIZE >> 2)
 
@@ -120,6 +107,8 @@ struct SndChannel {
     uint32_t out_messages;
     uint32_t ack_messages;
 
+    uint32_t frame_size;
+
     struct {
         uint64_t serial;
         SpiceDataHeader *header;
@@ -141,6 +130,8 @@ struct SndChannel {
     snd_channel_cleanup_channel_proc cleanup;
 };
 
+/* this wastes some memory because celt frames are 256 samples and
+ * opus are 480 */
 typedef struct AudioFrame AudioFrame;
 struct AudioFrame {
     uint32_t time;
@@ -361,11 +352,11 @@ static int snd_record_handle_write(RecordChannel *record_channel, size_t size, v
             return FALSE;
         }
         data = record_channel->decode_buf;
-        size = FRAME_SIZE;
+        size = record_channel->base.frame_size;
     } else if (record_channel->mode == SPICE_AUDIO_DATA_MODE_OPUS) {
 #ifdef HAVE_OPUS
         int opus_err = opus_decode(record_channel->opus_decoder, packet->data, size,
-                                   (opus_int16 *)record_channel->decode_buf, FRAME_SIZE,
+                                   (opus_int16 *)record_channel->decode_buf, record_channel->base.frame_size,
                                    1 /* decode_fec 0/1 TODO */);
         if (opus_err < 0) {
             red_printf("opus decode failed (%s)", opus_strerror(opus_err));
@@ -456,7 +447,7 @@ static int snd_record_handle_message(SndChannel *channel, size_t size, uint32_t
         if (record_channel->mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1) {
             if (!(celt_mode = celt051_mode_create(SPICE_INTERFACE_RECORD_FREQ,
                                                   SPICE_INTERFACE_RECORD_CHAN,
-                                                  FRAME_SIZE, &celt_error))) {
+                                                  record_channel->base.frame_size, &celt_error))) {
                 red_printf("create celt mode failed %d", celt_error);
             }
 
@@ -465,7 +456,7 @@ static int snd_record_handle_message(SndChannel *channel, size_t size, uint32_t
             }
         } else if (record_channel->mode == SPICE_AUDIO_DATA_MODE_OPUS) {
 #ifdef HAVE_OPUS
-            if (!(opus_decoder = opus_decoder_create(SPICE_INTERFACE_RECORD_FREQ_OPUS,
+            if (!(opus_decoder = opus_decoder_create(SPICE_INTERFACE_RECORD_FREQ,
                                                      SPICE_INTERFACE_RECORD_CHAN,
                                                      &opus_error))) {
                 red_printf("create opus decoder failed: %d", opus_error);
@@ -698,10 +689,10 @@ static int snd_playback_send_start(PlaybackChannel *playback_channel)
     }
 
     start.channels = SPICE_INTERFACE_PLAYBACK_CHAN;
-    start.frequency = SPICE_INTERFACE_PLAYBACK_FREQ;
     ASSERT(SPICE_INTERFACE_PLAYBACK_FMT == SPICE_INTERFACE_AUDIO_FMT_S16);
     start.format = SPICE_AUDIO_FMT_S16;
     start.time = reds_get_mm_time();
+    start.frequency = SPICE_INTERFACE_PLAYBACK_FREQ;
     spice_marshall_msg_playback_start(channel->send_data.marshaller, &start);
 
     return snd_begin_send_message(channel);
@@ -859,7 +850,7 @@ static int snd_playback_send_write(PlaybackChannel *playback_channel)
     } else if (playback_channel->mode == SPICE_AUDIO_DATA_MODE_OPUS) {
 #ifdef HAVE_OPUS
         int n = -1;
-        n = opus_encode(playback_channel->opus_encoder, (opus_int16 *)frame->samples, sizeof(frame->samples),
+        n = opus_encode(playback_channel->opus_encoder, (opus_int16 *)frame->samples, channel->frame_size,
                         playback_channel->send_data.encode_buf, COMPRESSED_FRAME_BYTES);
         if (n < 0) {
             red_printf("opus encode failed");
@@ -874,7 +865,7 @@ static int snd_playback_send_write(PlaybackChannel *playback_channel)
 #endif
     } else {
         spice_marshaller_add_ref(channel->send_data.marshaller,
-                                 (uint8_t *)frame->samples, sizeof(frame->samples));
+                                 (uint8_t *)frame->samples, channel->frame_size);
     }
 
     return snd_begin_send_message(channel);
@@ -1028,6 +1019,8 @@ static SndChannel *__new_channel(SndWorker *worker, int size, uint32_t channel_i
     channel->recive_data.now = channel->recive_data.buf;
     channel->recive_data.end = channel->recive_data.buf + sizeof(channel->recive_data.buf);
     channel->send_data.marshaller = spice_marshaller_new();
+    // set initial frame size - can be changed later?
+    channel->frame_size = FRAME_SIZE;
 
     stream->watch = core->watch_add(stream->socket, SPICE_WATCH_EVENT_READ,
                                   snd_event, channel);
@@ -1172,7 +1165,7 @@ SPICE_GNUC_VISIBLE void spice_server_playback_get_buffer(SpicePlaybackInstance *
 
     *frame = playback_channel->free_frames->samples;
     playback_channel->free_frames = playback_channel->free_frames->next;
-    *num_samples = FRAME_SIZE;
+    *num_samples = channel->frame_size;
 }
 
 SPICE_GNUC_VISIBLE void spice_server_playback_put_samples(SpicePlaybackInstance *sin, uint32_t *samples)
@@ -1241,6 +1234,7 @@ static void snd_playback_cleanup(SndChannel *channel)
 
     playback_channel->celt_encoder = NULL;
     playback_channel->celt_mode = NULL;
+    playback_channel->opus_encoder = NULL;
 }
 
 static uint32_t snd_select_best_mode(RedChannelClient *rcc)
@@ -1274,7 +1268,9 @@ static int snd_create_encoder(PlaybackChannel *playback_channel)
         if (!(celt_mode = celt051_mode_create(SPICE_INTERFACE_PLAYBACK_FREQ,
                                               SPICE_INTERFACE_PLAYBACK_CHAN,
                                               FRAME_SIZE, &celt_error))) {
-            red_printf("create celt mode failed %d", celt_error);
+            red_printf("create celt mode failed %d, freq=%d, chan=%d", celt_error,
+                       SPICE_INTERFACE_PLAYBACK_FREQ,
+                       SPICE_INTERFACE_PLAYBACK_CHAN);
             return -1;
         }
 
@@ -1285,9 +1281,12 @@ static int snd_create_encoder(PlaybackChannel *playback_channel)
     } else if (mode == SPICE_AUDIO_DATA_MODE_OPUS) {
 #ifdef HAVE_OPUS
         if (!(opus_encoder = opus_encoder_create(SPICE_INTERFACE_PLAYBACK_FREQ,
-                                                 SPICE_INTERFACE_PLAYBACK_CHAN, FRAME_SIZE,
+                                                 SPICE_INTERFACE_PLAYBACK_CHAN,
+                                                 OPUS_APPLICATION_RESTRICTED_LOWDELAY,
                                                  &opus_error))) {
-            red_printf("create opus encoder failed %d", opus_error);
+            red_printf("create opus encoder failed %d, freq=%d, chan=%d, frame_size=%d", opus_error,
+                       SPICE_INTERFACE_PLAYBACK_FREQ,
+                       SPICE_INTERFACE_PLAYBACK_CHAN, FRAME_SIZE);
             return -1;
         }
 #else
diff --git a/server/spice.h b/server/spice.h
index 271d6a91..b6c509de 100644
--- a/server/spice.h
+++ b/server/spice.h
@@ -313,9 +313,14 @@ enum {
     SPICE_INTERFACE_AUDIO_FMT_S16 = 1,
 };
 
+/* TODO: how to advertise to compile time dependencies (qemu) that this changed?
+ * increment SPICE_INTERFACE_PLAYBACK_MINOR? */
+#ifdef HAVE_OPUS
 /* Must be one of 8000, 12000, 16000, 24000, or 48000 */
-#define SPICE_INTERFACE_PLAYBACK_FREQ_OPUS  48000
+#define SPICE_INTERFACE_PLAYBACK_FREQ  48000
+#else
 #define SPICE_INTERFACE_PLAYBACK_FREQ  44100
+#endif
 #define SPICE_INTERFACE_PLAYBACK_CHAN  2
 #define SPICE_INTERFACE_PLAYBACK_FMT   SPICE_INTERFACE_AUDIO_FMT_S16
 
@@ -345,9 +350,12 @@ typedef struct SpiceRecordInterface SpiceRecordInterface;
 typedef struct SpiceRecordInstance SpiceRecordInstance;
 typedef struct SpiceRecordState SpiceRecordState;
 
-#define SPICE_INTERFACE_RECORD_FREQ  44100
+#ifdef HAVE_OPUS
 /* Must be one of 8000, 12000, 16000, 24000, or 48000 */
-#define SPICE_INTERFACE_RECORD_FREQ_OPUS  48000
+#define SPICE_INTERFACE_RECORD_FREQ 48000
+#else
+#define SPICE_INTERFACE_RECORD_FREQ 44100
+#endif
 #define SPICE_INTERFACE_RECORD_CHAN  2
 #define SPICE_INTERFACE_RECORD_FMT   SPICE_INTERFACE_AUDIO_FMT_S16
 
diff --git a/server/tests/test_playback.c b/server/tests/test_playback.c
index 0b95bfdf..0f515f1a 100644
--- a/server/tests/test_playback.c
+++ b/server/tests/test_playback.c
@@ -90,12 +90,21 @@ void playback_timer_cb(void *opaque)
     core->timer_start(playback_timer, playback_timer_ms);
 }
 
-int main(void)
+int main(int argc, char **argv)
 {
+    int port = 5701;
+
     SpiceServer *server = spice_server_new();
     core = basic_event_loop_init();
 
-    spice_server_set_port(server, 5701);
+#ifdef HAVE_OPUS
+    printf("testing opus\n");
+#else
+    printf("testing celt\n");
+#endif
+
+    printf("listening to port %d\n", port);
+    spice_server_set_port(server, port);
     spice_server_set_noauth(server);
     spice_server_init(server, core);