Addressing pr comments

Signed-off-by: Lance Chant <13349722+lancechant@users.noreply.github.com>
This commit is contained in:
Lance Chant
2026-07-02 08:43:20 +02:00
parent faa250bfdd
commit 4f31cd2b32
7 changed files with 69 additions and 36 deletions

View File

@@ -736,12 +736,12 @@ export default function SettingsTV() {
/>
{isMpv && (
<TVSettingsOptionButton
label='Horizontal Alignment'
label={t("home.settings.subtitles.mpv_subtitle_align_x")}
value={alignXLabel}
// ExoPlayer follows authored cue alignment; hide on ExoPlayer.
onPress={() =>
showOptions({
title: "Horizontal Alignment",
title: t("home.settings.subtitles.mpv_subtitle_align_x"),
options: alignXOptions,
onSelect: (value) =>
updateSettings({

View File

@@ -1,5 +1,4 @@
import * as React from "react";
import { Platform } from "react-native";
import type { MpvPlayerViewProps, MpvPlayerViewRef } from "@/modules";
import { MpvPlayerView } from "@/modules";
import { ExoPlayerView } from "@/modules/exoplayer-player";
@@ -14,21 +13,17 @@ import {
* can opt into ExoPlayer on Android TV via settings.videoPlayer. Both
* children conform to the same `MpvPlayerViewRef` interface, so the ref
* is forwarded transparently regardless of which player is rendered.
*
* The Android-TV capability gate lives in getActiveVideoPlayer so that
* the same resolver used for device-profile advertisement guarantees the
* rendered backend matches what Jellyfin was told to stream for.
*/
export const VideoPlayerView = React.forwardRef<
MpvPlayerViewRef,
MpvPlayerViewProps
>(function VideoPlayerView(props, ref) {
const { settings } = useSettings();
// ExoPlayer's native module only ships for Android TV. Even if a user
// somehow ends up with `videoPlayer: ExoPlayer` set on another platform
// (shouldn't happen — the selector is hidden outside Android TV — but
// MMKV-persisted settings can roam), fall back to MPV rather than
// crash on requireNativeView().
const isExoSupported = Platform.OS === "android" && Platform.isTV;
const useExo =
isExoSupported && getActiveVideoPlayer(settings) === VideoPlayer.ExoPlayer;
const useExo = getActiveVideoPlayer(settings) === VideoPlayer.ExoPlayer;
const Player = useExo ? ExoPlayerView : MpvPlayerView;
return <Player ref={ref} {...props} />;

View File

@@ -306,9 +306,10 @@ export const TechnicalInfoOverlay: FC<TechnicalInfoOverlayProps> = memo(
what's actually being decoded) over Jellyfin metadata. */}
{info?.hdrFormat
? ` ${info.hdrFormat}`
: formatVideoRange(streamInfo?.videoRange)
? ` ${formatVideoRange(streamInfo?.videoRange)}`
: ""}
: (() => {
const videoRange = formatVideoRange(streamInfo?.videoRange);
return videoRange ? ` ${videoRange}` : "";
})()}
</Text>
)}
{info?.videoCodec && (
@@ -322,11 +323,13 @@ export const TechnicalInfoOverlay: FC<TechnicalInfoOverlayProps> = memo(
Audio: {formatCodec(info.audioCodec)}
{/* Prefer player-reported channel count; fall back to
Jellyfin metadata for MPV which doesn't populate it. */}
{(info.audioChannels ?? streamInfo?.audioChannels)
? ` ${formatAudioChannels(
info.audioChannels ?? streamInfo!.audioChannels!,
)}`
: ""}
{(() => {
const audioChannels =
info.audioChannels ?? streamInfo?.audioChannels;
return audioChannels
? ` ${formatAudioChannels(audioChannels)}`
: "";
})()}
{info.audioSampleRate
? ` @ ${(info.audioSampleRate / 1000).toFixed(1)}kHz`
: ""}
@@ -349,7 +352,7 @@ export const TechnicalInfoOverlay: FC<TechnicalInfoOverlayProps> = memo(
)}
{(info?.colorSpace || info?.colorRange || info?.colorTransfer) && (
<Text style={textStyle}>
Color:
Color:{" "}
{[info.colorSpace, info.colorRange, info.colorTransfer]
.filter(Boolean)
.join(" / ")}

View File

@@ -82,6 +82,12 @@ class ExoPlayerView(context: Context, appContext: AppContext) : ExpoView(context
private var pendingConfig: VideoLoadConfig? = null
private var tracksReadyFired: Boolean = false
// Side-loaded subtitle configurations accumulated across loadVideo and
// addSubtitleFile. Media3 doesn't expose the live SubtitleConfiguration
// list on a playing MediaItem, so we shadow it here to preserve prior
// side-loaded subs when addSubtitleFile rebuilds the MediaItem.
private var sideLoadedSubs: List<MediaItem.SubtitleConfiguration> = emptyList()
// 1-based track ID mappings (matching MPV's contract).
// Each list is rebuilt on Tracks changed.
private var subtitleTrackList: List<TrackEntry> = emptyList()
@@ -371,8 +377,13 @@ class ExoPlayerView(context: Context, appContext: AppContext) : ExpoView(context
.build()
}
if (subtitleConfigs.isNotEmpty()) {
sideLoadedSubs = subtitleConfigs
builder.setSubtitleConfigurations(subtitleConfigs)
} else {
sideLoadedSubs = emptyList()
}
} else {
sideLoadedSubs = emptyList()
}
return builder.build()
@@ -406,6 +417,7 @@ class ExoPlayerView(context: Context, appContext: AppContext) : ExpoView(context
playerView.player = null
tracksReadyFired = false
currentUrl = null
sideLoadedSubs = emptyList()
subtitleTrackList = emptyList()
audioTrackList = emptyList()
currentSubtitleId = 0
@@ -547,14 +559,6 @@ class ExoPlayerView(context: Context, appContext: AppContext) : ExpoView(context
fun addSubtitleFile(url: String, select: Boolean) {
val p = player ?: return
// Media3 does not expose the current MediaItem's existing
// SubtitleConfigurations, so we cannot append a side-loaded
// subtitle to a running item without losing the originals.
// For TV, external subs are bundled at load time via
// VideoLoadConfig.externalSubtitles (see buildMediaItem). This
// method rebuilds the current MediaItem with just the new
// subtitle config — acceptable when no other external subs are
// in play, which is the typical TV case.
val mime = mimeTypeForSubtitleUrl(url) ?: return
val currentMediaItem = p.currentMediaItem ?: return
val newSubConfig = MediaItem.SubtitleConfiguration.Builder(Uri.parse(url))
@@ -562,8 +566,14 @@ class ExoPlayerView(context: Context, appContext: AppContext) : ExpoView(context
.setSelectionFlags(if (select) C.SELECTION_FLAG_DEFAULT else 0)
.build()
// Rebuild with the full accumulated list so previously loaded
// side-loaded subs (from VideoLoadConfig.externalSubtitles or
// earlier addSubtitleFile calls) survive.
val combined = sideLoadedSubs + newSubConfig
sideLoadedSubs = combined
val rebuilt = currentMediaItem.buildUpon()
.setSubtitleConfigurations(listOf(newSubConfig))
.setSubtitleConfigurations(combined)
.build()
val wasPlaying = p.isPlaying
@@ -571,6 +581,17 @@ class ExoPlayerView(context: Context, appContext: AppContext) : ExpoView(context
p.setMediaItem(rebuilt, pos)
p.prepare()
if (wasPlaying) p.play()
// If text tracks were disabled (e.g. disableSubtitles was called
// earlier, or playback started with subtitles off), the new
// subtitle — even with SELECTION_FLAG_DEFAULT — won't render.
// Re-enable the text track type when the caller asks us to select.
if (select) {
val params = p.trackSelectionParameters.buildUpon()
.setTrackTypeDisabled(C.TRACK_TYPE_TEXT, false)
.build()
p.trackSelectionParameters = params
}
}
// MARK: - Subtitle Positioning / Styling

View File

@@ -5,7 +5,7 @@ import { useImperativeHandle, useRef } from "react";
import type {
MpvPlayerViewProps,
MpvPlayerViewRef,
} from "../mpv-player/src/MpvPlayer.types";
} from "../../mpv-player/src/MpvPlayer.types";
const NativeView: React.ComponentType<MpvPlayerViewProps & { ref?: any }> =
requireNativeView("ExoPlayer");

View File

@@ -178,17 +178,31 @@ export enum VideoPlayer {
ExoPlayer = 1,
}
/**
* Whether ExoPlayer's native module is available on the current platform.
* ExoPlayer only ships for Android TV; on any other platform a persisted
* `videoPlayer: ExoPlayer` preference (e.g. MMKV roaming) must fall back
* to MPV rather than crash on requireNativeView().
*/
export const isExoPlayerSupported =
Platform.OS === "android" && Platform.isTV === true;
/**
* Resolve the actually-active video player for the current settings.
* MPV is the default on every platform; users can opt into ExoPlayer on
* Android TV via settings.videoPlayer. Centralized here so the rule has
* one source of truth (used by VideoPlayerView, direct-player's device
* profile, and the TV settings UI).
* Android TV via settings.videoPlayer. The Android-TV capability gate is
* folded in here so callers (VideoPlayerView, direct-player's device
* profile, PlaySettingsProvider) can never advertise ExoPlayer on a
* platform where MPV is actually rendering — that mismatch would let
* Jellyfin pick a stream for the wrong renderer.
*/
export const getActiveVideoPlayer = (
settings: Pick<Settings, "videoPlayer"> | null | undefined,
): VideoPlayer => {
return settings?.videoPlayer ?? VideoPlayer.MPV;
if (isExoPlayerSupported && settings?.videoPlayer === VideoPlayer.ExoPlayer) {
return VideoPlayer.ExoPlayer;
}
return VideoPlayer.MPV;
};
/**

View File

@@ -183,7 +183,7 @@ export const generateDeviceProfile = (options: ProfileOptions = {}) => {
CodecProfiles: [
{
Type: MediaTypes.Video,
Codec: "h263,h264,hevc,vp8,vp9,av1",
Codec: "h263,h264,vp8,vp9,av1",
},
{
Type: MediaTypes.Video,