On 05/20/2014 11:19 PM, Jonathon Jongsma wrote:
I've been investigating bug 1086820 that results in a failed assertion in
spice-server (causing the guest to exit) related to mjpeg streaming. This is
the debug output when we hit this issue:
(/usr/bin/qemu-kvm:3165): Spice-ERROR **:
mjpeg_encoder.c:627:mjpeg_encoder_adjust_params_to_bit_rate: assertion
`rate_control->num_recent_enc_frames' failed
Here's what I know so far:
- mjpeg_encoder_adjust_params_to_bit_rate() is actually called quite often with
num_recent_enc_frames set to 0. But usually when num_recent_enc_frames is 0,
last_enc_size is also 0, so we bail out of the function early and print a debug message
"missing sample size".
- Under some circumstances, mjpeg_encoder_reset_quality() gets called with
quality_id set to the same quality id that we're currently using. The code
anticipates this possibility and has a test for it: if the new quality ID is
the same as the old, we don't clear last_enc_size. But we do still clear
num_recent_enc_frames. This is where we become susceptible to the assert
mentioned above. Now last_enc_size is non-zero, but num_recent_enc_frames is 0.
It seems to me that these two values should probably be cleared together. But
I'm not sure whether it is more correct to clear them or *not* clear them when
new quality == old quality.
I've tested with both of the following alternative patches, and they both seem
to work properly and avoid the assert. I'd appreciate input from somebody with
more experience with spice-server streaming code.
Jonathon
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1086820
Hi Jonathon,
The first patch fixes the assert problem, but changes the logic.
It probably affects e.g. the calculation of new_avg_enc_size and new_fps.
The second patch may be missing another place where last_enc_size is set
to 0 (look
for "Not enough space").
A third option is to return from the function if
!rate_control->num_recent_enc_frames
similar to !rate_control->last_enc_size (see a patch below).
I'm not sure which one is better.
Thanks,
Uri.
----
diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index aea4964..4e628c9 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -624,7 +624,10 @@ static void
mjpeg_encoder_adjust_params_to_bit_rate(MJpegEn
return;
}
- spice_assert(rate_control->num_recent_enc_frames);
+ if (!rate_control->num_recent_enc_frames)
+ spice_debug("missing recent sample");
+ return;
+ }
if (rate_control->num_recent_enc_frames < MJPEG_AVERAGE_SIZE_WINDOW &&
rate_control->num_recent_enc_frames < rate_control->fps) {
_______________________________________________
Spice-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/spice-devel
