[Spice-commits] 2 commits - server/sound.cpp

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Fri Jun 19 13:24:09 UTC 2020


 server/sound.cpp |   49 +++++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

New commits:
commit 43c7cb167e000595964a864646c223a71249df26
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Fri Jun 5 10:08:19 2020 +0100

    sound: Better management of PersistentPipeItem life
    
    The handling of that specific pipe item reference counting
    was a bit confusing. In particular every time before adding to
    the queue the item was reset causing the reference counter to
    be 1 however it was not sure that the item was not still referenced
    when added again to the queue.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Julien Ropé <jrope at gmail.com>

diff --git a/server/sound.cpp b/server/sound.cpp
index 39a748e9..bfcf5124 100644
--- a/server/sound.cpp
+++ b/server/sound.cpp
@@ -76,6 +76,9 @@ struct AudioFrameContainer;
 
 struct PersistentPipeItem: public RedPipeItem
 {
+    PersistentPipeItem();
+private:
+    static void item_free(RedPipeItem *item);
 };
 
 /* Connects an audio client to a Spice client */
@@ -607,6 +610,11 @@ static bool playback_send_mode(PlaybackChannelClient *playback_client)
     return true;
 }
 
+PersistentPipeItem::PersistentPipeItem()
+{
+    red_pipe_item_init_full(this, RED_PIPE_ITEM_PERSISTENT, item_free);
+}
+
 /* 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.
@@ -616,10 +624,8 @@ static bool playback_send_mode(PlaybackChannelClient *playback_client)
  * much data or having retransmission preferring instead loosing some
  * samples.
  */
-static void snd_persistent_pipe_item_free(struct RedPipeItem *item)
+void PersistentPipeItem::item_free(RedPipeItem *item)
 {
-    red_pipe_item_init_full(item, RED_PIPE_ITEM_PERSISTENT,
-                            snd_persistent_pipe_item_free);
 }
 
 static void snd_send(SndChannelClient * client)
@@ -628,8 +634,7 @@ static void snd_send(SndChannelClient * client)
         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_pipe_item_ref(&client->persistent_pipe_item);
     client->pipe_add_push(&client->persistent_pipe_item);
 }
 
commit 66ee75a810c616bf10adace3a89b2ef75becc980
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Fri Jun 5 09:17:59 2020 +0100

    sound: Do not depend on pipe item remove to detect data message sent
    
    The assumption is a bit fragile as code could change adding
    additional references to the pipe item.
    Instead use marshaller cleanup routines, this surely will be
    triggered when the message is queued to the network.
    This also causes some additional cleanups.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Julien Ropé <jrope at gmail.com>

diff --git a/server/sound.cpp b/server/sound.cpp
index 9f9aed0b..39a748e9 100644
--- a/server/sound.cpp
+++ b/server/sound.cpp
@@ -76,7 +76,6 @@ struct AudioFrameContainer;
 
 struct PersistentPipeItem: public RedPipeItem
 {
-    SndChannelClient *client;
 };
 
 /* Connects an audio client to a Spice client */
@@ -92,8 +91,6 @@ public:
 
     PersistentPipeItem persistent_pipe_item;
 
-    virtual void on_message_done() {};
-
     inline SndChannel* get_channel();
 
     virtual bool config_socket() override;
@@ -149,9 +146,10 @@ public:
     uint32_t latency = 0;
     SndCodec codec = nullptr;
     uint8_t  encode_buf[SND_CODEC_MAX_COMPRESSED_BYTES];
+
+    static void on_message_marshalled(uint8_t *data, void *opaque);
 protected:
     virtual void send_item(RedPipeItem *item) override;
-    virtual void on_message_done() override;
 };
 
 typedef struct SpiceVolumeState {
@@ -243,14 +241,16 @@ static void snd_playback_free_frame(PlaybackChannelClient *playback_client, Audi
     playback_client->free_frames = frame;
 }
 
-void PlaybackChannelClient::on_message_done()
+void PlaybackChannelClient::on_message_marshalled(uint8_t *, void *opaque)
 {
-    if (in_progress) {
-        snd_playback_free_frame(this, in_progress);
-        in_progress = NULL;
-        if (pending_frame) {
-            command |= SND_PLAYBACK_PCM_MASK;
-            snd_send(this);
+    PlaybackChannelClient *client = reinterpret_cast<PlaybackChannelClient*>(opaque);
+
+    if (client->in_progress) {
+        snd_playback_free_frame(client, client->in_progress);
+        client->in_progress = NULL;
+        if (client->pending_frame) {
+            client->command |= SND_PLAYBACK_PCM_MASK;
+            snd_send(client);
         }
     }
 }
@@ -559,7 +559,6 @@ static bool snd_playback_send_write(PlaybackChannelClient *playback_client)
     SpiceMarshaller *m = rcc->get_marshaller();
     AudioFrame *frame;
     SpiceMsgPlaybackPacket msg;
-    RedPipeItem *pipe_item = &playback_client->persistent_pipe_item;
 
     rcc->init_send_data(SPICE_MSG_PLAYBACK_DATA);
 
@@ -572,7 +571,8 @@ static bool snd_playback_send_write(PlaybackChannelClient *playback_client)
         spice_marshaller_add_by_ref_full(m, (uint8_t *)frame->samples,
                                          snd_codec_frame_size(playback_client->codec) *
                                          sizeof(frame->samples[0]),
-                                         marshaller_unref_pipe_item, pipe_item);
+                                         PlaybackChannelClient::on_message_marshalled,
+                                         playback_client);
     }
     else {
         int n = sizeof(playback_client->encode_buf);
@@ -584,7 +584,8 @@ static bool snd_playback_send_write(PlaybackChannelClient *playback_client)
             return false;
         }
         spice_marshaller_add_by_ref_full(m, playback_client->encode_buf, n,
-                                         marshaller_unref_pipe_item, pipe_item);
+                                         PlaybackChannelClient::on_message_marshalled,
+                                         playback_client);
     }
 
     rcc->begin_send_message();
@@ -617,12 +618,8 @@ static bool playback_send_mode(PlaybackChannelClient *playback_client)
  */
 static void snd_persistent_pipe_item_free(struct RedPipeItem *item)
 {
-    SndChannelClient *client = static_cast<PersistentPipeItem*>(item)->client;
-
     red_pipe_item_init_full(item, RED_PIPE_ITEM_PERSISTENT,
                             snd_persistent_pipe_item_free);
-
-    client->on_message_done();
 }
 
 static void snd_send(SndChannelClient * client)
@@ -633,7 +630,6 @@ static void snd_send(SndChannelClient * client)
     // 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);
-    client->persistent_pipe_item.client = client;
     client->pipe_add_push(&client->persistent_pipe_item);
 }
 


More information about the Spice-commits mailing list