>
> You can see that SndChannelClient has much less field
> as the code to read/write from/to client is reused from
> RedChannelClient instead of creating a fake RedChannelClient
> just to make the system happy.
>
> One of the different between the old sound code and all other
> RedChannelClient objects was that the sound channel don't use
> a queue while RedChannelClient use RedPipeItem object. This was
> the main reason why RedChannelClient was not used. To implement
> the old behaviour a "persistent_pipe_item" is used. This RedPipeItem
> will be queued to RedChannelClient (only one!) so signal code we
> have data to send. The {playback,record}_channel_send_item will
> then send the messages to the client using RedChannelClient functions.
> For this reason snd_reset_send_data is replaced by a call to
> red_channel_client_init_send_data and snd_begin_send_message is
> replaced by red_channel_client_begin_send_message.
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> Signed-off-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
> ---
> server/sound.c | 659
> +++++++++++++++++++++------------------------------------
> 1 file changed, 245 insertions(+), 414 deletions(-)
>
> diff --git a/server/sound.c b/server/sound.c
> index f27a53d..2bb5688 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -82,8 +82,6 @@ typedef struct AudioFrameContainer AudioFrameContainer;
> typedef struct SpicePlaybackState PlaybackChannel;
> typedef struct SpiceRecordState RecordChannel;
>
> -typedef void (*snd_channel_send_messages_proc)(void *in_channel);
> -typedef int (*snd_channel_handle_message_proc)(SndChannelClient *client,
> size_t size, uint32_t type, void *message);
> typedef void (*snd_channel_on_message_done_proc)(SndChannelClient *client);
> typedef void (*snd_channel_cleanup_channel_proc)(SndChannelClient *client);
>
....
> @@ -994,6 +672,13 @@ static SndChannelClient *__new_channel(SndChannel
> *channel, int size, uint32_t c
> if (!client->channel_client) {
> goto error2;
> }
> +
> + /* SndChannelClient is not yet a RedChannelClient, but we still need to
> go from our
> + * RedChannelClient implementation (DummyChannelClient) to the
> SndChannelClient instance
> + * in various vfuncs
> + */
> + g_object_set_data(G_OBJECT(client->channel_client),
> "sound-channel-client", client);
> +
> if
> (!snd_channel_config_socket(RED_CHANNEL_CLIENT(client->channel_client)))
> {
> goto error2;
> }
> @@ -1005,6 +690,135 @@ error2:
> return NULL;
> }
>
> +/* This function is called when the "persistent" item is removed from the
> + * queue. Note that there is not free call as the item is allocated into
> + * SndChannelClient.
> + * This is used to have a simple item in RedChannelClient queue but to send
> + * multiple messages in a row if possible.
> + * During realtime sound transmission you usually don't want to queue too
> + * much data or having retransmission preferring instead loosing some
> + * samples.
> + */
> +
I would remove this extra empty line
> +static void snd_persistent_pipe_item_free(struct RedPipeItem *item)
> +{
> + SndChannelClient *client = SPICE_CONTAINEROF(item, SndChannelClient,
> persistent_pipe_item);
> +
> + red_pipe_item_init_full(item, RED_PIPE_ITEM_PERSISTENT,
> + snd_persistent_pipe_item_free);
> +
> + if (client->on_message_done) {
> + client->on_message_done(client);
> + }
> +}
> +
> +static void snd_send(SndChannelClient * client)
> +{
> + RedChannelClient *rcc = client->channel_client;
> +
> + if (!client || !red_channel_client_pipe_is_empty(rcc) ||
> !client->command) {
the !client check here is useless after its use
> + return;
> + }
> + // just append a dummy item and push!
> + red_pipe_item_init_full(&client->persistent_pipe_item,
> RED_PIPE_ITEM_PERSISTENT,
> + snd_persistent_pipe_item_free);
> + red_channel_client_pipe_add_push(rcc, &client->persistent_pipe_item);
> +}
> +
> +static void playback_channel_send_item(RedChannelClient *rcc, G_GNUC_UNUSED
> RedPipeItem *item)
> +{
> + SndChannelClient *client = snd_channel_client_from_dummy(rcc);
> + PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client,
> PlaybackChannelClient, base);
> +
> + client->command &= SND_PLAYBACK_MODE_MASK|SND_PLAYBACK_PCM_MASK|
> + SND_CTRL_MASK|SND_VOLUME_MUTE_MASK|
> + SND_MIGRATE_MASK|SND_PLAYBACK_LATENCY_MASK;
> + while (client->command) {
> + if (client->command & SND_PLAYBACK_MODE_MASK) {
> + client->command &= ~SND_PLAYBACK_MODE_MASK;
> + if (playback_send_mode(playback_client)) {
> + break;
> + }
> + }
> + if (client->command & SND_PLAYBACK_PCM_MASK) {
> + spice_assert(!playback_client->in_progress &&
> playback_client->pending_frame);
> + playback_client->in_progress = playback_client->pending_frame;
> + playback_client->pending_frame = NULL;
> + client->command &= ~SND_PLAYBACK_PCM_MASK;
> + if (snd_playback_send_write(playback_client)) {
> + break;
> + }
> + spice_printerr("snd_send_playback_write failed");
> + }
> + if (client->command & SND_CTRL_MASK) {
> + client->command &= ~SND_CTRL_MASK;
> + if (snd_playback_send_ctl(playback_client)) {
> + break;
> + }
> + }
> + if (client->command & SND_VOLUME_MASK) {
> + client->command &= ~SND_VOLUME_MASK;
> + if (snd_playback_send_volume(playback_client)) {
> + break;
> + }
> + }
> + if (client->command & SND_MUTE_MASK) {
> + client->command &= ~SND_MUTE_MASK;
> + if (snd_playback_send_mute(playback_client)) {
> + break;
> + }
> + }
> + if (client->command & SND_MIGRATE_MASK) {
> + client->command &= ~SND_MIGRATE_MASK;
> + if (snd_playback_send_migrate(playback_client)) {
> + break;
> + }
> + }
> + if (client->command & SND_PLAYBACK_LATENCY_MASK) {
> + client->command &= ~SND_PLAYBACK_LATENCY_MASK;
> + if (snd_playback_send_latency(playback_client)) {
> + break;
> + }
> + }
> + }
> + snd_send(client);
> +}
> +
....
Beside that
Acked-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel