[Spice-commits] 4 commits - gtk/channel-display-mjpeg.c
Christophe Fergau
teuf at kemper.freedesktop.org
Mon Aug 1 02:06:18 PDT 2011
gtk/channel-display-mjpeg.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
New commits:
commit a73f841b59faadccec2c0c410f15149d4a5cb5bb
Author: Christophe Fergeau <cfergeau at redhat.com>
Date: Fri Jul 29 16:40:23 2011 +0200
mjpeg: don't leak last stream image
When a stream is destroy, the memory allocated to handle the mjpeg
decoding is freed by calling stream_mjpeg_cleanup. However, the
memory allocated to contain the last uncompressed stream image
wasn't freed.
diff --git a/gtk/channel-display-mjpeg.c b/gtk/channel-display-mjpeg.c
index 9e8bc0b..cb1bc87 100644
--- a/gtk/channel-display-mjpeg.c
+++ b/gtk/channel-display-mjpeg.c
@@ -138,4 +138,6 @@ G_GNUC_INTERNAL
void stream_mjpeg_cleanup(display_stream *st)
{
jpeg_destroy_decompress(&st->mjpeg_cinfo);
+ free(st->out_frame);
+ st->out_frame = NULL;
}
commit bb30232fcdd05af5c0cf9a2819a245fe99dc3538
Author: Christophe Fergeau <cfergeau at redhat.com>
Date: Fri Jul 29 13:38:09 2011 +0200
mjpeg: remove wrong g_return_if_fail
After calling jpeg_read_scanlines, spice-gtk checks that we read
the amount of lines we expected, and if not, it returns early.
This is doubly wrong:
* jpeg_read_scanlines is documented as returning at most the number
of lines requested, but it also warns that an application shouldn't
rely on getting exactly the number of scanlines requested. In this
case, if rec_outbuf_height is bigger than 1, we'll get a short read
on the last line of odd-sized images, thus triggering the
g_return_if_fail
* returning from this function without calling jpeg_abort will cause
libjpeg to abort next time we use it because jpeg_start_decompress
was called before
This commit removes this check and early return.
diff --git a/gtk/channel-display-mjpeg.c b/gtk/channel-display-mjpeg.c
index 3789c87..9e8bc0b 100644
--- a/gtk/channel-display-mjpeg.c
+++ b/gtk/channel-display-mjpeg.c
@@ -116,8 +116,6 @@ void stream_mjpeg_data(display_stream *st)
}
lines_read = jpeg_read_scanlines(&st->mjpeg_cinfo, lines,
st->mjpeg_cinfo.rec_outbuf_height);
- // this shouldn't happen either..
- g_return_if_fail(lines_read == st->mjpeg_cinfo.rec_outbuf_height);
#ifndef JCS_EXTENSIONS
{
uint8_t *s = lines[0];
commit b306504cf967d9ec55a0de57f2261a0dfa521443
Author: Christophe Fergeau <cfergeau at redhat.com>
Date: Fri Jul 29 15:33:38 2011 +0200
mjpeg: restrict use of i and j
Using i and j as variable names that are used from one loop to the
other isn't really readable, and makes the code more fragile than
it could be. This commits adds a "lines_read" variable which is more
expressive than "j", restricts "j" lifetime to the loop where it's
used, and it removes the "i" variable and uses counters provided
by libjpeg to iterate all the image lines.
It also has the side-effect that if jpeg_read_scanlines returns a short
read (less lines than expected are read), "dest" won't go out of sync but
will be set to the right place at the end of the loop.
diff --git a/gtk/channel-display-mjpeg.c b/gtk/channel-display-mjpeg.c
index dee0ef9..3789c87 100644
--- a/gtk/channel-display-mjpeg.c
+++ b/gtk/channel-display-mjpeg.c
@@ -69,7 +69,6 @@ void stream_mjpeg_data(display_stream *st)
int height = info->stream_height;
uint8_t *dest;
uint8_t *lines[4];
- int i, j;
dest = malloc(width * height * 4);
@@ -103,8 +102,11 @@ void stream_mjpeg_data(display_stream *st)
g_return_if_reached();
}
- for (i = 0; i < height; ) {
- for (j = 0; j < st->mjpeg_cinfo.rec_outbuf_height; j++) {
+ while (st->mjpeg_cinfo.output_scanline < st->mjpeg_cinfo.output_height) {
+ /* only used when JCS_EXTENSIONS is undefined */
+ G_GNUC_UNUSED unsigned int lines_read;
+
+ for (unsigned int j = 0; j < st->mjpeg_cinfo.rec_outbuf_height; j++) {
lines[j] = dest;
#ifdef JCS_EXTENSIONS
dest += 4 * width;
@@ -112,16 +114,16 @@ void stream_mjpeg_data(display_stream *st)
dest += 3 * width;
#endif
}
- j = jpeg_read_scanlines(&st->mjpeg_cinfo, lines,
+ lines_read = jpeg_read_scanlines(&st->mjpeg_cinfo, lines,
st->mjpeg_cinfo.rec_outbuf_height);
// this shouldn't happen either..
- g_return_if_fail(j == st->mjpeg_cinfo.rec_outbuf_height);
+ g_return_if_fail(lines_read == st->mjpeg_cinfo.rec_outbuf_height);
#ifndef JCS_EXTENSIONS
{
uint8_t *s = lines[0];
- uint32_t *d = (uint32_t *)&st->out_frame[i * width * 4];
+ uint32_t *d = (uint32_t *)s;
- for (j = j * width; j > 0; ) {
+ for (unsigned int j = lines_read * width; j > 0; ) {
j -= 1; // reverse order, bad for cache?
d[j] = s[j * 3 + 0] << 16 |
s[j * 3 + 1] << 8 |
@@ -129,8 +131,7 @@ void stream_mjpeg_data(display_stream *st)
}
}
#endif
- i += st->mjpeg_cinfo.rec_outbuf_height;
- dest = &st->out_frame[i * width * 4];
+ dest = &st->out_frame[st->mjpeg_cinfo.output_scanline * width * 4];
}
jpeg_finish_decompress(&st->mjpeg_cinfo);
}
commit 756caca387ab17281afe7362f5cd56613d9fda92
Author: Christophe Fergeau <cfergeau at redhat.com>
Date: Fri Jul 29 13:23:28 2011 +0200
mjpeg: properly abort decompression in error path
spice-gtk jpeg decompression code honors libjpeg's recommended size
for its output buffer to improve performance. However, when this
recommended size is too big, it just gives up on the decompression
process by returning early from the function. But since
jpeg_start_decompress has been called to compute this recommended
size, the decompression must be aborted before returning, otherwise
libjpeg will get in an inconsistent state and will abort next time
we try to use it.
This commit also moves the check that the recommended size isn't
too big out of the decompression loop because it shouldn't changed
during decompression.
diff --git a/gtk/channel-display-mjpeg.c b/gtk/channel-display-mjpeg.c
index b9ff108..dee0ef9 100644
--- a/gtk/channel-display-mjpeg.c
+++ b/gtk/channel-display-mjpeg.c
@@ -95,9 +95,15 @@ void stream_mjpeg_data(display_stream *st)
#endif
// TODO: in theory should check cinfo.output_height match with our height
jpeg_start_decompress(&st->mjpeg_cinfo);
+ /* rec_outbuf_height is the recommended size of the output buffer we
+ * pass to libjpeg for optimum performance
+ */
+ if (st->mjpeg_cinfo.rec_outbuf_height > G_N_ELEMENTS(lines)) {
+ jpeg_abort_decompress(&st->mjpeg_cinfo);
+ g_return_if_reached();
+ }
+
for (i = 0; i < height; ) {
- // according to header
- g_return_if_fail(st->mjpeg_cinfo.rec_outbuf_height <= 4);
for (j = 0; j < st->mjpeg_cinfo.rec_outbuf_height; j++) {
lines[j] = dest;
#ifdef JCS_EXTENSIONS
More information about the Spice-commits
mailing list