diff --git a/Dockerfile b/Dockerfile index ad1e2a41c..e8a00f470 100644 --- a/Dockerfile +++ b/Dockerfile @@ -69,8 +69,16 @@ RUN --mount=type=bind,source=. \ set -e xx-go --wrap export CGO_ENABLED=1 + # Native libwebp (gen2brain/webp) uses ebitengine/purego reverse callbacks, + # which purego does not support on 32-bit ARM or x86 and crash with a SIGSEGV + # (issue #5597). Build those arches with the "nodynamic" tag so gen2brain/webp + # is WASM-only and never links the purego path. 64-bit arches keep native libwebp. + BUILD_TAGS=netgo,sqlite_fts5 + if [ "$(xx-info arch)" = "arm" ] || [ "$(xx-info arch)" = "386" ]; then + BUILD_TAGS=${BUILD_TAGS},nodynamic + fi # -latomic is required on 32-bit arm (arm/v6, arm/v7) so SQLite's 64-bit atomics resolve. - go build -tags=netgo,sqlite_fts5 -ldflags="-w -s \ + go build -tags=${BUILD_TAGS} -ldflags="-w -s \ -linkmode=external -extldflags '-latomic' \ -X github.com/navidrome/navidrome/consts.gitSha=${GIT_SHA} \ -X github.com/navidrome/navidrome/consts.gitTag=${GIT_TAG}" \ @@ -159,7 +167,6 @@ ENV ND_MUSICFOLDER=/music ENV ND_DATAFOLDER=/data ENV ND_CONFIGFILE=/data/navidrome.toml ENV ND_PORT=4533 -ENV ND_ENABLEWEBPENCODING=true RUN touch /.nddockerenv EXPOSE ${ND_PORT} diff --git a/core/artwork/e2e/album_test.go b/core/artwork/e2e/album_test.go index e765e1b1b..5e61684cc 100644 --- a/core/artwork/e2e/album_test.go +++ b/core/artwork/e2e/album_test.go @@ -357,6 +357,98 @@ var _ = Describe("Album artwork resolution", func() { }) }) + // Regression introduced in v0.62.0 (#5451 + #5457): the parent-folder + // fallback can pick up images from the ARTIST folder, serving the artist + // thumbnail as album art for any album without its own image files. + When("an album has no images and the artist folder has folder.jpg", func() { + // Artist/ + // ├── folder.jpg ← artist thumbnail, must NOT become album art + // ├── Album A/ + // │ └── 01 - Track.mp3 (no images) + // └── Album B/ + // ├── 01 - Track.mp3 + // └── cover.jpg + It("does not use the artist image as album art", func() { + conf.Server.CoverArtPriority = defaultCoverPriority + setLayout(fstest.MapFS{ + "Artist/folder.jpg": imageFile("artist-thumbnail"), + "Artist/Album A/01 - Track.mp3": trackFile(1, "Track A", map[string]any{"album": "Album A", "albumartist": "Artist"}), + "Artist/Album B/01 - Track.mp3": trackFile(1, "Track B", map[string]any{"album": "Album B", "albumartist": "Artist"}), + "Artist/Album B/cover.jpg": imageFile("album-b"), + }) + scan() + + alA := albumByName("Album A") + _, err := readArtworkOrErr(alA.CoverArtID()) + Expect(err).To(HaveOccurred(), + "Album A has no images of its own, so it must fall through to the placeholder "+ + "instead of inheriting the artist folder's folder.jpg") + + alB := albumByName("Album B") + Expect(readArtwork(alB.CoverArtID())).To(Equal(imageBytes("album-b"))) + }) + }) + + When("a single-disc album is spread across sibling folders under the artist folder", func() { + // Artist/ + // ├── folder.jpg ← artist thumbnail, must NOT become album art + // ├── Album A/ + // │ └── 01 - Track.mp3 (album: "Album A") + // ├── Album A bonus/ + // │ └── 02 - Track.mp3 (album: "Album A" — same album, second folder) + // └── Album B/ + // ├── 01 - Track.mp3 + // └── cover.jpg + It("does not use the artist image as album art for the spread album", func() { + conf.Server.CoverArtPriority = defaultCoverPriority + setLayout(fstest.MapFS{ + "Artist/folder.jpg": imageFile("artist-thumbnail"), + "Artist/Album A/01 - Track.mp3": trackFile(1, "Track A1", map[string]any{"album": "Album A", "albumartist": "Artist"}), + "Artist/Album A bonus/02 - Track.mp3": trackFile(2, "Track A2", map[string]any{"album": "Album A", "albumartist": "Artist"}), + "Artist/Album B/01 - Track.mp3": trackFile(1, "Track B", map[string]any{"album": "Album B", "albumartist": "Artist"}), + "Artist/Album B/cover.jpg": imageFile("album-b"), + }) + scan() + + alA := albumByName("Album A") + Expect(alA.FolderIDs).To(HaveLen(2), + "sanity check: scanner should treat the two sibling folders as one spread album") + _, err := readArtworkOrErr(alA.CoverArtID()) + Expect(err).To(HaveOccurred(), + "the spread album has no images of its own, so it must fall through to the "+ + "placeholder instead of inheriting the artist folder's folder.jpg") + }) + }) + + When("a spread album has its own front.jpg but the artist folder has cover.jpg", func() { + // Artist/ + // ├── cover.jpg ← artist image; matches cover.* (first pattern), + // │ must NOT shadow the album's own front.jpg + // ├── Album A/ + // │ ├── 01 - Track.mp3 (album: "Album A") + // │ └── front.jpg ← should win + // ├── Album A bonus/ + // │ └── 02 - Track.mp3 (album: "Album A") + // └── Album B/ + // └── 01 - Track.mp3 + It("prefers the album's own art over the artist image", func() { + conf.Server.CoverArtPriority = defaultCoverPriority + setLayout(fstest.MapFS{ + "Artist/cover.jpg": imageFile("artist-image"), + "Artist/Album A/01 - Track.mp3": trackFile(1, "Track A1", map[string]any{"album": "Album A", "albumartist": "Artist"}), + "Artist/Album A/front.jpg": imageFile("album-a-front"), + "Artist/Album A bonus/02 - Track.mp3": trackFile(2, "Track A2", map[string]any{"album": "Album A", "albumartist": "Artist"}), + "Artist/Album B/01 - Track.mp3": trackFile(1, "Track B", map[string]any{"album": "Album B", "albumartist": "Artist"}), + }) + scan() + + alA := albumByName("Album A") + Expect(alA.FolderIDs).To(HaveLen(2), + "sanity check: scanner should treat the two sibling folders as one spread album") + Expect(readArtwork(alA.CoverArtID())).To(Equal(imageBytes("album-a-front"))) + }) + }) + When("embedded is first in CoverArtPriority but the track has no embedded art", func() { // Artist/ // └── Album/ diff --git a/core/artwork/e2e/suite_test.go b/core/artwork/e2e/suite_test.go index 733e2e98c..06cc05b6f 100644 --- a/core/artwork/e2e/suite_test.go +++ b/core/artwork/e2e/suite_test.go @@ -2,6 +2,7 @@ package artworke2e_test import ( "context" + "fmt" "path/filepath" "testing" @@ -104,3 +105,16 @@ func firstAlbum() model.Album { Expect(albums).To(HaveLen(1), "expected exactly one album, got %d", len(albums)) return albums[0] } + +func albumByName(name string) model.Album { + GinkgoHelper() + albums, err := ds.Album(ctx).GetAll(model.QueryOptions{}) + Expect(err).ToNot(HaveOccurred()) + for _, al := range albums { + if al.Name == name { + return al + } + } + Fail(fmt.Sprintf("album %q not found among %d albums", name, len(albums))) + return model.Album{} +} diff --git a/core/artwork/reader_album.go b/core/artwork/reader_album.go index 73ba9b5ee..8ad07773b 100644 --- a/core/artwork/reader_album.go +++ b/core/artwork/reader_album.go @@ -113,28 +113,12 @@ func loadAlbumFoldersPaths(ctx context.Context, ds model.DataStore, albums ...mo return nil, nil, nil, err } - folderIDSet := make(map[string]bool, len(folderIDs)) - for _, id := range folderIDs { - folderIDSet[id] = true + parent, err := albumRootParent(ctx, ds, folders, folderIDs) + if err != nil { + return nil, nil, nil, err } - - // Check if all folders share a common parent that is not already included. - // This finds cover art in the album root folder (e.g., "Artist/Album/cover.jpg" - // when tracks are in disc subfolders like "Artist/Album/CD1/" and "Artist/Album/CD2/"). - // For single-folder albums, the parent is only included when the folder has no - // images of its own (indicating a disc subfolder needing parent artwork). - if commonParentID := commonParentFolder(folders, folderIDSet); commonParentID != "" { - if len(folders) >= 2 || !anyFolderHasImages(folders) { - parentFolder, err := ds.Folder(ctx).Get(commonParentID) - if errors.Is(err, model.ErrNotFound) { - log.Warn(ctx, "Parent folder not found for album cover art lookup", "parentID", commonParentID) - } else if err != nil { - return nil, nil, nil, err - } - if parentFolder != nil && parentFolder.ParentID != "" { - folders = append(folders, *parentFolder) - } - } + if parent != nil { + folders = append(folders, *parent) } var paths []string @@ -159,6 +143,50 @@ func loadAlbumFoldersPaths(ctx context.Context, ds model.DataStore, albums ...mo return paths, imgFiles, &updatedAt, nil } +// albumRootParent returns the common parent of the album's folders when it +// qualifies as the album's root folder (e.g. "Artist/Album" above disc +// subfolders), or nil when there is no such parent. This finds cover art in +// the album root folder when tracks live in disc subfolders, like +// "Artist/Album/cover.jpg" with tracks in "Artist/Album/CD1/" and +// "Artist/Album/CD2/". The parent must look like an album root, not an +// artist-level folder — it qualifies only when it holds no audio belonging to +// other albums — so artist images are never served as album art. +func albumRootParent(ctx context.Context, ds model.DataStore, folders []model.Folder, folderIDs []string) (*model.Folder, error) { + folderIDSet := make(map[string]bool, len(folderIDs)) + for _, id := range folderIDs { + folderIDSet[id] = true + } + commonParentID := commonParentFolder(folders, folderIDSet) + if commonParentID == "" { + return nil, nil + } + // Single-folder albums only use the parent when the folder has no images + // of its own (indicating a disc subfolder needing parent artwork). + if len(folders) < 2 && anyFolderHasImages(folders) { + return nil, nil + } + parent, err := ds.Folder(ctx).Get(commonParentID) + if errors.Is(err, model.ErrNotFound) { + log.Warn(ctx, "Parent folder not found for album cover art lookup", "parentID", commonParentID) + return nil, nil + } + if err != nil { + return nil, err + } + if parent.ParentID == "" { + // The library root can never be an album root + return nil, nil + } + hasOtherAudio, err := ds.Folder(ctx).HasAudioOutsideFolders(*parent, folderIDs) + if err != nil { + return nil, err + } + if hasOtherAudio { + return nil, nil + } + return parent, nil +} + func anyFolderHasImages(folders []model.Folder) bool { for _, f := range folders { if len(f.ImageFiles) > 0 { diff --git a/core/artwork/reader_album_test.go b/core/artwork/reader_album_test.go index 1cf039bee..fe4a1a545 100644 --- a/core/artwork/reader_album_test.go +++ b/core/artwork/reader_album_test.go @@ -339,6 +339,61 @@ var _ = Describe("Album Artwork Reader", func() { Expect(repo.getCallCount).To(Equal(1)) }) + It("does not include parent images when other albums' audio lives under the parent", func() { + // Simulates: Artist/folder.jpg with Artist/Album (no images) and + // another album's tracks elsewhere under the artist folder + repo.result = []model.Folder{ + { + ID: "folder1", + Path: "Artist", + Name: "Album", + ParentID: "artistFolder", + ImagesUpdatedAt: now, + ImageFiles: []string{}, + }, + } + repo.parentResult = &model.Folder{ + ID: "artistFolder", + Path: ".", + Name: "Artist", + ParentID: "libraryRoot", + ImagesUpdatedAt: expectedAt, + ImageFiles: []string{"folder.jpg"}, + } + repo.hasOtherAudio = true + + _, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album) + + Expect(err).ToNot(HaveOccurred()) + Expect(imgFiles).To(BeEmpty()) + }) + + It("propagates errors from the album-root check", func() { + repo.result = []model.Folder{ + { + ID: "folder1", + Path: "Artist/Album", + Name: "disc1", + ParentID: "albumFolder", + ImagesUpdatedAt: now, + ImageFiles: []string{}, + }, + } + repo.parentResult = &model.Folder{ + ID: "albumFolder", + Path: "Artist", + Name: "Album", + ParentID: "artistFolder", + ImagesUpdatedAt: expectedAt, + ImageFiles: []string{"cover.jpg"}, + } + repo.otherAudioErr = errors.New("db connection failed") + + _, _, _, err := loadAlbumFoldersPaths(ctx, ds, album) + + Expect(err).To(MatchError("db connection failed")) + }) + It("propagates non-ErrNotFound errors from parent folder lookup", func() { repo.result = []model.Folder{ { diff --git a/core/artwork/reader_artist_test.go b/core/artwork/reader_artist_test.go index 50ca3a2ce..6d6d58fc5 100644 --- a/core/artwork/reader_artist_test.go +++ b/core/artwork/reader_artist_test.go @@ -702,12 +702,20 @@ type fakeFolderRepo struct { getErr error getCallCount int err error + // hasOtherAudio is returned by HasAudioOutsideFolders (the album-root + // check). False means the parent qualifies as an album root. + hasOtherAudio bool + otherAudioErr error } func (f *fakeFolderRepo) GetAll(...model.QueryOptions) ([]model.Folder, error) { return f.result, f.err } +func (f *fakeFolderRepo) HasAudioOutsideFolders(model.Folder, []string) (bool, error) { + return f.hasOtherAudio, f.otherAudioErr +} + func (f *fakeFolderRepo) Get(id string) (*model.Folder, error) { f.getCallCount++ if f.getErr != nil { diff --git a/core/artwork/reader_resized.go b/core/artwork/reader_resized.go index 85a19a4c3..08f42f130 100644 --- a/core/artwork/reader_resized.go +++ b/core/artwork/reader_resized.go @@ -21,6 +21,12 @@ import ( func init() { conf.AddHook(func() { + // gen2brain/webp selects native (purego/libwebp) vs WASM in its own + // package init() and exposes the result only via webp.Dynamic(); there is + // no runtime way to switch back. On 32-bit ARM/x86 the purego callback path + // crashes (issue #5597), so those builds must be compiled with the + // "nodynamic" tag (see Dockerfile), which makes webp.Dynamic() report an + // error here and forces the safe WASM path. if err := webp.Dynamic(); err != nil { log.Debug("Using WASM WebP encoder/decoder", "reason", err) } else { diff --git a/core/stream/decider.go b/core/stream/decider.go index d6e48497c..7940c6862 100644 --- a/core/stream/decider.go +++ b/core/stream/decider.go @@ -12,6 +12,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/utils/gg" ) const fallbackBitrate = 256 // kbps @@ -142,7 +143,7 @@ func buildSourceStream(mf *model.MediaFile, probe *ffmpeg.AudioProbeResult) Deta sd.Codec = mf.AudioCodec() sd.Bitrate = mf.BitRate sd.SampleRate = mf.SampleRate - sd.BitDepth = mf.BitDepth + sd.BitDepth = gg.V(mf.BitDepth) sd.Channels = mf.Channels } sd.IsLossless = isLosslessFormat(sd.Codec) diff --git a/core/stream/decider_test.go b/core/stream/decider_test.go index f74953258..03c4ea437 100644 --- a/core/stream/decider_test.go +++ b/core/stream/decider_test.go @@ -12,6 +12,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/tests" + "github.com/navidrome/navidrome/utils/gg" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -23,7 +24,7 @@ func withProbe(mf *model.MediaFile) *model.MediaFile { Codec: mf.AudioCodec(), BitRate: mf.BitRate, SampleRate: mf.SampleRate, - BitDepth: mf.BitDepth, + BitDepth: gg.V(mf.BitDepth), Channels: mf.Channels, } data, _ := json.Marshal(probe) @@ -243,7 +244,7 @@ var _ = Describe("Decider", func() { Context("Transcoding", func() { It("selects transcoding when direct play isn't possible", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 256, // kbps DirectPlayProfiles: []DirectPlayProfile{ @@ -278,7 +279,7 @@ var _ = Describe("Decider", func() { }) It("uses default bitrate when client doesn't specify", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, BitDepth: new(16)}) ci := &ClientInfo{ TranscodingProfiles: []Profile{ {Container: "mp3", Protocol: ProtocolHTTP}, @@ -331,7 +332,7 @@ var _ = Describe("Decider", func() { }) It("selects first valid transcoding profile in order", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 48000, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 48000, BitDepth: new(16)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 320, DirectPlayProfiles: []DirectPlayProfile{ @@ -351,7 +352,7 @@ var _ = Describe("Decider", func() { Context("Lossless to lossless transcoding", func() { It("allows lossless to lossless when samplerate needs downsampling", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "dsf", Codec: "DSD", BitRate: 5644, Channels: 2, SampleRate: 176400, BitDepth: 1}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "dsf", Codec: "DSD", BitRate: 5644, Channels: 2, SampleRate: 176400, BitDepth: new(1)}) ci := &ClientInfo{ MaxAudioBitrate: 1000, DirectPlayProfiles: []DirectPlayProfile{ @@ -369,7 +370,7 @@ var _ = Describe("Decider", func() { It("sets IsLossless=true on transcoded stream when target is lossless", func() { // Transcoding to mp3 (lossy) should result in IsLossless=false. - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 96000, BitDepth: 24}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 96000, BitDepth: new(24)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 320, TranscodingProfiles: []Profile{ @@ -526,7 +527,7 @@ var _ = Describe("Decider", func() { }) It("rejects direct play due to samplerate limitation", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 96000, BitDepth: 24}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 96000, BitDepth: new(24)}) ci := &ClientInfo{ DirectPlayProfiles: []DirectPlayProfile{ {Containers: []string{"flac"}, Protocols: []string{ProtocolHTTP}}, @@ -573,7 +574,7 @@ var _ = Describe("Decider", func() { }) It("applies channel limitation to transcoded stream", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 48000, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 48000, BitDepth: new(16)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 320, TranscodingProfiles: []Profile{ @@ -596,7 +597,7 @@ var _ = Describe("Decider", func() { }) It("applies samplerate limitation to transcoded stream", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 96000, BitDepth: 24}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 96000, BitDepth: new(24)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 320, TranscodingProfiles: []Profile{ @@ -619,7 +620,7 @@ var _ = Describe("Decider", func() { }) It("applies bitdepth limitation to transcoded stream", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 96000, BitDepth: 24}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 96000, BitDepth: new(24)}) ci := &ClientInfo{ TranscodingProfiles: []Profile{ {Container: "flac", AudioCodec: "flac", Protocol: ProtocolHTTP}, @@ -642,7 +643,7 @@ var _ = Describe("Decider", func() { }) It("preserves source bit depth when no limitation applies", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 24}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(24)}) ci := &ClientInfo{ TranscodingProfiles: []Profile{ {Container: "flac", AudioCodec: "flac", Protocol: ProtocolHTTP}, @@ -656,7 +657,7 @@ var _ = Describe("Decider", func() { }) It("rejects transcoding profile when GreaterThanEqual cannot be satisfied", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 320, TranscodingProfiles: []Profile{ @@ -680,7 +681,7 @@ var _ = Describe("Decider", func() { Context("DSD sample rate conversion", func() { It("converts DSD sample rate to PCM-equivalent in decision", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "dsf", Codec: "DSD", BitRate: 5644, Channels: 2, SampleRate: 2822400, BitDepth: 1}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "dsf", Codec: "DSD", BitRate: 5644, Channels: 2, SampleRate: 2822400, BitDepth: new(1)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 320, TranscodingProfiles: []Profile{ @@ -700,7 +701,7 @@ var _ = Describe("Decider", func() { }) It("converts DSD sample rate for FLAC target without codec limit", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "dsf", Codec: "DSD", BitRate: 5644, Channels: 2, SampleRate: 2822400, BitDepth: 1}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "dsf", Codec: "DSD", BitRate: 5644, Channels: 2, SampleRate: 2822400, BitDepth: new(1)}) ci := &ClientInfo{ TranscodingProfiles: []Profile{ {Container: "flac", AudioCodec: "flac", Protocol: ProtocolHTTP}, @@ -719,7 +720,7 @@ var _ = Describe("Decider", func() { }) It("applies codec profile limit to DSD-converted FLAC sample rate", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "dsf", Codec: "DSD", BitRate: 5644, Channels: 2, SampleRate: 2822400, BitDepth: 1}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "dsf", Codec: "DSD", BitRate: 5644, Channels: 2, SampleRate: 2822400, BitDepth: new(1)}) ci := &ClientInfo{ TranscodingProfiles: []Profile{ {Container: "flac", AudioCodec: "flac", Protocol: ProtocolHTTP}, @@ -746,7 +747,7 @@ var _ = Describe("Decider", func() { }) It("applies audioBitdepth limitation to DSD-converted bit depth", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "dsf", Codec: "DSD", BitRate: 5644, Channels: 2, SampleRate: 2822400, BitDepth: 1}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "dsf", Codec: "DSD", BitRate: 5644, Channels: 2, SampleRate: 2822400, BitDepth: new(1)}) ci := &ClientInfo{ TranscodingProfiles: []Profile{ {Container: "flac", AudioCodec: "flac", Protocol: ProtocolHTTP}, @@ -775,7 +776,7 @@ var _ = Describe("Decider", func() { // Regression test for #5336: ffmpeg's mp3 encoder rejects >2 channels. // The decider must clamp to the codec's hard limit even when no // transcoding profile MaxAudioChannels is configured. - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: new(16)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 320, TranscodingProfiles: []Profile{ @@ -791,7 +792,7 @@ var _ = Describe("Decider", func() { }) It("honors a stricter profile MaxAudioChannels over the codec clamp", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: new(16)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 320, TranscodingProfiles: []Profile{ @@ -806,7 +807,7 @@ var _ = Describe("Decider", func() { }) It("applies the codec clamp when the profile limit is looser", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: new(16)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 320, TranscodingProfiles: []Profile{ @@ -821,7 +822,7 @@ var _ = Describe("Decider", func() { }) It("passes channels through unchanged for codecs with no hard limit", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: new(16)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 320, TranscodingProfiles: []Profile{ @@ -840,7 +841,7 @@ var _ = Describe("Decider", func() { Context("Probe-based lossless detection", func() { It("uses probe codec name for lossless detection", func() { // WavPack files: ffprobe reports codec as "wavpack", suffix is ".wv" - mf := &model.MediaFile{ID: "1", Suffix: "wv", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16} + mf := &model.MediaFile{ID: "1", Suffix: "wv", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)} probe := ffmpeg.AudioProbeResult{ Codec: "wavpack", BitRate: 1000, SampleRate: 44100, BitDepth: 16, Channels: 2, } @@ -884,7 +885,7 @@ var _ = Describe("Decider", func() { Context("Opus fixed sample rate", func() { It("sets Opus output to 48000Hz regardless of input", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 128, TranscodingProfiles: []Profile{ @@ -901,7 +902,7 @@ var _ = Describe("Decider", func() { }) It("sets Opus output to 48000Hz even for 96kHz input", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1500, Channels: 2, SampleRate: 96000, BitDepth: 24}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1500, Channels: 2, SampleRate: 96000, BitDepth: new(24)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 128, TranscodingProfiles: []Profile{ @@ -917,7 +918,7 @@ var _ = Describe("Decider", func() { Context("Container vs format separation", func() { It("preserves mp4 container when falling back to aac format", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 256, TranscodingProfiles: []Profile{ @@ -935,7 +936,7 @@ var _ = Describe("Decider", func() { }) It("uses container as format when container matches transcoding config", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 256, TranscodingProfiles: []Profile{ @@ -952,7 +953,7 @@ var _ = Describe("Decider", func() { Context("MP3 max sample rate", func() { It("caps sample rate at 48000 for MP3", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1500, Channels: 2, SampleRate: 96000, BitDepth: 24}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1500, Channels: 2, SampleRate: 96000, BitDepth: new(24)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 320, TranscodingProfiles: []Profile{ @@ -966,7 +967,7 @@ var _ = Describe("Decider", func() { }) It("preserves sample rate at 44100 for MP3", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 320, TranscodingProfiles: []Profile{ @@ -982,7 +983,7 @@ var _ = Describe("Decider", func() { Context("AAC max sample rate", func() { It("caps sample rate at 96000 for AAC", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "dsf", Codec: "DSD", BitRate: 5644, Channels: 2, SampleRate: 2822400, BitDepth: 1}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "dsf", Codec: "DSD", BitRate: 5644, Channels: 2, SampleRate: 2822400, BitDepth: new(1)}) ci := &ClientInfo{ MaxTranscodingAudioBitrate: 320, TranscodingProfiles: []Profile{ @@ -1025,7 +1026,7 @@ var _ = Describe("Decider", func() { Context("Source stream details", func() { It("populates source stream correctly with kbps bitrate", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 96000, BitDepth: 24, Duration: 300.5, Size: 50000000}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 96000, BitDepth: new(24), Duration: 300.5, Size: 50000000}) ci := &ClientInfo{ DirectPlayProfiles: []DirectPlayProfile{ {Containers: []string{"flac"}, Protocols: []string{ProtocolHTTP}}, @@ -1058,7 +1059,7 @@ var _ = Describe("Decider", func() { }) It("ignores player MaxBitRate in context", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) ci := &ClientInfo{ Name: "TestClient", DirectPlayProfiles: []DirectPlayProfile{ @@ -1074,7 +1075,7 @@ var _ = Describe("Decider", func() { Context("Format-aware default bitrate", func() { It("uses opus default bitrate from DB", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 48000, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 48000, BitDepth: new(16)}) ci := &ClientInfo{ TranscodingProfiles: []Profile{ {Container: "opus", AudioCodec: "opus", Protocol: ProtocolHTTP}, @@ -1087,7 +1088,7 @@ var _ = Describe("Decider", func() { }) It("uses aac default bitrate from DB", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) ci := &ClientInfo{ TranscodingProfiles: []Profile{ {Container: "aac", AudioCodec: "aac", Protocol: ProtocolHTTP}, diff --git a/core/stream/legacy_client_test.go b/core/stream/legacy_client_test.go index ce7b38650..2ddd74ce0 100644 --- a/core/stream/legacy_client_test.go +++ b/core/stream/legacy_client_test.go @@ -138,7 +138,7 @@ var _ = Describe("ResolveRequest", func() { }) It("transcodes to requested format", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) decider := svc.(*deciderService) req := decider.ResolveRequest(ctx, mf, "opus", 0, 0) @@ -147,7 +147,7 @@ var _ = Describe("ResolveRequest", func() { }) It("transcodes to requested format with bitrate limit", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) decider := svc.(*deciderService) req := decider.ResolveRequest(ctx, mf, "mp3", 128, 0) @@ -169,7 +169,7 @@ var _ = Describe("ResolveRequest", func() { DeferCleanup(configtest.SetupConfig()) conf.Server.DefaultDownsamplingFormat = "opus" - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) decider := svc.(*deciderService) req := decider.ResolveRequest(ctx, mf, "", 128, 0) @@ -179,7 +179,7 @@ var _ = Describe("ResolveRequest", func() { }) It("passes offset through", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) decider := svc.(*deciderService) req := decider.ResolveRequest(ctx, mf, "opus", 128, 30) @@ -259,7 +259,7 @@ var _ = Describe("ResolveRequest", func() { Context("Player MaxBitRate cap", func() { It("applies player MaxBitRate cap when client has no limit", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) playerCtx := request.WithPlayer(ctx, model.Player{MaxBitRate: 320}) decider := svc.(*deciderService) @@ -270,7 +270,7 @@ var _ = Describe("ResolveRequest", func() { }) It("uses client limit when it is more restrictive than player MaxBitRate", func() { - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) playerCtx := request.WithPlayer(ctx, model.Player{MaxBitRate: 500}) decider := svc.(*deciderService) @@ -332,7 +332,7 @@ var _ = Describe("ResolveRequest", func() { DeferCleanup(configtest.SetupConfig()) conf.Server.DefaultDownsamplingFormat = "opus" - mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16}) + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) decider := svc.(*deciderService) req := decider.ResolveRequest(ctx, mf, "xyz", 128, 0) diff --git a/db/migrations/20260612222838_make_bpm_bitdepth_nullable.sql b/db/migrations/20260612222838_make_bpm_bitdepth_nullable.sql new file mode 100644 index 000000000..c84323158 --- /dev/null +++ b/db/migrations/20260612222838_make_bpm_bitdepth_nullable.sql @@ -0,0 +1,35 @@ +-- +goose Up +drop index if exists media_file_bpm; + +alter table media_file add column bpm_new integer; +alter table media_file add column bit_depth_new integer; + +update media_file set + bpm_new = nullif(bpm, 0), + bit_depth_new = nullif(bit_depth, 0); + +alter table media_file drop column bpm; +alter table media_file drop column bit_depth; + +alter table media_file rename column bpm_new to bpm; +alter table media_file rename column bit_depth_new to bit_depth; + +create index if not exists media_file_bpm on media_file (bpm); + +-- +goose Down +drop index if exists media_file_bpm; + +alter table media_file add column bpm_old integer default 0 not null; +alter table media_file add column bit_depth_old integer default 0 not null; + +update media_file set + bpm_old = coalesce(bpm, 0), + bit_depth_old = coalesce(bit_depth, 0); + +alter table media_file drop column bpm; +alter table media_file drop column bit_depth; + +alter table media_file rename column bpm_old to bpm; +alter table media_file rename column bit_depth_old to bit_depth; + +create index if not exists media_file_bpm on media_file (bpm); diff --git a/model/criteria/fields.go b/model/criteria/fields.go index 36206712f..5c9ec898d 100644 --- a/model/criteria/fields.go +++ b/model/criteria/fields.go @@ -4,12 +4,14 @@ import "strings" // FieldInfo contains semantic metadata about a criteria field. type FieldInfo struct { - Alias string // If set, this field is a backward-compat alias for another canonical name - IsTag bool - IsRole bool - Numeric bool - Boolean bool - Nullable bool // If set, this column field can be NULL, so isMissing/isPresent are supported on it + Alias string // If set, this field is a backward-compat alias for another canonical name + IsTag bool + IsRole bool + Numeric bool + Boolean bool + // Nullable: isMissing/isPresent are supported on this column field. For numeric/boolean + // fields, missing means NULL; for string fields it means NULL or empty string. + Nullable bool tagAlias string // If set, a tag name from mappings.yaml that resolves to this field name string // Canonical name, populated by LookupField from the map key @@ -22,7 +24,7 @@ func (f FieldInfo) Name() string { var fieldMap = map[string]FieldInfo{ "title": {}, - "album": {}, + "album": {Nullable: true}, "hascoverart": {Boolean: true}, "tracknumber": {}, "discnumber": {}, @@ -35,26 +37,26 @@ var fieldMap = map[string]FieldInfo{ "size": {}, "compilation": {Boolean: true}, "missing": {Boolean: true}, - "explicitstatus": {}, + "explicitstatus": {Nullable: true}, "dateadded": {}, "datemodified": {}, - "discsubtitle": {}, - "comment": {}, - "lyrics": {}, - "sorttitle": {}, - "sortalbum": {}, - "sortartist": {}, - "sortalbumartist": {}, - "albumcomment": {}, - "catalognumber": {}, + "discsubtitle": {Nullable: true}, + "comment": {Nullable: true}, + "lyrics": {Nullable: true}, + "sorttitle": {Nullable: true}, + "sortalbum": {Nullable: true}, + "sortartist": {Nullable: true}, + "sortalbumartist": {Nullable: true}, + "albumcomment": {Nullable: true}, + "catalognumber": {Nullable: true}, "filepath": {}, "filetype": {}, "codec": {}, "duration": {}, "bitrate": {}, - "bitdepth": {}, + "bitdepth": {Numeric: true, Nullable: true}, "samplerate": {}, - "bpm": {}, + "bpm": {Numeric: true, Nullable: true}, "channels": {}, "loved": {Boolean: true}, "dateloved": {}, @@ -75,12 +77,12 @@ var fieldMap = map[string]FieldInfo{ "artistlastplayed": {}, "artistdateloved": {}, "artistdaterated": {}, - "mbz_album_id": {}, - "mbz_album_artist_id": {}, - "mbz_artist_id": {}, - "mbz_recording_id": {}, - "mbz_release_track_id": {}, - "mbz_release_group_id": {}, + "mbz_album_id": {Nullable: true}, + "mbz_album_artist_id": {Nullable: true}, + "mbz_artist_id": {Nullable: true}, + "mbz_recording_id": {Nullable: true}, + "mbz_release_track_id": {Nullable: true}, + "mbz_release_group_id": {Nullable: true}, "rgalbumgain": {Numeric: true, Nullable: true}, "rgalbumpeak": {Numeric: true, Nullable: true}, "rgtrackgain": {Numeric: true, Nullable: true}, diff --git a/model/criteria/fields_test.go b/model/criteria/fields_test.go index 3dc0c7b90..2367101a8 100644 --- a/model/criteria/fields_test.go +++ b/model/criteria/fields_test.go @@ -75,5 +75,17 @@ var _ = Describe("fields", func() { gomega.Expect(field.IsTag).To(gomega.BeFalse()) }) + It("marks mbz_* and lyrics string fields as nullable (empty means missing)", func() { + for _, name := range []string{"mbz_album_id", "mbz_album_artist_id", "mbz_artist_id", + "mbz_recording_id", "mbz_release_track_id", "mbz_release_group_id", "lyrics", + "album", "comment", "catalognumber", "discsubtitle", "albumcomment", + "sorttitle", "sortalbum", "sortartist", "sortalbumartist", "explicitstatus"} { + field, ok := LookupField(name) + gomega.Expect(ok).To(gomega.BeTrue(), name) + gomega.Expect(field.Nullable).To(gomega.BeTrue(), name) + gomega.Expect(field.Numeric).To(gomega.BeFalse(), name) + } + }) + }) }) diff --git a/model/folder.go b/model/folder.go index 7a769735e..39cb6db84 100644 --- a/model/folder.go +++ b/model/folder.go @@ -86,6 +86,10 @@ type FolderRepository interface { GetAll(...QueryOptions) ([]Folder, error) CountAll(...QueryOptions) (int64, error) GetFolderUpdateInfo(lib Library, targetPaths ...string) (map[string]FolderUpdateInfo, error) + // HasAudioOutsideFolders reports whether any folder in parent's subtree + // (including parent itself) contains audio files and is not one of the + // given folder IDs. + HasAudioOutsideFolders(parent Folder, excludeFolderIDs []string) (bool, error) Put(*Folder) error MarkMissing(missing bool, ids ...string) error GetTouchedWithPlaylists() (FolderCursor, error) diff --git a/model/mediafile.go b/model/mediafile.go index 6be8402ae..718f0443d 100644 --- a/model/mediafile.go +++ b/model/mediafile.go @@ -16,6 +16,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/gg" "github.com/navidrome/navidrome/utils/slice" ) @@ -54,7 +55,7 @@ type MediaFile struct { Duration float32 `structs:"duration" json:"duration"` BitRate int `structs:"bit_rate" json:"bitRate"` SampleRate int `structs:"sample_rate" json:"sampleRate"` - BitDepth int `structs:"bit_depth" json:"bitDepth"` + BitDepth *int `structs:"bit_depth" json:"bitDepth,omitempty"` Channels int `structs:"channels" json:"channels"` Codec string `structs:"codec" json:"codec"` ProbeData string `structs:"probe_data" json:"-" hash:"ignore"` @@ -71,7 +72,7 @@ type MediaFile struct { Compilation bool `structs:"compilation" json:"compilation"` Comment string `structs:"comment" json:"comment,omitempty"` Lyrics string `structs:"lyrics" json:"lyrics"` - BPM int `structs:"bpm" json:"bpm,omitempty"` + BPM *int `structs:"bpm" json:"bpm,omitempty"` ExplicitStatus string `structs:"explicit_status" json:"explicitStatus"` CatalogNum string `structs:"catalog_num" json:"catalogNum,omitempty"` MbzRecordingID string `structs:"mbz_recording_id" json:"mbzRecordingID,omitempty"` @@ -225,7 +226,7 @@ func (mf MediaFile) inferCodecFromSuffix() string { return "dsd" case "m4a": // AAC if BitDepth==0, ALAC if BitDepth>0 - if mf.BitDepth > 0 { + if gg.V(mf.BitDepth) > 0 { return "alac" } return "aac" diff --git a/model/mediafile_test.go b/model/mediafile_test.go index 3547ec4ef..65c5a0652 100644 --- a/model/mediafile_test.go +++ b/model/mediafile_test.go @@ -564,7 +564,7 @@ var _ = Describe("MediaFile", func() { DescribeTable("infers codec from suffix when Codec field is empty", func(suffix string, bitDepth int, expected string) { - mf := MediaFile{Suffix: suffix, BitDepth: bitDepth} + mf := MediaFile{Suffix: suffix, BitDepth: new(bitDepth)} Expect(mf.AudioCodec()).To(Equal(expected)) }, Entry("mp3", "mp3", 0, "mp3"), @@ -597,13 +597,30 @@ var _ = Describe("MediaFile", func() { ) It("prefers stored codec over suffix inference", func() { - mf := MediaFile{Codec: "ALAC", Suffix: "m4a", BitDepth: 0} + mf := MediaFile{Codec: "ALAC", Suffix: "m4a"} Expect(mf.AudioCodec()).To(Equal("alac")) }) }) }) +var _ = Describe("MediaFile.Hash", func() { + // Guards the upgrade guarantee: converting BPM/BitDepth from int to *int must not change hashes, + // or every file would be spuriously re-imported on the next scan. + // Golden hashes were captured at 46221d516 when those fields were plain ints. + It("keeps hashes identical to the pre-pointer-conversion values", func() { + // Golden hashes computed at 46221d516, when BPM/BitDepth were plain ints — pinning + // them guarantees the pointer conversion cannot trigger a full-library re-import. + Expect(MediaFile{Title: "Song"}.Hash()).To(Equal("1d856ced42cb96db39e354a4bac9a622")) + Expect(MediaFile{Title: "Song", BPM: new(120), BitDepth: new(16)}.Hash()).To(Equal("b2b0b1d1dd7fd767093588e4af3a0689")) + }) + It("changes the hash when a pointer field has a value", func() { + base := MediaFile{Title: "Song"} + Expect(base.Equals(MediaFile{Title: "Song", BPM: new(120)})).To(BeFalse()) + Expect(base.Equals(MediaFile{Title: "Song", BitDepth: new(24)})).To(BeFalse()) + }) +}) + func t(v string) time.Time { var timeFormats = []string{"2006-01-02", "2006-01-02 15:04", "2006-01-02 15:04:05", "2006-01-02T15:04:05", "2006-01-02T15:04", "2006-01-02 15:04:05.999999999 -0700 MST"} for _, f := range timeFormats { diff --git a/model/metadata/map_mediafile.go b/model/metadata/map_mediafile.go index 824cad7c2..966a545be 100644 --- a/model/metadata/map_mediafile.go +++ b/model/metadata/map_mediafile.go @@ -35,7 +35,11 @@ func (md Metadata) ToMediaFile(libID int, folderID string) model.MediaFile { mf.DiscSubtitle = md.String(model.TagDiscSubtitle) mf.CatalogNum = md.String(model.TagCatalogNumber) mf.Comment = md.String(model.TagComment) - mf.BPM = int(math.Round(md.Float(model.TagBPM))) + if f := md.NullableFloat(model.TagBPM); f != nil { + if v := int(math.Round(*f)); v != 0 { + mf.BPM = new(v) + } + } mf.Lyrics = md.mapLyrics() mf.ExplicitStatus = md.mapExplicitStatusTag() @@ -63,7 +67,9 @@ func (md Metadata) ToMediaFile(libID int, folderID string) model.MediaFile { mf.Duration = md.Length() mf.BitRate = md.AudioProperties().BitRate mf.SampleRate = md.AudioProperties().SampleRate - mf.BitDepth = md.AudioProperties().BitDepth + if bd := md.AudioProperties().BitDepth; bd > 0 { + mf.BitDepth = new(bd) + } mf.Channels = md.AudioProperties().Channels mf.Codec = md.AudioProperties().Codec mf.Path = md.FilePath() diff --git a/model/metadata/map_mediafile_test.go b/model/metadata/map_mediafile_test.go index 16142f526..75a7ed358 100644 --- a/model/metadata/map_mediafile_test.go +++ b/model/metadata/map_mediafile_test.go @@ -117,4 +117,32 @@ var _ = Describe("ToMediaFile", func() { Expect(actual).To(Equal(expected)) }) }) + + Describe("BPM", func() { + It("maps the BPM tag rounded to the nearest integer", func() { + mf = toMediaFile(model.RawTags{"BPM": {"120.6"}}) + Expect(mf.BPM).To(Equal(new(121))) + }) + It("leaves BPM nil when the tag is absent", func() { + mf = toMediaFile(model.RawTags{}) + Expect(mf.BPM).To(BeNil()) + }) + It("leaves BPM nil when the tag is zero or unparseable", func() { + Expect(toMediaFile(model.RawTags{"BPM": {"0"}}).BPM).To(BeNil()) + Expect(toMediaFile(model.RawTags{"BPM": {"fast"}}).BPM).To(BeNil()) + }) + }) + + Describe("BitDepth", func() { + It("maps the bit depth when present", func() { + props.AudioProperties = metadata.AudioProperties{BitDepth: 24} + mf = toMediaFile(model.RawTags{}) + Expect(mf.BitDepth).To(Equal(new(24))) + }) + It("leaves BitDepth nil when zero (lossy codecs have no bit depth)", func() { + props.AudioProperties = metadata.AudioProperties{BitDepth: 0} + mf = toMediaFile(model.RawTags{}) + Expect(mf.BitDepth).To(BeNil()) + }) + }) }) diff --git a/persistence/criteria_sql.go b/persistence/criteria_sql.go index ee0baca18..fa769ef40 100644 --- a/persistence/criteria_sql.go +++ b/persistence/criteria_sql.go @@ -28,9 +28,10 @@ func (j smartPlaylistJoinType) has(other smartPlaylistJoinType) bool { } type smartPlaylistField struct { - expr string - order string - joinType smartPlaylistJoinType + expr string + order string + joinType smartPlaylistJoinType + emptyValues []string // additional values that encode "missing" for string columns (e.g. '[]' for lyrics) } type smartPlaylistCriteria struct { @@ -72,7 +73,7 @@ var smartPlaylistFields = map[string]smartPlaylistField{ "datemodified": {expr: "media_file.updated_at"}, "discsubtitle": {expr: "media_file.disc_subtitle"}, "comment": {expr: "media_file.comment"}, - "lyrics": {expr: "media_file.lyrics"}, + "lyrics": {expr: "media_file.lyrics", emptyValues: []string{"[]"}}, "sorttitle": {expr: "media_file.sort_title"}, "sortalbum": {expr: "media_file.sort_album_name"}, "sortartist": {expr: "media_file.sort_artist_name"}, @@ -218,30 +219,44 @@ func missingExpr(values map[string]any, checkAbsence bool) (squirrel.Sqlizer, er } return nil, fmt.Errorf("invalid field in criteria: %s", field) } - if !info.IsTag && !info.IsRole && !info.Nullable { - return nil, fmt.Errorf("isMissing/isPresent operator is not supported for field: %s", field) - } - b, ok := value.(bool) if !ok { return nil, fmt.Errorf("invalid boolean value for 'missing' expression: %s: %v", field, value) } negate := checkAbsence == b - // Nullable column fields (e.g. ReplayGain) are stored in dedicated columns, not in the tags - // JSON, so "missing" maps to a NULL check on the column rather than a json_tree lookup. - if info.Nullable && !info.IsTag && !info.IsRole { - col, ok := fieldExpr(info.Name()) - if !ok || col == "" { + switch { + case info.IsTag || info.IsRole: + return jsonExpr(info, nil, negate), nil + case info.Nullable: + // Nullable column fields are stored in dedicated columns, not in the tags JSON, so + // "missing" maps to a column check rather than a json_tree lookup. Numeric/boolean + // columns (e.g. ReplayGain, BPM) encode absence as NULL only; string columns (e.g. + // mbz_* IDs, lyrics) additionally treat empty string — and any field-specific empty + // encodings (e.g. '[]' for lyrics) — as missing. The unified flow below handles both: + // numeric/boolean fields simply have no empties, so the loops are no-ops. + f, ok := smartPlaylistFields[info.Name()] + if !ok || f.expr == "" { return nil, fmt.Errorf("invalid field in criteria: %s", field) } - if negate { - return squirrel.Eq{col: nil}, nil + col := f.expr + var empties []string + if !info.Numeric && !info.Boolean { + empties = append([]string{""}, f.emptyValues...) } - return squirrel.NotEq{col: nil}, nil + missing := squirrel.Or{squirrel.Eq{col: nil}} + present := squirrel.And{squirrel.NotEq{col: nil}} + for _, e := range empties { + missing = append(missing, squirrel.Eq{col: e}) + present = append(present, squirrel.NotEq{col: e}) + } + if negate { + return missing, nil + } + return present, nil + default: + return nil, fmt.Errorf("isMissing/isPresent operator is not supported for field: %s", field) } - - return jsonExpr(info, nil, negate), nil } func mapExpr(values map[string]any, makeCond func(map[string]any) squirrel.Sqlizer, negateJSON bool) (squirrel.Sqlizer, error) { diff --git a/persistence/criteria_sql_test.go b/persistence/criteria_sql_test.go index 8e801a703..0257fa0cf 100644 --- a/persistence/criteria_sql_test.go +++ b/persistence/criteria_sql_test.go @@ -87,18 +87,68 @@ var _ = Describe("Smart playlist criteria SQL", func() { "not exists (select 1 from media_file_artists mfa where mfa.media_file_id = media_file.id and mfa.role = ?)", "composer"), // isMissing/isPresent — nullable column fields (ReplayGain) Entry("isMissing rgAlbumGain [true]", criteria.IsMissing{"rgAlbumGain": true}, - "media_file.rg_album_gain IS NULL"), + "(media_file.rg_album_gain IS NULL)"), Entry("isMissing rgAlbumGain [false]", criteria.IsMissing{"rgAlbumGain": false}, - "media_file.rg_album_gain IS NOT NULL"), + "(media_file.rg_album_gain IS NOT NULL)"), Entry("isPresent rgTrackPeak [true]", criteria.IsPresent{"rgTrackPeak": true}, - "media_file.rg_track_peak IS NOT NULL"), + "(media_file.rg_track_peak IS NOT NULL)"), Entry("isPresent rgTrackPeak [false]", criteria.IsPresent{"rgTrackPeak": false}, - "media_file.rg_track_peak IS NULL"), + "(media_file.rg_track_peak IS NULL)"), // isMissing — replaygain_* tag-name alias resolves to the nullable column (issue #5584) Entry("isMissing replaygain_album_gain alias [true]", criteria.IsMissing{"replaygain_album_gain": true}, - "media_file.rg_album_gain IS NULL"), + "(media_file.rg_album_gain IS NULL)"), Entry("isPresent replaygain_album_gain alias [true]", criteria.IsPresent{"replaygain_album_gain": true}, - "media_file.rg_album_gain IS NOT NULL"), + "(media_file.rg_album_gain IS NOT NULL)"), + // isMissing/isPresent — string column fields (empty string means missing) + Entry("isMissing mbz_recording_id [true]", criteria.IsMissing{"mbz_recording_id": true}, + "(media_file.mbz_recording_id IS NULL OR media_file.mbz_recording_id = ?)", ""), + Entry("isMissing mbz_recording_id [false]", criteria.IsMissing{"mbz_recording_id": false}, + "(media_file.mbz_recording_id IS NOT NULL AND media_file.mbz_recording_id <> ?)", ""), + Entry("isPresent mbz_album_id [true]", criteria.IsPresent{"mbz_album_id": true}, + "(media_file.mbz_album_id IS NOT NULL AND media_file.mbz_album_id <> ?)", ""), + Entry("isPresent mbz_album_id [false]", criteria.IsPresent{"mbz_album_id": false}, + "(media_file.mbz_album_id IS NULL OR media_file.mbz_album_id = ?)", ""), + // lyrics: absence is encoded as '' or '[]' (empty serialized LyricList) + Entry("isMissing lyrics [true]", criteria.IsMissing{"lyrics": true}, + "(media_file.lyrics IS NULL OR media_file.lyrics = ? OR media_file.lyrics = ?)", "", "[]"), + Entry("isPresent lyrics [true]", criteria.IsPresent{"lyrics": true}, + "(media_file.lyrics IS NOT NULL AND media_file.lyrics <> ? AND media_file.lyrics <> ?)", "", "[]"), + Entry("isMissing lyrics [false]", criteria.IsMissing{"lyrics": false}, + "(media_file.lyrics IS NOT NULL AND media_file.lyrics <> ? AND media_file.lyrics <> ?)", "", "[]"), + Entry("isPresent lyrics [false]", criteria.IsPresent{"lyrics": false}, + "(media_file.lyrics IS NULL OR media_file.lyrics = ? OR media_file.lyrics = ?)", "", "[]"), + // isMissing/isPresent — nullable numeric columns (BPM, BitDepth) + Entry("isMissing bpm [true]", criteria.IsMissing{"bpm": true}, + "(media_file.bpm IS NULL)"), + Entry("isPresent bpm [true]", criteria.IsPresent{"bpm": true}, + "(media_file.bpm IS NOT NULL)"), + Entry("isMissing bitdepth [true]", criteria.IsMissing{"bitdepth": true}, + "(media_file.bit_depth IS NULL)"), + Entry("isPresent bitdepth [false]", criteria.IsPresent{"bitdepth": false}, + "(media_file.bit_depth IS NULL)"), + // isMissing/isPresent — more string column fields (empty string means missing) + Entry("isMissing album [true]", criteria.IsMissing{"album": true}, + "(media_file.album IS NULL OR media_file.album = ?)", ""), + Entry("isMissing comment [true]", criteria.IsMissing{"comment": true}, + "(media_file.comment IS NULL OR media_file.comment = ?)", ""), + Entry("isMissing catalognumber [true]", criteria.IsMissing{"catalognumber": true}, + "(media_file.catalog_num IS NULL OR media_file.catalog_num = ?)", ""), + Entry("isMissing discsubtitle [true]", criteria.IsMissing{"discsubtitle": true}, + "(media_file.disc_subtitle IS NULL OR media_file.disc_subtitle = ?)", ""), + Entry("isMissing albumcomment [true]", criteria.IsMissing{"albumcomment": true}, + "(media_file.mbz_album_comment IS NULL OR media_file.mbz_album_comment = ?)", ""), + Entry("isMissing sorttitle [true]", criteria.IsMissing{"sorttitle": true}, + "(media_file.sort_title IS NULL OR media_file.sort_title = ?)", ""), + Entry("isMissing sortalbum [true]", criteria.IsMissing{"sortalbum": true}, + "(media_file.sort_album_name IS NULL OR media_file.sort_album_name = ?)", ""), + Entry("isMissing sortartist [true]", criteria.IsMissing{"sortartist": true}, + "(media_file.sort_artist_name IS NULL OR media_file.sort_artist_name = ?)", ""), + Entry("isMissing sortalbumartist [true]", criteria.IsMissing{"sortalbumartist": true}, + "(media_file.sort_album_artist_name IS NULL OR media_file.sort_album_artist_name = ?)", ""), + Entry("isMissing explicitstatus [true]", criteria.IsMissing{"explicitstatus": true}, + "(media_file.explicit_status IS NULL OR media_file.explicit_status = ?)", ""), + Entry("isPresent comment [true]", criteria.IsPresent{"comment": true}, + "(media_file.comment IS NOT NULL AND media_file.comment <> ?)", ""), ) Describe("playlist permissions", func() { diff --git a/persistence/folder_repository.go b/persistence/folder_repository.go index f7bb6a4fe..624ef21b8 100644 --- a/persistence/folder_repository.go +++ b/persistence/folder_repository.go @@ -7,6 +7,7 @@ import ( "iter" "maps" "os" + "path" "path/filepath" "slices" "strings" @@ -188,6 +189,33 @@ func (r folderRepository) queryFolderUpdateInfo(where And) (map[string]model.Fol return m, nil } +// HasAudioOutsideFolders reports whether any folder in parent's subtree +// (including parent itself) contains audio files and is not one of the given +// folder IDs. LIKE wildcards in the parent path are escaped, so it is always +// matched as a literal prefix. +func (r folderRepository) HasAudioOutsideFolders(parent model.Folder, excludeFolderIDs []string) (bool, error) { + if parent.NumAudioFiles > 0 { + return true, nil + } + parentPath := strings.TrimPrefix(path.Join(parent.Path, parent.Name), "/") + return r.exists(And{ + Eq{"library_id": parent.LibraryID, "missing": false}, + Gt{"num_audio_files": 0}, + NotEq{"id": excludeFolderIDs}, + Or{ + // Direct children have path = parentPath; deeper descendants match the prefix + Eq{"path": parentPath}, + Expr(`path LIKE ? ESCAPE '\'`, escapeLikePrefix(parentPath)+"/%"), + }, + }) +} + +// escapeLikePrefix escapes SQL LIKE wildcards so a string can be used as a +// literal prefix in a LIKE pattern (with ESCAPE '\'). +func escapeLikePrefix(s string) string { + return strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`).Replace(s) +} + func (r folderRepository) Put(f *model.Folder) error { dbf := dbFolder{Folder: f} _, err := r.put(dbf.ID, &dbf) diff --git a/persistence/folder_repository_test.go b/persistence/folder_repository_test.go index ebc08fd04..413a6b38f 100644 --- a/persistence/folder_repository_test.go +++ b/persistence/folder_repository_test.go @@ -217,6 +217,67 @@ var _ = Describe("FolderRepository", func() { }) }) + Describe("HasAudioOutsideFolders", func() { + var albumRoot, disc1, disc2 *model.Folder + + // TestHasAudio/Album/ + // ├── CD1/ (audio, belongs to the album) + // └── CD2/ (audio, belongs to the album) + BeforeEach(func() { + albumRoot = model.NewFolder(testLib, "TestHasAudio/Album") + disc1 = model.NewFolder(testLib, "TestHasAudio/Album/CD1") + disc1.NumAudioFiles = 5 + disc2 = model.NewFolder(testLib, "TestHasAudio/Album/CD2") + disc2.NumAudioFiles = 5 + for _, f := range []*model.Folder{albumRoot, disc1, disc2} { + Expect(repo.Put(f)).To(Succeed()) + } + }) + + It("returns false when all audio under the parent belongs to the given folders", func() { + Expect(repo.HasAudioOutsideFolders(*albumRoot, []string{disc1.ID, disc2.ID})).To(BeFalse()) + }) + + It("returns true when another folder under the parent has audio", func() { + bonus := model.NewFolder(testLib, "TestHasAudio/Album/Bonus") + bonus.NumAudioFiles = 1 + Expect(repo.Put(bonus)).To(Succeed()) + + Expect(repo.HasAudioOutsideFolders(*albumRoot, []string{disc1.ID, disc2.ID})).To(BeTrue()) + }) + + It("returns true when the parent itself contains audio files", func() { + albumRoot.NumAudioFiles = 2 + + Expect(repo.HasAudioOutsideFolders(*albumRoot, []string{disc1.ID, disc2.ID})).To(BeTrue()) + }) + + It("ignores audio outside the parent's subtree", func() { + other := model.NewFolder(testLib, "TestHasAudio/Other Album") + other.NumAudioFiles = 10 + Expect(repo.Put(other)).To(Succeed()) + + Expect(repo.HasAudioOutsideFolders(*albumRoot, []string{disc1.ID, disc2.ID})).To(BeFalse()) + }) + + It("ignores missing folders", func() { + gone := model.NewFolder(testLib, "TestHasAudio/Album/Gone") + gone.NumAudioFiles = 3 + gone.Missing = true + Expect(repo.Put(gone)).To(Succeed()) + + Expect(repo.HasAudioOutsideFolders(*albumRoot, []string{disc1.ID, disc2.ID})).To(BeFalse()) + }) + + It("does not treat LIKE wildcards in the parent path as patterns", func() { + // "TestHas_udio" would LIKE-match "TestHasAudio" if "_" were not escaped + wildcardRoot := model.NewFolder(testLib, "TestHas_udio/Album") + Expect(repo.Put(wildcardRoot)).To(Succeed()) + + Expect(repo.HasAudioOutsideFolders(*wildcardRoot, []string{"none"})).To(BeFalse()) + }) + }) + Describe("wrapFolderCursor", func() { It("does not panic when the cursor yields a dbFolder with nil Folder", func() { // Simulate what queryWithStableResults does on the rows.Err() path: diff --git a/persistence/mediafile_repository_test.go b/persistence/mediafile_repository_test.go index 532e9c10f..4c2363e43 100644 --- a/persistence/mediafile_repository_test.go +++ b/persistence/mediafile_repository_test.go @@ -824,4 +824,49 @@ var _ = Describe("MediaRepository", func() { Expect(mediafiles[0].ID).To(Equal("mf1")) }) }) + + Describe("BPM and BitDepth nullable round-trip", func() { + It("stores nil BPM and BitDepth as NULL and retrieves them as nil", func() { + newID := id.NewRandom() + mf := model.MediaFile{LibraryID: 1, ID: newID, Path: "test/bpm-nil.mp3"} + Expect(mr.Put(&mf)).To(Succeed()) + + retrieved, err := mr.Get(newID) + Expect(err).ToNot(HaveOccurred()) + Expect(retrieved.BPM).To(BeNil()) + Expect(retrieved.BitDepth).To(BeNil()) + + // Also verify via raw SQL that the columns are truly NULL (not 0) + db := GetDBXBuilder() + var row struct { + BPM *int `db:"bpm"` + BitDepth *int `db:"bit_depth"` + } + err = db.NewQuery("SELECT bpm, bit_depth FROM media_file WHERE id={:id}"). + Bind(dbx.Params{"id": newID}). + One(&row) + Expect(err).ToNot(HaveOccurred()) + Expect(row.BPM).To(BeNil(), "bpm should be stored as NULL in the database") + Expect(row.BitDepth).To(BeNil(), "bit_depth should be stored as NULL in the database") + + _ = mr.Delete(newID) + }) + + It("stores non-nil BPM and BitDepth and retrieves correct values", func() { + newID := id.NewRandom() + bpm := 120 + bitDepth := 24 + mf := model.MediaFile{LibraryID: 1, ID: newID, Path: "test/bpm-set.mp3", BPM: &bpm, BitDepth: &bitDepth} + Expect(mr.Put(&mf)).To(Succeed()) + + retrieved, err := mr.Get(newID) + Expect(err).ToNot(HaveOccurred()) + Expect(retrieved.BPM).ToNot(BeNil()) + Expect(*retrieved.BPM).To(Equal(120)) + Expect(retrieved.BitDepth).ToNot(BeNil()) + Expect(*retrieved.BitDepth).To(Equal(24)) + + _ = mr.Delete(newID) + }) + }) }) diff --git a/server/e2e/e2e_suite_test.go b/server/e2e/e2e_suite_test.go index 7ce8de2e6..12a7c95e0 100644 --- a/server/e2e/e2e_suite_test.go +++ b/server/e2e/e2e_suite_test.go @@ -133,7 +133,7 @@ func buildTestFS() storagetest.FakeFS { // Note: "musicbrainz_trackid" is an alias for the musicbrainz_recordingid tag (populates MbzRecordingID), // "musicbrainz_releasetrackid" is an alias for the musicbrainz_trackid tag (populates MbzReleaseTrackID). "Rock/The Beatles/Abbey Road/01 - Come Together.mp3": abbeyRoad(track(1, "Come Together", - _t{"musicbrainz_releasetrackid": mbidComeTogether, "musicbrainz_trackid": mbidComeTogetherRec})), + _t{"musicbrainz_releasetrackid": mbidComeTogether, "musicbrainz_trackid": mbidComeTogetherRec, "bpm": 120})), "Rock/The Beatles/Abbey Road/02 - Something.mp3": abbeyRoad(track(2, "Something", _t{"musicbrainz_releasetrackid": mbidSomething, "musicbrainz_trackid": mbidSomethingRec})), // Rock / The Beatles / Help! (no MBIDs) diff --git a/server/e2e/subsonic_playlists_test.go b/server/e2e/subsonic_playlists_test.go index 466e68cf0..467535df7 100644 --- a/server/e2e/subsonic_playlists_test.go +++ b/server/e2e/subsonic_playlists_test.go @@ -646,5 +646,43 @@ var _ = Describe("Playlist Endpoints", Ordered, func() { stringResp := doReq("getPlaylist", "id", stringPls.ID) Expect(stringResp.Playlist.SongCount).To(Equal(boolResp.Playlist.SongCount)) }) + + DescribeTable("isMissing/isPresent partition all songs for nullable column fields", + func(fieldName string) { + allPls := &model.Playlist{ + Name: "All Songs " + fieldName, + OwnerID: adminUser.ID, + Rules: &criteria.Criteria{Expression: criteria.Contains{"title": ""}}, + } + Expect(ds.Playlist(ctx).Put(allPls)).To(Succeed()) + missingPls := &model.Playlist{ + Name: "Missing " + fieldName, + OwnerID: adminUser.ID, + Rules: &criteria.Criteria{Expression: criteria.All{criteria.IsMissing{fieldName: true}}}, + } + Expect(ds.Playlist(ctx).Put(missingPls)).To(Succeed()) + presentPls := &model.Playlist{ + Name: "Present " + fieldName, + OwnerID: adminUser.ID, + Rules: &criteria.Criteria{Expression: criteria.All{criteria.IsPresent{fieldName: true}}}, + } + Expect(ds.Playlist(ctx).Put(presentPls)).To(Succeed()) + + allResp := doReq("getPlaylist", "id", allPls.ID) + missingResp := doReq("getPlaylist", "id", missingPls.ID) + presentResp := doReq("getPlaylist", "id", presentPls.ID) + + Expect(allResp.Status).To(Equal(responses.StatusOK)) + Expect(allResp.Playlist.SongCount).To(BeNumerically(">", int32(0))) + Expect(missingResp.Playlist.SongCount + presentResp.Playlist.SongCount). + To(Equal(allResp.Playlist.SongCount)) + }, + Entry("bpm", "bpm"), + Entry("bitdepth", "bitdepth"), + Entry("lyrics", "lyrics"), + Entry("mbz_recording_id", "mbz_recording_id"), + Entry("album", "album"), + Entry("comment", "comment"), + ) }) }) diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index e4c39e373..e6c6f9114 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -18,6 +18,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/subsonic/responses" + "github.com/navidrome/navidrome/utils/gg" "github.com/navidrome/navidrome/utils/number" "github.com/navidrome/navidrome/utils/req" "github.com/navidrome/navidrome/utils/slice" @@ -250,7 +251,7 @@ func osChildFromMediaFile(ctx context.Context, mf model.MediaFile) *responses.Op } child.Comment = mf.Comment child.SortName = sortName(mf.SortTitle, mf.OrderTitle) - child.BPM = int32(mf.BPM) + child.BPM = int32(gg.V(mf.BPM)) child.MediaType = responses.MediaTypeSong child.MusicBrainzId = mf.MbzRecordingID child.Isrc = mf.Tags.Values(model.TagISRC) @@ -262,7 +263,7 @@ func osChildFromMediaFile(ctx context.Context, mf model.MediaFile) *responses.Op } child.ChannelCount = int32(mf.Channels) child.SamplingRate = int32(mf.SampleRate) - child.BitDepth = int32(mf.BitDepth) + child.BitDepth = int32(gg.V(mf.BitDepth)) child.Genres = toItemGenres(mf.Genres) child.Moods = mf.Tags.Values(model.TagMood) child.Groupings = mf.Tags.Values(model.TagGrouping) diff --git a/server/subsonic/transcode_test.go b/server/subsonic/transcode_test.go index adc7b7600..29a883ac1 100644 --- a/server/subsonic/transcode_test.go +++ b/server/subsonic/transcode_test.go @@ -205,7 +205,7 @@ var _ = Describe("Transcode endpoints", func() { It("includes transcode stream when transcoding", func() { mockMFRepo.SetData(model.MediaFiles{ - {ID: "song-2", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 96000, BitDepth: 24}, + {ID: "song-2", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 96000, BitDepth: new(24)}, }) mockTD.decision = &stream.TranscodeDecision{ MediaID: "song-2",