Avoid MPD manifest parsing to fix regression with new TIDAL group="main" manifests#216
Conversation
…TIDAL group="main" manifests TIDAL appears to have changed the MPEG-DASH MPD manifests returned for some streams to include non-numeric values in `<AdaptationSet group="...">` (e.g. group="main"). The current `mpegdash` parser expects group to be an integer and raises a `ValueError`, which `tidalapi` wraps as `ManifestDecodeError`. This caused `mopidy-tidal` playback to fail and tracks to be marked “not playable”. This PR updates mopidy-tidal’s playback provider to avoid parsing MPD manifests via tidalapi/mpegdash. For MPD manifests we only need to persist the MPD XML and return a `file://` URI, so parsing is unnecessary. **Root cause** - [`tidalapi.StreamManifest`](https://github.com/EbbLabs/python-tidal/blob/08742362ee8cef7df1362387cef60c2846aa84ee/tidalapi/media.py#L644) → [`DashInfo.from_mpd()`](https://github.com/EbbLabs/python-tidal/blob/08742362ee8cef7df1362387cef60c2846aa84ee/tidalapi/media.py#L770) → `mpegdash` parser - New MPD contains: `<AdaptationSet ... group="main" ...>` - [`mpegdash` tries to parse group as int](https://github.com/sangwonl/python-mpegdash/blob/48b52122cdbcaebe1804b79f77d05dfc1ce32900/mpegdash/nodes.py#L782) → `ValueError` → `tidalapi.exceptions.ManifestDecodeError` - Mopidy logs: backend exception + _Track is not playable_ **Changes** - **MPD path** (`ManifestMimeType.MPD`) - Stop calling `stream.get_stream_manifest()` (this triggers MPD parsing and the crash) - Use `stream.get_manifest_data()` to fetch raw MPD XML - Write the MPD to `manifest.mpd` in the extension cache directory - Return `file://…/manifest.mpd` for playback (same end result as before, but without parsing) - **BTS path** (ManifestMimeType.BTS) - Keep existing behavior using stream.get_stream_manifest() **Why this is safe** mopidy-tidal already ultimately plays MPD by writing it to disk and handing Mopidy a `file://` URI. The parsed MPD data from tidalapi wasn’t required for playback logic here; it was effectively only used for logging (codecs). Closes: EbbLabs/python-tidal#397
blacklight
changed the title
There was a problem hiding this comment.
Pull request overview
This pull request fixes a regression in TIDAL playback caused by recent changes to TIDAL's MPEG-DASH MPD manifests that include non-numeric group attributes. The fix avoids unnecessary MPD manifest parsing that was causing ValueError exceptions.
Changes:
- Removed top-level call to
stream.get_stream_manifest()that was triggering MPD parsing errors - Moved manifest parsing to only occur for BTS manifest types where it's actually needed
- Split playback logging into separate MPD and BTS code paths, with codec information only logged for BTS
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| logger.info( | ||
| "Starting playback of track:{}, (quality:{}, {}bit/{}Hz)".format( | ||
| track_id, | ||
| stream.audio_quality, | ||
| stream.bit_depth, | ||
| stream.sample_rate, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
The logging statement for MPD playback occurs before verifying that manifest data is available. If get_manifest_data returns None or empty data, the log will indicate playback is starting even though it will fail. Consider moving the logging to after the successful data check or adjusting the message to indicate an attempt rather than definite playback start.
| ) | ||
| ) | ||
|
|
||
| data = stream.get_manifest_data() |
There was a problem hiding this comment.
The new MPD manifest handling logic lacks test coverage. The existing test in test_playback.py is marked as xfail. Consider adding tests to verify that get_manifest_data is called correctly for MPD manifests and that the file is written and returned properly.
Yes, I see no problem that we remove it here to avoid the error caused by python-tidal |
TIDAL appears to have changed the MPEG-DASH MPD manifests returned for some streams to include non-numeric values in
<AdaptationSet group="...">(e.g. group="main").The current
mpegdashparser expects group to be an integer and raises aValueError, whichtidalapiwraps asManifestDecodeError. This causedmopidy-tidalplayback to fail and tracks to be marked “not playable”.This PR updates mopidy-tidal’s playback provider to avoid parsing MPD manifests via tidalapi/mpegdash.
For MPD manifests we only need to persist the MPD XML and return a
file://URI, so parsing is unnecessary.Root cause
tidalapi.StreamManifest→DashInfo.from_mpd()→mpegdashparserNew MPD contains:
<AdaptationSet ... group="main" ...>mpegdashtries to parse group as int →ValueError→tidalapi.exceptions.ManifestDecodeErrorMopidy logs: backend exception + Track is not playable
Changes
ManifestMimeType.MPD)stream.get_stream_manifest()(this triggers MPD parsing and the crash)stream.get_manifest_data()to fetch raw MPD XMLmanifest.mpdin the extension cache directoryfile://…/manifest.mpdfor playback (same end result as before, but without parsing)Why this is safe
mopidy-tidal already ultimately plays MPD by writing it to disk and handing Mopidy a
file://URI.The parsed MPD data from tidalapi wasn’t required for playback logic here; it was effectively only used for logging (codecs).
Closes: EbbLabs/python-tidal#396