JPEG: Refactor error correction for broken files #2463 #2721 #3363

Signed-off-by: Michael Mayer <michael@photoprism.app>

internal/photoprism/mediafile_thumbs.go

@@ -101,19 +101,23 @@ func (m *MediaFile) CreateThumbnails(thumbPath string, force bool) (err error) {
 		} else if force || !fs.FileExists(fileName) {
 			// Open original if needed.
 			if original == nil {
-				img, err := thumb.Open(m.FileName(), m.Orientation())
+				img, imgErr := thumb.Open(m.FileName(), m.Orientation())
 
 				// Try to fix broken JPEGs if possible, fail otherwise.
-				if err != nil {
-					if !strings.HasPrefix(err.Error(), "invalid JPEG format") {
-						log.Debugf("media: %s in %s", err.Error(), clean.Log(m.RootRelName()))
-						return err
+				if imgErr != nil {
+					msg := imgErr.Error()
+
+					// Fixable file error?
+					if !(strings.HasPrefix(msg, "EOF") || strings.HasPrefix(msg, "invalid JPEG")) {
+						log.Debugf("media: %s in %s", msg, clean.Log(m.RootRelName()))
+						return imgErr
 					}
 
-					if fixed, err := NewConvert(conf).FixJpeg(m, false); err != nil {
-						return err
-					} else if fixedImg, err := thumb.Open(fixed.FileName(), m.Orientation()); err != nil {
-						return err
+					// Try to fix image.
+					if fixed, fixErr := NewConvert(conf).FixJpeg(m, false); fixErr != nil {
+						return fixErr
+					} else if fixedImg, openErr := thumb.Open(fixed.FileName(), m.Orientation()); openErr != nil {
+						return openErr
 					} else {
 						img = fixedImg
 					}

internal/thumb/open_jpeg.go

@@ -1,28 +1,21 @@
 package thumb
 
 import (
-	"bytes"
 	"fmt"
 	"image"
 	"io"
 	"os"
 	"path/filepath"
-	"syscall"
 
 	"github.com/disintegration/imaging"
 	"github.com/mandykoh/prism/meta"
 	"github.com/mandykoh/prism/meta/autometa"
-	"golang.org/x/sys/unix"
-
 	"github.com/photoprism/photoprism/pkg/clean"
 	"github.com/photoprism/photoprism/pkg/colors"
 )
 
-var (
-	EOI = []byte{0xff, 0xd9}
-)
-
-func decode(reader io.Reader, logName string) (md *meta.Data, img image.Image, err error) {
+// decodeImage opens an image and decodes its color metadata.
+func decodeImage(reader io.Reader, logName string) (md *meta.Data, img image.Image, err error) {
 	// Read color metadata.
 	md, imgStream, err := autometa.Load(reader)
 
@@ -32,26 +25,8 @@ func decode(reader io.Reader, logName string) (md *meta.Data, img image.Image, e
 	} else {
 		img, err = imaging.Decode(imgStream)
 	}
-	return md, img, err
-}
-
-func attemptRepair(fileReader *os.File) (io.Reader, error) {
-	fi, err := fileReader.Stat()
-	if err != nil {
-		return nil, fmt.Errorf("%s trying to stat() file", err)
-	}
-	size := int(fi.Size())
-	b, err := unix.Mmap(int(fileReader.Fd()), 0, size, syscall.PROT_READ|syscall.PROT_WRITE, syscall.MAP_PRIVATE)
-	if err != nil {
-		return nil, fmt.Errorf("%s while mmap()ing file", err)
-	}
-
-	// Check for missing EOI.
-	if !bytes.Equal(b[size-len(EOI):size], EOI) {
-		b = append(b, EOI...)
-	}
 
-	return bytes.NewReader(b), nil
+	return md, img, err
 }
 
 // OpenJpeg loads a JPEG image from disk, rotates it, and converts the color profile if necessary.
@@ -75,16 +50,12 @@ func OpenJpeg(fileName string, orientation int) (image.Image, error) {
 		return nil, fmt.Errorf("%s on seek", err)
 	}
 
-	md, img, err := decode(fileReader, logName)
+	// Decode image incl color metadata.
+	md, img, err := decodeImage(fileReader, logName)
+
+	// Ok?
 	if err != nil {
-		log.Warnf("%s during initial decoding attempt", err)
-		repaired, err := attemptRepair(fileReader)
-		if err != nil {
-			return nil, fmt.Errorf("%s while trying to recover image", err)
-		}
-		if md, img, err = decode(repaired, logName); err != nil {
-			return nil, fmt.Errorf("%s while decoding after recovery attempt", err)
-		}
+		return nil, fmt.Errorf("%s while decoding", err)
 	}
 
 	// Read ICC profile and convert colors if possible.

internal/thumb/open_jpeg_test.go

@@ -2,6 +2,8 @@ package thumb
 
 import (
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestOpenJpeg(t *testing.T) {
@@ -19,13 +21,8 @@ func TestOpenJpeg(t *testing.T) {
 	t.Run("testdata/broken.jpg", func(t *testing.T) {
 		img, err := OpenJpeg("testdata/broken.jpg", 0)
 
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		if img == nil {
-			t.Error("img must not be nil")
-		}
+		assert.Error(t, err)
+		assert.Nil(t, img)
 	})
 	t.Run("testdata/fixed.jpg", func(t *testing.T) {
 		img, err := OpenJpeg("testdata/fixed.jpg", 0)