Commit Graph

3299 Commits

Author SHA1 Message Date
Christophe Fergeau
39c22ee98f sound: Prefer snd_set_command() over snd_*_send_*()
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>
2017-02-02 16:31:05 +01:00
Christophe Fergeau
7ea1f2c133 sound: Use RedChannelClient to receive/send data
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>
2017-02-02 16:31:05 +01:00
Christophe Fergeau
cb0e45cd16 sound: Remove code from spice_server_record_get_samples()
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>
2017-02-02 16:31:04 +01:00
Christophe Fergeau
4bb9c1fe56 sound: Remove SndChannelClient::channel
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>
2017-02-02 16:31:04 +01:00
Christophe Fergeau
bc2a510438 sound: Remove SndChannelClient::send_data::marshaller
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>
2017-02-02 16:31:04 +01:00
Christophe Fergeau
85b73636d9 sound: Add sanity checks in snd_{playback,record}_send
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>
2017-02-02 16:31:04 +01:00
Christophe Fergeau
c4a58c90c6 sound: Implement snd_channel_config_socket
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>
2017-02-02 16:31:04 +01:00
Christophe Fergeau
f5a972fdbf sound: Rework spice_server_playback_get_buffer() error handling
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>
2017-02-02 16:31:04 +01:00
Frediano Ziglio
e622e09209 gstreamer: Include only needed fields in SpiceFormatForGStreamer structure
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>
2017-02-02 11:23:19 +00:00
Frediano Ziglio
98a168cb3f display-channel: Move _Drawable declaration to private header
The structure is used only to allocate private data.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-02-01 15:35:09 +00:00
Frediano Ziglio
d8a32e77f5 spicevmc: Avoid computing some variable value if not necessary
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-02-01 08:47:07 +00:00
Christophe Fergeau
1addd3c514 Add some NULL checks to spice_server_remove_interface()
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>
2017-01-31 17:29:15 +00:00
Frediano Ziglio
cbbc53bdf8 reds: Get state using red_char_device_get_server
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>
2017-01-31 12:18:47 +00:00
Frediano Ziglio
e269e61f8b display-channel: Remove current_size field
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>
2017-01-31 10:06:58 +00:00
Frediano Ziglio
0d14f96daa Support VP9 encoder using GStreamer
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2017-01-31 09:00:06 +00:00
Frediano Ziglio
c3d237075b gstreamer: Avoid memory copy if strides are different
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>
2017-01-28 09:48:17 +00:00
Frediano Ziglio
8cef0a4e8b reds-stream: Simplify error logic
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>
2017-01-27 10:50:25 +00:00
Frediano Ziglio
e4bb431191 reds: Check link header magic without waiting for the whole header
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>
2017-01-26 16:33:27 +00:00
Frediano Ziglio
7ce225f053 spicevmc: Reduce number of last saved IDs
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2017-01-26 14:48:01 +00:00
Frediano Ziglio
00ec69f4fe spicevmc: Remove leak of RedPortInitPipeItem::name
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2017-01-26 14:47:52 +00:00
Frediano Ziglio
71e1af9d8b spicevmc: Avoid useless pointer cast
red_channel_client_handle_message already accepts a void* pointer.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2017-01-26 14:46:44 +00:00
Jeremy White
35177a6c41 Avoid a 'missing braces around initializer' warning.
Static variables don't need initializers to be 0.

Signed-off-by: Jeremy White <jwhite@codeweavers.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-01-25 15:41:04 +00:00
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
Pavel Grunt
c1103b76fa build-sys: Warn on usage of unavailable glib functions
Warnings are printed when glib2 >= 2.32 is present

Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-01-24 21:50:38 +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
Victor Toso
aae324b479 Update spice-common
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-01-06 12:33:07 +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