[Spice-devel] [vdagent-win PATCH v8 3/5] Write code to decode PNG format
Frediano Ziglio
fziglio at redhat.com
Fri Jul 21 13:03:01 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??)
>
You are right. So gray PNG with 2 bit (not likely to be found) should
not be correctly converted. I'll try to add a test for this (and a fix).
>
> > +
> > +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..)
>
Yes, BMP does not support grey images so to store a grey image
you use a palette image with a grey palette.
Not sure how different software encode images but I think that grayscale
is more common then 2 bit images.
> Looks good otherwise.
>
> Christophe
>
Frediano
More information about the Spice-devel
mailing list