diff --git a/internal/photoprism/convert_command.go b/internal/photoprism/convert_command.go index df07523e6..e9679b2c6 100644 --- a/internal/photoprism/convert_command.go +++ b/internal/photoprism/convert_command.go @@ -11,6 +11,7 @@ import ( type ConvertCmd struct { Cmd *exec.Cmd Orientation media.Orientation + VerifyImage bool } // String returns the conversion command as string e.g. for logging. @@ -33,6 +34,13 @@ func (c *ConvertCmd) ResetOrientation() *ConvertCmd { return c.WithOrientation(media.ResetOrientation) } +// WithImageVerification marks the command's output for a decode check before acceptance, +// so corrupt embedded previews are rejected and the loop tries the next command. +func (c *ConvertCmd) WithImageVerification() *ConvertCmd { + c.VerifyImage = true + return c +} + // NewConvertCmd returns a new file converter command with default options. func NewConvertCmd(cmd *exec.Cmd) *ConvertCmd { if cmd == nil { diff --git a/internal/photoprism/convert_command_test.go b/internal/photoprism/convert_command_test.go index fa0437bd0..ca8242795 100644 --- a/internal/photoprism/convert_command_test.go +++ b/internal/photoprism/convert_command_test.go @@ -32,6 +32,14 @@ func TestNewConvertCmd(t *testing.T) { assert.Equal(t, "/usr/bin/sips -Z 123 -s format jpeg --out file.jpeg file.heic", result.String()) assert.Equal(t, media.ResetOrientation, result.Orientation) }) + t.Run("WithImageVerification", func(t *testing.T) { + result := NewConvertCmd( + exec.Command("exiftool", "-q", "-q", "-b", "-PreviewImage", "file.cr3"), + ) + assert.False(t, result.VerifyImage) + assert.Same(t, result, result.WithImageVerification()) + assert.True(t, result.VerifyImage) + }) } func TestNewConvertCmds(t *testing.T) { diff --git a/internal/photoprism/convert_image.go b/internal/photoprism/convert_image.go index 4970a5bc6..516bb422c 100644 --- a/internal/photoprism/convert_image.go +++ b/internal/photoprism/convert_image.go @@ -92,9 +92,10 @@ func (w *Convert) ToImage(f *MediaFile, force bool) (result *MediaFile, err erro start := time.Now() - // PNG, GIF, BMP, TIFF, HEIC/HEIF, AVIF, and WebP can be handled natively, as can - // JPEG XL when the libvips runtime provides JPEG XL support. - if f.IsImageOther() || (f.IsJpegXL() && thumb.JpegXLSupported() && w.conf.JpegXLEnabled()) { + // PNG, GIF, BMP, TIFF, HEIC/HEIF, AVIF, and WebP are handled natively. JPEG XL only when + // "djxl" is unavailable: some libvips builds report JPEG XL support but mis-render files + // without erroring, so the reference "djxl" decoder is preferred when present. + if f.IsImageOther() || (f.IsJpegXL() && thumb.JpegXLSupported() && w.conf.JpegXLEnabled() && w.conf.JpegXLDecoderBin() == "") { log.Infof("convert: converting %s to %s (%s)", clean.Log(filepath.Base(fileName)), clean.Log(filepath.Base(imageName)), f.FileType()) // Create PNG or JPEG image from source file. @@ -181,19 +182,30 @@ func (w *Convert) ToImage(f *MediaFile, force bool) (result *MediaFile, err erro log.Debugf("convert: %s (%s)", clean.Error(err), filepath.Base(cmd.Path)) continue } else if fs.FileExistsNotEmpty(imageName) { - log.Infof("convert: %s created in %s (%s)", clean.Log(filepath.Base(imageName)), time.Since(start), filepath.Base(cmd.Path)) - fileOrientation = c.Orientation - break + // The command wrote the target file directly (e.g. Darktable, RawTherapee). } else if res := out.Bytes(); len(res) < 512 || !mimetype.Detect(res).Is(expectedMime) { continue } else if err = os.WriteFile(imageName, res, fs.ModeFile); err != nil { log.Tracef("convert: %s (%s)", err, filepath.Base(cmd.Path)) continue - } else { - log.Infof("convert: %s created in %s (%s)", clean.Log(filepath.Base(imageName)), time.Since(start), filepath.Base(cmd.Path)) - fileOrientation = c.Orientation - break } + + // Reject undecodable output (e.g. a truncated/bogus embedded RAW preview that passed + // the MIME sniff) so the loop tries the next converter instead of indexing a file + // whose thumbnails will fail. + if c.VerifyImage { + if err = thumb.Verify(imageName); err != nil { + log.Debugf("convert: discarding undecodable %s from %s (%s)", clean.Log(filepath.Base(imageName)), filepath.Base(cmd.Path), clean.Error(err)) + if removeErr := os.Remove(imageName); removeErr != nil && !os.IsNotExist(removeErr) { + log.Tracef("convert: %s (%s)", removeErr, filepath.Base(cmd.Path)) + } + continue + } + } + + log.Infof("convert: %s created in %s (%s)", clean.Log(filepath.Base(imageName)), time.Since(start), filepath.Base(cmd.Path)) + fileOrientation = c.Orientation + break } // Ok? diff --git a/internal/photoprism/convert_image_jpeg.go b/internal/photoprism/convert_image_jpeg.go index f33be1a5c..3e63e9768 100644 --- a/internal/photoprism/convert_image_jpeg.go +++ b/internal/photoprism/convert_image_jpeg.go @@ -95,11 +95,11 @@ func (w *Convert) JpegConvertCmds(f *MediaFile, jpegName string, xmpName string) if w.conf.ExifToolEnabled() { result = append(result, NewConvertCmd( // #nosec G204 -- arguments are built from validated config and file paths. - exec.Command(w.conf.ExifToolBin(), "-q", "-q", "-b", "-JpgFromRaw", f.FileName())), + exec.Command(w.conf.ExifToolBin(), "-q", "-q", "-b", "-JpgFromRaw", f.FileName())).WithImageVerification(), ) result = append(result, NewConvertCmd( // #nosec G204 -- arguments are built from validated config and file paths. - exec.Command(w.conf.ExifToolBin(), "-q", "-q", "-b", "-PreviewImage", f.FileName())), + exec.Command(w.conf.ExifToolBin(), "-q", "-q", "-b", "-PreviewImage", f.FileName())).WithImageVerification(), ) } @@ -121,7 +121,7 @@ func (w *Convert) JpegConvertCmds(f *MediaFile, jpegName string, xmpName string) // Example: exiftool -b -PreviewImage -w IMG_4691.DNG.jpg IMG_4691.DNG result = append(result, NewConvertCmd( // #nosec G204 -- arguments are built from validated config and file paths. - exec.Command(w.conf.ExifToolBin(), "-q", "-q", "-b", "-PreviewImage", f.FileName())), + exec.Command(w.conf.ExifToolBin(), "-q", "-q", "-b", "-PreviewImage", f.FileName())).WithImageVerification(), ) } @@ -165,7 +165,7 @@ func (w *Convert) JpegConvertCmds(f *MediaFile, jpegName string, xmpName string) if (f.IsTiff() || f.IsPsd()) && w.conf.ExifToolEnabled() { result = append(result, NewConvertCmd( // #nosec G204 -- arguments are built from validated config and file paths. - exec.Command(w.conf.ExifToolBin(), "-q", "-q", "-b", "-PhotoshopThumbnail", f.FileName())), + exec.Command(w.conf.ExifToolBin(), "-q", "-q", "-b", "-PhotoshopThumbnail", f.FileName())).WithImageVerification(), ) } diff --git a/internal/photoprism/convert_image_test.go b/internal/photoprism/convert_image_test.go index 62327611d..1ace874a6 100644 --- a/internal/photoprism/convert_image_test.go +++ b/internal/photoprism/convert_image_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/photoprism/photoprism/internal/config" + "github.com/photoprism/photoprism/internal/thumb" "github.com/photoprism/photoprism/pkg/fs" ) @@ -50,6 +51,38 @@ func TestConvert_ToImage(t *testing.T) { _ = os.Remove(outputName) }) + t.Run("JpegXL", func(t *testing.T) { + if cnf.JpegXLDecoderBin() == "" { + t.Skip("djxl must be available for the JPEG XL conversion path") + } + + fileName := filepath.Join(cnf.SamplesPath(), "dice.jxl") + outputName := filepath.Join(cnf.SidecarPath(), cnf.SamplesPath(), "dice.jxl.jpg") + + _ = os.Remove(outputName) + + mf, err := NewMediaFile(fileName) + + if err != nil { + t.Fatal(err) + } + + assert.True(t, mf.IsJpegXL()) + + img, err := convert.ToImage(mf, false) + + if err != nil { + t.Fatal(err) + } + + // With djxl available the native libvips path is skipped (some builds + // mis-render JPEG XL); the djxl-produced preview must be decodable. + assert.Equal(t, outputName, img.FileName()) + assert.True(t, img.IsPreviewImage()) + assert.NoError(t, thumb.Verify(img.FileName())) + + _ = os.Remove(outputName) + }) t.Run("Raw", func(t *testing.T) { jpegFilename := filepath.Join(cnf.ImportPath(), "fern_green.jpg") diff --git a/internal/thumb/verify.go b/internal/thumb/verify.go new file mode 100644 index 000000000..344764261 --- /dev/null +++ b/internal/thumb/verify.go @@ -0,0 +1,34 @@ +package thumb + +import ( + "fmt" + + "github.com/davidbyttow/govips/v2/vips" +) + +// Verify reports an error if the image file cannot be decoded by the active rendering library. +// Used to reject corrupt converter output (e.g. a truncated embedded RAW preview that passes a +// MIME sniff but later fails libvips) so the conversion loop can try the next converter. +func Verify(fileName string) error { + if fileName == "" { + return fmt.Errorf("verify: empty filename") + } + + // Use the loader the thumbnailer will use so the check matches GenerateThumbnails. + if Library == LibVips { + VipsInit() + + img, err := vips.LoadImageFromFile(fileName, VipsImportParams()) + if err != nil { + return err + } + + img.Close() + + return nil + } + + _, err := Open(fileName, 1) + + return err +} diff --git a/internal/thumb/verify_test.go b/internal/thumb/verify_test.go new file mode 100644 index 000000000..2d0e61d0f --- /dev/null +++ b/internal/thumb/verify_test.go @@ -0,0 +1,29 @@ +package thumb + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestVerify(t *testing.T) { + t.Run("ValidJPEG", func(t *testing.T) { + assert.NoError(t, Verify("testdata/example.jpg")) + }) + t.Run("ValidPNG", func(t *testing.T) { + assert.NoError(t, Verify("testdata/example.png")) + }) + t.Run("EmptyFilename", func(t *testing.T) { + assert.Error(t, Verify("")) + }) + t.Run("NotAnImage", func(t *testing.T) { + name := filepath.Join(t.TempDir(), "garbage.jpg") + assert.NoError(t, os.WriteFile(name, []byte("this is not a valid jpeg payload, only plain text"), 0o644)) + assert.Error(t, Verify(name)) + }) + t.Run("MissingFile", func(t *testing.T) { + assert.Error(t, Verify(filepath.Join(t.TempDir(), "does-not-exist.jpg"))) + }) +}