Skip to content

Commit

Permalink
Better error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
deluan committed Dec 28, 2022
1 parent 949331e commit bc09de6
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 43 deletions.
11 changes: 7 additions & 4 deletions core/artwork/artwork.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,13 @@ func (a *artwork) Get(ctx context.Context, id string, size int) (reader io.ReadC
}

r, err := a.cache.Get(ctx, artReader)
if err != nil && !errors.Is(err, context.Canceled) {
log.Error(ctx, "Error accessing image cache", "id", id, "size", size, err)
if err != nil {
if !errors.Is(err, context.Canceled) {
log.Error(ctx, "Error accessing image cache", "id", id, "size", size, err)
}
return nil, time.Time{}, err
}
return r, artReader.LastUpdated(), err
return r, artReader.LastUpdated(), nil
}

func (a *artwork) getArtworkReader(ctx context.Context, artID model.ArtworkID, size int) (artworkReader, error) {
Expand All @@ -72,7 +75,7 @@ func (a *artwork) getArtworkReader(ctx context.Context, artID model.ArtworkID, s
case model.KindPlaylistArtwork:
artReader, err = newPlaylistArtworkReader(ctx, a, artID)
default:
artReader, err = newPlaceholderReader(ctx, artID)
artReader, err = newEmptyIDReader(ctx, artID)
}
}
return artReader, err
Expand Down
3 changes: 1 addition & 2 deletions core/artwork/reader_album.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ func (a *albumArtworkReader) LastUpdated() time.Time {
func (a *albumArtworkReader) Reader(ctx context.Context) (io.ReadCloser, string, error) {
var ff = fromCoverArtPriority(ctx, a.a.ffmpeg, conf.Server.CoverArtPriority, a.album)
ff = append(ff, fromPlaceholder())
r, source := extractImage(ctx, a.artID, ff...)
return r, source, nil
return selectImageReader(ctx, a.artID, ff...)
}

func fromCoverArtPriority(ctx context.Context, ffmpeg ffmpeg.FFmpeg, priority string, al model.Album) []sourceFunc {
Expand Down
20 changes: 10 additions & 10 deletions core/artwork/reader_emptyid.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,29 @@ import (
"time"

"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/model"
)

type placeholderReader struct {
type emptyIDReader struct {
artID model.ArtworkID
}

func newPlaceholderReader(_ context.Context, artID model.ArtworkID) (*placeholderReader, error) {
a := &placeholderReader{
func newEmptyIDReader(_ context.Context, artID model.ArtworkID) (*emptyIDReader, error) {
a := &emptyIDReader{
artID: artID,
}
return a, nil
}

func (a *placeholderReader) LastUpdated() time.Time {
return time.Now() // Basically make it non-cacheable
func (a *emptyIDReader) LastUpdated() time.Time {
return consts.ServerStart // Invalidate cached placeholder every server start
}

func (a *placeholderReader) Key() string {
return fmt.Sprintf("0.%d.0.%d", a.LastUpdated().UnixMilli(), conf.Server.CoverJpegQuality)
func (a *emptyIDReader) Key() string {
return fmt.Sprintf("placeholder.%d.0.%d", a.LastUpdated().UnixMilli(), conf.Server.CoverJpegQuality)
}

func (a *placeholderReader) Reader(ctx context.Context) (io.ReadCloser, string, error) {
r, source := extractImage(ctx, a.artID, fromPlaceholder())
return r, source, nil
func (a *emptyIDReader) Reader(ctx context.Context) (io.ReadCloser, string, error) {
return selectImageReader(ctx, a.artID, fromPlaceholder())
}
3 changes: 1 addition & 2 deletions core/artwork/reader_mediafile.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ func (a *mediafileArtworkReader) Reader(ctx context.Context) (io.ReadCloser, str
}
}
ff = append(ff, fromAlbum(ctx, a.a, a.mediafile.AlbumCoverArtID()))
r, source := extractImage(ctx, a.artID, ff...)
return r, source, nil
return selectImageReader(ctx, a.artID, ff...)
}

func fromAlbum(ctx context.Context, a *artwork, id model.ArtworkID) sourceFunc {
Expand Down
8 changes: 5 additions & 3 deletions core/artwork/reader_playlist.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ func (a *playlistArtworkReader) Reader(ctx context.Context) (io.ReadCloser, stri
ff = append(ff, a.fromGeneratedTile(ctx, pl.Tracks))
}
ff = append(ff, fromPlaceholder())
r, source := extractImage(ctx, a.artID, ff...)
return r, source, nil
return selectImageReader(ctx, a.artID, ff...)
}

func (a *playlistArtworkReader) fromGeneratedTile(ctx context.Context, tracks model.PlaylistTracks) sourceFunc {
Expand Down Expand Up @@ -129,7 +128,10 @@ func (a *playlistArtworkReader) createTiledImage(_ context.Context, tiles []imag
} else {
err = png.Encode(buf, tiles[0])
}
return io.NopCloser(buf), err
if err != nil {
return nil, err
}
return io.NopCloser(buf), nil
}

func rect(pos int) image.Rectangle {
Expand Down
25 changes: 8 additions & 17 deletions core/artwork/reader_resized.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (a *resizedArtworkReader) Reader(ctx context.Context) (io.ReadCloser, strin
r := io.TeeReader(orig, buf)
defer orig.Close()

resized, origSize, err := resizeImageIntoReader(r, a.size)
resized, origSize, err := resizeImage(r, a.size)
log.Trace(ctx, "Resizing artwork", "artID", a.artID, "original", origSize, "resized", a.size)
if err != nil {
log.Warn(ctx, "Could not resize image. Will return image as is", "artID", a.artID, "size", a.size, err)
Expand All @@ -74,7 +74,12 @@ func asImageReader(r io.Reader) (io.Reader, string, error) {
return br, http.DetectContentType(buf), nil
}

func resizeImage(r io.Reader, size int) (image.Image, int, error) {
func resizeImage(reader io.Reader, size int) (io.Reader, int, error) {
r, format, err := asImageReader(reader)
if err != nil {
return nil, 0, err
}

img, _, err := image.Decode(r)
if err != nil {
return nil, 0, err
Expand All @@ -89,26 +94,12 @@ func resizeImage(r io.Reader, size int) (image.Image, int, error) {
m = imaging.Resize(img, 0, size, imaging.Lanczos)
}

return m, number.Max(bounds.Max.X, bounds.Max.Y), err
}

func resizeImageIntoReader(reader io.Reader, size int) (io.Reader, int, error) {
r, format, err := asImageReader(reader)
if err != nil {
return nil, 0, err
}

m, origSize, err := resizeImage(r, size)
if err != nil {
return nil, 0, err
}

buf := new(bytes.Buffer)
buf.Reset()
if format == "image/png" {
err = png.Encode(buf, m)
} else {
err = jpeg.Encode(buf, m, &jpeg.Options{Quality: conf.Server.CoverJpegQuality})
}
return buf, origSize, err
return buf, number.Max(bounds.Max.X, bounds.Max.Y), err
}
9 changes: 4 additions & 5 deletions core/artwork/sources.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,19 @@ import (
"github.com/navidrome/navidrome/resources"
)

func extractImage(ctx context.Context, artID model.ArtworkID, extractFuncs ...sourceFunc) (io.ReadCloser, string) {
func selectImageReader(ctx context.Context, artID model.ArtworkID, extractFuncs ...sourceFunc) (io.ReadCloser, string, error) {
for _, f := range extractFuncs {
if ctx.Err() != nil {
return nil, ""
return nil, "", ctx.Err()
}
r, path, err := f()
if r != nil {
log.Trace(ctx, "Found artwork", "artID", artID, "path", path, "source", f)
return r, path
return r, path, nil
}
log.Trace(ctx, "Tried to extract artwork", "artID", artID, "source", f, err)
}
log.Error(ctx, "extractImage should never reach this point!", "artID", artID, "path")
return nil, ""
return nil, "", fmt.Errorf("could not get a cover art for %s", artID)
}

type sourceFunc func() (r io.ReadCloser, path string, err error)
Expand Down
1 change: 1 addition & 0 deletions ui/src/subsonic/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const getScanStatus = () => httpClient(url('getScanStatus'))

const getCoverArtUrl = (record, size) => {
const options = {
...(record.updatedAt && { _: record.updatedAt }),
...(size && { size }),
}

Expand Down

0 comments on commit bc09de6

Please sign in to comment.