libvips warnings from VipsJpeg not emitted in govips #219
Comments
|
This may seem related to #147, but the gist here is that we actually want to capture these warnings (although any other method of detecting 'incomplete' or 'corrupt' images would suffice as well) |
|
Hey @mrngm - thank you very much for the bug report and digging you did. The govips interface captures glib's native logging stuff, because there's a hook where govips can insert itself (so each logging message actually calls a govips function). You're right, WARNMS seems to be a libjpeg macro. I have no idea whether it's catchable and/or how host software could intercept the warnings. Some of them seem to imply data corruption, which should surely be caught by a host. Does the vips you run from command line abort or give an exit code, or simply display the error message and continue successfully? If the latter, I would look at filing a bug with libvips. |
|
Perhaps you could dig deeper into libvips internals to see how the error is being caught there that libvips surfaces? Apparently it's not being properly logged by the glib logging facilities libvips otherwise uses? Sounds like an upstream issue. |
|
Hi @tonimelisma, thanks for replying. To clarify with regards to the output of the I'll have a closer look at the |
|
I've recompiled libvips 8.10.5 with debugging information. A gdb backtrace gives somewhat more information, now. |
|
The warnings are emitted in It seems to me that @tonimelisma thoughts on the above? (edit: added code reference for |
|
Sounds like that's going pretty deep into the internals of libjpeg, but it also sounds like the ball is in our court at |
|
I've made a start by looking how diff --git a/vips/image.c b/vips/image.c
index 792b591..84bddf4 100644
--- a/vips/image.c
+++ b/vips/image.c
@@ -4,5 +4,5 @@ int has_alpha_channel(VipsImage *image) { return vips_image_hasalpha(image); }
void clear_image(VipsImage **image) {
// https://developer.gnome.org/gobject/stable/gobject-The-Base-Object-Type.html#g-clear-object
- if (G_IS_OBJECT(*image)) g_clear_object(image);
+ if (G_IS_OBJECT(*image)) g_object_unref(*image);
}Test program: package main
import (
"fmt"
"log"
"os"
"time"
"github.com/davidbyttow/govips/vips" // in go.mod: replace github.com/davidbyttow/govips/vips => /home/path/to/patched/govips
)
func main() {
// Note: setting these _after_ vips.Startup(nil) does not make a difference
vips.LoggingSettings(func(msgDomain string, level vips.LogLevel, msg string) {
fmt.Printf("%v: [%v] %q: %q\n", time.Now(), level, msgDomain, msg)
}, vips.LogLevelDebug)
vips.Startup(nil)
defer vips.Shutdown()
images := []string{"7.jpg"}
for _, v := range images {
image, err := os.ReadFile(v)
if err != nil {
log.Fatalf("error loading file %s: %v", v, err)
}
switch vips.DetermineImageType(image) {
case vips.ImageTypeJPEG:
img, err := vips.NewImageFromBuffer(image)
if err != nil {
log.Fatalf("error loading file: %v", err)
}
_, err = img.Copy()
if err != nil {
log.Fatalf("copy fail: %v", err)
}
_, _, err = img.ExportJpeg(nil) // line 37
if err != nil {
log.Fatalf("export fail: %v", err)
}
img.Close()
}
}
}Output: |
|
Apart from the stack trace (which is added in
@davidbyttow I could put this in a PR, but what would you require regarding tests and test files? There is this CC0 corrupted JPEG that I could include, and test that, on |
|
@mrngm yeah I think that would be sufficient in terms of testing, it's difficult to test it otherwise. Can you submit a PR? |
I seem to have missed your reply at the time. I'll submit a PR! |
mrngm
pushed a commit
to mrngm/govips
that referenced
this issue
Jul 28, 2023
… libvips documentation) (davidbyttow#219), add test case for checking return errors of a known corrupted JPEG
davidbyttow
pushed a commit
that referenced
this issue
Oct 22, 2023
… libvips documentation) (#219), add test case for checking return errors of a known corrupted JPEG (#374) Co-authored-by: Gerdriaan Mulder <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
On an Ubuntu 20.04 machine, we run govips @ 2.7.0, using libvips from the PPA mentioned in the readme:
For a known corrupt image,
vipsitself emits warnings (see also this issue inlibvips):However, when using this Go snippet, these warnings never seem to reach the Go layer:
Output:
From a bit of digging into
libvips@ 8.10.5, the warning is emitted here, through someWARNMSmacro (probably from some JPEG library like libjpeg-turbo).I've also tried using govips @ 4e728fa, using
LoadImageFromBufferand supplyingImportParams.FailErrorOnError(true), but this does not show thevipswarnings.Are we missing something on the Go side, is this something in
libvipsthat's not interceptable, or is something missing ingovipsC bindings?The text was updated successfully, but these errors were encountered: