Merge pull request #5871 from matrix-org/travis/voice/countdown

Limit voice recording length

src/components/views/elements/Field.tsx

@@ -262,7 +262,7 @@ export default class Field extends React.PureComponent<PropShapes, IState> {
                 tooltipClassName={classNames("mx_Field_tooltip", tooltipClassName)}
                 visible={(this.state.focused && this.props.forceTooltipVisible) || this.state.feedbackVisible}
                 label={tooltipContent || this.state.feedback}
-                forceOnRight
+                alignment={Tooltip.Alignment.Right}
             />;
         }
 

src/components/views/elements/InfoTooltip.tsx

@@ -18,8 +18,8 @@ limitations under the License.
 import React from 'react';
 import classNames from 'classnames';
 
-import Tooltip from './Tooltip';
-import { _t } from "../../../languageHandler";
+import Tooltip, {Alignment} from './Tooltip';
+import {_t} from "../../../languageHandler";
 import {replaceableComponent} from "../../../utils/replaceableComponent";
 
 interface ITooltipProps {
@@ -61,7 +61,7 @@ export default class InfoTooltip extends React.PureComponent<ITooltipProps, ISta
             className="mx_InfoTooltip_container"
             tooltipClassName={classNames("mx_InfoTooltip_tooltip", tooltipClassName)}
             label={tooltip || title}
-            forceOnRight={true}
+            alignment={Alignment.Right}
         /> : <div />;
         return (
             <div onMouseOver={this.onMouseOver} onMouseLeave={this.onMouseLeave} className="mx_InfoTooltip">

src/components/views/elements/Tooltip.tsx

@@ -25,6 +25,14 @@ import {replaceableComponent} from "../../../utils/replaceableComponent";
 
 const MIN_TOOLTIP_HEIGHT = 25;
 
+export enum Alignment {
+    Natural, // Pick left or right
+    Left,
+    Right,
+    Top, // Centered
+    Bottom, // Centered
+}
+
 interface IProps {
         // Class applied to the element used to position the tooltip
         className?: string;
@@ -36,7 +44,7 @@ interface IProps {
         visible?: boolean;
         // the react element to put into the tooltip
         label: React.ReactNode;
-        forceOnRight?: boolean;
+        alignment?: Alignment; // defaults to Natural
         yOffset?: number;
 }
 
@@ -46,10 +54,14 @@ export default class Tooltip extends React.Component<IProps> {
     private tooltip: void | Element | Component<Element, any, any>;
     private parent: Element;
 
+    // XXX: This is because some components (Field) are unable to `import` the Tooltip class,
+    // so we expose the Alignment options off of us statically.
+    public static readonly Alignment = Alignment;
 
     public static readonly defaultProps = {
         visible: true,
         yOffset: 0,
+        alignment: Alignment.Natural,
     };
 
     // Create a wrapper for the tooltip outside the parent and attach it to the body element
@@ -86,11 +98,35 @@ export default class Tooltip extends React.Component<IProps> {
             offset = Math.floor(parentBox.height - MIN_TOOLTIP_HEIGHT);
         }
 
-        style.top = (parentBox.top - 2 + this.props.yOffset) + window.pageYOffset + offset;
-        if (!this.props.forceOnRight && parentBox.right > window.innerWidth / 2) {
-            style.right = window.innerWidth - parentBox.right - window.pageXOffset - 16;
-        } else {
-            style.left = parentBox.right + window.pageXOffset + 6;
+        const baseTop = (parentBox.top - 2 + this.props.yOffset) + window.pageYOffset;
+        const top = baseTop + offset;
+        const right = window.innerWidth - parentBox.right - window.pageXOffset - 16;
+        const left = parentBox.right + window.pageXOffset + 6;
+        const horizontalCenter = parentBox.right - window.pageXOffset - (parentBox.width / 2);
+        switch (this.props.alignment) {
+            case Alignment.Natural:
+                if (parentBox.right > window.innerWidth / 2) {
+                    style.right = right;
+                    style.top = top;
+                    break;
+                }
+                // fall through to Right
+            case Alignment.Right:
+                style.left = left;
+                style.top = top;
+                break;
+            case Alignment.Left:
+                style.right = right;
+                style.top = top;
+                break;
+            case Alignment.Top:
+                style.top = baseTop - 16;
+                style.left = horizontalCenter;
+                break;
+            case Alignment.Bottom:
+                style.top = baseTop + parentBox.height;
+                style.left = horizontalCenter;
+                break;
         }
 
         return style;

src/components/views/rooms/MessageComposer.js

@@ -33,6 +33,8 @@ import {UPDATE_EVENT} from "../../../stores/AsyncStore";
 import {replaceableComponent} from "../../../utils/replaceableComponent";
 import VoiceRecordComposerTile from "./VoiceRecordComposerTile";
 import {VoiceRecordingStore} from "../../../stores/VoiceRecordingStore";
+import {RecordingState} from "../../../voice/VoiceRecording";
+import Tooltip, {Alignment} from "../elements/Tooltip";
 
 function ComposerAvatar(props) {
     const MemberStatusMessageAvatar = sdk.getComponent('avatars.MemberStatusMessageAvatar');
@@ -185,6 +187,7 @@ export default class MessageComposer extends React.Component {
             canSendMessages: this.props.room.maySendMessage(),
             isComposerEmpty: true,
             haveRecording: false,
+            recordingTimeLeftSeconds: null, // when set to a number, shows a toast
         };
     }
 
@@ -315,7 +318,17 @@ export default class MessageComposer extends React.Component {
     }
 
     _onVoiceStoreUpdate = () => {
-        this.setState({haveRecording: !!VoiceRecordingStore.instance.activeRecording});
+        const recording = VoiceRecordingStore.instance.activeRecording;
+        this.setState({haveRecording: !!recording});
+        if (recording) {
+            // We show a little heads up that the recording is about to automatically end soon. The 3s
+            // display time is completely arbitrary. Note that we don't need to deregister the listener
+            // because the recording instance will clean that up for us.
+            recording.on(RecordingState.EndingSoon, ({secondsLeft}) => {
+                this.setState({recordingTimeLeftSeconds: secondsLeft});
+                setTimeout(() => this.setState({recordingTimeLeftSeconds: null}), 3000);
+            });
+        }
     };
 
     render() {
@@ -396,8 +409,18 @@ export default class MessageComposer extends React.Component {
             );
         }
 
+        let recordingTooltip;
+        const secondsLeft = Math.round(this.state.recordingTimeLeftSeconds);
+        if (secondsLeft) {
+            recordingTooltip = <Tooltip
+                label={_t("%(seconds)ss left", {seconds: secondsLeft})}
+                alignment={Alignment.Top} yOffset={-50}
+            />;
+        }
+
         return (
             <div className="mx_MessageComposer mx_GroupLayout">
+                {recordingTooltip}
                 <div className="mx_MessageComposer_wrapper">
                     <ReplyPreview permalinkCreator={this.props.permalinkCreator} />
                     <div className="mx_MessageComposer_row">

src/i18n/strings/en_EN.json

@@ -1474,6 +1474,7 @@
     "The conversation continues here.": "The conversation continues here.",
     "This room has been replaced and is no longer active.": "This room has been replaced and is no longer active.",
     "You do not have permission to post to this room": "You do not have permission to post to this room",
+    "%(seconds)ss left": "%(seconds)ss left",
     "Bold": "Bold",
     "Italics": "Italics",
     "Strikethrough": "Strikethrough",

src/stores/VoiceRecordingStore.ts

@@ -73,9 +73,7 @@ export class VoiceRecordingStore extends AsyncStoreWithClient<IState> {
      */
     public disposeRecording(): Promise<void> {
         if (this.state.recording) {
-            // Stop for good measure, but completely async because we're not concerned with this
-            // passing or failing.
-            this.state.recording.stop().catch(e => console.error("Error stopping recording", e));
+            this.state.recording.destroy(); // stops internally
         }
         return this.updateState({recording: null});
     }

src/utils/Singleflight.ts

@@ -0,0 +1,126 @@
+/*
+Copyright 2021 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import {EnhancedMap} from "./maps";
+
+// Inspired by https://pkg.go.dev/golang.org/x/sync/singleflight
+
+const keyMap = new EnhancedMap<Object, EnhancedMap<string, unknown>>();
+
+/**
+ * Access class to get a singleflight context. Singleflights execute a
+ * function exactly once, unless instructed to forget about a result.
+ *
+ * Typically this is used to de-duplicate an action, such as a save button
+ * being pressed, without having to track state internally for an operation
+ * already being in progress. This doesn't expose a flag which can be used
+ * to disable a button, however it would be capable of returning a Promise
+ * from the first call.
+ *
+ * The result of the function call is cached indefinitely, just in case a
+ * second call comes through late. There are various functions named "forget"
+ * to have the cache be cleared of a result.
+ *
+ * Singleflights in our usecase are tied to an instance of something, combined
+ * with a string key to differentiate between multiple possible actions. This
+ * means that a "save" key will be scoped to the instance which defined it and
+ * not leak between other instances. This is done to avoid having to concatenate
+ * variables to strings to essentially namespace the field, for most cases.
+ */
+export class Singleflight {
+    private constructor() {
+    }
+
+    /**
+     * A void marker to help with returning a value in a singleflight context.
+     * If your code doesn't return anything, return this instead.
+     */
+    public static Void = Symbol("void");
+
+    /**
+     * Acquire a singleflight context.
+     * @param {Object} instance An instance to associate the context with. Can be any object.
+     * @param {string} key A string key relevant to that instance to namespace under.
+     * @returns {SingleflightContext} Returns the context to execute the function.
+     */
+    public static for(instance: Object, key: string): SingleflightContext {
+        if (!instance || !key) throw new Error("An instance and key must be supplied");
+        return new SingleflightContext(instance, key);
+    }
+
+    /**
+     * Forgets all results for a given instance.
+     * @param {Object} instance The instance to forget about.
+     */
+    public static forgetAllFor(instance: Object) {
+        keyMap.delete(instance);
+    }
+
+    /**
+     * Forgets all cached results for all instances. Intended for use by tests.
+     */
+    public static forgetAll() {
+        for (const k of keyMap.keys()) {
+            keyMap.remove(k);
+        }
+    }
+}
+
+class SingleflightContext {
+    public constructor(private instance: Object, private key: string) {
+    }
+
+    /**
+     * Forget this particular instance and key combination, discarding the result.
+     */
+    public forget() {
+        const map = keyMap.get(this.instance);
+        if (!map) return;
+        map.remove(this.key);
+        if (!map.size) keyMap.remove(this.instance);
+    }
+
+    /**
+     * Execute a function. If a result is already known, that will be returned instead
+     * of executing the provided function. However, if no result is known then the function
+     * will be called, with its return value cached. The function must return a value
+     * other than `undefined` - take a look at Singleflight.Void if you don't have a return
+     * to make.
+     *
+     * Note that this technically allows the caller to provide a different function each time:
+     * this is largely considered a bad idea and should not be done. Singleflights work off the
+     * premise that something needs to happen once, so duplicate executions will be ignored.
+     *
+     * For ideal performance and behaviour, functions which return promises are preferred. If
+     * a function is not returning a promise, it should return as soon as possible to avoid a
+     * second call potentially racing it. The promise returned by this function will be that
+     * of the first execution of the function, even on duplicate calls.
+     * @param {Function} fn The function to execute.
+     * @returns The recorded value.
+     */
+    public do<T>(fn: () => T): T {
+        const map = keyMap.getOrCreate(this.instance, new EnhancedMap<string, unknown>());
+
+        // We have to manually getOrCreate() because we need to execute the fn
+        let val = <T>map.get(this.key);
+        if (val === undefined) {
+            val = fn();
+            map.set(this.key, val);
+        }
+
+        return val;
+    }
+}

src/voice/VoiceRecording.ts

@@ -20,17 +20,30 @@ import {MatrixClient} from "matrix-js-sdk/src/client";
 import CallMediaHandler from "../CallMediaHandler";
 import {SimpleObservable} from "matrix-widget-api";
 import {clamp} from "../utils/numbers";
+import EventEmitter from "events";
+import {IDestroyable} from "../utils/IDestroyable";
+import {Singleflight} from "../utils/Singleflight";
 
 const CHANNELS = 1; // stereo isn't important
 const SAMPLE_RATE = 48000; // 48khz is what WebRTC uses. 12khz is where we lose quality.
 const BITRATE = 24000; // 24kbps is pretty high quality for our use case in opus.
+const TARGET_MAX_LENGTH = 120; // 2 minutes in seconds. Somewhat arbitrary, though longer == larger files.
+const TARGET_WARN_TIME_LEFT = 10; // 10 seconds, also somewhat arbitrary.
 
 export interface IRecordingUpdate {
     waveform: number[]; // floating points between 0 (low) and 1 (high).
     timeSeconds: number; // float
 }
 
-export class VoiceRecording {
+export enum RecordingState {
+    Started = "started",
+    EndingSoon = "ending_soon", // emits an object with a single numerical value: secondsLeft
+    Ended = "ended",
+    Uploading = "uploading",
+    Uploaded = "uploaded",
+}
+
+export class VoiceRecording extends EventEmitter implements IDestroyable {
     private recorder: Recorder;
     private recorderContext: AudioContext;
     private recorderSource: MediaStreamAudioSourceNode;
@@ -43,6 +56,7 @@ export class VoiceRecording {
     private observable: SimpleObservable<IRecordingUpdate>;
 
     public constructor(private client: MatrixClient) {
+        super();
     }
 
     private async makeRecorder() {
@@ -124,7 +138,7 @@ export class VoiceRecording {
         return this.mxc;
     }
 
-    private tryUpdateLiveData = (ev: AudioProcessingEvent) => {
+    private processAudioUpdate = (ev: AudioProcessingEvent) => {
         if (!this.recording) return;
 
         // The time domain is the input to the FFT, which means we use an array of the same
@@ -150,6 +164,19 @@ export class VoiceRecording {
             waveform: translatedData,
             timeSeconds: ev.playbackTime,
         });
+
+        // Now that we've updated the data/waveform, let's do a time check. We don't want to
+        // go horribly over the limit. We also emit a warning state if needed.
+        const secondsLeft = TARGET_MAX_LENGTH - ev.playbackTime;
+        if (secondsLeft <= 0) {
+            // noinspection JSIgnoredPromiseFromCall - we aren't concerned with it overlapping
+            this.stop();
+        } else if (secondsLeft <= TARGET_WARN_TIME_LEFT) {
+            Singleflight.for(this, "ending_soon").do(() => {
+                this.emit(RecordingState.EndingSoon, {secondsLeft});
+                return Singleflight.Void;
+            });
+        }
     };
 
     public async start(): Promise<void> {
@@ -164,12 +191,14 @@ export class VoiceRecording {
         }
         this.observable = new SimpleObservable<IRecordingUpdate>();
         await this.makeRecorder();
-        this.recorderProcessor.addEventListener("audioprocess", this.tryUpdateLiveData);
+        this.recorderProcessor.addEventListener("audioprocess", this.processAudioUpdate);
         await this.recorder.start();
         this.recording = true;
+        this.emit(RecordingState.Started);
     }
 
     public async stop(): Promise<Uint8Array> {
+        return Singleflight.for(this, "stop").do(async () => {
             if (!this.recording) {
                 throw new Error("No recording to stop");
             }
@@ -187,10 +216,19 @@ export class VoiceRecording {
 
             // Finally do our post-processing and clean up
             this.recording = false;
-        this.recorderProcessor.removeEventListener("audioprocess", this.tryUpdateLiveData);
+            this.recorderProcessor.removeEventListener("audioprocess", this.processAudioUpdate);
             await this.recorder.close();
+            this.emit(RecordingState.Ended);
 
             return this.buffer;
+        });
+    }
+
+    public destroy() {
+        // noinspection JSIgnoredPromiseFromCall - not concerned about stop() being called async here
+        this.stop();
+        this.removeAllListeners();
+        Singleflight.forgetAllFor(this);
     }
 
     public async upload(): Promise<string> {
@@ -200,11 +238,13 @@ export class VoiceRecording {
 
         if (this.mxc) return this.mxc;
 
+        this.emit(RecordingState.Uploading);
         this.mxc = await this.client.uploadContent(new Blob([this.buffer], {
             type: "audio/ogg",
         }), {
             onlyContentUri: false, // to stop the warnings in the console
         }).then(r => r['content_uri']);
+        this.emit(RecordingState.Uploaded);
         return this.mxc;
     }
 }

test/Singleflight-test.ts

@@ -0,0 +1,115 @@
+/*
+Copyright 2021 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import {Singleflight} from "../src/utils/Singleflight";
+
+describe('Singleflight', () => {
+    afterEach(() => {
+        Singleflight.forgetAll();
+    });
+
+    it('should throw for bad context variables', () => {
+        const permutations: [Object, string][] = [
+            [null, null],
+            [{}, null],
+            [null, "test"],
+        ];
+        for (const p of permutations) {
+            try {
+                Singleflight.for(p[0], p[1]);
+                // noinspection ExceptionCaughtLocallyJS
+                throw new Error("failed to fail: " + JSON.stringify(p));
+            } catch (e) {
+                expect(e.message).toBe("An instance and key must be supplied");
+            }
+        }
+    });
+
+    it('should execute the function once', () => {
+        const instance = {};
+        const key = "test";
+        const val = {}; // unique object for reference check
+        const fn = jest.fn().mockReturnValue(val);
+        const sf = Singleflight.for(instance, key);
+        const r1 = sf.do(fn);
+        expect(r1).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+        const r2 = sf.do(fn);
+        expect(r2).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+    });
+
+    it('should execute the function once, even with new contexts', () => {
+        const instance = {};
+        const key = "test";
+        const val = {}; // unique object for reference check
+        const fn = jest.fn().mockReturnValue(val);
+        let sf = Singleflight.for(instance, key);
+        const r1 = sf.do(fn);
+        expect(r1).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+        sf = Singleflight.for(instance, key); // RESET FOR TEST
+        const r2 = sf.do(fn);
+        expect(r2).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+    });
+
+    it('should execute the function twice if the result was forgotten', () => {
+        const instance = {};
+        const key = "test";
+        const val = {}; // unique object for reference check
+        const fn = jest.fn().mockReturnValue(val);
+        const sf = Singleflight.for(instance, key);
+        const r1 = sf.do(fn);
+        expect(r1).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+        sf.forget();
+        const r2 = sf.do(fn);
+        expect(r2).toBe(val);
+        expect(fn.mock.calls.length).toBe(2);
+    });
+
+    it('should execute the function twice if the instance was forgotten', () => {
+        const instance = {};
+        const key = "test";
+        const val = {}; // unique object for reference check
+        const fn = jest.fn().mockReturnValue(val);
+        const sf = Singleflight.for(instance, key);
+        const r1 = sf.do(fn);
+        expect(r1).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+        Singleflight.forgetAllFor(instance);
+        const r2 = sf.do(fn);
+        expect(r2).toBe(val);
+        expect(fn.mock.calls.length).toBe(2);
+    });
+
+    it('should execute the function twice if everything was forgotten', () => {
+        const instance = {};
+        const key = "test";
+        const val = {}; // unique object for reference check
+        const fn = jest.fn().mockReturnValue(val);
+        const sf = Singleflight.for(instance, key);
+        const r1 = sf.do(fn);
+        expect(r1).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+        Singleflight.forgetAll();
+        const r2 = sf.do(fn);
+        expect(r2).toBe(val);
+        expect(fn.mock.calls.length).toBe(2);
+    });
+});
+