From 135365a0b25ce2a159b993778ac634cd3961ca8f Mon Sep 17 00:00:00 2001 From: trtshen Date: Fri, 10 Apr 2026 23:20:15 +0800 Subject: [PATCH 1/2] [CORE-7691] navigate away warning --- docs/features/video-compression.md | 106 ++++++++++++++++-- .../src/app/components/components.module.ts | 2 - .../file-upload/file-upload.component.ts | 1 + .../uppy-uploader/uppy-uploader.component.ts | 8 +- .../uppy-uploader.service.spec.ts | 28 +++++ .../uppy-uploader/uppy-uploader.service.ts | 23 +++- .../single-page-deactivate.guard.spec.ts | 46 +++++++- .../guards/single-page-deactivate.guard.ts | 18 ++- .../src/app/services/ffmpeg.service.spec.ts | 35 ++++++ .../v3/src/app/services/ffmpeg.service.ts | 24 +++- 10 files changed, 260 insertions(+), 31 deletions(-) diff --git a/docs/features/video-compression.md b/docs/features/video-compression.md index c9d44f855..8c5a7f891 100644 --- a/docs/features/video-compression.md +++ b/docs/features/video-compression.md @@ -185,36 +185,120 @@ the installed `@ffmpeg/core` v0.12.10 is the **single-thread** build. this was a ### 4. wasm asset loading -assets are loaded lazily from `assets/ffmpeg/`: +assets are loaded lazily from `assets/ffmpeg/` using `document.baseURI` as the base (not `window.location.origin`) to correctly resolve under locale-prefixed paths like `/en-US/`: ```typescript await this.ffmpeg.load({ - coreURL: new URL('assets/ffmpeg/ffmpeg-core.js', window.location.origin).toString(), - wasmURL: new URL('assets/ffmpeg/ffmpeg-core.wasm', window.location.origin).toString(), - classWorkerURL: new URL('assets/ffmpeg/worker.js', window.location.origin).toString(), + coreURL: new URL('assets/ffmpeg/ffmpeg-core.js', document.baseURI).toString(), + wasmURL: new URL('assets/ffmpeg/ffmpeg-core.wasm', document.baseURI).toString(), + classWorkerURL: new URL('assets/ffmpeg/worker.js', document.baseURI).toString(), }); ``` +**why `document.baseURI`?** the app is served under `` on staging/production. `window.location.origin` returns `https://app.p2-stage.practera.com` — missing the locale prefix — so asset requests 404 and the SPA returns `index.html` instead. `document.baseURI` respects the `` tag and resolves to the correct path. + +#### vendored worker.js + +`projects/v3/src/assets/ffmpeg/worker.js` is a **vendored copy** of `node_modules/@ffmpeg/ffmpeg/dist/esm/worker.js` (v0.12.15). the upstream file has ES module `import` statements referencing `./const.js` and `./errors.js` — but angular's asset copy only copies the files we list in `angular.json`, not the full `dist/esm/` tree. when the worker is loaded as a standalone asset at `/en-US/assets/ffmpeg/worker.js`, those sibling modules don't exist at the serving path, so the worker silently fails and `ffmpeg.load()` hangs forever. + +the fix: all dependencies from `const.js` and `errors.js` are inlined directly into `worker.js`. the file header documents the exact upstream version and which constants were inlined. **keep this in sync when upgrading `@ffmpeg/ffmpeg`.** + +#### angular.json asset config + +```json +{ + "glob": "**/*", + "input": "node_modules/@ffmpeg/ffmpeg/dist/esm", + "output": "./assets/ffmpeg" +}, +"projects/v3/src/assets/ffmpeg" +``` + +the node_modules glob copies the upstream files first. then the local `projects/v3/src/assets/ffmpeg` directory is listed **after**, so its files (including the vendored `worker.js`) override the upstream versions. this is intentional — it provides a fallback for any new files added in future `@ffmpeg/ffmpeg` releases while ensuring the vendored worker takes priority. + **asset sizes:** - `ffmpeg-core.wasm`: ~31 MB (downloaded once, browser-cached) - `ffmpeg-core.js`: ~200 KB (emscripten glue) - `worker.js`: ~2 KB -**caching strategy:** the wasm file is loaded once and the `FFmpeg` instance is reused. subsequent compressions skip the load step entirely. +**caching strategy:** the wasm file is loaded once and the `FFmpeg` instance is reused. subsequent compressions skip the load step entirely (unless `terminate()` was called, in which case the next compression will lazy-load again). ### 5. known limitations 1. **memory**: wasm heap limited to 2 GB. very large files (>500 MB) may cause oom on constrained devices. 2. **no hardware acceleration**: wasm cannot access gpu/videotoolbox — encoding is pure cpu. 3. **single-thread**: only uses one cpu core. multi-thread would help but requires coop/coep headers and drops ios support. -4. **asset duplication in angular.json**: both `node_modules/@ffmpeg/ffmpeg/dist/esm` and `projects/v3/src/assets/ffmpeg` are copied to output. the node_modules copy should be removed once local assets are confirmed stable. + +--- + +## transcoding + +`FfmpegService.transcodeToMp4(file)` converts any user-supplied video to h.264/aac mp4 without custom compression parameters. this is used when the goal is format normalization (e.g. `.webm` → `.mp4`) rather than size reduction. it uses the same lazy wasm load and exec timeout as `compressVideo()`. + +--- + +## error handling + +### exec timeout + +all `ffmpeg.exec()` calls have a timeout: +- **desktop**: 10 minutes +- **mobile**: 5 minutes +- configurable via `CompressionOptions.timeout` (ms, -1 = unlimited) + +if the timeout fires, ffmpeg throws and the preprocessor `catch` block uploads the original file uncompressed. + +### compression cancellation (`cancelCompression`) + +`UppyUploaderService.cancelCompression()` terminates the wasm worker mid-encode and resets all state: + +``` +cancelCompression() → ffmpegService.terminate() → progress$.next(null) → compressingUppy = null +``` + +called automatically from: +- `FileUploadComponent.ngOnDestroy()` — triggered by route navigation away from assessment +- `UppyUploaderComponent.ngOnDestroy()` — triggered by modal dismissal or route change + +after termination, `FfmpegService` creates a fresh `FFmpeg` instance so the next compression starts cleanly. + +### navigation guards + +| scenario | protection | +|----------|------------| +| user clicks another task / browser back (angular route) | `SinglePageDeactivateGuard` — shows `window.confirm()` dialog, cancels compression if user confirms leave | +| user closes tab or reloads | `beforeunload` event listener on `UppyUploaderService` — browser shows native "leave page?" prompt | +| user swipe-dismisses modal (ionic gesture) | `canDismiss` on modal — returns `false` while `compressingUppy !== null` | +| modal dismissed programmatically (e.g. route forces modal stack clear) | `UppyUploaderComponent.ngOnDestroy()` calls `cancelCompression()` | + +### subscription leak prevention + +the progress subscription in `registerCompressionPreProcessor` uses `try/finally` to guarantee `sub.unsubscribe()` runs even if `compressVideo()` throws: + +```typescript +const sub = ffmpegService.progress$.subscribe(...); +try { + result = await ffmpegService.compressVideo(file); +} finally { + sub.unsubscribe(); +} +``` + +### graceful fallback + +if compression fails for any reason (timeout, wasm crash, out of memory), the preprocessor `catch` block: +1. logs the error +2. nulls `compressingUppy` +3. emits `progress: null` to hide the overlay +4. lets uppy proceed with the **original uncompressed file** + +the user is never blocked from uploading. ## future improvements | priority | improvement | effort | |----------|-------------|--------| -| p1 | remove angular.json asset duplication | low | -| p2 | add feature flag to toggle compression | low | -| p3 | compression quality preview (thumbnail before/after) | medium | -| p4 | evaluate `@ffmpeg/core-mt` when ios safari adds SharedArrayBuffer | medium | -| p5 | webcodecs api as alternative for simple transcode (chrome 94+) | high | +| p1 | add feature flag to toggle compression | low | +| p2 | compression quality preview (thumbnail before/after) | medium | +| p3 | evaluate `@ffmpeg/core-mt` when ios safari adds SharedArrayBuffer | medium | +| p4 | webcodecs api as alternative for simple transcode (chrome 94+) | high | diff --git a/projects/v3/src/app/components/components.module.ts b/projects/v3/src/app/components/components.module.ts index 713e6b8fe..6f314012a 100644 --- a/projects/v3/src/app/components/components.module.ts +++ b/projects/v3/src/app/components/components.module.ts @@ -41,7 +41,6 @@ import { ToggleLabelDirective } from '../directives/toggle-label/toggle-label.di import { TrafficLightGroupComponent } from './traffic-light-group/traffic-light-group.component'; import { UppyUploaderComponent } from './uppy-uploader/uppy-uploader.component'; import { FileUploadComponent } from './file-upload/file-upload.component'; -import { UppyUploaderService } from './uppy-uploader/uppy-uploader.service'; import { FilePopupComponent } from './file-popup/file-popup.component'; import { SliderComponent } from './slider/slider.component'; import { LanguageDetectionPipe } from '../pipes/language.pipe'; @@ -163,6 +162,5 @@ const largeCircleDefaultConfig = { FileUploadComponent, LanguageDetectionPipe, ], - providers: [UppyUploaderService] }) export class ComponentsModule { } diff --git a/projects/v3/src/app/components/file-upload/file-upload.component.ts b/projects/v3/src/app/components/file-upload/file-upload.component.ts index 3c0a5102a..2fc0967c2 100644 --- a/projects/v3/src/app/components/file-upload/file-upload.component.ts +++ b/projects/v3/src/app/components/file-upload/file-upload.component.ts @@ -102,6 +102,7 @@ export class FileUploadComponent implements OnInit, OnDestroy { ngOnDestroy(): void { this.compressionSub?.unsubscribe(); + this.uppyUploaderService.cancelCompression(); this.uppy.destroy(); } diff --git a/projects/v3/src/app/components/uppy-uploader/uppy-uploader.component.ts b/projects/v3/src/app/components/uppy-uploader/uppy-uploader.component.ts index 0312fc1dc..7dcc55ee3 100644 --- a/projects/v3/src/app/components/uppy-uploader/uppy-uploader.component.ts +++ b/projects/v3/src/app/components/uppy-uploader/uppy-uploader.component.ts @@ -119,13 +119,9 @@ export class UppyUploaderComponent implements OnInit, OnDestroy { ngOnDestroy() { this.compressionSub?.unsubscribe(); + this.uppyUploaderService.cancelCompression(); if (this.uppy) { - // eslint-disable-next-line no-console - this.uppy.off("upload-success", (res) => console.info(res)); - - // eslint-disable-next-line no-console - this.uppy.off("complete", (res) => console.info(res)); - this.uppy.resetProgress(); + this.uppy.destroy(); } } diff --git a/projects/v3/src/app/components/uppy-uploader/uppy-uploader.service.spec.ts b/projects/v3/src/app/components/uppy-uploader/uppy-uploader.service.spec.ts index a66c8ae77..0fa14d84b 100644 --- a/projects/v3/src/app/components/uppy-uploader/uppy-uploader.service.spec.ts +++ b/projects/v3/src/app/components/uppy-uploader/uppy-uploader.service.spec.ts @@ -21,6 +21,7 @@ describe('UppyUploaderService', () => { ffmpegServiceSpy = jasmine.createSpyObj('FfmpegService', [ 'shouldCompress', 'compressVideo', + 'terminate', ], { progress$: new Subject(), }); @@ -153,6 +154,33 @@ describe('UppyUploaderService', () => { }); }); + describe('cancelCompression', () => { + it('should do nothing if no compression is active', () => { + service.compressingUppy = null; + + service.cancelCompression(); + + expect(ffmpegServiceSpy.terminate).not.toHaveBeenCalled(); + }); + + it('should terminate ffmpeg and emit null progress when compressing', () => { + const fakeUppy = {} as Uppy; + service.compressingUppy = fakeUppy; + + const emitted: any[] = []; + const sub = service.compressionProgress$.subscribe(v => emitted.push(v)); + + service.cancelCompression(); + + expect(ffmpegServiceSpy.terminate).toHaveBeenCalled(); + expect(emitted.length).toBe(1); + expect(emitted[0]).toEqual({ uppy: fakeUppy, progress: null }); + expect(service.compressingUppy).toBeNull(); + + sub.unsubscribe(); + }); + }); + describe('open', () => { it('should create a modal with backdropDismiss false', async () => { const mockModal = jasmine.createSpyObj('HTMLIonModalElement', ['present']); diff --git a/projects/v3/src/app/components/uppy-uploader/uppy-uploader.service.ts b/projects/v3/src/app/components/uppy-uploader/uppy-uploader.service.ts index 53bd6459f..b7a13b1b5 100644 --- a/projects/v3/src/app/components/uppy-uploader/uppy-uploader.service.ts +++ b/projects/v3/src/app/components/uppy-uploader/uppy-uploader.service.ts @@ -114,6 +114,21 @@ export class UppyUploaderService { private ffmpegService: FfmpegService, private ngZone: NgZone, ) { + // warn the user before tab close/reload if compression is active + window.addEventListener('beforeunload', (e) => { + if (this.compressingUppy) { + e.preventDefault(); + } + }); + } + + /** cancel any in-flight compression, terminate the wasm worker, and reset state */ + cancelCompression(): void { + if (this.compressingUppy) { + this.ffmpegService.terminate(); + this.compressionProgress$.next({ uppy: this.compressingUppy, progress: null }); + this.compressingUppy = null; + } } /** @@ -240,9 +255,13 @@ export class UppyUploaderService { this.compressionProgress$.next({ uppy, progress: p }); }); - const result = await this.ffmpegService.compressVideo(file); + let result; + try { + result = await this.ffmpegService.compressVideo(file); + } finally { + sub.unsubscribe(); + } - sub.unsubscribe(); this.ngZone.run(() => { this.compressionProgress$.next({ uppy, progress: null }); this.compressingUppy = null; diff --git a/projects/v3/src/app/guards/single-page-deactivate.guard.spec.ts b/projects/v3/src/app/guards/single-page-deactivate.guard.spec.ts index 27dcfd750..02646980e 100644 --- a/projects/v3/src/app/guards/single-page-deactivate.guard.spec.ts +++ b/projects/v3/src/app/guards/single-page-deactivate.guard.spec.ts @@ -1,9 +1,11 @@ -import { TestBed, waitForAsync, inject } from '@angular/core/testing'; +import { TestBed, inject } from '@angular/core/testing'; import { BrowserStorageService } from '@v3/services/storage.service'; +import { UppyUploaderService } from '@v3/app/components/uppy-uploader/uppy-uploader.service'; import { SinglePageDeactivateGuard } from './single-page-deactivate.guard'; describe('SinglePageDeactivateGuard', () => { let storageSpy: jasmine.SpyObj; + let uppySpy: jasmine.SpyObj; beforeEach(() => { TestBed.configureTestingModule({ @@ -16,9 +18,17 @@ describe('SinglePageDeactivateGuard', () => { singlePageAccess: jasmine.createSpy('singlePageAccess') }) }, + { + provide: UppyUploaderService, + useValue: { + compressingUppy: null, + cancelCompression: jasmine.createSpy('cancelCompression'), + }, + }, ], }); storageSpy = TestBed.inject(BrowserStorageService) as jasmine.SpyObj; + uppySpy = TestBed.inject(UppyUploaderService) as jasmine.SpyObj; }); it('should be created', inject([SinglePageDeactivateGuard], (guard: SinglePageDeactivateGuard) => { @@ -26,17 +36,41 @@ describe('SinglePageDeactivateGuard', () => { })); describe('canDeactivate()', () => { - it('should be false if storage has singePageAccess set as true', inject([SinglePageDeactivateGuard], (guard: SinglePageDeactivateGuard) => { + it('should be false if storage has singePageAccess set as true', inject([SinglePageDeactivateGuard], async (guard: SinglePageDeactivateGuard) => { storageSpy.singlePageAccess = true; - expect(guard.canDeactivate()).toBeFalsy(); + expect(await guard.canDeactivate()).toBeFalsy(); })); - it('should be able to deactivated if storage has singePageAccess is false/null', inject([SinglePageDeactivateGuard], (guard: SinglePageDeactivateGuard) => { + it('should be able to deactivated if storage has singePageAccess is false/null', inject([SinglePageDeactivateGuard], async (guard: SinglePageDeactivateGuard) => { storageSpy.singlePageAccess = false; - expect(guard.canDeactivate()).toBeTruthy(); + expect(await guard.canDeactivate()).toBeTruthy(); storageSpy.singlePageAccess = null; - expect(guard.canDeactivate()).toBeTruthy(); + expect(await guard.canDeactivate()).toBeTruthy(); + })); + + it('should prompt and cancel compression when user confirms leave', inject([SinglePageDeactivateGuard], async (guard: SinglePageDeactivateGuard) => { + storageSpy.singlePageAccess = false; + (uppySpy as any).compressingUppy = {} as any; + spyOn(window, 'confirm').and.returnValue(true); + + const result = await guard.canDeactivate(); + + expect(window.confirm).toHaveBeenCalled(); + expect(uppySpy.cancelCompression).toHaveBeenCalled(); + expect(result).toBeTrue(); + })); + + it('should prompt and block navigation when user cancels leave', inject([SinglePageDeactivateGuard], async (guard: SinglePageDeactivateGuard) => { + storageSpy.singlePageAccess = false; + (uppySpy as any).compressingUppy = {} as any; + spyOn(window, 'confirm').and.returnValue(false); + + const result = await guard.canDeactivate(); + + expect(window.confirm).toHaveBeenCalled(); + expect(uppySpy.cancelCompression).not.toHaveBeenCalled(); + expect(result).toBeFalse(); })); }); }); diff --git a/projects/v3/src/app/guards/single-page-deactivate.guard.ts b/projects/v3/src/app/guards/single-page-deactivate.guard.ts index 10b63349d..7ed8802f3 100644 --- a/projects/v3/src/app/guards/single-page-deactivate.guard.ts +++ b/projects/v3/src/app/guards/single-page-deactivate.guard.ts @@ -1,17 +1,31 @@ import { Injectable } from '@angular/core'; import { BrowserStorageService } from '@v3/services/storage.service'; +import { UppyUploaderService } from '@v3/app/components/uppy-uploader/uppy-uploader.service'; @Injectable({ providedIn: 'root', }) export class SinglePageDeactivateGuard { - constructor(readonly storage: BrowserStorageService) {} + constructor( + readonly storage: BrowserStorageService, + private uppyUploaderService: UppyUploaderService, + ) {} - canDeactivate() { + async canDeactivate(): Promise { if (this.storage.singlePageAccess === true) { return false; } + if (this.uppyUploaderService.compressingUppy !== null) { + const leave = window.confirm( + 'Video is still compressing. If you leave, the file will not be uploaded. Continue?' + ); + if (leave) { + this.uppyUploaderService.cancelCompression(); + } + return leave; + } + return true; } } diff --git a/projects/v3/src/app/services/ffmpeg.service.spec.ts b/projects/v3/src/app/services/ffmpeg.service.spec.ts index aef516e64..c90adae88 100644 --- a/projects/v3/src/app/services/ffmpeg.service.spec.ts +++ b/projects/v3/src/app/services/ffmpeg.service.spec.ts @@ -186,4 +186,39 @@ describe('FfmpegService', () => { (service as any).isLoaded = false; }); }); + + describe('terminate', () => { + it('should call ffmpeg.terminate and reset isLoaded', () => { + (service as any).isLoaded = true; + const terminateSpy = spyOn((service as any).ffmpeg, 'terminate'); + + service.terminate(); + + expect(terminateSpy).toHaveBeenCalled(); + expect((service as any).isLoaded).toBeFalse(); + }); + + it('should create a new FFmpeg instance after termination', () => { + const oldFfmpeg = (service as any).ffmpeg; + spyOn(oldFfmpeg, 'terminate'); + + service.terminate(); + + expect((service as any).ffmpeg).not.toBe(oldFfmpeg); + }); + }); + + describe('getDefaultTimeout', () => { + it('should return mobile timeout when mobile', () => { + spyOn(service, 'isMobile').and.returnValue(true); + const timeout = (service as any).getDefaultTimeout(); + expect(timeout).toBe(5 * 60 * 1000); + }); + + it('should return desktop timeout when not mobile', () => { + spyOn(service, 'isMobile').and.returnValue(false); + const timeout = (service as any).getDefaultTimeout(); + expect(timeout).toBe(10 * 60 * 1000); + }); + }); }); diff --git a/projects/v3/src/app/services/ffmpeg.service.ts b/projects/v3/src/app/services/ffmpeg.service.ts index 8bf6c5330..090728820 100644 --- a/projects/v3/src/app/services/ffmpeg.service.ts +++ b/projects/v3/src/app/services/ffmpeg.service.ts @@ -12,6 +12,8 @@ export interface CompressionOptions { preset?: 'ultrafast' | 'superfast' | 'veryfast' | 'faster' | 'fast' | 'medium' | 'slow'; /** audio bitrate */ audioBitrate?: string; + /** exec timeout in milliseconds (-1 = unlimited) */ + timeout?: number; } export interface CompressionProgress { @@ -36,6 +38,9 @@ const MIN_COMPRESS_SIZE = 5 * 1024 * 1024; const MAX_MOBILE_SIZE = 200 * 1024 * 1024; /** max file size on desktop (500 MB) */ const MAX_DESKTOP_SIZE = 500 * 1024 * 1024; +/** default exec timeout: 10 min desktop, 5 min mobile */ +const DESKTOP_TIMEOUT_MS = 10 * 60 * 1000; +const MOBILE_TIMEOUT_MS = 5 * 60 * 1000; @Injectable({ providedIn: 'root', @@ -55,6 +60,20 @@ export class FfmpegService { return this.isLoaded; } + /** terminate the wasm worker and reset state so a fresh load can happen */ + terminate(): void { + try { + this.ffmpeg.terminate(); + } catch { /* already terminated or never loaded */ } + this.isLoaded = false; + this.ffmpeg = new FFmpeg(); + } + + /** default exec timeout based on device type */ + private getDefaultTimeout(): number { + return this.isMobile() ? MOBILE_TIMEOUT_MS : DESKTOP_TIMEOUT_MS; + } + /** detect if the current device is mobile */ isMobile(): boolean { return /Android|iPhone|iPad|iPod/i.test(navigator.userAgent); @@ -133,6 +152,7 @@ export class FfmpegService { crf = defaults.crf, preset = defaults.preset, audioBitrate = defaults.audioBitrate, + timeout = this.getDefaultTimeout(), } = options; if (!this.isLoaded) { @@ -155,7 +175,7 @@ export class FfmpegService { '-b:a', audioBitrate, '-movflags', '+faststart', outputName, - ]); + ], timeout); const fileData = await this.ffmpeg.readFile(outputName); const blob = new Blob([fileData as ArrayBuffer], { type: 'video/mp4' }); @@ -196,7 +216,7 @@ export class FfmpegService { '-b:a', '128k', '-movflags', '+faststart', outputName, - ]); + ], this.getDefaultTimeout()); const fileData = await this.ffmpeg.readFile(outputName); const blob = new Blob([fileData as ArrayBuffer], { type: 'video/mp4' }); From 5be678b47e50d0d212e9a5fc68ef55b01678b4b1 Mon Sep 17 00:00:00 2001 From: trtshen Date: Tue, 14 Apr 2026 16:28:55 +0800 Subject: [PATCH 2/2] [CORE-7691] add probeMetadata and conditional scale for video compression - implement probeMetadata to extract video dimensions - conditionally apply scale filter based on metadata - update tests for new probeMetadata functionality --- docs/features/video-compression.md | 69 +++++++++++++++--- .../src/app/services/ffmpeg.service.spec.ts | 72 +++++++++++++++++++ .../v3/src/app/services/ffmpeg.service.ts | 67 +++++++++++++++-- 3 files changed, 194 insertions(+), 14 deletions(-) diff --git a/docs/features/video-compression.md b/docs/features/video-compression.md index 8c5a7f891..1fabbf534 100644 --- a/docs/features/video-compression.md +++ b/docs/features/video-compression.md @@ -29,8 +29,11 @@ the app uses [@ffmpeg/ffmpeg](https://github.com/ffmpegwasm/ffmpeg.wasm) v0.12.1 │ │ FfmpegService │ │ │ │ ┌────────────────┐ │ │ │ │ │ compressVideo()│ │ │ -│ │ └────────────────┘ │ │ -│ │ ┌────────────────┐ │ │ +│ │ └───────┬────────┘ │ │ +│ │ ┌───────▼────────┐ │ │ +│ │ │probeMetadata() │ │