Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

image_buffer error in version 1.1.5 #388

Closed
slw287r opened this issue Jan 3, 2023 · 12 comments
Closed

image_buffer error in version 1.1.5 #388

slw287r opened this issue Jan 3, 2023 · 12 comments
Assignees

Comments

@slw287r
Copy link

slw287r commented Jan 3, 2023

I encountered the follow warning and failed to insert cairo picture from memory on CentOS Linux:

[WARNING]: worksheet image insertion: couldn't read image type for: image_buffer.

I switched on and off of USE_MEM_FILE and got the same error.

option(USE_MEM_FILE "Use fmemopen()/open_memstream() in place of temporary files" ON)

It worked on macOS for version 1.1.5 and CentOS Linux for version 1.1.3 (1.1.4 not tested).

My demo scripts (a.c):

#include <xlsxwriter.h>
#include <cairo/cairo.h>

#define WIDTH 1500
#define HEIGHT 300

lxw_image_options img_opt = {
	.x_scale = 0.1 / 1.5,
	.y_scale = 0.1 / 1.5,
	.object_position = LXW_OBJECT_MOVE_AND_SIZE_AFTER,
	.description = ""
};

typedef struct {
	unsigned char *data;
	size_t length;
} png_t;

static cairo_status_t png_write_callback(void *closure, const unsigned char *data,
        unsigned int length)
{
	png_t *png = (png_t *)closure;
	size_t new_length = png->length + length;
	unsigned char *new_data;
	if ((new_data = realloc(png->data, new_length)) == NULL)
		return CAIRO_STATUS_WRITE_ERROR;
	memcpy(&new_data[png->length], data, length);
	png->data = new_data;
	png->length = new_length;
	return CAIRO_STATUS_SUCCESS;
}

int main()
{
	png_t *png = calloc(1, sizeof(png_t));
	cairo_surface_t *sf = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, WIDTH, HEIGHT);
	cairo_t *cr = cairo_create(sf);
	cairo_set_antialias(cr, CAIRO_ANTIALIAS_BEST);
	cairo_set_source_rgb(cr, 0, 0, 0);
	cairo_select_font_face(cr, "Sans", CAIRO_FONT_SLANT_NORMAL, CAIRO_FONT_WEIGHT_NORMAL);
	cairo_set_font_size(cr, 200.0);
	cairo_move_to(cr, WIDTH / 10, HEIGHT / 1.25);
	cairo_show_text(cr, "FOOBAR");
	cairo_surface_write_to_png_stream(sf, png_write_callback, png);
	cairo_destroy(cr);
	cairo_surface_destroy(sf);

	lxw_workbook *wb = workbook_new("foo.xlsx");
	lxw_worksheet *ws = workbook_add_worksheet(wb, "bar");
	worksheet_insert_image_buffer_opt(ws, 0, 0, png->data, png->length, &img_opt);
	lxw_error e = workbook_close(wb);
	if (e) {
		fprintf(stderr, "Error closing workbook, %d = %s\n", e, lxw_strerror(e));
		return e;
	}
	free((char *)png->data);
	free(png);
	return 0;
}

Compile and run:

gcc -o a a.c -lcairo -lxlsxwriter
./a # create foo.xlsx

Normal foo.xlsx

@jmcnamara
Copy link
Owner

jmcnamara commented Jan 3, 2023

What is the issue? The image is in the file you attached and I compiled and confirmed that it works with libxlsxwriter version 1.1.5 on Ubuntu 22.04.

screenshot

@slw287r
Copy link
Author

slw287r commented Jan 3, 2023

I uploaded the normal xlsx from version 1.1.3 above. Please find the abnormal xlsx without the intended image generated via version 1.1.5 below.

My OS info:
Linux 3.10.0-693.5.2.el7.x86_64 #1 SMP Fri Oct 20 20:32:50 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

foo_bad.xlsx

@jmcnamara
Copy link
Owner

Could you add this this debug code to your example and run it on the system/version where is doesn't work as expected.

int main()
{
	png_t *png = calloc(1, sizeof(png_t));
	cairo_surface_t *sf = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, WIDTH, HEIGHT);
	cairo_t *cr = cairo_create(sf);
	cairo_set_antialias(cr, CAIRO_ANTIALIAS_BEST);
	cairo_set_source_rgb(cr, 0, 0, 0);
	cairo_select_font_face(cr, "Sans", CAIRO_FONT_SLANT_NORMAL, CAIRO_FONT_WEIGHT_NORMAL);
	cairo_set_font_size(cr, 200.0);
	cairo_move_to(cr, WIDTH / 10, HEIGHT / 1.25);
	cairo_show_text(cr, "FOOBAR");
	cairo_surface_write_to_png_stream(sf, png_write_callback, png);
	cairo_destroy(cr);
	cairo_surface_destroy(sf);

	// Add this to your code.
	printf("Version = %s\n", lxw_version());
	printf("Length  = %zu\n", png->length);
	printf("Data    = %c%c%c\n", png->data[1], png->data[2], png->data[3]);

	lxw_workbook *wb = workbook_new("foo.xlsx");
	lxw_worksheet *ws = workbook_add_worksheet(wb, "bar");
	worksheet_insert_image_buffer_opt(ws, 0, 0, png->data, png->length, &img_opt);
	lxw_error e = workbook_close(wb);
	if (e) {
		fprintf(stderr, "Error closing workbook, %d = %s\n", e, lxw_strerror(e));
		return e;
	}
	free((char *)png->data);
	free(png);
	return 0;
}

You should get ouput like this:

Version = 1.1.5
Length  = 13543
Data    = PNG

@slw287r
Copy link
Author

slw287r commented Jan 3, 2023

Here is what I get:

% ./a
Version = 1.1.5
Length  = 12489
Data    = PNG
[WARNING]: worksheet image insertion: couldn't read image type for: image_buffer.

and for v1.1.3, no warning and normal xlsx:

% ./a
Version = 1.1.3
Length  = 12489
Data    = PNG

@jmcnamara
Copy link
Owner

Thanks. In the 1.1.5 example are you using USE_MEM_FILE=ON?

@slw287r
Copy link
Author

slw287r commented Jan 3, 2023

  • option(USE_MEM_FILE "Use fmemopen()/open_memstream() in place of temporary files" OFF) - normal xlsx
% ./a_mem_off
Version = 1.1.5
Length  = 12489
Data    = PNG
  • option(USE_MEM_FILE "Use fmemopen()/open_memstream() in place of temporary files" ON) - abnormal xlsx
% ./a_mem_on
Version = 1.1.5
Length  = 12489
Data    = PNG
[WARNING]: worksheet image insertion: couldn't read image type for: image_buffer.

@jmcnamara
Copy link
Owner

jmcnamara commented Jan 3, 2023

@mohd-akram could you have a look at this issue. I can't reproduce it on the mac or Ubuntu systems that I have but I suspect that the issue may be the rewind() below similar to issues you fixed in the second part of your patchset:

https://github.com/jmcnamara/libxlsxwriter/blob/main/src/worksheet.c#L10396

That [WARNING] occurs when the library can't read any image markers in the data but the "PNG" marker is clearly there so I suspect the stream isn't being rewound.

@mohd-akram
Copy link
Contributor

This is a bit of a strange issue. The image buffer code wasn't really touched since it already used buffers. The CentOS version is quite old and I suspect it might be a glibc issue. Can you print what's the glibc version (ldd --version)? Also, try with version 1.1.3 and the USE_FMEMOPEN option set to on, I suspect it didn't work then either.

@mohd-akram
Copy link
Contributor

Tried this in a centos:7 podman container (glibc 2.17).

@jmcnamara Seems to be the bug mentioned here under notes. Changing w+b to wb+ removes the warning for me (I haven't tried opening the XLSX file). This will also need to be changed in worksheet_set_background_buffer.

@slw287r Try making that change and see if it works for you.

@jmcnamara
Copy link
Owner

@mohd-akram Thanks. Nice detective work.

@slw287r
Copy link
Author

slw287r commented Jan 4, 2023

wb+ works for me

Tried this in a centos:7 podman container (glibc 2.17).

@jmcnamara Seems to be the bug mentioned here under notes. Changing w+b to wb+ removes the warning for me (I haven't tried opening the XLSX file). This will also need to be changed in worksheet_set_background_buffer.

@slw287r Try making that change and see if it works for you.

jmcnamara added a commit that referenced this issue Jan 4, 2023
Fix/workaround for Centos 7 fmemopen() issue.

See #388 for details.
@jmcnamara
Copy link
Owner

@slw287r Thanks for the test/report. Fixed on main.

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants