Skip to content
Merged
48 changes: 45 additions & 3 deletions apps/mobile/src/components/MediaVideoPlayer.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import { useEvent } from "expo";
import { useVideoPlayer, VideoView } from "expo-video";
import { useEffect, useEffectEvent, useRef, useState } from "react";
import { useIsFocused } from "@react-navigation/native";
import { useEffect, useEffectEvent, useLayoutEffect, useRef, useState } from "react";
import { Pressable, View } from "react-native";

import { AppActivityIndicator } from "./ui/AppActivityIndicator";
import { AppText } from "./ui/AppText";
import { SymbolView } from "./ui/AppSymbol";
import { VideoThumbnailImage } from "./VideoThumbnailImage";
import { useMediaActions } from "../state/mediaActions";
import { fitVideoFrame } from "../lib/videoFrameFit";
import { loadVideoThumbnail } from "../lib/videoThumbnails";
import { type MediaActionsSource } from "../lib/mediaActionsSource";
import { useMediaVideoPlayback } from "../lib/useMediaVideoPlayback";
import { MediaActionsMenu } from "./MediaActionsMenu";
Expand Down Expand Up @@ -136,6 +139,8 @@ interface MediaVideoPlayerProps {
readonly visible?: boolean;
readonly unavailable?: boolean;
readonly expanded?: boolean;
/** Fill the parent and size the frame to the video's shape instead of a fixed 16:9 slot. */
readonly fitToVideo?: boolean;
readonly autoPlay?: boolean;
readonly paused?: boolean;
readonly onPlaybackStart?: () => void;
Expand All @@ -145,16 +150,41 @@ interface MediaVideoPlayerProps {

function MediaVideoPlayerContent(props: MediaVideoPlayerProps) {
const mediaActions = useMediaActions(props.actionsSource);
const isFocused = useIsFocused();
const [playbackUri, setPlaybackUri] = useState<string | null>(props.expanded ? props.uri : null);
// Keep an opened player mounted while signing or reconnecting temporarily has no usable URL.
if (playbackUri === null && props.expanded && props.uri !== null) setPlaybackUri(props.uri);
const [videoSize, setVideoSize] = useState<{ width: number; height: number } | null>(null);
const [bounds, setBounds] = useState<{ width: number; height: number } | null>(null);
const sizeSource = props.fitToVideo && isFocused && videoSize === null ? props.uri : null;
// Queue before the poster so early Play cannot cancel the extraction that supplies frame dimensions.
// oxlint-disable-next-line t3code/no-unreasoned-state-sync -- loads native thumbnail dimensions independently of the poster's lifetime
useLayoutEffect(() => {
if (sizeSource === null) return;
// Thumbnails are oriented like the displayed frame, unlike VideoTrack.size.
const controller = new AbortController();
void loadVideoThumbnail(
props.thumbnailKey,
async () => ({ uri: sizeSource, dispose: () => undefined }),
controller.signal,
).then((thumbnail) => {
if (thumbnail && !controller.signal.aborted) {
setVideoSize({ width: thumbnail.width, height: thumbnail.height });
}
});
return () => controller.abort();
}, [props.thumbnailKey, sizeSource]);
const fittedFrame =
props.fitToVideo && videoSize && bounds ? fitVideoFrame(videoSize, bounds, 480) : null;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

return (
const frame = (
<View
collapsable={false}
className="overflow-hidden rounded-[10px] bg-black"
style={[
props.expanded ? { flex: 1 } : { width: "100%", maxWidth: 480, aspectRatio: 16 / 9 },
props.expanded
? { flex: 1 }
: (fittedFrame ?? { width: "100%", maxWidth: 480, aspectRatio: 16 / 9 }),
props.borderRadius === undefined ? null : { borderRadius: props.borderRadius },
]}
>
Expand Down Expand Up @@ -214,6 +244,18 @@ function MediaVideoPlayerContent(props: MediaVideoPlayerProps) {
)}
</View>
);
if (!props.fitToVideo) return frame;
return (
<View
className="flex-1 items-center justify-center self-stretch"
onLayout={(event) => {
const { width, height } = event.nativeEvent.layout;
setBounds({ width, height });
}}
>
{frame}
</View>
);
}

export function MediaVideoPlayer(props: MediaVideoPlayerProps) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export function WorkspaceFileVideoPreview(props: {
resolvePlaybackUri={props.resolvePlaybackUri}
name={props.name}
thumbnailKey={props.thumbnailKey}
fitToVideo
actionsSource={props.source?.actionsSource}
/>
</View>
Expand Down
43 changes: 31 additions & 12 deletions apps/mobile/src/lib/useMediaVideoPlayback.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ import {
PullRequestMediaContext,
PullRequestMediaVisibleContext,
} from "../features/pull-requests/PullRequestMedia";
import { MediaVideoPlayer } from "../components/MediaVideoPlayer";

function createPlayer() {
let playing = false;
Expand Down Expand Up @@ -149,7 +150,7 @@ function createPlayer() {
};
}

function setup(kind: "thread" | "PR", compact = false) {
function setup(kind: "thread" | "PR" | "files", compact = false) {
(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
mocks.focused = true;
mocks.appState = "active";
Expand All @@ -166,10 +167,20 @@ function setup(kind: "thread" | "PR", compact = false) {
const environmentId = EnvironmentId.make("test");
return {
render(visible = true) {
if (kind === "files") mocks.focused = visible;
act(() =>
root.render(
<RegistryContext.Provider value={registry}>
{kind === "thread" ? (
{kind === "files" ? (
<MediaVideoPlayer
name="clip.mp4"
thumbnailKey="workspace-video:test"
uri="https://media.example/clip.mp4"
fitToVideo
visible={visible}
resolvePlaybackUri={mocks.refresh}
/>
) : kind === "thread" ? (
<ThreadMediaVisibleContext value={visible}>
<ThreadMarkdownVideo
source={{
Expand Down Expand Up @@ -267,23 +278,31 @@ it.each(["thread", "PR"] as const)(
},
);

it.each([false, true])(
"stops inactive PR thumbnail requests (compact: %s) and restarts on return",
async (compact) => {
using screen = setup("PR", compact);
it.each([
["PR", false],
["PR", true],
["files", false],
] as const)(
"stops inactive %s thumbnail requests (compact: %s) and restarts on return",
async (kind, compact) => {
using screen = setup(kind, compact);
mocks.thumbnail.mockReturnValue(new Promise(() => {}));
screen.render(false);
expect(mocks.thumbnail).not.toHaveBeenCalled();
await act(async () => screen.render(true));
expect(mocks.thumbnail).toHaveBeenCalledTimes(1);
const signal = mocks.thumbnail.mock.calls[0]![2];
expect(signal.aborted).toBe(false);
const aborted = new Promise((resolve) => signal.addEventListener("abort", resolve));
const requests = kind === "files" ? 2 : 1;
expect(mocks.thumbnail).toHaveBeenCalledTimes(requests);
const aborted = Promise.all(
mocks.thumbnail.mock.calls.map(([, , signal]) => {
expect(signal.aborted).toBe(false);
return new Promise((resolve) => signal.addEventListener("abort", resolve));
}),
);
screen.render(false);
await act(() => aborted);
expect(mocks.thumbnail).toHaveBeenCalledTimes(1);
expect(mocks.thumbnail).toHaveBeenCalledTimes(requests);
await act(async () => screen.render(true));
expect(mocks.thumbnail).toHaveBeenCalledTimes(2);
expect(mocks.thumbnail).toHaveBeenCalledTimes(requests * 2);
},
);

Expand Down
33 changes: 33 additions & 0 deletions apps/mobile/src/lib/videoFrameFit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { describe, expect, it } from "vite-plus/test";

import { fitVideoFrame } from "./videoFrameFit";

describe("fitVideoFrame", () => {
const bounds = { width: 360, height: 600 };

it("keeps landscape videos at the full capped width", () => {
expect(fitVideoFrame({ width: 1920, height: 1080 }, bounds, 480)).toEqual({
width: 360,
height: 202.5,
});
});

it("limits portrait videos by the available height", () => {
expect(fitVideoFrame({ width: 1080, height: 1920 }, bounds, 480)).toEqual({
width: 337.5,
height: 600,
});
});

it("caps wide bounds at the inline width", () => {
expect(fitVideoFrame({ width: 1080, height: 1080 }, { width: 800, height: 900 }, 480)).toEqual({
width: 480,
height: 480,
});
});

it("returns null before the video or bounds have a size", () => {
expect(fitVideoFrame({ width: 0, height: 0 }, bounds, 480)).toBeNull();
expect(fitVideoFrame({ width: 1080, height: 1920 }, { width: 0, height: 0 }, 480)).toBeNull();
});
});
14 changes: 14 additions & 0 deletions apps/mobile/src/lib/videoFrameFit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
interface Size {
readonly width: number;
readonly height: number;
}

/** Largest frame with the video's aspect ratio that fits the bounds and the inline width cap. */
export function fitVideoFrame(video: Size, bounds: Size, maxWidth: number): Size | null {
if (video.width <= 0 || video.height <= 0 || bounds.width <= 0 || bounds.height <= 0) {
return null;
}
const ratio = video.width / video.height;
const width = Math.min(bounds.width, maxWidth, bounds.height * ratio);
return { width, height: width / ratio };
}
Loading