Commit Graph

41 Commits

Author SHA1 Message Date
Frediano Ziglio
ed68d491fd Do not check for HAVE_CONFIG_H
This should always be defined and including config.h is a requirement.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-04-30 09:26:55 +01:00
Frediano Ziglio
98b8c725f2 Use proper format strings for spice_log
Formatting string should be compatible with GLib.
GLib uses formatting types compatible with GNU.
For Linux this is not an issue as both systems (like a printf) and
GLib one uses the same formatting type.  However on Windows they
differs potentially causing issues.
This is also make worse as GLib 2.58 changed format attribute from
__printf__ to gnu_printf (Microsoft compatibility formats like %I64d
are still supported but you'll get warnings using GCC/Clang
compilers).

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2019-03-05 13:55:47 +00:00
Frediano Ziglio
777af4fc5d gstreamer-encoder: Use GLib memory functions
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-10-11 12:52:17 +01:00
Frediano Ziglio
14aee7cd74 gstreamer: Check if ORC library can work
ORC library is used internally by GStreamer to generate code
dynamically.
If ORC cannot allocate executable memory, the failure causes
an abort(3) to be called.
This happens on some SELinux configurations that disable executable
memory allocation (execmem boolean).
Check that ORC could work before attempting to use GStreamer to
avoid crashes.
While this check is done, the ORC library outputs an error which will
be well visible in Qemu output.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-30 15:59:46 +01:00
Frediano Ziglio
4725ec03b1 gstreamer: Remove some leaks if pipeline cannot be created
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
2017-04-07 16:45:49 +01:00
Frediano Ziglio
9bad8c306c Fix GStreamer encoding if stride is not 4 bytes aligned
This is required by GStreamer.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
2017-03-22 15:52:15 +00:00
Frediano Ziglio
e3e2cbcb3a Revert "gstreamer: Avoid memory copy if strides are different"
This reverts commit c3d237075b.

When you call gst_buffer_add_video_meta_full GStreamer assumes that
buffer is contiguous. Specifically GStreamer assumes that the first
chunk is big enough to hold a whole frame. This results usually in
some pixel shifts in the video. The pixel shifts you can see are
artifacts due to how the guest sends the frames.

Assuming you allocate the 2 chunks with 2 malloc:

   p1 = malloc(size);
   ...
   p2 = malloc(size);

Usually the memory allocator tend to allocate linearly if there are
space adding a prefix to describe next block leading to a memory
arrangement as:

   +-------------------+
   | p1 prefix         |
   +-------------------+
   | p1 buffer         |
   +-------------------+
   | p2 prefix         |
   +-------------------+
   | p2 buffer         |
   +-------------------+

now if you take p1 pointer and you assume it points to a 2 * size
buffer you will get p2 prefix in the middle, this prefix is the pixel
shifts.

Problems happens specifically in gst_video_frame_map_id.
This bug is reported in https://bugzilla.gnome.org/show_bug.cgi?id=779524.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-03-16 13:19:01 +00:00
Christophe Fergeau
c02ae9835e gstreamer: Add #ifdef around zero_copy()
Since c3d237 "gstreamer: Avoid memory copy if strides are different" is
only needed when zero copy is disabled. This moves the function
definition to an already existing #ifdef block.

Acked-by: Pavel Grunt <pgrunt@redhat.com>
2017-02-15 11:57:59 +00:00
Christophe Fergeau
e87d0a3a84 gstreamer: Remove unused function
rate_control_is_active() is static and not used in gstreamer-encoder.c

Not needed since 97fcad82eb.

Acked-by: Pavel Grunt <pgrunt@redhat.com>
2017-02-15 10:50:27 +00: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
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
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
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
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
41d52961db gstreamer: Correctly don't allow too limited bit rates
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>
2016-12-02 08:39:51 +00: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
Frediano Ziglio
ffb47ecd88 gstreamer: Use pthread to avoid Glib compatibility issues with RHEL6
GCond/GMutex interface is different between Glib 2.32 and
previous versions. Use pthread for compatibility.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2016-11-15 10:41:00 +00:00
Frediano Ziglio
c18b696f1c Use SPICE_VERIFY macro for RHEL6 compatibility
The verify macro used currently has some problem
as it raise a warning in RHEL6.
Use new SPICE_VERIFY macro defined in spice-common
to avoid this issue.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2016-11-14 17:22:02 +00:00
Francois Gouget
fff92908b2 streaming: Clarify GStreamer's virtual buffer size documentation
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-11-01 12:36:04 +00:00
Francois Gouget
96818da95d streaming: Limit the h264 image compression level
When uncapped x264enc can compress the frames beyond recognition in low
bitrate situation. Beyond the set limit the gains are modest and it is
better to drop frames to reduce the bit rate further.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-10-29 10:33:36 +01:00
Frediano Ziglio
a318f3e981 gstreamer: Use a dummy format to make sure format is always defined.
This avoid a check for NULL.
Also will be used to catch invalid values when table will be extended.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
2016-08-11 14:47:25 +01:00
Frediano Ziglio
0a13066396 gstreamer: Peephole optimisation for SpiceFormatForGStreamer
Reduce structure length using static allocated string inside the
structure.
This will also avoid using .data.rel.ro section and relocations
reducing even more library size.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
2016-08-11 14:12:21 +01:00
Frediano Ziglio
62cc9643c3 gstreamer: Fix infinite loop in get_period_bit_rate
This was discovered by chance by me and Uri trying some remote connection
with slow network (8Mbit) and high latency

  $ ping 10.10.48.87 -c 3
  3 packets transmitted, 3 received, 0% packet loss, time 2002ms
  rtt min/avg/max/mdev = 281.069/316.758/374.413/41.153 ms

The encoder->history status was (edited for readability):

(gdb) p ((SpiceGstEncoder*)0x61c000264880)->history_first
$6 = 29
(gdb) p ((SpiceGstEncoder*)0x61c000264880)->history_last
$14 = 28
(gdb) p ((SpiceGstEncoder*)0x61c000264880)->history
[0] {mm_time = 11298131, duration = 7391006, size = 5616},
[1] {mm_time = 11298148, duration = 7373663, size = 5559},
[2] {mm_time = 11298166, duration = 7052209, size = 5511},
[3] {mm_time = 11298183, duration = 7006828, size = 5722},
[4] {mm_time = 11298199, duration = 7433311, size = 5756},
[5] {mm_time = 11298216, duration = 7134734, size = 5545},
[6] {mm_time = 11298232, duration = 7436589, size = 5521},
[7] {mm_time = 11298249, duration = 7152181, size = 5540},
[8] {mm_time = 11298266, duration = 7181308, size = 7796},
[9] {mm_time = 11298283, duration = 5053084, size = 50824},
[10] {mm_time = 11298298, duration = 7472305, size = 7427},
[11] {mm_time = 11298315, duration = 7385017, size = 6682},
[12] {mm_time = 11298333, duration = 7287125, size = 6377},
[13] {mm_time = 11298349, duration = 7191461, size = 6159},
[14] {mm_time = 11298367, duration = 7104546, size = 6035},
[15] {mm_time = 11298382, duration = 7266942, size = 5896},
[16] {mm_time = 11298400, duration = 7108001, size = 5812},
[17] {mm_time = 11298418, duration = 7020583, size = 5807},
[18] {mm_time = 11298433, duration = 7066056, size = 5758},
[19] {mm_time = 11298450, duration = 7052900, size = 5676},
[20] {mm_time = 11298467, duration = 7248233, size = 5765},
[21] {mm_time = 11298483, duration = 7077348, size = 5712},
[22] {mm_time = 11298502, duration = 7495368, size = 5835},
[23] {mm_time = 11298517, duration = 7068626, size = 5805},
[24] {mm_time = 11298534, duration = 7060375, size = 5801},
[25] {mm_time = 11298551, duration = 7020383, size = 5868},
[26] {mm_time = 11298568, duration = 7248400, size = 5830},
[27] {mm_time = 11298584, duration = 7001304, size = 6621},
[28] {mm_time = 11298600, duration = 7311600, size = 6113},
[29] {mm_time = 11297612, duration = 6999174, size = 5666},  <--- to
[30] {mm_time = 11297628, duration = 7506688, size = 5502},
[31] {mm_time = 11297646, duration = 7209494, size = 5687},
[32] {mm_time = 11297663, duration = 7396429, size = 5724},
[33] {mm_time = 11297679, duration = 7172624, size = 5839},
[34] {mm_time = 11297696, duration = 7300811, size = 5645},
[35] {mm_time = 11297713, duration = 7108985, size = 5553},
[36] {mm_time = 11297729, duration = 7171701, size = 5774},
[37] {mm_time = 11297745, duration = 7174018, size = 5637},
[38] {mm_time = 11297762, duration = 7313549, size = 5655},
[39] {mm_time = 11297780, duration = 5183985, size = 51014},
[40] {mm_time = 11297796, duration = 7038329, size = 7374},
[41] {mm_time = 11297813, duration = 7211506, size = 6585},
[42] {mm_time = 11297830, duration = 7112690, size = 5729},
[43] {mm_time = 11297846, duration = 7103074, size = 5761},
[44] {mm_time = 11297864, duration = 7599826, size = 5661},
[45] {mm_time = 11297879, duration = 7355392, size = 5351},
[46] {mm_time = 11297897, duration = 7454367, size = 5488},
[47] {mm_time = 11297913, duration = 7127145, size = 5573},
[48] {mm_time = 11297932, duration = 7550098, size = 5447},
[49] {mm_time = 11297948, duration = 7506884, size = 5809},
[50] {mm_time = 11297966, duration = 7405712, size = 5783},
[51] {mm_time = 11297982, duration = 7182025, size = 5599},
[52] {mm_time = 11298001, duration = 7323887, size = 5817},
[53] {mm_time = 11298014, duration = 7342091, size = 5575},
[54] {mm_time = 11298031, duration = 7596319, size = 5739},
[55] {mm_time = 11298052, duration = 7428169, size = 5669},
[56] {mm_time = 11298065, duration = 7457282, size = 5732},
[57] {mm_time = 11298081, duration = 7174029, size = 5541},
[58] {mm_time = 11298097, duration = 7340512, size = 5680},
[59] {mm_time = 11298115, duration = 7427439, size = 5701}}
(gdb) p from
$16 = 11297544
(gdb) p to
$17 = 11297612

You can see that encoder->history[encoder->history_first].mm_time == to
cause the check index == encoder->history_first to not be executed.

This code change was suggested by Uri.

Reported-by: Uri Lublin <uril@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Tested-by: Uri Lublin <uril@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
2016-08-11 14:12:21 +01:00
Frediano Ziglio
2f17d4745e gstreamer: Use static compile check
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
2016-08-11 11:24:49 +01:00
Frediano Ziglio
0d2c80c58b gstreamer: Allows to add output format to options
This allow option string to contain separator so you could set as
"field=value ! format".
This is useful as some encoders use the output format to specify
compression types (for instance with some H264 encoders you can
specify the profile to use).

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
2016-08-03 12:57:41 +01:00
Frediano Ziglio
22d41e71ae gstreamer: Peephole optimisation
SpiceGstFrameInformation change from 24 to 16 bytes reducing
SpiceGstEncoder 480 bytes.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
2016-08-03 11:09:42 +01:00
Francois Gouget
0503cd3d61 streaming: Add support for GStreamer 0.10
configure will use GStreamer 1.0 if present and fall back to
GStreamer 0.10 otherwise.
ffenc_mjpeg takes its bitrate as a long so extend set_gstenc_bitrate().

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
b64b9e5657 streaming: Dynamically adjust the GStreamer encoder bitrate if possible
This is faster and lets the encoder leverage past bitrate shaping
history to attain the target faster.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
fc68e2e5ba streaming: Respect the GStreamer encoder's valid bit rate range
GObject returns an error instead of clamping if given an out of range
property value.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
c6f6471eb8 streaming: Give up after a while if GStreamer cannot handle the video
This typically happens when sending very small frames (less than
16 pixels in one dimension) to the x264enc encoder.
This avoids repeatedly wasting time rebuilding the pipeline.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
ef6d6b063d streaming: Adjust the frame rate based on the GStreamer encoding time
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
669d7090ca streaming: Adjust the GStreamer encoder bit rate to the network
The video encoder uses the client reports and/or notifications of
server frame drops as its feedback mechanisms. In particular it keeps
track of the maximum video margin and reduces the bit rate whenever the
margin goes below certain thresholds or decreases too sharply.
It uses these to figure out the lowest bit rate that causes negative
feedback, and the highest bit rate that allows a return to positive
feedbacks. It then works to narrow this range and settles on the lower
end once the spread has gone below a given threshold.
All the while it monitors the effective bit rate to ensure the target
bit rate does not grow significantly beyond what the GStreamer encoder
will produce: this avoids target bit rate 'bubbles' which would
invariably be followed by a bit rate crash with accompanying frame loss.
As soon as the network feedback indicates a significant degradation the
bit rate is lowered to minimize the risk of frame loss and/or long
freezes.
It also relies on the existing shaping of the GStreamer output bit rate
to minimize the pipeline reconfigurations.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
4f04c9ac79 streaming: Shape the bit rate of the GStreamer codecs output
The GStreamer codecs don't follow the specified bit rate very closely:
they can decide to exceed it for ten seconds or more if they consider
the scene deserves it. Such long bursts are enough to cause network
congestion, resulting in many lost frames which cause significant
display corruption.
So the GStreamer video encoder now uses a short 300ms virtual buffer
to shape the compressed video output and ensure we don't exceed the
target bit rate for any significant length of time.
It could instead rely on the network feedback (when available) to lower
the bit rate. However frequent GStreamer bit rate changes lower the
overall compression level and also result in a lower average bit rate,
both of which result in lower video quality.
The GStreamer video encoder also keeps track of the encoded frame size
so it can gather statistics and call update_client_playback_delay()
with accurate information and also annotate the client report debug
traces with the corresponding bit rate information.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
a21ec4e251 streaming: Add h264 support to the GStreamer video encoder
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02: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
a6795d1971 streaming: Handle and recover from GStreamer encoding errors
If an error occurs for whatever reason (e.g. codec not supporting odd
frame sizes), the GStreamer pipeline will drop the current buffer,
causing the encoder to be stuck waiting for the sample. So this patch
tracks error notifications and ensures we don't wait for a sample if
none will come.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
2db59fef3a streaming: Let the video encoder manage the compressed buffer
This way the video encoder is not forced to use malloc()/free().
This also allows more flexibility in how the video encoder manages the
buffer which allows for a zero-copy implementation in both video
encoders.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-06-14 17:04:40 +02:00
Francois Gouget
6dc0dadf8d streaming: Add VP8 support to the GStreamer video encoder
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
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