[Spice-devel] [codec 2/spice] Revise the spice server to use the new snd_codec functions in spice-common.
Christophe Fergeau
cfergeau at redhat.com
Wed Oct 16 13:05:40 CEST 2013
The subject is misleading, it's not just changing spice-server, but both
server and client.
On Tue, Oct 15, 2013 at 09:28:46AM -0500, Jeremy White wrote:
> This makes celt optional, and paves the way to readily add additional codecs.
>
> Signed-off-by: Jeremy White <jwhite at codeweavers.com>
> ---
> README | 1 -
> client/audio_channels.h | 12 ++-
> client/playback_channel.cpp | 51 ++++-------
> client/record_channel.cpp | 65 +++++---------
> configure.ac | 22 +++--
> server/snd_worker.c | 208 ++++++++++++++++++-------------------------
> server/snd_worker.h | 4 +
> 7 files changed, 156 insertions(+), 207 deletions(-)
>
> diff --git a/README b/README
> index e146a95..018ab1a 100644
> --- a/README
> +++ b/README
> @@ -28,7 +28,6 @@ The following mandatory dependancies are required in order to
> build SPICE
>
> Spice protocol >= 0.9.0
> - Celt >= 0.5.1.1, < 0.6.0
> Pixman >= 0.17.7
> OpenSSL
> libjpeg
> diff --git a/client/audio_channels.h b/client/audio_channels.h
> index d38a79e..a6be70a 100644
> --- a/client/audio_channels.h
> +++ b/client/audio_channels.h
> @@ -18,7 +18,7 @@
> #ifndef _H_AUDIO_CHANNELS
> #define _H_AUDIO_CHANNELS
>
> -#include <celt051/celt.h>
> +#include "common/snd_codec.h"
>
> #include "red_channel.h"
> #include "debug.h"
> @@ -45,7 +45,7 @@ private:
> void handle_start(RedPeer::InMessage* message);
> void handle_stop(RedPeer::InMessage* message);
> void handle_raw_data(RedPeer::InMessage* message);
> - void handle_celt_data(RedPeer::InMessage* message);
> + void handle_compressed_data(RedPeer::InMessage* message);
> void null_handler(RedPeer::InMessage* message);
> void disable();
>
> @@ -57,8 +57,7 @@ private:
> WavePlaybackAbstract* _wave_player;
> uint32_t _mode;
> uint32_t _frame_bytes;
> - CELTMode *_celt_mode;
> - CELTDecoder *_celt_decoder;
> + SndCodec codec;
This should be name _codec for consistency
> bool _playing;
> uint32_t _frame_count;
> };
> @@ -96,11 +95,10 @@ private:
> Mutex _messages_lock;
> std::list<RecordSamplesMessage *> _messages;
> int _mode;
> - CELTMode *_celt_mode;
> - CELTEncoder *_celt_encoder;
> + SndCodec codec;
> uint32_t _frame_bytes;
>
> - static int data_mode;
> + uint8_t compressed_buf[SND_CODEC_MAX_COMPRESSED_BYTES];
>
> friend class RecordSamplesMessage;
> };
> diff --git a/client/playback_channel.cpp b/client/playback_channel.cpp
> index 802a4d3..922c464 100644
> --- a/client/playback_channel.cpp
> +++ b/client/playback_channel.cpp
> @@ -151,8 +151,6 @@ PlaybackChannel::PlaybackChannel(RedClient& client, uint32_t id)
> Platform::PRIORITY_HIGH)
> , _wave_player (NULL)
> , _mode (SPICE_AUDIO_DATA_MODE_INVALID)
> - , _celt_mode (NULL)
> - , _celt_decoder (NULL)
> , _playing (false)
> {
> #ifdef WAVE_CAPTURE
> @@ -169,7 +167,8 @@ PlaybackChannel::PlaybackChannel(RedClient& client, uint32_t id)
>
> handler->set_handler(SPICE_MSG_PLAYBACK_MODE, &PlaybackChannel::handle_mode);
>
> - set_capability(SPICE_PLAYBACK_CAP_CELT_0_5_1);
> + if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1))
> + set_capability(SPICE_PLAYBACK_CAP_CELT_0_5_1);
> }
>
> void PlaybackChannel::clear()
> @@ -182,15 +181,7 @@ void PlaybackChannel::clear()
> }
> _mode = SPICE_AUDIO_DATA_MODE_INVALID;
>
> - if (_celt_decoder) {
> - celt051_decoder_destroy(_celt_decoder);
> - _celt_decoder = NULL;
> - }
> -
> - if (_celt_mode) {
> - celt051_mode_destroy(_celt_mode);
> - _celt_mode = NULL;
> - }
> + snd_codec_destroy(&codec);
> }
>
> void PlaybackChannel::on_disconnect()
> @@ -214,18 +205,19 @@ void PlaybackChannel::set_data_handler()
>
> if (_mode == SPICE_AUDIO_DATA_MODE_RAW) {
> 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 (snd_codec_is_capable(_mode)) {
> + handler->set_handler(SPICE_MSG_PLAYBACK_DATA, &PlaybackChannel::handle_compressed_data);
> } else {
> THROW("invalid mode");
> }
> +
> }
>
> 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) {
> + if (playbacke_mode->mode != SPICE_AUDIO_DATA_MODE_RAW
> + && !snd_codec_is_capable(playbacke_mode->mode) ) {
> THROW("invalid mode");
> }
Can you fix the playbacke typo while at it?
>
> @@ -265,15 +257,11 @@ void PlaybackChannel::handle_start(RedPeer::InMessage* message)
> start_wave();
> #endif
> if (!_wave_player) {
> - // for now support only one setting
> - int celt_mode_err;
> -
> if (start->format != SPICE_AUDIO_FMT_S16) {
> THROW("unexpected format");
> }
> int bits_per_sample = 16;
> - int frame_size = 256;
> - _frame_bytes = frame_size * start->channels * bits_per_sample / 8;
> + int frame_size = SND_CODEC_MAX_FRAME_SIZE;
> try {
> _wave_player = Platform::create_player(start->frequency, bits_per_sample,
> start->channels);
> @@ -284,14 +272,13 @@ void PlaybackChannel::handle_start(RedPeer::InMessage* message)
> return;
> }
>
> - 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 (_mode != SPICE_AUDIO_DATA_MODE_RAW) {
> + if (snd_codec_create(&codec, _mode, FALSE, TRUE) != SND_CODEC_OK)
> + THROW("create decoder");
> + frame_size = snd_codec_frame_size(&codec);
> }
>
> - if (!(_celt_decoder = celt051_decoder_create(_celt_mode))) {
> - THROW("create celt decoder");
> - }
> + _frame_bytes = frame_size * start->channels * bits_per_sample / 8;
This does not seem right, snd_codec_create() hardcodes the numbers of
channels to SND_CODEC_CELT_PLAYBACK_CHAN, so we totally ignore whatever was
in 'start', but here we use start->channels to compute _frame_bytes.
> }
> _playing = true;
> _frame_count = 0;
> @@ -333,16 +320,16 @@ void PlaybackChannel::handle_raw_data(RedPeer::InMessage* message)
> _wave_player->write(data);
> }
>
> -void PlaybackChannel::handle_celt_data(RedPeer::InMessage* message)
> +void PlaybackChannel::handle_compressed_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 pcm_size = _frame_bytes;
> + uint8_t pcm[_frame_bytes];
>
> - if (celt051_decode(_celt_decoder, data, size, pcm) != CELT_OK) {
> - THROW("celt decode failed");
> - }
> + if (snd_codec_decode(&codec, data, size, pcm, &pcm_size) != SND_CODEC_OK)
> + THROW("decode failed");
> #ifdef WAVE_CAPTURE
> put_wave_data(pcm, _frame_bytes);
> return;
> diff --git a/client/record_channel.cpp b/client/record_channel.cpp
> index d9332c6..5c878f2 100644
> --- a/client/record_channel.cpp
> +++ b/client/record_channel.cpp
> @@ -60,8 +60,6 @@ void RecordSamplesMessage::release()
> _channel.release_message(this);
> }
>
> -int RecordChannel::data_mode = SPICE_AUDIO_DATA_MODE_CELT_0_5_1;
> -
> class RecordHandler: public MessageHandlerImp<RecordChannel, SPICE_CHANNEL_RECORD> {
> public:
> RecordHandler(RecordChannel& channel)
> @@ -72,8 +70,6 @@ RecordChannel::RecordChannel(RedClient& client, uint32_t id)
> : RedChannel(client, SPICE_CHANNEL_RECORD, id, new RecordHandler(*this))
> , _wave_recorder (NULL)
> , _mode (SPICE_AUDIO_DATA_MODE_INVALID)
> - , _celt_mode (NULL)
> - , _celt_encoder (NULL)
> {
> for (int i = 0; i < NUM_SAMPLES_MESSAGES; i++) {
> _messages.push_front(new RecordSamplesMessage(*this));
> @@ -90,7 +86,8 @@ RecordChannel::RecordChannel(RedClient& client, uint32_t id)
>
> handler->set_handler(SPICE_MSG_RECORD_START, &RecordChannel::handle_start);
>
> - set_capability(SPICE_RECORD_CAP_CELT_0_5_1);
> + if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1))
> + set_capability(SPICE_RECORD_CAP_CELT_0_5_1);
> }
>
> RecordChannel::~RecordChannel(void)
> @@ -114,9 +111,12 @@ void RecordChannel::on_connect()
> Message* message = new Message(SPICE_MSGC_RECORD_MODE);
> 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;
> + 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
> + _mode = SPICE_AUDIO_DATA_MODE_RAW;
> + mode.mode = _mode;
> _marshallers->msgc_record_mode(message->marshaller(), &mode);
> post_message(message);
> }
> @@ -142,7 +142,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);
>
> // for now support only one setting
> if (start->format != SPICE_AUDIO_FMT_S16) {
> @@ -159,17 +159,13 @@ void RecordChannel::handle_start(RedPeer::InMessage* message)
> 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");
> + int frame_size = SND_CODEC_MAX_FRAME_SIZE;
> + if (_mode != SPICE_AUDIO_DATA_MODE_RAW) {
> + if (snd_codec_create(&codec, _mode, 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;
Same comment as earlier
>
> send_start_mark();
> _wave_recorder->start();
> @@ -182,14 +178,7 @@ void RecordChannel::clear()
> delete _wave_recorder;
> _wave_recorder = NULL;
> }
> - if (_celt_encoder) {
> - celt051_encoder_destroy(_celt_encoder);
> - _celt_encoder = NULL;
> - }
> - if (_celt_mode) {
> - celt051_mode_destroy(_celt_mode);
> - _celt_mode = NULL;
> - }
> + snd_codec_destroy(&codec);
> }
>
> void RecordChannel::handle_stop(RedPeer::InMessage* message)
> @@ -200,7 +189,6 @@ void RecordChannel::handle_stop(RedPeer::InMessage* message)
> if (!_wave_recorder) {
> return;
> }
> - ASSERT(_celt_mode && _celt_encoder);
> clear();
> }
>
> @@ -242,10 +230,6 @@ 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)
> -
> void RecordChannel::push_frame(uint8_t *frame)
> {
> RecordSamplesMessage *message;
> @@ -254,19 +238,18 @@ void RecordChannel::push_frame(uint8_t *frame)
> DBG(0, "blocked");
> return;
> }
> - uint8_t celt_buf[CELT_COMPRESSED_FRAME_BYTES];
> int n;
>
> - if (_mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1) {
> - n = celt051_encode(_celt_encoder, (celt_int16_t *)frame, NULL, celt_buf,
> - CELT_COMPRESSED_FRAME_BYTES);
> - if (n < 0) {
> - THROW("celt encode failed");
> - }
> - frame = celt_buf;
> - } else {
> +
> + if (_mode == SPICE_AUDIO_DATA_MODE_RAW) {
> n = _frame_bytes;
> + } else {
> + n = sizeof(compressed_buf);
> + if (snd_codec_encode(&codec, frame, _frame_bytes, compressed_buf, &n) != SND_CODEC_OK)
> + THROW("encode failed");
> + frame = compressed_buf;
> }
> +
> RedPeer::OutMessage& peer_message = message->peer_message();
> peer_message.reset(SPICE_MSGC_RECORD_DATA);
> SpiceMsgcRecordPacket packet;
> diff --git a/configure.ac b/configure.ac
> index fa1ba31..4519a84 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -149,6 +149,10 @@ if test "x$enable_smartcard" = "xyes"; then
> AC_DEFINE([USE_SMARTCARD], [1], [Define if supporting smartcard proxying])
> fi
>
> +AC_ARG_ENABLE(celt051,
> +[ --disable-celt051 Disable celt051 audio codec (enabled by default)],,
> +[enable_celt051="yes"])
> +
> AC_ARG_ENABLE(client,
> [ --enable-client Enable spice client],,
> [enable_client="no"])
> @@ -247,11 +251,17 @@ AC_SUBST(PIXMAN_CFLAGS)
> AC_SUBST(PIXMAN_LIBS)
> SPICE_REQUIRES+=" pixman-1 >= 0.17.7"
>
> -PKG_CHECK_MODULES(CELT051, celt051 >= 0.5.1.1)
> -AC_SUBST(CELT051_CFLAGS)
> -AC_SUBST(CELT051_LIBS)
> -AC_SUBST(CELT051_LIBDIR)
> -SPICE_REQUIRES+=" celt051 >= 0.5.1.1"
> +if test "x$enable_celt051" = "xyes"; then
> + PKG_CHECK_MODULES(CELT051, celt051 >= 0.5.1.1, have_celt051=yes, have_celt051=no)
> + AC_SUBST(CELT051_CFLAGS)
> + AC_SUBST(CELT051_LIBS)
> + AC_SUBST(CELT051_LIBDIR)
> +else
> + have_celt051=no
> +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]))
Is it needed at all now that you moved the celt logic to spice-common?
>
> if test ! -e client/generated_marshallers.cpp; then
> AC_MSG_CHECKING([for pyparsing python module])
> @@ -537,6 +547,8 @@ echo "
>
> Smartcard: ${enable_smartcard}
>
> + celt051: ${have_celt051}
> +
> SASL support: ${enable_sasl}
>
> Automated tests: ${enable_automated_tests}
> diff --git a/server/snd_worker.c b/server/snd_worker.c
> index 5346d96..a1e452b 100644
> --- a/server/snd_worker.c
> +++ b/server/snd_worker.c
> @@ -25,7 +25,6 @@
> #include <sys/socket.h>
> #include <netinet/ip.h>
> #include <netinet/tcp.h>
> -#include <celt051/celt.h>
>
> #include "common/marshaller.h"
> #include "common/generated_server_marshallers.h"
> @@ -36,20 +35,14 @@
> #include "reds.h"
> #include "red_dispatcher.h"
> #include "snd_worker.h"
> +#include "common/snd_codec.h"
> #include "demarshallers.h"
>
> #ifndef IOV_MAX
> #define IOV_MAX 1024
> #endif
>
> -#define SND_RECEIVE_BUF_SIZE (16 * 1024 * 2)
> -
> -#define FRAME_SIZE 256
> -#define PLAYBACK_BUF_SIZE (FRAME_SIZE * 4)
> -
> -#define CELT_BIT_RATE (64 * 1024)
> -#define CELT_COMPRESSED_FRAME_BYTES (FRAME_SIZE * CELT_BIT_RATE / SPICE_INTERFACE_PLAYBACK_FREQ / 8)
> -
> +#define SND_RECEIVE_BUF_SIZE ((16 * 1024 * 2) >> 2)
> #define RECORD_SAMPLES_SIZE (SND_RECEIVE_BUF_SIZE >> 2)
Is it intentional that you made the buffer to receive sound data 4 times
smaller than it was?
>
> enum PlaybackeCommand {
> @@ -129,7 +122,7 @@ typedef struct PlaybackChannel PlaybackChannel;
> typedef struct AudioFrame AudioFrame;
> struct AudioFrame {
> uint32_t time;
> - uint32_t samples[FRAME_SIZE];
> + uint32_t samples[SND_CODEC_MAX_FRAME_SIZE];
> PlaybackChannel *channel;
> AudioFrame *next;
> };
> @@ -140,13 +133,10 @@ struct PlaybackChannel {
> AudioFrame *free_frames;
> AudioFrame *in_progress;
> AudioFrame *pending_frame;
> - CELTMode *celt_mode;
> - CELTEncoder *celt_encoder;
> uint32_t mode;
> - struct {
> - uint8_t celt_buf[CELT_COMPRESSED_FRAME_BYTES];
> - } send_data;
> uint32_t latency;
> + SndCodec codec;
> + uint8_t encode_buf[SND_CODEC_MAX_COMPRESSED_BYTES];
> };
>
> struct SndWorker {
> @@ -182,13 +172,12 @@ typedef struct RecordChannel {
> uint32_t mode;
> uint32_t mode_time;
> uint32_t start_time;
> - CELTDecoder *celt_decoder;
> - CELTMode *celt_mode;
> - uint32_t celt_buf[FRAME_SIZE];
> + SndCodec codec;
> + uint8_t decode_buf[SND_CODEC_MAX_FRAME_BYTES];
> } RecordChannel;
>
> static SndWorker *workers;
> -static uint32_t playback_compression = SPICE_AUDIO_DATA_MODE_CELT_0_5_1;
> +static uint32_t playback_compression = SPICE_PLAYBACK_COMPRESSION_AUTO;
>
> static void snd_receive(void* data);
>
> @@ -321,23 +310,19 @@ static int snd_record_handle_write(RecordChannel *record_channel, size_t size, v
> }
>
> packet = (SpiceMsgcRecordPacket *)message;
> - size = packet->data_size;
>
> - if (record_channel->mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1) {
> - int celt_err = celt051_decode(record_channel->celt_decoder, packet->data, size,
> - (celt_int16_t *)record_channel->celt_buf);
> - if (celt_err != CELT_OK) {
> - spice_printerr("celt decode failed (%d)", celt_err);
> - return FALSE;
> - }
> - data = record_channel->celt_buf;
> - size = FRAME_SIZE;
> - } else if (record_channel->mode == SPICE_AUDIO_DATA_MODE_RAW) {
> + if (record_channel->mode == SPICE_AUDIO_DATA_MODE_RAW) {
> data = (uint32_t *)packet->data;
> - size = size >> 2;
> + size = packet->data_size >> 2;
> size = MIN(size, RECORD_SAMPLES_SIZE);
> - } else {
> - return FALSE;
> + } else {
> + int decode_size;
> + decode_size = sizeof(record_channel->decode_buf);
> + if (snd_codec_decode(&record_channel->codec, packet->data, packet->data_size,
> + record_channel->decode_buf, &decode_size) != SND_CODEC_OK)
> + return FALSE;
> + data = (uint32_t *) record_channel->decode_buf;
> + size = decode_size >> 2;
I assume this is to go from bytes to samples? I still feel uncomfortable
that these parameters are supposed to be hidden by SndCodec, but we still
have some hardcoding about it here and there. With that said, I can
probably live with that one.
> }
>
> write_pos = record_channel->write_pos % RECORD_SAMPLES_SIZE;
> @@ -387,9 +372,9 @@ static int snd_record_handle_message(SndChannel *channel, size_t size, uint32_t
> SpiceMsgcRecordMode *mode = (SpiceMsgcRecordMode *)message;
> record_channel->mode = mode->mode;
> record_channel->mode_time = mode->time;
> - if (record_channel->mode != SPICE_AUDIO_DATA_MODE_CELT_0_5_1 &&
> - record_channel->mode != SPICE_AUDIO_DATA_MODE_RAW) {
> - spice_printerr("unsupported mode");
> + if (record_channel->mode != SPICE_AUDIO_DATA_MODE_RAW &&
> + ! snd_codec_is_capable(record_channel->mode)) {
> + spice_printerr("unsupported mode %d", record_channel->mode);
> }
> break;
> }
> @@ -758,19 +743,19 @@ static int snd_playback_send_write(PlaybackChannel *playback_channel)
>
> spice_marshall_msg_playback_data(channel->send_data.marshaller, &msg);
>
> - if (playback_channel->mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1) {
> - int n = celt051_encode(playback_channel->celt_encoder, (celt_int16_t *)frame->samples, NULL,
> - playback_channel->send_data.celt_buf, CELT_COMPRESSED_FRAME_BYTES);
> - if (n < 0) {
> - spice_printerr("celt encode failed");
> + if (playback_channel->mode == SPICE_AUDIO_DATA_MODE_RAW) {
> + spice_marshaller_add_ref(channel->send_data.marshaller,
> + (uint8_t *)frame->samples, sizeof(frame->samples));
> + }
> + else {
> + int n = sizeof(playback_channel->encode_buf);
> + if (snd_codec_encode(&playback_channel->codec, (uint8_t *) frame->samples, sizeof(frame->samples),
> + playback_channel->encode_buf, &n) != SND_CODEC_OK) {
> + spice_printerr("encode failed");
> snd_disconnect_channel(channel);
> return FALSE;
> }
> - spice_marshaller_add_ref(channel->send_data.marshaller,
> - playback_channel->send_data.celt_buf, n);
> - } else {
> - spice_marshaller_add_ref(channel->send_data.marshaller,
> - (uint8_t *)frame->samples, sizeof(frame->samples));
> + spice_marshaller_add_ref(channel->send_data.marshaller, playback_channel->encode_buf, n);
> }
>
> return snd_begin_send_message(channel);
> @@ -1090,7 +1075,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 = snd_codec_frame_size(&playback_channel->codec);
> }
>
> SPICE_GNUC_VISIBLE void spice_server_playback_put_samples(SpicePlaybackInstance *sin, uint32_t *samples)
> @@ -1140,6 +1125,18 @@ void snd_set_playback_latency(RedClient *client, uint32_t latency)
> }
> }
> }
> +
> +static int snd_desired_audio_mode(int client_can_celt)
> +{
> + if (playback_compression == SPICE_PLAYBACK_COMPRESSION_NONE)
> + return SPICE_AUDIO_DATA_MODE_RAW;
> +
> + 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;
> +
> + return SPICE_AUDIO_DATA_MODE_RAW;
> +}
> +
> static void on_new_playback_channel(SndWorker *worker)
> {
> PlaybackChannel *playback_channel =
> @@ -1168,8 +1165,7 @@ static void snd_playback_cleanup(SndChannel *channel)
> reds_enable_mm_timer();
> }
>
> - celt051_encoder_destroy(playback_channel->celt_encoder);
> - celt051_mode_destroy(playback_channel->celt_mode);
> + snd_codec_destroy(&playback_channel->codec);
> }
>
> static void snd_set_playback_peer(RedChannel *channel, RedClient *client, RedsStream *stream,
> @@ -1179,25 +1175,9 @@ static void snd_set_playback_peer(RedChannel *channel, RedClient *client, RedsSt
> SndWorker *worker = channel->data;
> PlaybackChannel *playback_channel;
> SpicePlaybackState *st = SPICE_CONTAINEROF(worker, SpicePlaybackState, worker);
> - CELTEncoder *celt_encoder;
> - CELTMode *celt_mode;
> - int celt_error;
> - RedChannelClient *rcc;
>
> snd_disconnect_channel(worker->connection);
>
> - if (!(celt_mode = celt051_mode_create(SPICE_INTERFACE_PLAYBACK_FREQ,
> - SPICE_INTERFACE_PLAYBACK_CHAN,
> - FRAME_SIZE, &celt_error))) {
> - spice_printerr("create celt mode failed %d", celt_error);
> - return;
> - }
> -
> - if (!(celt_encoder = celt051_encoder_create(celt_mode))) {
> - spice_printerr("create celt encoder failed");
> - goto error_1;
> - }
> -
> if (!(playback_channel = (PlaybackChannel *)__new_channel(worker,
> sizeof(*playback_channel),
> SPICE_CHANNEL_PLAYBACK,
> @@ -1210,32 +1190,31 @@ static void snd_set_playback_peer(RedChannel *channel, RedClient *client, RedsSt
> snd_playback_cleanup,
> common_caps, num_common_caps,
> caps, num_caps))) {
> - goto error_2;
> + return;
> }
> worker->connection = &playback_channel->base;
> - rcc = playback_channel->base.channel_client;
> snd_playback_free_frame(playback_channel, &playback_channel->frames[0]);
> snd_playback_free_frame(playback_channel, &playback_channel->frames[1]);
> snd_playback_free_frame(playback_channel, &playback_channel->frames[2]);
>
> - playback_channel->celt_mode = celt_mode;
> - playback_channel->celt_encoder = celt_encoder;
> - playback_channel->mode = red_channel_client_test_remote_cap(rcc,
> - SPICE_PLAYBACK_CAP_CELT_0_5_1) ?
> - playback_compression : SPICE_AUDIO_DATA_MODE_RAW;
> + 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);
> + 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) {
> + playback_channel->mode = desired_mode;
> + } else {
> + spice_printerr("create encoder failed");
> + }
> + }
>
> on_new_playback_channel(worker);
> if (worker->active) {
> spice_server_playback_start(st->sin);
> }
> snd_playback_send(worker->connection);
> - return;
> -
> -error_2:
> - celt051_encoder_destroy(celt_encoder);
> -
> -error_1:
> - celt051_mode_destroy(celt_mode);
> }
>
> static void snd_record_migrate_channel_client(RedChannelClient *rcc)
> @@ -1380,9 +1359,7 @@ static void on_new_record_channel(SndWorker *worker)
> static void snd_record_cleanup(SndChannel *channel)
> {
> RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
> -
> - celt051_decoder_destroy(record_channel->celt_decoder);
> - celt051_mode_destroy(record_channel->celt_mode);
> + snd_codec_destroy(&record_channel->codec);
> }
>
> static void snd_set_record_peer(RedChannel *channel, RedClient *client, RedsStream *stream,
> @@ -1392,24 +1369,9 @@ static void snd_set_record_peer(RedChannel *channel, RedClient *client, RedsStre
> SndWorker *worker = channel->data;
> RecordChannel *record_channel;
> SpiceRecordState *st = SPICE_CONTAINEROF(worker, SpiceRecordState, worker);
> - CELTDecoder *celt_decoder;
> - CELTMode *celt_mode;
> - int celt_error;
>
> snd_disconnect_channel(worker->connection);
>
> - if (!(celt_mode = celt051_mode_create(SPICE_INTERFACE_RECORD_FREQ,
> - SPICE_INTERFACE_RECORD_CHAN,
> - FRAME_SIZE, &celt_error))) {
> - spice_printerr("create celt mode failed %d", celt_error);
> - return;
> - }
> -
> - if (!(celt_decoder = celt051_decoder_create(celt_mode))) {
> - spice_printerr("create celt decoder failed");
> - goto error_1;
> - }
> -
> if (!(record_channel = (RecordChannel *)__new_channel(worker,
> sizeof(*record_channel),
> SPICE_CHANNEL_RECORD,
> @@ -1422,26 +1384,29 @@ static void snd_set_record_peer(RedChannel *channel, RedClient *client, RedsStre
> snd_record_cleanup,
> common_caps, num_common_caps,
> caps, num_caps))) {
> - goto error_2;
> + return;
> }
>
> - worker->connection = &record_channel->base;
> + 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");
> + }
> + }
>
> - record_channel->celt_mode = celt_mode;
> - record_channel->celt_decoder = celt_decoder;
> + worker->connection = &record_channel->base;
>
> on_new_record_channel(worker);
> if (worker->active) {
> spice_server_record_start(st->sin);
> }
> snd_record_send(worker->connection);
> - return;
> -
> -error_2:
> - celt051_decoder_destroy(celt_decoder);
> -
> -error_1:
> - celt051_mode_destroy(celt_mode);
> }
>
> static void snd_playback_migrate_channel_client(RedChannelClient *rcc)
> @@ -1498,7 +1463,10 @@ void snd_attach_playback(SpicePlaybackInstance *sin)
> client_cbs.migrate = snd_playback_migrate_channel_client;
> red_channel_register_client_cbs(channel, &client_cbs);
> red_channel_set_data(channel, playback_worker);
> - red_channel_set_cap(channel, SPICE_PLAYBACK_CAP_CELT_0_5_1);
> +
> + 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);
> +
> red_channel_set_cap(channel, SPICE_PLAYBACK_CAP_VOLUME);
>
> playback_worker->base_channel = channel;
> @@ -1525,7 +1493,8 @@ void snd_attach_record(SpiceRecordInstance *sin)
> client_cbs.migrate = snd_record_migrate_channel_client;
> red_channel_register_client_cbs(channel, &client_cbs);
> red_channel_set_data(channel, record_worker);
> - red_channel_set_cap(channel, SPICE_RECORD_CAP_CELT_0_5_1);
> + 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);
> red_channel_set_cap(channel, SPICE_RECORD_CAP_VOLUME);
>
> record_worker->base_channel = channel;
> @@ -1568,22 +1537,19 @@ void snd_detach_record(SpiceRecordInstance *sin)
> spice_record_state_free(sin->st);
> }
>
> -void snd_set_playback_compression(int on)
> +void snd_set_playback_compression(int comp)
> {
> SndWorker *now = workers;
>
> - playback_compression = on ? SPICE_AUDIO_DATA_MODE_CELT_0_5_1 : SPICE_AUDIO_DATA_MODE_RAW;
> + playback_compression = comp;
> +
> + int desired_mode = snd_desired_audio_mode(TRUE);
> +
> for (; now; now = now->next) {
> if (now->base_channel->type == SPICE_CHANNEL_PLAYBACK && now->connection) {
> - SndChannel* sndchannel = now->connection;
> PlaybackChannel* playback = (PlaybackChannel*)now->connection;
> - if (!red_channel_client_test_remote_cap(sndchannel->channel_client,
> - SPICE_PLAYBACK_CAP_CELT_0_5_1)) {
Shouldn't we keep the cap test when desired_mode != SPICE_AUDIO_DATA_MODE_RAW ?
> - spice_assert(playback->mode == SPICE_AUDIO_DATA_MODE_RAW);
> - continue;
> - }
> - if (playback->mode != playback_compression) {
> - playback->mode = playback_compression;
> + if (playback->mode != desired_mode) {
> + playback->mode = desired_mode;
> snd_set_command(now->connection, SND_PLAYBACK_MODE_MASK);
> }
> }
> @@ -1592,5 +1558,5 @@ void snd_set_playback_compression(int on)
>
> int snd_get_playback_compression(void)
> {
> - return (playback_compression == SPICE_AUDIO_DATA_MODE_RAW) ? FALSE : TRUE;
> + return playback_compression;
> }
> diff --git a/server/snd_worker.h b/server/snd_worker.h
> index 8de746d..1448eab 100644
> --- a/server/snd_worker.h
> +++ b/server/snd_worker.h
> @@ -18,6 +18,10 @@
> #ifndef _H_SND_WORKER
> #define _H_SND_WORKER
>
> +#define SPICE_PLAYBACK_COMPRESSION_NONE 0
> +#define SPICE_PLAYBACK_COMPRESSION_AUTO 1
> +#define SPICE_PLAYBACK_COMPRESSION_CELT051 2
> +
> #include "spice.h"
>
> void snd_attach_playback(SpicePlaybackInstance *sin);
> --
> 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/20131016/7670c8c0/attachment-0001.pgp>
More information about the Spice-devel
mailing list