fix(screencast): correctly process videos with 1 frame (#4144)

microsoft · Oct 14, 2020 · bb981fc · bb981fc
1 parent 25cb649
commit bb981fc
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 11 deletions.
diff --git a/src/server/chromium/videoRecorder.ts b/src/server/chromium/videoRecorder.ts
@@ -26,7 +26,7 @@ const fps = 25;
 export class VideoRecorder {
   private _process: ChildProcess | null = null;
   private _gracefullyClose: (() => Promise<void>) | null = null;
-  private _lastWritePromise: Promise<void>;
+  private _lastWritePromise: Promise<void> | undefined;
   private _lastFrameTimestamp: number = 0;
   private _lastFrameBuffer: Buffer | null = null;
   private _lastWriteTimestamp: number = 0;
@@ -47,7 +47,6 @@ export class VideoRecorder {
 
   private constructor(progress: Progress) {
     this._progress = progress;
-    this._lastWritePromise = Promise.resolve();
   }
 
   private async _launch(options: types.PageScreencastOptions) {
@@ -89,22 +88,24 @@ export class VideoRecorder {
     assert(this._process);
     if (!this._isRunning())
       return;
-    const repeatCount = this._lastFrameTimestamp ? Math.max(1, Math.round(25 * (timestamp - this._lastFrameTimestamp))) : 1;
-    this._progress.log(`writing ${repeatCount} frame(s)`);
-    this._lastWritePromise = this._flushLastFrame(repeatCount).catch(e => this._progress.log('Error while writing frame: ' + e));
+    this._progress.log(`writing frame ` + timestamp);
+    if (this._lastFrameBuffer)
+      this._lastWritePromise = this._flushLastFrame(timestamp - this._lastFrameTimestamp).catch(e => this._progress.log('Error while writing frame: ' + e));
     this._lastFrameBuffer = frame;
     this._lastFrameTimestamp = timestamp;
     this._lastWriteTimestamp = Date.now();
   }
 
-  private async _flushLastFrame(repeatCount: number): Promise<void> {
+  private async _flushLastFrame(durationSec: number): Promise<void> {
     assert(this._process);
     const frame = this._lastFrameBuffer;
     if (!frame)
       return;
     const previousWrites = this._lastWritePromise;
     let finishedWriting: () => void;
     const writePromise = new Promise<void>(fulfill => finishedWriting = fulfill);
+    const repeatCount = Math.max(1, Math.round(fps * durationSec));
+    this._progress.log(`flushing ${repeatCount} frame(s)`);
     await previousWrites;
     for (let i = 0; i < repeatCount; i++) {
       const callFinish = i === (repeatCount - 1);
@@ -124,8 +125,8 @@ export class VideoRecorder {
 
     if (this._lastWriteTimestamp) {
       const durationSec = (Date.now() - this._lastWriteTimestamp) / 1000;
-      if (durationSec > 1 / fps)
-        this.writeFrame(this._lastFrameBuffer!, this._lastFrameTimestamp + durationSec);
+      if (!this._lastWritePromise || durationSec > 1 / fps)
+        this._flushLastFrame(durationSec).catch(e => this._progress.log('Error while writing frame: ' + e));
     }
 
     const close = this._gracefullyClose;

diff --git a/test/screencast.spec.ts b/test/screencast.spec.ts
@@ -180,7 +180,7 @@ describe('screencast', suite => {
     }
   });
 
-  it('should emit video event', async ({browser, testInfo}) => {
+  it('should expose video path', async ({browser, testInfo}) => {
     const videosPath = testInfo.outputPath('');
     const size = { width: 320, height: 240 };
     const context = await browser.newContext({
@@ -190,10 +190,10 @@ describe('screencast', suite => {
     });
     const page = await context.newPage();
     await page.evaluate(() => document.body.style.backgroundColor = 'red');
-    await new Promise(r => setTimeout(r, 1000));
-    await context.close();
     const path = await page.video()!.path();
     expect(path).toContain(videosPath);
+    await context.close();
+    expect(fs.existsSync(path)).toBeTruthy();
   });
 
   it('should capture navigation', async ({browser, server, testInfo}) => {