Repository navigation
Support reading the AlphaTheta OneLibrary format - #196
Conversation
|
Hi @v1vendi , thank you very much for this PR. I'll take a look through, but I want to set expectations: as this is the first code for a new format in the library, it may take some time and back-and-forth to get this ready for merging. I don't have any problem with the use of AI to assist with coding - in fact, I should probably add some statements along these lines to the contributing guidelines. One thing I will say is that I will treat this PR as though Claude was not there: it'll still be assessed to the same levels of quality as any other. Anyway, this looks like a great start - first-pass review incoming soon... |
|
Hey! I'm interested in this topic so I might take a look at the proposed changes. I have a couple of questions:
|
|
Hey @acrilique Regarding ANLZ files, tbh I didn't think too far into the future, and now my goal is to make MIXXX import modern Pioneer USBs, and after that I am ready to start investigating export :) |
|
Hi @v1vendi I'd be cool if you managed to make mixxx import modern pioneer usbs as a first step. I'm mainly focusing on export since day one, and I'm trying to think of the exported USB as a whole (i.e. onelibrary + pdb + settings files + anlz). In my mind, a library that allowed (but not necessarily enforced) you to generate it all with ease would be the ideal as it would reach the highest point of compatibility with all hardware, old and new. However when it comes to reading, I've done less investigation and brainstorming for a design. I suppose it could be symmetrical: if on the write-side you're able to either add a track/playlist to pdb, to onelibrary or to both at once, then on the read side you could also have the option to read tracks from pdb, from onelibrary, or from both (with something that deduplicated them by e.g. the file path). Sorry if this was a bit too much thinking out loud. I'm looking (not very actively tho) for people to help on ideas for the library design, testing exports with real hardware, reviewing code, etc in case you're ever interested. |
|
I pushed a significantly more fast implementation of crypto algorithms, which is almost on par with OpenSSL - but still does not require ~4mb of OpenSSL as a dependency. |
|
So, as I found out, Onelibrary needs |
mr-smidge
left a comment
mr-smidge
left a comment
There was a problem hiding this comment.
This PR is a great start, and I've tried to be comprehensive in my review.
I think there are three main themes that we should think about:
- The choice of SQLCipher vs. an explicit crypto implementation in libdjinterop. Although the minimal code has some advantages, I keep thinking that libdjinterop is not a crypto library and shouldn't try to be. Yet at the same time, nobody has tried to get SQLCipher in yet so if there are any challenges we don't yet know them. This can be discussed more on #191.
- I think there needs to be cleaner separation between the high-level and low-level API for OneLibrary.
- The low-level API may presumably undergo version changes like any other format (I don't have enough experience with AlphaTheta equipment/formats to know how much this has already happened since the format was first released), and I think it would be sensible to organise low-level API code such that any breaking changes to the underlying format can be easily handled.
Many thanks for your efforts here - let me know what you think.
|
@mr-smidge Two things in favour of the current one: it only decrypts, and the passphrase is a published constant, so there's no secret to leak through a table-driven AES. And SQLCipher means vendoring its amalgamation in place of SQLite for everyone, plus a crypto provider and Ubuntu/vcpkg/brew changes. Every Engine user will have this change without using it. Your call either way, the seam makes it cheap. |
OneLibrary, also documented as Device Library Plus, is the successor to the DeviceSQL export.pdb library that rekordbox writes to USB media. A device usually carries both, and a player that understands OneLibrary prefers it. The library is a single SQLite database encrypted with SQLCipher 4. Its page format is implemented in util/crypto, so neither SQLCipher nor OpenSSL is needed to read one: AES-256-CBC, SHA-512, HMAC and PBKDF2 are written from their specifications, and the write-ahead log is folded in before SQLite sees the file, as rekordbox leaves most of a fresh export in the log. Support is read-only. Track metadata, playlists and crates can be read; everything that would change a database throws unsupported_operation. Beat grids, waveforms, hot cues and loops are not in the database at all, as rekordbox leaves them in the ANLZ files beside the music, so onelibrary::library gives the path recorded for a track alongside the key notation and track colour that the format-agnostic interface has nowhere to put. Reading a database needs SQLite 3.36 or newer, built without SQLITE_OMIT_DESERIALIZE.
Reading an encrypted database was dominated by software AES and by copying, so several layers of that are removed: * AES uses the processor's own AES instructions where it has them (x86 AES-NI, the ARMv8 cryptographic extension), decided at run time and confined to `aes_hardware.cpp`, which the build compiles with the intrinsics enabled only where they compile at all. Decryption runs eight independent blocks at a time to keep the pipeline full. * The software fallback moves to fused round tables and to the equivalent inverse cipher, so both directions have the same shape. * SHA-512 keeps a sixteen-word wrapping schedule and unrolls its rounds by rotating variable names instead of shifting values. * HMAC keys absorb both padded blocks once, which halves key derivation over its 256,000 iterations and saves two compressions per page. * Page tags are computed over the ciphertext, IV and page number in place, rather than gathering each page into a scratch buffer. * The write-ahead log is read before the database, so a page the log replaces or truncates away is never decrypted at all. * Page decryption is spread over the available processors, pages being independent of one another. Tests gain the NIST SP 800-38A chaining vectors and run every cipher test over both implementations, so the tables are still exercised on a machine that would otherwise only ever use its AES instructions.
GCC 12.3 on x86-64 rejects `resolve`, where the path is trimmed a level at a time by assigning a piece of a string back to itself. Inlining turns that into a memcpy whose bounds it cannot establish, and it warns of an overlap of nine quintillion bytes, which -Werror then makes fatal. Track the end of the prefix as an index instead and build the string once, at the end. The construct the warning fires on is gone, and no path resolves any differently: the two agree on every string up to length six over `/`, `\` and a letter, and on the absolute, relative, Windows and UNC cases besides.
Two things went wrong with `-DSYSTEM_SQLITE=OFF`, which is how the macOS builds are configured. A test that talks to SQLite directly failed to link. The bundled amalgamation is compiled into the library, whose symbols are hidden, so there was nothing for the test to link against; the tests that use SQLite now compile it themselves, as they already do for the internal sources they reach into. That then exposed the second: the bundled amalgamation is 3.33, and reading a decrypted database needs `sqlite3_deserialize` from 3.36. The library correctly reports the format unsupported there, so the database test is registered only where SQLite can do the work, and CMake says so when it is left out.
Reading a decrypted database goes through `sqlite3_deserialize`, which arrived in SQLite 3.36, so the test of it is registered only where that much is available. It asked about the system installation alone, which left the bundled copy out of the reckoning altogether. Read the bundled version out of the amalgamation instead, and ask the same question of both. Nothing changes while the bundled copy is 3.33, which cannot do the work; when it is newer the test follows.
Each of them talks to SQLite directly, and a test cannot borrow it from the library, whose symbols are hidden. With `-DSYSTEM_SQLITE=OFF` that means every one of them compiles the amalgamation into itself, some fifteen seconds apiece, on top of the copy the library already builds. They are commented out rather than deleted, and their sources are left where they are, so uncommenting the block runs them again.
Reading a device meant reaching for `sqlcipher_codec` and `sqlcipher_wal` by name, deriving a key, folding in the log and calling `sqlite3_deserialize`, all inside `onelibrary.cpp`. That is the format code knowing how the bytes are decrypted, which is not its business. `encrypted_database.hpp` now says the whole of it: a path and a passphrase in, a connection to the contents out. The implementation beside it decrypts the pages itself; a SQLCipher-backed build would replace that one source file, which CMake names in a single line. The two failures a caller must tell apart -- a build that cannot open encrypted databases, and a file the passphrase does not open -- are now two exception types, and their messages reach the caller intact, so a file that is merely too small no longer reports a bad passphrase. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`onelibrary.hpp` held both halves of the API: `database_exists` and `load_database`, which are the high-level entry to the format, and a `library` class whose accessors read three columns of `content` that the format-agnostic interface has nowhere to put. The tables those columns come from were not public at all, so a caller wanting anything else had no way to reach it. The low-level API is now public, under `onelibrary/v1`, laid out as the Engine formats are: `content_table`, `playlist_table` and a new `property_table`, reached from an `onelibrary::v1::library`. Each table reads a whole row or any one column on its own, so a caller need not pay for six joins to learn a track's album, and `track_impl` no longer takes a whole snapshot to answer for one field. `analysis_path` and `color_id` join the content row, and `library::analysis_path`, `key_name` and `color_id` are gone: they were the low-level API in disguise. The `v1` is the schema the tables describe. A device records it as `property.dbVersion`, and every export written so far reports `1000`, which `supported_db_version` names. A schema that is not compatible with this one gets a namespace of its own rather than changing these. `to_snapshot` and `parse_musical_key` translate between the format and the high-level interface, so they leave the table that has no use for them and move to `track_conversion`. `load_context` is now shared by both halves and lives in `loader.hpp`, and the query helpers that only ever wanted a database rather than a whole context move to `util/sqlite_query.hpp`, where any format can use them. Also from review: a duration is `std::chrono::seconds` rather than a bare count; `no_color_id` names the zero that means unmarked; the docstrings say what units a size, a bitrate and a sampling rate are in, and what a rating and a path actually hold; the schema the tests build on moves out of a C++ string literal into `testdata/ref/onelibrary/schema.sql`; a build that cannot read an encrypted database now says so of itself before it mentions SQLite; and `/build`, which is one developer's habit rather than anything this project writes, leaves the ignore list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Main now vendors SQLCipher, so the built-in decryption goes: the AES, SHA-512, page codec and WAL folding, with their tests and CMake checks. A OneLibrary database is opened in place with SQLCipher when built with EXPERIMENTAL_ENABLE_SQLCIPHER, and refused with an explanation otherwise. The OneLibrary tests build on that switch and write their fixtures with SQLCipher itself. Also simplify along the way: share the track field conversions between snapshots and accessors, check the schema beside the loader, drop the loader header and the internal exception types, and resolve the device root with std::filesystem. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@mr-smidge I integrated SQLCipher, removed crypto implementation, cleaned up the code and rebased, so at this point review kinda starts from the beginning :) |
A static build with EXPERIMENTAL_ENABLE_SQLCIPHER could not be installed, as the sqlcipher target it links was in no export set. It is now installed and exported beside DjInterop, and the package config finds OpenSSL for it. The exported include directory also pointed one level too deep, so the installed headers could not be included as <djinterop/...>. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mr-smidge
left a comment
mr-smidge
left a comment
There was a problem hiding this comment.
Feels like this PR is nearly there... a few more comments.
| std::string to_relative_path(const std::string& path) | ||
| { | ||
| return !path.empty() && path.front() == '/' ? path.substr(1) : path; | ||
| } | ||
|
|
||
| std::string to_device_path(const std::string& relative_path) | ||
| { | ||
| return "/" + to_relative_path(relative_path); | ||
| } |
There was a problem hiding this comment.
Might be worth having a look at #207 - there is an initiative to switch all file/dir paths to use std::filesystem::path.
Not sure what immediate impact that has on this PR, but these path-wrangling functions may need to be adapted.
There was a problem hiding this comment.
Noted. I'll switch these to std::filesystem::path once #207 lands.
|
|
||
| CREATE TABLE playlist_content(playlist_id integer, content_id integer, sequenceNo integer); | ||
|
|
||
| CREATE TABLE property(deviceName varchar, dbVersion varchar, numberOfContents integer, createdDate varchar, backGroundColorType integer, myTagMasterDBID integer); |
There was a problem hiding this comment.
Are the contents of the property table considered a fundamental part of the schema? i.e. if this testdata script creates an empty database, is it valid without any data in this table?
(Note: for comparison, the Engine implementation considers having data in the equivalent Information table a fundamental part of the schema setup.)
There was a problem hiding this comment.
rekordbox always writes exactly one row, so the script now inserts one. libdjinterop only uses it for version_name() and accepts an empty table. I can add a verify() check if you'd like.
There was a problem hiding this comment.
What would Rekordbox or a real Pioneer device do if it tried to load a database with the property table empty?
Sebastian recently fixed a bug (see #202) where not writing correctly to the Engine-equivalent table could lead to corrupt databases, so yes please, I do think it would be a good idea to add such a check to verify().
Once this is done, I will be happy to approve the PR!
(On a separate note, I don't believe there is an equivalent check in the Engine code, so I'll make a note to add one!)
There was a problem hiding this comment.
Added to verify(): it needs exactly one row with a non-empty dbVersion. Tests added.
- Follow the Engine pattern for `onelibrary::v1::library`: a constructor from the context, and static `exists()`, `load()` and `create_from_scripts()`. - Add `onelibrary::create_database_from_scripts()`, mirroring Engine, and make the OneLibrary tests use only the public API, so they no longer link SQLCipher or compile library sources into themselves. - Move the reference schema to `testdata/ref/onelibrary/v1`, and give it the `property` row that every export carries. - Open encrypted databases through sqlite_modern_cpp, setting the key with `sqlite3_key`. - Use `#pragma once` alone in the OneLibrary headers, and list the `v1` headers among the library's sources. - Clarify the rating and path docstrings, and name the special playlist identifiers. - Stop `descendant_ids()` recursing forever on a playlist tree with a cycle. - Use `std::string_view` and `std::unordered_set` where suggested, and trim the low-level section of GUIDE.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An application that links its own SQLite, as Mixxx does for Qt, would otherwise interpose its copy on the calls libdjinterop makes, and that copy cannot decrypt a OneLibrary database. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Found while testing with Mixxx: an app that links its own SQLite can't also link libdjinterop and SQLCipher statically. Both define the same symbols, the linker keeps the app's copy, and opening a OneLibrary database fails with "SQL logic error". Linking libdjinterop as a shared library avoids this. I've pushed f9843a4, which hides SQLCipher's symbols inside the shared library, so the app's SQLite can't replace them on Linux either. On macOS, Mixxx built this way reads a real export correctly. Static linking next to another SQLite would still clash. Fixing that would mean renaming SQLCipher's symbols; happy to do it if you'd prefer. |
My understanding (I'm not an expert on this - happy to be educated!) is that this may be an unavoidable problem with static linking - both Mixxx and libdjinterop would be trying to link with libraries that expose the sqlite C function names. SQLCipher is a superset of SQLite in terms of the functions it exports, and so an end application wanting to statically link to libdjinterop needs to ensure that there is one copy of sqlite/sqlcipher that satisfies what the application and libdjinterop needs. I may raise this on the Mixxx developer forums - perhaps it's possible to get Mixxx to use SQLCipher by default... |
|
As of now, worst case scenario for mixxx, I think that using SQLite and SQLCipher side by side is not such a big issue. One mp3 track will take more memory than another instance of SQLite :) |
| @@ -0,0 +1,35 @@ | |||
| -- The schema a real OneLibrary export carries, abbreviated to the tables that | |||
There was a problem hiding this comment.
How was this file generated? What program and what version? When I export from Djay pro and rekordbox, I get 22 tables and 4 indexes.
Additionally, how does rekordbox and others behave when opening a db with only these tables? I have found rekordbox to mutate the db when it's not just right.
Is that desirable or should we better align with Rekordbox? I know this is specific to test data but we probably will want the test data to align with however we init the db.
cc @mr-smidge (cc'd for the last question)
There was a problem hiding this comment.
I trimmed it by hand from a rekordbox 7.2.6 export, keeping only the tables we read. It's only used for tests while the library is read-only. When write support lands, creating a database should use rekordbox's full schema, and I'll do that in that PR.
There was a problem hiding this comment.
I'll do that in that PR
I have writing done for every stubbed function in this PR. I rebased my branch onto this branch. I am just waiting for this to merge to open the PR. I am currently working on ANLZ.
There was a problem hiding this comment.
This is a good point: the testdata/ref/ sub-directory is called ref because it is meant to be a reference for actual schema dumps, so the trimmed version is not strictly reference material.
However, this won't block my review of this PR - I'd be perfectly happy if we upgraded this DB dump to a genuine reference schema in a follow-up PR.
| { | ||
| /// Location of the library database within a device, relative to its root. | ||
| constexpr const char* database_relative_path = | ||
| "PIONEER/rekordbox/exportLibrary.db"; |
There was a problem hiding this comment.
When I export to usb from rekordbox 7, it writes to .PIONEER/. Non-exports, like the normal collection, use PIONEER. Can you confirm this path is correct on the usb?
There was a problem hiding this comment.
rekordbox 7.2.6 exports to PIONEER/rekordbox/exportLibrary.db and imports from there. I haven't tested on a player. Which version gave you .PIONEER/? I can make it check both. Passing the .db path directly also works.
There was a problem hiding this comment.
Looks like further work might be needed here, as this constant seems to imply something that may not be true on certain filesystems.
I don't think this is a blocker for the PR though - OneLibrary support is still experimental, partial filesystem support is fine for now.
Stop shipping rekordbox's passphrase: load and create functions now take it as a required argument, and the example reads it from argv or ONELIBRARY_PASSPHRASE. Also have verify() require the `key` table, and exactly one `property` row recording a non-empty `dbVersion`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Partially solves #177
The PR is huge and made with help of AI, but what is not nowadays...
At least I did multiple rounds of guided review and cleanup.
Tested on MacOS with a real USB exported from Rekordbox.
I already prepared and tested integration to MIXXX too.
Writing also shouldn't be hard, but seems reasonable to start with readonly mode.
No need for SQLCipher dependency mentioned in #191
The library is one SQLite database encrypted with SQLCipher 4. Its page format is implemented in
util/crypto, so neither SQLCipher nor OpenSSL becomes a dependency: AES-256-CBC, SHA-512, HMAC and PBKDF2 are written from their specifications and covered by test vectors from FIPS 197, FIPS 180-4, RFC 4231 and RFC 6070.Rekordbox leaves most of a fresh export in the write-ahead log rather than the database file, so a reader that ignores the log reports a nearly empty library with no error at all. The log is folded in before SQLite ever sees the bytes.
The passphrase is the same on every installation and depends on neither licence nor machine, so any player can read any device. It can be overridden, should a future release of rekordbox change it.
Scope
Support is read-only: track metadata, playlists and crates can be read, and everything that would change a database throws
unsupported_operation. The format keeps a single tree serving as both playlists and crates, soplaylists_and_crates_are_distinctis false.Beat grids, waveforms, hot cues and loops are not in the database at all — rekordbox leaves them in the ANLZ files beside the music, and exports an empty
cuetable — so those accessors return nothing rather than throwing. For a caller that reads ANLZ files itself,onelibrary::librarygives the analysis path recorded for a track, along with the key notation as written (which may be Camelot, and so cannot be represented bytrack::key()) and the track colour.Tests
Unit tests cover the crypto primitives against published test vectors, the content and playlist tables, and the database as a whole, reading fixtures that are built as plain SQLite and encrypted independently by the test suite, so a round trip checks the implementation against the format rather than against itself.