#839: move the restride code to the ximage wrapper

git-svn-id: https://xpra.org/svn/Xpra/trunk@9235 3bb7dfac-3a0b-4e04-842a-767bc560f471

src/xpra/codecs/argb/argb.pyx

@@ -230,55 +230,3 @@ cdef do_unpremultiply_argb(unsigned int * argb_in, Py_ssize_t argb_len):
         argb_out[i*4+R] = r
         argb_out[i*4+A] = a
     return argb_out
-
-
-cdef roundup(int n, int m):
-    return (n + m - 1) & ~(m - 1)
-
-def restride_image(image):
-    #NOTE: this must be called from the UI thread!
-    cdef int stride = image.get_rowstride()
-    cdef int width = image.get_width()
-    pixel_format = image.get_pixel_format()
-    cdef int rstride = roundup(width*len(pixel_format), 4)   #a reasonable stride: rounded up to 4
-    cdef int height = image.get_height()
-    if stride<8 or rstride>stride or height<=2:
-        return False                    #not worth it
-    pixels = image.get_pixels()
-    cdef unsigned char *img_buf
-    cdef Py_ssize_t img_buf_len
-    assert object_as_buffer(pixels, <const void**> &img_buf, &img_buf_len)==0, "cannot convert %s to a readable buffer" % type(pixels)
-    if img_buf_len<=0:
-        return False
-    cdef int out_size = rstride*height                  #desirable size we could have
-    #is it worth re-striding to save space:
-    if img_buf_len-out_size<1024 or out_size*110/100>img_buf_len:
-        return False
-    #we'll save at least 1KB and 10%, do it
-    #Note: we could also change the pixel format whilst we're at it
-    # and convert BGRX to RGB for example (assuming RGB is also supported by the client)
-    #this buffer is allocated by the imagewrapper, so it will be freed after use for us,
-    #but we need to tell allocate_buffer not to free the current buffer (if there is one),
-    #and we have to deal with this ourselves after we're done copying it
-    cdef unsigned long ptr
-
-    #save pixels pointer to free later:
-    ptr = int(image.get_pixel_ptr())
-    cdef unsigned long pixptr = ptr
-
-    ptr = int(image.allocate_buffer(out_size, False))
-    if ptr==0:
-        #not an ximage, cannot restride (shouldn't be needed anyway)
-        return False
-    cdef unsigned char *out = <unsigned char*> ptr
-
-    cdef int ry = height
-    for 0 <= ry < height:
-        memcpy(out, img_buf, rstride)
-        out += rstride
-        img_buf += stride
-    if pixptr:
-        free(<void *> pixptr)
-    log("restride_image: %s pixels re-stride saving %i%% from %s (%s bytes) to %s (%s bytes)" % (pixel_format, 100-100*out_size/img_buf_len, stride, img_buf_len, rstride, out_size))
-    image.set_rowstride(rstride)
-    return True

src/xpra/codecs/image_wrapper.py

@@ -98,6 +98,10 @@ def allocate_buffer(self, buf_len, free_existing=1):
         #only defined for XImage wrappers:
         return 0
 
+    def restride(self, *args):
+        #not supported by the generic image wrapper:
+        return False
+
     def clone_pixel_data(self):
         assert not self.freed, "image has already been freed!"
         if self.planes == 0:

src/xpra/server/picture_encode.py

@@ -11,7 +11,7 @@
 log = Logger("window", "encoding")
 
 from xpra.net import compression
-from xpra.codecs.argb.argb import bgra_to_rgb, bgra_to_rgba, argb_to_rgb, argb_to_rgba, restride_image   #@UnresolvedImport
+from xpra.codecs.argb.argb import bgra_to_rgb, bgra_to_rgba, argb_to_rgb, argb_to_rgba  #@UnresolvedImport
 from xpra.os_util import StringIOClass
 from xpra.codecs.loader import get_codec, get_codec_version
 from xpra.codecs.codec_constants import get_PIL_encodings
@@ -96,7 +96,7 @@ def rgb_encode(coding, image, rgb_formats, supports_transparency, speed, rgb_zli
     options = {"rgb_format" : pixel_format}
 
     #we may want to re-stride:
-    restride_image(image)
+    image.restride()
 
     #compress here and return a wrapper so network code knows it is already zlib compressed:
     pixels = image.get_pixels()

src/xpra/server/window_source.py

@@ -43,7 +43,6 @@
 from xpra.server.batch_delay_calculator import calculate_batch_delay, get_target_speed, get_target_quality
 from xpra.server.cystats import time_weighted_average   #@UnresolvedImport
 from xpra.server.region import rectangle, add_rectangle, remove_rectangle   #@UnresolvedImport
-from xpra.codecs.argb.argb import restride_image    #@UnresolvedImport
 from xpra.codecs.xor.cyxor import xor_str           #@UnresolvedImport
 from xpra.server.picture_encode import webp_encode, rgb_encode, PIL_encode, mmap_encode, mmap_send
 from xpra.codecs.loader import PREFERED_ENCODING_ORDER, get_codec
@@ -1486,7 +1485,7 @@ def make_data_packet(self, damage_time, process_damage_time, wid, image, coding,
         #if client supports delta pre-compression for this encoding, use it if we can:
         elif self.delta_buckets>0 and (coding in self.supports_delta) and self.min_delta_size<isize<self.max_delta_size:
             #this may save space (and lower the cost of xoring):
-            restride_image(image)
+            image.restride()
             #we need to copy the pixels because some encodings
             #may modify the pixel array in-place!
             dpixels = image.get_pixels()

src/xpra/x11/bindings/ximage.pyx

@@ -16,6 +16,10 @@ xshmdebug = Logger("x11", "bindings", "ximage", "xshm", "verbose")
 ximagedebug = Logger("x11", "bindings", "ximage", "verbose")
 
 
+cdef roundup(int n, int m):
+    return (n + m - 1) & ~(m - 1)
+
+
 ###################################
 # Headers, python magic
 ###################################
@@ -289,17 +293,15 @@ cdef class XImageWrapper:
         return self.pixel_format
 
     def get_pixels(self):
-        if self.pixels!=NULL:
-            return self.get_char_pixels()
-        return self.get_image_pixels()
+        cdef void *pix_ptr = self.get_pixels_ptr()
+        return memory_as_pybuffer(pix_ptr, self.get_size(), False)
 
-    cdef get_char_pixels(self):
-        assert self.pixels!=NULL
-        return memory_as_pybuffer(self.pixels, self.get_size(), False)
-
-    cdef get_image_pixels(self):
-        assert self.image!=NULL
-        return memory_as_pybuffer(self.image.data, self.get_size(), False)
+    cdef void *get_pixels_ptr(self):
+        if self.pixels!=NULL:
+            return self.pixels
+        if self.image==NULL:
+            return NULL
+        return self.image.data
 
     def is_thread_safe(self):
         return self.thread_safe
@@ -328,27 +330,23 @@ cdef class XImageWrapper:
         cdef const unsigned char * buf = NULL
         cdef Py_ssize_t buf_len = 0
         assert object_as_buffer(pixels, <const void**> &buf, &buf_len)==0
-        self.allocate_buffer(buf_len, True)
-        memcpy(self.pixels, buf, buf_len)
-
-    def get_pixel_ptr(self):
-        return int(<unsigned long> self.pixels)
-
-    def allocate_buffer(self, Py_ssize_t buf_len, int free_existing=1):
-        if self.pixels!=NULL and free_existing!=0:
+        if self.pixels!=NULL:
             free(self.pixels)
-        self.pixels = NULL
+            self.pixels = NULL
         #Note: we can't free the XImage, because it may
         #still be used somewhere else (see XShmWrapper)
         if posix_memalign(<void **> &self.pixels, 64, buf_len):
             raise Exception("posix_memalign failed!")
         assert self.pixels!=NULL
         if self.image==NULL:
             self.thread_safe = 1
-            #we can only mark this object as thread safe
+            #we can now mark this object as thread safe
             #if we have already freed the XImage
-            #(since it needs to be freed from the UI thread)
-        return int(<unsigned long> self.pixels)
+            #which needs to be freed from the UI thread
+            #but our new buffer is just a malloc buffer,
+            #which is safe from any thread
+        memcpy(self.pixels, buf, buf_len)
+
 
     def free(self):                                     #@DuplicatedSignature
         ximagedebug("%s.free()", self)
@@ -370,6 +368,46 @@ cdef class XImageWrapper:
             self.pixels = NULL
 
 
+    def restride(self, int newstride=0):
+        #NOTE: this must be called from the UI thread!
+        if newstride==0:
+            newstride = roundup(self.width*len(self.pixel_format), 4)   #a reasonable stride: rounded up to 4
+        if self.rowstride<8 or newstride>self.rowstride or self.height<=2:
+            return False                    #not worth it
+        cdef int size = self.rowstride*self.height
+        cdef int newsize = newstride*self.height                  #desirable size we could have
+        #is it worth re-striding to save space:
+        #(save at least 1KB and 10%)
+        if size-newsize<1024 or newsize*110/100>size:
+            log("restride(%s) not enough savings: size=%s, newsize=%s", newstride, size, newsize) 
+            return False
+        #Note: we could also change the pixel format whilst we're at it
+        # and convert BGRX to RGB for example (assuming RGB is also supported by the client)
+        cdef void *img_buf = self.get_pixels_ptr()
+        assert img_buf!=NULL, "this image wrapper is empty!"
+        cdef void *new_buf
+        if posix_memalign(<void **> &new_buf, 64, newsize):
+            raise Exception("posix_memalign failed!")
+        cdef int ry
+        cdef void *to = new_buf
+        for 0 <= ry < self.height:
+            memcpy(to, img_buf, newstride)
+            to += newstride
+            img_buf += self.rowstride
+        log("restride() %s pixels re-stride saving %i%% from %s (%s bytes) to %s (%s bytes)", 
+            self.pixel_format, 100-100*newsize/size, self.rowstride, size, newstride, newsize)
+        #we can now free the pixels buffer if present
+        #(but not the ximage - this is not running in the UI thread!)
+        self.free_pixels()
+        #set the new attributes:
+        self.rowstride = newstride
+        self.pixels = <char *> new_buf
+        #without any X11 image to free, this is now thread safe:
+        if self.image==NULL:
+            self.thread_safe = 1
+        return True
+
+
 cdef class XShmWrapper(object):
     cdef Display *display                              #@DuplicatedSignature
     cdef Visual *visual
@@ -523,17 +561,16 @@ cdef class XShmImageWrapper(XImageWrapper):
     def __init__(self, *args):                      #@DuplicatedSignature
         self.free_callback = None
 
-    def __repr__(self):                              #@DuplicatedSignature
+    def __repr__(self):                             #@DuplicatedSignature
         return "XShmImageWrapper(%s: %s, %s, %s, %s)" % (self.pixel_format, self.x, self.y, self.width, self.height)
 
-    cdef get_image_pixels(self):                     #@DuplicatedSignature
-        cdef char *offset
-        xshmdebug("XShmImageWrapper.get_image_pixels() self=%s", self)
+    cdef void *get_pixels_ptr(self):                #@DuplicatedSignature
+        if self.pixels!=NULL:
+            return self.pixels
         assert self.image!=NULL and self.height>0
+        xshmdebug("XShmImageWrapper.get_pixels_ptr() self=%s", self)
         #calculate offset (assuming 4 bytes "pixelstride"):
-        offset = self.image.data + (self.y * self.rowstride) + (4 * self.x)
-        cdef Py_ssize_t size = self.height * self.rowstride
-        return memory_as_pybuffer(offset, size, False)
+        return self.image.data + (self.y * self.rowstride) + (4 * self.x)
 
     def free(self):                                 #@DuplicatedSignature
         #ensure we never try to XDestroyImage: