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

ImageMagick warnings cause binarization segfaults #4

Closed
bertsky opened this issue Aug 5, 2019 · 3 comments · Fixed by #6
Closed

ImageMagick warnings cause binarization segfaults #4

bertsky opened this issue Aug 5, 2019 · 3 comments · Fixed by #6
Assignees
Labels
bug Something isn't working

Comments

@bertsky
Copy link
Collaborator

bertsky commented Aug 5, 2019

When ImageMagick outputs warnings about bad image tags, OLENA's binarization programs all segfault like so:

ocrd-olena-binarize -I OCR-D-IMG -O OCR-D-IMG-BIN -p param-binarize-sauvola-ms-split.json
terminate called after throwing an instance of 'Magick::WarningCoder'
  what():  sauvola_ms_split: ASCII value for tag "Artist" does not end in null byte. `TIFFFetchNormalTag' @ warning/tiff.c/TIFFWarnings/912
Aborted (core dumped)

This most likely needs to be fixed upstream in OLENA.

@bertsky bertsky added the bug Something isn't working label Aug 5, 2019
@bertsky
Copy link
Collaborator Author

bertsky commented Aug 15, 2019

I backtraced with the debugger. Now I have a sketch of a solution. We need to catch ImageMagick's exceptions with a patch for olena:

When scribo calls milena's mln::io::magick::load, this uses Magick::Image::read without any try/catch clause whatsoever. Thankfully, the comment already concedes that:

	// FIXME: Handle Magick++'s exceptions (see either
	// ImageMagick++'s or GraphicsMagick++'s documentation).
	Magick::Image magick_ima(filename);
	magick_ima.read(filename);

So using Magick's documentation I made the following patch:

--- olena-2.1/milena/mln/io/magick/load.hh	2014-07-01 17:31:28.911721811 +0200
+++ olena-2.1/milena/mln/io/magick/load.hh	2019-08-16 00:05:22.921562018 +0200
@@ -153,7 +153,24 @@
 	// FIXME: Handle Magick++'s exceptions (see either
 	// ImageMagick++'s or GraphicsMagick++'s documentation).
 	Magick::Image magick_ima(filename);
-	magick_ima.read(filename);
+    try {
+      magick_ima.read(filename);
+    }
+    catch( Magick::WarningCoder &warning ) {
+      // Process coder warning while loading file (e.g. TIFF warning)
+      // Maybe the user will be interested in these warnings (or not).
+      // If a warning is produced while loading an image, the image
+      // can normally still be used (but not if the warning was about
+      // something important!)
+      std::cerr << "warning: magick read: " << warning.what() << std::endl;
+    } catch( Magick::Warning &warning ) {
+      // Handle any other Magick++ warning.
+      std::cerr << "warning: magick read: " << warning.what() << std::endl;
+    } catch( Magick::ErrorFileOpen &error ) {
+      // Process Magick++ file open error
+      std::cerr << "error: magick read: " << error.what() << std::endl;
+      abort();
+    }
 	magick_ima.type(Magick::TrueColorType);
 	int nrows = magick_ima.rows();
 	int ncols = magick_ima.columns();

This works – it turns the above abort into the following warning and resumes operation normally:

warning: magick read: wolf: Incorrect value for "Photoshop"; tag ignored. `TIFFFetchNormalTag' @ warning/tiff.c/TIFFWarnings/912

Now, how do we proceed? Package our patch into our wrapper, or hope for upstream approval? @wrznr @kba

@bertsky bertsky self-assigned this Aug 15, 2019
@bertsky
Copy link
Collaborator Author

bertsky commented Aug 15, 2019

(And if the former – in #5 or as a new PR?)

@bertsky
Copy link
Collaborator Author

bertsky commented Aug 16, 2019

created #6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant