From a9f99d93bebbcf94b71a517be917fb78b48616e4 Mon Sep 17 00:00:00 2001
From: Tamius Han <tamius.han@gmail.com>
Date: Sat, 19 Dec 2020 03:18:14 +0100
Subject: [PATCH] Fix aspect ratio calculations on height-compensated videos,
 episode 1

---
 src/ext/lib/video-transform/Scaler.js    | 13 +++++++++--
 src/ext/lib/video-transform/Stretcher.js | 28 ++++++++++++------------
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/src/ext/lib/video-transform/Scaler.js b/src/ext/lib/video-transform/Scaler.js
index 0181a36..1da4125 100644
--- a/src/ext/lib/video-transform/Scaler.js
+++ b/src/ext/lib/video-transform/Scaler.js
@@ -71,7 +71,13 @@ class Scaler {
      *
      * Video width is normalized based on 100% of the parent. That means if the player AR 
      * is narrower than video ar, we need to pre-downscale the video. This scaling already
-     * undoes any zoom that style="height:123%" on the video element adds.
+     * undoes any zoom that style="height:123%" on the video element adds. 
+     * 
+     * There are few exceptions and additional caveatss:
+     *   * AspectRatio.FitHeight: we don't want to pre-downscale the video at all, as things
+     *     will be scaled to fit height as-is.
+     *   * When player is wider than stream, we want to undo any height compensations site
+     *     tacks on the video tag.
      * 
      * Quick notes:
      *   * when I say 'video AR', I actually mean aspect ratio after we've compensated for
@@ -81,11 +87,14 @@ class Scaler {
      */
     const streamAr = this.conf.video.videoWidth / this.conf.video.videoHeight;
     const playerAr = this.conf.player.dimensions.width / this.conf.player.dimensions.height;
-    const compensatedStreamAr = streamAr * this.conf.getHeightCompensationFactor();
+    const heightCompensationFactor = this.conf.getHeightCompensationFactor();
+    const compensatedStreamAr = streamAr * heightCompensationFactor;
 
     let arCorrectionFactor = 1;
     if (playerAr < compensatedStreamAr) {
       arCorrectionFactor = this.conf.player.dimensions.width / this.conf.video.offsetWidth;
+    } else if (ar.type !== AspectRatio.Reset) {
+      arCorrectionFactor /= heightCompensationFactor;
     }
 
     if(!this.conf.video){
diff --git a/src/ext/lib/video-transform/Stretcher.js b/src/ext/lib/video-transform/Stretcher.js
index 5ba6a72..24d649d 100644
--- a/src/ext/lib/video-transform/Stretcher.js
+++ b/src/ext/lib/video-transform/Stretcher.js
@@ -138,15 +138,14 @@ squeezeFactor:          ${squeezeFactor}`, '\nvideo', this.conf.video);
     const playerAr = this.conf.player.dimensions.width / this.conf.player.dimensions.height;
 
     let arCorrectionFactor = 1;
-    if (playerAr < streamAr) {
-      arCorrectionFactor = this.conf.player.dimensions.width / this.conf.video.offsetWidth;
-    }
+    arCorrectionFactor = this.conf.player.dimensions.width / this.conf.video.offsetWidth;
+
     return arCorrectionFactor;
   }
 
-  calculateStretch(actualAr, playerArOverride) {
+   calculateStretch(actualAr, playerArOverride) {
     const playerAr = playerArOverride || this.conf.player.dimensions.width / this.conf.player.dimensions.height;
-    const videoAr = this.conf.video.videoWidth / this.conf.video.videoHeight;
+    const streamAr = this.conf.video.videoWidth / this.conf.video.videoHeight;
 
     if (! actualAr){
       actualAr = playerAr;
@@ -157,7 +156,7 @@ squeezeFactor:          ${squeezeFactor}`, '\nvideo', this.conf.video);
       yFactor: 1
     };
 
-    if (playerAr >= videoAr){
+    if (playerAr >= streamAr){
       // player adds PILLARBOX
 
       if(actualAr >= playerAr){
@@ -166,18 +165,18 @@ squeezeFactor:          ${squeezeFactor}`, '\nvideo', this.conf.video);
         // actual > player > video  — video is letterboxed
         // solution: horizontal stretch according to difference between video and player AR
         //           vertical stretch according to difference between actual AR and player AR
-        stretchFactors.xFactor = playerAr / videoAr;
-        stretchFactors.yFactor = actualAr / videoAr;
+        stretchFactors.xFactor = playerAr / streamAr;
+        stretchFactors.yFactor = actualAr / streamAr;
 
         this.logger.log('info', 'stretcher', "[Stretcher.js::calculateStretch] stretching strategy 1")
-      } else if ( actualAr >= videoAr) {
+      } else if ( actualAr >= streamAr) {
         // VERIFIED WORKS
 
         // player > actual > video — video is still letterboxed
         // we need vertical stretch to remove black bars in video
         // we need horizontal stretch to make video fit width
-        stretchFactors.xFactor = playerAr / videoAr;
-        stretchFactors.yFactor = actualAr / videoAr;
+        stretchFactors.xFactor = playerAr / streamAr;
+        stretchFactors.yFactor = actualAr / streamAr;
 
         this.logger.log('info', 'stretcher', "[Stretcher.js::calculateStretch] stretching strategy 2")
       } else {
@@ -197,10 +196,10 @@ squeezeFactor:          ${squeezeFactor}`, '\nvideo', this.conf.video);
         // video > player > actual
         // video is PILLARBOXED
         stretchFactors.xFactor = actualAr / playerAr;
-        stretchFactors.yFactor = videoAr / playerAr;
+        stretchFactors.yFactor = streamAr / playerAr;
 
         this.logger.log('info', 'stretcher', "[Stretcher.js::calculateStretch] stretching strategy 4")
-      } else if ( actualAr < videoAr ) {
+      } else if ( actualAr < streamAr ) {
         // NEEDS CHECKING 
 
         // video > actual > player
@@ -222,10 +221,11 @@ squeezeFactor:          ${squeezeFactor}`, '\nvideo', this.conf.video);
       }
     }
 
-    // correct factors
     const arCorrectionFactor = this.getArCorrectionFactor();
+    // correct factors, unless we're trying to reset
     stretchFactors.xFactor *= arCorrectionFactor;
     stretchFactors.yFactor *= arCorrectionFactor;
+    stretchFactors.arCorrectionFactor = this.getArCorrectionFactor();
 
     return stretchFactors;
   }