[Spice-commits] 3 commits - client/display_channel.cpp client/mjpeg_decoder.cpp client/mjpeg_decoder.h common/messages.h python_modules/demarshal.py server/red_worker.c spice1.proto spice.proto

Alexander Larsson alexl at kemper.freedesktop.org
Tue Jul 20 02:47:05 PDT 2010


 client/display_channel.cpp  |   11 ++++------
 client/mjpeg_decoder.cpp    |   29 ++++++++++++++++++---------
 client/mjpeg_decoder.h      |    3 +-
 common/messages.h           |    1 
 python_modules/demarshal.py |   46 ++++++++++++++++++++++++++++++--------------
 server/red_worker.c         |    7 +-----
 spice.proto                 |    2 -
 spice1.proto                |    4 +--
 8 files changed, 62 insertions(+), 41 deletions(-)

New commits:
commit ccbb922d5ae5757343df64c4184ef3927e680bf9
Author: Alexander Larsson <alexl at redhat.com>
Date:   Tue Jul 20 11:45:37 2010 +0200

    Swap red and blue when decoding 0.4 mjpeg streams
    
    There was an error in how this was encoded in 0.4, which we need
    to handle. There is still some issues with the old streams as
    the luminocity handling in 0.4 was not correct.

diff --git a/client/display_channel.cpp b/client/display_channel.cpp
index 4f89e06..c371f4a 100644
--- a/client/display_channel.cpp
+++ b/client/display_channel.cpp
@@ -290,7 +290,7 @@ VideoStream::VideoStream(RedClient& client, Canvas& canvas, DisplayChannel& chan
         _pixmap.width = src_width;
         _pixmap.height = src_height;
 
-	_mjpeg_decoder = new MJpegDecoder(stream_width, stream_height, _stride, _uncompressed_data);
+	_mjpeg_decoder = new MJpegDecoder(stream_width, stream_height, _stride, _uncompressed_data, channel.get_peer_major() == 1);
 
 #ifdef WIN32
         SetViewportOrgEx(_dc, 0, stream_height - src_height, NULL);
diff --git a/client/mjpeg_decoder.cpp b/client/mjpeg_decoder.cpp
index 42bf2f9..8a3aede 100644
--- a/client/mjpeg_decoder.cpp
+++ b/client/mjpeg_decoder.cpp
@@ -65,7 +65,8 @@ extern "C" {
 
 MJpegDecoder::MJpegDecoder(int width, int height,
                            int stride,
-                           uint8_t *frame) :
+                           uint8_t *frame,
+                           bool back_compat) :
     _data(NULL)
     , _data_size(0)
     , _data_start(0)
@@ -75,6 +76,7 @@ MJpegDecoder::MJpegDecoder(int width, int height,
     , _height(height)
     , _stride(stride)
     , _frame(frame)
+    , _back_compat(back_compat)
     , _y(0)
     , _state(0)
 {
@@ -114,16 +116,23 @@ void MJpegDecoder::convert_scanline(void)
    row = (uint32_t *)(_frame + _y * _stride);
     s = _scanline;
 
-    /* TODO after major bump.
-       We need to check for the old major and for backwards compat
-       a) swap r and b
-       b) to-yuv with right values and then from-yuv with old wrong values
-    */
 
-    for (x = 0; x < _width; x++) {
-        c = s[0] << 16 | s[1] << 8 | s[2];
-        s += 3;
-        *row++ = c;
+    if (_back_compat) {
+        /* We need to check for the old major and for backwards compat
+           a) swap r and b (done)
+           b) to-yuv with right values and then from-yuv with old wrong values (TODO)
+        */
+        for (x = 0; x < _width; x++) {
+            c = s[2] << 16 | s[1] << 8 | s[0];
+            s += 3;
+            *row++ = c;
+        }
+    } else {
+        for (x = 0; x < _width; x++) {
+            c = s[0] << 16 | s[1] << 8 | s[2];
+            s += 3;
+            *row++ = c;
+        }
     }
 }
 
diff --git a/client/mjpeg_decoder.h b/client/mjpeg_decoder.h
index f435f3f..ccafd37 100644
--- a/client/mjpeg_decoder.h
+++ b/client/mjpeg_decoder.h
@@ -37,7 +37,7 @@ extern "C" {
 class MJpegDecoder {
 public:
     MJpegDecoder(int width, int height, int stride,
-                 uint8_t *frame);
+                 uint8_t *frame, bool back_compat);
     ~MJpegDecoder();
 
     bool decode_data(uint8_t *data, size_t length);
@@ -63,6 +63,7 @@ private:
     int _height;
     int _stride;
     uint8_t *_frame;
+    bool _back_compat;
 
     int _y;
     uint8_t *_scanline;
commit 2e9604d00d655f276f22b9ad1cce4fb631ac06a8
Author: Alexander Larsson <alexl at redhat.com>
Date:   Tue Jul 20 11:34:23 2010 +0200

    demarshaller: Fix palette marshalling
    
    It turns out that using base + sizeof(struct) is not a good way to
    access an array at the end of a struct. For SpicePalette sizeof is 16, but
    offset of ents is 12. Using this calculation in the demarshaller breaks
    things badly, so now we use the actual array member.

diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index a4345d4..c588f98 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -797,29 +797,47 @@ def write_array_parser(writer, member, nelements, array, dest, scope):
     is_byte_size = array.is_bytes_length()
 
     element_type = array.element_type
+    if member:
+        array_start = dest.get_ref(member.name)
+        at_end = member.has_attr("end")
+    else:
+        array_start = "end"
+        at_end = True
+
     if element_type == ptypes.uint8 or element_type == ptypes.int8:
-        if not member or member.has_attr("end"):
-            writer.statement("memcpy(end, in, %s)" % (nelements))
-            writer.increment("end", nelements)
-        else:
-            writer.statement("memcpy(%s, in, %s)" % (dest.get_ref(member.name), nelements))
+        writer.statement("memcpy(%s, in, %s)" % (array_start, nelements))
         writer.increment("in", nelements)
+        if at_end:
+            writer.increment("end", nelements)
     else:
-        if array.has_attr("ptr_array"):
-            scope.variable_def("void **", "ptr_array")
-            scope.variable_def("int", "ptr_array_index")
-            writer.assign("ptr_array_index", 0)
-            writer.assign("ptr_array", "(void **)end")
-            writer.increment("end", "sizeof(void *) * %s" % nelements)
         with writer.index() as index:
+            if member:
+                array_pos = "%s[%s]" % (array_start, index)
+            else:
+                array_pos = "*(%s *)end" % (element_type.c_type())
+
+            if array.has_attr("ptr_array"):
+                scope.variable_def("void **", "ptr_array")
+                scope.variable_def("int", "ptr_array_index")
+                writer.assign("ptr_array_index", 0)
+                writer.assign("ptr_array", "(void **)%s" % array_start)
+                writer.increment("end", "sizeof(void *) * %s" % nelements)
+                array_start = "end"
+                array_pos = "*(%s *)end" % (element_type.c_type())
+                at_end = True
+
             with writer.for_loop(index, nelements) as array_scope:
                 if array.has_attr("ptr_array"):
                     writer.statement("ptr_array[ptr_array_index++] = end")
                 if element_type.is_primitive():
-                    writer.statement("*(%s *)end = consume_%s(&in)" % (element_type.c_type(), element_type.primitive_type()))
-                    writer.increment("end", element_type.sizeof())
+                    writer.statement("%s = consume_%s(&in)" % (array_pos, element_type.primitive_type()))
+                    if at_end:
+                        writer.increment("end", element_type.sizeof())
                 else:
-                    dest2 = dest.child_at_end(writer, element_type)
+                    if at_end:
+                        dest2 = dest.child_at_end(writer, element_type)
+                    else:
+                        dest2 = RootDemarshallingDestination(dest, element_type.c_type(), element_type.c_type(), array_pos)
                     dest2.reuse_scope = array_scope
                     write_container_parser(writer, element_type, dest2)
                 if array.has_attr("ptr_array"):
commit 5f664e0ffb62c4a1d13b7647aea3c1115150eb88
Author: Alexander Larsson <alexl at redhat.com>
Date:   Tue Jul 20 11:33:57 2010 +0200

    Don't send padding over the network with video data

diff --git a/client/display_channel.cpp b/client/display_channel.cpp
index e66f845..4f89e06 100644
--- a/client/display_channel.cpp
+++ b/client/display_channel.cpp
@@ -158,7 +158,7 @@ public:
                 SpiceRect* dest, int clip_type, uint32_t num_clip_rects, SpiceRect* clip_rects);
     ~VideoStream();
 
-    void push_data(uint32_t mm_time, uint32_t length, uint8_t* data, uint32_t pad_size);
+    void push_data(uint32_t mm_time, uint32_t length, uint8_t* data);
     void set_clip(int type, uint32_t num_clip_rects, SpiceRect* clip_rects);
     const SpiceRect& get_dest() {return _dest;}
     void handle_update_mark(uint64_t update_mark);
@@ -445,7 +445,7 @@ uint32_t VideoStream::alloc_frame_slot()
     return frame_slot(_frames_head++);
 }
 
-void VideoStream::push_data(uint32_t mm_time, uint32_t length, uint8_t* data, uint32_t pad_size)
+void VideoStream::push_data(uint32_t mm_time, uint32_t length, uint8_t* data)
 {
     maintenance();
     uint32_t frame_slot = alloc_frame_slot();
@@ -1345,12 +1345,11 @@ void DisplayChannel::handle_stream_data(RedPeer::InMessage* message)
         THROW("invalid stream");
     }
 
-    if (message->size() < sizeof(SpiceMsgDisplayStreamData) + stream_data->data_size + stream_data->pad_size) {
+    if (message->size() < sizeof(SpiceMsgDisplayStreamData) + stream_data->data_size) {
         THROW("access violation");
     }
 
-    stream->push_data(stream_data->multi_media_time, stream_data->data_size, stream_data->data,
-                      stream_data->pad_size);
+    stream->push_data(stream_data->multi_media_time, stream_data->data_size, stream_data->data);
 }
 
 void DisplayChannel::handle_stream_clip(RedPeer::InMessage* message)
diff --git a/common/messages.h b/common/messages.h
index 8af38f5..1a60a9f 100644
--- a/common/messages.h
+++ b/common/messages.h
@@ -288,7 +288,6 @@ typedef struct SpiceMsgDisplayStreamData {
     uint32_t id;
     uint32_t multi_media_time;
     uint32_t data_size;
-    uint32_t pad_size;
     uint8_t data[0];
 } SpiceMsgDisplayStreamData;
 
diff --git a/server/red_worker.c b/server/red_worker.c
index bfb4560..bc3a7a3 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -8012,8 +8012,6 @@ static int red_rgb16bpp_to_24 (RedWorker *worker, const SpiceRect *src,
     return TRUE;
 }
 
-#define PADDING 8 /* old ffmpeg padding */
-
 static inline int red_send_stream_data(DisplayChannel *display_channel, Drawable *drawable)
 {
     Stream *stream = drawable->stream;
@@ -8074,7 +8072,7 @@ static inline int red_send_stream_data(DisplayChannel *display_channel, Drawable
 
     while ((n = mjpeg_encoder_encode_frame(stream->mjpeg_encoder,
                                            display_channel->send_data.stream_outbuf,
-                                           display_channel->send_data.stream_outbuf_size - PADDING)) == 0) {
+                                           display_channel->send_data.stream_outbuf_size)) == 0) {
         uint8_t *new_buf;
         size_t new_size;
 
@@ -8095,10 +8093,9 @@ static inline int red_send_stream_data(DisplayChannel *display_channel, Drawable
     stream_data.id = stream - worker->streams_buf;
     stream_data.multi_media_time = drawable->red_drawable->mm_time;
     stream_data.data_size = n;
-    stream_data.pad_size = PADDING;
     spice_marshall_msg_display_stream_data(channel->send_data.marshaller, &stream_data);
     spice_marshaller_add_ref(channel->send_data.marshaller,
-                             display_channel->send_data.stream_outbuf, n + PADDING);
+                             display_channel->send_data.stream_outbuf, n);
 
     display_begin_send_message(display_channel, NULL);
     agent->lats_send_time = time_now;
diff --git a/spice.proto b/spice.proto
index 3f7c30c..3c0911d 100644
--- a/spice.proto
+++ b/spice.proto
@@ -621,9 +621,7 @@ channel DisplayChannel : BaseChannel {
 	uint32 id;
 	uint32 multi_media_time;
 	uint32 data_size;
-	uint32 pad_size;
 	uint8 data[data_size] @end  @nomarshal;
-	uint8 padding[pad_size] @end @ctype(uint8_t)  @nomarshal; /* Uhm, why are we sending padding over network? */
     } stream_data;
 
     message {
diff --git a/spice1.proto b/spice1.proto
index e38a214..ebb2d6f 100644
--- a/spice1.proto
+++ b/spice1.proto
@@ -583,9 +583,9 @@ channel DisplayChannel : BaseChannel {
 	uint32 id;
 	uint32 multi_media_time;
 	uint32 data_size;
-	uint32 pad_size;
+	uint32 pad_size @zero;
 	uint8 data[data_size] @end  @nomarshal;
-	uint8 padding[pad_size] @end @ctype(uint8_t)  @nomarshal; /* Uhm, why are we sending padding over network? */
+	/* Ignore: uint8 padding[pad_size] */
     } stream_data;
 
     message {


More information about the Spice-commits mailing list