From 7327549eea3704a84ec042effeabb5a1cd48f862 Mon Sep 17 00:00:00 2001 From: Matthias Blankertz Date: Sun, 7 Sep 2025 17:45:27 +0200 Subject: [PATCH] refactor(playlistdb): Enforce constant entry index length To simplify the playlist handling, enforce that the indices are always formatted to the same length (5, which allows for 100000 entries, that should be enough). Then make the position stored in the Playlist object be a simple integer instead of a database key. This simplifies the code, and will make implementing shuffle much easier. Signed-off-by: Matthias Blankertz --- DEVELOP.md | 6 +- software/src/utils/playlistdb.py | 102 ++++++++++++---------- software/tests/mocks/btree.py | 4 + software/tests/utils_test/test_btreedb.py | 74 ++++++---------- 4 files changed, 91 insertions(+), 95 deletions(-) diff --git a/DEVELOP.md b/DEVELOP.md index 6595a3b..9de3655 100644 --- a/DEVELOP.md +++ b/DEVELOP.md @@ -18,10 +18,8 @@ pip install -U micropython-rp2-pico_w-stubs --target typings The playlist data is stored in the btree database in a hierarchical schema. The hierarchy levels are separated by the '/' character. Currently, the schema is as follows: The top level for a playlist is the 'tag' id encoded as a hexadecimal string. Beneath this, the 'playlist' key contains the -elements in the playlist. The keys used for the playlist entries should be decimal integers, -prefixed with sufficient zeros such that they are in the correct order when sorted -lexicographically. All keys must have the same width. When writing a playlist using the playlistdb -module, the keys are 00000, 00001, etc. by default. +elements in the playlist. The keys used for the playlist entries must be decimal integers, +left-padded with zeros so their length is 5 (e.g. format `{:05}`). #### Playlist modes diff --git a/software/src/utils/playlistdb.py b/software/src/utils/playlistdb.py index e397b98..126577e 100644 --- a/software/src/utils/playlistdb.py +++ b/software/src/utils/playlistdb.py @@ -31,12 +31,13 @@ class BTreeDB(IPlaylistDB): PERSIST_OFFSET = b'offset' class Playlist(IPlaylist): - def __init__(self, parent, tag, pos, persist, shuffle): + def __init__(self, parent: "BTreeDB", tag: bytes, pos: int, persist, shuffle): self.parent = parent self.tag = tag self.pos = pos self.persist = persist self.shuffle = shuffle + self.length = self.parent._getPlaylistLength(self.tag) def getPaths(self): """ @@ -54,15 +55,17 @@ class BTreeDB(IPlaylistDB): """ Select next track and return path. """ - try: - self.pos = self.parent._getNextTrack(self.tag, self.pos) - except StopIteration: - self.pos = self.parent._getFirstTrack(self.tag) - return None - finally: + if self.pos + 1 >= self.length: + self.pos = 0 if self.persist != BTreeDB.PERSIST_NO: self.parent._setPlaylistPos(self.tag, self.pos) self.setPlaybackOffset(0) + return None + + self.pos += 1 + if self.persist != BTreeDB.PERSIST_NO: + self.parent._setPlaylistPos(self.tag, self.pos) + self.setPlaybackOffset(0) return self.getCurrentPath() def setPlaybackOffset(self, offset): @@ -122,27 +125,42 @@ class BTreeDB(IPlaylistDB): if self.flush_func is not None: self.flush_func() - def _getPlaylistValueIterator(self, tag): + def _getPlaylistValueIterator(self, tag: bytes): start, end = self._keyPlaylistStartEnd(tag) return self.db.values(start, end) - def _getPlaylistEntry(self, _, pos): - return self.db[pos] + def _getPlaylistEntry(self, tag: bytes, pos: int) -> bytes: + return self.db[self._keyPlaylistEntry(tag, pos)] - def _setPlaylistPos(self, tag, pos, flush=True): - assert pos.startswith(self._keyPlaylistStart(tag)) - self.db[self._keyPlaylistPos(tag)] = pos[len(self._keyPlaylistStart(tag)):] + def _setPlaylistPos(self, tag: bytes, pos: int, flush=True): + self.db[self._keyPlaylistPos(tag)] = str(pos).encode() if flush: self._flush() - def _setPlaylistPosOffset(self, tag, offset, flush=True): + def _setPlaylistPosOffset(self, tag: bytes, offset: int, flush=True): self.db[self._keyPlaylistPosOffset(tag)] = str(offset).encode() if flush: self._flush() - def _getPlaylistPosOffset(self, tag): + def _getPlaylistPosOffset(self, tag: bytes) -> int: return int(self.db.get(self._keyPlaylistPosOffset(tag), b'0')) + def _getPlaylistLength(self, tag: bytes) -> int: + start, end = self._keyPlaylistStartEnd(tag) + for k in self.db.keys(end, start, btree.DESC): + # There is a bug in btreedb that causes an additional key after 'end' to be returned when iterating in + # descending order + # Check for this and skip it if needed + elements = k.split(b'/') + if len(elements) >= 2 and elements[1] == b'playlist': + last = k + break + print(last) + elements = last.split(b'/') + if len(elements) != 3: + raise RuntimeError("Malformed playlist key") + return int(elements[2])+1 + def _savePlaylist(self, tag, entries, persist, shuffle, flush=True): self._deletePlaylist(tag, False) for idx, entry in enumerate(entries): @@ -159,41 +177,32 @@ class BTreeDB(IPlaylistDB): del self.db[k] except KeyError: pass - try: - del self.db[self._keyPlaylistPos(tag)] - except KeyError: - pass + for k in (self._keyPlaylistPos(tag), self._keyPlaylistPosOffset(tag), + self._keyPlaylistPersist(tag), self._keyPlaylistShuffle(tag)): + try: + del self.db[k] + except KeyError: + pass if flush: self._flush() - def _getFirstTrack(self, tag: bytes): - start_key, end_key = self._keyPlaylistStartEnd(tag) - return next(self.db.keys(start_key, end_key)) - - def _getNextTrack(self, tag, pos): - _, end_key = self._keyPlaylistStartEnd(tag) - it = self.db.keys(pos, end_key) - next(it) - return next(it) - def getPlaylistForTag(self, tag: bytes): """ Lookup the playlist for 'tag' and return the Playlist object. Return None if no playlist exists for the given tag. """ persist = self.db.get(self._keyPlaylistPersist(tag), self.PERSIST_TRACK) - if persist != self.PERSIST_NO: - pos = self.db.get(self._keyPlaylistPos(tag)) - else: - pos = None - if pos is None: + pos = 0 + if persist != self.PERSIST_NO and self._keyPlaylistPos(tag) in self.db: try: - pos = self._getFirstTrack(tag) - except StopIteration: - # playist does not exist - return None - else: - pos = self._keyPlaylistStart(tag) + pos + pos = int(self.db[self._keyPlaylistPos(tag)]) + except ValueError: + pass + if self._keyPlaylistEntry(tag, 0) not in self.db: + # Empty playlist + return None + if self._keyPlaylistEntry(tag, pos) not in self.db: + pos = 0 shuffle = self.db.get(self._keyPlaylistShuffle(tag), self.SHUFFLE_NO) return self.Playlist(self, tag, pos, persist, shuffle) @@ -221,7 +230,6 @@ class BTreeDB(IPlaylistDB): last_tag = None last_pos = None - index_width = None for k in self.db.keys(): fields = k.split(b'/') if len(fields) <= 1: @@ -240,13 +248,12 @@ class BTreeDB(IPlaylistDB): except ValueError: fail(f'Malformed playlist entry: {k!r}') continue - if index_width is not None and len(fields[2]) != index_width: - fail(f'Inconsistent index width for {last_tag} at {idx}') + if len(fields[2]) != 5: + fail(f'Bad index width for {last_tag} at {idx}') if (last_pos is not None and last_pos + 1 != idx) or \ (last_pos is None and idx != 0): fail(f'Bad playlist entry sequence for {last_tag} at {idx}') last_pos = idx - index_width = len(fields[2]) if dump: print(f'\tTrack {idx}: {self.db[k]!r}') elif fields[1] == b'playlistpos': @@ -276,8 +283,11 @@ class BTreeDB(IPlaylistDB): # Format TBD pass elif fields[1] == b'playlistposoffset': - # Format TBD - pass + val = self.db[k] + try: + _ = int(val) + except ValueError: + fail(f' Bad playlistposoffset value for {last_tag}: {val!r}') else: fail(f'Unknown key {k!r}') return result diff --git a/software/tests/mocks/btree.py b/software/tests/mocks/btree.py index 1078f6b..c536d84 100644 --- a/software/tests/mocks/btree.py +++ b/software/tests/mocks/btree.py @@ -19,3 +19,7 @@ class BTree: def open(dbfile) -> BTree: pass + + +DESC = 1 +INCL = 2 diff --git a/software/tests/utils_test/test_btreedb.py b/software/tests/utils_test/test_btreedb.py index d0430e7..f38df7b 100644 --- a/software/tests/utils_test/test_btreedb.py +++ b/software/tests/utils_test/test_btreedb.py @@ -1,6 +1,7 @@ # SPDX-License-Identifier: MIT # Copyright (c) 2025 Matthias Blankertz +import btree import pytest from utils import BTreeDB @@ -24,12 +25,18 @@ class FakeDB: res.append(self.contents[key]) def keys(self, start_key=None, end_key=None, flags=None): + keys = [] + if flags is not None and flags & btree.DESC != 0: + start_key, end_key = end_key, start_key for key in sorted(self.contents): if start_key is not None and start_key > key: continue if end_key is not None and end_key <= key: break - yield key + keys.append(key) + if flags is not None and flags & btree.DESC != 0: + keys.reverse() + return iter(keys) def get(self, key, default=None): return self.contents.get(key, default) @@ -43,11 +50,14 @@ class FakeDB: def __delitem__(self, key): del self.contents[key] + def __contains__(self, key): + return key in self.contents + def test_playlist_load(): contents = {b'foo/part': b'no', - b'foo/playlist/0': b'track1', - b'foo/playlist/1': b'track2', + b'foo/playlist/00000': b'track1', + b'foo/playlist/00001': b'track2', b'foo/playlisttt': b'no' } uut = BTreeDB(FakeDB(contents)) @@ -58,8 +68,8 @@ def test_playlist_load(): def test_playlist_nextpath(): contents = FakeDB({b'foo/part': b'no', - b'foo/playlist/0': b'track1', - b'foo/playlist/1': b'track2', + b'foo/playlist/00000': b'track1', + b'foo/playlist/00001': b'track2', b'foo/playlisttt': b'no' }) uut = BTreeDB(contents) @@ -69,8 +79,8 @@ def test_playlist_nextpath(): def test_playlist_nextpath_last(): - contents = FakeDB({b'foo/playlist/0': b'track1', - b'foo/playlist/1': b'track2', + contents = FakeDB({b'foo/playlist/00000': b'track1', + b'foo/playlist/00001': b'track2', b'foo/playlistpos': b'1' }) uut = BTreeDB(contents) @@ -80,8 +90,8 @@ def test_playlist_nextpath_last(): def test_playlist_create(): - contents = FakeDB({b'foo/playlist/0': b'track1', - b'foo/playlist/1': b'track2', + contents = FakeDB({b'foo/playlist/00000': b'track1', + b'foo/playlist/00001': b'track2', b'foo/playlistpos': b'1' }) newplaylist = [b'never gonna give you up.mp3', b'durch den monsun.mp3'] @@ -93,43 +103,17 @@ def test_playlist_create(): def test_playlist_load_notexist(): - contents = FakeDB({b'foo/playlist/0': b'track1', - b'foo/playlist/1': b'track2', + contents = FakeDB({b'foo/playlist/00000': b'track1', + b'foo/playlist/00001': b'track2', b'foo/playlistpos': b'1' }) uut = BTreeDB(contents) assert uut.getPlaylistForTag(b'notfound') is None -def test_playlist_remains_lexicographically_ordered_by_key(): - contents = FakeDB({b'foo/playlist/3': b'track3', - b'foo/playlist/2': b'track2', - b'foo/playlist/1': b'track1', - b'foo/playlistpos': b'1' - }) - uut = BTreeDB(contents) - pl = uut.getPlaylistForTag(b'foo') - assert pl.getCurrentPath() == b'track1' - assert pl.getNextPath() == b'track2' - assert pl.getNextPath() == b'track3' - - -def test_playlist_remains_lexicographically_ordered_with_non_numeric_keys(): - contents = FakeDB({b'foo/playlist/k': b'trackk', - b'foo/playlist/l': b'trackl', - b'foo/playlist/i': b'tracki', - b'foo/playlistpos': b'k' - }) - uut = BTreeDB(contents) - pl = uut.getPlaylistForTag(b'foo') - assert pl.getCurrentPath() == b'trackk' - assert pl.getNextPath() == b'trackl' - assert pl.getNextPath() is None - - def test_playlist_starts_at_beginning_in_persist_no_mode(): - contents = FakeDB({b'foo/playlist/0': b'track1', - b'foo/playlist/1': b'track2', + contents = FakeDB({b'foo/playlist/00000': b'track1', + b'foo/playlist/00001': b'track2', b'foo/playlistpersist': b'no', }) uut = BTreeDB(contents) @@ -143,8 +127,8 @@ def test_playlist_starts_at_beginning_in_persist_no_mode(): @pytest.mark.parametrize("mode", [b'no', b'track']) def test_playlist_ignores_offset_in_other_modes(mode): - contents = FakeDB({b'foo/playlist/0': b'track1', - b'foo/playlist/1': b'track2', + contents = FakeDB({b'foo/playlist/00000': b'track1', + b'foo/playlist/00001': b'track2', b'foo/playlistpersist': mode, }) uut = BTreeDB(contents) @@ -156,8 +140,8 @@ def test_playlist_ignores_offset_in_other_modes(mode): def test_playlist_stores_offset_in_offset_mode(): - contents = FakeDB({b'foo/playlist/0': b'track1', - b'foo/playlist/1': b'track2', + contents = FakeDB({b'foo/playlist/00000': b'track1', + b'foo/playlist/00001': b'track2', b'foo/playlistpersist': b'offset', }) uut = BTreeDB(contents) @@ -169,8 +153,8 @@ def test_playlist_stores_offset_in_offset_mode(): def test_playlist_resets_offset_on_next_track(): - contents = FakeDB({b'foo/playlist/0': b'track1', - b'foo/playlist/1': b'track2', + contents = FakeDB({b'foo/playlist/00000': b'track1', + b'foo/playlist/00001': b'track2', b'foo/playlistpersist': b'offset', }) uut = BTreeDB(contents)