[Spice-devel] [vdagent-win PATCH v4 4/5] Support encoding PNG images

[Christophe Fergeau]

On Mon, Jul 17, 2017 at 09:54:05AM +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  vdagent/imagepng.cpp | 138 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 131 insertions(+), 7 deletions(-)
> 
> diff --git a/vdagent/imagepng.cpp b/vdagent/imagepng.cpp
> index 19d9efc..c5fc6c0 100644
> --- a/vdagent/imagepng.cpp
> +++ b/vdagent/imagepng.cpp
> @@ -19,6 +19,7 @@
>  
>  #include <png.h>
>  #include <algorithm>
> +#include <vector>
>  
>  #include "imagepng.h"
>  
> @@ -36,17 +37,47 @@ private:
>  struct BufferIo {
>      uint8_t *buf;
>      uint32_t pos, size;
> +    bool allocated;
> +    BufferIo(uint8_t *_buf, uint32_t _size):
> +        buf(_buf), pos(0), size(_size),
> +        allocated(false)
> +    {}
> +    ~BufferIo() { if (allocated) free(buf); }
> +    uint8_t *release() { allocated = false; return buf; }
>  };
>  
>  static void read_from_bufio(png_structp png, png_bytep out, png_size_t size)
>  {
>      BufferIo& io(*(BufferIo*)png_get_io_ptr(png));
> -    if (io.pos + size >= io.size)
> +    if (io.pos + size > io.size)

Looks like this could be squashed in the previous patch?

>          png_error(png, "read past end");
>      memcpy(out, io.buf+io.pos, size);
>      io.pos += size;
>  }
>  
> +static void write_to_bufio(png_structp png, png_bytep in, png_size_t size)
> +{
> +    BufferIo& io(*(BufferIo*)png_get_io_ptr(png));
> +    if (io.pos + size > io.size) {
> +        uint32_t new_size = io.size ? io.size * 2 : 4096;
> +        while (io.pos + size >= new_size) {
> +            new_size *= 2;
> +        }
> +        uint8_t *p = (uint8_t*) realloc(io.buf, new_size);
> +        if (!p)
> +            png_error(png, "out of memory");
> +        io.allocated = true;
> +        io.buf = p;
> +        io.size = new_size;
> +    }
> +    memcpy(io.buf+io.pos, in, size);
> +    io.pos += size;
> +}
> +
> +static void flush_bufio(png_structp png)
> +{
> +}
> +
>  size_t PngCoder::get_dib_size(const uint8_t *data, size_t size)
>  {
>      return convert_to_dib(NULL, data, size);
> @@ -76,12 +107,21 @@ static void line_fixup_rgb2bgr(uint8_t *line, unsigned width)
>      }
>  }
>  
> +static void line_fixup_bgr2rgb(uint8_t *dst, const uint8_t *src, unsigned width,
> +                               unsigned src_pixel_len)
> +{
> +    for (; width; --width) {
> +        dst[0] = src[2];
> +        dst[1] = src[1];
> +        dst[2] = src[0];
> +        dst += 3;
> +        src += src_pixel_len;
> +    }
> +}

Could this be png_set_bgr?

> +
>  size_t PngCoder::convert_to_dib(uint8_t *out_buf, const uint8_t *data, size_t size)
>  {
> -    BufferIo io;
> -    io.buf = (uint8_t *)data;
> -    io.pos = 0;
> -    io.size = size;
> +    BufferIo io((uint8_t *)data, size);

Fixup in previous commit?

Apart from these small comments, looks good.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170718/75b3aea2/attachment.sig>