Skip to content

Rewrite tidal streaming#3518

Draft
jozefKruszynski wants to merge 2 commits intodevfrom
rewrite-tidal-streaming
Draft

Rewrite tidal streaming#3518
jozefKruszynski wants to merge 2 commits intodevfrom
rewrite-tidal-streaming

Conversation

@jozefKruszynski
Copy link
Copy Markdown
Contributor

DASH manifests are now parsed by the provider rather than being handed off to ffmpeg.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR changes Tidal playback for DASH manifests to be handled inside the Tidal provider instead of delegating DASH parsing/fetching behavior to ffmpeg, by converting DASH playback into a StreamType.CUSTOM stream that yields init + media segments.

Changes:

  • Refactors TidalStreamingManager.get_stream_details to parse base64 DASH MPDs and return StreamType.CUSTOM stream details with segment metadata in StreamDetails.data.
  • Adds provider-side audio streaming for Tidal via a new get_audio_stream implementation that downloads and yields the init segment and subsequent media segments (with basic seek support).
  • Updates/extends Tidal streaming tests to validate manifest parsing, segment extraction, seeking behavior, and error handling.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
music_assistant/providers/tidal/streaming.py Implements DASH MPD parsing and CUSTOM audio streaming by downloading and yielding init + media segments.
music_assistant/providers/tidal/provider.py Exposes provider-level get_audio_stream to support StreamType.CUSTOM streams from the streaming manager.
tests/providers/tidal/test_streaming.py Updates existing DASH tests and adds coverage for MPD parsing and segment-streaming/seek behavior.

Comment thread music_assistant/providers/tidal/streaming.py Outdated
Comment thread music_assistant/providers/tidal/streaming.py Outdated
@marcelveldt
Copy link
Copy Markdown
Member

Can you explain why this change is needed ? It would be more efficient to let ffmpeg deal with the heavy lifting.
If there's an issue with ffmpeg's DASH demuxer, we can explore if we should update to ffmpeg 8.

If this is about handling over the dash playlist/manifest as base64 string, why not host an extra endpoint on the streamserver that serves the dash manifest ?

@jozefKruszynski
Copy link
Copy Markdown
Contributor Author

Can you explain why this change is needed ? It would be more efficient to let ffmpeg deal with the heavy lifting. If there's an issue with ffmpeg's DASH demuxer, we can explore if we should update to ffmpeg 8.

ffmpeg still does all the heavy lifting, it decodes the FLAC-in-fMP4, applies volume normalization, resampling, crossfade mixing, and encodes the output format. I'm only bypassing the demuxer, which appears to be what is the root cause if the initial bug.

The failure mode is that ffmpeg's DASH demuxer silently stops fetching segments partway through the track (return code 0, no error), producing a truncated stream. It's not a crash or timeout — it just decides it's done early. This happened repeatedly during testing with both data: URIs and HTTP-served manifests.

If this is about handling over the dash playlist/manifest as base64 string, why not host an extra endpoint on the streamserver that serves the dash manifest ?

That's exactly what PR #3369 did, but I introduced a new bug that we saw in discord, dynamic routes get orphaned or expire before playback completes, causing 404s.

I fixed this bug, but then found that the exact same behaviour of streams prematurely stopping was still present.

@marcelveldt
Copy link
Copy Markdown
Member

maybe check the ffmpeg bugtracker if this was an issue they fixed ?

@jozefKruszynski
Copy link
Copy Markdown
Contributor Author

Will do.

@jozefKruszynski
Copy link
Copy Markdown
Contributor Author

jozefKruszynski commented Mar 31, 2026

There was a fix that was added to address a very similar issue to what we're seeing. Unfortunately, that fix was already added to the 7.1 branch, so our 7.1.2 version should already have it.

I'll look further into it.

Edit: looks like there is a relevant bugfix that might help.
I'll do some testing when I next get the opportunity.

@MarvinSchenkel MarvinSchenkel marked this pull request as draft April 2, 2026 07:30
@MarvinSchenkel
Copy link
Copy Markdown
Contributor

Can you mark this as ready for review when you want another review please? :)

@jozefKruszynski
Copy link
Copy Markdown
Contributor Author

Will do, I should hopefully have some time to test tomorrow again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants