After the previous patch moving self_bitmap freeing inside red_drawable
ref count, we have a possible self_bitmap leak:
red_process_commands
red_get_drawable | red_drawable #1, red_drawable->self_bitmap == 1
red_process_drawable | rd #2, d #1, d->self_bitmap != NULL
release_drawable | rd #1, d# = 0, try to release self_bitmap, but
blocked by rd #1
put_red_drawable | rd #0
This patch moves the call to release_drawable after the call to
put_red_drawable, thereby fixing the above situation.
rhbz #813826
When playing a youtube video on Windows guest, the driver sometimes(**) sends
images which contain the video frames, but also other parts of the
screen (e.g., the youtube process bar). In order to prevent glitches, we send these
images as part of the stream, using SPICE_MSG_DISPLAY_STREAM_DATA_SIZED.
(**) It happens regularly with the you tube html5 player. With flash,
it occurs when moving the cursor in the player area.
spice-common changes: STREAM_DATA_SIZED message was added in order to support
video streams with frames that their size is different from the initial size
that the stream was created with.
This patch also includes server and client adjustments to the new
SpiceMsgDisplayStreamData.
The previous patch took care that streams will be upgraded by a surface
screenshot and not the last frame, if necessary. Thus, there is no
more a reason for detaching drawables from streams when they are
rendered. Moreover, detaching such drawables can cause glitches, in
case they are still in the pipe, and red_update_area is called
frequently and leads to stream frames rendering.
Upgrading a stream: When the stream's visible region is bigger than the one of the last
frame, we will send an updated screenshot of the visible region, instead
of sending the last frame losslessly.
Differentiate between the clipping of the video stream, and the region
that currently displays fragments of the video stream (henceforth,
vis_region). The latter equals or contains the former one. For example,
let c1 be the clip area at time t1, and c2 be the clip area at time t2,
where t1 < t2. If c1 contains c2, and at least part of c1/c2, hasn't been
covered by a non-video images, vis_region will contain c2, and also the part
of c1/c2 that still displays fragments of the video.
When we consider if a stream should be "upgraded" (1), due to its area
being used by a rendering operation, or due to stopping the video, we
should take into account the vis_region, and not the clip region (next
patch: not upgrade by the last frame, but rather by vis_region).
This fix will be more necessary when sized frames are introduced (see the
following patches). Then, the vis_region might be larger
than the last frame, and contain it, more frequently than before.
(1) "upgrading a stream" stands for sending its last frame losslessly. Or more
precisely, lossless resending of all the currently displayed lossy areas, that were
sent as part of the stream.
Methods which longjump, unconditionally raise an
exception, or call _exit() cannot return control
to the caller so should be annotated with 'noreturn'
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The 3 part of the conditional in the is_equal_brush method
compared the b1->u.color field to itself, instead of b2->u.color
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Tell the compiler that was really do intend to cast from int
to pointer, to prevent warnings about implicit casts
* server/tests/test_display_base.c: Add explicit casts
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If a 'goto' statement jumps across a variable declaration
which also has an initializer, the variable is in an undefined
state. Splitting the the declaration & initialization doesn't
change that, but the compiler can at least now detect use of
the unintialized variable
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The red_worker_main method allocates a RedWorker struct instance
on the stack. This struct is a full 2 MB in size which is not
at all resonable to allocate on the stack.
* server/red_worker.c: Move RedWorker struct to the heap
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When using setjmp/longjmp the state of local variables can be
undefined in certain scenarios:
[quote man(longjmp)]
The values of automatic variables are unspecified after a
call to longjmp() if they meet all the following criteria:
· they are local to the function that made the correspond‐
ing setjmp(3) call;
· their values are changed between the calls to setjmp(3)
and longjmp(); and
· they are not declared as volatile.
[/quote]
* server/red_worker.c: Mark some vars as volatile
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
* client/red_channel.cpp: AbortTrigger::on_event can't return
given its current impl
* server/red_worker.c: red_worker_main can't return
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
All printf var-args style methods should be annotation with
their format. All format strings must be const strings.
* client/application.cpp, client/cmd_line_parser.cpp,
client/hot_keys.cpp: Avoid non-const format
* client/client_net_socket.cpp: Fix broken format specifier
* client/red_peer.cpp: Fix missing format specifier
* client/platform.h: Add SPICE_GNUC_PRINTF annotation to term_printf
* client/utils.h: Add SPICE_GNUC_PRINTF annotation to string_printf
* server/glz_encoder_config.h, server/red_worker.c: Add
SPICE_GNUC_PRINTF annotation to warning callbacks
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
* server/red_worker.c: Add missing const for return type
* server/reds.c: Static strings must be declared const
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Arithmetic on void * types is non-portable & trivially avoided
* server/dispatcher.c: Use uint8_t for arithmetic
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This patch changed getvirt to continue working even if spice_critical
doesn't abort (i.e. SPICE_ABORT_LEVEL != -1). This is in preparation to
make getvirt not abort at all. The reason is that getvirt is run on
guest provided memory, so a bad driver can crash the vm.
This patch will replace the common/ directory with the spice-common
project. It is for now a simple project subdirectory shared with
spice-gtk, but the goal is to make it a proper library later on.
With this change, the spice-server build is broken. The following
commits fix the build, and have been seperated to ease the review.
v2
- moves all the generated marshallers to spice-common library
- don't attempt to fix windows VS build, which should somehow be
splitted with spice-common (or built from tarball only to avoid
generation tools/libs deps)
v3
- uses libspice-common-client
- fix a mutex.h inclusion reported by Alon
