Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add recording controls enhancement #733

Open
wants to merge 37 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
46e31d8
WIP Add btn to PreviewView
p-malecki Nov 12, 2024
176488a
Add recoding to DeviceSettingsDropdown
p-malecki Nov 12, 2024
966399a
Make logs btn round, remove btns labels
p-malecki Nov 12, 2024
cc311ee
Update icons
p-malecki Nov 12, 2024
60b3576
Remove recording from DeviceSettingsDropdown
p-malecki Nov 15, 2024
2110684
Update recording btn style
p-malecki Nov 15, 2024
0b9f997
Add startRecording and captureRecording
p-malecki Nov 15, 2024
4b77af6
Update PreviewView
p-malecki Nov 15, 2024
cd2140a
Rename functions to use more general VideoRecording instead of Replays
p-malecki Nov 15, 2024
4d7dcb4
Use directly saveVideoRecording
p-malecki Nov 15, 2024
f0868de
Add timestamp to saveVideoRecording
p-malecki Nov 15, 2024
7ce0a4f
Stop recording after save
p-malecki Nov 15, 2024
d5ddbb9
Update rec indicator css
p-malecki Nov 15, 2024
60a0c86
Rm
p-malecki Nov 15, 2024
5704638
Update IconButton css
p-malecki Nov 15, 2024
1809870
Update IconButton css
p-malecki Nov 15, 2024
c03cda5
Fix timestamp
p-malecki Nov 15, 2024
b7f6a40
Merge branch 'main' into @p-malecki/add-recording-enhancement
p-malecki Nov 15, 2024
5831986
fix lint
p-malecki Nov 15, 2024
7aa47cb
fix lint
p-malecki Nov 15, 2024
c3780f1
Merge branch 'main' into @p-malecki/add-recording-enhancement
p-malecki Nov 18, 2024
fb88a61
Use one method stopAndCaptureRecording instead of two stopRecording a…
p-malecki Nov 18, 2024
7ef8c3d
Rename replay methods
p-malecki Nov 18, 2024
55f0763
Use ts to prevent from passing wrong videoId in startVideoRecording etc
p-malecki Nov 18, 2024
4a07610
Fix last commit
p-malecki Nov 18, 2024
ff937f9
Move rec indicator and timer to recording button
p-malecki Nov 18, 2024
899f74b
Format css
p-malecki Nov 18, 2024
f3f9647
Remove videoId from methods in preview.ts
p-malecki Nov 18, 2024
7af6ec9
Refactor by adding handleVideoRecordingPromise
p-malecki Nov 18, 2024
e23b6f3
Rename startReplays to enableReplay
p-malecki Nov 19, 2024
598c678
Use videoRecordingPromises map
p-malecki Nov 19, 2024
c01b50c
Refactor captureAndStopRecording
p-malecki Nov 19, 2024
1612f80
Rm test recording
p-malecki Nov 19, 2024
8e86486
Remove stopRecording from DeviceBase
p-malecki Nov 19, 2024
1d9cc95
Merge branch 'main' into @p-malecki/add-recording-enhancement
p-malecki Nov 19, 2024
1126d84
Apply changes from review comments
p-malecki Nov 19, 2024
94e7f28
Apply changes from review comments
p-malecki Nov 19, 2024
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
2 changes: 2 additions & 0 deletions packages/vscode-extension/src/common/Project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ export interface ProjectInterface {
getDeepLinksHistory(): Promise<string[]>;
openDeepLink(link: string): Promise<void>;

startRecording(): void;
captureAndStopRecording(): Promise<RecordingData>;
Copy link
Member

Choose a reason for hiding this comment

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

For symmetry we could just call it "stopRecording" – I believe there's no way for sim server to stop recording without producing the recorded file anyways, is there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, simserver allows starting, stopping, and saving video recordings without stopping them. CaptureReplay is responsible only for saving, unlike captureAndStopRecording, which saves and stops recordings, so name captureAndStopRecording is more appropriate

Copy link
Member

Choose a reason for hiding this comment

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

what if we change it to stopRecording(capture: boolean)

captureReplay(): Promise<RecordingData>;

dispatchTouches(touches: Array<TouchPoint>, type: "Up" | "Move" | "Down"): Promise<void>;
Expand Down
22 changes: 19 additions & 3 deletions packages/vscode-extension/src/devices/DeviceBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,33 @@ export abstract class DeviceBase implements Disposable {
return this.preview?.hideTouches();
}

public stopReplays() {
return this.preview?.stopReplays();
public startRecording() {
if (!this.preview) {
throw new Error("Preview not started");
}
return this.preview.startRecording();
}

public async captureAndStopRecording() {
if (!this.preview) {
throw new Error("Preview not started");
}
const recordingDataPromise = this.preview.captureRecording();
this.preview?.stopRecording();
return recordingDataPromise;
Copy link
Member

Choose a reason for hiding this comment

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

for symmetry, it'd be better if this method just proxied the call down to preview instead of making two calls

}

public startReplays() {
public enableReplay() {
if (!this.preview) {
throw new Error("Preview not started");
}
return this.preview.startReplays();
}

public disableReplays() {
return this.preview?.stopReplays();
}

public async captureReplay() {
if (!this.preview) {
throw new Error("Preview not started");
Expand Down
119 changes: 75 additions & 44 deletions packages/vscode-extension/src/devices/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,54 @@ import { Logger } from "../Logger";
import { Platform } from "../utilities/platform";
import { RecordingData, TouchPoint } from "../common/Project";

interface ReplayPromiseHandlers {
interface VideoRecordingPromiseHandlers {
resolve: (value: RecordingData) => void;
reject: (reason?: any) => void;
}

export class Preview implements Disposable {
private videoRecordingPromises = new Map<string, VideoRecordingPromiseHandlers>();
private subprocess?: ChildProcess;
public streamURL?: string;
private lastReplayPromise?: ReplayPromiseHandlers;

constructor(private args: string[]) {}

dispose() {
this.subprocess?.kill();
}

private sendCommandOrThrow(command: string) {
const stdin = this.subprocess?.stdin;
if (!stdin) {
throw new Error("sim-server process not available");
}
stdin.write(command);
}

private saveVideoWithID(videoId: string): Promise<RecordingData> {
const stdin = this.subprocess?.stdin;
if (!stdin) {
throw new Error("sim-server process not available");
}

let resolvePromise: (value: RecordingData) => void;
let rejectPromise: (reason?: any) => void;
const promise = new Promise<RecordingData>((resolve, reject) => {
resolvePromise = resolve;
rejectPromise = reject;
});

const lastPromise = this.videoRecordingPromises.get(videoId);
if (lastPromise) {
promise.then(lastPromise.resolve, lastPromise.reject);
}

const newPromiseHandler = { resolve: resolvePromise!, reject: rejectPromise! };
this.videoRecordingPromises.set(videoId, newPromiseHandler);
stdin.write(`video ${videoId} save\n`);
return promise;
}

async start() {
const simControllerBinary = path.join(
extensionContext.extensionPath,
Expand Down Expand Up @@ -60,33 +92,43 @@ export class Preview implements Disposable {
this.streamURL = match[1];
resolve(this.streamURL);
}
} else if (line.includes("video_ready replay") || line.includes("video_error replay")) {
// video response format for replays looks as follows:
// video_ready replay <HTTP_URL> <FILE_URL>
// video_error replay <Error message>
const videoReadyMatch = line.match(/video_ready replay (\S+) (\S+)/);
const videoErrorMatch = line.match(/video_error replay (.*)/);

const handlers = this.lastReplayPromise;
this.lastReplayPromise = undefined;
} else if (line.includes("video_ready") || line.includes("video_error")) {
// video response format for recordings looks as follows:
// video_ready <VIDEO_ID> <HTTP_URL> <FILE_URL>
// video_error <VIDEO_ID> <Error message>
const videoReadyMatch = line.match(/video_ready (\S+) (\S+) (\S+)/);
const videoErrorMatch = line.match(/video_error (\S+) (.*)/);

const videoId = videoReadyMatch
? videoReadyMatch[1]
: videoErrorMatch
? videoErrorMatch[1]
: "";

if (!this.videoRecordingPromises.has(videoId)) {
throw new Error(`Invalid video ID: ${videoId}`);
}

const handlers = this.videoRecordingPromises.get(videoId);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (!this.videoRecordingPromises.has(videoId)) {
throw new Error(`Invalid video ID: ${videoId}`);
}
const handlers = this.videoRecordingPromises.get(videoId);
const handlers = this.videoRecordingPromises.get(videoId);
if (!handlers) {
throw new Error(`Invalid video ID: ${videoId}`);
}

Copy link
Member

Choose a reason for hiding this comment

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

After this we don't need to check if handlers is set as we do in the lines below

this.videoRecordingPromises.delete(videoId);
if (handlers && videoReadyMatch) {
// match array looks as follows:
// [0] - full match
// [1] - URL or error message
// [2] - File URL
const tempFileLocation = videoReadyMatch[2];
// [1] - ID of the video
// [2] - URL or error message
// [3] - File URL
const tempFileLocation = videoReadyMatch[3];
const ext = path.extname(tempFileLocation);
const fileName = workspace.name
? `${workspace.name}-RadonIDE-replay${ext}`
: `RadonIDE-replay${ext}`;
? `${workspace.name}-RadonIDE-${videoId}${ext}`
: `RadonIDE-${videoId}${ext}`;
handlers.resolve({
url: videoReadyMatch[1],
url: videoReadyMatch[2],
tempFileLocation,
fileName,
});
} else if (handlers && videoErrorMatch) {
handlers.reject(new Error(videoErrorMatch[1]));
handlers.reject(new Error(videoErrorMatch[2]));
}
}
Logger.info("sim-server:", line);
Expand All @@ -102,39 +144,28 @@ export class Preview implements Disposable {
this.subprocess?.stdin?.write("pointer show false\n");
}

public startRecording() {
this.sendCommandOrThrow(`video recording start -m -b 50\n`); // 50MB buffer for in-memory video
}

public stopRecording() {
this.sendCommandOrThrow(`video recording stop\n`);
}

public captureRecording() {
return this.saveVideoWithID("recording");
}

public startReplays() {
const stdin = this.subprocess?.stdin;
if (!stdin) {
throw new Error("sim-server process not available");
}
stdin.write(`video replay start -m -b 50\n`); // 50MB buffer for in-memory video
this.sendCommandOrThrow(`video replay start -m -b 50\n`); // 50MB buffer for in-memory video
}

public stopReplays() {
const stdin = this.subprocess?.stdin;
if (!stdin) {
throw new Error("sim-server process not available");
}
stdin.write(`video replay stop\n`);
this.sendCommandOrThrow(`video replay stop\n`);
}

public captureReplay() {
const stdin = this.subprocess?.stdin;
if (!stdin) {
throw new Error("sim-server process not available");
}
let resolvePromise: (value: RecordingData) => void;
let rejectPromise: (reason?: any) => void;
const promise = new Promise<RecordingData>((resolve, reject) => {
resolvePromise = resolve;
rejectPromise = reject;
});
if (this.lastReplayPromise) {
promise.then(this.lastReplayPromise.resolve, this.lastReplayPromise.reject);
}
this.lastReplayPromise = { resolve: resolvePromise!, reject: rejectPromise! };
stdin.write(`video replay save\n`);
return promise;
return this.saveVideoWithID("replay");
}

public sendTouches(touches: Array<TouchPoint>, type: "Up" | "Move" | "Down") {
Expand Down
16 changes: 12 additions & 4 deletions packages/vscode-extension/src/project/deviceSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export class DeviceSession implements Disposable {
});

this.isLaunching = true;
this.device.stopReplays();
this.device.disableReplays();

// FIXME: Windows getting stuck waiting for the promise to resolve. This
// seems like a problem with app connecting to Metro and using embedded
Expand Down Expand Up @@ -154,7 +154,7 @@ export class DeviceSession implements Disposable {

this.isLaunching = false;
if (this.deviceSettings?.replaysEnabled) {
this.device.startReplays();
this.device.enableReplay();
}
if (this.deviceSettings?.showTouches) {
this.device.showTouches();
Expand Down Expand Up @@ -257,6 +257,14 @@ export class DeviceSession implements Disposable {
}
}

public startRecording() {
return this.device.startRecording();
}

public async captureAndStopRecording() {
return this.device.captureAndStopRecording();
}

public async captureReplay() {
return this.device.captureReplay();
}
Expand Down Expand Up @@ -305,9 +313,9 @@ export class DeviceSession implements Disposable {
public async changeDeviceSettings(settings: DeviceSettings): Promise<boolean> {
this.deviceSettings = settings;
if (settings.replaysEnabled && !this.isLaunching) {
this.device.startReplays();
this.device.enableReplay();
} else {
this.device.stopReplays();
this.device.disableReplays();
}
if (settings.showTouches && !this.isLaunching) {
this.device.showTouches();
Expand Down
14 changes: 14 additions & 0 deletions packages/vscode-extension/src/project/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,20 @@ export class Project
}
//#endregion

startRecording(): void {
if (!this.deviceSession) {
throw new Error("No device session available");
}
this.deviceSession.startRecording();
}

async captureAndStopRecording(): Promise<RecordingData> {
if (!this.deviceSession) {
throw new Error("No device session available");
}
return this.deviceSession.captureAndStopRecording();
}

async captureReplay(): Promise<RecordingData> {
if (!this.deviceSession) {
throw new Error("No device session available");
Expand Down
16 changes: 14 additions & 2 deletions packages/vscode-extension/src/utilities/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,14 @@ export class Utils implements UtilsInterface {

public async saveVideoRecording(recordingData: RecordingData) {
const extension = path.extname(recordingData.tempFileLocation);
const defaultUri = Uri.file(
path.join(workspace.workspaceFolders![0].uri.fsPath, recordingData.fileName)
const timestamp = this.getTimestamp();
const baseFileName = recordingData.fileName.substring(
0,
recordingData.fileName.length - extension.length
);
const newFileName = `${baseFileName}_${timestamp}${extension}`;
const defaultUri = Uri.file(path.join(workspace.workspaceFolders![0].uri.fsPath, newFileName));

// save dialog open the location dialog, it also warns the user if the file already exists
let saveUri = await window.showSaveDialog({
defaultUri: defaultUri,
Expand Down Expand Up @@ -116,4 +121,11 @@ export class Utils implements UtilsInterface {
public async log(type: "info" | "error" | "warn" | "log", message: string, ...args: any[]) {
Logger[type]("[WEBVIEW LOG]", message, ...args);
}

private getTimestamp() {
// e.g. "2024-11-15_at_14-09-51"
const timezoneOffset = new Date().getTimezoneOffset() * 60000;
const localISOTime = new Date(Date.now() - timezoneOffset).toISOString().slice(0, -1);
return localISOTime.replace(/T/g, "_at_").replace(/:/g, "-").slice(0, 22);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { DevicePlatform } from "../../common/DeviceManager";
import { KeybindingInfo } from "./shared/KeybindingInfo";
import { DeviceLocalizationView } from "../views/DeviceLocalizationView";
import { OpenDeepLinkView } from "../views/OpenDeepLinkView";
import ReplayIcon from "./icons/ReplayIcon";

const contentSizes = [
"xsmall",
Expand Down Expand Up @@ -160,10 +161,7 @@ function DeviceSettingsDropdown({ children, disabled }: DeviceSettingsDropdownPr
Open Deep Link
</DropdownMenu.Item>
<div className="dropdown-menu-item">
<span className="icons-container">
<span className="codicon codicon-triangle-left icons-rewind" />
<span className="codicon codicon-triangle-left icons-rewind" />
</span>
<ReplayIcon />
Enable Replays
<Switch.Root
className="switch-root small-switch"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
interface RecordingIconProps {
color?: string;
}

const RecordingIcon = ({ color = "currentColor", ...rest }: RecordingIconProps) => (
<svg
xmlns="http://www.w3.org/2000/svg"
width={18}
height={18}
viewBox="0 0 24 24"
fill="none"
{...rest}>
<path
stroke={color}
strokeWidth={1.25}
strokeLinecap="round"
strokeLinejoin="round"
d="m17 13 5.223 3.482a.5.5 0 0 0 .777-.416V7.87a.5.5 0 0 0-.752-.432L17 10.5"
/>
<rect
x="3"
y="6"
width="14"
height="12"
rx="2"
stroke={color}
strokeWidth={1.25}
strokeLinecap="round"
strokeLinejoin="round"
/>
</svg>
);

export default RecordingIcon;
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
interface ReplayIconProps {
color?: string;
}

const ReplayIcon = ({ color = "currentColor", ...rest }: ReplayIconProps) => (
<svg
xmlns="http://www.w3.org/2000/svg"
width={20}
height={20}
viewBox="0 0 24 24"
fill="none"
{...rest}>
<path
d="M12 13.8L19 18V6L12 10.2M12 18L12 6L3 12L12 18Z"
stroke={color}
strokeWidth={1.5}
strokeLinecap="round"
strokeLinejoin="round"
/>
</svg>
);

export default ReplayIcon;
Loading