Skip to content

fix(download): refuse SABR streams instead of crashing post-processing#66

Closed
Priveetee wants to merge 1 commit into
InfinityLoop1308:devfrom
Priveetee:fix/2569-sabr-download-refuse-beta2
Closed

fix(download): refuse SABR streams instead of crashing post-processing#66
Priveetee wants to merge 1 commit into
InfinityLoop1308:devfrom
Priveetee:fix/2569-sabr-download-refuse-beta2

Conversation

@Priveetee

Copy link
Copy Markdown
Contributor

Fixes InfinityLoop1308/PipePipe#2569

Summary

Force SABR currently exposes YouTube streams with DeliveryMethod.SABR, but the download stack still expects normal downloadable resources.

This PR refuses SABR downloads early in both the normal download dialog and the playlist/batch download path, with a clear message instead of letting the current downloader crash during post-processing.

Problem

SABR streams are not direct media URLs.

When Force SABR is enabled, the extractor exposes audio/video streams whose content is the serverAbrStreamingUrl. That URL is the SABR session endpoint used by playback. It is not a plain WebM/MP4/DASH resource that the downloader can GET and pass to WebMReader or Mp4DashReader.

Before this change, the downloader could start a mission with that SABR endpoint anyway. The downloaded bytes were then not a valid WebM/MP4 resource, so post-processing failed with errors like:

java.util.NoSuchElementException: expected 0xa45dfa3 found ...
    at org.schabi.newpipe.streams.WebMReader.parse(...)

There is also a cache-related variant of the same bug: if a video was opened while Force SABR was enabled, its StreamInfo can stay in InfoCache for a while. Disabling Force SABR after that does not necessarily refresh the already cached stream list immediately, so the download dialog may still receive SABR streams.

Changes

  • Detect DeliveryMethod.SABR before starting a normal single-video download.
  • Detect DeliveryMethod.SABR before starting playlist/batch downloads through DirectDownloader.
  • Show a clear user-facing message for the visible download dialog.
  • Fail the batch download item cleanly instead of letting it crash later in post-processing.
  • Add one shared string for the unsupported SABR download message.

Validation

Build:

  • ./gradlew :app:compileDebugJavaWithJavac
  • ./gradlew :app:assembleDebug
  • git diff --check

Local code validation:

  • Applied on top of current upstream/dev / v5.2.1-beta2.
  • Patch applies cleanly after the recent upstream changes.
  • Verified the guard covers:
    • selected video stream is SABR
    • secondary audio stream is SABR
    • selected audio-only stream is SABR
    • stale cached StreamInfo still exposing SABR streams after Force SABR was disabled

Notes

This is intentionally not the real SABR download implementation.

A proper SABR downloader is coming, but it needs more implementation and a lot more testing. It cannot just reuse the current direct downloader path. It needs to drive a YoutubeSabrSession, request the selected audio/video formats, collect init/media segments, write or mux them correctly, and handle the same SABR session/protection behavior that playback already has to deal with.

So this PR only closes the crashy unsupported path for now. Users still need to disable Force SABR and reopen the video with fresh non-SABR streams if they want to download before real SABR download support lands.

@InfinityLoop1308

Copy link
Copy Markdown
Owner

The old dash/http path are all deprecated now. Only SABR and HLS (only available on web_safari) are usable. That means if refuse SABR, users setting youtube extraction endpoint to mweb or web won't be able to download anymore.

I think maybe easier - always fetch data from web_safari, and only use HLS for downloading before SABR download landing. What's your opionion?

@InfinityLoop1308

Copy link
Copy Markdown
Owner

I use an alternative way: always use hls for now.

@Priveetee

Copy link
Copy Markdown
Contributor Author

@InfinityLoop1308 Sorry for the late answer, it was night here :)

Yeah I agree with you. Refusing SABR was only meant to stop the current crashy path, but if we can force/download through HLS for now, that is better for users than blocking downloads entirely.

So your temporary HLS path makes sense to me.

For the real implementation, I’m working on proper SABR download support. It needs more than just passing the SABR endpoint to the current downloader: it has to drive the SABR session, request the selected audio/video formats, collect init/media segments, and then write/mux them correctly.

So yeah, HLS for now, real SABR download later.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Encountering Error when Downloading with Force SABR enabled

2 participants