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

[Christophe Fergeau]

Now with the right goggles on ;) Sorry for the previous review.

On Wed, May 03, 2017 at 10:47:48AM +0100, Frediano Ziglio wrote:
> The list is used only to iterate playback channels
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/sound.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index be7e607..64cd6d4 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -234,7 +234,7 @@ G_DEFINE_TYPE(RecordChannelClient, record_channel_client, TYPE_SND_CHANNEL_CLIEN
>  
>  
>  /* A list of all Spice{Playback,Record}State objects */
> -static GList *snd_channels;
> +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) {
> +    for (l = snd_playback_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 &&
> +        if (scc &&
>              red_channel_client_get_client(RED_CHANNEL_CLIENT(scc)) == client) {
>  
>              if (red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(scc),

Another option would be to just kill the snd_playback_channels list, and
do something like that instead:


diff --git a/server/reds.c b/server/reds.c
index 2a8f905b5..4e8a7ced1 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -4012,8 +4012,13 @@ static void reds_set_video_codecs(RedsState *reds, GArray *video_codecs)

 SPICE_GNUC_VISIBLE int spice_server_set_playback_compression(SpiceServer *reds, int enable)
 {
+    GList *it;
     reds->config->playback_compression = !!enable;
-    snd_set_playback_compression(enable);
+    for (it = reds->channels; it != NULL; it++) {
+       if (IS_SND_CHANNEL(it->data)) {
+          snd_channel_set_playback_compression(SND_CHANNEL(it->data), enable);
+       }
+    }
     return 0;
 }

(yes, neither snd_channel_set_playback_compression nor IS_SND_CHANNEL exist for now)



> @@ -1285,12 +1283,12 @@ static void snd_set_record_peer(RedChannel *red_channel, RedClient *client, Reds
>  
>  static void add_channel(SndChannel *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)
>  {
> -    snd_channels = g_list_remove(snd_channels, channel);
> +    snd_playback_channels = g_list_remove(snd_playback_channels, channel);
>  }

Since there is only one caller of add_channel/remove_channel, I'd just kill
these helpers, and use direct g_list_xxx calls.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170503/474b496a/attachment.sig>