Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 10 additions & 2 deletions src/services/ios/display/recording/av-capture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,16 @@ export async function recordScreenAndAudioToFiles(
log.debug(`Failed to stop cleanly: ${error instanceof Error ? error.message : String(error)}`);
});
await audioCapture.stop().catch((): void => undefined);
audioWritten = await audioOut.close();
await videoOut.close();
// The video writer closes even when the audio writer throws. M4aFileWriter
// finalizes by reopening the finished file to patch mdat's length, so it can
// fail on a full disk long after every frame is safely on disk. Sequencing
// the two closes would leak the video file descriptor for the life of the
// process, and the error its stream is holding would never be read.
try {
audioWritten = await audioOut.close();
} finally {
await videoOut.close();
}
}

const audioDurationMs = aacEldDurationMs(audioWritten.sampleCount);
Expand Down
61 changes: 60 additions & 1 deletion test/unit/display/recording/av-capture.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@ import assert from 'node:assert/strict';
import {mkdtemp, rm} from 'node:fs/promises';
import {tmpdir} from 'node:os';
import {join} from 'node:path';
import {after, before, describe, it} from 'node:test';
import {type TestContext, after, before, describe, it} from 'node:test';

import type {DisplayService, MediaStreamAnswer} from '../../../../src/services/ios/display/index.js';
import {recordScreenAndAudioToFiles} from '../../../../src/services/ios/display/recording/av-capture.js';
import {mockImport} from '../../../helpers/mock-module.js';

const AV_CAPTURE_MODULE = '../../../../src/services/ios/display/recording/av-capture.js';
const M4A_WRITER_MODULE = '../../../../src/services/ios/display/audio/m4a-writer.js';
const SCREEN_CAPTURE_MODULE = '../../../../src/services/ios/display/video/screen-stream-capture.js';

/** A stub service that negotiates without a device and counts teardowns. */
function makeStubService(): {service: DisplayService; stopCalls: () => number} {
Expand Down Expand Up @@ -60,4 +65,58 @@ describe('recordScreenAndAudioToFiles', function () {

assert.strictEqual(stopCalls(), 2, 'both captures must be stopped before the error propagates');
});

it('closes the video writer even when closing the audio writer throws', async function (t: TestContext) {
// M4aFileWriter finalizes by reopening the finished file to patch mdat's
// length, so it can fail on a full disk after every frame is already on
// disk. Sequencing the closes would skip the video one, leaking its file
// descriptor and leaving the error its stream holds unread.
let videoCloseCalls = 0;
const closeFailure = new Error('failed to patch the mdat header');

class StubM4aFileWriter {
static async create(): Promise<StubM4aFileWriter> {
return new StubM4aFileWriter();
}
async write(): Promise<void> {
return undefined;
}
async close(): Promise<never> {
throw closeFailure;
}
}

class StubAnnexBFileWriter {
constructor(path: string) {
void path;
}
async write(): Promise<void> {
return undefined;
}
async close(): Promise<void> {
videoCloseCalls += 1;
}
}

const {recordScreenAndAudioToFiles: record} = await mockImport<{
recordScreenAndAudioToFiles: typeof recordScreenAndAudioToFiles;
}>(t, AV_CAPTURE_MODULE, import.meta.url, {
[M4A_WRITER_MODULE]: {M4aFileWriter: StubM4aFileWriter},
// Merged over the real exports, so ScreenStreamCapture stays intact.
[SCREEN_CAPTURE_MODULE]: {AnnexBFileWriter: StubAnnexBFileWriter},
});

const {service} = makeStubService();
await assert.rejects(
() =>
record(service, {
videoPath: join(directory, 'screen-close.h265'),
audioPath: join(directory, 'audio-close.m4a'),
durationMs: 50,
}),
/failed to patch the mdat header/,
);

assert.strictEqual(videoCloseCalls, 1, 'the video writer must be closed even though the audio close threw');
});
});