Skip to content

Commit 82f1b77

Browse files
floriandurosHalf-Shot
authored andcommitted
Prevent voice message from displaying spurious errors (#30736)
* fix: avoid to render `AudioPlayerViewModel` when `MAudioBody` is inherited * fix: avoid `Playback.prepare` to fail when called twice * fix: add `decoding` to playback type * refactor: fix circular deps * refactor: extract `MockedPlayback` from `AudioPlayerViewModel` * test: add `MAudioBody` basic test * test: add tests for `MVoiceMessageBody` * fix: lint
1 parent d408097 commit 82f1b77

File tree

9 files changed

+219
-54
lines changed

9 files changed

+219
-54
lines changed

src/audio/Playback.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { DEFAULT_WAVEFORM, PLAYBACK_WAVEFORM_SAMPLES } from "./consts";
2020
import { PlaybackEncoder } from "../PlaybackEncoder";
2121

2222
export enum PlaybackState {
23+
Preparing = "preparing", // preparing to decode
2324
Decoding = "decoding",
2425
Stopped = "stopped", // no progress on timeline
2526
Paused = "paused", // some progress on timeline
@@ -146,6 +147,8 @@ export class Playback extends EventEmitter implements IDestroyable, PlaybackInte
146147
return;
147148
}
148149

150+
this.state = PlaybackState.Preparing;
151+
149152
// The point where we use an audio element is fairly arbitrary, though we don't want
150153
// it to be too low. As of writing, voice messages want to show a waveform but audio
151154
// messages do not. Using an audio element means we can't show a waveform preview, so

src/components/views/audio_messages/RecordingPlayback.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ export default class RecordingPlayback extends AudioPlayerBase<IProps> {
7474
}
7575

7676
return (
77-
<div className="mx_MediaBody mx_VoiceMessagePrimaryContainer" onKeyDown={this.onKeyDown}>
77+
<div
78+
className="mx_MediaBody mx_VoiceMessagePrimaryContainer"
79+
onKeyDown={this.onKeyDown}
80+
data-testid="recording-playback"
81+
>
7882
<PlayPauseButton
7983
playback={this.props.playback}
8084
playbackPhase={this.state.playbackPhase}

src/components/views/messages/MAudioBody.tsx

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
66
Please see LICENSE files in the repository root for full details.
77
*/
88

9-
import React from "react";
9+
import React, { type JSX, useEffect, useMemo } from "react";
1010
import { logger } from "matrix-js-sdk/src/logger";
1111
import { type IContent } from "matrix-js-sdk/src/matrix";
1212
import { type MediaEventContent } from "matrix-js-sdk/src/types";
@@ -17,8 +17,6 @@ import { _t } from "../../../languageHandler";
1717
import MFileBody from "./MFileBody";
1818
import { type IBodyProps } from "./IBodyProps";
1919
import { PlaybackManager } from "../../../audio/PlaybackManager";
20-
import { isVoiceMessage } from "../../../utils/EventUtils";
21-
import { PlaybackQueue } from "../../../audio/PlaybackQueue";
2220
import RoomContext, { TimelineRenderingType } from "../../../contexts/RoomContext";
2321
import MediaProcessingError from "./shared/MediaProcessingError";
2422
import { AudioPlayerViewModel } from "../../../viewmodels/audio/AudioPlayerViewModel";
@@ -27,7 +25,6 @@ import { AudioPlayerView } from "../../../shared-components/audio/AudioPlayerVie
2725
interface IState {
2826
error?: boolean;
2927
playback?: Playback;
30-
audioPlayerVm?: AudioPlayerViewModel;
3128
}
3229

3330
export default class MAudioBody extends React.PureComponent<IBodyProps, IState> {
@@ -38,7 +35,6 @@ export default class MAudioBody extends React.PureComponent<IBodyProps, IState>
3835

3936
public async componentDidMount(): Promise<void> {
4037
let buffer: ArrayBuffer;
41-
4238
try {
4339
try {
4440
const blob = await this.props.mediaEventHelper!.sourceBlob.value;
@@ -63,18 +59,16 @@ export default class MAudioBody extends React.PureComponent<IBodyProps, IState>
6359
// We should have a buffer to work with now: let's set it up
6460
const playback = PlaybackManager.instance.createPlaybackInstance(buffer, waveform);
6561
playback.clockInfo.populatePlaceholdersFrom(this.props.mxEvent);
66-
this.setState({ playback, audioPlayerVm: new AudioPlayerViewModel({ playback, mediaName: content.body }) });
67-
68-
if (isVoiceMessage(this.props.mxEvent)) {
69-
PlaybackQueue.forRoom(this.props.mxEvent.getRoomId()!).unsortedEnqueue(this.props.mxEvent, playback);
70-
}
62+
this.setState({ playback });
7163

72-
// Note: the components later on will handle preparing the Playback class for us.
64+
this.onMount(playback);
65+
// Note: the components later on will handle preparing the Playback class for us
7366
}
7467

68+
protected onMount(playback: Playback): void {}
69+
7570
public componentWillUnmount(): void {
7671
this.state.playback?.destroy();
77-
this.state.audioPlayerVm?.dispose();
7872
}
7973

8074
protected get showFileBody(): boolean {
@@ -116,9 +110,35 @@ export default class MAudioBody extends React.PureComponent<IBodyProps, IState>
116110
// At this point we should have a playable state
117111
return (
118112
<span className="mx_MAudioBody">
119-
{this.state.audioPlayerVm && <AudioPlayerView vm={this.state.audioPlayerVm} />}
113+
<AudioPlayer playback={this.state.playback} mediaName={this.props.mxEvent.getContent().body} />
120114
{this.showFileBody && <MFileBody {...this.props} showGenericPlaceholder={false} />}
121115
</span>
122116
);
123117
}
124118
}
119+
120+
interface AudioPlayerProps {
121+
/**
122+
* The playback instance to control audio playback.
123+
*/
124+
playback: Playback;
125+
/**
126+
* The name of the media being played
127+
*/
128+
mediaName: string;
129+
}
130+
131+
/**
132+
* AudioPlayer component that initializes the AudioPlayerViewModel and renders the AudioPlayerView.
133+
*/
134+
function AudioPlayer({ playback, mediaName }: AudioPlayerProps): JSX.Element {
135+
const vm = useMemo(() => new AudioPlayerViewModel({ playback, mediaName }), [playback, mediaName]);
136+
137+
useEffect(() => {
138+
return () => {
139+
vm.dispose();
140+
};
141+
}, [vm]);
142+
143+
return <AudioPlayerView vm={vm} />;
144+
}

src/components/views/messages/MVoiceMessageBody.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,17 @@ import RecordingPlayback from "../audio_messages/RecordingPlayback";
1414
import MAudioBody from "./MAudioBody";
1515
import MFileBody from "./MFileBody";
1616
import MediaProcessingError from "./shared/MediaProcessingError";
17+
import { isVoiceMessage } from "../../../utils/EventUtils";
18+
import { PlaybackQueue } from "../../../audio/PlaybackQueue";
19+
import { type Playback } from "../../../audio/Playback";
1720

1821
export default class MVoiceMessageBody extends MAudioBody {
22+
protected onMount(playback: Playback): void {
23+
if (isVoiceMessage(this.props.mxEvent)) {
24+
PlaybackQueue.forRoom(this.props.mxEvent.getRoomId()!).unsortedEnqueue(this.props.mxEvent, playback);
25+
}
26+
}
27+
1928
// A voice message is an audio file but rendered in a special way.
2029
public render(): React.ReactNode {
2130
if (this.state.error) {

src/shared-components/audio/playback.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@
77

88
/**
99
* Represents the possible states of playback.
10+
* - "preparing": The audio is being prepared for playback (e.g., loading or buffering).
1011
* - "decoding": The audio is being decoded and is not ready for playback.
1112
* - "stopped": The playback has been stopped, with no progress on the timeline.
1213
* - "paused": The playback is paused, with some progress on the timeline.
1314
* - "playing": The playback is actively progressing through the timeline.
1415
*/
15-
export type PlaybackState = "decoding" | "stopped" | "paused" | "playing";
16+
export type PlaybackState = "decoding" | "stopped" | "paused" | "playing" | "preparing";
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*
2+
* Copyright 2025 New Vector Ltd.
3+
*
4+
* SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
5+
* Please see LICENSE files in the repository root for full details.
6+
*/
7+
8+
import EventEmitter from "events";
9+
import { SimpleObservable } from "matrix-widget-api";
10+
11+
import { PlaybackState } from "../../../src/audio/Playback";
12+
13+
/**
14+
* A mocked playback implementation for testing purposes.
15+
* It simulates a playback with a fixed size and allows state changes.
16+
*/
17+
export class MockedPlayback extends EventEmitter {
18+
public sizeBytes = 8000;
19+
private waveformObservable = new SimpleObservable<number[]>();
20+
public liveData = new SimpleObservable<number[]>();
21+
22+
public constructor(
23+
public currentState: PlaybackState,
24+
public durationSeconds: number,
25+
public timeSeconds: number,
26+
) {
27+
super();
28+
}
29+
30+
public setState(state: PlaybackState): void {
31+
this.currentState = state;
32+
this.emit("update", state);
33+
}
34+
35+
public get isPlaying(): boolean {
36+
return this.currentState === PlaybackState.Playing;
37+
}
38+
39+
public get clockInfo() {
40+
return {
41+
liveData: this.liveData,
42+
populatePlaceholdersFrom: () => undefined,
43+
};
44+
}
45+
46+
public get waveform(): number[] {
47+
return [];
48+
}
49+
50+
public get waveformData(): SimpleObservable<number[]> {
51+
return this.waveformObservable;
52+
}
53+
54+
public prepare = jest.fn().mockResolvedValue(undefined);
55+
public skipTo = jest.fn();
56+
public toggle = jest.fn();
57+
public destroy = jest.fn().mockResolvedValue(undefined);
58+
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*
2+
* Copyright 2025 New Vector Ltd.
3+
*
4+
* SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
5+
* Please see LICENSE files in the repository root for full details.
6+
*/
7+
8+
import React from "react";
9+
import { EventType, MatrixEvent } from "matrix-js-sdk/src/matrix";
10+
import { render, screen, act } from "jest-matrix-react";
11+
12+
import { MockedPlayback } from "../../../audio/MockedPlayback";
13+
import { type Playback, PlaybackState } from "../../../../../src/audio/Playback";
14+
import MAudioBody from "../../../../../src/components/views/messages/MAudioBody";
15+
import { PlaybackManager } from "../../../../../src/audio/PlaybackManager";
16+
import { type MediaEventHelper } from "../../../../../src/utils/MediaEventHelper";
17+
18+
describe("<MAudioBody />", () => {
19+
let event: MatrixEvent;
20+
beforeEach(() => {
21+
const playback = new MockedPlayback(PlaybackState.Decoding, 50, 10) as unknown as Playback;
22+
jest.spyOn(PlaybackManager.instance, "createPlaybackInstance").mockReturnValue(playback);
23+
24+
event = new MatrixEvent({
25+
room_id: "!room:server",
26+
sender: "@alice.example.org",
27+
type: EventType.RoomMessage,
28+
content: {
29+
body: "audio name ",
30+
msgtype: "m.audio",
31+
url: "mxc://server/audio",
32+
},
33+
});
34+
});
35+
36+
it("should render", async () => {
37+
const mediaEventHelper = {
38+
sourceBlob: {
39+
value: {
40+
arrayBuffer: () => new ArrayBuffer(8),
41+
},
42+
},
43+
} as unknown as MediaEventHelper;
44+
45+
await act(() => render(<MAudioBody mxEvent={event} mediaEventHelper={mediaEventHelper} />));
46+
expect(await screen.findByRole("region", { name: "Audio player" })).toBeInTheDocument();
47+
});
48+
});
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*
2+
* Copyright 2025 New Vector Ltd.
3+
*
4+
* SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
5+
* Please see LICENSE files in the repository root for full details.
6+
*/
7+
8+
import { EventType, MatrixEvent, Room } from "matrix-js-sdk/src/matrix";
9+
import { act, render, screen } from "jest-matrix-react";
10+
import React from "react";
11+
12+
import { MockedPlayback } from "../../../audio/MockedPlayback";
13+
import { type Playback, PlaybackState } from "../../../../../src/audio/Playback";
14+
import { PlaybackManager } from "../../../../../src/audio/PlaybackManager";
15+
import type { MediaEventHelper } from "../../../../../src/utils/MediaEventHelper";
16+
import MVoiceMessageBody from "../../../../../src/components/views/messages/MVoiceMessageBody";
17+
import { PlaybackQueue } from "../../../../../src/audio/PlaybackQueue";
18+
import { createTestClient } from "../../../../test-utils";
19+
20+
describe("<MVvoiceMessageBody />", () => {
21+
let event: MatrixEvent;
22+
beforeEach(() => {
23+
const playback = new MockedPlayback(PlaybackState.Decoding, 50, 10) as unknown as Playback;
24+
jest.spyOn(PlaybackManager.instance, "createPlaybackInstance").mockReturnValue(playback);
25+
26+
const matrixClient = createTestClient();
27+
const room = new Room("!TESTROOM", matrixClient, "@alice:example.org");
28+
const playbackQueue = new PlaybackQueue(room);
29+
30+
jest.spyOn(PlaybackQueue, "forRoom").mockReturnValue(playbackQueue);
31+
jest.spyOn(playbackQueue, "unsortedEnqueue").mockReturnValue(undefined);
32+
33+
event = new MatrixEvent({
34+
room_id: "!room:server",
35+
sender: "@alice.example.org",
36+
type: EventType.RoomMessage,
37+
content: {
38+
"body": "audio name ",
39+
"msgtype": "m.audio",
40+
"url": "mxc://server/audio",
41+
"org.matrix.msc3946.voice": true,
42+
},
43+
});
44+
});
45+
46+
it("should render", async () => {
47+
const mediaEventHelper = {
48+
sourceBlob: {
49+
value: {
50+
arrayBuffer: () => new ArrayBuffer(8),
51+
},
52+
},
53+
} as unknown as MediaEventHelper;
54+
55+
await act(() => render(<MVoiceMessageBody mxEvent={event} mediaEventHelper={mediaEventHelper} />));
56+
expect(await screen.findByTestId("recording-playback")).toBeInTheDocument();
57+
});
58+
});

test/viewmodels/audio/AudioPlayerViewModel-test.tsx

Lines changed: 3 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,17 @@
55
* Please see LICENSE files in the repository root for full details.
66
*/
77

8-
import EventEmitter from "events";
9-
import { SimpleObservable } from "matrix-widget-api";
108
import { type ChangeEvent, type KeyboardEvent as ReactKeyboardEvent } from "react";
119
import { waitFor } from "@testing-library/dom";
1210

1311
import { type Playback, PlaybackState } from "../../../src/audio/Playback";
1412
import { AudioPlayerViewModel } from "../../../src/viewmodels/audio/AudioPlayerViewModel";
13+
import { MockedPlayback } from "../../unit-tests/audio/MockedPlayback";
1514

1615
describe("AudioPlayerViewModel", () => {
17-
let playback: MockedPlayback & Playback;
16+
let playback: Playback;
1817
beforeEach(() => {
19-
playback = new MockedPlayback(PlaybackState.Decoding, 50, 10) as unknown as MockedPlayback & Playback;
18+
playback = new MockedPlayback(PlaybackState.Decoding, 50, 10) as unknown as Playback;
2019
});
2120

2221
it("should return the snapshot", () => {
@@ -66,38 +65,3 @@ describe("AudioPlayerViewModel", () => {
6665
expect(playback.skipTo).toHaveBeenCalledWith(10 + 5); // 5 seconds forward
6766
});
6867
});
69-
70-
/**
71-
* A mocked playback implementation for testing purposes.
72-
* It simulates a playback with a fixed size and allows state changes.
73-
*/
74-
class MockedPlayback extends EventEmitter {
75-
public sizeBytes = 8000;
76-
77-
public constructor(
78-
public currentState: PlaybackState,
79-
public durationSeconds: number,
80-
public timeSeconds: number,
81-
) {
82-
super();
83-
}
84-
85-
public setState(state: PlaybackState): void {
86-
this.currentState = state;
87-
this.emit("update", state);
88-
}
89-
90-
public get isPlaying(): boolean {
91-
return this.currentState === PlaybackState.Playing;
92-
}
93-
94-
public get clockInfo() {
95-
return {
96-
liveData: new SimpleObservable(),
97-
};
98-
}
99-
100-
public prepare = jest.fn().mockResolvedValue(undefined);
101-
public skipTo = jest.fn();
102-
public toggle = jest.fn();
103-
}

0 commit comments

Comments
 (0)