Convert: Prefer embedded preview over untrustworthy RAW render photoprism#5673

Author: lastzero

internal/photoprism/convert_command.go

@@ -2,16 +2,18 @@ package photoprism
 
 import (
 	"os/exec"
+	"strings"
 
 	"github.com/photoprism/photoprism/pkg/media"
 )
 
 // ConvertCmd represents a command to be executed for converting a MediaFile.
 // including any options to be used for this.
 type ConvertCmd struct {
-	Cmd         *exec.Cmd
-	Orientation media.Orientation
-	VerifyImage bool
+	Cmd          *exec.Cmd
+	Orientation  media.Orientation
+	VerifyImage  bool
+	RejectStderr []string
 }
 
 // String returns the conversion command as string e.g. for logging.
@@ -41,6 +43,28 @@ func (c *ConvertCmd) WithImageVerification() *ConvertCmd {
 	return c
 }
 
+// WithStderrRejection rejects the command's output when its stderr contains any of the given
+// substrings, even on a zero exit code, so the loop tries the next converter.
+func (c *ConvertCmd) WithStderrRejection(patterns ...string) *ConvertCmd {
+	c.RejectStderr = append(c.RejectStderr, patterns...)
+	return c
+}
+
+// StderrRejected reports whether the given stderr output matches a rejection pattern.
+func (c *ConvertCmd) StderrRejected(stderr string) bool {
+	if stderr == "" {
+		return false
+	}
+
+	for _, pattern := range c.RejectStderr {
+		if pattern != "" && strings.Contains(stderr, pattern) {
+			return true
+		}
+	}
+
+	return false
+}
+
 // NewConvertCmd returns a new file converter command with default options.
 func NewConvertCmd(cmd *exec.Cmd) *ConvertCmd {
 	if cmd == nil {

internal/photoprism/convert_command_test.go

@@ -40,6 +40,31 @@ func TestNewConvertCmd(t *testing.T) {
 		assert.Same(t, result, result.WithImageVerification())
 		assert.True(t, result.VerifyImage)
 	})
+	t.Run("WithStderrRejection", func(t *testing.T) {
+		result := NewConvertCmd(
+			exec.Command("rawtherapee-cli", "-o", "file.jpg", "-c", "file.cr3"),
+		)
+		assert.Empty(t, result.RejectStderr)
+		assert.Same(t, result, result.WithStderrRejection("first error", "second error"))
+		assert.Equal(t, []string{"first error", "second error"}, result.RejectStderr)
+	})
+}
+
+func TestConvertCmd_StderrRejected(t *testing.T) {
+	cmd := NewConvertCmd(exec.Command("rawtherapee-cli", "-c", "file.cr3")).WithStderrRejection("Cannot use camera white balance")
+	t.Run("Match", func(t *testing.T) {
+		assert.True(t, cmd.StderrRejected("Processing...\nCannot use camera white balance.\n"))
+	})
+	t.Run("NoMatch", func(t *testing.T) {
+		assert.False(t, cmd.StderrRejected("Warning: sidecar file requested but not found for: file.cr3"))
+	})
+	t.Run("EmptyStderr", func(t *testing.T) {
+		assert.False(t, cmd.StderrRejected(""))
+	})
+	t.Run("NoPatterns", func(t *testing.T) {
+		plain := NewConvertCmd(exec.Command("darktable-cli", "file.cr3", "file.jpg"))
+		assert.False(t, plain.StderrRejected("Cannot use camera white balance."))
+	})
 }
 
 func TestNewConvertCmds(t *testing.T) {

internal/photoprism/convert_image.go

@@ -190,6 +190,16 @@ func (w *Convert) ToImage(f *MediaFile, force bool) (result *MediaFile, err erro
 			continue
 		}
 
+		// Reject output flagged untrustworthy by its stderr (e.g. a RawTherapee render of an
+		// unsupported sensor) so the loop falls back to the next converter.
+		if c.StderrRejected(stderr.String()) {
+			log.Debugf("convert: discarding %s from %s (untrustworthy output)", clean.Log(filepath.Base(imageName)), filepath.Base(cmd.Path))
+			if removeErr := os.Remove(imageName); removeErr != nil && !os.IsNotExist(removeErr) {
+				log.Tracef("convert: %s (%s)", removeErr, filepath.Base(cmd.Path))
+			}
+			continue
+		}
+
 		// 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.

internal/photoprism/convert_image_jpeg.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/photoprism/photoprism/internal/ffmpeg"
 	"github.com/photoprism/photoprism/internal/ffmpeg/encode"
+	"github.com/photoprism/photoprism/internal/raw"
 )
 
 // JpegConvertCmds returns the supported commands for converting a MediaFile to JPEG, sorted by priority.
@@ -21,10 +22,11 @@ func (w *Convert) JpegConvertCmds(f *MediaFile, jpegName string, xmpName string)
 	// Add suitable conversion commands depending on the file type, codec, runtime environment, and configuration.
 	fileExt := f.Extension()
 	maxSize := strconv.Itoa(w.conf.JpegSize())
+	rawEnabled := w.conf.RawEnabled()
 
 	// On a Mac, use the Apple Scriptable image processing system to convert images to JPEG,
 	// see https://ss64.com/osx/sips.html.
-	if (f.IsRaw() || f.IsHeif()) && w.conf.SipsEnabled() && w.sipsExclude.Allow(fileExt) {
+	if (f.IsRaw() && rawEnabled || f.IsHeif()) && w.conf.SipsEnabled() && w.sipsExclude.Allow(fileExt) {
 		result = append(result, NewConvertCmd(
 			// #nosec G204 -- arguments are built from validated config and file paths.
 			exec.Command(w.conf.SipsBin(), "-Z", maxSize, "-s", "format", "jpeg", "--out", jpegName, f.FileName())),
@@ -47,82 +49,43 @@ func (w *Convert) JpegConvertCmds(f *MediaFile, jpegName string, xmpName string)
 		)
 	}
 
-	// Convert RAW files to JPEG with Darktable and/or RawTherapee.
-	if f.IsRaw() && w.conf.RawEnabled() {
-		if w.conf.DarktableEnabled() && w.darktableExclude.Allow(fileExt) {
-			var args []string
-
-			// Set RAW, XMP, and JPEG filenames.
-			if xmpName != "" {
-				args = []string{f.FileName(), xmpName, jpegName}
-			} else {
-				args = []string{f.FileName(), jpegName}
-			}
-
-			// Set RAW to JPEG conversion options.
-			if w.conf.RawPresets() {
-				useMutex = true // can run one instance only with presets enabled
-				args = append(args, "--width", maxSize, "--height", maxSize, "--hq", "true", "--upscale", "false")
-			} else {
-				useMutex = false // --apply-custom-presets=false disables locking
-				args = append(args, "--apply-custom-presets", "false", "--width", maxSize, "--height", maxSize, "--hq", "true", "--upscale", "false")
+	// Convert RAW files to JPEG: Darktable/RawTherapee render when RAW conversion is enabled, while
+	// ExifTool preview extraction also runs when rendering is disabled, so existing previews stay usable.
+	if f.IsRaw() {
+		if rawEnabled {
+			if w.conf.DarktableEnabled() && w.darktableExclude.Allow(fileExt) {
+				cmd, mutex := raw.DarktableCmd(raw.DarktableOptions{
+					Bin:       w.conf.DarktableBin(),
+					RawName:   f.FileName(),
+					XmpName:   xmpName,
+					JpegName:  jpegName,
+					MaxSize:   w.conf.JpegSize(),
+					Presets:   w.conf.RawPresets(),
+					ConfigDir: conf.DarktableConfigPath(),
+					CacheDir:  conf.DarktableCachePath(),
+				})
+				useMutex = mutex
+				result = append(result, NewConvertCmd(cmd))
 			}
 
-			// Set library, config, and cache location.
-			args = append(args, "--core", "--library", ":memory:")
+			// Render the RAW with RawTherapee when Darktable is unavailable or fails. Output is
+			// rejected on an untrustworthy decode (raw.DecoderErrors) so the embedded preview wins.
+			if w.conf.RawTherapeeEnabled() && w.rawTherapeeExclude.Allow(fileExt) {
+				profile := filepath.Join(conf.AssetsPath(), "profiles", "raw.pp3")
 
-			if dir := conf.DarktableConfigPath(); dir != "" {
-				args = append(args, "--configdir", dir)
+				result = append(result, NewConvertCmd(
+					raw.TherapeeCmd(w.conf.RawTherapeeBin(), f.FileName(), jpegName, profile, int(w.conf.JpegQuality())),
+				).WithStderrRejection(raw.DecoderErrors...))
 			}
-
-			if dir := conf.DarktableCachePath(); dir != "" {
-				args = append(args, "--cachedir", dir)
-			}
-
-			result = append(result, NewConvertCmd(
-				// #nosec G204 -- arguments are built from validated config and file paths.
-				exec.Command(w.conf.DarktableBin(), args...)),
-			)
 		}
 
-		// Extract the largest embedded JPEG preview with ExifTool as a fallback before
-		// RawTherapee. The camera-rendered preview always has correct colors, unlike a
-		// generic demosaic of a sensor the RAW decoder cannot identify (e.g. very recent
-		// Canon CR3 bodies, which otherwise come out magenta). It only wins when Darktable
-		// is unavailable or fails, so supported cameras keep their full RAW rendering.
-		// JpgFromRaw is the full-resolution image and PreviewImage the smaller fallback;
-		// listed largest-first so the convert loop prefers the bigger one.
-		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())).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())).WithImageVerification(),
-			)
+		// Extract the embedded camera preview (largest first): the fallback after the RAW developers,
+		// and the only option when RAW rendering is disabled. Colors stay correct for sensors they
+		// cannot identify (recent Canon CR3 bodies otherwise come out magenta). Skipped if unusable.
+		if w.conf.ExifToolEnabled() && raw.PreviewExtAllowed(fileExt) {
+			result = append(result, NewConvertCmd(raw.ExifToolJpgFromRawCmd(w.conf.ExifToolBin(), f.FileName())).WithImageVerification())
+			result = append(result, NewConvertCmd(raw.ExifToolPreviewImageCmd(w.conf.ExifToolBin(), f.FileName())).WithImageVerification())
 		}
-
-		if w.conf.RawTherapeeEnabled() && w.rawTherapeeExclude.Allow(fileExt) {
-			jpegQuality := fmt.Sprintf("-j%d", w.conf.JpegQuality())
-			profile := filepath.Join(conf.AssetsPath(), "profiles", "raw.pp3")
-
-			args := []string{"-o", jpegName, "-p", profile, "-s", "-d", jpegQuality, "-js3", "-b8", "-c", f.FileName()}
-
-			result = append(result, NewConvertCmd(
-				// #nosec G204 -- arguments are built from validated config and file paths.
-				exec.Command(w.conf.RawTherapeeBin(), args...)),
-			)
-		}
-	}
-
-	// Use ExifTool to extract JPEG thumbnails from Digital Negative (DNG) files.
-	if f.IsDng() && w.conf.ExifToolEnabled() {
-		// 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())).WithImageVerification(),
-		)
 	}
 
 	// Use "djxl" to convert JPEG XL images as a fallback when libvips lacks native support.

internal/photoprism/convert_image_png.go

@@ -21,9 +21,15 @@ func (w *Convert) PngConvertCmds(f *MediaFile, pngName string) (result ConvertCm
 	fileExt := f.Extension()
 	maxSize := strconv.Itoa(w.conf.PngSize())
 
+	// Unlike JpegConvertCmds, this builder does not have a Darktable/RawTherapee renderer or an embedded preview
+	// fallback for converting RAW images to PNG — see internal/raw/README.md for more information.
+	if f.IsRaw() && w.conf.RawEnabled() {
+		log.Debugf("convert: PNG is not a supported target format for %s files", f.FileType().ToUpper())
+	}
+
 	// On a Mac, use the Apple Scriptable image processing system to convert images to PNG,
 	// see https://ss64.com/osx/sips.html.
-	if (f.IsRaw() || f.IsHeif()) && w.conf.SipsEnabled() && w.sipsExclude.Allow(fileExt) {
+	if f.IsHeif() && w.conf.SipsEnabled() && w.sipsExclude.Allow(fileExt) {
 		result = append(result, NewConvertCmd(
 			// #nosec G204 -- arguments are built from validated config and file paths.
 			exec.Command(w.conf.SipsBin(), "-Z", maxSize, "-s", "format", "png", "--out", pngName, f.FileName())),

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/raw"
 	"github.com/photoprism/photoprism/internal/thumb"
 	"github.com/photoprism/photoprism/pkg/fs"
 )
@@ -341,9 +342,9 @@ func TestConvert_JpegConvertCmds(t *testing.T) {
 }
 
 // TestConvert_JpegConvertCmds_RawEmbeddedPreview verifies that RAW inputs emit
-// ExifTool embedded-preview extraction commands (largest-first) ordered after
-// Darktable and before RawTherapee, so an unsupported camera falls back to the
-// camera-rendered JPEG instead of a wrong-color demosaic.
+// ExifTool embedded-preview extraction commands (largest-first) ordered after the
+// RAW developers (Darktable and RawTherapee), so an unsupported camera falls back
+// to the camera-rendered JPEG instead of a wrong-color demosaic.
 func TestConvert_JpegConvertCmds_RawEmbeddedPreview(t *testing.T) {
 	cnf := config.TestConfig()
 
@@ -367,9 +368,9 @@ func TestConvert_JpegConvertCmds_RawEmbeddedPreview(t *testing.T) {
 
 	assert.NotEmpty(t, cmds)
 
-	// Record the first occurrence of each command; DNG inputs additionally emit a
-	// trailing -PreviewImage extraction, which is not the priority position.
+	// Record the position of each command in the priority-ordered list.
 	jpgFromRaw, previewImage, rawTherapee := -1, -1, -1
+	var rawTherapeeCmd *ConvertCmd
 	for i, cmd := range cmds {
 		s := cmd.String()
 		switch {
@@ -379,6 +380,7 @@ func TestConvert_JpegConvertCmds_RawEmbeddedPreview(t *testing.T) {
 			previewImage = i
 		case rawTherapee < 0 && strings.Contains(s, filepath.Base(cnf.RawTherapeeBin())):
 			rawTherapee = i
+			rawTherapeeCmd = cmd
 		}
 	}
 
@@ -387,10 +389,56 @@ func TestConvert_JpegConvertCmds_RawEmbeddedPreview(t *testing.T) {
 	assert.Less(t, jpgFromRaw, previewImage, "JpgFromRaw must be tried before PreviewImage")
 	if cnf.RawTherapeeEnabled() {
 		assert.GreaterOrEqual(t, rawTherapee, 0, "expected a RawTherapee command")
-		assert.Less(t, previewImage, rawTherapee, "embedded preview must be tried before RawTherapee")
+		assert.Less(t, rawTherapee, jpgFromRaw, "RawTherapee must be tried before the embedded preview")
+		assert.Contains(t, rawTherapeeCmd.RejectStderr, raw.WhiteBalanceError, "RawTherapee output must be rejected on a white-balance failure")
 	}
 }
 
+// TestConvert_JpegConvertCmds_RawDisabled verifies that with RAW conversion disabled (--disable-raw)
+// PhotoPrism only extracts an existing embedded preview and never renders the RAW with Darktable or
+// RawTherapee.
+func TestConvert_JpegConvertCmds_RawDisabled(t *testing.T) {
+	cnf := config.TestConfig()
+
+	if !cnf.ExifToolEnabled() {
+		t.Skip("ExifTool must be available for the RAW embedded-preview fallback")
+	}
+
+	origRaw := cnf.Options().DisableRaw
+	cnf.Options().DisableRaw = true
+	t.Cleanup(func() {
+		cnf.Options().DisableRaw = origRaw
+	})
+
+	convert := NewConvert(cnf)
+	rawFile := filepath.Join(cnf.SamplesPath(), "canon_eos_6d.dng")
+	jpegFile := filepath.Join(cnf.SamplesPath(), "canon_eos_6d.dng.jpg")
+
+	mediaFile, err := NewMediaFile(rawFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cmds, _, err := convert.JpegConvertCmds(mediaFile, jpegFile, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assert.NotEmpty(t, cmds)
+
+	previewImage := false
+	for _, cmd := range cmds {
+		base := filepath.Base(cmd.Cmd.Path)
+		assert.NotEqual(t, "darktable-cli", base, "Darktable must not run when RAW conversion is disabled")
+		assert.NotEqual(t, "rawtherapee-cli", base, "RawTherapee must not run when RAW conversion is disabled")
+		if strings.Contains(cmd.String(), "-PreviewImage") {
+			previewImage = true
+		}
+	}
+
+	assert.True(t, previewImage, "embedded preview must still be extracted when RAW conversion is disabled")
+}
+
 // TestConvert_JpegConvertCmds_HeifFallback verifies that the documented external
 // fallback command is emitted for HEIC and AVIF inputs when libheif tooling
 // (heif-dec / heif-convert) is available — see issue #5509.
@@ -516,4 +564,16 @@ func TestConvert_PngConvertCmds(t *testing.T) {
 
 		t.Logf("commands: %#v", cmds)
 	})
+	t.Run("Raw", func(t *testing.T) {
+		// RAW is converted to JPEG, not PNG: no converter command is emitted and the call reports
+		// the format as unsupported (see internal/raw/README.md).
+		mediaFile, err := NewMediaFile(filepath.Join(cnf.SamplesPath(), "canon_eos_6d.dng"))
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		cmds, _, err := convert.PngConvertCmds(mediaFile, filepath.Join(t.TempDir(), "canon_eos_6d.png"))
+		assert.Error(t, err)
+		assert.Empty(t, cmds)
+	})
 }