[Spice-devel] [PATCH] mjpeg_encoder: allocate "row" on demand

[Christophe Fergeau]

On Wed, Jul 06, 2011 at 01:04:24PM +0200, Alon Levy wrote:
> On Wed, Jul 06, 2011 at 12:32:53PM +0200, Christophe Fergeau wrote:
> > It's not used when we use jpeg-turbo colorspaces, so it's better
> > to allocate it when we know we'll need it rather than always
> > allocating it even if it won't be used.
> 
> Just one thing - you're turning an abort to a return FALSE. have you tested
> this? does it get handled correctly all the way up?

Nope, I haven't tested it extensively. However the only caller is
red_send_stream_data which already had codepaths returning FALSE before the
mjpeg refactoring, so I'd rather assume that FALSE is correctly handled in
layers above red_send_stream_data. This leaves checking if it's correctly
handled in red_send_stream_data, and it was not ;) I'll squash the patch
below in patch 7/9.

Yaniv also asked about the purpose of that if (width > 3*width) test which
triggers the abort()/return FALSE. My understanding is that it checks for
3*width overflowing MAX_UINT, I'll add a comment saying that as well before
pushing.

diff --git a/server/red_worker.c b/server/red_worker.c
index 70346f8..b547433 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -7294,9 +7294,11 @@ static inline int
red_send_stream_data(DisplayChannel *display_channel,
     }
 
     outbuf_size = display_channel->send_data.stream_outbuf_size;
-    mjpeg_encoder_start_frame(stream->mjpeg_encoder,
     image->u.bitmap.format,
-                              &display_channel->send_data.stream_outbuf,
-                              &outbuf_size);
+    if (!mjpeg_encoder_start_frame(stream->mjpeg_encoder,
image->u.bitmap.format,
+
&display_channel->send_data.stream_outbuf,
+                                   &outbuf_size)) {
+        return FALSE;
+    }
     if (!encode_frame(worker, &drawable->red_drawable->u.copy.src_area,
                       &image->u.bitmap, stream)) {
         return FALSE;

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20110706/12d9d864/attachment.pgp>