[PATCH 0/2] convert img to xpm

[PATCH 0/2] convert img to xpm

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]

Changes from last version:
* Added length parameter
* created enum for scheme
* added sanity checking for img length
* added character lookup table to keep color strings
  to no more than 2 chars per pixel.

Kristen Carlson Accardi (2):
  stkutil: convert img to xpm
  test-stkutil: unit test for img to xpm converter

 src/stkutil.c       |  148 +++++++++++++++++++++++++++++++
 src/stkutil.h       |    8 ++
 unit/test-stkutil.c |  240 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 396 insertions(+), 0 deletions(-)

[PATCH 1/2] stkutil: convert img to xpm

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 5258 bytes --]

---
 src/stkutil.c |  148 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/stkutil.h |    8 +++
 2 files changed, 156 insertions(+), 0 deletions(-)

diff --git a/src/stkutil.c b/src/stkutil.c
index 9cac850..46ae026 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -6076,3 +6076,151 @@ char *stk_text_to_html(const char *utf8,
 	/* return characters from string. Caller must free char data */
 	return g_string_free(string, FALSE);
 }
+
+static const char chars_table[] = {
+	'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C',
+	'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P',
+	'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', 'a', 'b', 'c',
+	'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p',
+	'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '+', '.' };
+
+char *stk_image_to_xpm(const unsigned char *img, unsigned int len,
+			enum stk_img_scheme scheme)
+{
+	guint8 width, height;
+	unsigned int ncolors, nbits, entry, cpp;
+	unsigned int i, j;
+	int bit, k;
+	unsigned short clut_offset = 0;
+	guint8 *clut;
+	GString *xpm;
+	unsigned int pos = 0;
+	const char xpm_header[] = "/* XPM */\n";
+	const char declaration[] = "static char *xpm[] = {\n";
+	char c[3];
+
+	if (img == NULL)
+		return NULL;
+
+	/* sanity check length */
+	if (len < 3)
+		return NULL;
+
+	width = img[pos++];
+	height = img[pos++];
+
+	if (scheme == STK_IMG_SCHEME_BASIC) {
+		nbits = 1;
+		ncolors = 2;
+
+		if (pos + ((width * height)/8) > len)
+			return NULL;
+	} else {
+		/* sanity check length */
+		if (pos + 4 > len)
+			return NULL;
+
+		nbits = img[pos++];
+		ncolors = img[pos++];
+
+		/* the value of zero should be interpreted as 256 */
+		if (ncolors == 0)
+			ncolors = 256;
+
+		clut_offset = img[pos++] << 8;
+		clut_offset |= img[pos++];
+
+		if ((clut_offset + (ncolors * 3) > len) ||
+			(pos + (width * height * nbits)/8) > clut_offset)
+			return NULL;
+	}
+
+	/* determine the number of chars need to represent the pixel */
+	cpp = ncolors > 64 ? 2 : 1;
+
+	/*
+	 * space needed:
+	 * 	header line
+	 *	declaration and beginning of assignment line
+	 *	values - max length of 19
+	 *	colors - ncolors * (cpp + whitespace + deliminators + color)
+	 *	pixels - width * height * cpp + height deliminators "",\n
+	 *	end of assignment - 2 chars "};"
+	 */
+	xpm = g_string_sized_new(strlen(xpm_header) + strlen(declaration) +
+				19 + ((cpp + 14) * ncolors) +
+				(width * height * cpp) + (4 * height) + 2);
+	if (xpm == NULL)
+		return NULL;
+
+	/* add header, declaration, values */
+	g_string_append(xpm, xpm_header);
+	g_string_append(xpm, declaration);
+	g_string_append_printf(xpm, "\"%d %d %d %d\",\n", width, height,
+				ncolors, cpp);
+
+	/* create colors */
+	if (scheme == STK_IMG_SCHEME_BASIC) {
+		g_string_append(xpm, "\"0\tc #000000\",\n");
+		g_string_append(xpm, "\"1\tc #FFFFFF\",\n");
+	} else {
+		clut = (guint8 *) &img[clut_offset];
+
+		for (i = 0; i < ncolors; i++) {
+			/* lookup char representation of this number */
+			if (ncolors > 64) {
+				c[0] = chars_table[i / 64];
+				c[1] = chars_table[i % 64];
+				c[2] = '\0';
+			} else {
+				c[0] = chars_table[i % 64];
+				c[1] = '\0';
+			}
+
+			if ((i == (ncolors - 1)) &&
+					scheme == STK_IMG_SCHEME_TRANSPARENCY)
+				g_string_append_printf(xpm,
+					"\"%s\tc None\",\n", c);
+			else
+				g_string_append_printf(xpm,
+					"\"%s\tc #%02hhX%02hhX%02hhX\",\n",
+					c, clut[0], clut[1], clut[2]);
+			clut += 3;
+		}
+	}
+
+	/* height rows of width pixels */
+	k = 7;
+	for (i = 0; i < height; i++) {
+		g_string_append(xpm, "\"");
+		for (j = 0; j < width; j++) {
+			entry = 0;
+			for (bit = nbits - 1; bit >= 0; bit--) {
+				entry |= (img[pos] >> k & 0x1) << bit;
+				k--;
+
+				/* see if we crossed a byte boundary */
+				if (k < 0) {
+					k = 7;
+					pos++;
+				}
+			}
+
+			/* lookup char representation of this number */
+			if (ncolors > 64) {
+				c[0] = chars_table[entry / 64];
+				c[1] = chars_table[entry % 64];
+				c[2] = '\0';
+			} else {
+				c[0] = chars_table[entry % 64];
+				c[1] = '\0';
+			}
+			g_string_append_printf(xpm, "%s", c);
+		}
+		g_string_append(xpm, "\",\n");
+	}
+	g_string_append(xpm, "};");
+
+	/* Caller must free char data */
+	return g_string_free(xpm, FALSE);
+}
diff --git a/src/stkutil.h b/src/stkutil.h
index 1fbd68b..1e33a65 100644
--- a/src/stkutil.h
+++ b/src/stkutil.h
@@ -557,6 +557,12 @@ enum stk_me_status {
 	STK_ME_STATUS_NOT_IDLE = 	0x01
 };
 
+enum stk_img_scheme {
+	STK_IMG_SCHEME_BASIC =		0x11,
+	STK_IMG_SCHEME_COLOR =		0x21,
+	STK_IMG_SCHEME_TRANSPARENCY =	0x22,
+};
+
 /* For data object that only has a byte array with undetermined length */
 struct stk_common_byte_array {
 	unsigned char *array;
@@ -1644,3 +1650,5 @@ const unsigned char *stk_pdu_from_envelope(const struct stk_envelope *envelope,
 						unsigned int *out_length);
 char *stk_text_to_html(const char *text,
 				const unsigned short *attrs, int num_attrs);
+char *stk_image_to_xpm(const unsigned char *img, unsigned int len,
+			enum stk_img_scheme scheme);
-- 
1.6.6.1

Re: [PATCH 1/2] stkutil: convert img to xpm

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 7233 bytes --]

Hi Kristen,

On 07/22/2010 07:10 PM, Kristen Carlson Accardi wrote:
> ---
>  src/stkutil.c |  148 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/stkutil.h |    8 +++
>  2 files changed, 156 insertions(+), 0 deletions(-)
> 
> diff --git a/src/stkutil.c b/src/stkutil.c
> index 9cac850..46ae026 100644
> --- a/src/stkutil.c
> +++ b/src/stkutil.c
> @@ -6076,3 +6076,151 @@ char *stk_text_to_html(const char *utf8,
>  	/* return characters from string. Caller must free char data */
>  	return g_string_free(string, FALSE);
>  }
> +
> +static const char chars_table[] = {
> +	'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C',
> +	'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P',
> +	'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', 'a', 'b', 'c',
> +	'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p',
> +	'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '+', '.' };
> +
> +char *stk_image_to_xpm(const unsigned char *img, unsigned int len,
> +			enum stk_img_scheme scheme)
> +{
> +	guint8 width, height;
> +	unsigned int ncolors, nbits, entry, cpp;
> +	unsigned int i, j;
> +	int bit, k;
> +	unsigned short clut_offset = 0;
> +	guint8 *clut;
> +	GString *xpm;
> +	unsigned int pos = 0;
> +	const char xpm_header[] = "/* XPM */\n";
> +	const char declaration[] = "static char *xpm[] = {\n";
> +	char c[3];
> +
> +	if (img == NULL)
> +		return NULL;
> +
> +	/* sanity check length */
> +	if (len < 3)
> +		return NULL;
> +
> +	width = img[pos++];
> +	height = img[pos++];
> +
> +	if (scheme == STK_IMG_SCHEME_BASIC) {
> +		nbits = 1;
> +		ncolors = 2;
> +
> +		if (pos + ((width * height)/8) > len)
> +			return NULL;

You probably want (pos + (width * height + 7) / 8) > len here.  Also,
please keep doc/coding-style.txt Section M3 in mind here.

> +	} else {
> +		/* sanity check length */
> +		if (pos + 4 > len)
> +			return NULL;
> +
> +		nbits = img[pos++];
> +		ncolors = img[pos++];
> +
> +		/* the value of zero should be interpreted as 256 */
> +		if (ncolors == 0)
> +			ncolors = 256;
> +
> +		clut_offset = img[pos++] << 8;
> +		clut_offset |= img[pos++];
> +
> +		if ((clut_offset + (ncolors * 3) > len) ||
> +			(pos + (width * height * nbits)/8) > clut_offset)

Please keep in mind doc/coding-style.txt Section M4.  Sometimes it is
better to separate the || conditions into two separate ifs to satisfy
this rule.

Also, you're off by 1 again here.  You probably want (width * height *
nbits + 7) / 8.

This also brings up another point.  You're assuming that the caller is
appending the CLUT right after the image data and massaging the clut
offset appropriately.  This is a really bad idea since the caller will
have to do some significant pre-processing.

You can handle this in one of two ways:

1. Assume the calling logic will read the entire image file before
calling this function.  In this case, modifying the signature as follows
might be a good idea:

char *stk_image_to_xpm(const unsigned char *file,
			unsigned short file_len,
			enum stk_img_scheme scheme,
			unsigned short img_offset,
			unsigned short img_len);

2. Assume the calling logic is clever and will optimize reading of the
file, including peeking into the image header to determine the where the
CLUT is located and reading it.  In this case you can either reuse the
signature from 1 above, or come up with something else.

Remember, reading from the SIM is extremely slow, so any and all clever
optimization tricks are definitely wanted.

> +			return NULL;
> +	}
> +
> +	/* determine the number of chars need to represent the pixel */
> +	cpp = ncolors > 64 ? 2 : 1;
> +
> +	/*
> +	 * space needed:
> +	 * 	header line
> +	 *	declaration and beginning of assignment line
> +	 *	values - max length of 19
> +	 *	colors - ncolors * (cpp + whitespace + deliminators + color)
> +	 *	pixels - width * height * cpp + height deliminators "",\n
> +	 *	end of assignment - 2 chars "};"
> +	 */
> +	xpm = g_string_sized_new(strlen(xpm_header) + strlen(declaration) +
> +				19 + ((cpp + 14) * ncolors) +
> +				(width * height * cpp) + (4 * height) + 2);
> +	if (xpm == NULL)
> +		return NULL;
> +
> +	/* add header, declaration, values */
> +	g_string_append(xpm, xpm_header);
> +	g_string_append(xpm, declaration);
> +	g_string_append_printf(xpm, "\"%d %d %d %d\",\n", width, height,
> +				ncolors, cpp);
> +
> +	/* create colors */
> +	if (scheme == STK_IMG_SCHEME_BASIC) {
> +		g_string_append(xpm, "\"0\tc #000000\",\n");
> +		g_string_append(xpm, "\"1\tc #FFFFFF\",\n");
> +	} else {
> +		clut = (guint8 *) &img[clut_offset];
> +
> +		for (i = 0; i < ncolors; i++) {
> +			/* lookup char representation of this number */
> +			if (ncolors > 64) {
> +				c[0] = chars_table[i / 64];
> +				c[1] = chars_table[i % 64];
> +				c[2] = '\0';
> +			} else {
> +				c[0] = chars_table[i % 64];
> +				c[1] = '\0';
> +			}
> +
> +			if ((i == (ncolors - 1)) &&
> +					scheme == STK_IMG_SCHEME_TRANSPARENCY)
> +				g_string_append_printf(xpm,
> +					"\"%s\tc None\",\n", c);
> +			else
> +				g_string_append_printf(xpm,
> +					"\"%s\tc #%02hhX%02hhX%02hhX\",\n",
> +					c, clut[0], clut[1], clut[2]);
> +			clut += 3;
> +		}
> +	}
> +
> +	/* height rows of width pixels */
> +	k = 7;
> +	for (i = 0; i < height; i++) {
> +		g_string_append(xpm, "\"");
> +		for (j = 0; j < width; j++) {
> +			entry = 0;
> +			for (bit = nbits - 1; bit >= 0; bit--) {
> +				entry |= (img[pos] >> k & 0x1) << bit;
> +				k--;
> +
> +				/* see if we crossed a byte boundary */
> +				if (k < 0) {
> +					k = 7;
> +					pos++;
> +				}
> +			}
> +
> +			/* lookup char representation of this number */
> +			if (ncolors > 64) {
> +				c[0] = chars_table[entry / 64];
> +				c[1] = chars_table[entry % 64];
> +				c[2] = '\0';
> +			} else {
> +				c[0] = chars_table[entry % 64];
> +				c[1] = '\0';
> +			}

There should be an empty line here

> +			g_string_append_printf(xpm, "%s", c);
> +		}

And another empty line here.

> +		g_string_append(xpm, "\",\n");
> +	}

And another empty line here.

> +	g_string_append(xpm, "};");
> +
> +	/* Caller must free char data */
> +	return g_string_free(xpm, FALSE);
> +}
> diff --git a/src/stkutil.h b/src/stkutil.h
> index 1fbd68b..1e33a65 100644
> --- a/src/stkutil.h
> +++ b/src/stkutil.h
> @@ -557,6 +557,12 @@ enum stk_me_status {
>  	STK_ME_STATUS_NOT_IDLE = 	0x01
>  };
>  
> +enum stk_img_scheme {
> +	STK_IMG_SCHEME_BASIC =		0x11,
> +	STK_IMG_SCHEME_COLOR =		0x21,
> +	STK_IMG_SCHEME_TRANSPARENCY =	0x22,
> +};
> +
>  /* For data object that only has a byte array with undetermined length */
>  struct stk_common_byte_array {
>  	unsigned char *array;
> @@ -1644,3 +1650,5 @@ const unsigned char *stk_pdu_from_envelope(const struct stk_envelope *envelope,
>  						unsigned int *out_length);
>  char *stk_text_to_html(const char *text,
>  				const unsigned short *attrs, int num_attrs);
> +char *stk_image_to_xpm(const unsigned char *img, unsigned int len,
> +			enum stk_img_scheme scheme);

Regards,
-Denis

Re: [PATCH 1/2] stkutil: convert img to xpm

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 1744 bytes --]

On Fri, 23 Jul 2010 15:03:59 -0500
Denis Kenzior <denkenz@gmail.com> wrote:
> 
> This also brings up another point.  You're assuming that the caller is
> appending the CLUT right after the image data and massaging the clut
> offset appropriately.  This is a really bad idea since the caller will
> have to do some significant pre-processing.
> 
> You can handle this in one of two ways:
> 
> 1. Assume the calling logic will read the entire image file before
> calling this function.  In this case, modifying the signature as follows
> might be a good idea:
> 
> char *stk_image_to_xpm(const unsigned char *file,
> 			unsigned short file_len,
> 			enum stk_img_scheme scheme,
> 			unsigned short img_offset,
> 			unsigned short img_len);
> 
> 2. Assume the calling logic is clever and will optimize reading of the
> file, including peeking into the image header to determine the where the
> CLUT is located and reading it.  In this case you can either reuse the
> signature from 1 above, or come up with something else.
> 
> Remember, reading from the SIM is extremely slow, so any and all clever
> optimization tricks are definitely wanted.

So, is it likely given normal usage that we'll access an image a single
image at a time, or is it more likely that we'll access a bunch of images
all at once?  It may be better to read an entire image data file (with
multiple images) and keep it cached if we are likely to immediately
need the other images.  In which case I'd be inclined to just pass
in the entire data image file and the offset like you have above.  If 
we are only likely to use a single image for any given length of time,
then it seems better to have the caller be smart and pass us the clut.

Re: [PATCH 1/2] stkutil: convert img to xpm

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2569 bytes --]

Hi Kristen,

On 07/23/2010 03:52 PM, Kristen Carlson Accardi wrote:
> On Fri, 23 Jul 2010 15:03:59 -0500
> Denis Kenzior <denkenz@gmail.com> wrote:
>>
>> This also brings up another point.  You're assuming that the caller is
>> appending the CLUT right after the image data and massaging the clut
>> offset appropriately.  This is a really bad idea since the caller will
>> have to do some significant pre-processing.
>>
>> You can handle this in one of two ways:
>>
>> 1. Assume the calling logic will read the entire image file before
>> calling this function.  In this case, modifying the signature as follows
>> might be a good idea:
>>
>> char *stk_image_to_xpm(const unsigned char *file,
>> 			unsigned short file_len,
>> 			enum stk_img_scheme scheme,
>> 			unsigned short img_offset,
>> 			unsigned short img_len);
>>
>> 2. Assume the calling logic is clever and will optimize reading of the
>> file, including peeking into the image header to determine the where the
>> CLUT is located and reading it.  In this case you can either reuse the
>> signature from 1 above, or come up with something else.
>>
>> Remember, reading from the SIM is extremely slow, so any and all clever
>> optimization tricks are definitely wanted.
> 
> So, is it likely given normal usage that we'll access an image a single
> image at a time, or is it more likely that we'll access a bunch of images
> all at once?  It may be better to read an entire image data file (with
> multiple images) and keep it cached if we are likely to immediately
> need the other images.  In which case I'd be inclined to just pass
> in the entire data image file and the offset like you have above.  If 
> we are only likely to use a single image for any given length of time,
> then it seems better to have the caller be smart and pass us the clut.

The problem is we just don't know, so we have to assume the worst case.
 Anything that minimizes the number of reads is a good thing (TM).

For instance, you might have couple of dozen images from EFimg dispersed
among multiple EFiidf files.  Each EFiidf file might be 65K in length,
but EFimg files might only refer to about 10k from all of them.

It is perfectly OK for EFiidf to contain mostly garbage (e.g. for future
updates, installation of new SIM Toolkit applications on the SIM +
associated image data, etc)  So you simply can't assume any sort of
packing or efficient storage use.

In this case, a clever algorithm that minimizes the number of SIM
fetches is needed.

Regards,
-Denis

Re: [PATCH 1/2] stkutil: convert img to xpm

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 3243 bytes --]

On Fri, 23 Jul 2010 16:03:16 -0500
Denis Kenzior <denkenz@gmail.com> wrote:

> Hi Kristen,
> 
> On 07/23/2010 03:52 PM, Kristen Carlson Accardi wrote:
> > On Fri, 23 Jul 2010 15:03:59 -0500
> > Denis Kenzior <denkenz@gmail.com> wrote:
> >>
> >> This also brings up another point.  You're assuming that the caller is
> >> appending the CLUT right after the image data and massaging the clut
> >> offset appropriately.  This is a really bad idea since the caller will
> >> have to do some significant pre-processing.
> >>
> >> You can handle this in one of two ways:
> >>
> >> 1. Assume the calling logic will read the entire image file before
> >> calling this function.  In this case, modifying the signature as follows
> >> might be a good idea:
> >>
> >> char *stk_image_to_xpm(const unsigned char *file,
> >> 			unsigned short file_len,
> >> 			enum stk_img_scheme scheme,
> >> 			unsigned short img_offset,
> >> 			unsigned short img_len);
> >>
> >> 2. Assume the calling logic is clever and will optimize reading of the
> >> file, including peeking into the image header to determine the where the
> >> CLUT is located and reading it.  In this case you can either reuse the
> >> signature from 1 above, or come up with something else.
> >>
> >> Remember, reading from the SIM is extremely slow, so any and all clever
> >> optimization tricks are definitely wanted.
> > 
> > So, is it likely given normal usage that we'll access an image a single
> > image at a time, or is it more likely that we'll access a bunch of images
> > all at once?  It may be better to read an entire image data file (with
> > multiple images) and keep it cached if we are likely to immediately
> > need the other images.  In which case I'd be inclined to just pass
> > in the entire data image file and the offset like you have above.  If 
> > we are only likely to use a single image for any given length of time,
> > then it seems better to have the caller be smart and pass us the clut.
> 
> The problem is we just don't know, so we have to assume the worst case.
>  Anything that minimizes the number of reads is a good thing (TM).
> 
> For instance, you might have couple of dozen images from EFimg dispersed
> among multiple EFiidf files.  Each EFiidf file might be 65K in length,
> but EFimg files might only refer to about 10k from all of them.
> 
> It is perfectly OK for EFiidf to contain mostly garbage (e.g. for future
> updates, installation of new SIM Toolkit applications on the SIM +
> associated image data, etc)  So you simply can't assume any sort of
> packing or efficient storage use.
> 
> In this case, a clever algorithm that minimizes the number of SIM
> fetches is needed.
> 
> Regards,
> -Denis

If that is the case then I propose we assume a smart algorithm fetched
our data for us and change the signature to something like this:

unsigned char *stk_image_to_xpm(const unsigned char *image_body,
				enum stk_image_scheme scheme,
				unsigned int height,
				unsigned int width, unsigned int ncolors,
				const unsigned char *clut);

We would assume that the caller has done all the sanity checking on
the file and handed us a clut of appropriate size as well.

Re: [PATCH 1/2] stkutil: convert img to xpm

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 3571 bytes --]

Hi Kristen,

On 07/23/2010 04:39 PM, Kristen Carlson Accardi wrote:
> On Fri, 23 Jul 2010 16:03:16 -0500
> Denis Kenzior <denkenz@gmail.com> wrote:
> 
>> Hi Kristen,
>>
>> On 07/23/2010 03:52 PM, Kristen Carlson Accardi wrote:
>>> On Fri, 23 Jul 2010 15:03:59 -0500
>>> Denis Kenzior <denkenz@gmail.com> wrote:
>>>>
>>>> This also brings up another point.  You're assuming that the caller is
>>>> appending the CLUT right after the image data and massaging the clut
>>>> offset appropriately.  This is a really bad idea since the caller will
>>>> have to do some significant pre-processing.
>>>>
>>>> You can handle this in one of two ways:
>>>>
>>>> 1. Assume the calling logic will read the entire image file before
>>>> calling this function.  In this case, modifying the signature as follows
>>>> might be a good idea:
>>>>
>>>> char *stk_image_to_xpm(const unsigned char *file,
>>>> 			unsigned short file_len,
>>>> 			enum stk_img_scheme scheme,
>>>> 			unsigned short img_offset,
>>>> 			unsigned short img_len);
>>>>
>>>> 2. Assume the calling logic is clever and will optimize reading of the
>>>> file, including peeking into the image header to determine the where the
>>>> CLUT is located and reading it.  In this case you can either reuse the
>>>> signature from 1 above, or come up with something else.
>>>>
>>>> Remember, reading from the SIM is extremely slow, so any and all clever
>>>> optimization tricks are definitely wanted.
>>>
>>> So, is it likely given normal usage that we'll access an image a single
>>> image at a time, or is it more likely that we'll access a bunch of images
>>> all at once?  It may be better to read an entire image data file (with
>>> multiple images) and keep it cached if we are likely to immediately
>>> need the other images.  In which case I'd be inclined to just pass
>>> in the entire data image file and the offset like you have above.  If 
>>> we are only likely to use a single image for any given length of time,
>>> then it seems better to have the caller be smart and pass us the clut.
>>
>> The problem is we just don't know, so we have to assume the worst case.
>>  Anything that minimizes the number of reads is a good thing (TM).
>>
>> For instance, you might have couple of dozen images from EFimg dispersed
>> among multiple EFiidf files.  Each EFiidf file might be 65K in length,
>> but EFimg files might only refer to about 10k from all of them.
>>
>> It is perfectly OK for EFiidf to contain mostly garbage (e.g. for future
>> updates, installation of new SIM Toolkit applications on the SIM +
>> associated image data, etc)  So you simply can't assume any sort of
>> packing or efficient storage use.
>>
>> In this case, a clever algorithm that minimizes the number of SIM
>> fetches is needed.
>>
>> Regards,
>> -Denis
> 
> If that is the case then I propose we assume a smart algorithm fetched
> our data for us and change the signature to something like this:
> 
> unsigned char *stk_image_to_xpm(const unsigned char *image_body,
> 				enum stk_image_scheme scheme,
> 				unsigned int height,
> 				unsigned int width, unsigned int ncolors,
> 				const unsigned char *clut);
> 
> We would assume that the caller has done all the sanity checking on
> the file and handed us a clut of appropriate size as well.

You sure you want unsigned char * as the return and not char *?

Also, you might want to change unsigned ints to unsigned chars.  The
image size and CLUT size can never be more than 256.

Regards,
-Denis

Re: [PATCH 1/2] stkutil: convert img to xpm

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 3982 bytes --]

On Fri, 23 Jul 2010 16:46:51 -0500
Denis Kenzior <denkenz@gmail.com> wrote:

> Hi Kristen,
> 
> On 07/23/2010 04:39 PM, Kristen Carlson Accardi wrote:
> > On Fri, 23 Jul 2010 16:03:16 -0500
> > Denis Kenzior <denkenz@gmail.com> wrote:
> > 
> >> Hi Kristen,
> >>
> >> On 07/23/2010 03:52 PM, Kristen Carlson Accardi wrote:
> >>> On Fri, 23 Jul 2010 15:03:59 -0500
> >>> Denis Kenzior <denkenz@gmail.com> wrote:
> >>>>
> >>>> This also brings up another point.  You're assuming that the caller is
> >>>> appending the CLUT right after the image data and massaging the clut
> >>>> offset appropriately.  This is a really bad idea since the caller will
> >>>> have to do some significant pre-processing.
> >>>>
> >>>> You can handle this in one of two ways:
> >>>>
> >>>> 1. Assume the calling logic will read the entire image file before
> >>>> calling this function.  In this case, modifying the signature as follows
> >>>> might be a good idea:
> >>>>
> >>>> char *stk_image_to_xpm(const unsigned char *file,
> >>>> 			unsigned short file_len,
> >>>> 			enum stk_img_scheme scheme,
> >>>> 			unsigned short img_offset,
> >>>> 			unsigned short img_len);
> >>>>
> >>>> 2. Assume the calling logic is clever and will optimize reading of the
> >>>> file, including peeking into the image header to determine the where the
> >>>> CLUT is located and reading it.  In this case you can either reuse the
> >>>> signature from 1 above, or come up with something else.
> >>>>
> >>>> Remember, reading from the SIM is extremely slow, so any and all clever
> >>>> optimization tricks are definitely wanted.
> >>>
> >>> So, is it likely given normal usage that we'll access an image a single
> >>> image at a time, or is it more likely that we'll access a bunch of images
> >>> all at once?  It may be better to read an entire image data file (with
> >>> multiple images) and keep it cached if we are likely to immediately
> >>> need the other images.  In which case I'd be inclined to just pass
> >>> in the entire data image file and the offset like you have above.  If 
> >>> we are only likely to use a single image for any given length of time,
> >>> then it seems better to have the caller be smart and pass us the clut.
> >>
> >> The problem is we just don't know, so we have to assume the worst case.
> >>  Anything that minimizes the number of reads is a good thing (TM).
> >>
> >> For instance, you might have couple of dozen images from EFimg dispersed
> >> among multiple EFiidf files.  Each EFiidf file might be 65K in length,
> >> but EFimg files might only refer to about 10k from all of them.
> >>
> >> It is perfectly OK for EFiidf to contain mostly garbage (e.g. for future
> >> updates, installation of new SIM Toolkit applications on the SIM +
> >> associated image data, etc)  So you simply can't assume any sort of
> >> packing or efficient storage use.
> >>
> >> In this case, a clever algorithm that minimizes the number of SIM
> >> fetches is needed.
> >>
> >> Regards,
> >> -Denis
> > 
> > If that is the case then I propose we assume a smart algorithm fetched
> > our data for us and change the signature to something like this:
> > 
> > unsigned char *stk_image_to_xpm(const unsigned char *image_body,
> > 				enum stk_image_scheme scheme,
> > 				unsigned int height,
> > 				unsigned int width, unsigned int ncolors,
> > 				const unsigned char *clut);
> > 
> > We would assume that the caller has done all the sanity checking on
> > the file and handed us a clut of appropriate size as well.
> 
> You sure you want unsigned char * as the return and not char *?

Oops - no, I typed that wrong.

> 
> Also, you might want to change unsigned ints to unsigned chars.  The
> image size and CLUT size can never be more than 256.
>

The image size can be greater than 256, but I think you mean the value of
height, width, and ncolors can never be greater than 256, which is true. 

Re: [PATCH 1/2] stkutil: convert img to xpm

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 363 bytes --]

Hi Kristen,

>> Also, you might want to change unsigned ints to unsigned chars.  The
>> image size and CLUT size can never be more than 256.
>>
> 
> The image size can be greater than 256, but I think you mean the value of
> height, width, and ncolors can never be greater than 256, which is true. 

Sorry, yes that is what I meant.

Regards,
-Denis

[PATCH 2/2] test-stkutil: unit test for img to xpm converter

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 9323 bytes --]

---
 unit/test-stkutil.c |  240 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 240 insertions(+), 0 deletions(-)

diff --git a/unit/test-stkutil.c b/unit/test-stkutil.c
index 1d7485a..ea65621 100644
--- a/unit/test-stkutil.c
+++ b/unit/test-stkutil.c
@@ -22693,6 +22693,233 @@ static void test_html_attr(gconstpointer data)
 	check_text_attr_html(&test->text_attr, test->text, test->html);
 }
 
+struct img_xpm_test {
+	const unsigned char *img;
+	unsigned int len;
+	guint8 scheme;
+	char *xpm;
+};
+
+const unsigned char img1[] = { 0x05, 0x05, 0xFE, 0xEB, 0xBF, 0xFF, 0xFF, 0xFF };
+
+const unsigned char img2[] = { 0x08, 0x08, 0x02, 0x03, 0x00, 0x16, 0xAA,
+					0xAA, 0x80, 0x02, 0x85, 0x42, 0x81,
+					0x42, 0x81, 0x42, 0x81, 0x52, 0x80,
+					0x02, 0xAA, 0xAA, 0xFF, 0x00, 0x00,
+					0x00, 0xFF, 0x00, 0x00, 0x00, 0xFF };
+
+const unsigned char img3[] = { 0x2E, 0x28, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+				0x00, 0x01, 0xFF, 0x80, 0x00, 0x00, 0x00, 0x0F,
+				0xFF, 0x00, 0x00, 0x00, 0x00, 0x77, 0xFE, 0x00,
+				0x00, 0x00, 0x01, 0xBF, 0xF8, 0x00, 0x00, 0x00,
+				0x06, 0xFF, 0xE0, 0x00, 0x00, 0x00, 0x1A, 0x03,
+				0x80, 0x00, 0x00, 0x00, 0x6B, 0xF6, 0xBC, 0x00,
+				0x00, 0x01, 0xAF, 0xD8, 0x38, 0x00, 0x00, 0x06,
+				0xBF, 0x60, 0x20, 0x00, 0x00, 0x1A, 0xFD, 0x80,
+				0x40, 0x00, 0x00, 0x6B, 0xF6, 0x00, 0x80, 0x00,
+				0x01, 0xA0, 0x1F, 0x02, 0x00, 0x00, 0x06, 0xFF,
+				0xE4, 0x04, 0x00, 0x00, 0x1B, 0xFF, 0x90, 0x10,
+				0x00, 0x00, 0x6D, 0xEE, 0x40, 0x40, 0x00, 0x01,
+				0xBF, 0xF9, 0x01, 0x00, 0x00, 0x6F, 0xFF, 0xE4,
+				0x04, 0x00, 0x00, 0x1B, 0xFF, 0x90, 0x10, 0x00,
+				0x00, 0x6F, 0xFE, 0x40, 0x40, 0x00, 0x01, 0xBF,
+				0xF9, 0x01, 0x00, 0x00, 0x06, 0xFF, 0xE6, 0x04,
+				0x00, 0x00, 0x1B, 0xFF, 0x88, 0x10, 0x00, 0x00,
+				0x6F, 0xFE, 0x20, 0x40, 0x00, 0x01, 0xBF, 0xF8,
+				0x66, 0x00, 0x00, 0x06, 0xFF, 0xE0, 0xF0, 0x00,
+				0x00, 0x1B, 0xFF, 0x80, 0x80, 0x00, 0x00, 0x7F,
+				0xFE, 0x00, 0x00, 0x00, 0x03, 0x00, 0x0C, 0x00,
+				0x00, 0x00, 0x1F, 0xFF, 0xF8, 0x00, 0x00, 0x00,
+				0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+				0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+				0x1C, 0x21, 0x08, 0x44, 0xEE, 0x00, 0x48, 0xC4,
+				0x31, 0x92, 0x20, 0x01, 0x25, 0x11, 0x45, 0x50,
+				0x80, 0x07, 0x14, 0x45, 0x15, 0x43, 0x80, 0x12,
+				0x71, 0x1C, 0x4D, 0x08, 0x00, 0x4A, 0x24, 0x89,
+				0x32, 0x20, 0x01, 0xC8, 0x9E, 0x24, 0x4E,
+				0xE0 };
+
+const unsigned char img4[] = { 0x18, 0x10, 0xFF, 0xFF, 0xFF, 0x80, 0x00, 0x01,
+				0x80, 0x00, 0x01, 0x80, 0x00, 0x01, 0x8F,
+				0x3C, 0xF1, 0x89, 0x20, 0x81, 0x89, 0x20,
+				0x81, 0x89, 0x20, 0xF1, 0x89, 0x20, 0x11,
+				0x89, 0x20, 0x11, 0x89, 0x20, 0x11, 0x8F,
+				0x3C, 0xF1, 0x80, 0x00, 0x01, 0x80, 0x00,
+				0x01, 0x80, 0x00, 0x01, 0xFF, 0xFF, 0xFF };
+
+const unsigned char img5[] = { 0x08, 0x08, 0xFF, 0x03, 0xA5, 0x99, 0x99,
+				0xA5, 0xC3, 0xFF };
+
+static struct img_xpm_test xpm_test_1 = {
+	.img = img1,
+	.len = sizeof(img1),
+	.scheme = STK_IMG_SCHEME_BASIC,
+	.xpm = "/* XPM */\n"
+		"static char *xpm[] = {\n"
+		"\"5 5 2 1\",\n"
+		"\"0	c #000000\",\n"
+		"\"1	c #FFFFFF\",\n"
+		"\"11111\",\n"
+		"\"11011\",\n"
+		"\"10101\",\n"
+		"\"11011\",\n"
+		"\"11111\",\n"
+		"};",
+};
+
+static struct img_xpm_test xpm_test_2 = {
+	.img = img2,
+	.len = sizeof(img2),
+	.scheme = STK_IMG_SCHEME_COLOR,
+	.xpm = "/* XPM */\n"
+		"static char *xpm[] = {\n"
+		"\"8 8 3 1\",\n"
+		"\"0	c #FF0000\",\n"
+		"\"1	c #00FF00\",\n"
+		"\"2	c #0000FF\",\n"
+		"\"22222222\",\n"
+		"\"20000002\",\n"
+		"\"20111002\",\n"
+		"\"20011002\",\n"
+		"\"20011002\",\n"
+		"\"20011102\",\n"
+		"\"20000002\",\n"
+		"\"22222222\",\n"
+		"};",
+};
+
+static struct img_xpm_test xpm_test_3 = {
+	.img = img3,
+	.len = sizeof(img3),
+	.scheme = STK_IMG_SCHEME_BASIC,
+	.xpm = "/* XPM */\n"
+		"static char *xpm[] = {\n"
+		"\"46 40 2 1\",\n"
+		"\"0	c #000000\",\n"
+		"\"1	c #FFFFFF\",\n"
+		"\"0000000000000000000000000000000000000000000000\",\n"
+		"\"0000000000000000011111111110000000000000000000\",\n"
+		"\"0000000000000000111111111111000000000000000000\",\n"
+		"\"0000000000000001110111111111100000000000000000\",\n"
+		"\"0000000000000001101111111111100000000000000000\",\n"
+		"\"0000000000000001101111111111100000000000000000\",\n"
+		"\"0000000000000001101000000011100000000000000000\",\n"
+		"\"0000000000000001101011111101101011110000000000\",\n"
+		"\"0000000000000001101011111101100000111000000000\",\n"
+		"\"0000000000000001101011111101100000001000000000\",\n"
+		"\"0000000000000001101011111101100000000100000000\",\n"
+		"\"0000000000000001101011111101100000000010000000\",\n"
+		"\"0000000000000001101000000001111100000010000000\",\n"
+		"\"0000000000000001101111111111100100000001000000\",\n"
+		"\"0000000000000001101111111111100100000001000000\",\n"
+		"\"0000000000000001101101111011100100000001000000\",\n"
+		"\"0000000000000001101111111111100100000001000000\",\n"
+		"\"0000000000011011111111111111100100000001000000\",\n"
+		"\"0000000000000001101111111111100100000001000000\",\n"
+		"\"0000000000000001101111111111100100000001000000\",\n"
+		"\"0000000000000001101111111111100100000001000000\",\n"
+		"\"0000000000000001101111111111100110000001000000\",\n"
+		"\"0000000000000001101111111111100010000001000000\",\n"
+		"\"0000000000000001101111111111100010000001000000\",\n"
+		"\"0000000000000001101111111111100001100110000000\",\n"
+		"\"0000000000000001101111111111100000111100000000\",\n"
+		"\"0000000000000001101111111111100000001000000000\",\n"
+		"\"0000000000000001111111111111100000000000000000\",\n"
+		"\"0000000000000011000000000000110000000000000000\",\n"
+		"\"0000000000000111111111111111111000000000000000\",\n"
+		"\"0000000000000000000000000000000000000000000000\",\n"
+		"\"0000000000000000000000000000000000000000000000\",\n"
+		"\"0000000000000000000000000000000000000000000000\",\n"
+		"\"0000011100001000010000100001000100111011100000\",\n"
+		"\"0000010010001100010000110001100100100010000000\",\n"
+		"\"0000010010010100010001010001010101000010000000\",\n"
+		"\"0000011100010100010001010001010101000011100000\",\n"
+		"\"0000010010011100010001110001001101000010000000\",\n"
+		"\"0000010010100010010010001001001100100010000000\",\n"
+		"\"0000011100100010011110001001000100111011100000\",\n"
+		"};",
+};
+
+static struct img_xpm_test xpm_test_4 = {
+	.img = img4,
+	.len = sizeof(img4),
+	.scheme = STK_IMG_SCHEME_BASIC,
+	.xpm = "/* XPM */\n"
+		"static char *xpm[] = {\n"
+		"\"24 16 2 1\",\n"
+		"\"0	c #000000\",\n"
+		"\"1	c #FFFFFF\",\n"
+		"\"111111111111111111111111\",\n"
+		"\"100000000000000000000001\",\n"
+		"\"100000000000000000000001\",\n"
+		"\"100000000000000000000001\",\n"
+		"\"100011110011110011110001\",\n"
+		"\"100010010010000010000001\",\n"
+		"\"100010010010000010000001\",\n"
+		"\"100010010010000011110001\",\n"
+		"\"100010010010000000010001\",\n"
+		"\"100010010010000000010001\",\n"
+		"\"100010010010000000010001\",\n"
+		"\"100011110011110011110001\",\n"
+		"\"100000000000000000000001\",\n"
+		"\"100000000000000000000001\",\n"
+		"\"100000000000000000000001\",\n"
+		"\"111111111111111111111111\",\n"
+		"};",
+};
+
+static struct img_xpm_test xpm_test_5 = {
+	.img = img5,
+	.len = sizeof(img5),
+	.scheme = STK_IMG_SCHEME_BASIC,
+	.xpm = "/* XPM */\n"
+		"static char *xpm[] = {\n"
+		"\"8 8 2 1\",\n"
+		"\"0	c #000000\",\n"
+		"\"1	c #FFFFFF\",\n"
+		"\"11111111\",\n"
+		"\"00000011\",\n"
+		"\"10100101\",\n"
+		"\"10011001\",\n"
+		"\"10011001\",\n"
+		"\"10100101\",\n"
+		"\"11000011\",\n"
+		"\"11111111\",\n"
+		"};",
+};
+
+static struct img_xpm_test xpm_test_6 = {
+	.img = img2,
+	.len = sizeof(img2),
+	.scheme = STK_IMG_SCHEME_TRANSPARENCY,
+	.xpm = "/* XPM */\n"
+		"static char *xpm[] = {\n"
+		"\"8 8 3 1\",\n"
+		"\"0	c #FF0000\",\n"
+		"\"1	c #00FF00\",\n"
+		"\"2	c None\",\n"
+		"\"22222222\",\n"
+		"\"20000002\",\n"
+		"\"20111002\",\n"
+		"\"20011002\",\n"
+		"\"20011002\",\n"
+		"\"20011102\",\n"
+		"\"20000002\",\n"
+		"\"22222222\",\n"
+		"};",
+};
+
+static void test_img_to_xpm(gconstpointer data)
+{
+	const struct img_xpm_test *test = data;
+	char *xpm;
+
+	xpm = stk_image_to_xpm(test->img, test->len, test->scheme);
+
+	g_assert(memcmp(xpm, test->xpm, strlen(test->xpm)) == 0);
+	g_free(xpm);
+}
+
 int main(int argc, char **argv)
 {
 	g_test_init(&argc, &argv, NULL);
@@ -24727,5 +24954,18 @@ int main(int argc, char **argv)
 	g_test_add_data_func("/teststk/HTML Attribute Test 4",
 				&html_attr_data_4, test_html_attr);
 
+	g_test_add_data_func("/teststk/IMG to XPM Test 1",
+				&xpm_test_1, test_img_to_xpm);
+	g_test_add_data_func("/teststk/IMG to XPM Test 2",
+				&xpm_test_2, test_img_to_xpm);
+	g_test_add_data_func("/teststk/IMG to XPM Test 3",
+				&xpm_test_3, test_img_to_xpm);
+	g_test_add_data_func("/teststk/IMG to XPM Test 4",
+				&xpm_test_4, test_img_to_xpm);
+	g_test_add_data_func("/teststk/IMG to XPM Test 5",
+				&xpm_test_5, test_img_to_xpm);
+	g_test_add_data_func("/teststk/IMG to XPM Test 6",
+				&xpm_test_6, test_img_to_xpm);
+
 	return g_test_run();
 }
-- 
1.6.6.1

[PATCH 0/2] convert images to xpm

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 521 bytes --]

Changes since last version:
* whitespace changes
* corrected sanity tests
* changed signature to pass clut and clut_len to function
* updated unit tests for new signature

Kristen Carlson Accardi (2):
  stkutil: convert img to xpm
  test-stkutil: unit test for img to xpm converter

 src/stkutil.c       |  147 ++++++++++++++++++++++++++++++
 src/stkutil.h       |    9 ++
 unit/test-stkutil.c |  247 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 403 insertions(+), 0 deletions(-)

[PATCH 1/2] stkutil: convert img to xpm

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 5223 bytes --]

---
 src/stkutil.c |  147 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/stkutil.h |    9 ++++
 2 files changed, 156 insertions(+), 0 deletions(-)

diff --git a/src/stkutil.c b/src/stkutil.c
index 9cac850..ae4cc32 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -6076,3 +6076,150 @@ char *stk_text_to_html(const char *utf8,
 	/* return characters from string. Caller must free char data */
 	return g_string_free(string, FALSE);
 }
+
+static const char chars_table[] = {
+	'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C',
+	'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P',
+	'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', 'a', 'b', 'c',
+	'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p',
+	'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '+', '.' };
+
+char *stk_image_to_xpm(const unsigned char *img, unsigned int len,
+			enum stk_img_scheme scheme, const unsigned char *clut,
+			unsigned short clut_len)
+{
+	guint8 width, height;
+	unsigned int ncolors, nbits, entry, cpp;
+	unsigned int i, j;
+	int bit, k;
+	GString *xpm;
+	unsigned int pos = 0;
+	const char xpm_header[] = "/* XPM */\n";
+	const char declaration[] = "static char *xpm[] = {\n";
+	char c[3];
+
+	if (img == NULL)
+		return NULL;
+
+	/* sanity check length */
+	if (len < 3)
+		return NULL;
+
+	width = img[pos++];
+	height = img[pos++];
+
+	if (scheme == STK_IMG_SCHEME_BASIC) {
+		nbits = 1;
+		ncolors = 2;
+	} else {
+		/* sanity check length */
+		if ((pos + 4 > len) || (clut == NULL))
+			return NULL;
+
+		nbits = img[pos++];
+		ncolors = img[pos++];
+
+		/* the value of zero should be interpreted as 256 */
+		if (ncolors == 0)
+			ncolors = 256;
+
+		/* skip clut offset bytes */
+		pos += 2;
+
+		if ((ncolors * 3) > clut_len)
+			return NULL;
+	}
+
+	if (pos + ((width * height + 7) / 8) > len)
+		return NULL;
+
+	/* determine the number of chars need to represent the pixel */
+	cpp = ncolors > 64 ? 2 : 1;
+
+	/*
+	 * space needed:
+	 * 	header line
+	 *	declaration and beginning of assignment line
+	 *	values - max length of 19
+	 *	colors - ncolors * (cpp + whitespace + deliminators + color)
+	 *	pixels - width * height * cpp + height deliminators "",\n
+	 *	end of assignment - 2 chars "};"
+	 */
+	xpm = g_string_sized_new(strlen(xpm_header) + strlen(declaration) +
+				19 + ((cpp + 14) * ncolors) +
+				(width * height * cpp) + (4 * height) + 2);
+	if (xpm == NULL)
+		return NULL;
+
+	/* add header, declaration, values */
+	g_string_append(xpm, xpm_header);
+	g_string_append(xpm, declaration);
+	g_string_append_printf(xpm, "\"%d %d %d %d\",\n", width, height,
+				ncolors, cpp);
+
+	/* create colors */
+	if (scheme == STK_IMG_SCHEME_BASIC) {
+		g_string_append(xpm, "\"0\tc #000000\",\n");
+		g_string_append(xpm, "\"1\tc #FFFFFF\",\n");
+	} else {
+		for (i = 0; i < ncolors; i++) {
+			/* lookup char representation of this number */
+			if (ncolors > 64) {
+				c[0] = chars_table[i / 64];
+				c[1] = chars_table[i % 64];
+				c[2] = '\0';
+			} else {
+				c[0] = chars_table[i % 64];
+				c[1] = '\0';
+			}
+
+			if ((i == (ncolors - 1)) &&
+					scheme == STK_IMG_SCHEME_TRANSPARENCY)
+				g_string_append_printf(xpm,
+					"\"%s\tc None\",\n", c);
+			else
+				g_string_append_printf(xpm,
+					"\"%s\tc #%02hhX%02hhX%02hhX\",\n",
+					c, clut[0], clut[1], clut[2]);
+			clut += 3;
+		}
+	}
+
+	/* height rows of width pixels */
+	k = 7;
+	for (i = 0; i < height; i++) {
+		g_string_append(xpm, "\"");
+		for (j = 0; j < width; j++) {
+			entry = 0;
+			for (bit = nbits - 1; bit >= 0; bit--) {
+				entry |= (img[pos] >> k & 0x1) << bit;
+				k--;
+
+				/* see if we crossed a byte boundary */
+				if (k < 0) {
+					k = 7;
+					pos++;
+				}
+			}
+
+			/* lookup char representation of this number */
+			if (ncolors > 64) {
+				c[0] = chars_table[entry / 64];
+				c[1] = chars_table[entry % 64];
+				c[2] = '\0';
+			} else {
+				c[0] = chars_table[entry % 64];
+				c[1] = '\0';
+			}
+
+			g_string_append_printf(xpm, "%s", c);
+		}
+
+		g_string_append(xpm, "\",\n");
+	}
+
+	g_string_append(xpm, "};");
+
+	/* Caller must free char data */
+	return g_string_free(xpm, FALSE);
+}
diff --git a/src/stkutil.h b/src/stkutil.h
index 1fbd68b..ea9294c 100644
--- a/src/stkutil.h
+++ b/src/stkutil.h
@@ -557,6 +557,12 @@ enum stk_me_status {
 	STK_ME_STATUS_NOT_IDLE = 	0x01
 };
 
+enum stk_img_scheme {
+	STK_IMG_SCHEME_BASIC =		0x11,
+	STK_IMG_SCHEME_COLOR =		0x21,
+	STK_IMG_SCHEME_TRANSPARENCY =	0x22,
+};
+
 /* For data object that only has a byte array with undetermined length */
 struct stk_common_byte_array {
 	unsigned char *array;
@@ -1644,3 +1650,6 @@ const unsigned char *stk_pdu_from_envelope(const struct stk_envelope *envelope,
 						unsigned int *out_length);
 char *stk_text_to_html(const char *text,
 				const unsigned short *attrs, int num_attrs);
+char *stk_image_to_xpm(const unsigned char *img, unsigned int len,
+			enum stk_img_scheme scheme, const unsigned char *clut,
+			unsigned short clut_len);
-- 
1.6.6.1

Re: [PATCH 1/2] stkutil: convert img to xpm

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]

Hi Kristen,

On 07/26/2010 01:27 PM, Kristen Carlson Accardi wrote:
> ---
>  src/stkutil.c |  147 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/stkutil.h |    9 ++++
>  2 files changed, 156 insertions(+), 0 deletions(-)
> 

Both patches in this series have been applied, thanks.

Regards,
-Denis

[PATCH 0/2] convert img to xpm

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 340 bytes --]



Kristen Carlson Accardi (2):
  stkutil: convert img to xpm
  test-stkutil: unit test for img to xpm converter

 src/stkutil.c       |  108 ++++++++++++++++++++++++
 src/stkutil.h       |    1 +
 unit/test-stkutil.c |  233 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 342 insertions(+), 0 deletions(-)

[PATCH 1/2] stkutil: convert img to xpm

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 3766 bytes --]

---
 src/stkutil.c |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/stkutil.h |    1 +
 2 files changed, 109 insertions(+), 0 deletions(-)

diff --git a/src/stkutil.c b/src/stkutil.c
index 9cac850..dfac661 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -26,6 +26,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <stdint.h>
+#include <stdio.h>
 
 #include <glib.h>
 
@@ -6076,3 +6077,110 @@ char *stk_text_to_html(const char *utf8,
 	/* return characters from string. Caller must free char data */
 	return g_string_free(string, FALSE);
 }
+
+char *stk_image_to_xpm(const unsigned char *img, guint8 scheme)
+{
+	guint8 width, height;
+	int ncolors, nbits, bit, entry, cpp;
+	int i, j, k;
+	short clut_offset = 0;
+	guint8 *clut;
+	GString *xpm;
+	int pos = 0;
+	const char xpm_header[] = "/* XPM */\n";
+	const char declaration[] = "static char *xpm[] = {\n";
+	char temp[17];
+
+	if (img == NULL)
+		return NULL;
+
+	width = img[pos++];
+	height = img[pos++];
+
+	if (scheme == 0x11) {
+		nbits = 1;
+		ncolors = 2;
+	} else {
+		nbits = img[pos++];
+		ncolors = img[pos++];
+
+		/* the value of zero should be interpreted as 256 */
+		if (ncolors == 0)
+			ncolors = 256;
+
+		clut_offset = img[pos++] << 8;
+		clut_offset |= img[pos++];
+	}
+
+	/* determine the number of chars need to represent the pixel */
+	sprintf(temp, "%d", ncolors);
+	cpp = strlen(temp);
+
+	/* create values line */
+	sprintf(temp, "\"%d %d %d %d\",\n", width, height, ncolors, cpp);
+
+	/*
+	 * space needed:
+	 * 	header line
+	 *	declaration and beginning of assignment line
+	 *	values - strlen(values)
+	 *	colors - ncolors * (cpp + whitespace + deliminators + color)
+	 *	pixels - width * height * cpp + height deliminators "",\n
+	 *	end of assignment - 2 chars "};"
+	 */
+	xpm = g_string_sized_new(strlen(xpm_header) + strlen(declaration) +
+				strlen(temp) + ((cpp + 14) * ncolors) +
+				(width * height * cpp) + (4 * height) + 2);
+	if (xpm == NULL)
+		return NULL;
+
+	/* add header, declaration, values */
+	g_string_append(xpm, xpm_header);
+	g_string_append(xpm, declaration);
+	g_string_append(xpm, temp);
+
+	/* create colors */
+	if (scheme == 0x11) {
+		g_string_append(xpm, "\"0\tc #000000\",\n");
+		g_string_append(xpm, "\"1\tc #FFFFFF\",\n");
+	} else {
+		clut = (guint8 *) &img[clut_offset];
+
+		for (i = 0; i < ncolors; i++) {
+			if ((i == (ncolors - 1)) && (scheme == 0x22))
+				g_string_append_printf(xpm,
+					"\"%*d\tc None\",\n",
+					cpp, i);
+			else
+				g_string_append_printf(xpm,
+					"\"%*d\tc #%02hhX%02hhX%02hhX\",\n",
+					cpp, i, clut[0], clut[1], clut[2]);
+			clut += 3;
+		}
+	}
+
+	/* height rows of width pixels */
+	k = 7;
+	for (i = 0; i < height; i++) {
+		g_string_append(xpm, "\"");
+		for (j = 0; j < width; j++) {
+			entry = 0;
+			for (bit = nbits - 1; bit >= 0; bit--) {
+				entry |= (img[pos] >> k & 0x1) << bit;
+				k--;
+
+				/* see if we crossed a byte boundary */
+				if (k < 0) {
+					k = 7;
+					pos++;
+				}
+			}
+			g_string_append_printf(xpm, "%*d", cpp, entry);
+		}
+		g_string_append(xpm, "\",\n");
+	}
+	g_string_append(xpm, "};");
+
+	/* Caller must free char data */
+	return g_string_free(xpm, FALSE);
+}
diff --git a/src/stkutil.h b/src/stkutil.h
index 1fbd68b..56ed255 100644
--- a/src/stkutil.h
+++ b/src/stkutil.h
@@ -1644,3 +1644,4 @@ const unsigned char *stk_pdu_from_envelope(const struct stk_envelope *envelope,
 						unsigned int *out_length);
 char *stk_text_to_html(const char *text,
 				const unsigned short *attrs, int num_attrs);
+char *stk_image_to_xpm(const unsigned char *img, guint8 scheme);
-- 
1.6.6.1

Re: [PATCH 1/2] stkutil: convert img to xpm

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 5005 bytes --]

Hi Kristen,

On 07/22/2010 12:06 AM, Kristen Carlson Accardi wrote:
> ---
>  src/stkutil.c |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/stkutil.h |    1 +
>  2 files changed, 109 insertions(+), 0 deletions(-)
> 
> diff --git a/src/stkutil.c b/src/stkutil.c
> index 9cac850..dfac661 100644
> --- a/src/stkutil.c
> +++ b/src/stkutil.c
> @@ -26,6 +26,7 @@
>  #include <string.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> +#include <stdio.h>
>  
>  #include <glib.h>
>  
> @@ -6076,3 +6077,110 @@ char *stk_text_to_html(const char *utf8,
>  	/* return characters from string. Caller must free char data */
>  	return g_string_free(string, FALSE);
>  }
> +
> +char *stk_image_to_xpm(const unsigned char *img, guint8 scheme)
> +{

I suggest making scheme into an enum, and passing in the size argument
here as well.  You have to be more paranoid that the input data is
proper and not just assume so.

> +	guint8 width, height;
> +	int ncolors, nbits, bit, entry, cpp;
> +	int i, j, k;
> +	short clut_offset = 0;
> +	guint8 *clut;
> +	GString *xpm;
> +	int pos = 0;
> +	const char xpm_header[] = "/* XPM */\n";
> +	const char declaration[] = "static char *xpm[] = {\n";
> +	char temp[17];
> +
> +	if (img == NULL)
> +		return NULL;
> +
> +	width = img[pos++];
> +	height = img[pos++];

You want to make sure no buffer overruns can occur here

> +
> +	if (scheme == 0x11) {
> +		nbits = 1;
> +		ncolors = 2;
> +	} else {
> +		nbits = img[pos++];
> +		ncolors = img[pos++];
> +
> +		/* the value of zero should be interpreted as 256 */
> +		if (ncolors == 0)
> +			ncolors = 256;
> +
> +		clut_offset = img[pos++] << 8;
> +		clut_offset |= img[pos++];

Or here

> +	}
> +
> +	/* determine the number of chars need to represent the pixel */
> +	sprintf(temp, "%d", ncolors);
> +	cpp = strlen(temp);
> +

Erm, is ncolors > 100 ? 3 : ncolors > 10 ? 2 : 1 not good enough for you
for some reason?

Also, XPM is pretty flexible here, there's no need to use 0-9 digits
only.  For instance, you can do two look up tables, one for images with
LUTs up to 64 entries and another for LUTs with more.  E.g.

A-Za-z0-9@$ and 'aa ab ac .. pp' or something like that.

This will keep the image sizes way down.

> +	/* create values line */
> +	sprintf(temp, "\"%d %d %d %d\",\n", width, height, ncolors, cpp);
> +
> +	/*
> +	 * space needed:
> +	 * 	header line
> +	 *	declaration and beginning of assignment line
> +	 *	values - strlen(values)
> +	 *	colors - ncolors * (cpp + whitespace + deliminators + color)
> +	 *	pixels - width * height * cpp + height deliminators "",\n
> +	 *	end of assignment - 2 chars "};"
> +	 */
> +	xpm = g_string_sized_new(strlen(xpm_header) + strlen(declaration) +
> +				strlen(temp) + ((cpp + 14) * ncolors) +
> +				(width * height * cpp) + (4 * height) + 2);
> +	if (xpm == NULL)
> +		return NULL;
> +
> +	/* add header, declaration, values */
> +	g_string_append(xpm, xpm_header);
> +	g_string_append(xpm, declaration);
> +	g_string_append(xpm, temp);

Seems to me you can get rid of the temp variable and
string_append_printf the contents of temp here directly (while updating
the size in the g_string_sized_new call).  In which case you can get rid
of the stdio include above.

> +
> +	/* create colors */
> +	if (scheme == 0x11) {
> +		g_string_append(xpm, "\"0\tc #000000\",\n");
> +		g_string_append(xpm, "\"1\tc #FFFFFF\",\n");
> +	} else {
> +		clut = (guint8 *) &img[clut_offset];
> +
> +		for (i = 0; i < ncolors; i++) {
> +			if ((i == (ncolors - 1)) && (scheme == 0x22))
> +				g_string_append_printf(xpm,
> +					"\"%*d\tc None\",\n",
> +					cpp, i);
> +			else
> +				g_string_append_printf(xpm,
> +					"\"%*d\tc #%02hhX%02hhX%02hhX\",\n",
> +					cpp, i, clut[0], clut[1], clut[2]);
> +			clut += 3;
> +		}
> +	}
> +
> +	/* height rows of width pixels */
> +	k = 7;
> +	for (i = 0; i < height; i++) {
> +		g_string_append(xpm, "\"");
> +		for (j = 0; j < width; j++) {
> +			entry = 0;
> +			for (bit = nbits - 1; bit >= 0; bit--) {
> +				entry |= (img[pos] >> k & 0x1) << bit;
> +				k--;
> +
> +				/* see if we crossed a byte boundary */
> +				if (k < 0) {
> +					k = 7;
> +					pos++;
> +				}
> +			}
> +			g_string_append_printf(xpm, "%*d", cpp, entry);
> +		}
> +		g_string_append(xpm, "\",\n");
> +	}
> +	g_string_append(xpm, "};");
> +
> +	/* Caller must free char data */
> +	return g_string_free(xpm, FALSE);
> +}
> diff --git a/src/stkutil.h b/src/stkutil.h
> index 1fbd68b..56ed255 100644
> --- a/src/stkutil.h
> +++ b/src/stkutil.h
> @@ -1644,3 +1644,4 @@ const unsigned char *stk_pdu_from_envelope(const struct stk_envelope *envelope,
>  						unsigned int *out_length);
>  char *stk_text_to_html(const char *text,
>  				const unsigned short *attrs, int num_attrs);
> +char *stk_image_to_xpm(const unsigned char *img, guint8 scheme);

Regards,
-Denis

Re: [PATCH 1/2] stkutil: convert img to xpm

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 522 bytes --]

On Thu, 22 Jul 2010 11:12:40 -0500
Denis Kenzior <denkenz@gmail.com> wrote:

> Also, XPM is pretty flexible here, there's no need to use 0-9 digits
> only.  For instance, you can do two look up tables, one for images with
> LUTs up to 64 entries and another for LUTs with more.  E.g.
> 
> A-Za-z0-9@$ and 'aa ab ac .. pp' or something like that.

I realize this - this was an implementation choice to keep the code
less complex, thinking that it would be better to favor that vs.
saving some bytes on the xpm.

Re: [PATCH 1/2] stkutil: convert img to xpm

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

Hi Kristen,

On 07/22/2010 02:53 PM, Kristen Carlson Accardi wrote:
> On Thu, 22 Jul 2010 11:12:40 -0500
> Denis Kenzior <denkenz@gmail.com> wrote:
> 
>> Also, XPM is pretty flexible here, there's no need to use 0-9 digits
>> only.  For instance, you can do two look up tables, one for images with
>> LUTs up to 64 entries and another for LUTs with more.  E.g.
>>
>> A-Za-z0-9@$ and 'aa ab ac .. pp' or something like that.
> 
> I realize this - this was an implementation choice to keep the code
> less complex, thinking that it would be better to favor that vs.
> saving some bytes on the xpm.

The wording on my part was poor.  Of course you knew this.  My intent
was to encourage the optimization of the xpm format.  Remember these
images are up to 64k in size, so a difference between transferring /
using up 128k and 192k respectively is quite significant.  The added
(minor) code complexity is definitely worth it.

Regards,
-Denis

Re: [PATCH 1/2] stkutil: convert img to xpm

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]

Hi Kristen,

> >> Also, XPM is pretty flexible here, there's no need to use 0-9 digits
> >> only.  For instance, you can do two look up tables, one for images with
> >> LUTs up to 64 entries and another for LUTs with more.  E.g.
> >>
> >> A-Za-z0-9@$ and 'aa ab ac .. pp' or something like that.
> > 
> > I realize this - this was an implementation choice to keep the code
> > less complex, thinking that it would be better to favor that vs.
> > saving some bytes on the xpm.
> 
> The wording on my part was poor.  Of course you knew this.  My intent
> was to encourage the optimization of the xpm format.  Remember these
> images are up to 64k in size, so a difference between transferring /
> using up 128k and 192k respectively is quite significant.  The added
> (minor) code complexity is definitely worth it.

we need such optimization in this case. We are already accepting a
penalty hit by having to transfer the images over D-Bus. So every single
byte we can save here is a win.

Regards

Marcel