Commit Graph

73 Commits

Author SHA1 Message Date
Frediano Ziglio
20676792a8 Avoid to access data before a NULL check
If you are testing for NULL data this means that variable could be
NULL so avoid to access before the check to make sure the check is hit.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-30 15:56:43 +01:00
Frediano Ziglio
e3bff1eea4 Remove iterator from list iteration macros
Avoid to have to declare iterator and pass as an argument.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2017-08-21 12:54:47 +01:00
Victor Toso
31fb967f1a display-channel: introduce display_channel_get_nth_stream()
To help avoid stream.c and dcc.c to access display-channel private
structure to get the nth Stream structure pointer.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-06-30 12:28:30 +02:00
Victor Toso
970cb2a1d3 stream: use display_channel_get_stream_id()
As we have a function for that, don't do the math elsewhere.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-06-30 12:28:13 +02:00
Christophe Fergeau
1569d429b7 Remove use of spice_debug(NULL)
This is causing issues with potential improvements to the logging
system, and I've always found this usage a bit odd anyway.
Using spice_debug(""); was not possible as this triggers
-Wformat-zero-length warnings from our use of -Wall.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-06-15 16:40:25 +01:00
Jonathon Jongsma
9b0824bae4 red_channel_client_test_remote_cap() returns bool
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.
2017-04-07 15:08:43 -05:00
Christophe Fergeau
6377b72d44 Use bool rather than int return values when appropriate
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>
2017-03-09 18:39:29 +01:00
Victor Toso
5ba0bc6663 dcc: handle preferred video codec message
[0] SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE

This message provides a list of video codecs based on client's order
of preference.

We duplicate the video codecs array from reds.c and sort it using the
order of codecs as reference.

This message will not change an ongoing streaming but it could change
newly created streams depending the rank value of each video codec
that can be set by spice_server_set_video_codecs()

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-03-02 17:48:17 +00:00
Frediano Ziglio
e4d06191ee stream: Remove region leak
clip_in_draw_dest was not freed correctly.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-02-28 23:05:39 +00:00
Victor Toso
f007c0a991 display-channel: Make video-codecs property read-write
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>
2016-12-06 13:47:00 +01:00
Francois Gouget
97fcad82eb streaming: Always delegate bit rate control to the video encoder
The video encoders already have sophisticated bit rate control code that
can react to both stream reports sent by the client, and server frame
drop notifications. Furthermore they can adjust both the frame rate and
the image quality to best match the network conditions.

But if the client cannot send stream reports all this is bypassed and
instead the streaming code performs its own primitive bit rate control
that can only adjust the frame rate.

So this patch removes the code duplication and lets the video encoders
do their job.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-11-21 16:47:51 +00:00
Jonathon Jongsma
bc8d967e67 Move RedClient to a separate file
Also move the RedClient struct out of the header to avoid accessing the
internals from other files.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-11-02 19:30:58 +00:00
Jonathon Jongsma
96e94c6f32 Convert RedChannel hierarchy to GObject
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-10-25 11:32:13 -05:00
Frediano Ziglio
d38ae73683 Include main-channel-client.h where MainChannelClient is used
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
2016-10-19 21:49:49 +01:00
Frediano Ziglio
a874ebadff Rename detach_stream to stream_detach_drawable and removed unused argument
This function deal only with Stream.
Also the name was misleading and was not clear if it detached the stream
from the DisplayChannel.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2016-09-27 14:25:43 +01:00
Frediano Ziglio
6d4db4f0bf Base FOREACH_DCC on GLIST_FOREACH
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-09-21 16:42:24 +01:00
Frediano Ziglio
3fa2f12c1f Use proper FOREACH_DCC instead of FOREACH_CLIENT
FOREACH_DCC should be more DisplayChannel related.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-09-21 16:42:20 +01:00
Jonathon Jongsma
039d3150ab Change Drawable->pipes from Ring to GList
This improves the readability of the code and keeps things
encapsulated better.
2016-09-19 07:47:05 -05:00
Jonathon Jongsma
a78a7d2510 RedChannelClient: store pipe items in a GQueue
Instead of using a Ring (and having a ring item link in every pipe
item), store them in a GQueue. This also necesitated changing
RedCharDeviceVDIPort->priv->read_bufs to a GList as well.

Also Optimise client pipe by passing pipe position instead of data.
This avoids having the search the data scanning all the queue changing
the order of these operations from O(n) to O(1).

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-09-19 07:47:05 -05:00
Jonathon Jongsma
5e74387d25 Add DisplayChannelPrivate struct
Move all of the DisplayChannel data memembers into a private struct to
encapsulate things better. This necessitated a few new 'public' methods
and a small bit of refactoring to avoid poking into DisplayChannel
internals from too many places. The DisplayChannel and the
DisplayChannelClient are still far too intertwined to completely avoid
accessing private data, so at the moment the private struct is defined
in display-channel.h and the DisplayChannelClient implementation
still accesses it sometimes.
2016-09-16 13:40:09 -05:00
Jonathon Jongsma
fede3f38c5 Improve encapsulation of DisplayChannel
Add a couple new functions to the header so that they can be called by
other objects rather than poking into the internals of the struct.
2016-09-16 13:40:09 -05:00
Jonathon Jongsma
2fa1c1e3fd Use red_channel_client_get_client() everywhere
Instead of having all other classes poke into the RedChannelClient
struct to get the RedClient associated with the channel client, call the
accessor function. This commit allows us to encapsulate RedChannelClient
and move it to its own file soon.
2016-08-30 16:53:29 -05:00
Jonathon Jongsma
d5d59d0858 Add red_channel_client_get_channel()
Don't poke into the structure to get the channel

This prepares for encapsulating RedChannelClient a bit more and
separating it into its own source file.
2016-08-30 16:53:29 -05:00
Frediano Ziglio
76cecd66fb remove CommonGraphicsChannelClient structure
Was just containing a RedChannelClient field

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-08-09 10:22:46 -05:00
Frediano Ziglio
cbc86e7a1f move is_low_bandwidth to DisplayChannelClient
The field is only used by DisplayChannelClient, not by CursorChannelClient

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-08-09 10:22:46 -05:00
Jonathon Jongsma
a84a433e08 Limit direct access to DisplayChannelClient
Add a few more methods and accessors so that other files don't need to
manipulate the struct members directly. Move the struct definition to a
private header which only the dcc-* files will include.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-08-09 10:22:46 -05:00
Frediano Ziglio
f27ee4c0db Reduce indentation inverting condition
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2016-08-02 10:24:12 +01:00
Frediano Ziglio
777a4c4987 Fix typo for macro names
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2016-08-02 10:23:40 +01:00
Frediano Ziglio
67eb0edb62 Remove useless return statement
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2016-07-13 11:22:55 +01:00
Pavel Grunt
58ed103636 Fix missing prototypes
Signed-off-by: Pavel Grunt <pgrunt@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-06-15 15:02:42 +01:00
Francois Gouget
49036e6aae streaming: Avoid copying the input frame in the GStreamer encoder
Note that we can only avoid copies for the first 1 Mpixels or so.
That's because Spice splits larger frames into more chunks than we can
fit GstMemory fragments in a GStreamer buffer. So if there are more
pixels we will avoid copies for the first 3840 KB and copy the rest.
Furthermore, while in practice the GStreamer encoder will only modify
the RedDrawable refcount during the encode_frame(), in theory the
refcount could be decremented from the GStreamer thread after
encode_frame() returns.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
497fcbb0a3 streaming: Let the administrator pick the video encoder and codec
The Spice server administrator can specify the encoder and codec
preferences to optimize for CPU or bandwidth usage. Preferences are
described in a semi-colon separated list of encoder:codec pairs.
The server has a default preference list which can explicitly be
selected by specifying 'auto'.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
f4414af48c streaming: Check the client video codec capabilities
The server picks a codec supported by the client based on the following
new client capabilities:
 * SPICE_DISPLAY_CAP_MULTI_CODEC which denotes a recent client that
   supports multiple codecs. This capability is needed to not have to
   hardcode that MJPEG is supported. This makes it possible to write
   clients that don't support MJPEG.
 * SPICE_DISPLAY_CAP_CODEC_XXX, where XXX is a supported codec. Note
   that for now the server only supports the MJPEG codec.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
a697bd971b streaming: Add a GStreamer 1.0 MJPEG video encoder and use it by default
This introduces a pared down GStreamer-based video encoder to serve as
the basis for later enhancements.
In this form the new encoder supports both regular and sized streams
but lacks any rate control. It should still work fine if bandwidth is
sufficient such as on LANs.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
618255168e streaming: Simplify is_next_stream_frame()
After the removal of Drawable::sized_stream, we no longer need to detect
if the stream changes size in is_next_stream_frame() so it can return a
boolean rather than a value from an enum.
2016-06-14 17:04:40 +02:00
Francois Gouget
47509b1e6e streaming: Remove unused detach_stream() argument
After the removal of Drawable::sized_stream, the last argument to
detach_stream() is no longer used.
2016-06-14 17:04:40 +02:00
Francois Gouget
42a5794845 streaming: Remove the Drawable.sized_stream field
Only red_marshall_stream_data() needs to know whether to send the frame
using a SpiceMsgDisplayStreamDataSized or a regular StreamData message.
So check whether we have a sized frame there and simplify the rest of
the code.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
709c9d71f1 streaming: Better check for sized frames
Usually the RedDrawable bbox dimensions match the src_area dimensions
so that checking that the bbox matches the stream's original dest_area
should be enough to determine if sized stream support is needed to
send the frame.
But making the bbox different could be used to have the scaling be
performed on the client side. So it's better not to assume the bbox and
src_area have the same dimensions.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Jonathon Jongsma
a31efd7e75 RedChannel: Add FOREACH_CLIENT and use it to iterate
Remove the custom FOREACH_DCC macro and use the more generic
FOREACH_CLIENT macro and use it for all channels.
2016-05-24 14:56:45 -05:00
Jonathon Jongsma
4028fb1c79 Replace RedChannel::clients with GList
Instead of using a Ring, use a GList to store the list of channel
clients. This allows us to iterate the clients without poking inside of
the client struct to get the channel_link. This is required in order to
make the RedChannelClient struct private.
2016-05-24 14:56:45 -05:00
Frediano Ziglio
02adcf354e Introduce SPICE_UPCAST macro
This was proposed by Christophe as improvement over some typesafe
patches.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2016-05-24 18:00:51 +01:00
Frediano Ziglio
8782bb3211 Make some function static
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2016-05-24 13:39:48 +01:00
Frediano Ziglio
fb655ff481 reduce casts to RedPipeItem and RingItem
Make code more type safe. This allow to move or delete structure
fields more safely

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-05-21 04:11:12 +01:00
Frediano Ziglio
8d764488ca make red_pipe_item_init_full more typesafe
Use a proper type for free callback

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-05-21 04:11:12 +01:00
Frediano Ziglio
767818da66 Handle STREAM_CREATE and STREAM_DESTROY pipe items more coherently
The items of these pipe items were allocated staticaly inside the
StreamAgent structure. All others RedPipeItem are allocated dynamically.
This could solve possible future maintenance as the life of these
item is more easier to understand.
It's more easier to understand why reference where incremented.
It also make the StreamAgent structure a bit smaller.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-05-17 23:52:06 +01:00
Frediano Ziglio
2c3fc80e51 fix wrong assert causing streaming to crash
This regression was introduced by commit
18d3876b78
("UpgradeItem: use base PipeItem for refcounting")

This fix https://bugs.freedesktop.org/show_bug.cgi?id=95365.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2016-05-13 12:37:54 +01:00
Francois Gouget
0e10629d70 server: Enable adding alternative MJPEG video encoders
This replaces the original mjpeg_encoder API with a VideoEncoder base
class which can be reimplemented by other encoders.
This also renames the members and enums from mjpeg_* to video_*.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2016-05-04 10:51:55 +01:00
Jonathon Jongsma
920e117c24 Rename all RedPipeItem subclasses
Use 'Red' prefix to match internal type naming convention

Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-04-27 10:22:26 -05:00
Jonathon Jongsma
b9720d80e0 Rename PipeItem to RedPipeItem
Following internal type naming conventions

Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-04-27 10:22:01 -05:00
Jonathon Jongsma
18d3876b78 UpgradeItem: use base PipeItem for refcounting
No need to re-implement refcounting in this subclass.

Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-04-27 09:40:57 -05:00