[Spice-commits] 4 commits - server/sound.c

Frediano Ziglio fziglio at kemper.freedesktop.org
Fri Dec 2 21:58:26 UTC 2016


 server/sound.c |  136 ++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 86 insertions(+), 50 deletions(-)

New commits:
commit f191fb4717156300a7fc5c02952c01b1fb2aef1a
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Fri Dec 2 10:27:07 2016 +0000

    sound: Reuse client variable if available
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/sound.c b/server/sound.c
index 603e29b..870d9b9 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -1125,15 +1125,15 @@ SPICE_GNUC_VISIBLE void spice_server_playback_stop(SpicePlaybackInstance *sin)
     sin->st->channel.active = 0;
     if (!client)
         return;
-    spice_assert(playback_client->base.active);
+    spice_assert(client->active);
     reds_enable_mm_time(snd_channel_get_server(client));
-    playback_client->base.active = FALSE;
-    if (playback_client->base.client_active) {
-        snd_set_command(&playback_client->base, SND_CTRL_MASK);
-        snd_playback_send(&playback_client->base);
+    client->active = FALSE;
+    if (client->client_active) {
+        snd_set_command(client, SND_CTRL_MASK);
+        snd_playback_send(client);
     } else {
-        playback_client->base.command &= ~SND_CTRL_MASK;
-        playback_client->base.command &= ~SND_PLAYBACK_PCM_MASK;
+        client->command &= ~SND_CTRL_MASK;
+        client->command &= ~SND_PLAYBACK_PCM_MASK;
 
         if (playback_client->pending_frame) {
             spice_assert(!playback_client->in_progress);
@@ -1155,7 +1155,7 @@ SPICE_GNUC_VISIBLE void spice_server_playback_get_buffer(SpicePlaybackInstance *
         *num_samples = 0;
         return;
     }
-    spice_assert(playback_client->base.active);
+    spice_assert(client->active);
     if (!playback_client->free_frames->allocated) {
         playback_client->free_frames->allocated = TRUE;
         ++playback_client->frames->refs;
@@ -1267,7 +1267,7 @@ static void snd_playback_cleanup(SndChannelClient *client)
         free(playback_client->frames);
     }
 
-    if (playback_client->base.active) {
+    if (client->active) {
         reds_enable_mm_time(snd_channel_get_server(client));
     }
 
@@ -1403,18 +1403,17 @@ SPICE_GNUC_VISIBLE void spice_server_record_start(SpiceRecordInstance *sin)
 SPICE_GNUC_VISIBLE void spice_server_record_stop(SpiceRecordInstance *sin)
 {
     SndChannelClient *client = sin->st->channel.connection;
-    RecordChannelClient *record_client = SPICE_CONTAINEROF(client, RecordChannelClient, base);
 
     sin->st->channel.active = 0;
     if (!client)
         return;
-    spice_assert(record_client->base.active);
-    record_client->base.active = FALSE;
-    if (record_client->base.client_active) {
-        snd_set_command(&record_client->base, SND_CTRL_MASK);
-        snd_record_send(&record_client->base);
+    spice_assert(client->active);
+    client->active = FALSE;
+    if (client->client_active) {
+        snd_set_command(client, SND_CTRL_MASK);
+        snd_record_send(client);
     } else {
-        record_client->base.command &= ~SND_CTRL_MASK;
+        client->command &= ~SND_CTRL_MASK;
     }
 }
 
@@ -1429,7 +1428,7 @@ SPICE_GNUC_VISIBLE uint32_t spice_server_record_get_samples(SpiceRecordInstance
 
     if (!client)
         return 0;
-    spice_assert(record_client->base.active);
+    spice_assert(client->active);
 
     if (record_client->write_pos < RECORD_SAMPLES_SIZE / 2) {
         return 0;
@@ -1438,8 +1437,8 @@ SPICE_GNUC_VISIBLE uint32_t spice_server_record_get_samples(SpiceRecordInstance
     len = MIN(record_client->write_pos - record_client->read_pos, bufsize);
 
     if (len < bufsize) {
-        SndChannel *channel = record_client->base.channel;
-        snd_receive(&record_client->base);
+        SndChannel *channel = client->channel;
+        snd_receive(client);
         if (!channel->connection) {
             return 0;
         }
commit 815da98f45af0dd28699adf0d5026ea172b50401
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Fri Dec 2 10:26:51 2016 +0000

    sound: Introduce a macro to cast to SndChannelClient
    
    This make easier the transition to GObject.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/sound.c b/server/sound.c
index fbe4b4c..603e29b 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -87,6 +87,8 @@ typedef int (*snd_channel_handle_message_proc)(SndChannelClient *client, size_t
 typedef void (*snd_channel_on_message_done_proc)(SndChannelClient *client);
 typedef void (*snd_channel_cleanup_channel_proc)(SndChannelClient *client);
 
+#define SND_CHANNEL_CLIENT(obj) (&(obj)->base)
+
 /* Connects an audio client to a Spice client */
 struct SndChannelClient {
     RedsStream *stream;
@@ -608,7 +610,7 @@ static int snd_channel_send_migrate(SndChannelClient *client)
 
 static int snd_playback_send_migrate(PlaybackChannelClient *client)
 {
-    return snd_channel_send_migrate(&client->base);
+    return snd_channel_send_migrate(SND_CHANNEL_CLIENT(client));
 }
 
 static int snd_send_volume(SndChannelClient *client, uint32_t cap, int msg)
@@ -637,7 +639,7 @@ static int snd_send_volume(SndChannelClient *client, uint32_t cap, int msg)
 
 static int snd_playback_send_volume(PlaybackChannelClient *playback_client)
 {
-    return snd_send_volume(&playback_client->base, SPICE_PLAYBACK_CAP_VOLUME,
+    return snd_send_volume(SND_CHANNEL_CLIENT(playback_client), SPICE_PLAYBACK_CAP_VOLUME,
                            SPICE_MSG_PLAYBACK_VOLUME);
 }
 
@@ -661,13 +663,13 @@ static int snd_send_mute(SndChannelClient *client, uint32_t cap, int msg)
 
 static int snd_playback_send_mute(PlaybackChannelClient *playback_client)
 {
-    return snd_send_mute(&playback_client->base, SPICE_PLAYBACK_CAP_VOLUME,
+    return snd_send_mute(SND_CHANNEL_CLIENT(playback_client), SPICE_PLAYBACK_CAP_VOLUME,
                          SPICE_MSG_PLAYBACK_MUTE);
 }
 
 static int snd_playback_send_latency(PlaybackChannelClient *playback_client)
 {
-    SndChannelClient *client = &playback_client->base;
+    SndChannelClient *client = SND_CHANNEL_CLIENT(playback_client);
     SpiceMsgPlaybackLatency latency_msg;
 
     spice_debug("latency %u", playback_client->latency);
@@ -711,7 +713,7 @@ static int snd_playback_send_stop(PlaybackChannelClient *playback_client)
 
 static int snd_playback_send_ctl(PlaybackChannelClient *playback_client)
 {
-    SndChannelClient *client = (SndChannelClient *)playback_client;
+    SndChannelClient *client = SND_CHANNEL_CLIENT(playback_client);
 
     if ((client->client_active = client->active)) {
         return snd_playback_send_start(playback_client);
@@ -751,7 +753,7 @@ static int snd_record_send_stop(RecordChannelClient *record_client)
 
 static int snd_record_send_ctl(RecordChannelClient *record_client)
 {
-    SndChannelClient *client = (SndChannelClient *)record_client;
+    SndChannelClient *client = SND_CHANNEL_CLIENT(record_client);
 
     if ((client->client_active = client->active)) {
         return snd_record_send_start(record_client);
@@ -762,13 +764,13 @@ static int snd_record_send_ctl(RecordChannelClient *record_client)
 
 static int snd_record_send_volume(RecordChannelClient *record_client)
 {
-    return snd_send_volume(&record_client->base, SPICE_RECORD_CAP_VOLUME,
+    return snd_send_volume(SND_CHANNEL_CLIENT(record_client), SPICE_RECORD_CAP_VOLUME,
                            SPICE_MSG_RECORD_VOLUME);
 }
 
 static int snd_record_send_mute(RecordChannelClient *record_client)
 {
-    return snd_send_mute(&record_client->base, SPICE_RECORD_CAP_VOLUME,
+    return snd_send_mute(SND_CHANNEL_CLIENT(record_client), SPICE_RECORD_CAP_VOLUME,
                          SPICE_MSG_RECORD_MUTE);
 }
 
@@ -778,7 +780,7 @@ static int snd_record_send_migrate(RecordChannelClient *record_client)
      * the client receives RECORD_STOP from the src before the migration completion
      * notification (when the vm is stopped).
      * Afterwards, when the vm starts on the dest, the client receives RECORD_START. */
-    return snd_channel_send_migrate(&record_client->base);
+    return snd_channel_send_migrate(SND_CHANNEL_CLIENT(record_client));
 }
 
 static int snd_playback_send_write(PlaybackChannelClient *playback_client)
@@ -834,7 +836,7 @@ static int playback_send_mode(PlaybackChannelClient *playback_client)
 static void snd_playback_send(void* data)
 {
     PlaybackChannelClient *playback_client = (PlaybackChannelClient*)data;
-    SndChannelClient *client = (SndChannelClient*)playback_client;
+    SndChannelClient *client = SND_CHANNEL_CLIENT(playback_client);
 
     if (!playback_client || !snd_send_data(data)) {
         return;
@@ -888,7 +890,7 @@ static void snd_playback_send(void* data)
 static void snd_record_send(void* data)
 {
     RecordChannelClient *record_client = (RecordChannelClient*)data;
-    SndChannelClient *client = (SndChannelClient*)record_client;
+    SndChannelClient *client = SND_CHANNEL_CLIENT(record_client);
 
     if (!record_client || !snd_send_data(data)) {
         return;
@@ -1178,21 +1180,20 @@ SPICE_GNUC_VISIBLE void spice_server_playback_put_samples(SpicePlaybackInstance
         }
     }
     playback_client = frame->client;
-    if (!playback_client ||
-        sin->st->channel.connection != &playback_client->base) {
+    if (!playback_client || sin->st->channel.connection != SND_CHANNEL_CLIENT(playback_client)) {
         /* lost last reference, client has been destroyed previously */
         spice_info("audio samples belong to a disconnected client");
         return;
     }
-    spice_assert(playback_client->base.active);
+    spice_assert(SND_CHANNEL_CLIENT(playback_client)->active);
 
     if (playback_client->pending_frame) {
         snd_playback_free_frame(playback_client, playback_client->pending_frame);
     }
     frame->time = reds_get_mm_time();
     playback_client->pending_frame = frame;
-    snd_set_command(&playback_client->base, SND_PLAYBACK_PCM_MASK);
-    snd_playback_send(&playback_client->base);
+    snd_set_command(SND_CHANNEL_CLIENT(playback_client), SND_PLAYBACK_PCM_MASK);
+    snd_playback_send(SND_CHANNEL_CLIENT(playback_client));
 }
 
 void snd_set_playback_latency(RedClient *client, uint32_t latency)
@@ -1318,7 +1319,7 @@ static void snd_set_playback_peer(RedChannel *red_channel, RedClient *client, Re
     }
 
     if (!red_client_during_migrate_at_target(client)) {
-        on_new_playback_channel(channel, &playback_client->base);
+        on_new_playback_channel(channel, SND_CHANNEL_CLIENT(playback_client));
     }
 
     if (channel->active) {
@@ -1542,7 +1543,7 @@ static void snd_set_record_peer(RedChannel *red_channel, RedClient *client, Reds
 
     record_client->mode = SPICE_AUDIO_DATA_MODE_RAW;
 
-    on_new_record_channel(channel, &record_client->base);
+    on_new_record_channel(channel, SND_CHANNEL_CLIENT(record_client));
     if (channel->active) {
         snd_record_start(channel);
     }
commit 852202a0497bd42e238fe338a93d8cf073f9c8d1
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Fri Dec 2 09:57:21 2016 +0000

    sound: Change AudioFrame allocation
    
    When qemu (for example) delivers audio samples to the spice server, it
    does so by requesting a buffer from the spice server
    (spice_server_playback_get_buffer()), filling them with audio data, and
    then calling spice_server_playback_put_samples() to send them to the
    client. Between the call to _get_buffer() and the call to
    _put_samples(), we need to ensure that the buffer remains valid. Since
    this buffer is allocated within PlaybackChannelClient, we did this by
    incrementing a ref on the PlaybackChannelClient in _get_buffer(), and
    decrementing the ref in _put_samples(). This has the effect of
    potentially keeping the PlaybackChannelClient alive after the spice
    client has disconnected.
    
    This was not a problem when PlaybackChannelClient was a simple helper
    class. But we intend to change PlaybackChannelClient so that it
    inherits from RedChannelClient. In that case, the reference taken in
    _get_buffer() would result in the RedChannelClient (and associated
    RedChannel, etc) being kept alive longer than expected. To avoid this,
    we add an additional ref-counted adapter class (AudioFrameContainer)
    that owns the allocated audio frames and can outlive the
    RedChannelClient if necessary. When the client is freed, the
    AudioFrameContainer is just unreferenced.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/sound.c b/server/sound.c
index 9c8f98b..fbe4b4c 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -77,6 +77,8 @@ typedef struct SndChannelClient SndChannelClient;
 typedef struct SndChannel SndChannel;
 typedef struct PlaybackChannelClient PlaybackChannelClient;
 typedef struct RecordChannelClient RecordChannelClient;
+typedef struct AudioFrame AudioFrame;
+typedef struct AudioFrameContainer AudioFrameContainer;
 typedef struct SpicePlaybackState PlaybackChannel;
 typedef struct SpiceRecordState RecordChannel;
 
@@ -126,11 +128,21 @@ struct AudioFrame {
     uint32_t samples[SND_CODEC_MAX_FRAME_SIZE];
     PlaybackChannelClient *client;
     AudioFrame *next;
+    AudioFrameContainer *container;
+    gboolean allocated;
+};
+
+#define NUM_AUDIO_FRAMES 3
+struct AudioFrameContainer
+{
+    int refs;
+    AudioFrame items[NUM_AUDIO_FRAMES];
 };
 
 struct PlaybackChannelClient {
     SndChannelClient base;
-    AudioFrame frames[3];
+
+    AudioFrameContainer *frames;
     AudioFrame *free_frames;
     AudioFrame *in_progress;   /* Frame being sent to the client */
     AudioFrame *pending_frame; /* Next frame to send to the client */
@@ -218,12 +230,7 @@ static SndChannel *snd_channels;
 static void snd_receive(SndChannelClient *client);
 static void snd_playback_start(SndChannel *channel);
 static void snd_record_start(SndChannel *channel);
-
-static SndChannelClient *snd_channel_ref(SndChannelClient *client)
-{
-    client->refs++;
-    return client;
-}
+static void snd_playback_alloc_frames(PlaybackChannelClient *playback);
 
 static SndChannelClient *snd_channel_unref(SndChannelClient *client)
 {
@@ -1147,7 +1154,10 @@ SPICE_GNUC_VISIBLE void spice_server_playback_get_buffer(SpicePlaybackInstance *
         return;
     }
     spice_assert(playback_client->base.active);
-    snd_channel_ref(client);
+    if (!playback_client->free_frames->allocated) {
+        playback_client->free_frames->allocated = TRUE;
+        ++playback_client->frames->refs;
+    }
 
     *frame = playback_client->free_frames->samples;
     playback_client->free_frames = playback_client->free_frames->next;
@@ -1160,9 +1170,15 @@ SPICE_GNUC_VISIBLE void spice_server_playback_put_samples(SpicePlaybackInstance
     AudioFrame *frame;
 
     frame = SPICE_CONTAINEROF(samples, AudioFrame, samples[0]);
+    if (frame->allocated) {
+        frame->allocated = FALSE;
+        if (--frame->container->refs == 0) {
+            free(frame->container);
+            return;
+        }
+    }
     playback_client = frame->client;
-    spice_assert(playback_client);
-    if (!snd_channel_unref(&playback_client->base) ||
+    if (!playback_client ||
         sin->st->channel.connection != &playback_client->base) {
         /* lost last reference, client has been destroyed previously */
         spice_info("audio samples belong to a disconnected client");
@@ -1240,6 +1256,15 @@ static void on_new_playback_channel(SndChannel *channel, SndChannelClient *snd_c
 static void snd_playback_cleanup(SndChannelClient *client)
 {
     PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, PlaybackChannelClient, base);
+    int i;
+
+    // free frames, unref them
+    for (i = 0; i < NUM_AUDIO_FRAMES; ++i) {
+        playback_client->frames->items[i].client = NULL;
+    }
+    if (--playback_client->frames->refs == 0) {
+        free(playback_client->frames);
+    }
 
     if (playback_client->base.active) {
         reds_enable_mm_time(snd_channel_get_server(client));
@@ -1271,9 +1296,8 @@ static void snd_set_playback_peer(RedChannel *red_channel, RedClient *client, Re
                                                                    caps, num_caps))) {
         return;
     }
-    snd_playback_free_frame(playback_client, &playback_client->frames[0]);
-    snd_playback_free_frame(playback_client, &playback_client->frames[1]);
-    snd_playback_free_frame(playback_client, &playback_client->frames[2]);
+
+    snd_playback_alloc_frames(playback_client);
 
     int client_can_celt = red_channel_client_test_remote_cap(playback_client->base.channel_client,
                                           SPICE_PLAYBACK_CAP_CELT_0_5_1);
@@ -1716,3 +1740,15 @@ void snd_set_playback_compression(int on)
         }
     }
 }
+
+static void snd_playback_alloc_frames(PlaybackChannelClient *playback)
+{
+    int i;
+
+    playback->frames = spice_new0(AudioFrameContainer, 1);
+    playback->frames->refs = 1;
+    for (i = 0; i < NUM_AUDIO_FRAMES; ++i) {
+        playback->frames->items[i].container = playback->frames;
+        snd_playback_free_frame(playback, &playback->frames->items[i]);
+    }
+}
commit f301d3efc1bb6be94555b2b2878a8a1acae3d071
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Fri Dec 2 09:39:59 2016 +0000

    sound: Reuse record_client variable
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/sound.c b/server/sound.c
index 912dc7a..9c8f98b 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -418,7 +418,7 @@ static int snd_record_handle_message(SndChannelClient *client, size_t size, uint
     }
     switch (type) {
     case SPICE_MSGC_RECORD_DATA:
-        return snd_record_handle_write((RecordChannelClient *)client, size, message);
+        return snd_record_handle_write(record_client, size, message);
     case SPICE_MSGC_RECORD_MODE: {
         SpiceMsgcRecordMode *mode = (SpiceMsgcRecordMode *)message;
         SndChannel *channel = client->channel;


More information about the Spice-commits mailing list