fix errors with Aperio Versa SCN file #394
Conversation
Signed-off-by: Wei Chen <chenw1@uthscsa.edu>
Signed-off-by: Wei Chen <chenw1@uthscsa.edu>
Signed-off-by: Wei Chen <chenw1@uthscsa.edu>
With
g_autoptr(xmlDoc) doc = _openslide_xml_parse(xml, err);
there is a memory double free. Looks like it happens when call
xmlFreeNodeList to free doc->children. For the mement use the old form
"xmlDoc *doc".
Signed-off-by: Wei Chen <chenw1@uthscsa.edu>
DCO signed off ✔️All commits have been signed off. You have certified to the terms of the Developer Certificate of Origin, version 1.1. In particular, you certify that this contribution has not been developed using information obtained under a non-disclosure agreement or other license terms that forbid you from contributing it under the GNU Lesser General Public License, version 2.1. |
There was a problem hiding this comment.
Thanks for the PR! I've made some initial comments but haven't done a full review yet. To do that, I'll need a sample of an affected slide.
Please submit a sample file using this form. The form allows you to submit the sample file privately to the OpenSlide maintainers, or to give us permission to redistribute the sample file in openslide-testdata. If you can, please give us permission to redistribute the sample so we can add it to our automated tests.
| @@ -483,6 +483,20 @@ static struct collection *parse_xml_description(const char *xml, | |||
| image->nm_across == collection->nm_across && | |||
| image->nm_down == collection->nm_down); | |||
|
|
|||
| float objective; | |||
| if (strcmp(image->device_model, "Versa") == 0) { | |||
There was a problem hiding this comment.
Rather than making assumptions about the behavior of particular scanner models, we should detect the characteristics of the slide file that require us to behave differently, and adjust our behavior based on that.
There was a problem hiding this comment.
Sorry, I do not have the right to provide Versa slide files.
src/openslide-vendor-leica.c
Outdated
| In an Aperio Versa SCN file taken at UTHSCSA, the macro image | ||
| has different view sizeY from collection's sizeY, the view offsetY | ||
| is also not zero. | ||
|
|
||
| Our application uses image taken by 20x objective, objective < 2x | ||
| is a macro image. |
There was a problem hiding this comment.
Please put this material in the commit message rather than a comment.
src/openslide-vendor-leica.c
Outdated
| _openslide_xml_xpath_get_node(ctx, | ||
| "/d:scn/d:collection/d:supplementalImage"); | ||
| if (sup_image_node) { | ||
| g_autofree xmlChar *s = xmlGetProp(sup_image_node, BAD_CAST "type"); |
There was a problem hiding this comment.
g_autofree frees with the glib allocator, not the libxml one. You want g_autoptr(xmlChar).
src/openslide-vendor-leica.c
Outdated
| @@ -536,6 +543,18 @@ static struct collection *parse_xml_description(const char *xml, | |||
| g_ptr_array_sort(image->dimensions, dimension_compare); | |||
| } | |||
|
|
|||
| // find label ifd | |||
| g_autofree xmlNode *sup_image_node = | |||
There was a problem hiding this comment.
This is a borrowed reference so shouldn't be freed at all.
src/openslide-vendor-leica.c
Outdated
| x0 = MIN(x0, area->offset_x); | ||
| y0 = MIN(y0, area->offset_y); | ||
| x1 = MAX(x1, area->offset_x + area->tiffl.image_w); | ||
| y1 = MAX(y1, area->offset_y + area->tiffl.image_h); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Please don't add stray blank lines.
src/openslide-vendor-leica.c
Outdated
| @@ -501,7 +503,7 @@ static struct collection *parse_xml_description(const char *xml, | |||
| is a macro image. | |||
| */ | |||
| objective = atof(image->objective); | |||
| image->is_macro = objective < 2 ? 1 : 0; | |||
| image->is_macro = objective < 2; | |||
There was a problem hiding this comment.
Let's move this into the original commit.
- free the sup_image_node pointer causes xmlDoc double free in parse_xml_description(). - free dimension pointer in match_main_image_dimensions() causes g_ptr_array_new() allocates the same pointer in consecutive calls. Signed-off-by: Wei Chen <chenw1@uthscsa.edu>
In the ImageDescription tag embeded in SCN file, all image XML nodes look similar. Without SCN specification, one can only guess the image taken with lowest power objective is the macro image. So far this assumption agrees with the Aperio ImageScope viewer. It may also be image of the first XML node in ImageDescription tag, but there is no way to confirm. Signed-off-by: Wei Chen <chenw1@uthscsa.edu>
|
I made few changes mainly fixed segmentation fault bug. Also removed stray blank lines. I will try to get sample slide but don't count on it. I most likely won't use that Aperio Versa again. |
No description provided.