Use spice_marshaller_add_by_ref_full() instead of
spice_marshaller_add_by_ref() to allow the marshaller to manage the
lifetime of the referenced data buffer rather than having to manage it
by passing a PipeItem to red_channel_client_init_send_data(). Since the
data is owned by CursorItem (which is not in fact a RedPipeItem, but is
owned by a pipe item and is itself refcounted), we take a reference on
the CursorItem when adding the data buf to the marshaller, and then
unreference it in the marshaller free func.
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
The only time that the pipe item needs to be passed as the third
argument to red_channel_client_init_send_data() is when the pipe item
holds a data buffer that has been added to the marshaller by reference
(spice_marshaller_add_by_ref()) and needs to be kept alive until the
data has been sent. In all other cases, the item does not need to be
kept alive, so we can safely pass NULL for this third parameter.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Any data that is added to the marshaller by reference (using e.g.
spice_marshaller_add_by_ref_full()) is freed during
spice_marshaller_reset(). But the marshaller is not currently reset
until we begin to send the next message (in
red_channel_client_send_item()). This means that the sent message data
lives longer than expected and can violate some assumptions in other
parts of the code.
To make sure that the data is cleaned up right after being sent, I've
added a reset call to clear_sent_item() and called that function from
_on_out_msg_done(). This means that _restore_main_sender() no longer
needs to reset the marshaller, and we no longer need to call
_reset_sent_item() within _on_out_msg_done() (since this function is
called from _clear_sent_item()).
This prepares the way for refactoring
red_channel_client_init_send_data() to change how we keep the
RedPipeItem alive while sending a message.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
We should replace the video_codecs GArray only after the parsing of
input is done, otherwise we might lose previous configuration.
Tests were updated to match this situation. Input that fails to
replace video_codecs are considered bad.
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This make more obvious which directory they refer
and potentially avoid ignoring unwanted files.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
The partial expression "MSEC_PER_SEC * size * 8" can overflow if
size is 536870 or more. This as the operation is done using
32 bit unsigned integers. Being the size potentially double of
a compressed frame size the limit can be easily reached.
As get_average_frame_size already return a 64 bit use 64 bit
even for the size to avoid the integer overflow.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Francois Gouget <fgouget@codeweavers.com>
This function is supposed to add an item to the queue to
be sent before all other queued items.
Was never used.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
refs was used before GObject for reference counting.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
All RedWorker messages starts with RedWorker except
SpiceMsgDisplayGlDraw.
For coherence introduce a RedWorkerMessageGlDraw structure
holding just SpiceMsgDisplayGlDraw. This also allows possible
extensions.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@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>
spice_debug was called for not-EINTR case, move
it to the right place.
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
man 2 dup2 specifies:
The close-on-exec flag (FD_CLOEXEC; see fcntl(2)) for
the duplicate descriptor is off.
Since the purpose of the fcntl call is to turn off FD_CLOEXEC
flag, and it's already done, just remove this call.
Suggested-by: Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Spice coding style suggests to use curly braces
for while blocks.
Some prefer the block to not be empty so continue
is untouched.
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This patch prevents possible memory leak.
Found by coverity.
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
In both image_encoders_restore_glz_dictionary() and
image_encoders_get_glz_dictionary() shared-dict may
be NULL if size is too large, and the server gets
size from the network.
Both functions end up calling glz_enc_dictionary_create()
that calls glz_dictionary_window_create() where size is
checked.
Found by coverity.
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It shadows the outer one.
Renamed also the outer 'container' variable.
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Allow to catch minor issue with test code.
Although test usually are coded with less care than production
code the current changes required are not so extensive and
can catch different issues.
Most of the patch is making functions static to avoid warnings for
undeclared functions.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This patch creates display_channel_get_video_codecs() helper to let
stream.c get the video-codecs without accessing the internal structure
of DisplayChannel.
As video-codecs is a property of DisplayChannel, this change means
making this property readable as well.
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Allow to dynamically remove QXL interfaces. This could be used to
support hot swapping of QXL cards.
This code is actually not used in any way.
QXL interfaces are destroyed by spice_server_destroy automatically.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Make finalization of DisplayChannel consistent with other code.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@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>
As data is packae in a single piece of memory send it
altogether.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
Including g_clear_pointer() in glib-compat.h and using it to avoid
warnings in odd situations.
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@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>
Try to arrange destruction in the opposite order of the creation
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
The check to limit too low bit rates was setting encoder->bit_rate
instead of bit_rate. However after some lines bit_rate was used
to set encoder->bit_rate basically removing the lower threshold.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Francois Gouget <fgouget@codeweavers.com>
By removing the stream's video encoder we force the stream to send
future frames using the fallback code, that is as regular screen
updates.
However note that we keep the stream object: we have to. Otherwise
future frames would trigger the creation of a new stream object with a
new video encoder which would again try to stream the video and fail
again and again.
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Reviewed-by: Victor Toso <victortoso@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>
