Fix video letterboxing issue (#9339)

* give video a black background on web

* Video crop on web tweaks (#9371)

* Remove video embed crop option to reduce confusion

* Improve default thumb and border on web

---------

Co-authored-by: Eric Bailey <git@esb.lol>

src/components/Post/Embed/VideoEmbed/VideoEmbedInner/VideoEmbedInnerWeb.tsx

@@ -7,7 +7,6 @@ import type * as HlsTypes from 'hls.js'
 
 import {useNonReactiveCallback} from '#/lib/hooks/useNonReactiveCallback'
 import {atoms as a} from '#/alf'
-import {MediaInsetBorder} from '#/components/MediaInsetBorder'
 import * as BandwidthEstimate from './bandwidth-estimate'
 import {Controls} from './web-controls/VideoControls'
 
@@ -102,7 +101,6 @@ export function VideoEmbedInnerWeb({
           hasSubtitleTrack={hasSubtitleTrack}
         />
       </div>
-      <MediaInsetBorder />
     </View>
   )
 }

src/components/Post/Embed/VideoEmbed/index.tsx

@@ -7,7 +7,7 @@ import {useLingui} from '@lingui/react'
 
 import {ErrorBoundary} from '#/view/com/util/ErrorBoundary'
 import {ConstrainedImage} from '#/view/com/util/images/AutoSizedImage'
-import {atoms as a, useTheme} from '#/alf'
+import {atoms as a} from '#/alf'
 import {Button} from '#/components/Button'
 import {useThrottledValue} from '#/components/hooks/useThrottledValue'
 import {PlayButtonIcon} from '#/components/video/PlayButtonIcon'
@@ -16,11 +16,9 @@ import * as VideoFallback from './VideoEmbedInner/VideoFallback'
 
 interface Props {
   embed: AppBskyEmbedVideo.View
-  crop?: 'none' | 'square' | 'constrained'
 }
 
-export function VideoEmbed({embed, crop}: Props) {
+export function VideoEmbed({embed}: Props) {
-  const t = useTheme()
   const [key, setKey] = useState(0)
 
   const renderError = useCallback(
@@ -40,13 +38,10 @@ export function VideoEmbed({embed, crop}: Props) {
   }
 
   let constrained: number | undefined
-  let max: number | undefined
   if (aspectRatio !== undefined) {
     const ratio = 1 / 2 // max of 1:2 ratio in feeds
     constrained = Math.max(aspectRatio, ratio)
-    max = Math.max(aspectRatio, 0.25) // max of 1:4 in thread
   }
-  const cropDisabled = crop === 'none'
 
   const contents = (
     <ErrorBoundary renderError={renderError} key={key}>
@@ -56,28 +51,13 @@ export function VideoEmbed({embed, crop}: Props) {
 
   return (
     <View style={[a.pt_xs]}>
-      {cropDisabled ? (
+      <ConstrainedImage
-        <View
+        aspectRatio={constrained || 1}
-          style={[
+        // slightly smaller max height than images
-            a.w_full,
+        // images use 16 / 9, for reference
-            a.overflow_hidden,
+        minMobileAspectRatio={14 / 9}>
-            {aspectRatio: max ?? 1},
+        {contents}
-            a.rounded_md,
+      </ConstrainedImage>
-            a.overflow_hidden,
-            t.atoms.bg_contrast_25,
-          ]}>
-          {contents}
-        </View>
-      ) : (
-        <ConstrainedImage
-          fullBleed={crop === 'square'}
-          aspectRatio={constrained || 1}
-          // slightly smaller max height than images
-          // images use 16 / 9, for reference
-          minMobileAspectRatio={14 / 9}>
-          {contents}
-        </ConstrainedImage>
-      )}
     </View>
   )
 }

src/components/Post/Embed/VideoEmbed/index.web.tsx

@@ -17,6 +17,7 @@ import {ConstrainedImage} from '#/view/com/util/images/AutoSizedImage'
 import {atoms as a, useTheme} from '#/alf'
 import {useIsWithinMessage} from '#/components/dms/MessageContext'
 import {useFullscreen} from '#/components/hooks/useFullscreen'
+import {MediaInsetBorder} from '#/components/MediaInsetBorder'
 import {
   HLSUnsupportedError,
   VideoEmbedInnerWeb,
@@ -25,13 +26,7 @@ import {
 import {useActiveVideoWeb} from './ActiveVideoWebContext'
 import * as VideoFallback from './VideoEmbedInner/VideoFallback'
 
-export function VideoEmbed({
+export function VideoEmbed({embed}: {embed: AppBskyEmbedVideo.View}) {
-  embed,
-  crop,
-}: {
-  embed: AppBskyEmbedVideo.View
-  crop?: 'none' | 'square' | 'constrained'
-}) {
   const t = useTheme()
   const ref = useRef<HTMLDivElement>(null)
   const {active, setActive, sendPosition, currentActiveView} =
@@ -76,13 +71,10 @@ export function VideoEmbed({
   }
 
   let constrained: number | undefined
-  let max: number | undefined
   if (aspectRatio !== undefined) {
     const ratio = 1 / 2 // max of 1:2 ratio in feeds
     constrained = Math.max(aspectRatio, ratio)
-    max = Math.max(aspectRatio, 0.25) // max of 1:4 in thread
   }
-  const cropDisabled = crop === 'none'
 
   const contents = (
     <div
@@ -91,8 +83,11 @@ export function VideoEmbed({
         display: 'flex',
         flex: 1,
         cursor: 'default',
+        backgroundColor: t.palette.black,
         backgroundImage: `url(${embed.thumbnail})`,
-        backgroundSize: 'cover',
+        backgroundSize: 'contain',
+        backgroundPosition: 'center',
+        backgroundRepeat: 'no-repeat',
       }}
       onClick={evt => evt.stopPropagation()}>
       <ErrorBoundary renderError={renderError} key={key}>
@@ -114,28 +109,15 @@ export function VideoEmbed({
       <ViewportObserver
         sendPosition={sendPosition}
         isAnyViewActive={currentActiveView !== null}>
-        {cropDisabled ? (
+        <ConstrainedImage
-          <View
+          fullBleed
-            style={[
+          aspectRatio={constrained || 1}
-              a.w_full,
+          // slightly smaller max height than images
-              a.overflow_hidden,
+          // images use 16 / 9, for reference
-              {aspectRatio: max ?? 1},
+          minMobileAspectRatio={14 / 9}>
-              a.rounded_md,
+          {contents}
-              a.overflow_hidden,
+          <MediaInsetBorder />
-              t.atoms.bg_contrast_25,
+        </ConstrainedImage>
-            ]}>
-            {contents}
-          </View>
-        ) : (
-          <ConstrainedImage
-            fullBleed={crop === 'square'}
-            aspectRatio={constrained || 1}
-            // slightly smaller max height than images
-            // images use 16 / 9, for reference
-            minMobileAspectRatio={14 / 9}>
-            {contents}
-          </ConstrainedImage>
-        )}
       </ViewportObserver>
     </View>
   )

src/components/Post/Embed/index.tsx

@@ -112,7 +112,7 @@ function MediaEmbed({
         <ContentHider
           modui={rest.moderation?.ui('contentMedia')}
           activeStyle={[a.mt_sm]}>
-          <VideoEmbed embed={embed.view} crop="constrained" />
+          <VideoEmbed embed={embed.view} />
         </ContentHider>
       )
     }

src/view/com/util/images/AutoSizedImage.tsx

@@ -13,7 +13,7 @@ import {useLingui} from '@lingui/react'
 import {type Dimensions} from '#/lib/media/types'
 import {isNative} from '#/platform/detection'
 import {useLargeAltBadgeEnabled} from '#/state/preferences/large-alt-badge'
-import {atoms as a, useBreakpoints, useTheme} from '#/alf'
+import {atoms as a, useTheme} from '#/alf'
 import {ArrowsDiagonalOut_Stroke2_Corner0_Rounded as Fullscreen} from '#/components/icons/ArrowsDiagonal'
 import {MediaInsetBorder} from '#/components/MediaInsetBorder'
 import {Text} from '#/components/Typography'
@@ -30,18 +30,16 @@ export function ConstrainedImage({
   children: React.ReactNode
 }) {
   const t = useTheme()
-  const {gtMobile} = useBreakpoints()
   /**
    * Computed as a % value to apply as `paddingTop`, this basically controls
    * the height of the image.
    */
   const outerAspectRatio = React.useMemo<DimensionValue>(() => {
-    const ratio =
+    const ratio = isNative
-      isNative || !gtMobile
+      ? Math.min(1 / aspectRatio, minMobileAspectRatio ?? 16 / 9) // 9:16 bounding box
-        ? Math.min(1 / aspectRatio, minMobileAspectRatio ?? 16 / 9) // 9:16 bounding box
+      : Math.min(1 / aspectRatio, 1) // 1:1 bounding box
-        : Math.min(1 / aspectRatio, 1) // 1:1 bounding box
     return `${ratio * 100}%`
-  }, [aspectRatio, gtMobile, minMobileAspectRatio])
+  }, [aspectRatio, minMobileAspectRatio])
 
   return (
     <View style={[a.w_full]}>