Make video sizing consistent with images

res/css/views/messages/_MVideoBody.scss

@@ -16,8 +16,6 @@ limitations under the License.
 
 span.mx_MVideoBody {
     video.mx_MVideoBody {
-        max-width: 100%;
-        height: auto;
         border-radius: $timeline-image-border-radius;
     }
 }

src/components/views/messages/MImageBody.tsx

@@ -377,28 +377,13 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
             infoHeight = this.state.loadedImageDimensions.naturalHeight;
         }
 
-        // The maximum size of the thumbnail as it is rendered as an <img>
-        // check for any height constraints
-        const imageSize = SettingsStore.getValue("Images.size") as ImageSize;
-        const isPortrait = infoWidth < infoHeight;
-        const suggestedAndPossibleWidth = Math.min(suggestedImageSize(imageSize, isPortrait).w, infoWidth);
-        const suggestedAndPossibleHeight = Math.min(suggestedImageSize(imageSize, isPortrait).h, infoHeight);
-        const aspectRatio = infoWidth / infoHeight;
-
-        let maxWidth: number;
-        let maxHeight: number;
-        const maxHeightConstraint = forcedHeight || this.props.maxImageHeight || suggestedAndPossibleHeight;
-        if (maxHeightConstraint * aspectRatio < suggestedAndPossibleWidth || imageSize === ImageSize.Large) {
-            // The width is dictated by the maximum height that was defined by the props or the function param `forcedHeight`
-            // If the thumbnail size is set to Large, we always let the size be dictated by the height.
-            maxWidth = maxHeightConstraint * aspectRatio;
-            // there is no need to check for infoHeight here since this is done with `maxHeightConstraint * aspectRatio < suggestedAndPossibleWidth`
-            maxHeight = maxHeightConstraint;
-        } else {
-            // height is dictated by suggestedWidth (based on the Image.size setting)
-            maxWidth = suggestedAndPossibleWidth;
-            maxHeight = suggestedAndPossibleWidth / aspectRatio;
-        }
+        // The maximum size of the thumbnail as it is rendered as an <img>,
+        // accounting for any height constraints
+        const { w: maxWidth, h: maxHeight } = suggestedImageSize(
+            SettingsStore.getValue("Images.size") as ImageSize,
+            { w: infoWidth, h: infoHeight },
+            forcedHeight ?? this.props.maxImageHeight,
+        );
 
         let img = null;
         let placeholder = null;

src/components/views/messages/MVideoBody.tsx

@@ -62,38 +62,6 @@ export default class MVideoBody extends React.PureComponent<IBodyProps, IState>
         };
     }
 
-    private suggestedDimensions(isPortrait): { w: number, h: number } {
-        return suggestedVideoSize(SettingsStore.getValue("Images.size") as ImageSize);
-    }
-
-    private thumbScale(
-        fullWidth: number,
-        fullHeight: number,
-        thumbWidth?: number,
-        thumbHeight?: number,
-    ): number {
-        if (!fullWidth || !fullHeight) {
-            // Cannot calculate thumbnail height for image: missing w/h in metadata. We can't even
-            // log this because it's spammy
-            return undefined;
-        }
-
-        if (!thumbWidth || !thumbHeight) {
-            const dims = this.suggestedDimensions(fullWidth < fullHeight);
-            thumbWidth = dims.w;
-            thumbHeight = dims.h;
-        }
-
-        if (fullWidth < thumbWidth && fullHeight < thumbHeight) {
-            // no scaling needs to be applied
-            return 1;
-        }
-
-        // always scale the videos based on their width.
-        const widthMulti = thumbWidth / fullWidth;
-        return widthMulti;
-    }
-
     private getContentUrl(): string|null {
         const content = this.props.mxEvent.getContent<IMediaEventContent>();
         // During export, the content url will point to the MSC, which will later point to a local url
@@ -135,13 +103,10 @@ export default class MVideoBody extends React.PureComponent<IBodyProps, IState>
 
         const canvas = document.createElement("canvas");
 
-        let width = info.w;
-        let height = info.h;
-        const scale = this.thumbScale(info.w, info.h);
-        if (scale) {
-            width = Math.floor(info.w * scale);
-            height = Math.floor(info.h * scale);
-        }
+        const { w: width, h: height } = suggestedVideoSize(
+            SettingsStore.getValue("Images.size") as ImageSize,
+            { w: info.w, h: info.h },
+        );
 
         canvas.width = width;
         canvas.height = height;
@@ -277,24 +242,18 @@ export default class MVideoBody extends React.PureComponent<IBodyProps, IState>
             );
         }
 
+        const { w: maxWidth, h: maxHeight } = suggestedVideoSize(
+            SettingsStore.getValue("Images.size") as ImageSize,
+            { w: content.info?.w, h: content.info?.h },
+        );
+
         const contentUrl = this.getContentUrl();
         const thumbUrl = this.getThumbUrl();
-        const defaultDims = this.suggestedDimensions(false);
-        let height = defaultDims.h;
-        let width = defaultDims.w;
         let poster = null;
         let preload = "metadata";
-        if (content.info) {
-            const scale = this.thumbScale(content.info.w, content.info.h);
-            if (scale) {
-                width = Math.floor(content.info.w * scale);
-                height = Math.floor(content.info.h * scale);
-            }
-
-            if (thumbUrl) {
-                poster = thumbUrl;
-                preload = "none";
-            }
+        if (content.info && thumbUrl) {
+            poster = thumbUrl;
+            preload = "none";
         }
 
         const fileBody = this.getFileBody();
@@ -309,8 +268,7 @@ export default class MVideoBody extends React.PureComponent<IBodyProps, IState>
                     preload={preload}
                     muted={autoplay}
                     autoPlay={autoplay}
-                    height={height}
-                    width={width}
+                    style={{ maxHeight, maxWidth }}
                     poster={poster}
                     onPlay={this.videoOnPlay}
                 />

src/settings/enums/ImageSize.ts

@@ -15,24 +15,46 @@ limitations under the License.
 */
 
 // For Large the image gets drawn as big as possible.
-// constraint by: timeline width, manual heigh overrides, SIZE_LARGE.h
+// constraint by: timeline width, manual height overrides, SIZE_LARGE.h
 const SIZE_LARGE = { w: 800, h: 600 };
-
 // For Normal the image gets drawn to never exceed SIZE_NORMAL.w, SIZE_NORMAL.h
-// constraint by: timeline width, manual heigh overrides
+// constraint by: timeline width, manual height overrides
 const SIZE_NORMAL_LANDSCAPE = { w: 324, h: 324 }; // for w > h
 const SIZE_NORMAL_PORTRAIT = { w: 324 * (9/16), h: 324 }; // for h > w
+
+type Dimensions = { w: number, h: number };
+
 export enum ImageSize {
     Normal = "normal",
     Large = "large",
 }
 
-export function suggestedSize(size: ImageSize, portrait = false): { w: number, h: number} {
-    switch (size) {
-        case ImageSize.Large:
-            return SIZE_LARGE;
-        case ImageSize.Normal:
-        default:
-            return portrait ? SIZE_NORMAL_PORTRAIT : SIZE_NORMAL_LANDSCAPE;
+/**
+ * @param {ImageSize} size The user's image size preference
+ * @param {Dimensions} contentSize The natural dimensions of the content
+ * @param {number} maxHeight Overrides the default height limit
+ * @returns {Dimensions} The suggested maximum dimensions for the image
+ */
+export function suggestedSize(size: ImageSize, contentSize: Dimensions, maxHeight?: number): Dimensions {
+    const aspectRatio = contentSize.w / contentSize.h;
+    const portrait = aspectRatio < 1;
+
+    const maxSize = (size === ImageSize.Large) ? SIZE_LARGE :
+        portrait ? SIZE_NORMAL_PORTRAIT : SIZE_NORMAL_LANDSCAPE;
+    if (!contentSize.w || !contentSize.h) {
+        return maxSize;
+    }
+
+    const constrainedSize = {
+        w: Math.min(maxSize.w, contentSize.w),
+        h: maxHeight ? Math.min(maxSize.h, contentSize.h, maxHeight) : Math.min(maxSize.h, contentSize.h),
+    };
+
+    if (constrainedSize.h * aspectRatio < constrainedSize.w) {
+        // Height dictates width
+        return { w: constrainedSize.h * aspectRatio, h: constrainedSize.h };
+    } else {
+        // Width dictates height
+        return { w: constrainedSize.w, h: constrainedSize.w / aspectRatio };
     }
 }

test/settings/enums/ImageSize-test.ts

@@ -0,0 +1,38 @@
+/*
+Copyright 2022 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 { ImageSize, suggestedSize } from "../../../src/settings/enums/ImageSize";
+
+describe("ImageSize", () => {
+    describe("suggestedSize", () => {
+        it("constrains width", () => {
+            const size = suggestedSize(ImageSize.Normal, { w: 648, h: 162 });
+            expect(size).toStrictEqual({ w: 324, h: 81 });
+        });
+        it("constrains height", () => {
+            const size = suggestedSize(ImageSize.Normal, { w: 162, h: 648 });
+            expect(size).toStrictEqual({ w: 81, h: 324 });
+        });
+        it("constrains width in large mode", () => {
+            const size = suggestedSize(ImageSize.Large, { w: 2400, h: 1200 });
+            expect(size).toStrictEqual({ w: 800, h: 400 });
+        });
+        it("returns max values if content size is not specified", () => {
+            const size = suggestedSize(ImageSize.Normal, { w: null, h: null });
+            expect(size).toStrictEqual({ w: 324, h: 324 });
+        });
+    });
+});