From 7b873860893abff67fdd951025ed104d717d477a Mon Sep 17 00:00:00 2001
From: Deluan <deluan@navidrome.org>
Date: Mon, 19 Dec 2022 15:34:21 -0500
Subject: [PATCH] Load artwork from embedded

---
 core/artwork.go                  |  69 ++++++++++++++++++++++++++++++-
 core/artwork_internal_test.go    |  60 +++++++++++++++++++++++++++
 core/artwork_test.go             |  29 -------------
 model/artwork_id.go              |   6 ++-
 model/mediafile.go               |   4 +-
 tests/fixtures/front.png         | Bin 0 -> 3949 bytes
 ui/src/reducers/playerReducer.js |   2 +-
 ui/src/subsonic/index.js         |  10 +++--
 8 files changed, 143 insertions(+), 37 deletions(-)
 create mode 100644 core/artwork_internal_test.go
 delete mode 100644 core/artwork_test.go
 create mode 100644 tests/fixtures/front.png

diff --git a/core/artwork.go b/core/artwork.go
index e1d5ca831..c8899cccc 100644
--- a/core/artwork.go
+++ b/core/artwork.go
@@ -1,12 +1,17 @@
 package core
 
 import (
+	"bytes"
 	"context"
+	"errors"
 	_ "image/gif"
 	_ "image/png"
 	"io"
+	"os"
 
+	"github.com/dhowden/tag"
 	"github.com/navidrome/navidrome/consts"
+	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/resources"
 	_ "golang.org/x/image/webp"
@@ -25,5 +30,67 @@ type artwork struct {
 }
 
 func (a *artwork) Get(ctx context.Context, id string, size int) (io.ReadCloser, error) {
-	return resources.FS().Open(consts.PlaceholderAlbumArt)
+	r, _, err := a.get(ctx, id, size)
+	return r, err
+}
+
+func (a *artwork) get(ctx context.Context, id string, size int) (io.ReadCloser, string, error) {
+	artId, err := model.ParseArtworkID(id)
+	if err != nil {
+		return nil, "", errors.New("invalid ID")
+	}
+	id = artId.ID
+	al, err := a.ds.Album(ctx).Get(id)
+	if errors.Is(err, model.ErrNotFound) {
+		r, path := fromPlaceholder()()
+		return r, path, nil
+	}
+	if err != nil {
+		return nil, "", err
+	}
+	r, path := extractImage(ctx, artId,
+		fromTag(al.EmbedArtPath),
+		fromPlaceholder(),
+	)
+	return r, path, nil
+}
+
+func extractImage(ctx context.Context, artId model.ArtworkID, extractFuncs ...func() (io.ReadCloser, string)) (io.ReadCloser, string) {
+	for _, f := range extractFuncs {
+		r, path := f()
+		if r != nil {
+			log.Trace(ctx, "Found artwork", "artId", artId, "path", path)
+			return r, path
+		}
+	}
+	log.Error(ctx, "extractImage should never reach this point!", "artId", artId, "path")
+	return nil, ""
+}
+
+func fromTag(path string) func() (io.ReadCloser, string) {
+	return func() (io.ReadCloser, string) {
+		f, err := os.Open(path)
+		if err != nil {
+			return nil, ""
+		}
+		defer f.Close()
+
+		m, err := tag.ReadFrom(f)
+		if err != nil {
+			return nil, ""
+		}
+
+		picture := m.Picture()
+		if picture == nil {
+			return nil, ""
+		}
+		return io.NopCloser(bytes.NewReader(picture.Data)), path
+	}
+}
+
+func fromPlaceholder() func() (io.ReadCloser, string) {
+	return func() (io.ReadCloser, string) {
+		r, _ := resources.FS().Open(consts.PlaceholderAlbumArt)
+		return r, consts.PlaceholderAlbumArt
+	}
 }
diff --git a/core/artwork_internal_test.go b/core/artwork_internal_test.go
new file mode 100644
index 000000000..64b9f05af
--- /dev/null
+++ b/core/artwork_internal_test.go
@@ -0,0 +1,60 @@
+package core
+
+import (
+	"context"
+
+	"github.com/navidrome/navidrome/consts"
+	"github.com/navidrome/navidrome/log"
+	"github.com/navidrome/navidrome/model"
+	"github.com/navidrome/navidrome/tests"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = FDescribe("Artwork", func() {
+	var aw *artwork
+	var ds model.DataStore
+	ctx := log.NewContext(context.TODO())
+	var alOnlyEmbed, alEmbedNotFound model.Album
+
+	BeforeEach(func() {
+		ds = &tests.MockDataStore{MockedTranscoding: &tests.MockTranscodingRepo{}}
+		alOnlyEmbed = model.Album{ID: "222", Name: "Only embed", EmbedArtPath: "tests/fixtures/test.mp3"}
+		alEmbedNotFound = model.Album{ID: "333", Name: "Embed not found", EmbedArtPath: "tests/fixtures/NON_EXISTENT.mp3"}
+		//	{ID: "666", Name: "All options", EmbedArtPath: "tests/fixtures/test.mp3",
+		//		ImageFiles: "tests/fixtures/cover.jpg:tests/fixtures/front.png"},
+		//})
+		aw = NewArtwork(ds).(*artwork)
+	})
+
+	When("cover art is not found", func() {
+		BeforeEach(func() {
+			ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{
+				alOnlyEmbed,
+			})
+		})
+		It("returns placeholder if album is not in the DB", func() {
+			_, path, err := aw.get(context.Background(), "al-999-0", 0)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(path).To(Equal(consts.PlaceholderAlbumArt))
+		})
+	})
+	When("album has only embed images", func() {
+		BeforeEach(func() {
+			ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{
+				alOnlyEmbed,
+				alEmbedNotFound,
+			})
+		})
+		It("returns embed cover", func() {
+			_, path, err := aw.get(context.Background(), alOnlyEmbed.CoverArtID().String(), 0)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(path).To(Equal("tests/fixtures/test.mp3"))
+		})
+		It("returns placeholder if embed path is not available", func() {
+			_, path, err := aw.get(context.Background(), alEmbedNotFound.CoverArtID().String(), 0)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(path).To(Equal(consts.PlaceholderAlbumArt))
+		})
+	})
+})
diff --git a/core/artwork_test.go b/core/artwork_test.go
deleted file mode 100644
index 1aa6838eb..000000000
--- a/core/artwork_test.go
+++ /dev/null
@@ -1,29 +0,0 @@
-package core
-
-import (
-	"context"
-
-	"github.com/navidrome/navidrome/log"
-	"github.com/navidrome/navidrome/model"
-	"github.com/navidrome/navidrome/tests"
-	. "github.com/onsi/ginkgo/v2"
-)
-
-var _ = Describe("Artwork", func() {
-	var ds model.DataStore
-	ctx := log.NewContext(context.TODO())
-
-	BeforeEach(func() {
-		ds = &tests.MockDataStore{MockedTranscoding: &tests.MockTranscodingRepo{}}
-		ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{
-			{ID: "222", EmbedArtPath: "tests/fixtures/test.mp3"},
-			{ID: "333"},
-			{ID: "444", EmbedArtPath: "tests/fixtures/cover.jpg"},
-		})
-		ds.MediaFile(ctx).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{
-			{ID: "123", AlbumID: "222", Path: "tests/fixtures/test.mp3", HasCoverArt: true},
-			{ID: "456", AlbumID: "222", Path: "tests/fixtures/test.ogg", HasCoverArt: false},
-		})
-	})
-
-})
diff --git a/model/artwork_id.go b/model/artwork_id.go
index 83a0855c4..9a750f255 100644
--- a/model/artwork_id.go
+++ b/model/artwork_id.go
@@ -22,7 +22,11 @@ type ArtworkID struct {
 }
 
 func (id ArtworkID) String() string {
-	return fmt.Sprintf("%s-%s-%x", id.Kind.prefix, id.ID, id.LastAccess.Unix())
+	s := fmt.Sprintf("%s-%s", id.Kind.prefix, id.ID)
+	if id.LastAccess.Unix() < 0 {
+		return s + "-0"
+	}
+	return fmt.Sprintf("%s-%x", s, id.LastAccess.Unix())
 }
 
 func ParseArtworkID(id string) (ArtworkID, error) {
diff --git a/model/mediafile.go b/model/mediafile.go
index 83275cc6e..8b79523fa 100644
--- a/model/mediafile.go
+++ b/model/mediafile.go
@@ -69,11 +69,11 @@ func (mf MediaFile) ContentType() string {
 }
 
 func (mf MediaFile) CoverArtID() ArtworkID {
-	// If it is a mediaFile, and it has cover art, return it (if feature is disabled, skip)
+	// If it has a cover art, return it (if feature is disabled, skip)
 	if mf.HasCoverArt && !conf.Server.DevFastAccessCoverArt {
 		return artworkIDFromMediaFile(mf)
 	}
-	// if the mediaFile does not have a coverArt, fallback to the album cover
+	// if it does not have a coverArt, fallback to the album cover
 	return artworkIDFromAlbum(Album{ID: mf.AlbumID, UpdatedAt: mf.UpdatedAt})
 }
 
diff --git a/tests/fixtures/front.png b/tests/fixtures/front.png
new file mode 100644
index 0000000000000000000000000000000000000000..d4a3663912de13719b0a16f65d108b9d80bb9595
GIT binary patch
literal 3949
zcmZ`+cQl+^*MIchB1AVtCx(b98J!7&QKPqEhS3HyqK`x`2@*tNbP*+@iyGw;k?8&E
z5j_a56rz12H(%~u@4Mc!*6-|n_Wtd?pS{od=ZP~kxJgINK@9)^osPDqF+nn(gOZ%E
z^X{KH0syTFN<+g?M?(W-=!-_8JP-h&9hYWKVPW!)HQU;luU1vWSREZ6i+vV3L`<p8
z;t?sJDk4oD9&c-D>V2g+Hk?j8f87eOG}YXa2!9P`HBI$1y2L4lHddVsT?yU~#jLfR
zWgZSb#jMTWAh{bPV=gfOl?8g3R&?O%#7~$XJ$m}A=q@pqh`j;ulmwgi#U~~Lm9%wX
z;2keObHdIq8Ff-~*2b#e3QDH{F3|aisidcq>@fkmy4e<wC;+2w{9q4ruT+_;H8DwK
zKnZP;xOX9Kk=U+UqrMF-=f~SXPpM7005?!a-y^dhxM8UAk^wHvWG_OjnnhKGHW;|3
z+$G95qsl4Wbu~0;YzI4X_WY@0*}IkJ3{G!smZP{t`dC~rE+;t~2MdD2){4uW)VIfJ
zbzJXz3`n8ttDi^pR*H4K&L8U$iLC>jf|h%?`Jg52BSQgEN^U*9h|WwR@-BlAF9-Rq
zXhw>J>dTi?L)<c?o{&h}@U81P@(9zzm15JWeIwv$*R2Id;i;`dP1mUKh0B8&LEpBk
z_6*M~_aq8W@w7xYOOi@hoVx_o^`iddw|~M&RmIVj*l1a`sEe;6W8doG<BYooWo;wt
zk|s9NzN^mL3rit=E3%z{@r_wc@a$*w<xe{>w1e6d^Q%~hCy6KV4v=j!##-!YD2C5c
zF==2HX^0~(V%}!cXzo?%W-)whxy$y(B2#Q2@%Ho*GJK1X9Zg<!%L`ZLE0LW{ap{>l
zFN}Fj>#gX=E}E;2j>GW*yJjHbN;QxJv1nO|14j?(AK&JG*aZ}=Q<2+%h&EIKuGGGv
zSk1sBN;@Ytq5~3sq8@IZOQ0Q=yCFSc>_`@Kie#ak{`90CVi?~5(ISX*{?<x?X6O8c
zcIF<+v&AeLafWU@-HWobg?)`JxoEPdl!BMJ|LQU2trj3>B8kiEa>&2pb@%Awd;n?7
zkIql`gFj?z>>nUu0lMn~5PlnBERz?EMe@-x<BI$c+1H4)M(q>jOARbz*f%8Jyv6is
zKIC2im-nUhn5ZMQ!SZB;_|juU)M3Q81?2ABsNu9xrih4E`bSvRq0h67tc=adc!|k{
zwVO=tpS)YAI+av@xP+yLDtmN>)pk9G$5dhd6x|}8h#V&ox^uz3eY$e#Dx{?;yT^Po
zxt=W$Vrh8ufFVo+cKh)=n<9=Po}$6{R!E25lpIH8QJjlT<OX}79_#UC$|qb*gFXVI
zQ}3qOfvGdCy~Cj1<TE+O?l<UJxIYj=O+i6{Z}nNcK_s6^YJcXwAitTk%jrqqe%KEw
z5wP!AtfnEt363q|pEKOQr%-uspU`7F=RpLBkvw0OOPkol;S0NW)~$A0DGCWvtz6PF
z;>~twC7B0Ba68VMax6l~n>v#$U6rDN{_`K>qjYxR%DeQmAWC(0S4!^Tn|h45x_Eq`
z5Jr0e$~aYcvVZ`Y&P~nOi%%mYpt{M%&X=p{N}xEkM82YA8f{@;&cN6&AapWJB)T+z
zx|qo=Dp6g@kd-5`J1=Dzi&eC`%bG7f<XcNU7c-GpG<0j-ua+h>?wb=}(!usbboFka
zgNFq5t?m&is_P8m-R~>K%2?ea2M@(2qQ1g;efSODK5Zbe>BtL!40IMP?eDG}KtmYp
zV(LGd@5Kc0o~Ef@X1lOOWk*WyNX(Zd6=A3;a(RoNOEB}^j<MV#_d{)G#+PxQcWw3f
zkr!N;)0yR%7#Q(!SzT|ta2Mj9!K#H1lk>H0jCo38Ywy)2VIN_m#`J%v-sZz6XT9rK
zcCL<XiM4^w-%?g<RmVYja$OB2MD0>VsB^9+R6nYY7}0Lj+jvfEC|fdN%In4@o6?kQ
zW$>~vJ8!<A)y&99+GNj^y@KD=GS|4=*Knofy+&G6sa5HnQjc++QsZ%!(q=Qf8Tv(Q
z;jruNYhhMbuOz(+zpZ~mKUnoB7ix|%T`JxiUwvU#JYHgJ6kKRvRB!510y6>|f=yJ)
zaF(ylBk~;!J0?E3^^Fz}$v^ioDX>o|SFn=NmAQ#&T(#?`$r!BDwz_SlW)<>M{pGe*
zQ60~ybdy4s{@U=RQU9UAq2{`{>4|BsZN2Ra+b7f8)1iu1ijs;{iZXs;*u+h&v{W=p
zL}w4O%$W;5>ku*`JA&At#-?vfe6kx3t&me6w1?%Im-(yiG|dZ#KF1~F9-q3M1f3j_
zH{8qW2#8=O$I}h4eMubRSyT#_%Ma+vWAdjp(zwyd);SeDLpMWv^x*yj_4{2=&ie+O
zUpZv%pFEIxFv*c4^;tR(e~>|`Kar7nEwE$EvHHnWpD$9pW^ioi+L)<ZQ-AFow!Lp!
zV&3E76j9C5swswH|Jr`t>Xp?{Rw33UiB*d+i`lAY7C{zBi}HGi%i;X{X|Z9qIJ4K|
z7<VQ2M8A7}556jKW<1F0$Ku8C?yoeh8D=DA^j~q`Db9Lf8*LjkhX@{<@3(Vpd)JB)
zaD29=J=|0JP>$P_+k*a(UY4GLn>~>!F@bB7+x|f{TN=deYcZyvRk2YKabw%N>4ud%
z_Rn>#0?wOlKCdvV3g+_moEn_W5aj8TXz2>uEn8AayscN`i0#`(Ngq$2laH&Pg?wK4
zZTi%E!+a{RZ5w&pOxsnnOD)13GVPdFmz1@eV7UrNg@#>N(2W3>z@4u+<%ys&hpoh_
zq|m;Q2IZNMrjVztqOI1U&BuBt4LCDg(sBRE;c4@6PL+q{)3VSqb0PzxM|TT|cBbsM
z8n;$eC8Pu@<tvr6#YL1|orWSDYCe4Vkny3FaWZ*|uRkwxXm@BlPfh{K#Z#0qdTEvC
zx?Gv8jV$DPh|@s<WizFY=8`<t=FbM%rO#Sc(0KmM0@aJcZu(;<uAA!{!)=MeiAg;6
z(x<4t^(P9_Yw(RK6Q}RptqC--*6jXr^FnqU3y*TMr9|@-7enC=a1?wp(ujH0pdr;c
zSwPQCTS!n?U`O|J4%;=KxvtMT<tgz(b5A^!0#}!5OlY4w!xWUwLdE<)EnZaDg}<Lq
z9h?>MVyRb2OR;)7{nlxJV&CqslSB7IC1PH+k`I;{Fe}vbJ~roz?1mCMjvc}NTK$!G
z!LA?Db@g%IcS|G7<{I)E<{F0OWQ?(IVqC4JW&Q-JdlYG~d2uEoq04sk_45k+yf9Vz
z)_W~q;kE0_sLlr=_%Y~@lrN60Xq?t*3_Fz!{oq)pXvWm_UayI2%R%aJnmYsCF8j#(
zKIyC)?}5(7vc{QJzJ24hx<0uad-=9twVrS#ni85C*3(x#D5H)1HdbLtFNtenHiicW
zMaemC@ni7<T5{<D;Hm11_F^{EtsZ^VbiJ*t`w8I3@@c7QA$9}SxV^PumGm=61HY%%
zk#2n&-kixi?)IF;c6|`61c_4HSdv|u7<-8*zhZYZ*;KMAncA4@X$i9h+b)>#7|mC9
zz&lIna^#R6Ep|+XKDi^6zLqOT`p5d_*N@QUbMV?i=iZ{1f4r=J73J~bMRlpzjKOSn
z+sQ^^B=_2#pslibMCA+T$@IDWIon|Px7ypy=H@%Hv(44yFCR{P*}~UWIE8&Ujkb!J
z_M~8=(4`NL7>NMmbPiX#^L90^#3<2E@>D^L15<Ie4fG{=vq|$DqlNM0v7tbTEsI+P
z{!7F5++lNb&eNtoHfsm(na7x;gvnv7nOkU9x$oJ!DqkBAZoPA(%m()E+zZn<q2s>8
znhqCOj#Ji+!1=Y1QL+k}w~|3NWo^rMO3M4|w%5_+ZQP#z?iI~jxW>HBnu02f$6A~?
z&fLJ3t|jOj$J%t^=EKPw=&pqZKg>vfx%IL2AlhXC7b<)<&^|}@o$`D9j{7V)*x?}e
zlmE;D1GdqRJB;n1a@%ODpgAcp$zJ(g=wFS^yQ15uDVY?P@xiPdxv!+(u`N#y@{bA+
zR#ef$ycuDpt<`~k%dj(*nIA%5{MsJ3@PyPHzdW6p3u`<cAK{%l`(QiJeDC`k-Y+U!
zhwFQLvqfF7L2_h>P^jb?{HSox{a`XULLnjoULGmFK<!YOk3s=0Q*;2`XYTdg5NKAh
z{MkF2eEFL9Ob0LvA86_&Il#^F@=SLPo=BeZ#W?Yy5n0cxU7u5cTL#%cpEbU`rfbRf
z2c~^OmA&aT@-O0D4WmqjwuGa%tUac-?Md5h00<XMUQRuPB>YjtBP?`~`uczvL8k;r
zi8uf<f<{DuG7;x*9ZDnuko>e013(N4ApONLAjtETOu%`}-;(4}G(bT(T_7MJm-siE
zF_+{Yo!|r1Of+<K2-3vK7lH7?xT3Kss~mL%1C_V7B?bUk1<ygGV=S;s$UlZMwZK~F
z>nS>+JtZBT(Qt%hfT#C)9sm-cNKicySVvHRr-v6tF+hp`Cqt2-pPRw_pq~`1yAr>J
zz9C2h?TY|imz0u};#Z~yfj|&nXQZOBCiGW0;iSaxip6>>g2Dd&{*wMOl4xHSu(X1L
z0$A!Q`0CXw1jZFipcmFL;EEUK^504RKaVB?<K&C-#-h+(p!2+raI_y*iJ$+x(C_v4
zcOn8%|5WnA{A!EPAo$z^mX?$P|ISSag`8Uz4N(CI4@*syCt+rU8p_hwAwTK=<@l%I
zzXC1(SKz-K{~2h8LHKH*JqZP|%Ku!}ud4rc{tASE&-?z@9R42iPb*={%G40>?;BI5
z#z5fPgiGW?X~IkilJLZSE=s~KLcqB^hi-dI<19h4>S(H&$}_=_8Axd_0^LR=Y|(0L
o1UqncHY8Z`l%QNZXE^<bUyxU2_`5ac**Q~3%RsYS-68z{01Btw{r~^~

literal 0
HcmV?d00001

diff --git a/ui/src/reducers/playerReducer.js b/ui/src/reducers/playerReducer.js
index 428be7fcf..ff39499aa 100644
--- a/ui/src/reducers/playerReducer.js
+++ b/ui/src/reducers/playerReducer.js
@@ -37,7 +37,7 @@ const mapToAudioLists = (item) => {
     musicSrc: subsonic.streamUrl(trackId),
     cover: subsonic.getCoverArtUrl(
       {
-        coverArtId: config.devFastAccessCoverArt ? item.albumId : trackId,
+        id: config.devFastAccessCoverArt ? item.albumId : trackId,
         updatedAt: item.updatedAt,
       },
       300
diff --git a/ui/src/subsonic/index.js b/ui/src/subsonic/index.js
index 54b71a018..9908ee442 100644
--- a/ui/src/subsonic/index.js
+++ b/ui/src/subsonic/index.js
@@ -51,10 +51,14 @@ const getCoverArtUrl = (record, size) => {
     ...(size && { size }),
   }
 
-  if (record.coverArtId) {
-    return baseUrl(url('getCoverArt', record.coverArtId, options))
+  const lastUpdate = Math.floor(Date.parse(record.updatedAt) / 1000).toString(
+    16
+  )
+  const id = record.id + '-' + lastUpdate
+  if (record.album) {
+    return baseUrl(url('getCoverArt', 'mf-' + id, options))
   } else {
-    return baseUrl(url('getCoverArt', 'not_found', size && { size }))
+    return baseUrl(url('getCoverArt', 'al-' + id, options))
   }
 }