Many function already used bool as boolean type.
Change last gboolean occurrencies and values.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Commit 590acf3c55 introduced a regression after migration:
PlaybackChannel::connection is never set even if a client is connected,
which means the playback channel is non-functional until a client
reconnects as all the snd_channel_set_xxx() method checks for a non-NULL
PlaybackChannel::connection before sending data to the client.
This commit slightly changes the code flow in
on_new_playback_channel_client()/playback_channel_client_constructed()
so that PlaybackChannel::connection is set regardless of what
red_client_during_migrate_at_target() returns. This is what was done
prior to 590acf3c55.
This resolves https://bugs.freedesktop.org/show_bug.cgi?id=100136
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
It can be useful for debug to know what the codec of the playback and
record channels are, so add a debug-level print statement indicating
this.
Resolves: rhbz#1436251
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Both _test_remote_cap() and _test_remote_common_cap() are used as
boolean values, so change the return type from int to bool to follow our
coding standard.
This is a setting for determining whether to compress the audio playback
channel or not. It is variously typed as int or uint32_t. Convert it to
a 'bool' to make it more clear that it is a true/false value rather than
an enumeration or something like that.
The code to enable/disable on a TCP socket is duplicated in multiple
places in the code base, this commit replaces this duplicated code with
a helper in RedsStream.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This commit changes all functions returning TRUE/FALSE from having an
'int' return value to 'bool'.
This way it's obvious that such a function is not going to return
anything else than TRUE or FALSE.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
config_socket is configuring the client stream socket.
As is responsibility of RedChannelClient to handle the stream
it make more sense to have the function in this object.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
These vfuncs are more appropriate in RedChannelClient.
The buffer they allocated are related to the client stream
which is managed directly by RedChannelClient.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
For each channel there are two set of capabilities, one
for the common ones and one for the specific ones.
A single set were almost always passed using 2 arguments,
a number of elements and an array but then before using
these were converted to a GArray.
Use a single structure (already available) to pass all
channel capabilites using a single argument.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Using spice_info() gets in the way of tests using
g_test_expect_message() as all the messages emitted using
a non-debug log level must be listed as expected, otherwise we get a
critical about an expected message not having been logged.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The usage was removed with commit 7ea1f2c133
("sound: Use RedChannelClient to receive/send data").
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
Non blocking flag is set for all connection inside reds.c so
there's no need to set again for the single client channel.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
red_channel_client_parse() currently does roughly:
if (klass->parser) {
parsed = klass->parser(msg, msg_size, &parsed_size);
klass->handle_parsed(rcc, parsed_size, msg_type, parsed);
} else {
klass->handle_message(rcc, msg_type, msg, msg_size);
}
The handle_parsed implementation expects a void * 'parsed' argument,
which will then be cast to the correct type. There is not really a need
to provide distinct handle_parsed/handle_message vfuncs, instead we can
say that if a RedChannel subclass provides a 'parser' vfunc, then it's
'handle_message' vfunc will be called with the parsed message, otherwise
it will be called with unparsed data (ie what handle_message currently
expects).
This makes the code slightly easier to follow as messages will always be
handled by the same vfunc.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
red_channel_client_handle_message can handle base messages
so reuse it.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
We support only a single client so don't waste code just
to check this.
The worst stuff can happen is that we'll migrate multiple
connections.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Move the freeing of SndChannel data members from snd_detach_common() to
the finalize function to encapsulate things a bit more cleanly. It
doesn't really change the behavior or order of destruction since
snd_detach_common() destroys the channel.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Now that SndChannelClient has switched from using its own code for
sending data to using RedChannelClient, it's very close to being an
actual RedChannelClient.
This commit makes it directly inherit from RedChannelClient rather than
having a channel_client field. This allows to get rid of the whole
DummyChannel/DummyChannelClient code.
Based on a patch from Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
snd_set_command()/snd_send() are higher level methods which take care of
scheduling calls to the corresponding snd_*_send_*() methods when
appropriate. This commit switches a few direct snd_*_send_*() calls to
snd_set_command()/snd_send().
Based on a patch from Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
You can see that SndChannelClient has much less field
as the code to read/write from/to client is reused from
RedChannelClient instead of creating a fake RedChannelClient
just to make the system happy.
One of the different between the old sound code and all other
RedChannelClient objects was that the sound channel don't use
a queue while RedChannelClient use RedPipeItem object. This was
the main reason why RedChannelClient was not used. To implement
the old behaviour a "persistent_pipe_item" is used. This RedPipeItem
will be queued to RedChannelClient (only one!) so signal code we
have data to send. The {playback,record}_channel_send_item will
then send the messages to the client using RedChannelClient functions.
For this reason snd_reset_send_data is replaced by a call to
red_channel_client_init_send_data and snd_begin_send_message is
replaced by red_channel_client_begin_send_message.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
The removed code was trying to read data when
spice_server_record_get_samples() is called. Since reading of data is
event-driven anyway (see snd_event), it's redundant to try
again to read more data.
This commit removes this code as this will some refactoring easier in
the next commits.
Based on a patch from Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
We can get it from our DummyChannelClient rather than storing it in
SndChannelClient.
Based on a patch from Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
We can use the marshaller provided by the dummy RedChannelClient
associated with SndChannelClient.
Based on a patch from Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Filter out commands which should not happen. Should it be a
g_warn_if_fail() or such instead?
Based on a patch from Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This is in preparation for switching SndChannelClient into a proper
RedChannelClient. The prototype of the new helper matches what is
expected from the RedChannel::config_socket vfunc.
To be able to achieve that, this commit associates the sound channel
RedsStream instance with the DummyChannelClient instance we have, and
then call snd_channel_config_socket() on that instance.
Based on a patch from Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The main goal of this commit is to avoid to dereference 'client' before
it's checked for NULL. This meant splitting one error condition in 2
separate ones.
This also sets default values for the 'frame' and 'num-samples'
out parameters so that we just have to return on error conditions.
Based on a patch from Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Currently MUTE and VOLUME commands use the same VOLUME mask. This commit
introduces a separate SND_MUTE_MASK for MUTE commands to make things
a bit more clear.
Based on a patch from Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This is no longer used since "sound: Convert SndChannel to GObject"
Based on a patch from Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It's already defined before in the same source file.
Based on a patch from Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Based on a patch from Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It became unused in 26027036c 'red_channel: remove unused migrate flag
from RedChannel' but was never removed from the function prototype.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Previously, the object we now call SndChannel was named SndWorker, and
the object we now call SndChannelClient was called SndChannel. When
these names were changed, the functions
on_new_(record|playback)_channel() were not updated, so the function
names and the arguments are both a bit confusing now. Update them to
match the new names.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The spice_marshaller_add_ref() family of functions is confusing since it
sounds like you're incrementing a reference on the marshaller. What it
is actually doing is adding a data buffer to the marshaller by reference
rather than by value. Changing the function names to _add_by_ref() makes
this clearer.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
Reduce GObject changes coming in the next commit since we'll need to
change how we access the marshaller anyway. This will make the
following commits easier to review.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
When qemu (for example) delivers audio samples to the spice server, it
does so by requesting a buffer from the spice server
(spice_server_playback_get_buffer()), filling them with audio data, and
then calling spice_server_playback_put_samples() to send them to the
client. Between the call to _get_buffer() and the call to
_put_samples(), we need to ensure that the buffer remains valid. Since
this buffer is allocated within PlaybackChannelClient, we did this by
incrementing a ref on the PlaybackChannelClient in _get_buffer(), and
decrementing the ref in _put_samples(). This has the effect of
potentially keeping the PlaybackChannelClient alive after the spice
client has disconnected.
This was not a problem when PlaybackChannelClient was a simple helper
class. But we intend to change PlaybackChannelClient so that it
inherits from RedChannelClient. In that case, the reference taken in
_get_buffer() would result in the RedChannelClient (and associated
RedChannel, etc) being kept alive longer than expected. To avoid this,
we add an additional ref-counted adapter class (AudioFrameContainer)
that owns the allocated audio frames and can outlive the
RedChannelClient if necessary. When the client is freed, the
AudioFrameContainer is just unreferenced.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Stops using the dummy channel.
Data handling still goes through DummyChannelClient which is why
empty implementation of some required vfuncs is working.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Make easier to understand that they refer to client and not
all channel.
Specifically:
- RecordChannel -> RecordChannelClient
- PlaybackChannel -> PlaybackChannelClient
- playback_channel -> playback_client
- record_channel -> record_client
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
SndWorker has been historically based on RedChannel, initial git commit
has:
struct SndWorker {
Channel base;
...
};
SndChannel, contrary to what its name may imply is more focused on
marshalling/sending of sound data, which is the responsibility of
RedChannelClient for the other SPICE channels.
This commit and following ones make the naming of these 2 types more
consistent with how the rest of the code is structured.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>