Feat/music categorization - #36
Merged
Merged
Conversation
Replaces audiotags with symphonia as the only tag reader. audiotags covers MP3/M4A/FLAC alone, so an OGG, Opus or WAV library indexed with no artist, album, genre or year at all — which is why the music categories on issue #11 were reported as empty. symphonia demuxes every container it supports and reads APEv1/APEv2 alongside ID3 and Vorbis comments, so the same code now covers ogg, opus, wav, aac, aiff and caf as well. Two gaps remain and are documented where the reader lives: .wma has no ASF reader, and a bare Monkey's Audio .ape file has no demuxer, so its APE tags are unreachable. Both still fall back to filename parsing. Everything the reader finds is now kept. Twenty tag and stream fields that browsing, sorting and DIDL consume get columns of their own; the rest goes to media_tags verbatim, so using a new tag later is a query rather than another migration. symphonia/opt-simd is scoped to `casting` rather than `metadata`: it only swaps in rustfft's SIMD FFT, and the FFT is reached solely by the decoders, which is the AirPlay path. Reading tags never decodes, so a metadata-only build stays free of that dependency tree. This is the first schema change since the SQLite move, so it also adds the migration mechanism that move made possible. v1 -> v2 is additive: ALTER TABLE for each new column, the media_tags table, and a VIRTUAL disc_sort for disc-aware ordering (SQLite cannot alter the existing STORED track_sort, and ALTER TABLE only accepts VIRTUAL generated columns). Rebuilding instead would have discarded AirPlay pairings and imported playlists, and renumbered every record — and those numbers are the object ids DIDL hands to renderers, so every saved favourite and resume point on a TV would break. Existing rows land at tags_version 0, which trails the reader, so the next scan rewrites them with the new fields filled in. That costs one database write per record and no extra I/O: the scanner already parses every file on every pass and the fingerprint only decides whether to write. Refs #11
Music was one flat list per category: Artists led straight to every track by that artist, with no album level, and an album reached from Albums matched on title alone, so two records with the same name from different artists merged into one container. There was no All Music, and Album Artists was implemented in the database but unreachable. The tree now matches minidlna. Artists and Album Artists descend through albums, Genres descends through artists and then albums, and each level offers All Songs so a renderer can play everything without walking down. Playlists list their tracks in stored order. Every container is now described by a MusicNode: one enum of the addressable containers, one parser from object id, one builder back to child ids. That replaces the chain of prefix tests and the per-category handlers, each of which carried its own copy of the cache and metrics code. Tag values are percent-encoded into object ids, which nesting requires: otherwise an artist called "AC/DC" cannot be told from an artist "AC" holding an album "DC". Structural segments are spelled "!all" and the encoder escapes "!", so no tag value can collide with one — an album genuinely called "!all" round-trips as "%21all". Containers also stop lying about themselves. They carry their real UPnP class — musicArtist, musicAlbum, musicGenre, playlistContainer — instead of announcing everything as a storage folder, and their real child count instead of a hardcoded 1 with the count spliced into the title. Album containers point albumArtURI at a representative track, so a control point shows cover art without opening the album. Items gained the res attributes the new stream columns make available: bitrate, sampleFrequency, nrAudioChannels and bitsPerSample, plus composer. Note that DLNA's res@bitrate is bytes per second, not bits. Closes #11
Adds the cases that pin down what the two previous commits changed: - Object ids round-trip through every node shape, including the ones that motivated encoding them: an artist called "AC/DC", a value containing a percent sign, and an album genuinely named "!all" which must not be mistaken for the reserved segment. - A DLNA walk over the real router, driving Browse the way a control point does: root to artist to album to tracks, genre to artist to album, playlists in stored order, and BrowseMetadata reporting the right class and a non-zero child count. - A hand-built AIFF proving a container the old reader could not open now fills the categories, which is the regression the issue reported. - An MP3 carrying an APEv2 tag alongside its ID3 frame. Both survive, which is what draining the whole metadata log buys over skipping to the newest revision. - Disc-aware ordering, nested category filters, and playlist counts as backend conformance cases. Two fixes fell out of writing these. ID3v2.3 stores the year in TYER, which symphonia maps to RecordingYear — a variant the reader did not handle, so every v2.3 file lost its year and the Years category with it. And WAV turns out to read no tags at all: symphonia 0.6.0 parses the RIFF INFO list into a metadata log and then overwrites that log with the empty one from its options before returning. AIFF, from the same crate, does it correctly. The fixture uses AIFF and the limitation is recorded where the reader lives, alongside the WMA and Monkey's Audio gaps. get_media_tags reaches the tags kept in media_tags, so the long tail is queryable rather than merely stored.
…aring good ones A track or disc number too large to be real is a malformed tag. Skipping it rather than assigning None keeps whatever an earlier revision of the metadata got right, which matters for a file carrying both ID3 and APE.
…ffset A control point that pages needs the full total in every response, or it stops after the first one. Covers containers, items, and an offset past the end, which must be an empty page rather than an error.
Seven issues found reviewing the branch, all in code it introduced.
A container whose children are containers was announcing its track count
as childCount. An artist with four tracks across two albums claimed four
children and returned three, and BrowseMetadata — which counts properly
— disagreed with the listing that had just named the same object. The
category query now takes the tag one level down and counts distinct
values of it in the same GROUP BY, which costs nothing: the grouping had
already visited every row.
Internet radio was leaking into the categories. A station is stored with
its source playlist path as the album, so importing a radio M3U grew an
album container named after a file path — and opening it returned
nothing, because every track query the tree builds already excludes
radio. The category query now excludes it too.
The non-recursive scan path never picked up the tags_version staleness
check, so a library configured with recursive = false would have
migrated to the new schema and then never backfilled the columns the
migration added. That is the whole point of the version marker, missing
from half the paths that need it.
Also: BrowseMetadata on a playlist reported "Playlist 7" where the
listing said "Roadtrip", since the name lives in the database rather
than the object id; extra_tags stored promoted tags as well as the
others, contradicting its own documented contract and roughly doubling
the side table; media_tags survived a write that cleared the columns
beside it, leaving get_media_tags answering for a record whose columns
are empty; and a legacy audio/<folder> id lost its audio-filtered child
count when the old branch was removed.
The child-count test was verified to fail without its fix ("artist
announced 4 children but returned 3") rather than passing by
construction.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.