From 30bb3f7b435c2396f498e68fad896999434c73c6 Mon Sep 17 00:00:00 2001 From: Brian Schrameck Date: Wed, 5 May 2021 21:35:01 -0400 Subject: [PATCH] BPM metadata enhancement (#1087) * BPM metadata enhancement Related to #1036. Adds BPM to the stored metadata about MediaFiles. Displays BPM in the following locations: - Listing songs in the song list (desktop, sortable) - Listing songs in playlists (desktop, sortable) - Listing songs in albums (desktop) - Expanding song details When listing, shows a blank field if no BPM is present. When showing song details, shows a question mark. Updates test MP3 file to have BPM tag. Updated test to ensure tag is read correctly. Updated localization files. Most languages just use "BPM" as discovered during research on Wikipedia. However, a couple use some different nomenclature. Spanish uses PPM and Japanese uses M.M. * Enhances support for BPM metadata extraction - Supports reading floating point BPM (still storing it as an integer) and FFmpeg as the extractor - Replaces existing .ogg test file with one that shouldn't fail randomly - Adds supporting tests for both FFmpeg and TagLib * Addresses various issues with PR #1087. - Adds index for BPM. Removes drop column as it's not supported by SQLite (duh). - Removes localizations for BPM as those will be done in POEditor. - Moves BPM before Comment in Song Details and removes BPM altogether if it's empty. - Omits empty BPM in JSON responses, eliminating need for FunctionField. - Fixes copy/paste error in ffmpeg_test. --- .../20210430212322_add_bpm_metadata.go | 30 ++++++++++++++++++ model/mediafile.go | 1 + scanner/mapping.go | 2 +- scanner/metadata/ffmpeg_test.go | 17 ++++++++++ scanner/metadata/metadata.go | 11 +++++++ scanner/metadata/taglib_test.go | 23 +++++++------- tests/fixtures/test.mp3 | Bin 60845 -> 51876 bytes tests/fixtures/test.ogg | Bin 4408 -> 5065 bytes ui/src/album/AlbumSongs.js | 2 ++ ui/src/common/SongDetails.js | 12 ++++++- ui/src/i18n/en.json | 3 +- ui/src/playlist/PlaylistSongs.js | 2 ++ ui/src/song/SongList.js | 1 + 13 files changed, 90 insertions(+), 14 deletions(-) create mode 100644 db/migration/20210430212322_add_bpm_metadata.go diff --git a/db/migration/20210430212322_add_bpm_metadata.go b/db/migration/20210430212322_add_bpm_metadata.go new file mode 100644 index 00000000..3fc0c865 --- /dev/null +++ b/db/migration/20210430212322_add_bpm_metadata.go @@ -0,0 +1,30 @@ +package migrations + +import ( + "database/sql" + + "github.com/pressly/goose" +) + +func init() { + goose.AddMigration(upAddBpmMetadata, downAddBpmMetadata) +} + +func upAddBpmMetadata(tx *sql.Tx) error { + _, err := tx.Exec(` +alter table media_file + add bpm integer; + +create index if not exists media_file_bpm + on media_file (bpm); +`) + if err != nil { + return err + } + notice(tx, "A full rescan needs to be performed to import more tags") + return forceFullRescan(tx) +} + +func downAddBpmMetadata(tx *sql.Tx) error { + return nil +} diff --git a/model/mediafile.go b/model/mediafile.go index 859fe226..f65f0916 100644 --- a/model/mediafile.go +++ b/model/mediafile.go @@ -39,6 +39,7 @@ type MediaFile struct { Compilation bool `json:"compilation"` Comment string `json:"comment"` Lyrics string `json:"lyrics"` + Bpm int `json:"bpm,omitempty"` CatalogNum string `json:"catalogNum"` MbzTrackID string `json:"mbzTrackId" orm:"column(mbz_track_id)"` MbzAlbumID string `json:"mbzAlbumId" orm:"column(mbz_album_id)"` diff --git a/scanner/mapping.go b/scanner/mapping.go index 6ee2bf23..7f99e537 100644 --- a/scanner/mapping.go +++ b/scanner/mapping.go @@ -64,7 +64,7 @@ func (s *mediaFileMapper) toMediaFile(md metadata.Metadata) model.MediaFile { mf.MbzAlbumComment = md.MbzAlbumComment() mf.Comment = s.policy.Sanitize(md.Comment()) mf.Lyrics = s.policy.Sanitize(md.Lyrics()) - + mf.Bpm = md.Bpm() mf.CreatedAt = time.Now() mf.UpdatedAt = md.ModificationTime() diff --git a/scanner/metadata/ffmpeg_test.go b/scanner/metadata/ffmpeg_test.go index 0d6f0ff4..33c0db9a 100644 --- a/scanner/metadata/ffmpeg_test.go +++ b/scanner/metadata/ffmpeg_test.go @@ -286,4 +286,21 @@ Input #0, mp3, from '/Users/deluan/Music/Music/Media/_/Wyclef Jean - From the Hu Expect(args).To(Equal([]string{"ffmpeg", "-i", "/music library/one.mp3", "-i", "/music library/two.mp3", "-f", "ffmetadata"})) }) + It("parses an integer TBPM tag", func() { + const output = ` + Input #0, mp3, from 'tests/fixtures/test.mp3': + Metadata: + TBPM : 123` + md, _ := e.extractMetadata("tests/fixtures/test.mp3", output) + Expect(md.Bpm()).To(Equal(123)) + }) + + It("parses and rounds a floating point fBPM tag", func() { + const output = ` + Input #0, ogg, from 'tests/fixtures/test.ogg': + Metadata: + FBPM : 141.7` + md, _ := e.extractMetadata("tests/fixtures/test.ogg", output) + Expect(md.Bpm()).To(Equal(142)) + }) }) diff --git a/scanner/metadata/metadata.go b/scanner/metadata/metadata.go index 137482ad..a4ca658c 100644 --- a/scanner/metadata/metadata.go +++ b/scanner/metadata/metadata.go @@ -2,6 +2,7 @@ package metadata import ( "fmt" + "math" "os" "path" "regexp" @@ -66,6 +67,7 @@ type Metadata interface { FilePath() string Suffix() string Size() int64 + Bpm() int } type baseMetadata struct { @@ -127,6 +129,15 @@ func (m *baseMetadata) Suffix() string { func (m *baseMetadata) Duration() float32 { panic("not implemented") } func (m *baseMetadata) BitRate() int { panic("not implemented") } func (m *baseMetadata) HasPicture() bool { panic("not implemented") } +func (m *baseMetadata) Bpm() int { + var bpmStr = m.getTag("tbpm", "bpm", "fbpm") + var bpmFloat, err = strconv.ParseFloat(bpmStr, 64) + if err == nil { + return (int)(math.Round(bpmFloat)) + } else { + return 0 + } +} func (m *baseMetadata) parseInt(tagName string) int { if v, ok := m.tags[tagName]; ok { diff --git a/scanner/metadata/taglib_test.go b/scanner/metadata/taglib_test.go index 82c44ad5..8c638827 100644 --- a/scanner/metadata/taglib_test.go +++ b/scanner/metadata/taglib_test.go @@ -33,19 +33,20 @@ var _ = Describe("taglibExtractor", func() { Expect(m.BitRate()).To(Equal(192)) Expect(m.FilePath()).To(Equal("tests/fixtures/test.mp3")) Expect(m.Suffix()).To(Equal("mp3")) - Expect(m.Size()).To(Equal(int64(60845))) + Expect(m.Size()).To(Equal(int64(51876))) Expect(m.Comment()).To(Equal("Comment1\nComment2")) + Expect(m.Bpm()).To(Equal(123)) - //TODO This file has some weird tags that makes the following tests fail sometimes. - //m = mds["tests/fixtures/test.ogg"] - //Expect(err).To(BeNil()) - //Expect(m.Title()).To(BeEmpty()) - //Expect(m.HasPicture()).To(BeFalse()) - //Expect(m.Duration()).To(Equal(float32(3))) - //Expect(m.BitRate()).To(Equal(10)) - //Expect(m.Suffix()).To(Equal("ogg")) - //Expect(m.FilePath()).To(Equal("tests/fixtures/test.ogg")) - //Expect(m.Size()).To(Equal(int64(4408))) + m = mds["tests/fixtures/test.ogg"] + Expect(err).To(BeNil()) + Expect(m.Title()).To(BeEmpty()) + Expect(m.HasPicture()).To(BeFalse()) + Expect(m.Duration()).To(Equal(float32(1))) + Expect(m.BitRate()).To(Equal(39)) + Expect(m.Suffix()).To(Equal("ogg")) + Expect(m.FilePath()).To(Equal("tests/fixtures/test.ogg")) + Expect(m.Size()).To(Equal(int64(5065))) + Expect(m.Bpm()).To(Equal(142)) // This file has a floating point BPM set to 141.7 under the fBPM tag. Ensure we parse and round correctly. }) }) }) diff --git a/tests/fixtures/test.mp3 b/tests/fixtures/test.mp3 index e6941d360609765e8c895dbae1edff05969f5a95..6f7c494c9531ef75db48da704e6b2345d8d060c4 100644 GIT binary patch delta 449 zcmYLG%TB^T6rEBYL4%PEQ4k!$}pyKO>dBC_Y>-b!w1v0 zEYLkf-M|~uJP;j2Fpb)MSyhBCMR!`(d9_}}Jxxly|B}A#SgfqYr@pe}zLzpfkfG4iEaSZg`V5aXRfKyY*W#ZPnzOIaZ)z$u6dj0eAd7UGpBaN*pqoeF* Vcff;8YzF8sEq`W}A04Ms#XmWZXIcON delta 330 zcmZ27m3i%L=5S9JV?VeZ*BiJQ0z91= zQnVPn0^B#}GODN4hZun@2+q$-X9x)}WMJT6a4agxEG_}^fHHgxjyXxCxe72bXP^_f z7@YHS3-XIoix@&2eHa+nA!-;xg1o`*);BZ&YIbB`U}Z4WHv)PgfPsOT!H^-;Cxn4P z9OQlmpUR@lWT0z#eJ1Otu|pjw&M?_AO>}c%+DGQi4QY>vCMGwez1y6WbtoPHNH$r# diff --git a/tests/fixtures/test.ogg b/tests/fixtures/test.ogg index 220f76f0cef41c7e50736c9134ffb20f0d297961..be672812991e95e597a322eb3bb021f22143d087 100644 GIT binary patch literal 5065 zcmeGrjaYVuKiQ(ghjHdIOr$!Mz->=;bFnrhnHq1!fVYqs`hnn(SZhE^ z2c)K3NK#r%6eDO!3`%b4sYc!D1=OJ+Wkf^IK$`~B(Od+<%jweH_U_W94_Mgi1*_7I zJ8FxQT?yPh@GUiD58h6f=1gdg08rrN8OUsFI z105JQS3md|j_A;$=bCnQOww>^z4@L}J0XfJ>buS0n{T=)+qpBP2#P$!gf#39X>=XZ z*~Q*)SLQnj1^63+aCs9rHrA0H?86K3;YpHs6<4b^f37y3t)6??xPp~=P8PZaL0TL5 z9`|-+vmBK1Q#iC9OO=Z5X3BAYGyyp&PS2Hq>7hx7wq$!n_YuPSu?9_}r zXaR_SpsKF9M1|1ZkcqBOSx> zB@o1vQ2^y^Ey+Mr0Y?fpfK*pP0d5+1-&+A<8p*{oLy}a6%MmiFs$Nq2tz0z#qvxRK zM3OL^53Z`0jc={oNiOalE}jYg$Av(ol(Ny-)?LG;A5bQQXiFIUst76NC1vM64;PxJ zzc&x)7FIS^8;%lHfj}2euD_4GlnxL@jY9n%o{zX8!}1kXRgKkAqvjM(si0vVY6;(< z(wYEdMDGPLIJOL;FxBGO?jdUC{DTuH@C|m5svM}XZJZ!jyA#LJgxd-^Y-&Y?XaIq^ z9F8wRoW@3AaWLB#HmX;mD(Z2O9XPQF0p-OK#M$zDoE4#Gplq^m&vjU;FkU-u1r;r3 zsLF__|Y zwfe^X29SW+63M^Lz9aq{gB(<7wGE;e3~g(JANWj!9~9{Q zh+;klTUW1BCvGCfT&e!)^OzQxBaHVL#H=>k&ZwKChvl4a$wo}o9bkMut>k(h4 zN|qZLCuSiCp_22!cew!vWCC7{z!8;VFVKjvR>F#65lA_s3`B|mUshT7z&AoHS_!Zs zARb@uMRhQ!^YC@Ti`j^iIKuL1R!22g#)l0HsXjEJ%F7Iq}>yq z>j6v6fC-giHtbkgC$*331#IScfQ01oKpJw{RItW|74h{vU#XX2!g&tht>k#Yc!N3w zX6ONIq_`fu0)u?U9Ye5DFXj|^H_RU?JT zKiXsL-0U5k$u>4rdz&3Qt?Ve4)a?}8omRW7tn3`@ciK>MyrPP7y!=8#hWW5U)M$yq zuIQ|W*(*A~5)Vg_%iB~4<^3`FY9qPA*}yFoS>xY6g!;`csC7>Ko@zDO*pyNqcK+RE zbSA&zmYe!2?1){$`P`T@UrMXm0xih{=ORagV$6SgVosu`pKy15Wz}}}q7xT$(q7Ws zuk!=cdBiCHM(SowR@>$3jQtlZAO6sAq^$UhH4nM745J(au&IUa^S@Sg7QrLFZ`Buy!}uuML_L`(dXvno%;g2RjX*Ksmzto;VN zV#UZ%LIyj*UqkmKk-RX4JGAG1^W`HyuQwp;=+2dVD-KIesJz}?lyL3khnXRz?Dogm zLr`Wp&P%7GfA1}GE)u-1V0mL*^+>>IK>WE;XXEfm)CTI^pt zC$1ZtGa5cx;P)*h1f$WxX2g8Uq1PlaKa`^vIITv zeoj_+g{^EDOe?wj^XIVr#RZE!7riY$=j84GlGNpP_i-rcB%<_Uf>9xgD}8YH-sI4n z=H~-z=f{$?dfv~ybMb|;`d77UdN-L~c>hQD=gmjoJ*&RkamBJW6U*QKOgz=4<=HQj zVeVbYFNq>+5kp{Xcw}B>#<7Jul(D*I9RouSW;_~NIGmNRtD*t>nS!wmeRV$fUJSo{ zaB}nX*foeRfL|+WE_@{Jmeetq-D9|7UW7)(YmwIlaQ3-&dCl*AqA&7t^E|RpVXAsgMOHHJEM8lVGWn@=R8qRpDk zWs-AL>HS-k+s3JVH|G8t2r776)Yg z?*v}toFvL`nc%O2mX>`|s81nTNp9Ptc=L<=i&ezCg`T{&@{enk>VWNl}UDNb_Iot~U<_ zx896Qq@JfGfBgKB_m!o$d4E@tYV-J*VGK{${(<<1cT4W5M^*TvedYOPaD33~{9{K= zqn;(&7=N(X>4(%2HY8%Px(eNCtX$FcV_H*5@|)=CMFRg%r>0IDI@_?tv4)PY-`9)z z?sqxDh)zT79|NS|!tgcSAv1!h#+GY_BVCfHFzo!+atobIdyjD^vgIRec?mg+HM{9a zA}7fX#Vc?3V{5$mT9fZyrE%-%u)F>z`Q}Z9b14mu!cwD;+af;ee|V|VqUy;X->2+yaC2s=!!u`$7&CXT zX_L}$Dben9ZCL4w6t_=@Zo9Esl~C~~(;8~-O4jo|YZo=b)&H6f>nl_ePH?lzCSn|I zGK`sT*1RDybCU*MpwOTIKd{HiV z^{doK{ZA{*XJReewbbKZrQ4E|IhId@qk`7r=NydoCPG48XTGl7$CNZi`fW}R;Ym+W zh#ZSpO*SH!-Rtap7+?D;^FvAB)yW9^Ulq{N#<*3@^OwasokdZWrZz=q4`V;s8T>v) zEGzczeSB^FObs-W60id+hKA!B`T}sm{)K|slFTj?P~?hIPJiab4$5+fM> zQexm(CwED^sY45Un|5Xye$(0XQmHMi*832m3IhL+HwfTR_p6F;RJ9*g6Z z#GRd){^{vQ{uW$7nQR(UO;ZOjNg)+iXrb2KopnX2ibXyux^=s?r3;A8j=TO*XaCtf?*%`)|8!^E zKRP>m=jP_zd(J)Q-ru?J+;j75Yl~0>S}Mage*XeRE-Smelkp7W)xC{XMiVsYHU63@(SC8+$lcau;)5_0yv3yKq zati%jYJ0i(z=Ttes0pvDnz=TIiY2s(HkP=osT+Z^D`3n>jbZ8sjYd%iTor4zZB~}W zw*DpSKHsKMlwdHnAh4}epYmFdD?k{}%q_4+D;D^+vr@66^_=X~DjPFX+|l}t?80Z( zsgyXUXzHN2zB*{ z9X;Y$EO{^%T|fv^7IHt?$G`Y1{>1_Qa@LB{D1;b@1#-C#e{SbDxh}laC8F!x8zJe~ zAYC))>A46M6tXq@xXb8yH3+e~vK)aDN1)CzZ|`K;4jzv{7KE5!M!u;idy6~gO0oXh z@SYow)pCy^iJlG^VpGPQebIJVWKuNk2Uv!Md9?$I z)xO-J7S}tDj}`Mi)A+?XU-~#uJ-$-57v$!B=ekN`!&K@(%Y;`#jqMx1wZq351!`@7 zi_`n&5KvM5#!m0n0ibd}ZXw^{ubYtKy`b6_p#@liSt0^kKX#T4|^y= z%h4diewEn4H!J@{u%_ng1e-Zk$r-m)-!8~vlC3`V;^ab|-@Sn)I<$|}riKBJKuVUd zjZ5Y}XsYbOgS^H|#Cu(>T1j7TPe;a>^n=TEv*FJ158_^6r=P<%J=cGZEZUPfQ?mQ7 zig~Z~!);}|`-ey^5pa{SG=2iWULBoO<1C~!SJ{d8e2uY^-Y}d3$~6e(|D8X{S^cQ~ zjemtBDvXDkHl2|Q4c9#8(vS9+UvI1UcK;Vp@0%-2JciQm)bQ3JNC@Dthc`lu z@=TW%3%DD5uuX6%YT$06&s*LgzI6)!bV{0kPoAH!v5>2omg^@ID++(Kr|@~hbY?}- z^A-ASdtUbJDIYf9c-wH}+x=fYRx6g#TZ~6&gKG`pN+DdcS)F*EvhIQeDWj<`(($#p zb2iI`=ea~iS8a=@_lWn_k)x6jErKvgges0;v!k!s(c9d)V0YCW@$~k2pSu*ibNa;X z@2kt{1CArqgl}oYw~)Ao1cFA_!luyWkEG+8M(Nm;JiN!rgpB~`%T3{+vE%@R*cVT7 z$0qr@c+pidO<$CDmDF4<(tlskP0Mu&0Bj6$GLRE1N=qB{#?6;>y0=CFz3GBIGMKDcG&S zb6=yjgU6Xj9fA0cs#5P<%h6Ovu+JOTqM4NIiu8G|V9QZRmW|) z(r_rW2!`^xAg#Y@{)*B77DJ1tX`|D{I=#6<;0PMMp-||=@|=7w=&ZPAJ~01Ak1-T_ z|MWUcN2xPcltxo44Bftzq11z>ikCeMYRc`?gX{wRHQ;*;i0Vnl0QERNB-$NaZ*@r8 zPfpsFuT*vmpNp`F5jw*Qa*Hm>)(ZVt@+fZOs@IAFQ)EcsCrKjFR;-nXj=9NQgdQV_ zK6k8IB05AObp1di{MriYE@+WpC9=piQ44jMpHs}2X%Vr_YbLsF`9Cc zi%S%yCElfGP){TxV5Pa3RvhqimtIVLem6ytYS=9}ik1`kA;YD{Sl!?}Ezt(k0&dtc z5s25uh-~pBK?Jy3mJpaQR;-Qbw#CEYq_77CS^~Vw&w}Kbr%FxIuc|y2^VBops8Rzt zPmh}<(lo6p0iK_?b#tPrpv=+R$R#wc8X2QBj2~7AHcH%&7I~%uV!_32TaAZh0D61UWnHIV0+Z=7JcEQ^ZhKwW`;l z;*#zDitWm`8S56lJ`pKo3Rx;?T$hE(_ce?p>}@`~;-v{fVitg2gjS%tu)ytz! { {isDesktop && } {isDesktop && } + {isDesktop && } {isDesktop && config.enableStarRating && ( { size: , updatedAt: , playCount: , + bpm: , comment: , } if (!record.discSubtitle) { @@ -40,6 +47,9 @@ export const SongDetails = (props) => { if (!record.comment) { delete data.comment } + if (!record.bpm) { + delete data.bpm + } if (record.playCount > 0) { data.playDate = } diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index a957c047..c6904499 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -22,7 +22,8 @@ "starred": "Favourite", "rating": "Rating", "comment": "Comment", - "quality": "Quality" + "quality": "Quality", + "bpm": "BPM" }, "actions": { "addToQueue": "Play Later", diff --git a/ui/src/playlist/PlaylistSongs.js b/ui/src/playlist/PlaylistSongs.js index 4d202d51..fbf6c901 100644 --- a/ui/src/playlist/PlaylistSongs.js +++ b/ui/src/playlist/PlaylistSongs.js @@ -3,6 +3,7 @@ import { BulkActionsToolbar, ListToolbar, TextField, + NumberField, useRefresh, useDataProvider, useNotify, @@ -166,6 +167,7 @@ const PlaylistSongs = ({ playlistId, readOnly, actions, ...props }) => { {isDesktop && } {isDesktop && } + {isDesktop && } { )} {isDesktop && } + {isDesktop && } {config.enableStarRating && (