Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Sources/Kaset/Models/YouTube/YouTubeFeed.swift
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,17 @@ struct YouTubeCaptionTrack: Identifiable, Hashable {
}
}

// MARK: - YouTubeAudioTrack

/// An alternate audio track (dubbed language) offered by the watch page player.
/// `id` is the track's index in the player's live audio-track array, which is
/// all `setAudioTrack` needs — the opaque player track objects never cross the
/// JS bridge.
struct YouTubeAudioTrack: Identifiable, Hashable {
let id: String
let displayName: String
}

// MARK: - YouTubeQuality

/// Display helpers for YouTube's quality-level identifiers.
Expand Down
21 changes: 21 additions & 0 deletions Sources/Kaset/Services/Player/YouTubePlayerService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ protocol YouTubeWatchPlaybackControlling: AnyObject {
func availableCaptionTracks() async -> [YouTubeCaptionTrack]
func currentCaptionLanguageCode() async -> String?
func setCaptionTrack(languageCode: String?)
func availableAudioTracks() async -> [YouTubeAudioTrack]
func currentAudioTrackId() async -> String?
func setAudioTrack(id: String)
Comment on lines +37 to +39

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The test target's MockYouTubeWatchPlaybackController conforms directly to this protocol but does not implement these three newly required methods, so adding the requirements causes the test target to fail to compile until the mock is updated or default protocol implementations are provided. [api mismatch]

Severity Level: Critical 🚨
- ❌ Test target fails compilation for every build.
- ❌ All service tests are blocked by protocol nonconformance.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** Sources/Kaset/Services/Player/YouTubePlayerService.swift
**Line:** 37:39
**Comment:**
	*Api Mismatch: The test target's `MockYouTubeWatchPlaybackController` conforms directly to this protocol but does not implement these three newly required methods, so adding the requirements causes the test target to fail to compile until the mock is updated or default protocol implementations are provided.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

func availableQualityLevels() async -> [String]
func currentQualityLevel() async -> String?
func setQualityLevel(_ level: String)
Expand Down Expand Up @@ -453,6 +456,13 @@ final class YouTubePlayerService {
/// Language code of the active caption track (nil = captions off).
private(set) var activeCaptionLanguageCode: String?

/// Alternate audio tracks (dubbed languages) on the current watch page.
/// Empty when the video has a single audio track.
private(set) var audioTracks: [YouTubeAudioTrack] = []

/// Id of the active audio track (nil until resolved).
private(set) var activeAudioTrackId: String?

/// Quality levels available on the current watch page.
private(set) var qualityLevels: [String] = []

Expand Down Expand Up @@ -1171,6 +1181,8 @@ final class YouTubePlayerService {
self.heatmap = []
self.captionTracks = []
self.activeCaptionLanguageCode = nil
self.audioTracks = []
self.activeAudioTrackId = nil
self.qualityLevels = []
self.currentQuality = nil
self.userPinnedQuality = nil
Expand Down Expand Up @@ -1247,6 +1259,8 @@ final class YouTubePlayerService {
: levels + ["auto"]
self.currentQuality = await self.playbackController.currentQualityLevel()
self.activeCaptionLanguageCode = await self.playbackController.currentCaptionLanguageCode()
self.audioTracks = await self.playbackController.availableAudioTracks()
self.activeAudioTrackId = await self.playbackController.currentAudioTrackId()
Comment on lines +1262 to +1263

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: These awaits can suspend while the user navigates to another video. The only video-id guard runs before the quality, caption-state, and audio queries, so an old refresh can resume afterward and overwrite the new video's audioTracks and activeAudioTrackId state. Recheck currentVideo?.videoId == videoId immediately before applying these results, or associate the refresh with a generation token. [stale reference]

Severity Level: Critical 🚨
- ⚠️ New video can display previous audio tracks.
- ⚠️ Audio checkmark can identify the wrong active track.
- ⚠️ Quality and caption state can also be overwritten by the same race.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** Sources/Kaset/Services/Player/YouTubePlayerService.swift
**Line:** 1262:1263
**Comment:**
	*Stale Reference: These awaits can suspend while the user navigates to another video. The only video-id guard runs before the quality, caption-state, and audio queries, so an old refresh can resume afterward and overwrite the new video's `audioTracks` and `activeAudioTrackId` state. Recheck `currentVideo?.videoId == videoId` immediately before applying these results, or associate the refresh with a generation token.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


if !tracks.isEmpty || attempt == 2 {
return
Expand Down Expand Up @@ -1308,6 +1322,13 @@ final class YouTubePlayerService {
HapticService.toggle()
}

/// Selects an alternate audio track (dubbed language) by id.
func selectAudioTrack(id: String) {
self.activeAudioTrackId = id
self.playbackController.setAudioTrack(id: id)
HapticService.toggle()
Comment on lines +1325 to +1329

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The service marks the requested track active before the JavaScript switch succeeds, while setAudioTrack has no completion result and swallows all player errors. If the player disappears during navigation or rejects the switch, the checkmark remains on the requested track even though playback still uses the previous track. Update the state only after confirming the switch, or refresh the active track after the command completes. [logic error]

Severity Level: Major ⚠️
- ⚠️ Audio menu checkmark can disagree with actual playback.
- ⚠️ Navigation or player errors can leave stale selection state.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** Sources/Kaset/Services/Player/YouTubePlayerService.swift
**Line:** 1325:1329
**Comment:**
	*Logic Error: The service marks the requested track active before the JavaScript switch succeeds, while `setAudioTrack` has no completion result and swallows all player errors. If the player disappears during navigation or rejects the switch, the checkmark remains on the requested track even though playback still uses the previous track. Update the state only after confirming the switch, or refresh the active track after the command completes.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

}

/// Selects a playback quality level.
func selectQuality(_ level: String) {
self.currentQuality = level
Expand Down
46 changes: 46 additions & 0 deletions Sources/Kaset/Views/YouTube/YouTubePlayerBar.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
/// the watch page response exposes chapters.
/// - The minimize button drives the video pop-out (picture in picture);
/// the TV button toggles fullscreen on the popped-out window.
struct YouTubePlayerBar: View {

Check failure on line 17 in Sources/Kaset/Views/YouTube/YouTubePlayerBar.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Struct body should span 700 lines or less excluding comments and whitespace: currently spans 961 lines (type_body_length)
/// Where these controls are shown.
enum Mode {
/// The docked transport bar at the bottom of the window (default).
Expand Down Expand Up @@ -579,6 +579,7 @@
self.downloadButton
}
self.compactCaptionsMenu
self.compactAudioMenu
self.compactQualityMenu
case .reduced:
self.overflowMenu
Expand Down Expand Up @@ -719,6 +720,14 @@
Label(String(localized: "Captions"), systemImage: "captions.bubble")
}

if self.hasAlternateAudio {
Menu {
self.audioMenuItems
} label: {
Label(String(localized: "Audio"), systemImage: "waveform.circle")
}
}

Menu {
self.qualityMenuItems
} label: {
Expand Down Expand Up @@ -777,6 +786,43 @@
.disabled(self.youtubePlayer.currentVideo == nil)
}

/// True only for multi-audio videos; single-track videos hide the control.
private var hasAlternateAudio: Bool {
self.youtubePlayer.audioTracks.count > 1
}

@ViewBuilder private var audioMenuItems: some View {
ForEach(self.youtubePlayer.audioTracks) { track in
Button {
self.youtubePlayer.selectAudioTrack(id: track.id)
} label: {
if self.youtubePlayer.activeAudioTrackId == track.id {
Label(track.displayName, systemImage: "checkmark")
} else {
Text(track.displayName)
}
}
}
}

@ViewBuilder private var compactAudioMenu: some View {
if self.hasAlternateAudio {
Menu {
self.audioMenuItems
} label: {
Image(systemName: "waveform.circle")
.font(.system(size: 16, weight: .regular))
.frame(width: 16, height: 16)
.foregroundStyle(.primary)
}
.menuStyle(.borderlessButton)
.menuIndicator(.hidden)
.buttonStyle(.plain)
.frame(width: 28, height: 28)
.disabled(self.youtubePlayer.currentVideo == nil)
}
}

@ViewBuilder private var qualityMenuItems: some View {
Text("Speed").font(.caption.weight(.semibold)).foregroundStyle(.secondary)
ForEach([0.5, 0.75, 1.0, 1.25, 1.5, 2.0], id: \.self) { speed in
Expand Down Expand Up @@ -1119,4 +1165,4 @@
static let watchFullView = "youtubeContent.watchFullView"
static let captionsButton = "youtubeContent.captionsButton"
static let qualityButton = "youtubeContent.qualityButton"
}

Check failure on line 1168 in Sources/Kaset/Views/YouTube/YouTubePlayerBar.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

File should contain 900 lines or less: currently contains 1168 (file_length)
118 changes: 118 additions & 0 deletions Sources/Kaset/Views/YouTube/YouTubeWatchWebView+Scripts.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1301,6 +1301,124 @@ extension YouTubeWatchWebView {
"""
}

/// Fetches the alternate audio tracks (dubbed languages) the player offers.
/// Switching is by index into the player's live track array, so the opaque
/// player track objects never have to cross the JS bridge.
func availableAudioTracks() async -> [YouTubeAudioTrack] {
guard let json = await self.evaluateForString(Self.availableAudioTracksScript),
let data = json.data(using: .utf8),
let entries = try? JSONSerialization.jsonObject(with: data) as? [[String: String]]
else {
return []
}
return entries.compactMap { entry in
guard let id = entry["id"], !id.isEmpty,
let name = entry["name"], !name.isEmpty
else {
return nil
}
return YouTubeAudioTrack(id: id, displayName: name)
}
}

/// The index (as a string) of the player's active audio track, or nil.
func currentAudioTrackId() async -> String? {
let script = """
(function() {
try {
const p = document.getElementById('movie_player');
if (!p || typeof p.getAvailableAudioTracks !== 'function') { return ''; }
const tracks = p.getAvailableAudioTracks() || [];
let cur = null;
try { cur = p.getAudioTrack(); } catch (e) {}
for (let i = 0; i < tracks.length; i++) {
if (tracks[i] === cur) { return String(i); }
}
return '';
} catch (e) { return ''; }
})();
"""
let id = await self.evaluateForString(script)
return (id?.isEmpty == false) ? id : nil
}

/// Activates the audio track at the given index in the player's live array.
func setAudioTrack(id: String) {
self.webView?.evaluateJavaScript(Self.setAudioTrackScript(id: id), completionHandler: nil)
}

static var availableAudioTracksScript: String {
// ponytail: display names are heuristic — first from the opaque track
// object, else the player-response `audioTrack.displayName` by order,
// else a generic "Audio N". Switching (by index) is exact regardless.
// If names come out wrong on a multi-audio video, dump
// JSON.stringify(getAvailableAudioTracks()) and map the real name key.
"""
(function() {
try {
const p = document.getElementById('movie_player');
if (!p || typeof p.getAvailableAudioTracks !== 'function') { return '[]'; }
let tracks = [];
try { tracks = p.getAvailableAudioTracks() || []; } catch (e) { return '[]'; }
if (tracks.length < 2) { return '[]'; }

function playerResponse() {
if (typeof p.getPlayerResponse === 'function') {
try { const r = p.getPlayerResponse(); if (r) { return r; } } catch (e) {}
}
return window.ytInitialPlayerResponse || null;
}
const responseNames = [];
const seenName = {};
const resp = playerResponse();
const fmts = (resp && resp.streamingData && resp.streamingData.adaptiveFormats) || [];
fmts.forEach(function(f) {
const dn = f && f.audioTrack && f.audioTrack.displayName;
if (dn && !seenName[dn]) { seenName[dn] = true; responseNames.push(dn); }
});
Comment on lines +1373 to +1378

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: responseNames is deduplicated across adaptive formats and then indexed against the live audio-track array. Adaptive formats can contain several formats for one language or use an order different from the player track array, so the menu can display one language while selecting a different track. Map each live track using its own identifying metadata rather than assuming these arrays share the same order. [logic error]

Severity Level: Major ⚠️
- ⚠️ Multi-audio menu can show incorrect language labels.
- ⚠️ Users may select tracks based on misleading names.
- ⚠️ Playback switching remains exact but discoverability is impaired.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** Sources/Kaset/Views/YouTube/YouTubeWatchWebView+Scripts.swift
**Line:** 1373:1378
**Comment:**
	*Logic Error: `responseNames` is deduplicated across adaptive formats and then indexed against the live audio-track array. Adaptive formats can contain several formats for one language or use an order different from the player track array, so the menu can display one language while selecting a different track. Map each live track using its own identifying metadata rather than assuming these arrays share the same order.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


function nameFrom(track, i) {
try {
for (const k in track) {
const v = track[k];
if (v && typeof v === 'object') {
if (typeof v.name === 'string' && v.name) { return v.name; }
if (v.name && typeof v.name.simpleText === 'string' && v.name.simpleText) {
return v.name.simpleText;
}
}
}
} catch (e) {}
if (responseNames[i]) { return responseNames[i]; }
return 'Audio ' + (i + 1);
}

return JSON.stringify(tracks.map(function(track, i) {
return { id: String(i), name: nameFrom(track, i) };
}));
} catch (e) { return '[]'; }
})();
"""
}

static func setAudioTrackScript(id: String) -> String {
let idLiteral = Self.jsStringLiteral(id)
return """
(function() {
try {
const p = document.getElementById('movie_player');
if (!p || typeof p.getAvailableAudioTracks !== 'function'
|| typeof p.setAudioTrack !== 'function') { return; }
const tracks = p.getAvailableAudioTracks() || [];
const idx = parseInt(\(idLiteral), 10);
if (idx >= 0 && idx < tracks.length) {
try { p.setAudioTrack(tracks[idx]); } catch (e) {}
}
} catch (e) {}
})();
"""
}

/// The storyboard spec string for the current video, read from the player
/// response. Drives the ambient backdrop's fine-grained live color.
///
Expand Down
Loading