Skip to content

Feature/mediacontrol api - #785

Open
tvanlaerhoven wants to merge 60 commits into
developfrom
feature/mediacontrol-api
Open

Feature/mediacontrol api#785
tvanlaerhoven wants to merge 60 commits into
developfrom
feature/mediacontrol-api

Conversation

@tvanlaerhoven

@tvanlaerhoven tvanlaerhoven commented Mar 25, 2026

Copy link
Copy Markdown
Member

Add MediaControl API.

  • Updated list of example sources to include posters, descriptions and consistent metadata.
  • Added a custom usePlaylist hook to allow switching sources through mediaSession & lock-screen controls.

Caution

Android needs an update of the media-session connector.


Open in Devin Review

@tvanlaerhoven
tvanlaerhoven force-pushed the feature/mediacontrol-api branch from 043c11c to 7246a54 Compare March 25, 2026 09:02
@tvanlaerhoven tvanlaerhoven added the enhancement New feature or request label Mar 25, 2026
@tvanlaerhoven
tvanlaerhoven force-pushed the feature/mediacontrol-api branch from 93ba7c2 to 0a8a959 Compare April 23, 2026 12:47
@tvanlaerhoven
tvanlaerhoven force-pushed the feature/mediacontrol-api branch 2 times, most recently from 864adcb to 8381de3 Compare May 8, 2026 08:30
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

tvanlaerhoven and others added 3 commits July 24, 2026 14:48
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…t-native-theoplayer into feature/mediacontrol-api
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot 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.

Devin Review found 3 new potential issues.

Open in Devin Review

Comment on lines +120 to +123
const previousTrackHandler = this.mediaControlAdapter.getHandler(MediaControlAction.SKIP_TO_PREVIOUS);
mediaSession.setActionHandler('previoustrack', previousTrackHandler ? () => previousTrackHandler(this._webAdapter) : NoOp);
const nextTrackHandler = this.mediaControlAdapter.getHandler(MediaControlAction.SKIP_TO_NEXT);
mediaSession.setActionHandler('nexttrack', nextTrackHandler ? () => nextTrackHandler(this._webAdapter) : NoOp);

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.

🟡 Web playback shows non-working previous/next buttons by default

On web the previous/next media controls are always registered (setActionHandler('previoustrack'/'nexttrack', ... : NoOp) at src/internal/adapter/web/WebMediaSession.ts:121-123) even when no skip handler is set, so browsers present previous/next buttons that do nothing on every web playback.
Impact: Users on web get dead skip-track buttons in the media notification / hardware media keys for content that has no playlist, where previously no such buttons appeared.

Why NoOp keeps the control visible

Per the Media Session spec, a media session action is removed only by passing null to setActionHandler; registering any function (including a no-op) marks the action as supported, so the user agent may render the corresponding control. Before this PR previoustrack/nexttrack were never registered, so no button was shown. Now updateMediaSession() always calls setActionHandler('previoustrack'/'nexttrack', ...) and falls back to NoOp when the user has not set a SKIP_TO_PREVIOUS/SKIP_TO_NEXT handler (src/internal/adapter/web/WebMediaSession.ts:120-123). To avoid presenting a dead control, pass null instead of NoOp when no handler is registered.

Suggested change
const previousTrackHandler = this.mediaControlAdapter.getHandler(MediaControlAction.SKIP_TO_PREVIOUS);
mediaSession.setActionHandler('previoustrack', previousTrackHandler ? () => previousTrackHandler(this._webAdapter) : NoOp);
const nextTrackHandler = this.mediaControlAdapter.getHandler(MediaControlAction.SKIP_TO_NEXT);
mediaSession.setActionHandler('nexttrack', nextTrackHandler ? () => nextTrackHandler(this._webAdapter) : NoOp);
const previousTrackHandler = this.mediaControlAdapter.getHandler(MediaControlAction.SKIP_TO_PREVIOUS);
mediaSession.setActionHandler('previoustrack', previousTrackHandler ? () => previousTrackHandler(this._webAdapter) : null);
const nextTrackHandler = this.mediaControlAdapter.getHandler(MediaControlAction.SKIP_TO_NEXT);
mediaSession.setActionHandler('nexttrack', nextTrackHandler ? () => nextTrackHandler(this._webAdapter) : null);
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +149 to +152
let playPauseControlsEnabled = self.hasSource && !self.inAd && (!self.isLive || self.allowLivePlayPause)
let positionControlEnabled = self.hasSource && !self.inAd && !self.isLive
let seekControlEnabled = self.hasSource && !self.inAd && !self.isLive && !self.hasActionHandler(for: .SKIP_TO_NEXT) && !self.hasActionHandler(for: .SKIP_TO_PREVIOUS)
let trackControlEnabled = self.hasActionHandler(for: .SKIP_TO_NEXT) && self.hasActionHandler(for: .SKIP_TO_PREVIOUS)

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.

🔍 Cross-platform inconsistency in queue/track action gating

The availability rules for skip-track controls differ between platforms. Android's queueActionsEnabled is true if EITHER a SKIP_TO_NEXT or SKIP_TO_PREVIOUS handler is set (or convertSkipToSeek) and is not gated on ads/live (MediaControlProxy.kt:228-230), so a single handler shows both buttons. iOS's trackControlEnabled requires BOTH handlers to be present (THEOplayerRCTRemoteCommandsManager.swift:151-152), so setting only one handler shows neither track button (and also disables seek since seekControlEnabled excludes the case where any track handler exists). This asymmetry means the same JS handler registration yields different lock-screen controls on Android vs iOS; worth confirming this matches the intended documented behavior.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +220 to +237
private fun withLargeIcon(
showPlaceholderWhileFetching: Boolean = false,
block: (Bitmap?) -> Unit
) {
val source = player?.source
if (source != null && source == largeIconSource) {
block(largeIcon)
return
}
if (showPlaceholderWhileFetching) {
block(placeholderIcon)
}
fetchImageFromMetadata(source) { icon ->
largeIconSource = source
largeIcon = icon
block(icon)
}
}

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.

🔍 Artwork caching relies on SourceDescription equality semantics

withLargeIcon caches artwork per source using source == largeIconSource (android/src/main/java/com/theoplayer/media/MediaPlaybackService.kt:225). If player.source returns a fresh SourceDescription instance on each access and the type does not implement structural equals, this comparison would always be false and the icon would be refetched on every notification rebuild — defeating the intended anti-flicker cache. If the SDK returns a stable reference (or SourceDescription implements value equality) the cache works as intended. Worth confirming the equality behavior of SourceDescription.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants