Return album art as a Reader

core/artwork.go

@@ -10,6 +10,7 @@ import (
 	"image/jpeg"
 	_ "image/png"
 	"io"
+	"io/ioutil"
 	"os"
 	"strings"
 	"sync"
@@ -29,7 +30,7 @@ import (
 )
 
 type Artwork interface {
-	Get(ctx context.Context, id string, size int, out io.Writer) error
+	Get(ctx context.Context, id string, size int) (io.ReadCloser, error)
 }
 
 type ArtworkCache cache.FileCache
@@ -55,10 +56,10 @@ func (ci *imageInfo) Key() string {
 	return fmt.Sprintf("%s.%d.%s.%d", ci.path, ci.size, ci.lastUpdate.Format(time.RFC3339Nano), conf.Server.CoverJpegQuality)
 }
 
-func (a *artwork) Get(ctx context.Context, id string, size int, out io.Writer) error {
+func (a *artwork) Get(ctx context.Context, id string, size int) (io.ReadCloser, error) {
 	path, lastUpdate, err := a.getImagePath(ctx, id)
 	if err != nil && err != model.ErrNotFound {
-		return err
+		return nil, err
 	}
 
 	if !conf.Server.DevFastAccessCoverArt {
@@ -78,12 +79,9 @@ func (a *artwork) Get(ctx context.Context, id string, size int, out io.Writer) e
 	r, err := a.cache.Get(ctx, info)
 	if err != nil {
 		log.Error(ctx, "Error accessing image cache", "path", path, "size", size, err)
-		return err
+		return nil, err
 	}
-	defer r.Close()
+	return r, err
-
-	_, err = io.Copy(out, r)
-	return err
 }
 
 func (a *artwork) getImagePath(ctx context.Context, id string) (path string, lastUpdated time.Time, err error) {
@@ -126,7 +124,7 @@ func (a *artwork) getImagePath(ctx context.Context, id string) (path string, las
 	return a.getImagePath(ctx, "al-"+mf.AlbumID)
 }
 
-func (a *artwork) getArtwork(ctx context.Context, id string, path string, size int) (reader io.Reader, err error) {
+func (a *artwork) getArtwork(ctx context.Context, id string, path string, size int) (reader io.ReadCloser, err error) {
 	defer func() {
 		if err != nil {
 			log.Warn(ctx, "Error extracting image", "path", path, "size", size, err)
@@ -138,35 +136,28 @@ func (a *artwork) getArtwork(ctx context.Context, id string, path string, size i
 		return nil, errors.New("empty path given for artwork")
 	}
 
-	var data []byte
-
 	if size == 0 {
 		// If requested original size, just read from the file
 		if utils.IsAudioFile(path) {
-			data, err = readFromTag(path)
+			reader, err = readFromTag(path)
 		} else {
-			data, err = readFromFile(path)
+			reader, err = readFromFile(path)
 		}
 	} else {
 		// If requested a resized image, get the original (possibly from cache) and resize it
-		buf := new(bytes.Buffer)
+		var r io.ReadCloser
-		err = a.Get(ctx, id, 0, buf)
+		r, err = a.Get(ctx, id, 0)
 		if err != nil {
 			return
 		}
-		data, err = resizeImage(buf, size)
+		defer r.Close()
-	}
+		reader, err = resizeImage(r, size)
-
-	// Confirm the image is valid. Costly, but necessary
-	_, _, err = image.Decode(bytes.NewReader(data))
-	if err == nil {
-		reader = bytes.NewReader(data)
 	}
 
 	return
 }
 
-func resizeImage(reader io.Reader, size int) ([]byte, error) {
+func resizeImage(reader io.Reader, size int) (io.ReadCloser, error) {
 	img, _, err := image.Decode(reader)
 	if err != nil {
 		return nil, err
@@ -183,10 +174,10 @@ func resizeImage(reader io.Reader, size int) ([]byte, error) {
 
 	buf := new(bytes.Buffer)
 	err = jpeg.Encode(buf, m, &jpeg.Options{Quality: conf.Server.CoverJpegQuality})
-	return buf.Bytes(), err
+	return ioutil.NopCloser(buf), err
 }
 
-func readFromTag(path string) ([]byte, error) {
+func readFromTag(path string) (io.ReadCloser, error) {
 	f, err := os.Open(path)
 	if err != nil {
 		return nil, err
@@ -202,22 +193,11 @@ func readFromTag(path string) ([]byte, error) {
 	if picture == nil {
 		return nil, errors.New("file does not contain embedded art")
 	}
-	return picture.Data, nil
+	return ioutil.NopCloser(bytes.NewReader(picture.Data)), nil
 }
 
-func readFromFile(path string) ([]byte, error) {
+func readFromFile(path string) (io.ReadCloser, error) {
-	f, err := os.Open(path)
+	return os.Open(path)
-	if err != nil {
-		return nil, err
-	}
-	defer f.Close()
-
-	var buf bytes.Buffer
-	if _, err := buf.ReadFrom(f); err != nil {
-		return nil, err
-	}
-
-	return buf.Bytes(), nil
 }
 
 var (

core/artwork_test.go

@@ -1,7 +1,6 @@
 package core
 
 import (
-	"bytes"
 	"context"
 	"image"
 	"io/ioutil"
@@ -42,102 +41,100 @@ var _ = Describe("Artwork", func() {
 		})
 
 		It("retrieves the external artwork art for an album", func() {
-			buf := new(bytes.Buffer)
+			r, err := artwork.Get(ctx, "al-444", 0)
+			Expect(err).To(BeNil())
 
-			Expect(artwork.Get(ctx, "al-444", 0, buf)).To(BeNil())
+			_, format, err := image.Decode(r)
-
-			_, format, err := image.Decode(bytes.NewReader(buf.Bytes()))
 			Expect(err).To(BeNil())
 			Expect(format).To(Equal("jpeg"))
+			Expect(r.Close()).To(BeNil())
 		})
 
 		It("retrieves the embedded artwork art for an album", func() {
-			buf := new(bytes.Buffer)
+			r, err := artwork.Get(ctx, "al-222", 0)
+			Expect(err).To(BeNil())
 
-			Expect(artwork.Get(ctx, "al-222", 0, buf)).To(BeNil())
+			_, format, err := image.Decode(r)
-
-			_, format, err := image.Decode(bytes.NewReader(buf.Bytes()))
 			Expect(err).To(BeNil())
 			Expect(format).To(Equal("jpeg"))
+			Expect(r.Close()).To(BeNil())
 		})
 
 		It("returns the default artwork if album does not have artwork", func() {
-			buf := new(bytes.Buffer)
+			r, err := artwork.Get(ctx, "al-333", 0)
+			Expect(err).To(BeNil())
 
-			Expect(artwork.Get(ctx, "al-333", 0, buf)).To(BeNil())
+			_, format, err := image.Decode(r)
-
-			_, format, err := image.Decode(bytes.NewReader(buf.Bytes()))
 			Expect(err).To(BeNil())
 			Expect(format).To(Equal("png"))
+			Expect(r.Close()).To(BeNil())
 		})
 
 		It("returns the default artwork if album is not found", func() {
-			buf := new(bytes.Buffer)
+			r, err := artwork.Get(ctx, "al-0101", 0)
+			Expect(err).To(BeNil())
 
-			Expect(artwork.Get(ctx, "al-0101", 0, buf)).To(BeNil())
+			_, format, err := image.Decode(r)
-
-			_, format, err := image.Decode(bytes.NewReader(buf.Bytes()))
 			Expect(err).To(BeNil())
 			Expect(format).To(Equal("png"))
+			Expect(r.Close()).To(BeNil())
 		})
 
 		It("retrieves the original artwork art from a media_file", func() {
-			buf := new(bytes.Buffer)
+			r, err := artwork.Get(ctx, "123", 0)
+			Expect(err).To(BeNil())
 
-			Expect(artwork.Get(ctx, "123", 0, buf)).To(BeNil())
+			img, format, err := image.Decode(r)
-
-			img, format, err := image.Decode(bytes.NewReader(buf.Bytes()))
 			Expect(err).To(BeNil())
 			Expect(format).To(Equal("jpeg"))
 			Expect(img.Bounds().Size().X).To(Equal(600))
 			Expect(img.Bounds().Size().Y).To(Equal(600))
+			Expect(r.Close()).To(BeNil())
 		})
 
 		It("retrieves the album artwork art if media_file does not have one", func() {
-			buf := new(bytes.Buffer)
+			r, err := artwork.Get(ctx, "456", 0)
+			Expect(err).To(BeNil())
 
-			Expect(artwork.Get(ctx, "456", 0, buf)).To(BeNil())
+			_, format, err := image.Decode(r)
-
-			_, format, err := image.Decode(bytes.NewReader(buf.Bytes()))
 			Expect(err).To(BeNil())
 			Expect(format).To(Equal("jpeg"))
+			Expect(r.Close()).To(BeNil())
 		})
 
 		It("retrieves the album artwork by album id", func() {
-			buf := new(bytes.Buffer)
+			r, err := artwork.Get(ctx, "222", 0)
+			Expect(err).To(BeNil())
 
-			Expect(artwork.Get(ctx, "222", 0, buf)).To(BeNil())
+			_, format, err := image.Decode(r)
-
-			_, format, err := image.Decode(bytes.NewReader(buf.Bytes()))
 			Expect(err).To(BeNil())
 			Expect(format).To(Equal("jpeg"))
+			Expect(r.Close()).To(BeNil())
 		})
 
 		It("resized artwork art as requested", func() {
-			buf := new(bytes.Buffer)
+			r, err := artwork.Get(ctx, "123", 200)
+			Expect(err).To(BeNil())
 
-			Expect(artwork.Get(ctx, "123", 200, buf)).To(BeNil())
+			img, format, err := image.Decode(r)
-
-			img, format, err := image.Decode(bytes.NewReader(buf.Bytes()))
 			Expect(err).To(BeNil())
 			Expect(format).To(Equal("jpeg"))
 			Expect(img.Bounds().Size().X).To(Equal(200))
 			Expect(img.Bounds().Size().Y).To(Equal(200))
+			Expect(r.Close()).To(BeNil())
 		})
 
 		Context("Errors", func() {
 			It("returns err if gets error from album table", func() {
 				ds.Album(ctx).(*tests.MockAlbum).SetError(true)
-				buf := new(bytes.Buffer)
+				_, err := artwork.Get(ctx, "al-222", 0)
-
+				Expect(err).To(MatchError("Error!"))
-				Expect(artwork.Get(ctx, "al-222", 0, buf)).To(MatchError("Error!"))
 			})
 
 			It("returns err if gets error from media_file table", func() {
 				ds.MediaFile(ctx).(*tests.MockMediaFile).SetError(true)
-				buf := new(bytes.Buffer)
+				_, err := artwork.Get(ctx, "123", 0)
-
+				Expect(err).To(MatchError("Error!"))
-				Expect(artwork.Get(ctx, "123", 0, buf)).To(MatchError("Error!"))
 			})
 		})
 	})

core/cache_warmer.go

@@ -2,6 +2,7 @@ package core
 
 import (
 	"context"
+	"io"
 	"io/ioutil"
 	"time"
 
@@ -78,10 +79,13 @@ func (w *warmer) execute(workload interface{}) {
 	ctx := context.Background()
 	item := workload.(artworkItem)
 	log.Trace(ctx, "Pre-caching album artwork", "albumID", item.albumID)
-	err := w.artwork.Get(ctx, item.albumID, 0, ioutil.Discard)
+	r, err := w.artwork.Get(ctx, item.albumID, 0)
 	if err != nil {
 		log.Warn("Error pre-caching artwork from album", "id", item.albumID, err)
+		return
 	}
+	defer r.Close()
+	_, _ = io.Copy(ioutil.Discard, r)
 }
 
 type artworkItem struct {

server/subsonic/media_retrieval.go

@@ -65,8 +65,8 @@ func (c *MediaRetrievalController) GetCoverArt(w http.ResponseWriter, r *http.Re
 	size := utils.ParamInt(r, "size", 0)
 
 	w.Header().Set("cache-control", "public, max-age=315360000")
-	err = c.artwork.Get(r.Context(), id, size, w)
 
+	imgReader, err := c.artwork.Get(r.Context(), id, size)
 	switch {
 	case err == model.ErrNotFound:
 		log.Error(r, "Couldn't find coverArt", "id", id, err)
@@ -76,5 +76,8 @@ func (c *MediaRetrievalController) GetCoverArt(w http.ResponseWriter, r *http.Re
 		return nil, err
 	}
 
-	return nil, nil
+	defer imgReader.Close()
+	_, err = io.Copy(w, imgReader)
+
+	return nil, err
 }

server/subsonic/media_retrieval_test.go

@@ -1,9 +1,11 @@
 package subsonic
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"io"
+	"io/ioutil"
 	"net/http/httptest"
 
 	"github.com/deluan/navidrome/model"
@@ -67,12 +69,11 @@ type fakeArtwork struct {
 	recvSize int
 }
 
-func (c *fakeArtwork) Get(ctx context.Context, id string, size int, out io.Writer) error {
+func (c *fakeArtwork) Get(ctx context.Context, id string, size int) (io.ReadCloser, error) {
 	if c.err != nil {
-		return c.err
+		return nil, c.err
 	}
 	c.recvId = id
 	c.recvSize = size
-	_, err := out.Write([]byte(c.data))
+	return ioutil.NopCloser(bytes.NewReader([]byte(c.data))), nil
-	return err
 }