[FFmpeg-devel] [PATCH v2] avcodec/vc1: correct aspect ratio calculation

[Jerome Borsboom]

According to VC-1 spec:
* Display size defaults to max coded size when not explicitly set in
sequence header
* Aspect ratio in the sequence header refers to the display size elements.
Therefore, the aspect ratio for the coded samples (SAR) needs to take into
account the scaling from coded size to display size, and the aspect ratio
of the display size elements.

Signed-off-by: Jerome Borsboom <jerome.borsboom at carpalis.nl>
---
 libavcodec/vc1.c       | 42 +++++++++++++++++++++++++-----------------
 libavcodec/vc1.h       |  2 ++
 tests/ref/fate/vc1-ism |  2 +-
 3 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/libavcodec/vc1.c b/libavcodec/vc1.c
index 3581d87b57..51ad665f4b 100644
--- a/libavcodec/vc1.c
+++ b/libavcodec/vc1.c
@@ -442,30 +442,28 @@ static int decode_sequence_header_adv(VC1Context *v, GetBitContext *gb)
     }
     v->s.max_b_frames = v->s.avctx->max_b_frames = 7;
     if (get_bits1(gb)) { //Display Info - decoding is not affected by it
-        int w, h, ar = 0;
+        int ar = 0;
         av_log(v->s.avctx, AV_LOG_DEBUG, "Display extended info:\n");
-        w = get_bits(gb, 14) + 1;
-        h = get_bits(gb, 14) + 1;
-        av_log(v->s.avctx, AV_LOG_DEBUG, "Display dimensions: %ix%i\n", w, h);
+        v->disp_horiz_size = get_bits(gb, 14) + 1;
+        v->disp_vert_size = get_bits(gb, 14) + 1;
+        av_log(v->s.avctx, AV_LOG_DEBUG, "Display dimensions: %ix%i\n",
+               v->disp_horiz_size, v->disp_vert_size);
         if (get_bits1(gb))
             ar = get_bits(gb, 4);
         if (ar && ar < 14) {
-            v->s.avctx->sample_aspect_ratio = ff_vc1_pixel_aspect[ar];
+            v->aspect_ratio = ff_vc1_pixel_aspect[ar];
         } else if (ar == 15) {
-            w = get_bits(gb, 8) + 1;
-            h = get_bits(gb, 8) + 1;
-            v->s.avctx->sample_aspect_ratio = (AVRational){w, h};
+            v->aspect_ratio.num = get_bits(gb, 8) + 1;
+            v->aspect_ratio.den = get_bits(gb, 8) + 1;
+            av_reduce(&v->aspect_ratio.num, &v->aspect_ratio.den,
+                      v->aspect_ratio.num, v->aspect_ratio.den,
+                      256);
         } else {
-            av_reduce(&v->s.avctx->sample_aspect_ratio.num,
-                      &v->s.avctx->sample_aspect_ratio.den,
-                      v->s.avctx->height * w,
-                      v->s.avctx->width * h,
-                      1 << 30);
+            v->aspect_ratio = (AVRational){1, 1};
         }
-        ff_set_sar(v->s.avctx, v->s.avctx->sample_aspect_ratio);
-        av_log(v->s.avctx, AV_LOG_DEBUG, "Aspect: %i:%i\n",
-               v->s.avctx->sample_aspect_ratio.num,
-               v->s.avctx->sample_aspect_ratio.den);
+        av_log(v->s.avctx, AV_LOG_DEBUG, "Aspect ratio: %i:%i\n",
+               v->aspect_ratio.num,
+               v->aspect_ratio.den);
 
         if (get_bits1(gb)) { //framerate stuff
             if (get_bits1(gb)) {
@@ -490,6 +488,10 @@ static int decode_sequence_header_adv(VC1Context *v, GetBitContext *gb)
             v->transfer_char = get_bits(gb, 8);
             v->matrix_coef   = get_bits(gb, 8);
         }
+    } else {
+        v->disp_horiz_size = v->max_coded_width;
+        v->disp_vert_size = v->max_coded_height;
+        v->aspect_ratio = (AVRational){1, 1};
     }
 
     v->hrd_param_flag = get_bits1(gb);
@@ -544,6 +546,12 @@ int ff_vc1_decode_entry_point(AVCodecContext *avctx, VC1Context *v, GetBitContex
         av_log(avctx, AV_LOG_ERROR, "Failed to set dimensions %d %d\n", w, h);
         return ret;
     }
+    av_reduce(&avctx->sample_aspect_ratio.num,
+              &avctx->sample_aspect_ratio.den,
+              v->disp_horiz_size * v->aspect_ratio.num * h,
+              v->disp_vert_size * v->aspect_ratio.den * w,
+              1 << 30);
+    ff_set_sar(avctx, avctx->sample_aspect_ratio);
 
     if (v->extended_mv)
         v->extended_dmv = get_bits1(gb);
diff --git a/libavcodec/vc1.h b/libavcodec/vc1.h
index 69f6ca9e4d..7674b0f9a1 100644
--- a/libavcodec/vc1.h
+++ b/libavcodec/vc1.h
@@ -209,6 +209,8 @@ typedef struct VC1Context{
     int hrd_param_flag;   ///< Presence of Hypothetical Reference
                           ///< Decoder parameters
     int psf;              ///< Progressive Segmented Frame
+    int disp_horiz_size, disp_vert_size;
+    AVRational aspect_ratio;
     //@}
 
     /** Sequence header data for all Profiles
diff --git a/tests/ref/fate/vc1-ism b/tests/ref/fate/vc1-ism
index 1bd6c643d9..4f460f6754 100644
--- a/tests/ref/fate/vc1-ism
+++ b/tests/ref/fate/vc1-ism
@@ -2,7 +2,7 @@
 #media_type 0: video
 #codec_id 0: rawvideo
 #dimensions 0: 240x104
-#sar 0: 156/156
+#sar 0: 13/30
 0,          0,          0,        1,    37440, 0xd1bc5235
 0,          2,          2,        1,    37440, 0x158e6167
 0,          3,          3,        1,    37440, 0x0faa4481
-- 
2.13.6