Re: [PATCH v2 6/6] sound: don't store client in SndChannel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2017-04-24 at 12:09 -0500, Jonathon Jongsma wrote:
> The base RedChannel already keeps a list of channel clients, so
> there's
> no need for the SndChannel to also keep track of the client itself.
> 
> Since the SndChannel only supports a single client (whereas other
> channels may have some partial support for multiple clients), I've
> provided a convenience function for getting the client and warning
> if
> there is more than one.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> ---
>  server/sound.c | 74 +++++++++++++++++++++++++++++++++------------
> -------------
>  1 file changed, 42 insertions(+), 32 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index 543449a..ca83eae 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -166,8 +166,6 @@ GType snd_channel_get_type(void) G_GNUC_CONST;
>  struct SndChannel {
>      RedChannel parent;
>  
> -    SndChannelClient *connection; /* Only one client is supported
> */
> -
>      gboolean active;
>      SpiceVolumeState volume;
>      uint32_t frequency;
> @@ -240,6 +238,20 @@ static GList *snd_channels;
>  
>  static void snd_send(SndChannelClient * client);
>  
> +static SndChannelClient *snd_channel_get_client(SndChannel
> *channel)
> +{
> +    /* sound channels only support a single client */
> +    GList *clients = red_channel_get_clients(RED_CHANNEL(channel));
> +    if (clients == NULL)
> +        return NULL;
> +
> +    if (clients->next != NULL) {
> +        g_warning("Only one sound client is supported");
> +    }
> +
> +    return clients->data;
> +}
> +
>  static RedsState* snd_channel_get_server(SndChannelClient *client)
>  {
>      g_return_val_if_fail(client != NULL, NULL);
> @@ -782,10 +794,6 @@ static bool
> snd_channel_client_config_socket(RedChannelClient *rcc)
>  
>  static void snd_channel_on_disconnect(RedChannelClient *rcc)
>  {
> -    SndChannel *channel =
> SND_CHANNEL(red_channel_client_get_channel(rcc));
> -    if (channel->connection && rcc == RED_CHANNEL_CLIENT(channel-
> >connection)) {
> -        channel->connection = NULL;
> -    }
>  }
>  
>  static uint8_t*
> @@ -821,7 +829,7 @@ static void snd_channel_set_volume(SndChannel
> *channel,
>                                     uint8_t nchannels, uint16_t
> *volume)
>  {
>      SpiceVolumeState *st = &channel->volume;
> -    SndChannelClient *client = channel->connection;
> +    SndChannelClient *client = snd_channel_get_client(channel);
>  
>      st->volume_nchannels = nchannels;
>      free(st->volume);
> @@ -844,7 +852,7 @@ SPICE_GNUC_VISIBLE void
> spice_server_playback_set_volume(SpicePlaybackInstance *
>  static void snd_channel_set_mute(SndChannel *channel, uint8_t mute)
>  {
>      SpiceVolumeState *st = &channel->volume;
> -    SndChannelClient *client = channel->connection;
> +    SndChannelClient *client = snd_channel_get_client(channel);
>  
>      st->mute = mute;
>  
> @@ -879,12 +887,12 @@ SPICE_GNUC_VISIBLE void
> spice_server_playback_start(SpicePlaybackInstance *sin)
>  {
>      SndChannel *channel = &sin->st->channel;
>      channel->active = TRUE;
> -    return snd_playback_start(channel->connection);
> +    return snd_playback_start(snd_channel_get_client(channel));
>  }
>  
>  SPICE_GNUC_VISIBLE void
> spice_server_playback_stop(SpicePlaybackInstance *sin)
>  {
> -    SndChannelClient *client = sin->st->channel.connection;
> +    SndChannelClient *client = snd_channel_get_client(&sin->st-
> >channel);
>  
>      sin->st->channel.active = FALSE;
>      if (!client)
> @@ -912,7 +920,7 @@ SPICE_GNUC_VISIBLE void
> spice_server_playback_stop(SpicePlaybackInstance *sin)
>  SPICE_GNUC_VISIBLE void
> spice_server_playback_get_buffer(SpicePlaybackInstance *sin,
>                                                           uint32_t
> **frame, uint32_t *num_samples)
>  {
> -    SndChannelClient *client = sin->st->channel.connection;
> +    SndChannelClient *client = snd_channel_get_client(&sin->st-
> >channel);
>  
>      *frame = NULL;
>      *num_samples = 0;
> @@ -948,7 +956,7 @@ SPICE_GNUC_VISIBLE void
> spice_server_playback_put_samples(SpicePlaybackInstance
>          }
>      }
>      playback_client = frame->client;
> -    if (!playback_client || sin->st->channel.connection !=
> SND_CHANNEL_CLIENT(playback_client)) {
> +    if (!playback_client || snd_channel_get_client(&sin->st-
> >channel) != SND_CHANNEL_CLIENT(playback_client)) {
>          /* lost last reference, client has been destroyed
> previously */
>          spice_debug("audio samples belong to a disconnected
> client");
>          return;
> @@ -970,18 +978,19 @@ void snd_set_playback_latency(RedClient
> *client, uint32_t latency)
>  
>      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 && now->connection &&
> -            red_channel_client_get_client(RED_CHANNEL_CLIENT(now-
> >connection)) == client) {
> +        if (type == SPICE_CHANNEL_PLAYBACK && scc &&
> +            red_channel_client_get_client(RED_CHANNEL_CLIENT(scc))
> == client) {
>  
> -            if
> (red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(now-
> >connection),
> +            if
> (red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(scc),
>                  SPICE_PLAYBACK_CAP_LATENCY)) {
> -                PlaybackChannelClient* playback =
> (PlaybackChannelClient*)now->connection;
> +                PlaybackChannelClient* playback =
> (PlaybackChannelClient*)scc;
>  
>                  playback->latency = latency;
> -                snd_set_command(now->connection,
> SND_PLAYBACK_LATENCY_MASK);
> -                snd_send(now->connection);
> +                snd_set_command(scc, SND_PLAYBACK_LATENCY_MASK);
> +                snd_send(scc);
>              } else {
>                  spice_debug("client doesn't not support
> SPICE_PLAYBACK_CAP_LATENCY");
>              }
> @@ -1062,7 +1071,6 @@ playback_channel_client_constructed(GObject
> *object)
>      spice_debug("playback client %p using mode %s",
> playback_client,
>                  spice_audio_data_mode_to_string(playback_client-
> >mode));
>  
> -    channel->connection = scc;
>      if (!red_client_during_migrate_at_target(red_client)) {
>          snd_set_command(scc, SND_PLAYBACK_MODE_MASK);
>          if (channel->volume.volume_nchannels) {
> @@ -1080,11 +1088,11 @@ static void snd_set_peer(RedChannel
> *red_channel, RedClient *client, RedsStream
>                           RedChannelCapabilities *caps, GType type)
>  {
>      SndChannel *channel = SND_CHANNEL(red_channel);
> -    SndChannelClient *snd_client;
> +    SndChannelClient *snd_client =
> snd_channel_get_client(channel);;
extra semicolon

>  
> -    if (channel->connection) {
> -        red_channel_client_disconnect(RED_CHANNEL_CLIENT(channel-
> >connection));
> -        channel->connection = NULL;
> +    /* sound channels currently only support a single client */
> +    if (snd_client) {
> +        red_channel_client_disconnect(RED_CHANNEL_CLIENT(snd_client
> ));
>      }
>  
>      snd_client = g_initable_new(type,
> @@ -1145,12 +1153,12 @@ SPICE_GNUC_VISIBLE void
> spice_server_record_start(SpiceRecordInstance *sin)
>  {
>      SndChannel *channel = &sin->st->channel;
>      channel->active = TRUE;
> -    snd_record_start(channel->connection);
> +    snd_record_start(snd_channel_get_client(channel));
>  }
>  
>  SPICE_GNUC_VISIBLE void
> spice_server_record_stop(SpiceRecordInstance *sin)
>  {
> -    SndChannelClient *client = sin->st->channel.connection;
> +    SndChannelClient *client = snd_channel_get_client(&sin->st-
> >channel);
>  
>      sin->st->channel.active = FALSE;
>      if (!client)
> @@ -1168,7 +1176,7 @@ SPICE_GNUC_VISIBLE void
> spice_server_record_stop(SpiceRecordInstance *sin)
>  SPICE_GNUC_VISIBLE uint32_t
> spice_server_record_get_samples(SpiceRecordInstance *sin,
>                                                              uint32_
> t *samples, uint32_t bufsize)
>  {
> -    SndChannelClient *client = sin->st->channel.connection;
> +    SndChannelClient *client = snd_channel_get_client(&sin->st-
> >channel);
>      uint32_t read_pos;
>      uint32_t now;
>      uint32_t len;
> @@ -1218,7 +1226,8 @@ static void snd_set_rate(SndChannel *channel,
> uint32_t frequency, uint32_t cap_o
>  
>  SPICE_GNUC_VISIBLE uint32_t
> spice_server_get_best_playback_rate(SpicePlaybackInstance *sin)
>  {
> -    return snd_get_best_rate(sin ? sin->st->channel.connection :
> NULL, SPICE_PLAYBACK_CAP_OPUS);
> +    SndChannelClient *client = sin ? snd_channel_get_client(&sin-
> >st->channel) : NULL;
> +    return snd_get_best_rate(client, SPICE_PLAYBACK_CAP_OPUS);
>  }
>  
>  SPICE_GNUC_VISIBLE void
> spice_server_set_playback_rate(SpicePlaybackInstance *sin, uint32_t
> frequency)
> @@ -1228,7 +1237,8 @@ SPICE_GNUC_VISIBLE void
> spice_server_set_playback_rate(SpicePlaybackInstance *si
>  
>  SPICE_GNUC_VISIBLE uint32_t
> spice_server_get_best_record_rate(SpiceRecordInstance *sin)
>  {
> -    return snd_get_best_rate(sin ? sin->st->channel.connection :
> NULL, SPICE_RECORD_CAP_OPUS);
> +    SndChannelClient *client = sin ? snd_channel_get_client(&sin-
> >st->channel) : NULL;
> +    return snd_get_best_rate(client, SPICE_RECORD_CAP_OPUS);
>  }
>  
>  SPICE_GNUC_VISIBLE void
> spice_server_set_record_rate(SpiceRecordInstance *sin, uint32_t
> frequency)
> @@ -1256,7 +1266,6 @@ record_channel_client_constructed(GObject
> *object)
>  
>      G_OBJECT_CLASS(record_channel_client_parent_class)-
> >constructed(object);
>  
> -    channel->connection = scc;
>      if (channel->volume.volume_nchannels) {
>          snd_set_command(scc, SND_VOLUME_MUTE_MASK);
>      }
> @@ -1443,10 +1452,11 @@ void snd_set_playback_compression(bool on)
>  
>      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 && now->connection) {
> -            PlaybackChannelClient* playback =
> (PlaybackChannelClient*)now->connection;
> +        if (type == SPICE_CHANNEL_PLAYBACK && 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);
> @@ -1456,7 +1466,7 @@ void snd_set_playback_compression(bool on)
>                                                        client_can_op
> us, client_can_celt);
>              if (playback->mode != desired_mode) {
>                  playback->mode = desired_mode;
> -                snd_set_command(now->connection,
> SND_PLAYBACK_MODE_MASK);
> +                snd_set_command(client, SND_PLAYBACK_MODE_MASK);
>                  spice_debug("playback client %p using mode %s",
> playback,
>                              spice_audio_data_mode_to_string(playbac
> k->mode));
>              }
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]