Patchwork [1/9] add pflib: PixelFormat conversion library.

login
register
mail settings
Submitter Gerd Hoffmann
Date Aug. 19, 2010, 12:40 p.m.
Message ID <1282221625-29501-2-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/62146/
State New
Headers show

Comments

Gerd Hoffmann - Aug. 19, 2010, 12:40 p.m.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Makefile.objs |    1 +
 pflib.c       |  213 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 pflib.h       |   20 ++++++
 3 files changed, 234 insertions(+), 0 deletions(-)
 create mode 100644 pflib.c
 create mode 100644 pflib.h
Anthony Liguori - Aug. 19, 2010, 2:04 p.m.
On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>    

This code is more or less stand alone so it's a good candidate for 
having some in-tree unit tests.

I can provide some examples of how it should be done if you like, but 
just something that lives in tests/ that exercises the functionality and 
can be run under valgrind to verify that we aren't leaking memory.

Using libcheck is also reasonable (take a look at the various check-q* 
in the top level).

Regards,

Anthony Liguori

> ---
>   Makefile.objs |    1 +
>   pflib.c       |  213 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   pflib.h       |   20 ++++++
>   3 files changed, 234 insertions(+), 0 deletions(-)
>   create mode 100644 pflib.c
>   create mode 100644 pflib.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 4a1eaa1..edfca87 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -83,6 +83,7 @@ common-obj-y += qemu-char.o savevm.o #aio.o
>   common-obj-y += msmouse.o ps2.o
>   common-obj-y += qdev.o qdev-properties.o
>   common-obj-y += block-migration.o
> +common-obj-y += pflib.o
>
>   common-obj-$(CONFIG_BRLAPI) += baum.o
>   common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
> diff --git a/pflib.c b/pflib.c
> new file mode 100644
> index 0000000..1154d0c
> --- /dev/null
> +++ b/pflib.c
> @@ -0,0 +1,213 @@
> +/*
> + * PixelFormat conversion library.
> + *
> + * Author: Gerd Hoffmann<kraxel@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +#include "qemu-common.h"
> +#include "console.h"
> +#include "pflib.h"
> +
> +typedef struct QemuPixel QemuPixel;
> +
> +typedef void (*pf_convert)(QemuPfConv *conv,
> +                           void *dst, void *src, uint32_t cnt);
> +typedef void (*pf_convert_from)(PixelFormat *pf,
> +                                QemuPixel *dst, void *src, uint32_t cnt);
> +typedef void (*pf_convert_to)(PixelFormat *pf,
> +                              void *dst, QemuPixel *src, uint32_t cnt);
> +
> +struct QemuPfConv {
> +    pf_convert        convert;
> +    PixelFormat       src;
> +    PixelFormat       dst;
> +
> +    /* for copy_generic() */
> +    pf_convert_from   conv_from;
> +    pf_convert_to     conv_to;
> +    QemuPixel         *conv_buf;
> +    uint32_t          conv_cnt;
> +};
> +
> +struct QemuPixel {
> +    uint8_t red;
> +    uint8_t green;
> +    uint8_t blue;
> +    uint8_t alpha;
> +};
> +
> +/* ----------------------------------------------------------------------- */
> +/* PixelFormat ->  QemuPixel conversions                                    */
> +
> +static void conv_16_to_pixel(PixelFormat *pf,
> +                             QemuPixel *dst, void *src, uint32_t cnt)
> +{
> +    uint16_t *src16 = src;
> +
> +    while (cnt>  0) {
> +        dst->red   = ((*src16&  pf->rmask)>>  pf->rshift)<<  (8 - pf->rbits);
> +        dst->green = ((*src16&  pf->gmask)>>  pf->gshift)<<  (8 - pf->gbits);
> +        dst->blue  = ((*src16&  pf->bmask)>>  pf->bshift)<<  (8 - pf->bbits);
> +        dst->alpha = ((*src16&  pf->amask)>>  pf->ashift)<<  (8 - pf->abits);
> +        dst++, src16++, cnt--;
> +    }
> +}
> +
> +/* assumes pf->{r,g,b,a}bits == 8 */
> +static void conv_32_to_pixel_fast(PixelFormat *pf,
> +                                  QemuPixel *dst, void *src, uint32_t cnt)
> +{
> +    uint32_t *src32 = src;
> +
> +    while (cnt>  0) {
> +        dst->red   = (*src32&  pf->rmask)>>  pf->rshift;
> +        dst->green = (*src32&  pf->gmask)>>  pf->gshift;
> +        dst->blue  = (*src32&  pf->bmask)>>  pf->bshift;
> +        dst->alpha = (*src32&  pf->amask)>>  pf->ashift;
> +        dst++, src32++, cnt--;
> +    }
> +}
> +
> +static void conv_32_to_pixel_generic(PixelFormat *pf,
> +                                     QemuPixel *dst, void *src, uint32_t cnt)
> +{
> +    uint32_t *src32 = src;
> +
> +    while (cnt>  0) {
> +        if (pf->rbits<  8) {
> +            dst->red   = ((*src32&  pf->rmask)>>  pf->rshift)<<  (8 - pf->rbits);
> +        } else {
> +            dst->red   = ((*src32&  pf->rmask)>>  pf->rshift)>>  (pf->rbits - 8);
> +        }
> +        if (pf->gbits<  8) {
> +            dst->green = ((*src32&  pf->gmask)>>  pf->gshift)<<  (8 - pf->gbits);
> +        } else {
> +            dst->green = ((*src32&  pf->gmask)>>  pf->gshift)>>  (pf->gbits - 8);
> +        }
> +        if (pf->bbits<  8) {
> +            dst->blue  = ((*src32&  pf->bmask)>>  pf->bshift)<<  (8 - pf->bbits);
> +        } else {
> +            dst->blue  = ((*src32&  pf->bmask)>>  pf->bshift)>>  (pf->bbits - 8);
> +        }
> +        if (pf->abits<  8) {
> +            dst->alpha = ((*src32&  pf->amask)>>  pf->ashift)<<  (8 - pf->abits);
> +        } else {
> +            dst->alpha = ((*src32&  pf->amask)>>  pf->ashift)>>  (pf->abits - 8);
> +        }
> +        dst++, src32++, cnt--;
> +    }
> +}
> +
> +/* ----------------------------------------------------------------------- */
> +/* QemuPixel ->  PixelFormat conversions                                    */
> +
> +static void conv_pixel_to_16(PixelFormat *pf,
> +                             void *dst, QemuPixel *src, uint32_t cnt)
> +{
> +    uint16_t *dst16 = dst;
> +
> +    while (cnt>  0) {
> +        *dst16  = ((uint16_t)src->red>>  (8 - pf->rbits))<<  pf->rshift;
> +        *dst16 |= ((uint16_t)src->green>>  (8 - pf->gbits))<<  pf->gshift;
> +        *dst16 |= ((uint16_t)src->blue>>  (8 - pf->bbits))<<  pf->bshift;
> +        *dst16 |= ((uint16_t)src->alpha>>  (8 - pf->abits))<<  pf->ashift;
> +        dst16++, src++, cnt--;
> +    }
> +}
> +
> +static void conv_pixel_to_32(PixelFormat *pf,
> +                             void *dst, QemuPixel *src, uint32_t cnt)
> +{
> +    uint32_t *dst32 = dst;
> +
> +    while (cnt>  0) {
> +        *dst32  = ((uint32_t)src->red>>  (8 - pf->rbits))<<  pf->rshift;
> +        *dst32 |= ((uint32_t)src->green>>  (8 - pf->gbits))<<  pf->gshift;
> +        *dst32 |= ((uint32_t)src->blue>>  (8 - pf->bbits))<<  pf->bshift;
> +        *dst32 |= ((uint32_t)src->alpha>>  (8 - pf->abits))<<  pf->ashift;
> +        dst32++, src++, cnt--;
> +    }
> +}
> +
> +/* ----------------------------------------------------------------------- */
> +/* PixelFormat ->  PixelFormat conversions                                  */
> +
> +static void convert_copy(QemuPfConv *conv, void *dst, void *src, uint32_t cnt)
> +{
> +    uint32_t bytes = cnt * conv->src.bytes_per_pixel;
> +    memcpy(dst, src, bytes);
> +}
> +
> +static void convert_generic(QemuPfConv *conv, void *dst, void *src, uint32_t cnt)
> +{
> +    if (conv->conv_cnt<  cnt) {
> +        conv->conv_cnt = cnt;
> +        conv->conv_buf = qemu_realloc(conv->conv_buf, sizeof(QemuPixel) * conv->conv_cnt);
> +    }
> +    conv->conv_from(&conv->src, conv->conv_buf, src, cnt);
> +    conv->conv_to(&conv->dst, dst, conv->conv_buf, cnt);
> +}
> +
> +/* ----------------------------------------------------------------------- */
> +/* public interface                                                        */
> +
> +QemuPfConv *qemu_pf_conv_get(PixelFormat *dst, PixelFormat *src)
> +{
> +    QemuPfConv *conv = qemu_mallocz(sizeof(QemuPfConv));
> +
> +    conv->src = *src;
> +    conv->dst = *dst;
> +
> +    if (memcmp(&conv->src,&conv->dst, sizeof(PixelFormat)) == 0) {
> +        /* formats identical, can simply copy */
> +        conv->convert = convert_copy;
> +    } else {
> +        /* generic two-step conversion: src ->  QemuPixel ->  dst  */
> +        switch (conv->src.bytes_per_pixel) {
> +        case 2:
> +            conv->conv_from = conv_16_to_pixel;
> +            break;
> +        case 4:
> +            if (conv->src.rbits == 8&&  conv->src.gbits == 8&&  conv->src.bbits == 8) {
> +                conv->conv_from = conv_32_to_pixel_fast;
> +            } else {
> +                conv->conv_from = conv_32_to_pixel_generic;
> +            }
> +            break;
> +        default:
> +            goto err;
> +        }
> +        switch (conv->dst.bytes_per_pixel) {
> +        case 2:
> +            conv->conv_to = conv_pixel_to_16;
> +            break;
> +        case 4:
> +            conv->conv_to = conv_pixel_to_32;
> +            break;
> +        default:
> +            goto err;
> +        }
> +        conv->convert = convert_generic;
> +    }
> +    return conv;
> +
> +err:
> +    qemu_free(conv);
> +    return NULL;
> +}
> +
> +void qemu_pf_conv_run(QemuPfConv *conv, void *dst, void *src, uint32_t cnt)
> +{
> +    conv->convert(conv, dst, src, cnt);
> +}
> +
> +void qemu_pf_conv_put(QemuPfConv *conv)
> +{
> +    if (conv) {
> +        qemu_free(conv->conv_buf);
> +        qemu_free(conv);
> +    }
> +}
> diff --git a/pflib.h b/pflib.h
> new file mode 100644
> index 0000000..b70c313
> --- /dev/null
> +++ b/pflib.h
> @@ -0,0 +1,20 @@
> +#ifndef __QEMU_PFLIB_H
> +#define __QEMU_PFLIB_H
> +
> +/*
> + * PixelFormat conversion library.
> + *
> + * Author: Gerd Hoffmann<kraxel@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +typedef struct QemuPfConv QemuPfConv;
> +
> +QemuPfConv *qemu_pf_conv_get(PixelFormat *dst, PixelFormat *src);
> +void qemu_pf_conv_run(QemuPfConv *conv, void *dst, void *src, uint32_t cnt);
> +void qemu_pf_conv_put(QemuPfConv *conv);
> +
> +#endif
>
Gerd Hoffmann - Aug. 19, 2010, 3:34 p.m.
On 08/19/10 16:04, Anthony Liguori wrote:
> On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
>> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>
> This code is more or less stand alone so it's a good candidate for
> having some in-tree unit tests.
>
> I can provide some examples of how it should be done if you like, but
> just something that lives in tests/ that exercises the functionality and
> can be run under valgrind to verify that we aren't leaking memory.

Yea, some example would be great.  Commented tests/example.c maybe?

thanks,
   Gerd
Anthony Liguori - Aug. 19, 2010, 7:09 p.m.
On 08/19/2010 10:34 AM, Gerd Hoffmann wrote:
> On 08/19/10 16:04, Anthony Liguori wrote:
>> On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
>>> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>>
>> This code is more or less stand alone so it's a good candidate for
>> having some in-tree unit tests.
>>
>> I can provide some examples of how it should be done if you like, but
>> just something that lives in tests/ that exercises the functionality and
>> can be run under valgrind to verify that we aren't leaking memory.
>
> Yea, some example would be great.  Commented tests/example.c maybe?

This is one of a few that I've written:

http://repo.or.cz/w/qemu/aliguori-queue.git/blob/refs/heads/qpi:/tests/test-buffer.c

You can see the rest of the plumbing in the tree.  The libcheck tests 
are also pretty reasonable although to be honest I dislike libcheck.  I 
don't find it to offer any real value compared to just something that 
succeeds silently or fails upon error.

I'm working on isolating the serial device right now to write tests 
against it but decoupling it from everything else is a fair bit of 
work.  I'd eventually like to get to the point where all new code had 
corresponding in tree unit tests.

Regards,

Anthony Liguori

> thanks,
>   Gerd
>
Gerd Hoffmann - Aug. 20, 2010, 10:38 a.m.
Hi,

> You can see the rest of the plumbing in the tree.

Hmm, "make test" (and trying to build anything in tests/) fails with 
separate source/build dirs.  But it isn't obvious to me why it does. 
The Makefile takes care to set vpath, so I think it should have picked 
up the source files just fine ...

Ideas anyone?

cheers,
   Gerd

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 4a1eaa1..edfca87 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -83,6 +83,7 @@  common-obj-y += qemu-char.o savevm.o #aio.o
 common-obj-y += msmouse.o ps2.o
 common-obj-y += qdev.o qdev-properties.o
 common-obj-y += block-migration.o
+common-obj-y += pflib.o
 
 common-obj-$(CONFIG_BRLAPI) += baum.o
 common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
diff --git a/pflib.c b/pflib.c
new file mode 100644
index 0000000..1154d0c
--- /dev/null
+++ b/pflib.c
@@ -0,0 +1,213 @@ 
+/*
+ * PixelFormat conversion library.
+ *
+ * Author: Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+#include "qemu-common.h"
+#include "console.h"
+#include "pflib.h"
+
+typedef struct QemuPixel QemuPixel;
+
+typedef void (*pf_convert)(QemuPfConv *conv,
+                           void *dst, void *src, uint32_t cnt);
+typedef void (*pf_convert_from)(PixelFormat *pf,
+                                QemuPixel *dst, void *src, uint32_t cnt);
+typedef void (*pf_convert_to)(PixelFormat *pf,
+                              void *dst, QemuPixel *src, uint32_t cnt);
+
+struct QemuPfConv {
+    pf_convert        convert;
+    PixelFormat       src;
+    PixelFormat       dst;
+
+    /* for copy_generic() */
+    pf_convert_from   conv_from;
+    pf_convert_to     conv_to;
+    QemuPixel         *conv_buf;
+    uint32_t          conv_cnt;
+};
+
+struct QemuPixel {
+    uint8_t red;
+    uint8_t green;
+    uint8_t blue;
+    uint8_t alpha;
+};
+
+/* ----------------------------------------------------------------------- */
+/* PixelFormat -> QemuPixel conversions                                    */
+
+static void conv_16_to_pixel(PixelFormat *pf,
+                             QemuPixel *dst, void *src, uint32_t cnt)
+{
+    uint16_t *src16 = src;
+
+    while (cnt > 0) {
+        dst->red   = ((*src16 & pf->rmask) >> pf->rshift) << (8 - pf->rbits);
+        dst->green = ((*src16 & pf->gmask) >> pf->gshift) << (8 - pf->gbits);
+        dst->blue  = ((*src16 & pf->bmask) >> pf->bshift) << (8 - pf->bbits);
+        dst->alpha = ((*src16 & pf->amask) >> pf->ashift) << (8 - pf->abits);
+        dst++, src16++, cnt--;
+    }
+}
+
+/* assumes pf->{r,g,b,a}bits == 8 */
+static void conv_32_to_pixel_fast(PixelFormat *pf,
+                                  QemuPixel *dst, void *src, uint32_t cnt)
+{
+    uint32_t *src32 = src;
+
+    while (cnt > 0) {
+        dst->red   = (*src32 & pf->rmask) >> pf->rshift;
+        dst->green = (*src32 & pf->gmask) >> pf->gshift;
+        dst->blue  = (*src32 & pf->bmask) >> pf->bshift;
+        dst->alpha = (*src32 & pf->amask) >> pf->ashift;
+        dst++, src32++, cnt--;
+    }
+}
+
+static void conv_32_to_pixel_generic(PixelFormat *pf,
+                                     QemuPixel *dst, void *src, uint32_t cnt)
+{
+    uint32_t *src32 = src;
+
+    while (cnt > 0) {
+        if (pf->rbits < 8) {
+            dst->red   = ((*src32 & pf->rmask) >> pf->rshift) << (8 - pf->rbits);
+        } else {
+            dst->red   = ((*src32 & pf->rmask) >> pf->rshift) >> (pf->rbits - 8);
+        }
+        if (pf->gbits < 8) {
+            dst->green = ((*src32 & pf->gmask) >> pf->gshift) << (8 - pf->gbits);
+        } else {
+            dst->green = ((*src32 & pf->gmask) >> pf->gshift) >> (pf->gbits - 8);
+        }
+        if (pf->bbits < 8) {
+            dst->blue  = ((*src32 & pf->bmask) >> pf->bshift) << (8 - pf->bbits);
+        } else {
+            dst->blue  = ((*src32 & pf->bmask) >> pf->bshift) >> (pf->bbits - 8);
+        }
+        if (pf->abits < 8) {
+            dst->alpha = ((*src32 & pf->amask) >> pf->ashift) << (8 - pf->abits);
+        } else {
+            dst->alpha = ((*src32 & pf->amask) >> pf->ashift) >> (pf->abits - 8);
+        }
+        dst++, src32++, cnt--;
+    }
+}
+
+/* ----------------------------------------------------------------------- */
+/* QemuPixel -> PixelFormat conversions                                    */
+
+static void conv_pixel_to_16(PixelFormat *pf,
+                             void *dst, QemuPixel *src, uint32_t cnt)
+{
+    uint16_t *dst16 = dst;
+
+    while (cnt > 0) {
+        *dst16  = ((uint16_t)src->red   >> (8 - pf->rbits)) << pf->rshift;
+        *dst16 |= ((uint16_t)src->green >> (8 - pf->gbits)) << pf->gshift;
+        *dst16 |= ((uint16_t)src->blue  >> (8 - pf->bbits)) << pf->bshift;
+        *dst16 |= ((uint16_t)src->alpha >> (8 - pf->abits)) << pf->ashift;
+        dst16++, src++, cnt--;
+    }
+}
+
+static void conv_pixel_to_32(PixelFormat *pf,
+                             void *dst, QemuPixel *src, uint32_t cnt)
+{
+    uint32_t *dst32 = dst;
+
+    while (cnt > 0) {
+        *dst32  = ((uint32_t)src->red   >> (8 - pf->rbits)) << pf->rshift;
+        *dst32 |= ((uint32_t)src->green >> (8 - pf->gbits)) << pf->gshift;
+        *dst32 |= ((uint32_t)src->blue  >> (8 - pf->bbits)) << pf->bshift;
+        *dst32 |= ((uint32_t)src->alpha >> (8 - pf->abits)) << pf->ashift;
+        dst32++, src++, cnt--;
+    }
+}
+
+/* ----------------------------------------------------------------------- */
+/* PixelFormat -> PixelFormat conversions                                  */
+
+static void convert_copy(QemuPfConv *conv, void *dst, void *src, uint32_t cnt)
+{
+    uint32_t bytes = cnt * conv->src.bytes_per_pixel;
+    memcpy(dst, src, bytes);
+}
+
+static void convert_generic(QemuPfConv *conv, void *dst, void *src, uint32_t cnt)
+{
+    if (conv->conv_cnt < cnt) {
+        conv->conv_cnt = cnt;
+        conv->conv_buf = qemu_realloc(conv->conv_buf, sizeof(QemuPixel) * conv->conv_cnt);
+    }
+    conv->conv_from(&conv->src, conv->conv_buf, src, cnt);
+    conv->conv_to(&conv->dst, dst, conv->conv_buf, cnt);
+}
+
+/* ----------------------------------------------------------------------- */
+/* public interface                                                        */
+
+QemuPfConv *qemu_pf_conv_get(PixelFormat *dst, PixelFormat *src)
+{
+    QemuPfConv *conv = qemu_mallocz(sizeof(QemuPfConv));
+
+    conv->src = *src;
+    conv->dst = *dst;
+
+    if (memcmp(&conv->src, &conv->dst, sizeof(PixelFormat)) == 0) {
+        /* formats identical, can simply copy */
+        conv->convert = convert_copy;
+    } else {
+        /* generic two-step conversion: src -> QemuPixel -> dst  */
+        switch (conv->src.bytes_per_pixel) {
+        case 2:
+            conv->conv_from = conv_16_to_pixel;
+            break;
+        case 4:
+            if (conv->src.rbits == 8 && conv->src.gbits == 8 && conv->src.bbits == 8) {
+                conv->conv_from = conv_32_to_pixel_fast;
+            } else {
+                conv->conv_from = conv_32_to_pixel_generic;
+            }
+            break;
+        default:
+            goto err;
+        }
+        switch (conv->dst.bytes_per_pixel) {
+        case 2:
+            conv->conv_to = conv_pixel_to_16;
+            break;
+        case 4:
+            conv->conv_to = conv_pixel_to_32;
+            break;
+        default:
+            goto err;
+        }
+        conv->convert = convert_generic;
+    }
+    return conv;
+
+err:
+    qemu_free(conv);
+    return NULL;
+}
+
+void qemu_pf_conv_run(QemuPfConv *conv, void *dst, void *src, uint32_t cnt)
+{
+    conv->convert(conv, dst, src, cnt);
+}
+
+void qemu_pf_conv_put(QemuPfConv *conv)
+{
+    if (conv) {
+        qemu_free(conv->conv_buf);
+        qemu_free(conv);
+    }
+}
diff --git a/pflib.h b/pflib.h
new file mode 100644
index 0000000..b70c313
--- /dev/null
+++ b/pflib.h
@@ -0,0 +1,20 @@ 
+#ifndef __QEMU_PFLIB_H
+#define __QEMU_PFLIB_H
+
+/*
+ * PixelFormat conversion library.
+ *
+ * Author: Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+typedef struct QemuPfConv QemuPfConv;
+
+QemuPfConv *qemu_pf_conv_get(PixelFormat *dst, PixelFormat *src);
+void qemu_pf_conv_run(QemuPfConv *conv, void *dst, void *src, uint32_t cnt);
+void qemu_pf_conv_put(QemuPfConv *conv);
+
+#endif