Skip to content

Add audio/video toggle for HLS streams#4686

Open
leandroalonso wants to merge 9 commits into
trunkfrom
pcios-811/hls-add-toggle
Open

Add audio/video toggle for HLS streams#4686
leandroalonso wants to merge 9 commits into
trunkfrom
pcios-811/hls-add-toggle

Conversation

@leandroalonso

@leandroalonso leandroalonso commented Jul 6, 2026

Copy link
Copy Markdown
Member

Fixes PCIOS-811

Adds an audio/video toggle to the player shelf for HLS streams that carry video.

When an HLS stream is found to contain video, a new Show/Hide Video action appears on the player shelf. Tapping it switches the Now Playing screen between the video surface and the episode artwork, letting the user listen audio-only. The whole feature is gated behind the existing FeatureFlag.hls flag and only shows for HLS streams found to carry video (not for regular video podcasts).

Notable details

  • Used same icons as Android
  • New PlayerAction.videoToggle case, added last in the default actions (so it defaults into the overflow menu; users can reorder it).
  • PlaybackManager gains videoRenderingEnabled state plus shouldRenderVideo() / canToggleVideoRendering() / toggleVideoRendering(). The state resets per episode.
  • The toggle is display-only: it swaps the in-app surface but does not change the system now-playing media type or stop video decoding.
  • Fixed a first-time blank-artwork issue by forcing an artwork reload + layout pass when switching video → audio.

To test

  1. Play an episode that has an HLS stream containing video.
  2. Open the full-screen player; once the video is detected the artwork is replaced by the video surface.
  3. Open the shelf overflow (•••) — a Hide Video action appears (or drag it onto the shelf via Rearrange Actions).
  4. Tap it: the video is replaced by the episode artwork and the icon/label flips to Show Video.
  5. Tap again: the video surface returns.

Checklist

  • I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
  • I have considered adding unit tests for my changes.
  • I have updated (or requested that someone edit) the Event Horizon schema to reflect any new or changed analytics.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds an in-player audio/video visibility toggle for HLS streams that contain video, letting users switch Now Playing between the video surface and episode artwork while continuing audio playback. The feature is gated by FeatureFlag.hls and only appears when runtime stream inspection detects video tracks.

Changes:

  • Introduces a new PlayerAction.videoToggle with localized title/icon and shelf + overflow handling.
  • Adds PlaybackManager state for videoRenderingEnabled and helpers to determine availability and whether the video surface should be rendered.
  • Updates Now Playing UI to show/hide the floating video surface and force an artwork refresh when switching video → audio.

Reviewed changes

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

Show a summary per file
File Description
podcasts/ShelfLoadState.swift Extends shelf reload caching to include video toggle availability + rendering state.
podcasts/ShelfActionsViewController+Table.swift Wires overflow menu tap handling for the new .videoToggle action.
podcasts/PlayerZoomAnimator.swift Uses shouldRenderVideo() to decide video vs artwork transition behavior.
podcasts/PlayerImages.xcassets/video_on.imageset/video_on.svg Adds “video on” shelf icon (SVG).
podcasts/PlayerImages.xcassets/video_on.imageset/Contents.json Registers video_on asset with template rendering.
podcasts/PlayerImages.xcassets/video_off.imageset/video_off.svg Adds “video off” shelf icon (SVG).
podcasts/PlayerImages.xcassets/video_off.imageset/Contents.json Registers video_off asset with template rendering.
podcasts/PlaybackManager.swift Adds video rendering state + toggle APIs; resets state per episode.
podcasts/NowPlayingPlayerItemViewController+Update.swift Updates Now Playing surface switching logic and forces artwork reload when hiding video.
podcasts/NowPlayingPlayerItemViewController+Shelf.swift Adds shelf button + delegate handling for .videoToggle.
podcasts/NowPlayingPlayerItemViewController.swift Uses shouldRenderVideo() when deciding whether to open the full video controller.
podcasts/MiniPlayerToFullPlayerAnimator.swift Uses shouldRenderVideo() for transition decisions.
podcasts/Enumerations.swift Adds analytics mapping, title/icon, and applicability for .videoToggle.
podcasts/en.lproj/Localizable.strings Adds “Show Video” / “Hide Video” localized strings.
PocketCastsTests/Tests/Utilities/SettingsTests.swift Updates expected default player action list to include .videoToggle.
Modules/Sources/PocketCastsUtils/General/AtomicBool.swift Adds AtomicBool initializer for setting initial value.
Modules/Sources/PocketCastsDataModel/Public/Enums.swift Adds PlayerAction.videoToggle to the shared model enum.

Comment thread podcasts/PlaybackManager.swift
Comment thread podcasts/PlaybackManager.swift Outdated
…y docstring/guard

Post a distinct videoRenderingToggled notification from toggleVideoRendering()
instead of reusing videoPlaybackEngineSwitched, so the user toggle no longer
feeds MiniPlayerViewController's auto-open path (only runtime video detection
does). The Now Playing screen observes both notifications.

Also guard toggleVideoRendering() on canToggleVideoRendering() and correct the
docstring: the toggle is display-only and does not stop video decoding.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leandroalonso

Copy link
Copy Markdown
Member Author

Review feedback addressed

  • PlaybackManager.swift (@copilot): toggleVideoRendering() reused videoPlaybackEngineSwitched, which MiniPlayerViewController uses to auto-open the full player — it now posts a dedicated videoRenderingToggled notification so the toggle can't trigger auto-open. Done in 023faab
  • PlaybackManager.swift (@copilot): Corrected the toggleVideoRendering() docstring (display-only; does not stop decoding or change system media type) and added a canToggleVideoRendering() guard. Done in 023faab

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread podcasts/PlayerZoomAnimator.swift Outdated
Comment thread podcasts/MiniPlayerToFullPlayerAnimator.swift Outdated
…tors

The property now returns shouldRenderVideo(), which reflects whether the video
surface is currently shown (false when the user toggles video off), so the old
name was misleading.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leandroalonso

Copy link
Copy Markdown
Member Author

Review feedback addressed

  • PlayerZoomAnimator.swift:16 (@copilot): Renamed misleading isVideoPodcast (now backed by shouldRenderVideo()) to isVideoShown and updated its call sites. Done in d82dc26
  • MiniPlayerToFullPlayerAnimator.swift:44 (@copilot): Same rename to isVideoShown and updated call sites. Done in d82dc26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread Modules/Sources/PocketCastsUtils/General/AtomicBool.swift
Comment thread podcasts/PlaybackManager.swift
Comment thread podcasts/NowPlayingPlayerItemViewController+Update.swift Outdated
leandroalonso and others added 2 commits July 6, 2026 14:10
…endering

value.toggle() is a separate read + write and can race. Add an atomic toggle()
that flips storageValue on the serial queue, and use it in toggleVideoRendering().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
update() already reloads the artwork at the end of the method, so the explicit
loadImage in the hide branch was a redundant second call. Keep only the layout
pass that fixes the first-time blank artwork.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leandroalonso

Copy link
Copy Markdown
Member Author

Review feedback addressed

  • AtomicBool.swift:17 (@copilot): Added an atomic toggle() (queue-synchronized read/modify/write, @discardableResult). Done in 22fbc0c
  • PlaybackManager.swift:781 (@copilot): toggleVideoRendering() now uses the atomic AtomicBool.toggle() instead of value.toggle(). Done in 22fbc0c
  • NowPlayingPlayerItemViewController+Update.swift:68 (@copilot): Removed the duplicate loadImage in the hide branch (update() already reloads at the end); kept the layout pass. Done in 45ed7d5

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 18 changed files in this pull request and generated no new comments.

@leandroalonso leandroalonso marked this pull request as ready for review July 6, 2026 17:18
@leandroalonso leandroalonso requested a review from a team as a code owner July 6, 2026 17:18
@leandroalonso leandroalonso requested review from kean and removed request for a team July 6, 2026 17:18
@leandroalonso leandroalonso modified the milestones: 8.16, 8.17 Jul 6, 2026
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.

2 participants