fix(link): filter unsupported YouTube qualities from media metadata#79
fix(link): filter unsupported YouTube qualities from media metadata#79
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughFilters YouTube yt-dlp JSON format heights to a canonical allow-list {360, 480, 720, 1080, 1440, 2160, 4320} for YouTube sources only; adds detection helpers and unit tests; documents the changelog entry. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR fixes a "Requested format is not available" error by filtering Confidence Score: 5/5Safe to merge — targeted, well-tested fix with no regressions introduced. All findings are P2 or lower. The filtering logic is correct, properly scoped to YouTube sources, and backed by five dedicated regression tests. No data integrity, security, or correctness issues were identified. No files require special attention.
|
| Filename | Overview |
|---|---|
| src-tauri/src/adapters/driving/tauri_ipc.rs | Adds SUPPORTED_YOUTUBE_HEIGHTS constant, is_supported_youtube_height helper, and is_youtube_source detector; threads the filter into parse_ytdlp_json's quality collection loop; backs it with 5 new regression tests covering YouTube drops, non-standard heights, Vimeo passthrough, youtu.be domain fallback, and unknown-source passthrough. |
| CHANGELOG.md | Adds a clear Fixed entry under [Unreleased] describing the root cause, the allow-list values, and the YouTube-source scoping — accurate and complete. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[command_get_media_metadata\nyt-dlp --dump-single-json] --> B[parse_ytdlp_json]
B --> C{is_youtube_source?}
C -- extractor_key contains 'youtube'\nor webpage_url_domain matches --> D[youtube_only = true]
C -- neither matches --> E[youtube_only = false]
D --> F{height > 0 AND\nis_supported_youtube_height?}
E --> G{height > 0?}
F -- yes\n360/480/720/1080/1440/2160/4320 --> H[Include in available_qualities]
F -- no\n144/240/270/1072/etc. --> I[Drop — not in allow-list]
G -- yes --> H
G -- no --> I
H --> J[Sort by height DESC\nDeduplicate by height]
J --> K[MediaMetadataDto\navailableQualities]
Reviews (3): Last reviewed commit: "fix(link): scope YouTube height filter t..." | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/adapters/driving/tauri_ipc.rs`:
- Around line 1171-1175: The filtering that applies is_supported_youtube_height
to every format in video_formats is currently unscoped and removes valid heights
for non-YouTube sources; modify the logic in parse_ytdlp_json so the
is_supported_youtube_height filter is only applied when the parsed metadata
indicates a YouTube source (e.g., check the extractor/source field such as
"extractor" or a "webpage_url" host contains "youtube") and skip that filter for
Vimeo/SoundCloud/other extractors—update the video_formats construction (the
block that builds video_formats) to conditionally include the height check only
when the source is YouTube while leaving other filters (like vcodec != "none")
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bf19ecbb-c46b-4c87-b957-b5dc81b715c6
📒 Files selected for processing (2)
CHANGELOG.mdsrc-tauri/src/adapters/driving/tauri_ipc.rs
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src-tauri/src/adapters/driving/tauri_ipc.rs">
<violation number="1" location="src-tauri/src/adapters/driving/tauri_ipc.rs:1174">
P2: The new quality filter applies the YouTube height allow-list to every provider, which can hide valid non-YouTube qualities and break quality selection for those sources.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| .iter() | ||
| .filter(|f| f["vcodec"].as_str().unwrap_or("none") != "none") | ||
| .filter(|f| f["height"].as_u64().unwrap_or(0) > 0) | ||
| .filter(|f| is_supported_youtube_height(f["height"].as_u64().unwrap_or(0) as u32)) |
There was a problem hiding this comment.
P2: The new quality filter applies the YouTube height allow-list to every provider, which can hide valid non-YouTube qualities and break quality selection for those sources.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src-tauri/src/adapters/driving/tauri_ipc.rs, line 1174:
<comment>The new quality filter applies the YouTube height allow-list to every provider, which can hide valid non-YouTube qualities and break quality selection for those sources.</comment>
<file context>
@@ -1150,11 +1162,16 @@ fn parse_ytdlp_json(json: &serde_json::Value) -> Result<MediaMetadataDto, String
.iter()
.filter(|f| f["vcodec"].as_str().unwrap_or("none") != "none")
- .filter(|f| f["height"].as_u64().unwrap_or(0) > 0)
+ .filter(|f| is_supported_youtube_height(f["height"].as_u64().unwrap_or(0) as u32))
.collect();
video_formats.sort_by(|a, b| {
</file context>
Review feedback on PR #79 (CodeRabbit + Cubic): `parse_ytdlp_json` is called unconditionally for every URL the media grabber resolves, not just YouTube ones. Applying the `SUPPORTED_YOUTUBE_HEIGHTS` allow-list to Vimeo/SoundCloud/other extractors would hide valid heights those providers expose (Vimeo serves 540p, for example), silently regressing quality selection for non-YouTube sources. Detect YouTube via yt-dlp's own `extractor_key` (case-insensitive contains "youtube") with a `webpage_url_domain` fallback covering youtube.com / www.youtube.com / m.youtube.com / music.youtube.com / youtu.be. Only when the payload is identified as YouTube does the canonical-ladder filter engage; otherwise any positive height passes through. Renamed the two existing regression tests to `drops_youtube_*_heights` to reflect the scoping and added three new ones: - `preserves_vimeo_non_canonical_heights`: 240/360/540/720/1080 all survive on a Vimeo payload. - `detects_youtube_via_webpage_domain_only`: filter still fires when `extractor_key` is absent but the URL is youtu.be. - `unknown_source_keeps_all_positive_heights`: payloads without any source hint keep every positive height (including 144p/540p) so new providers are not silently broken.
The grabber dialog was offering 144p (and any non-canonical height
yt-dlp reports, e.g. 240p, 270p, 1072p) even though vortex-mod-youtube
only declares {360, 480, 720, 1080, 1440, 2160, 4320} in its plugin.toml
`default_quality` options. Picking one of those produced a raw
"Requested format is not available" error because
`resolve_stream_url` only bypasses its pre-merged-HTTPS path for heights
>= 720 on the canonical ladder; 144p and 240p fall through and yt-dlp
refuses the selector.
`parse_ytdlp_json` now filters `available_qualities` against a
`SUPPORTED_YOUTUBE_HEIGHTS` allow-list kept in sync with the plugin
manifest, so the UI never surfaces a resolution the plugin cannot fulfill.
Covered by two regression tests: explicit 144p/240p drop, and
non-canonical heights (270, 1072) dropped while keeping 1080 and 2160.
Review feedback on PR #79 (CodeRabbit + Cubic): `parse_ytdlp_json` is called unconditionally for every URL the media grabber resolves, not just YouTube ones. Applying the `SUPPORTED_YOUTUBE_HEIGHTS` allow-list to Vimeo/SoundCloud/other extractors would hide valid heights those providers expose (Vimeo serves 540p, for example), silently regressing quality selection for non-YouTube sources. Detect YouTube via yt-dlp's own `extractor_key` (case-insensitive contains "youtube") with a `webpage_url_domain` fallback covering youtube.com / www.youtube.com / m.youtube.com / music.youtube.com / youtu.be. Only when the payload is identified as YouTube does the canonical-ladder filter engage; otherwise any positive height passes through. Renamed the two existing regression tests to `drops_youtube_*_heights` to reflect the scoping and added three new ones: - `preserves_vimeo_non_canonical_heights`: 240/360/540/720/1080 all survive on a Vimeo payload. - `detects_youtube_via_webpage_domain_only`: filter still fires when `extractor_key` is absent but the URL is youtu.be. - `unknown_source_keeps_all_positive_heights`: payloads without any source hint keep every positive height (including 144p/540p) so new providers are not silently broken.
3946d72 to
918213b
Compare
Summary
Failed to resolve stream URL: [...] yt-dlp failed (exit code 1): ERROR: [youtube] <id>: Requested format is not available.command_get_media_metadata(tauri_ipc.rs :: parse_ytdlp_json) fed every height yt-dlp reported intoavailableQualities. yt-dlp returns 144p/240p for most YouTube videos (as DASH-only streams), so the UI offered them — butvortex-mod-youtubeonly declares{360, 480, 720, 1080, 1440, 2160, 4320}in itsplugin.tomland itsresolve_stream_urlonly bypasses the pre-merged-HTTPS path for heights>= 720. 144p fell through, hit yt-dlp with an impossible format selector, and blew up.availableQualitiesagainst aSUPPORTED_YOUTUBE_HEIGHTSallow-list kept in sync withplugin.toml :: default_quality.options. Non-canonical heights (270p, 1072p, etc.) are also dropped so only ladder entries reach the UI.Why filter in
tauri_ipc.rsand not in the plugincommand_get_media_metadatais a YouTube/yt-dlp-specific IPC that lives in Vortex core (it shells out toyt-dlp --dump-single-jsondirectly). It is the source ofavailableQualitiesfor the media grabber, so filtering there stops unsupported options at the shortest, most authoritative hop. A future per-plugin metadata contract can replace the allow-list with capability introspection.Test plan
cargo test --workspace— 592 passed (2 new regression tests inparse_ytdlpsuite:test_parse_ytdlp_drops_heights_below_360p,test_parse_ytdlp_drops_non_standard_heights)cargo fmt --checkcargo clippy— no new warnings on the touched file (pre-existing warnings elsewhere unrelated)npx vitest run— 367 passed{360p, 480p, 720p, 1080p, …}appear (no 144p / 240p)Files touched
src-tauri/src/adapters/driving/tauri_ipc.rs—SUPPORTED_YOUTUBE_HEIGHTSallow-list +is_supported_youtube_heighthelper + filter inparse_ytdlp_json+ 2 regression testsCHANGELOG.md— Fixed entry under[Unreleased]Summary by cubic
Filter YouTube qualities in media metadata so the link grabber only shows resolutions the
vortex-mod-youtubeplugin supports, preventing “Requested format is not available” errors when selecting 144p/240p or odd heights.tauri_ipc.rs,parse_ytdlp_jsonnow allow-lists YouTube heights{360, 480, 720, 1080, 1440, 2160, 4320}viaSUPPORTED_YOUTUBE_HEIGHTS.available_qualities; added two regression tests to cover this.Written for commit dd0db32. Summary will update on new commits.
Summary by CodeRabbit