[Spice-devel,spice-server,v2] sound: Store only playback channels in global list

Submitted by Frediano Ziglio on May 3, 2017, 10:27 a.m.

Details

Message ID 20170503102758.12998-1-fziglio@redhat.com
State New
Headers show
Series "sound: Store only playback channels in global list" ( rev: 3 ) in Spice

Not browsing as part of any series.

Commit Message

Frediano Ziglio May 3, 2017, 10:27 a.m.
The list is used only to iterate playback channels

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 server/sound.c | 48 +++++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 23 deletions(-)

Changes since v1:
- rename function dealing with the global list;
- update code comment;
- use PlaybackChannel as parameter.

Patch hide | download patch | download mbox

diff --git a/server/sound.c b/server/sound.c
index be7e607..ebb61a5 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -233,8 +233,8 @@  typedef struct RecordChannelClientClass {
 G_DEFINE_TYPE(RecordChannelClient, record_channel_client, TYPE_SND_CHANNEL_CLIENT)
 
 
-/* A list of all Spice{Playback,Record}State objects */
-static GList *snd_channels;
+/* A list of all PlaybackChannel objects */
+static GList *snd_playback_channels;
 
 static void snd_send(SndChannelClient * client);
 
@@ -980,12 +980,10 @@  void snd_set_playback_latency(RedClient *client, uint32_t latency)
 {
     GList *l;
 
-    for (l = snd_channels; l != NULL; l = l->next) {
-        SndChannel *now = l->data;
-        SndChannelClient *scc = snd_channel_get_client(now);
-        uint32_t type;
-        g_object_get(RED_CHANNEL(now), "channel-type", &type, NULL);
-        if (type == SPICE_CHANNEL_PLAYBACK && scc &&
+    for (l = snd_playback_channels; l != NULL; l = l->next) {
+        PlaybackChannel *now = l->data;
+        SndChannelClient *scc = snd_channel_get_client(SND_CHANNEL(now));
+        if (scc &&
             red_channel_client_get_client(RED_CHANNEL_CLIENT(scc)) == client) {
 
             if (red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(scc),
@@ -1283,14 +1281,14 @@  static void snd_set_record_peer(RedChannel *red_channel, RedClient *client, Reds
                  TYPE_RECORD_CHANNEL_CLIENT);
 }
 
-static void add_channel(SndChannel *channel)
+static void add_playback_channel(PlaybackChannel *channel)
 {
-    snd_channels = g_list_prepend(snd_channels, channel);
+    snd_playback_channels = g_list_prepend(snd_playback_channels, channel);
 }
 
-static void remove_channel(SndChannel *channel)
+static void remove_playback_channel(PlaybackChannel *channel)
 {
-    snd_channels = g_list_remove(snd_channels, channel);
+    snd_playback_channels = g_list_remove(snd_playback_channels, channel);
 }
 
 static void
@@ -1304,8 +1302,6 @@  snd_channel_finalize(GObject *object)
 {
     SndChannel *channel = SND_CHANNEL(object);
 
-    remove_channel(channel);
-
     free(channel->volume.volume);
     channel->volume.volume = NULL;
 
@@ -1346,17 +1342,26 @@  playback_channel_constructed(GObject *object)
     }
     red_channel_set_cap(RED_CHANNEL(self), SPICE_PLAYBACK_CAP_VOLUME);
 
-    add_channel(self);
+    add_playback_channel(PLAYBACK_CHANNEL(self));
     reds_register_channel(reds, RED_CHANNEL(self));
 }
 
 static void
+playback_channel_finalize(GObject *object)
+{
+    remove_playback_channel(PLAYBACK_CHANNEL(object));
+
+    G_OBJECT_CLASS(playback_channel_parent_class)->finalize(object);
+}
+
+static void
 playback_channel_class_init(PlaybackChannelClass *klass)
 {
     GObjectClass *object_class = G_OBJECT_CLASS(klass);
     RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
 
     object_class->constructed = playback_channel_constructed;
+    object_class->finalize = playback_channel_finalize;
 
     channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_PLAYBACK, NULL);
     channel_class->handle_message = red_channel_client_handle_message;
@@ -1396,7 +1401,6 @@  record_channel_constructed(GObject *object)
     }
     red_channel_set_cap(RED_CHANNEL(self), SPICE_RECORD_CAP_VOLUME);
 
-    add_channel(self);
     reds_register_channel(reds, RED_CHANNEL(self));
 }
 
@@ -1448,19 +1452,17 @@  void snd_set_playback_compression(bool on)
 {
     GList *l;
 
-    for (l = snd_channels; l != NULL; l = l->next) {
-        SndChannel *now = l->data;
-        SndChannelClient *client = snd_channel_get_client(now);
-        uint32_t type;
-        g_object_get(RED_CHANNEL(now), "channel-type", &type, NULL);
-        if (type == SPICE_CHANNEL_PLAYBACK && client) {
+    for (l = snd_playback_channels; l != NULL; l = l->next) {
+        PlaybackChannel *now = l->data;
+        SndChannelClient *client = snd_channel_get_client(SND_CHANNEL(now));
+        if (client) {
             PlaybackChannelClient* playback = PLAYBACK_CHANNEL_CLIENT(client);
             RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback);
             bool client_can_celt = red_channel_client_test_remote_cap(rcc,
                                     SPICE_PLAYBACK_CAP_CELT_0_5_1);
             bool client_can_opus = red_channel_client_test_remote_cap(rcc,
                                     SPICE_PLAYBACK_CAP_OPUS);
-            int desired_mode = snd_desired_audio_mode(on, now->frequency,
+            int desired_mode = snd_desired_audio_mode(on, SND_CHANNEL(now)->frequency,
                                                       client_can_opus, client_can_celt);
             if (playback->mode != desired_mode) {
                 playback->mode = desired_mode;

Comments

Hey,

Most of my comments from
https://lists.freedesktop.org/archives/spice-devel/2017-May/037413.html
are still relevant.

Christophe

On Wed, May 03, 2017 at 11:27:58AM +0100, Frediano Ziglio wrote:
> The list is used only to iterate playback channels
> 
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>  server/sound.c | 48 +++++++++++++++++++++++++-----------------------
>  1 file changed, 25 insertions(+), 23 deletions(-)
> 
> Changes since v1:
> - rename function dealing with the global list;
> - update code comment;
> - use PlaybackChannel as parameter.
> 
> diff --git a/server/sound.c b/server/sound.c
> index be7e607..ebb61a5 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -233,8 +233,8 @@ typedef struct RecordChannelClientClass {
>  G_DEFINE_TYPE(RecordChannelClient, record_channel_client, TYPE_SND_CHANNEL_CLIENT)
>  
>  
> -/* A list of all Spice{Playback,Record}State objects */
> -static GList *snd_channels;
> +/* A list of all PlaybackChannel objects */
> +static GList *snd_playback_channels;
>  
>  static void snd_send(SndChannelClient * client);
>  
> @@ -980,12 +980,10 @@ void snd_set_playback_latency(RedClient *client, uint32_t latency)
>  {
>      GList *l;
>  
> -    for (l = snd_channels; l != NULL; l = l->next) {
> -        SndChannel *now = l->data;
> -        SndChannelClient *scc = snd_channel_get_client(now);
> -        uint32_t type;
> -        g_object_get(RED_CHANNEL(now), "channel-type", &type, NULL);
> -        if (type == SPICE_CHANNEL_PLAYBACK && scc &&
> +    for (l = snd_playback_channels; l != NULL; l = l->next) {
> +        PlaybackChannel *now = l->data;
> +        SndChannelClient *scc = snd_channel_get_client(SND_CHANNEL(now));
> +        if (scc &&
>              red_channel_client_get_client(RED_CHANNEL_CLIENT(scc)) == client) {
>  
>              if (red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(scc),
> @@ -1283,14 +1281,14 @@ static void snd_set_record_peer(RedChannel *red_channel, RedClient *client, Reds
>                   TYPE_RECORD_CHANNEL_CLIENT);
>  }
>  
> -static void add_channel(SndChannel *channel)
> +static void add_playback_channel(PlaybackChannel *channel)
>  {
> -    snd_channels = g_list_prepend(snd_channels, channel);
> +    snd_playback_channels = g_list_prepend(snd_playback_channels, channel);
>  }
>  
> -static void remove_channel(SndChannel *channel)
> +static void remove_playback_channel(PlaybackChannel *channel)
>  {
> -    snd_channels = g_list_remove(snd_channels, channel);
> +    snd_playback_channels = g_list_remove(snd_playback_channels, channel);
>  }
>  
>  static void
> @@ -1304,8 +1302,6 @@ snd_channel_finalize(GObject *object)
>  {
>      SndChannel *channel = SND_CHANNEL(object);
>  
> -    remove_channel(channel);
> -
>      free(channel->volume.volume);
>      channel->volume.volume = NULL;
>  
> @@ -1346,17 +1342,26 @@ playback_channel_constructed(GObject *object)
>      }
>      red_channel_set_cap(RED_CHANNEL(self), SPICE_PLAYBACK_CAP_VOLUME);
>  
> -    add_channel(self);
> +    add_playback_channel(PLAYBACK_CHANNEL(self));
>      reds_register_channel(reds, RED_CHANNEL(self));
>  }
>  
>  static void
> +playback_channel_finalize(GObject *object)
> +{
> +    remove_playback_channel(PLAYBACK_CHANNEL(object));
> +
> +    G_OBJECT_CLASS(playback_channel_parent_class)->finalize(object);
> +}
> +
> +static void
>  playback_channel_class_init(PlaybackChannelClass *klass)
>  {
>      GObjectClass *object_class = G_OBJECT_CLASS(klass);
>      RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
>  
>      object_class->constructed = playback_channel_constructed;
> +    object_class->finalize = playback_channel_finalize;
>  
>      channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_PLAYBACK, NULL);
>      channel_class->handle_message = red_channel_client_handle_message;
> @@ -1396,7 +1401,6 @@ record_channel_constructed(GObject *object)
>      }
>      red_channel_set_cap(RED_CHANNEL(self), SPICE_RECORD_CAP_VOLUME);
>  
> -    add_channel(self);
>      reds_register_channel(reds, RED_CHANNEL(self));
>  }
>  
> @@ -1448,19 +1452,17 @@ void snd_set_playback_compression(bool on)
>  {
>      GList *l;
>  
> -    for (l = snd_channels; l != NULL; l = l->next) {
> -        SndChannel *now = l->data;
> -        SndChannelClient *client = snd_channel_get_client(now);
> -        uint32_t type;
> -        g_object_get(RED_CHANNEL(now), "channel-type", &type, NULL);
> -        if (type == SPICE_CHANNEL_PLAYBACK && client) {
> +    for (l = snd_playback_channels; l != NULL; l = l->next) {
> +        PlaybackChannel *now = l->data;
> +        SndChannelClient *client = snd_channel_get_client(SND_CHANNEL(now));
> +        if (client) {
>              PlaybackChannelClient* playback = PLAYBACK_CHANNEL_CLIENT(client);
>              RedChannelClient *rcc = RED_CHANNEL_CLIENT(playback);
>              bool client_can_celt = red_channel_client_test_remote_cap(rcc,
>                                      SPICE_PLAYBACK_CAP_CELT_0_5_1);
>              bool client_can_opus = red_channel_client_test_remote_cap(rcc,
>                                      SPICE_PLAYBACK_CAP_OPUS);
> -            int desired_mode = snd_desired_audio_mode(on, now->frequency,
> +            int desired_mode = snd_desired_audio_mode(on, SND_CHANNEL(now)->frequency,
>                                                        client_can_opus, client_can_celt);
>              if (playback->mode != desired_mode) {
>                  playback->mode = desired_mode;
> -- 
> 2.9.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel