Skip to content

fix: improve performance on windows and macos by passing canvas direclty to VideoFrame() #448

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.

Sign up for GitHub

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

Merged
siddharthvaddem merged 4 commits into from
Apr 18, 2026
Merged

fix: improve performance on windows and macos by passing canvas direclty to VideoFrame() #448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d12f398
fix: only read back frames from canvas if the OS is linux, work aroun…
theopfr Apr 14, 2026
934f05c
fix: pass platform from video/gifExporter to FrameRenderer, skip read…
theopfr Apr 15, 2026
2f24038
fix: use existing getPlatform() so the OS based CPU readback check al…
theopfr Apr 16, 2026
9e4ec79
chore: fix linting issue
theopfr Apr 18, 2026
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
1 change: 0 additions & 1 deletion scripts/i18n-check.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import path from "node:path";

const LOCALES_DIR = path.resolve("src/i18n/locales");
const BASE_LOCALE = "en";
const COMPARE_LOCALES = ["zh-CN", "zh-TW", "es", "tr", "ko-KR"];

function getKeys(obj, prefix = "") {
const keys = [];
Expand Down
12 changes: 10 additions & 2 deletions src/lib/exporter/frameRenderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ interface FrameRenderConfig {
previewWidth?: number;
previewHeight?: number;
cursorTelemetry?: import("@/components/video-editor/types").CursorTelemetryPoint[];
platform: string;
}

interface AnimationState {
Expand Down Expand Up @@ -124,9 +125,11 @@ export class FrameRenderer {
private smoothedAutoFocus: { cx: number; cy: number } | null = null;
private prevAnimationTimeMs: number | null = null;
private prevTargetProgress = 0;
private isLinux = false;

constructor(config: FrameRenderConfig) {
this.config = config;
this.isLinux = config.platform === "linux";
this.animationState = {
scale: 1,
focusX: DEFAULT_FOCUS.cx,
Expand Down Expand Up @@ -187,8 +190,10 @@ export class FrameRenderer {
this.compositeCanvas = document.createElement("canvas");
this.compositeCanvas.width = this.config.width;
this.compositeCanvas.height = this.config.height;

// On Linux, getImageData() is called frequently causing frequent CPU readback
this.compositeCtx = this.compositeCanvas.getContext("2d", {
willReadFrequently: false,
willReadFrequently: this.isLinux,
});

if (!this.compositeCtx) {
Expand Down Expand Up @@ -726,7 +731,10 @@ export class FrameRenderer {
private compositeWithShadows(webcamFrame?: VideoFrame | null): void {
if (!this.compositeCanvas || !this.compositeCtx || !this.app) return;

const videoCanvas = this.readbackVideoCanvas();
const videoCanvas = this.isLinux
? this.readbackVideoCanvas()
: (this.app.canvas as HTMLCanvasElement);

const ctx = this.compositeCtx;
const w = this.compositeCanvas.width;
const h = this.compositeCanvas.height;
Expand Down
5 changes: 5 additions & 0 deletions src/lib/exporter/gifExporter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {
WebcamSizePreset,
ZoomRegion,
} from "@/components/video-editor/types";
import { getPlatform } from "@/utils/platformUtils";
import { AsyncVideoFrameQueue } from "./asyncVideoFrameQueue";
import { FrameRenderer } from "./frameRenderer";
import { StreamingVideoDecoder } from "./streamingDecoder";
Expand Down Expand Up @@ -115,7 +116,10 @@ export class GifExporter {

async export(): Promise<ExportResult> {
let webcamFrameQueue: AsyncVideoFrameQueue | null = null;

try {
const platform = await getPlatform();

this.cleanup();
this.cancelled = false;

Expand Down Expand Up @@ -153,6 +157,7 @@ export class GifExporter {
previewWidth: this.config.previewWidth,
previewHeight: this.config.previewHeight,
cursorTelemetry: this.config.cursorTelemetry,
platform,
});
await this.renderer.initialize();

Expand Down
46 changes: 27 additions & 19 deletions src/lib/exporter/videoExporter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
WebcamSizePreset,
ZoomRegion,
} from "@/components/video-editor/types";
import { getPlatform } from "@/utils/platformUtils";
import { AsyncVideoFrameQueue } from "./asyncVideoFrameQueue";
import { AudioProcessor } from "./audioEncoder";
import { FrameRenderer } from "./frameRenderer";
Expand Down Expand Up @@ -112,6 +113,8 @@ export class VideoExporter {
this.fatalEncoderError = null;

try {
const platform = await getPlatform();

const streamingDecoder = new StreamingVideoDecoder();
this.streamingDecoder = streamingDecoder;
const videoInfo = await streamingDecoder.loadMetadata(this.config.videoUrl);
Expand Down Expand Up @@ -146,6 +149,7 @@ export class VideoExporter {
previewWidth: this.config.previewWidth,
previewHeight: this.config.previewHeight,
cursorTelemetry: this.config.cursorTelemetry,
platform,
});
this.renderer = renderer;
await renderer.initialize();
Expand Down Expand Up @@ -237,25 +241,29 @@ export class VideoExporter {

const canvas = renderer.getCanvas();

// Read raw pixels from the canvas instead of passing
// the canvas directly to VideoFrame. On some Linux
// systems the GPU shared-image path (EGL/Ozone) fails
// silently, producing empty frames.
const canvasCtx = canvas.getContext("2d")!;
const imageData = canvasCtx.getImageData(0, 0, canvas.width, canvas.height);
const exportFrame = new VideoFrame(imageData.data.buffer, {
format: "RGBA",
codedWidth: canvas.width,
codedHeight: canvas.height,
timestamp,
duration: frameDuration,
colorSpace: {
primaries: "bt709",
transfer: "iec61966-2-1",
matrix: "rgb",
fullRange: true,
},
});
let exportFrame: VideoFrame;

// On some Linux systems the GPU shared-image path (EGL/Ozone) fails
// silently, producing empty frames, so we force a CPU readback instead.
if (platform === "linux") {
const canvasCtx = canvas.getContext("2d")!;
const imageData = canvasCtx.getImageData(0, 0, canvas.width, canvas.height);
exportFrame = new VideoFrame(imageData.data.buffer, {
format: "RGBA",
codedWidth: canvas.width,
codedHeight: canvas.height,
timestamp,
duration: frameDuration,
colorSpace: {
primaries: "bt709",
transfer: "iec61966-2-1",
matrix: "rgb",
fullRange: true,
},
});
Comment on lines +248 to +263
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how FrameRenderer acquires/uses the canvas context
ast-grep --pattern $'getContext($$$)'

Repository: siddharthvaddem/openscreen

Length of output: 52

🏁 Script executed:

# Find FrameRenderer class definition
rg "class FrameRenderer" -A 5

Repository: siddharthvaddem/openscreen

Length of output: 548

🏁 Script executed:

# Search for readbackVideoCanvas function mentioned in the summary
rg "readbackVideoCanvas" -B 2 -A 10

Repository: siddharthvaddem/openscreen

Length of output: 1810

🏁 Script executed:

# Also look for where canvas context is initialized in videoExporter
rg "getContext" -B 2 -A 2

Repository: siddharthvaddem/openscreen

Length of output: 3212

non-null assertion on getContext("2d") is risky

the canvas from readbackVideoCanvas() could be a WebGL canvas if the raster canvas fails to initialize or isn't available. in that case, getContext("2d") returns null, and the ! blows up on the next line when calling getImageData().

even though FrameRenderer throws if rasterCtx initialization fails, the fallback to glCanvas in readbackVideoCanvas() means the 2D context isn't guaranteed. either:

  • add a guard: const canvasCtx = canvas.getContext("2d"); if (!canvasCtx) throw new Error(...)
  • or ensure the canvas is always 2D-compatible before this point
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/lib/exporter/videoExporter.ts` around lines 248 - 263, The code assumes a
non-null 2D context by calling canvas.getContext("2d")! before using
getImageData which can throw if readbackVideoCanvas() returned a WebGL canvas;
update the block around canvas.getContext("2d") in videoExporter.ts to check for
a valid 2D context (e.g., const canvasCtx = canvas.getContext("2d"); if
(!canvasCtx) throw new Error("Expected 2D canvas context for video readback"))
before calling getImageData and creating the VideoFrame, or alternatively ensure
the canvas returned by readbackVideoCanvas() is 2D-compatible earlier (reference
readbackVideoCanvas(), getContext("2d"), and the VideoFrame creation block).

Copy link
Copy Markdown
Contributor Author

@theopfr theopfr Apr 16, 2026

Choose a reason for hiding this comment

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

unrelated

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 16, 2026

Choose a reason for hiding this comment

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

@theopfr, fair point — I misread the canvas source. The canvas here comes from renderer.getCanvas() which is the composite output canvas (always 2D), not the result of readbackVideoCanvas(). The non-null assertion concern doesn't apply. My bad!

🧠 Learnings used 
Learnt from: theodorpeifer
Repo: siddharthvaddem/openscreen PR: 414
File: src/lib/exporter/streamingDecoder.ts:249-251
Timestamp: 2026-04-14T20:54:11.170Z
Learning: In `src/lib/exporter/streamingDecoder.ts` (TypeScript), `getExportMetrics()` intentionally computes `totalFrames` with a slight over-estimate (using `Math.ceil` with `EPSILON_SEC` subtracted per segment). This is a deliberate design trade-off: since metrics are computed before rendering starts, perfect prediction is impossible. A slightly high `totalFrames` (progress bar stops just below 100%) is acceptable and preferred over under-counting (progress bar exceeds 100%, the original bug).

Learnt from: zoharbabin
Repo: siddharthvaddem/openscreen PR: 415
File: src/components/video-editor/KalturaBrowseDialog.tsx:102-106
Timestamp: 2026-04-11T01:41:30.163Z
Learning: In `src/components/video-editor/KalturaBrowseDialog.tsx` and `electron/windows.ts`, the `createKalturaBrowseWindow` function creates a BrowserWindow with the full preload/electronAPI bridge attached. A remote Kaltura CDN ESM module is dynamically imported (`await import(loaderUrl)`) in this window. The author is aware of the security concern and has noted a future hardening plan to migrate to a `<webview>` tag or sandboxed BrowserWindow with no preload. `contextIsolation: true` is in place but does not prevent remote code from accessing `window.electronAPI`.

Learnt from: michTheBrandofficial
Repo: siddharthvaddem/openscreen PR: 450
File: src/components/launch/SourceSelector.tsx:68-68
Timestamp: 2026-04-15T13:59:58.679Z
Learning: In this codebase, Tailwind is configured such that `duration-*` classes (e.g., `duration-500`) produce both `transition-duration` and `animation-duration` rules. As a result, `duration-*` used together with `animate-*` utilities (e.g., `animate-spin`) is effective here and sets the animation duration (e.g., to 500ms). Do not flag occurrences of `duration-*` alongside `animate-*` as ineffective.

} else {
exportFrame = new VideoFrame(canvas, { timestamp, duration: frameDuration });
}

while (
this.encoder &&
Expand Down
11 changes: 8 additions & 3 deletions src/utils/platformUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ let cachedPlatform: string | null = null;
/**
* Gets the current platform from Electron
*/
const getPlatform = async (): Promise<string> => {
export const getPlatform = async (): Promise<string> => {
if (cachedPlatform) return cachedPlatform;

try {
Expand All @@ -14,9 +14,14 @@ const getPlatform = async (): Promise<string> => {
console.warn("Failed to get platform from Electron, falling back to navigator:", error);
// Fallback for development/testing
let fallbackPlatform = "win32";
if (typeof navigator !== "undefined" && /Mac|iPhone|iPad|iPod/.test(navigator.platform)) {
fallbackPlatform = "darwin";
if (typeof navigator !== "undefined") {
if (/Mac|iPhone|iPad|iPod/.test(navigator.platform)) {
fallbackPlatform = "darwin";
} else if (/Linux/.test(navigator.platform)) {
fallbackPlatform = "linux";
}
}

cachedPlatform = fallbackPlatform;
return fallbackPlatform;
}
Expand Down
Loading