[Spice-devel] [vdagent-win PATCH v8 3/5] Write code to decode PNG format

Christophe Fergeau cfergeau at redhat.com
Fri Jul 21 12:48:09 UTC 2017


On Wed, Jul 19, 2017 at 09:42:41AM +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  Makefile.am          |   6 +-
>  configure.ac         |   3 +
>  vdagent/image.cpp    |   8 +-
>  vdagent/imagepng.cpp | 237 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  vdagent/imagepng.h   |  25 ++++++
>  5 files changed, 270 insertions(+), 9 deletions(-)
>  create mode 100644 vdagent/imagepng.cpp
>  create mode 100644 vdagent/imagepng.h
> 
> diff --git a/Makefile.am b/Makefile.am
> index b35dd57..175d8f7 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -23,8 +23,8 @@ LIBS = -lversion
>  
>  bin_PROGRAMS = vdagent vdservice
>  
> -vdagent_LDADD = -lwtsapi32 -lgdi32 vdagent_rc.$(OBJEXT)
> -vdagent_CXXFLAGS = $(AM_CXXFLAGS)
> +vdagent_LDADD = $(LIBPNG_LIBS) $(ZLIB_LIBS) -lwtsapi32 -lgdi32 vdagent_rc.$(OBJEXT)
> +vdagent_CXXFLAGS = $(AM_CXXFLAGS) $(LIBPNG_CFLAGS)
>  vdagent_LDFLAGS = $(AM_LDFLAGS) -Wl,--subsystem,windows
>  vdagent_SOURCES =			\
>  	common/vdcommon.cpp             \
> @@ -44,6 +44,8 @@ vdagent_SOURCES =			\
>  	vdagent/as_user.h		\
>  	vdagent/image.cpp		\
>  	vdagent/image.h			\
> +	vdagent/imagepng.cpp		\
> +	vdagent/imagepng.h		\
>  	$(NULL)
>  
>  vdagent_rc.$(OBJEXT): vdagent/vdagent.rc
> diff --git a/configure.ac b/configure.ac
> index 4eac4b4..bb33075 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -101,6 +101,9 @@ dnl ---------------------------------------------------------------------------
>  dnl - Check library dependencies
>  dnl ---------------------------------------------------------------------------
>  
> +PKG_CHECK_MODULES(LIBPNG, [libpng])
> +PKG_CHECK_MODULES(ZLIB, [zlib])
> +
>  dnl ---------------------------------------------------------------------------
>  dnl - Makefiles, etc.
>  dnl ---------------------------------------------------------------------------
> diff --git a/vdagent/image.cpp b/vdagent/image.cpp
> index faec18f..82cfb0e 100644
> --- a/vdagent/image.cpp
> +++ b/vdagent/image.cpp
> @@ -21,9 +21,9 @@
>  
>  #include "vdcommon.h"
>  #include "image.h"
> +#include "imagepng.h"
>  
>  ImageCoder *create_bitmap_coder();
> -ImageCoder *create_png_coder();
>  
>  static ImageCoder *get_coder(uint32_t vdagent_type)
>  {
> @@ -172,9 +172,3 @@ ImageCoder *create_bitmap_coder()
>  {
>      return new BitmapCoder();
>  }
> -
> -// TODO
> -ImageCoder *create_png_coder()
> -{
> -    return NULL;
> -}
> diff --git a/vdagent/imagepng.cpp b/vdagent/imagepng.cpp
> new file mode 100644
> index 0000000..9530d71
> --- /dev/null
> +++ b/vdagent/imagepng.cpp
> @@ -0,0 +1,237 @@
> +/*
> +   Copyright (C) 2017 Red Hat, Inc.
> +
> +   This program is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU General Public License as
> +   published by the Free Software Foundation; either version 2 of
> +   the License, or (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +#include "vdcommon.h"
> +
> +#include <png.h>
> +#include <algorithm>
> +
> +#include "imagepng.h"
> +
> +class PngCoder: public ImageCoder
> +{
> +public:
> +    PngCoder() {};
> +    size_t get_dib_size(const uint8_t *data, size_t size);
> +    void get_dib_data(uint8_t *dib, const uint8_t *data, size_t size);
> +    uint8_t *from_bitmap(const BITMAPINFO& info, const void *bits, long &size);
> +private:
> +    size_t convert_to_dib(uint8_t *out_buf, const uint8_t *data, size_t size);
> +};
> +
> +struct BufferIo {
> +    uint8_t *buf;
> +    uint32_t pos, size;
> +    BufferIo(uint8_t *_buf, uint32_t _size):
> +        buf(_buf), pos(0), size(_size)
> +    {}
> +};
> +
> +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)
> +        png_error(png, "read past end");
> +    memcpy(out, io.buf+io.pos, size);
> +    io.pos += size;
> +}
> +
> +size_t PngCoder::get_dib_size(const uint8_t *data, size_t size)
> +{
> +    return convert_to_dib(NULL, data, size);
> +}
> +
> +typedef void line_fixup_t(uint8_t *line, unsigned int width);
> +
> +static void line_fixup_none(uint8_t *line, unsigned int width)
> +{
> +}
> +
> +static void line_fixup_2bpp_to_4bpp(uint8_t *line, unsigned int width)
> +{
> +    width = (width + 3) / 4u;
> +    while (width--) {
> +        uint8_t from = line[width];
> +        line[width*2+1] = ((from & 0x03) << 0) | ((from & 0x0c) << 2);
> +        line[width*2+0] = ((from & 0x30) >> 4) | ((from & 0xc0) >> 2);
> +    }
> +}

This is used in the 2bpp greyscale case, and will convert this way:
00b -> 0000b
01b -> 0001b
10b -> 0010b
11b -> 0011b

We'll then generate a palette this way:

+        const unsigned int colors = 1 << out_bits;
+        const unsigned int mult = 255 / (colors - 1);
+        for (unsigned int color = 0; color < colors; ++color) {
+            rgb->rgbBlue = rgb->rgbGreen = rgb->rgbRed = color * mult;
+            rgb->rgbReserved = 0;
+            ++rgb;
+        }

This means in particular that 11b will not be mapped to 1111b, so I
suspect we might be making 2bpp images paler than they should?
(on the other hand, are we going to encounter such images in the wild??)


> +
> +size_t PngCoder::convert_to_dib(uint8_t *out_buf, const uint8_t *data, size_t size)
> +{
> +    BufferIo io((uint8_t *)data, size);
> +
> +    png_structp png = png_create_read_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
> +    if (!png)
> +        return 0;
> +
> +    png_infop info = png_create_info_struct(png);
> +    if (!info) {
> +        png_destroy_read_struct(&png, &info, NULL);
> +        return 0;
> +    }
> +
> +    if (setjmp(png_jmpbuf(png))) {
> +        png_destroy_read_struct(&png, &info, NULL);
> +        return 0;
> +    }
> +
> +    png_set_read_fn(png, &io, read_from_bufio);
> +
> +    png_read_info(png, info);
> +
> +    // not so much precision is supported
> +    unsigned int bits = png_get_bit_depth(png, info);
> +    if (bits == 16)
> +        png_set_strip_16(png);
> +
> +    unsigned int out_bits;
> +    bool is_gray = false;
> +    line_fixup_t *line_fixup = line_fixup_none;
> +    switch (png_get_color_type(png, info)) {
> +    case PNG_COLOR_TYPE_GRAY:
> +        is_gray = true;
> +        if (bits == 16) {
> +            out_bits = 8;
> +        } else if (bits == 2) {
> +            line_fixup = line_fixup_2bpp_to_4bpp;
> +            out_bits = 4;
> +        } else {
> +            out_bits = bits;
> +        }
> +        break;
> +    case PNG_COLOR_TYPE_PALETTE:
> +        // should return 1, 2, 4 and 8, BMP does not support 2
> +        out_bits = bits;
> +        if (bits == 2) {
> +            line_fixup = line_fixup_2bpp_to_4bpp;
> +            out_bits = 4;
> +        }
> +        break;
> +    case PNG_COLOR_TYPE_RGB:
> +        png_set_bgr(png);
> +        out_bits = 24;
> +        break;
> +    case PNG_COLOR_TYPE_RGB_ALPHA:
> +        png_set_bgr(png);
> +        out_bits = 24;
> +        png_set_strip_alpha(png);
> +        break;
> +    case PNG_COLOR_TYPE_GRAY_ALPHA:
> +        is_gray = true;
> +        // gray with alpha should always be 8 bit but make it sure
> +        // in case format change
> +        png_set_expand_gray_1_2_4_to_8(png);
> +        out_bits = 8;
> +        png_set_strip_alpha(png);
> +        break;
> +    default:
> +        png_error(png, "PNG color type not supported");
> +        break;
> +    }
> +
> +    const unsigned int width = png_get_image_width(png, info);
> +    const unsigned int height = png_get_image_height(png, info);
> +    const size_t stride = compute_dib_stride(width, out_bits);
> +    const size_t image_size = stride * height;
> +    const size_t palette_size = out_bits > 8 ? 0 : sizeof(RGBQUAD) * (1 << out_bits);
> +    const size_t dib_size = sizeof(BITMAPINFOHEADER) + palette_size + image_size;
> +
> +    // just called to get the size, return the information
> +    if (!out_buf) {
> +        png_destroy_read_struct(&png, &info, NULL);
> +        return dib_size;
> +    }
> +
> +    // TODO tests
> +    // bits, 1, 2, 4, 8, 16
> +    // all color types
> +    // alpha/not alpha
> +    // indexed with not all colors
> +
> +    // fill header
> +    BITMAPINFOHEADER& head(*(BITMAPINFOHEADER *)out_buf);
> +    memset(&head, 0, sizeof(head));
> +    head.biSize = sizeof(head);
> +    head.biWidth = width;
> +    head.biHeight = height;
> +    head.biPlanes = 1;
> +    head.biBitCount = out_bits;
> +    head.biCompression = BI_RGB;
> +    head.biSizeImage = image_size;
> +
> +    // copy palette
> +    RGBQUAD *rgb = (RGBQUAD *)(out_buf + sizeof(BITMAPINFOHEADER));
> +    if (is_gray) {
> +        const unsigned int colors = 1 << out_bits;
> +        const unsigned int mult = 255 / (colors - 1);
> +        for (unsigned int color = 0; color < colors; ++color) {
> +            rgb->rgbBlue = rgb->rgbGreen = rgb->rgbRed = color * mult;
> +            rgb->rgbReserved = 0;
> +            ++rgb;
> +        }
> +        head.biClrUsed = colors;

Do we have to have a palette for 8bpp greyscale images? (another case
which we might not encounter in practice anyway..)

Looks good otherwise.

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/20170721/0e259c7a/attachment.sig>


More information about the Spice-devel mailing list