Fix voice note review races

Signed-off-by: kenny lopez <klopez4212@gmail.com>
This commit is contained in:
kenny lopez
2026-09-15 11:17:00 -04:00
parent 69908368b6
commit 3b298e8f7b
4 changed files with 207 additions and 10 deletions

View File

@@ -1,6 +1,6 @@
import * as React from "react";
import { summarizeWaveform, waveformPeaks } from "./audioAttachment";
import { summarizeWaveform } from "./audioAttachment";
import { encodeVoiceNoteWav } from "./voiceNoteWav";
const MIME_CANDIDATES = [
@@ -150,9 +150,8 @@ export function useVoiceNoteRecorder() {
file: new File([wavBuffer], `voice-note-${Date.now()}.wav`, {
type: "audio/wav",
}),
waveform: waveformPeaks(
waveform: Array.from(
summarizeWaveform(channels[0] ?? new Float32Array(), 48),
48,
),
};
}

View File

@@ -35,13 +35,22 @@ async function waitForMockLiveSubscription(page: Page, channelName: string) {
}
test.beforeEach(async ({ page }, testInfo) => {
await page.addInitScript(() => {
await page.addInitScript((silentRecording) => {
if (silentRecording) {
AudioContext.prototype.decodeAudioData = async () =>
({
duration: 0.1,
getChannelData: () => new Float32Array(4_800),
numberOfChannels: 1,
sampleRate: 48_000,
}) as AudioBuffer;
}
Object.defineProperty(navigator.mediaDevices, "getUserMedia", {
configurable: true,
async value() {
const context = new AudioContext();
const oscillator = context.createOscillator();
const destination = context.createMediaStreamDestination();
const oscillator = context.createOscillator();
oscillator.frequency.value = 220;
oscillator.connect(destination);
oscillator.start();
@@ -56,7 +65,7 @@ test.beforeEach(async ({ page }, testInfo) => {
return destination.stream;
},
});
});
}, testInfo.title.includes("silent recording"));
await installMockBridge(page, {
deferredComposerUploads: true,
@@ -853,6 +862,70 @@ test("records from the composer and renders an inline waveform card", async ({
});
});
test("keeps a silent recording at minimum waveform height after sending", async ({
page,
}) => {
await page.goto("/");
await page.getByTestId("channel-general").click();
await page.getByRole("button", { name: "Record voice note" }).click();
await page.waitForTimeout(100);
await page.getByRole("button", { name: "Finish voice note" }).click();
await expect(page.getByTestId("composer-voice-note-card")).toBeVisible();
const send = page.getByTestId("send-message");
await expect(send).toBeEnabled();
await send.click();
await expect
.poll(() =>
page.evaluate(() => {
const payload = window.__BUZZ_E2E_COMMAND_LOG__?.findLast(
(call) => call.command === "send_channel_message",
)?.payload as { mediaTags?: string[][] } | undefined;
return payload?.mediaTags?.find(
(tag) =>
tag[0] === "imeta" &&
tag.some((field) => field.startsWith("waveform ")),
);
}),
)
.not.toBeUndefined();
const persistedSamples = await page.evaluate(() => {
const payload = window.__BUZZ_E2E_COMMAND_LOG__?.findLast(
(call) => call.command === "send_channel_message",
)?.payload as { mediaTags?: string[][] } | undefined;
const tag = payload?.mediaTags?.find(
(candidate) =>
candidate[0] === "imeta" &&
candidate.some((field) => field.startsWith("waveform ")),
);
const waveform = tag?.find((field) => field.startsWith("waveform "));
return waveform?.slice("waveform ".length).split(" ").map(Number) ?? [];
});
expect(persistedSamples).toHaveLength(48);
expect(new Set(persistedSamples)).toEqual(new Set([0]));
const waveform = page
.getByTestId("audio-message-attachment")
.last()
.getByTestId("voice-note-playback-waveform");
await expect(waveform).toHaveAttribute("data-waveform-state", "ready");
await waitForAnimations(page);
expect(
await waveform
.locator("span")
.evaluateAll((bars) =>
bars.map((bar) => bar.getBoundingClientRect().height),
),
).toEqual(expect.arrayContaining([3]));
expect(
await waveform
.locator("span")
.evaluateAll((bars) =>
Math.max(...bars.map((bar) => bar.getBoundingClientRect().height)),
),
).toBe(3);
});
test("keeps emoji available but blocks GIFs beside a queued voice note", async ({
page,
}) => {

View File

@@ -878,15 +878,23 @@ class DeviceVoiceNotePlayerController extends VoiceNotePlayerController {
}
@override
Future<void> seek(Duration position) => _player.seek(position);
Future<void> seek(Duration position) {
_playbackOperationGeneration += 1;
return _player.seek(position);
}
@override
Future<void> setSpeed(double speed) async {
final preservedPosition = _state.position;
final wasPlaying = _state.isPlaying || _player.playing;
final sourceGeneration = _sourceGeneration;
final playbackOperationGeneration = ++_playbackOperationGeneration;
await _player.setSpeed(speed);
if (_disposed || sourceGeneration != _sourceGeneration) return;
if (_disposed ||
sourceGeneration != _sourceGeneration ||
playbackOperationGeneration != _playbackOperationGeneration) {
return;
}
// Some platform decoders rebuild the MP4 playback item when its rate
// changes and briefly report position zero. Restore the user's position
@@ -894,10 +902,14 @@ class DeviceVoiceNotePlayerController extends VoiceNotePlayerController {
if (preservedPosition > Duration.zero &&
_state.position < preservedPosition) {
await _player.seek(preservedPosition);
if (_disposed || sourceGeneration != _sourceGeneration) return;
if (_disposed ||
sourceGeneration != _sourceGeneration ||
playbackOperationGeneration != _playbackOperationGeneration) {
return;
}
_update(_state.copyWith(position: preservedPosition));
}
if (wasPlaying && !_player.playing) {
if (wasPlaying && !_player.playing && _coordinator.ownsPlayback(this)) {
unawaited(_play(sourceGeneration));
}
}

View File

@@ -87,6 +87,7 @@ class _FakeAudioPlayerBackend implements VoiceNoteAudioPlayerBackend {
bool delayPathLoads = false;
bool delayUrlLoads = false;
bool delayPlay = false;
bool delaySpeedChanges = false;
bool _playing = false;
int playCount = 0;
int pauseCount = 0;
@@ -94,6 +95,7 @@ class _FakeAudioPlayerBackend implements VoiceNoteAudioPlayerBackend {
bool resetPositionOnSpeedChange = false;
Duration? lastSeekPosition;
double speed = 1;
final speedChanges = <Completer<void>>[];
final loadedPaths = <String>[];
final loadedUrls = <String>[];
final loadedUrlHeaders = <Map<String, String>?>[];
@@ -170,6 +172,12 @@ class _FakeAudioPlayerBackend implements VoiceNoteAudioPlayerBackend {
Future<void> setSpeed(double value) async {
speed = value;
if (resetPositionOnSpeedChange) positions.add(Duration.zero);
if (delaySpeedChanges) {
final change = Completer<void>();
speedChanges.add(change);
await change.future;
speedChanges.remove(change);
}
}
@override
@@ -962,6 +970,111 @@ void main() {
expect(player.state.position, const Duration(milliseconds: 3250));
});
test('a pending speed change cannot restart playback after pause', () async {
final audioPlayer = _FakeAudioPlayerBackend()
..delaySpeedChanges = true
..resetPositionOnSpeedChange = true;
final player = DeviceVoiceNotePlayerController(
coordinator: VoiceNotePlaybackCoordinator(),
client: _SequencedHttpClient(),
requiresAuthenticatedLocalFile: false,
player: audioPlayer,
);
addTearDown(player.dispose);
await player.loadLocal(
'/tmp/voice-note.m4a',
fallbackDuration: const Duration(seconds: 7),
);
await player.toggle();
audioPlayer.positions.add(const Duration(seconds: 3));
await Future<void>.delayed(Duration.zero);
final speedChange = player.setSpeed(1.5);
await Future<void>.delayed(Duration.zero);
await player.pause();
audioPlayer.speedChanges.single.complete();
await speedChange;
await Future<void>.delayed(Duration.zero);
expect(audioPlayer.playCount, 1);
expect(audioPlayer.playing, isFalse);
expect(audioPlayer.lastSeekPosition, isNull);
});
test('a pending speed change cannot overwrite a later seek', () async {
final audioPlayer = _FakeAudioPlayerBackend()
..delaySpeedChanges = true
..resetPositionOnSpeedChange = true;
final player = DeviceVoiceNotePlayerController(
coordinator: VoiceNotePlaybackCoordinator(),
client: _SequencedHttpClient(),
requiresAuthenticatedLocalFile: false,
player: audioPlayer,
);
addTearDown(player.dispose);
await player.loadLocal(
'/tmp/voice-note.m4a',
fallbackDuration: const Duration(seconds: 7),
);
audioPlayer.positions.add(const Duration(seconds: 3));
await Future<void>.delayed(Duration.zero);
final speedChange = player.setSpeed(1.5);
await Future<void>.delayed(Duration.zero);
await player.seek(const Duration(seconds: 1));
audioPlayer.speedChanges.single.complete();
await speedChange;
await Future<void>.delayed(Duration.zero);
expect(audioPlayer.lastSeekPosition, const Duration(seconds: 1));
expect(player.state.position, const Duration(seconds: 1));
});
test('a pending speed change cannot reclaim playback ownership', () async {
final coordinator = VoiceNotePlaybackCoordinator();
final firstBackend = _FakeAudioPlayerBackend()
..delaySpeedChanges = true
..resetPositionOnSpeedChange = true;
final secondBackend = _FakeAudioPlayerBackend();
final first = DeviceVoiceNotePlayerController(
coordinator: coordinator,
client: _SequencedHttpClient(),
requiresAuthenticatedLocalFile: false,
player: firstBackend,
);
final second = DeviceVoiceNotePlayerController(
coordinator: coordinator,
client: _SequencedHttpClient(),
requiresAuthenticatedLocalFile: false,
player: secondBackend,
);
addTearDown(first.dispose);
addTearDown(second.dispose);
await first.loadLocal(
'/tmp/first.m4a',
fallbackDuration: const Duration(seconds: 7),
);
await second.loadLocal(
'/tmp/second.m4a',
fallbackDuration: const Duration(seconds: 7),
);
await first.toggle();
firstBackend.positions.add(const Duration(seconds: 3));
await Future<void>.delayed(Duration.zero);
final speedChange = first.setSpeed(1.5);
await Future<void>.delayed(Duration.zero);
await second.toggle();
firstBackend.speedChanges.single.complete();
await speedChange;
await Future<void>.delayed(Duration.zero);
expect(firstBackend.playCount, 1);
expect(firstBackend.playing, isFalse);
expect(firstBackend.lastSeekPosition, isNull);
expect(secondBackend.playing, isTrue);
});
test(
'Android cancel interrupts pending preload, preserves loading capability, and retries',
() async {