Opened 8 years ago

Closed 6 years ago

#15331 closed enhancement (fixed)

Give more information in PNG error messages

Reported by: rocrail Owned by: Vadim Zeitlin <vadim@…>
Priority: low Milestone: 3.1.0
Component: 3rdparty Version: stable-latest
Keywords: png wxLogWarning simple Cc:
Blocked By: Blocking:
Patch: no

Description

Hi,

since the last git pull on wxWidget 2.9.5 I get a popup at program start after rebuild it against 2.9.5:

[quote]known incorrect sRGB profilequote
(Without any hints to file names...)

I commented out line 2214 in png.c to get rid of this message.

Is there a reason for this new warning?
Can I suppress those warnings with a build flag? (configure...)

I tried to convert all my png files with:
[code]for x in *png; do convert $x -profile sRGB_v4_ICC_preference.icc $x; donecode
but they are gotten much too big in size after this convert so I reverted them.

Change History (25)

comment:1 Changed 8 years ago by vadz

  • Component changed from wxOSX-Cocoa to 3rdparty
  • Keywords png wxLogWarning added
  • Milestone 2.9.5 deleted
  • Resolution set to wontfix
  • Status changed from new to closed
  • Summary changed from known incorrect sRGB profile to PNG "known incorrect sRGB profile" warning

I don't think we should be suppressing this warning, I assume it's there for a good reason and it would also appear anyhow if you're using the system library.

You can suppress (or redirect) all wxLog messages during PNG loading if you wish but I don't think we should do this by default.

comment:2 Changed 8 years ago by rocrail

OK, see your point.
But I have a lot of PNGs in the menus and I become 5 warnings if I open the details.
It would be of great help to identify the offending PNG file names.

comment:3 Changed 8 years ago by rocrail

  • Resolution wontfix deleted
  • Status changed from closed to reopened

comment:4 Changed 8 years ago by vadz

  • Keywords simple added
  • Priority changed from normal to low
  • Status changed from reopened to confirmed
  • Summary changed from PNG "known incorrect sRGB profile" warning to Give more information in PNG error messages
  • Type changed from defect to enhancement

It would indeed be better to include more information, e.g. we could save the file name and what are we doing with it in wxPNGInfoStruct and give messages of the form

wxLogWarning("Error from libpng while loading file \"%s\": %s", info->filename, message);

This should be pretty simple to do if anybody is motivated.

comment:5 Changed 8 years ago by rocrail

  • Priority changed from low to high
  • Type changed from enhancement to defect

In my case the PNGs are not directly loaded from the file system but provided as memory array.
The PNGs in question are all manipulated by The Gimp and I do not see any need to popup errors or warnings about some suspected format.
If I convert the PNGs to your desired specs they grow too big in size for my purpose.

The PNGs are showed as for 10 years so why should they become invalid over night!?

I will comment out this warning in the source as long there is no satisfying solution.

comment:6 Changed 8 years ago by vadz

  • Priority changed from high to low
  • Type changed from defect to enhancement

I don't know what the actual problem is, you should ask libpng developers. However our policy has always been, and remains, to modify the third party libraries as little as possible (typically just enough to make them compile on all the supported platforms) and I won't be changing libpng sources, especially because I don't really understand what the problem is.

Again, commenting out the warning just means that you'll have this problem in the future when your application runs using the system libpng under Linux. Not really a solution IMO but if it works for you, all the better.

comment:7 Changed 8 years ago by rocrail

OK, just a simple reply:
Before I updated to the latest 2.9.5 revision this error popup did not appear.
The PNGs are unchanged.

I do not see the need to popup an error for PNG images which do not offend anything and its a big disadvantage for a program build against this revision.
A better strategy is to provide a return code to enable a developer to check the problem instead of firing a popup to an end user without any meaning.

I will not change the priority again but it would be great if you consider things.
The return code could give me as developer a hint which PNG is in question.

comment:8 Changed 8 years ago by vadz

The change in behaviour is due to the change in libpng. wxWidgets has always forwarded libpng errors to wxLog, nothing has changed here. However this particular warning is new in libpng and this is why you see it now when you didn't see it before. Although if you built your app on a recent Linux distribution you would have got it nevertheless.

Also, from the comment in libpng sources it seems that PNG files provoking this warning are really problematic so I wouldn't just ignore it. But this is up to you to decide.

comment:9 follow-up: Changed 8 years ago by rocrail

Actually this happens under Mac OS X Mountain Lion and does not popup under Linux with the same wx 2.9.5 revision...

comment:10 follow-up: Changed 8 years ago by swiss_kinkajou

I encounter the same annoying problem with some of my PNG. I manage to remove profile information from my images using:

convert filename.png –strip filename.png

And I got no more warnings

comment:11 in reply to: ↑ 9 Changed 8 years ago by vadz

Replying to rocrail:

Actually this happens under Mac OS X Mountain Lion and does not popup under Linux with the same wx 2.9.5 revision...

And you know exactly why does this happen. Wait for some time for the system version under Linux to be upgraded too and it will start happening there as well.

comment:12 in reply to: ↑ 10 Changed 8 years ago by rocrail

Replying to swiss_kinkajou:

I encounter the same annoying problem with some of my PNG. I manage to remove profile information from my images using:

convert filename.png –strip filename.png

And I got no more warnings

This command did screwup my PNGs. :-(

comment:13 Changed 8 years ago by obfuscated

I can confirm that this warning has nothing to do with wx, because I see it if I use wx2.8, too. I'm running libpng 1.6.3 on a Gentoo linux.

comment:14 Changed 8 years ago by cbowen

This warning should absolutely be suppressed! I'll tell you how to create file it will not gripe about: Use Photoshop to save a .PNG file and leave the default option of ICC Profile: sRGB checked in the Save As dialog box. Now, do I think Adobe is writing files that have invalid profiles? I don't think so. Even if you think the files are actually invalid, this is not a reasonable library behavior.

I'm not that familiar with the wxWidgets source, but I don't believe the PNG code at any point pops up a dialog box. Instead, it provides a warning callback to the client program. If that is the case, then I assume this is the problematic code here:

static void
PNGLINKAGEMODE wx_PNG_warning(png_structp png_ptr, png_const_charp message)
{

wxPNGInfoStruct *info = png_ptr ? WX_PNG_INFO(png_ptr) : NULL;

if ( !info
info->verbose )

{

wxLogWarning( wxString::FromAscii(message) );

}

}

It is not reasonable for a warning about an image to pop up a warning box in a client program without programmer intervention. I could definitely see having a way to indicate that there was a warning so my program might decide how to handle it, but this solution makes an existing program stop working when an attempt is make to open an image that otherwise will display correctly. Since programs may very well load user images that the programmer has no control over, that could break a lot of code. The only solution I can see for this problem now appears to be suppressing the warnings.

I have a large number of images saved this way. Am I going to have to re-save every one of them?

comment:15 Changed 8 years ago by ericj

Temporary solution: Wrap the image loading code with a wxLogNull instance to suppress the error message.

Or call wxLog::SetLogLevel(0) to disable all warnings application-wide.

comment:16 Changed 8 years ago by robind

Or use some other wxLog target instead of the default. Then there won't be a dialog popping up (unless your target does that) and your application can deal with the warning however it chooses to. You probably shouldn't be letting other potential log messages pop up for the user anyway.

comment:17 Changed 8 years ago by vaclavslavik

wxWidgets has always forwarded libpng errors to wxLog, nothing has changed here

This is the problem. It's really not appropriate for the library to throw deeply technical, low-level warnings into the user's face like this. This -- or I'm pretty sure any other libpng-level warning is IMNSHO not a user-level warning and should be logged with wxLogDebug or wxLogTrace instead.

comment:18 Changed 8 years ago by vadz

This really depends on how important these warnings are (which is something I just don't know myself). Also, if the warnings are followed by error -- i.e. if the file can't be loaded -- then I think they can be quite valuable to the user as they may explain what exactly is wrong with the file, e.g. if it's really corrupted or if it's just in an unsupported format. But it's true that non-fatal warnings are apparently not that important... Still, I don't feel it's a good idea to disable them just because it's not convenient.

comment:19 Changed 7 years ago by eco

Our end users are complaining about this one too. I'll try to suppress it using the aforementioned techniques but I don't think this warning is appropriate to show to end users by default.

comment:20 Changed 7 years ago by vadz

We can't suppress this particular warning only (well, we can, but this is too ugly to consider). The choice is between not showing any libpng warnings or showing all of them and I still think that, on balance, the latter is better.

And while such broken PNGs indeed seem very common, it's really not a problem to fix them.

comment:21 Changed 7 years ago by vadz

  • Milestone set to 3.1.0

We need to do something about this for 3.1.0, people keep complaining about this. I still don't know what though :-(

Any suggestions not boiling down to nuking all libpng warnings from orbit?

comment:22 Changed 7 years ago by mmarsan

The importance of a warning/error message is a developer's decision. I agree with robind's comment 16 on using other wxLog target. So, nothing to do from wx POV.
The developer would use some kind of signal so the user can see the warning, if he wants to, (and refer to the developer some technical info), or even show a modal dialog in case of fatal errors.

comment:23 Changed 6 years ago by vadz

So I looked at this again. I still don't know what is the problem with these incorrect sRGB profiles exactly, but it seems pretty obvious that libpng behaviour is intentional, see e.g. this comment from png.c:

/* These profiles are known to have bad data that may cause
 * problems if they are used, therefore attempt to
 * discourage their use [...]
 */

And there is also the fact that while libpng does allow disabling these checks (as well as sRGB support entirely) during compile-time, it doesn't provide any way to disable it during run-time, so even if we changed the way our own libpng is built, it wouldn't help when using the system one under Linux.

So libpng developers clearly don't want people to ignore this warning.

OTOH we already have a way of ignoring all libpng warnings, via the verbose parameter of wxImageHandler::LoadFile(). The stupid thing is that we have no way to pass false to this method from wxImage::LoadFile() which is usually used for loading images. Adding yet another parameter to wxImage::LoadFile() which already has 3 of them doesn't seem appealing and wouldn't help anyhow with the images loaded by e.g. wxHTML.

The only way to make this work that I see is to add a global, or at least a per-handler global, flag for the verbosity of image loading methods. I.e. add a static void wxImageHandler::SetDefaultVerbosity(bool) and change bool verbose = true argument to something like int flags = 0 and then have ImageHandler_Quiet and ImageHandler_Verbose flags and use the global verbosity if none of them is specified. This would require quite some work as all the handlers will need to be updated, will break compatibility (although probably not that much code uses wxImageHandler in practice) and we'll be stuck with a global variable, so I don't really like this neither -- but I don't see what else can we do.

Frankly, I'm tempted to just leave it as is, catching wxLog warnings in the application code is not that difficult and most of invalid PNGs must have been already updated by now.

Any thoughts?

comment:24 Changed 6 years ago by vadz

Actually, after thinking more about this, there is a simple solution which allows disabling the warnings either globally or locally: add the flag to wxImage itself. I'm going to implement it.

comment:25 Changed 6 years ago by Vadim Zeitlin <vadim@…>

  • Owner set to Vadim Zeitlin <vadim@…>
  • Resolution set to fixed
  • Status changed from confirmed to closed

In a016e6b896a3d22ccd7a5257154fa80622622048/git-wxWidgets:

Allow suppressing warnings from wxImage::LoadFile()

Add wxImage::SetLoadFlags() and static SetDefaultLoadFlags() to allow
suppressing the warning messages that can be logged when loading some files,
notably PNG ones with invalid sRGB profiles which, unfortunately, seem to be
rather common and result in annoying warnings about them with libpng 1.6+.

Closes #15331.

Note: See TracTickets for help on using tickets.