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>
This structure is used to store format information for
both Gstreamer 0.10 and 1.0 however the two format uses
different fields from it.
Use a macro to filter only needed fields.
This currently also fixes a compile error using Gstreamer 0.10
(GST_VIDEO_FORMAT_RGB15 not defined as not available).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
The structure is used only to allocate private data.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Currently, calling spice_server_remove_interface() twice in a row with
the same SPICE_INTERFACE_CHAR_DEVICE is going to cause a crash when
calling red_char_device_get_server(char_device->st); because
char_device->st will have been set to NULL by the first call.
This commit adds a few sanity checks before trying to use the various
'st' members of the interfaces.
This should avoid the crash described in
https://bugzilla.redhat.com/show_bug.cgi?id=1411194 even though it's not
clear how we got in that situation.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Avoid to use g_object_get if not necessary.
red_char_device_get_server is more type safe and we are
not bound to dynamic fields.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
This field is used only for debugging.
Remove it reducing a bit all these "current" fields around.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
If bitmap stride and stream stride are different copy was used.
Using GStreamer 1.0 you can avoid the copy setting correctly
image offset and stride.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Handling read returning 0 (usually end of connection/pipe)
is the same of handling an error (read result -1) with errno == 0
so merge the two paths to reuse code and simplify.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This allows the connection to early fail in case initial bytes
are not correct.
This allows for instance VNC client to graceful fail connecting
to a spice-server. This happens easily as the two protocols
share the same range of ports.
This resolves rhbz#1416692.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Tested-by: Daniel P. Berrange <berrange@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
Code to read and process display commands were the same
so use a common function for better reuse.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This format is required to add metadata to the source buffer
using gst_buffer_add_video_meta_full.
This metadata can be used to pass strides/offsets, or
dmabuf-specific information.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This happened during VM resume.
RedSurfaceCmd were allocated but never freed.
We don't need to malloc the RedSurfaceCmd used in handle_dev_close()
as display_channel_process_surface_cmd() will not try to reference
it after it has returned.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Code to read and process cursor commands were the same
so use a common function for better reuse.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This allows to do some possible statistics or graph.
Currently the report contains encoded sizes.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@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>
This became obsolete when RedChannel became GObject-based.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
These formats were tested manually using test-gst utility
in server/tests.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
Add an utility to make possible to check various features of
VideoEncoder.
2 GStreamer plugins are used in a chain like this:
(1) input pipeline -> (2) video encoder -> (3) output pipeline
While converting output from (1) is compared with output of (3)
making sure the streaming is working correctly.
You can set various options:
- part of the input pipeline description to allow specifying different
video from GStreamer test ones to a video file;
- the encoder to use;
- different image properties to use for (2) input:
- different bit depth;
- top/down or down/up;
- initial bitrate.
The idea is to use this helper in combination with a shell script
and some video sources to make able to test various settings.
Also can be used to extend the current encoder list.
As an example you can use a command like
$ ./test-gst -e gstreamer:vp8 -i \
'filesrc location=bbb_sunflower_1080p_30fps_normal.mp4 \
! decodebin ! videoconvert'
to check vp8 encoding.
Currently it does not emulate bandwidth changes as stream reports
from the client are not coded.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
Small refactor. As reds_get_video_codecs() returns the video codecs as
GArray, we should match reds_set_video_codecs() to have a GArray as
parameter instead of string.
reds_set_video_codecs_from_string() seems more appropriate for the
previous function.
Signed-off-by: Victor Toso <victortoso@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>
When the initial image was sent to the client the reference
was not incremented leading to some user after free.
This regression was introduced in
3bde2e570c
("DCC: remove more init_send_data() arguments").
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
This fixes a regression caused by
a43c21b6bc
("DCC: change how fill_bits() marshalls data by reference").
Before the mentioned patch there were a few references to Drawable
structure so an uint8_t was enough.
Now that every chunk of the image is accounted you can easily
get an overflow.
This fixes https://bugs.freedesktop.org/show_bug.cgi?id=99258.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
Fix compilation error due to -Werror=maybe-uninitialized:
CC test-display-base.o
test-display-base.c: In function 'do_wakeup':
test-display-base.c:579:13: error: 'update' may be used uninitialized...
push_command(&update->ext);
Signed-off-by: Snir Sheriber <ssheribe@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This third argument (and the 'item' member of
RedChannelClient::priv::send_data) was a somewhat roundabout way to keep
the RedPipeItem alive until a message is sent, just in case some data
owned by that pipeitem was added to the marshaller by reference. This
was a rather confusing mechanism, however, since it did not have any
obvious connection to the _add_by_ref() call. It was never very clear
whether you needed to pass an item to this function or not. The previous
series of patches made this parameter unnecessary since the referencing
of the pipe item (or other related structure) is now more explicitly
connected to the calls to spice_marshaller_add_by_ref_full().
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The fill_bits() function marshalls some data by reference. This data is
owned by the RedDrawable that is owned by the Drawable that is owned by
the RedDrawablePipeItem. Instead of keeping the RedPipeItem alive by
passing it to red_channel_client_init_send_data(), simply reference the
Drawable and marshall it with _add_by_ref_full(). This means that we
can't use the _add_chunks_by_ref() convenience function since that
function doesn't allow us to pass a free function to clean up the data
after it is sent.
This change is not perfect since the fill_bits() function makes an
assumption that 'simage' is owned by the 'drawable'. On the other hand,
the previous code made a much bigger assumption: that the caller would
ensure that the data would be kept alive
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Use spice_marshaller_add_by_ref_full() instead of _add_by_ref() to
handle the referenced data properly rather than passing the pipe item to
red_channel_client_init_send_data() to keep the pipe item alive
indirectly.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
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>
