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

JpegImporter: redirect libjpeg errors to Corrade::Utility::Error #53

Closed
wants to merge 1 commit into
base: master
from

Conversation

2 participants
@xqms
Copy link
Contributor

xqms commented Feb 23, 2019

I'm crawling a large dataset with a few faulty JPEGs - which I would like to ignore silently. However, libjpeg errors are currently directly printed to stderr. This PR redirects them to Corrage::Utility::Error, which can then be redirected by the user to capture any errors that might occur.

@mosra mosra added this to the 2019.0b milestone Feb 23, 2019

@mosra mosra added this to TODO in Asset management via automation Feb 23, 2019

@mosra mosra self-assigned this Feb 23, 2019

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Feb 23, 2019

Hi, thanks a lot! 👍

For some reason this was already done for the JpegImageConverter. Not sure why I didn't do the exact same thing here too.

Looks like this code path is not tested either. Will merge this later today / tomorrow together with a test case.

@xqms

This comment has been minimized.

Copy link
Contributor Author

xqms commented Feb 23, 2019

For some reason this was already done for the JpegImageConverter. Not sure why I didn't do the exact same thing here too.

Ah ok, didn't see that. That implementation even looks slightly better with an integrated one-line error message, so maybe just take that one ;-)

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Feb 27, 2019

Finally merged in ee69e5c. While adding the test cases for proper code coverage I discovered a handful of random bugs in all image importers (the PNG importer, especially) and, 20 commits later, these should be fixed as well.

@mosra mosra closed this Feb 27, 2019

Asset management automation moved this from TODO to Done Feb 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.