Commit Graph

2423 Commits

Author SHA1 Message Date
Pavel Grunt
fe1b819a97 Include compat header for g_clear_pointer
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-01-24 21:52:36 +00:00
Frediano Ziglio
0959305ecf red-worker: Reuse code to process display command
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>
2017-01-24 15:56:50 +00:00
Frediano Ziglio
e442388703 gstreamer: Add gst_format to the table of supported formats
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>
2017-01-24 12:09:04 +00:00
Frediano Ziglio
90f3655ad4 red-worker: Do not leak memory for surface commands
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>
2017-01-24 12:00:17 +00:00
Frediano Ziglio
e36e700d17 red-worker: Reuse code to process cursor command
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>
2017-01-24 12:00:08 +00:00
Frediano Ziglio
19a900f396 tests: Make possible to have a report of the video encoding
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>
2017-01-21 09:05:42 +00:00
Frediano Ziglio
345d7bde23 Compatibility for GStreamer 0.10 for test utility
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-01-21 09:04:50 +00:00
Christophe Fergeau
0e52f55887 sound: Add separate SND_MUTE_MASK
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>
2017-01-11 14:47:55 +00:00
Christophe Fergeau
e4ddd19180 sound: Remove dummy-channel.[ch]
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>
2017-01-11 14:21:16 +00:00
Christophe Fergeau
f5453923a4 sound: Remove duplicate AudioFrame typedef
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>
2017-01-11 12:39:44 +00:00
Christophe Fergeau
188f05d7ae sound: Don't dereference pointer before NULL check
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>
2017-01-11 11:14:10 +00:00
Christophe Fergeau
70a66b2600 sound: Remove unused __new_channel() argument
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>
2017-01-11 11:07:38 +00:00
Christophe Fergeau
a1b68f3631 sound: Remove extraneous whitespace
No need for this whitespace before ';'

Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-01-11 10:51:06 +00:00
Christophe Fergeau
2165db758c channel: Remove commented out function prototype
This became obsolete when RedChannel became GObject-based.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-01-11 10:40:51 +00:00
Frediano Ziglio
e8d078673a gstreamer: Do not warn for tested formats
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>
2017-01-10 13:42:33 +00:00
Frediano Ziglio
623e7d4a93 Add an helper to test VideoEncoder
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>
2017-01-10 13:42:25 +00:00
Victor Toso
ce2ef83df4 reds: set the video_codecs in a separated function
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>
2017-01-10 11:13:52 +00:00
Jonathon Jongsma
5ef3c6cda0 Sound: Fix confusing channel/client terminology
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>
2017-01-06 09:38:38 +00:00
Frediano Ziglio
fe6ad8ba11 Increment correctly reference before adding the item to marshaller
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>
2017-01-05 09:36:20 +00:00
Frediano Ziglio
1e1ed93ea7 Avoid integer overflow for Drawable::refs field
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>
2017-01-05 09:34:14 +00:00
Snir Sheriber
af390d53ca tests: Fix compilation error
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>
2017-01-04 16:09:50 +00:00
Jonathon Jongsma
0ace8a81c7 Remove third argument from red_channel_client_init_send_data()
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>
2016-12-20 16:11:39 +00:00
Jonathon Jongsma
a43c21b6bc DCC: change how fill_bits() marshalls data by reference
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>
2016-12-20 16:11:35 +00:00
Jonathon Jongsma
3bde2e570c DCC: remove more init_send_data() arguments
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-12-20 16:11:33 +00:00
Jonathon Jongsma
b5758229ad Spicevmc: don't pass pipe item to init_send_data()
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-12-20 16:11:31 +00:00
Jonathon Jongsma
9971c1eda0 Smartcard: Don't pass pipe item to _init_send_data()
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-12-20 16:11:28 +00:00
Jonathon Jongsma
825880d919 MainChannel: remove another init_send_data arg
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>
2016-12-20 16:11:13 +00:00
Frediano Ziglio
333ef51ece Refactor cursor marshalling for SET, INIT
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>
2016-12-20 16:10:56 +00:00
Jonathon Jongsma
5c0f2e341c Avoid passing pipe item to red_channel_client_init_send_data()
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>
2016-12-20 10:14:02 +00:00
Jonathon Jongsma
0239dfa908 Reset marshaller as soon as a message is sent
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>
2016-12-20 10:13:55 +00:00
Jonathon Jongsma
f184aed83e CursorChannel: minor improvement to cursor_fill()
Move all 'out' parameters to the end of the function.

Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-12-16 09:46:46 +00:00
Frediano Ziglio
91668cdaab Sort include order in source files
Sort based on coding style.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-12-16 08:16:21 +00:00
Frediano Ziglio
48e732e08b red-worker: Optimise check
Use compile time check instead of runtime one.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-12-16 08:13:18 +00:00
Victor Toso
aebc51f91a reds: don't replace video_codecs on failure
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>
2016-12-15 11:13:36 +00:00
Jonathon Jongsma
481e1528ae Rename cursor_set_item() to cursor_channel_set_item()
Follow C method naming convention.

Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-12-14 20:16:02 +00:00
Frediano Ziglio
67df9f760f gitignore: Reuse top-level gitignore
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2016-12-14 19:09:21 +00:00
Frediano Ziglio
54ee6bbb8b gitignore: Limit scope of some files
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>
2016-12-14 19:09:21 +00:00
Frediano Ziglio
893f3fef87 gitignore: Remove obsolete files
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2016-12-14 19:09:21 +00:00
Frediano Ziglio
554b20bbdb gstreamer: Prevent integer overflow in delay computation
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>
2016-12-12 19:37:34 +00:00
Frediano Ziglio
6940f7ff53 Removed unused red_channel_pipes_new_add_tail function
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>
2016-12-12 13:37:00 +00:00
Frediano Ziglio
dc9177689e Remove unused refs field
refs was used before GObject for reference counting.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2016-12-12 13:36:54 +00:00
Frediano Ziglio
ed500d1a4c red-worker: Introduce RedWorkerMessageGlDraw structure
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>
2016-12-12 11:45:07 +00:00
Victor Toso
8faf7a6f45 Fix gitignore
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
2016-12-08 15:18:25 +01:00
Frediano Ziglio
3613ead4d8 marshaller: rename _add_ref() to _add_by_ref()
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>
2016-12-08 14:05:04 +00:00
Uri Lublin
51d0ed6abb dispatcher: write_safe: move EINTR debug message
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>
2016-12-06 16:45:11 +00:00
Uri Lublin
c3d5689f4a red-record-qxl: child_output_setup: remove fcntl call
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>
2016-12-06 16:45:08 +00:00
Uri Lublin
4be1a6ec8c red-record-qxl: add curly braces to empty while loop
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>
2016-12-06 16:45:07 +00:00
Uri Lublin
547f9f4387 red_get_image_data_flat: allocate mem after sanity check
This patch prevents possible memory leak.

Found by coverity.

Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-12-06 16:45:03 +00:00
Uri Lublin
dad108edb1 image_encoders: check shared_dict before accessing it
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>
2016-12-06 16:45:00 +00:00
Uri Lublin
a286da42d2 display-channel: current_remove: rename inner variable 'container'
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>
2016-12-06 16:44:53 +00:00