[Spice-devel] [PATCH spice-server v2 3/4] sound: Remove sin field from SpicePlaybackState and SpiceRecordState
Christophe Fergeau
cfergeau at redhat.com
Tue Nov 15 10:23:48 UTC 2016
This would have deserved a "why?" in the log even if it's just a few
lines (something like "it's redundant because all we need is sin->st
which we can pass directly/get directly from yyy").
In my opinion, commits with only a short log are only acceptable
for the most trivial of commits, which this one is not.
Christophe
On Mon, Nov 14, 2016 at 09:32:11AM +0000, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
> server/sound.c | 63 ++++++++++++++++++++++++++++++++--------------------------
> 1 file changed, 35 insertions(+), 28 deletions(-)
>
> diff --git a/server/sound.c b/server/sound.c
> index c078f28..915756f 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -166,12 +166,10 @@ struct SndWorker {
>
> struct SpicePlaybackState {
> struct SndWorker worker;
> - SpicePlaybackInstance *sin;
> };
>
> struct SpiceRecordState {
> struct SndWorker worker;
> - SpiceRecordInstance *sin;
> };
>
> typedef struct RecordChannel {
> @@ -190,6 +188,8 @@ typedef struct RecordChannel {
> static SndWorker *workers;
>
> static void snd_receive(SndChannel *channel);
> +static void snd_playback_start(SpicePlaybackState *st);
> +static void snd_record_start(SpiceRecordState *st);
>
> static SndChannel *snd_channel_ref(SndChannel *channel)
> {
> @@ -393,11 +393,11 @@ static int snd_record_handle_message(SndChannel *channel, size_t size, uint32_t
> return snd_record_handle_write((RecordChannel *)channel, size, message);
> case SPICE_MSGC_RECORD_MODE: {
> SpiceMsgcRecordMode *mode = (SpiceMsgcRecordMode *)message;
> - SpiceRecordState *st = SPICE_CONTAINEROF(channel->worker, SpiceRecordState, worker);
> + SndWorker *worker = channel->worker;
> record_channel->mode_time = mode->time;
> if (mode->mode != SPICE_AUDIO_DATA_MODE_RAW) {
> - if (snd_codec_is_capable(mode->mode, st->worker.frequency)) {
> - if (snd_codec_create(&record_channel->codec, mode->mode, st->worker.frequency,
> + if (snd_codec_is_capable(mode->mode, worker->frequency)) {
> + if (snd_codec_create(&record_channel->codec, mode->mode, worker->frequency,
> SND_CODEC_DECODE) == SND_CODEC_OK) {
> record_channel->mode = mode->mode;
> } else {
> @@ -1035,25 +1035,29 @@ SPICE_GNUC_VISIBLE void spice_server_playback_set_mute(SpicePlaybackInstance *si
> snd_playback_send_mute(playback_channel);
> }
>
> -SPICE_GNUC_VISIBLE void spice_server_playback_start(SpicePlaybackInstance *sin)
> +static void snd_playback_start(SpicePlaybackState *st)
> {
> - SndChannel *channel = sin->st->worker.connection;
> - PlaybackChannel *playback_channel = SPICE_CONTAINEROF(channel, PlaybackChannel, base);
> + SndChannel *channel = st->worker.connection;
>
> - sin->st->worker.active = 1;
> + st->worker.active = 1;
> if (!channel)
> return;
> - spice_assert(!playback_channel->base.active);
> + spice_assert(!channel->active);
> reds_disable_mm_time(snd_channel_get_server(channel));
> - playback_channel->base.active = TRUE;
> - if (!playback_channel->base.client_active) {
> - snd_set_command(&playback_channel->base, SND_PLAYBACK_CTRL_MASK);
> - snd_playback_send(&playback_channel->base);
> + channel->active = TRUE;
> + if (!channel->client_active) {
> + snd_set_command(channel, SND_PLAYBACK_CTRL_MASK);
> + snd_playback_send(channel);
> } else {
> - playback_channel->base.command &= ~SND_PLAYBACK_CTRL_MASK;
> + channel->command &= ~SND_PLAYBACK_CTRL_MASK;
> }
> }
>
> +SPICE_GNUC_VISIBLE void spice_server_playback_start(SpicePlaybackInstance *sin)
> +{
> + return snd_playback_start(sin->st);
> +}
> +
> SPICE_GNUC_VISIBLE void spice_server_playback_stop(SpicePlaybackInstance *sin)
> {
> SndChannel *channel = sin->st->worker.connection;
> @@ -1245,7 +1249,7 @@ static void snd_set_playback_peer(RedChannel *channel, RedClient *client, RedsSt
> }
>
> if (worker->active) {
> - spice_server_playback_start(st->sin);
> + snd_playback_start(st);
> }
> snd_playback_send(worker->connection);
> }
> @@ -1299,26 +1303,31 @@ SPICE_GNUC_VISIBLE void spice_server_record_set_mute(SpiceRecordInstance *sin, u
> snd_record_send_mute(record_channel);
> }
>
> -SPICE_GNUC_VISIBLE void spice_server_record_start(SpiceRecordInstance *sin)
> +static void snd_record_start(SpiceRecordState *st)
> {
> - SndChannel *channel = sin->st->worker.connection;
> + SndChannel *channel = st->worker.connection;
> RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
>
> - sin->st->worker.active = 1;
> + st->worker.active = 1;
> if (!channel)
> return;
> - spice_assert(!record_channel->base.active);
> - record_channel->base.active = TRUE;
> + spice_assert(!channel->active);
> record_channel->read_pos = record_channel->write_pos = 0; //todo: improve by
> //stream generation
> - if (!record_channel->base.client_active) {
> - snd_set_command(&record_channel->base, SND_RECORD_CTRL_MASK);
> - snd_record_send(&record_channel->base);
> + channel->active = TRUE;
> + if (!channel->client_active) {
> + snd_set_command(channel, SND_RECORD_CTRL_MASK);
> + snd_record_send(channel);
> } else {
> - record_channel->base.command &= ~SND_RECORD_CTRL_MASK;
> + channel->command &= ~SND_RECORD_CTRL_MASK;
> }
> }
>
> +SPICE_GNUC_VISIBLE void spice_server_record_start(SpiceRecordInstance *sin)
> +{
> + snd_record_start(sin->st);
> +}
> +
> SPICE_GNUC_VISIBLE void spice_server_record_stop(SpiceRecordInstance *sin)
> {
> SndChannel *channel = sin->st->worker.connection;
> @@ -1465,7 +1474,7 @@ static void snd_set_record_peer(RedChannel *channel, RedClient *client, RedsStre
>
> on_new_record_channel(worker, &record_channel->base);
> if (worker->active) {
> - spice_server_record_start(st->sin);
> + snd_record_start(st);
> }
> snd_record_send(worker->connection);
> }
> @@ -1513,7 +1522,6 @@ void snd_attach_playback(RedsState *reds, SpicePlaybackInstance *sin)
> ClientCbs client_cbs = { NULL, };
>
> sin->st = spice_new0(SpicePlaybackState, 1);
> - sin->st->sin = sin;
> playback_worker = &sin->st->worker;
> playback_worker->frequency = SND_CODEC_CELT_PLAYBACK_FREQ; /* Default to the legacy rate */
>
> @@ -1543,7 +1551,6 @@ void snd_attach_record(RedsState *reds, SpiceRecordInstance *sin)
> ClientCbs client_cbs = { NULL, };
>
> sin->st = spice_new0(SpiceRecordState, 1);
> - sin->st->sin = sin;
> record_worker = &sin->st->worker;
> record_worker->frequency = SND_CODEC_CELT_PLAYBACK_FREQ; /* Default to the legacy rate */
>
> --
> 2.7.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- 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/20161115/a8d6a3c9/attachment-0001.sig>
More information about the Spice-devel
mailing list