Fix DASH progress bar and check that the remux succeeded - #1
Merged
joshtheclipper merged 1 commit intoAug 25, 2026
Conversation
Two fixes for the DASH download path.
TidalDASHDownloadable inherits size(), which HEADs self.url -- for a DASH
track that is only the init segment, a few hundred bytes against a track
of tens of MB. The progress bar's total was therefore tiny, so the first
chunk took it past 100% and every track appeared to start finished:
Track 5 ━━━━━━━━━━━━━━━━━━━━━━━━━ 100.0% • 3.9 MB/s • 0:00:00
Track 6 ━━━━━━━━━━━━━━━━━━━━━━━━━ 100.0% • 5.5 MB/s • 0:00:00
Sum the init segment and every media segment instead. The requests are
bounded to 8 at a time: issuing one per segment at once got about half
of them refused, and counting a refusal as zero put the total at half
the real size (17.0 MB reported against 34.6 MB downloaded). Segments
that still do not answer are estimated at the average of those that did.
Verified against the progress callback: 34.59 MB reported, 34.59 MB
delivered.
Separately, the remux discarded ffmpeg's exit status. If ffmpeg failed
the temp file was removed, no output was written, and _download returned
normally -- so the caller counted the track as downloaded with nothing
on disk, and on current dev recorded it in the downloads database, which
means it is skipped on every later run. Check the exit status and that
the output exists, and raise NonStreamableError otherwise.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
As offered in my comment on #1024 — here are the two fixes, on top of your
dev.Worth saying: the hard part was yours. Working out that Tidal serves HI_RES_LOSSLESS as an MPEG-DASH manifest, and that the old code was silently falling back to a lower quality when the JSON parse failed, is the reason this works at all. These are just two rough edges I hit while running it.
1. Progress bar shows every track as instantly complete
TidalDASHDownloadableinheritssize(), which HEADsself.url— for a DASH track that's only the init segment, a few hundred bytes against a track of tens of MB. So the progress total is tiny and the first chunk takes it past 100%:Now sums the init segment plus every media segment.
One detail that cost me a debugging cycle, in case it looks over-engineered: issuing one HEAD per segment concurrently got roughly half of them refused, and treating a refusal as zero bytes reported 17.0 MB for a track that downloaded 34.6 MB — so the bar was then wrong in the other direction. Hence the semaphore, and estimating any stragglers from the average rather than dropping them.
Verified by counting bytes handed to the progress callback: 34.59 MB reported against 34.59 MB delivered.
2. A failed remux is treated as a successful download
If ffmpeg fails, the temp file is removed, no output is written, and
_downloadreturns normally — so the caller counts the track as downloaded with nothing on disk. On currentdevthat also records it in the downloads database, so it's skipped on every future run and the track is quietly missing for good.Now checks the exit status and that the output exists, and raises
NonStreamableErrorotherwise, with ffmpeg's stderr in the message.Notes
asyncioandoswere already imported at module level, so I dropped the two function-local imports.test_meta.py::test_album_metadata_qobuz, a genre assertion) also fails on an unmodifiednathom/streamripdev, so it's not from this.README.mdalone. My only thought there is that the "Changes from upstream / 🚨 End Fork Changes 🚨" section reads as fork documentation, so it may attract review comments if this goes upstream — entirely your call.Tested against a Tidal "Max" subscription:
q=2returns AAC 320kbps in an m4a,q=4with your changes returns 24-bit FLAC (96kHz on one album, 48kHz on another), decoding cleanly.Happy to change anything here, or split it into two if you'd rather take them separately.