[media] Use pixel aspect ratio to compute |natural_size| in all decod…

…ers.

This CL standardizes the |natural_size| computation in all decoder
implementations to use the decoded |visible_size| and the
VideoDecoderConfig's pixel aspect ratio.

Bug: 766657, 837337
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I01a5bd59c353d6f07fd02c9fc3d3e64ab1a04434
Reviewed-on: https://chromium-review.googlesource.com/1026992
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555136}

media/base/video_decoder_config.cc

@@ -10,6 +10,7 @@
 #include "media/base/media_util.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_types.h"
+#include "media/base/video_util.h"
 
 namespace media {
 
@@ -171,6 +172,10 @@ std::string VideoDecoderConfig::AsHumanReadableString() const {
   return s.str();
 }
 
+double VideoDecoderConfig::GetPixelAspectRatio() const {
+  return ::media::GetPixelAspectRatio(visible_rect_, natural_size_);
+}
+
 void VideoDecoderConfig::SetExtraData(const std::vector<uint8_t>& extra_data) {
   extra_data_ = extra_data;
 }

media/base/video_decoder_config.h

@@ -103,6 +103,11 @@ class MEDIA_EXPORT VideoDecoderConfig {
   // into account.
   const gfx::Size& natural_size() const { return natural_size_; }
 
+  // TODO(crbug.com/837337): This should be explicitly set (replacing
+  // |natural_size|). It should also be possible to determine whether it was set
+  // at all, since in-stream information may override it if it was not.
+  double GetPixelAspectRatio() const;
+
   // Optional byte data required to initialize video decoders, such as H.264
   // AVCC data.
   void SetExtraData(const std::vector<uint8_t>& extra_data);

media/base/video_util.cc

@@ -51,52 +51,43 @@ void FillRegionOutsideVisibleRect(uint8_t* data,
 
 }  // namespace
 
-gfx::Size GetNaturalSize(const gfx::Size& visible_size,
-                         int aspect_ratio_numerator,
-                         int aspect_ratio_denominator) {
-  if (aspect_ratio_denominator <= 0 || aspect_ratio_numerator <= 0) {
-    return gfx::Size();
-  }
+double GetPixelAspectRatio(const gfx::Rect& visible_rect,
+                           const gfx::Size& natural_size) {
+  double visible_width = visible_rect.width();
+  double visible_height = visible_rect.height();
+  double natural_width = natural_size.width();
+  double natural_height = natural_size.height();
+  return (visible_height * natural_width) / (visible_width * natural_height);
+}
 
-  double aspect_ratio = aspect_ratio_numerator /
-      static_cast<double>(aspect_ratio_denominator);
+gfx::Size GetNaturalSize(const gfx::Rect& visible_rect,
+                         double pixel_aspect_ratio) {
+  // TODO(sandersd): Also handle conversion back to integers overflowing.
+  if (!std::isfinite(pixel_aspect_ratio) || pixel_aspect_ratio <= 0.0)
+    return gfx::Size();
 
   // The HTML spec requires that we always grow a dimension to match aspect
   // ratio, rather than modify just the width:
   // github.com/whatwg/html/commit/2e94aa64fcf9adbd2f70d8c2aecd192c8678e298
-  if (aspect_ratio_numerator > aspect_ratio_denominator) {
-    return gfx::Size(round(visible_size.width() * aspect_ratio),
-                     visible_size.height());
+  if (pixel_aspect_ratio >= 1.0) {
+    return gfx::Size(std::round(visible_rect.width() * pixel_aspect_ratio),
+                     visible_rect.height());
   }
 
-  return gfx::Size(visible_size.width(),
-                   round(visible_size.height() / aspect_ratio));
+  return gfx::Size(visible_rect.width(),
+                   std::round(visible_rect.height() / pixel_aspect_ratio));
 }
 
-gfx::Size GetNaturalSizeWithDAR(const gfx::Size& visible_size,
-                                const gfx::Size& display_aspect) {
-  // No reasonable aspect interpretation, return an empty size.
-  // TODO(sandersd): Is it more useful to return the original |visible_size|?
-  if (visible_size.width() <= 0 || visible_size.height() <= 0 ||
-      display_aspect.width() <= 0 || display_aspect.height() <= 0) {
+gfx::Size GetNaturalSize(const gfx::Size& visible_size,
+                         int aspect_ratio_numerator,
+                         int aspect_ratio_denominator) {
+  if (aspect_ratio_denominator <= 0 || aspect_ratio_numerator <= 0)
     return gfx::Size();
-  }
 
-  double visible_aspect_ratio =
-      visible_size.width() / static_cast<double>(visible_size.height());
-
-  double display_aspect_ratio =
-      display_aspect.width() / static_cast<double>(display_aspect.height());
-
-  if (display_aspect_ratio > visible_aspect_ratio) {
-    // |display_aspect| is wider than |visible_size|; increase width.
-    return gfx::Size(round(visible_size.height() * display_aspect_ratio),
-                     visible_size.height());
-  }
+  double pixel_aspect_ratio =
+      aspect_ratio_numerator / static_cast<double>(aspect_ratio_denominator);
 
-  // |display_aspect| is narrower than |visible_size|; increase height.
-  return gfx::Size(visible_size.width(),
-                   round(visible_size.width() / display_aspect_ratio));
+  return GetNaturalSize(gfx::Rect(visible_size), pixel_aspect_ratio);
 }
 
 void FillYUV(VideoFrame* frame, uint8_t y, uint8_t u, uint8_t v) {

media/base/video_util.h

@@ -16,22 +16,38 @@ namespace media {
 
 class VideoFrame;
 
-// Computes the size of |visible_size| for a given sample aspect ratio.
+// Computes the pixel aspect ratio of a given |visible_rect| from its
+// |natural_size|. This describes the shape of a coded pixel as the ratio
+// of its width to its height.
 //
-// TODO(sandersd): Rename as GetNaturalSizeWithSAR() to make it more clear
-// at the call site what this does. Perhaps some wrapper classes would be
-// best:
-//   GetNaturalSize(visible_size, SampleAspectRatio(n, d))
-//   GetNaturalSize(visible_size, DisplayAspectRatio(w, h))
+// See https://en.wikipedia.org/wiki/Pixel_aspect_ratio for a detailed
+// definition.
+//
+// Returns NaN or Infinity if |visible_rect| or |natural_size| are empty.
+//
+// Note: Something has probably gone wrong if you need to call this function;
+// pixel aspect ratios should be the source of truth.
+//
+// TODO(crbug.com/837337): Decide how to encode 'not provided' for pixel aspect
+// ratios, and return that if one of the inputs is empty.
+MEDIA_EXPORT double GetPixelAspectRatio(const gfx::Rect& visible_rect,
+                                        const gfx::Size& natural_size);
+
+// Increases (at most) one of the dimensions of |visible_rect| to produce
+// a |natural_size| with the given pixel aspect ratio.
+//
+// Returns gfx::Size() if |pixel_aspect_ratio| is not finite and positive.
+MEDIA_EXPORT gfx::Size GetNaturalSize(const gfx::Rect& visible_rect,
+                                      double pixel_aspect_ratio);
+
+// Overload that takes the pixel aspect ratio as an integer fraction (and
+// |visible_size| instead of |visible_rect|).
+//
+// Returns gfx::Size() if numerator or denominator are not positive.
 MEDIA_EXPORT gfx::Size GetNaturalSize(const gfx::Size& visible_size,
                                       int aspect_ratio_numerator,
                                       int aspect_ratio_denominator);
 
-// Increases (at most) one of the dimensions of |visible_size| such that
-// the display aspect ratio matches |display_aspect|.
-MEDIA_EXPORT gfx::Size GetNaturalSizeWithDAR(const gfx::Size& visible_size,
-                                             const gfx::Size& display_aspect);
-
 // Fills |frame| containing YUV data to the given color values.
 MEDIA_EXPORT void FillYUV(VideoFrame* frame, uint8_t y, uint8_t u, uint8_t v);
 

media/base/video_util_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <stdint.h>
 
+#include <cmath>
 #include <memory>
 
 #include "base/macros.h"
@@ -190,7 +191,56 @@ class VideoUtilTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(VideoUtilTest);
 };
 
-TEST_F(VideoUtilTest, GetNaturalSize) {
+TEST_F(VideoUtilTest, GetPixelAspectRatio) {
+  gfx::Rect visible_rect(320, 240);
+
+  // Test empty or invalid combinations.
+  EXPECT_TRUE(std::isnan(GetPixelAspectRatio(gfx::Rect(), gfx::Size())));
+  EXPECT_TRUE(std::isnan(GetPixelAspectRatio(gfx::Rect(1, 1), gfx::Size())));
+  EXPECT_TRUE(std::isnan(GetPixelAspectRatio(gfx::Rect(), gfx::Size(1, 1))));
+  EXPECT_TRUE(
+      std::isinf(GetPixelAspectRatio(gfx::Rect(1, 1), gfx::Size(1, 0))));
+  EXPECT_EQ(0.0, GetPixelAspectRatio(gfx::Rect(1, 1), gfx::Size(0, 1)));
+  EXPECT_EQ(0.0, GetPixelAspectRatio(gfx::Rect(1, 0), gfx::Size(1, 1)));
+  EXPECT_TRUE(
+      std::isinf(GetPixelAspectRatio(gfx::Rect(0, 1), gfx::Size(1, 1))));
+
+  // Some normal ratios.
+  EXPECT_DOUBLE_EQ(1.0, GetPixelAspectRatio(visible_rect, gfx::Size(320, 240)));
+  EXPECT_DOUBLE_EQ(2.0, GetPixelAspectRatio(visible_rect, gfx::Size(640, 240)));
+  EXPECT_DOUBLE_EQ(0.5, GetPixelAspectRatio(visible_rect, gfx::Size(320, 480)));
+}
+
+TEST_F(VideoUtilTest, GetNaturalSize_Double) {
+  gfx::Rect visible_rect(320, 240);
+
+  // Test 0 sizes.
+  EXPECT_EQ(gfx::Size(0, 0), GetNaturalSize(gfx::Rect(0, 0), 1.0));
+  EXPECT_EQ(gfx::Size(0, 1), GetNaturalSize(gfx::Rect(0, 1), 1.0));
+  EXPECT_EQ(gfx::Size(1, 0), GetNaturalSize(gfx::Rect(1, 0), 1.0));
+
+  // Test abnormal ratios.
+  EXPECT_EQ(gfx::Size(), GetNaturalSize(visible_rect, NAN));
+  EXPECT_EQ(gfx::Size(), GetNaturalSize(visible_rect, 0.0));
+  EXPECT_EQ(gfx::Size(), GetNaturalSize(visible_rect, INFINITY));
+  EXPECT_EQ(gfx::Size(), GetNaturalSize(visible_rect, -INFINITY));
+  EXPECT_EQ(gfx::Size(), GetNaturalSize(visible_rect, -1.0));
+
+  // Test normal sizes and ratios.
+  EXPECT_EQ(gfx::Size(320, 240), GetNaturalSize(visible_rect, 1.0 / 1.0));
+  EXPECT_EQ(gfx::Size(640, 240), GetNaturalSize(visible_rect, 2.0 / 1.0));
+  EXPECT_EQ(gfx::Size(320, 480), GetNaturalSize(visible_rect, 1.0 / 2.0));
+  EXPECT_EQ(gfx::Size(427, 240), GetNaturalSize(visible_rect, 4.0 / 3.0));
+  EXPECT_EQ(gfx::Size(320, 320), GetNaturalSize(visible_rect, 3.0 / 4.0));
+  EXPECT_EQ(gfx::Size(569, 240), GetNaturalSize(visible_rect, 16.0 / 9.0));
+  EXPECT_EQ(gfx::Size(320, 427), GetNaturalSize(visible_rect, 9.0 / 16.0));
+
+  // Test some random ratios.
+  EXPECT_EQ(gfx::Size(495, 240), GetNaturalSize(visible_rect, 17.0 / 11.0));
+  EXPECT_EQ(gfx::Size(320, 371), GetNaturalSize(visible_rect, 11.0 / 17.0));
+}
+
+TEST_F(VideoUtilTest, GetNaturalSize_Fraction) {
   gfx::Size visible_size(320, 240);
 
   // Test 0 sizes.
@@ -199,11 +249,11 @@ TEST_F(VideoUtilTest, GetNaturalSize) {
   EXPECT_EQ(gfx::Size(1, 0), GetNaturalSize(gfx::Size(1, 0), 1, 1));
 
   // Test abnormal ratios.
-  EXPECT_EQ(gfx::Size(0, 0), GetNaturalSize(visible_size, 0, 0));
-  EXPECT_EQ(gfx::Size(0, 0), GetNaturalSize(visible_size, 0, 1));
-  EXPECT_EQ(gfx::Size(0, 0), GetNaturalSize(visible_size, 1, 0));
-  EXPECT_EQ(gfx::Size(0, 0), GetNaturalSize(visible_size, 1, -1));
-  EXPECT_EQ(gfx::Size(0, 0), GetNaturalSize(visible_size, -1, 1));
+  EXPECT_EQ(gfx::Size(), GetNaturalSize(visible_size, 0, 0));
+  EXPECT_EQ(gfx::Size(), GetNaturalSize(visible_size, 0, 1));
+  EXPECT_EQ(gfx::Size(), GetNaturalSize(visible_size, 1, 0));
+  EXPECT_EQ(gfx::Size(), GetNaturalSize(visible_size, 1, -1));
+  EXPECT_EQ(gfx::Size(), GetNaturalSize(visible_size, -1, 1));
 
   // Test normal sizes and ratios.
   EXPECT_EQ(gfx::Size(320, 240), GetNaturalSize(visible_size, 1, 1));
@@ -219,38 +269,6 @@ TEST_F(VideoUtilTest, GetNaturalSize) {
   EXPECT_EQ(gfx::Size(320, 371), GetNaturalSize(visible_size, 11, 17));
 }
 
-TEST_F(VideoUtilTest, GetNaturalSizeWithDAR) {
-  gfx::Size visible_size(320, 240);
-
-  // Test 0 sizes.
-  EXPECT_EQ(gfx::Size(), GetNaturalSizeWithDAR(gfx::Size(0, 0), visible_size));
-  EXPECT_EQ(gfx::Size(), GetNaturalSizeWithDAR(gfx::Size(0, 1), visible_size));
-  EXPECT_EQ(gfx::Size(), GetNaturalSizeWithDAR(gfx::Size(1, 0), visible_size));
-
-  // Test abnormal ratios.
-  EXPECT_EQ(gfx::Size(), GetNaturalSizeWithDAR(visible_size, gfx::Size(0, 0)));
-  EXPECT_EQ(gfx::Size(), GetNaturalSizeWithDAR(visible_size, gfx::Size(0, 1)));
-  EXPECT_EQ(gfx::Size(), GetNaturalSizeWithDAR(visible_size, gfx::Size(1, 0)));
-  EXPECT_EQ(gfx::Size(), GetNaturalSizeWithDAR(visible_size, gfx::Size(1, -1)));
-  EXPECT_EQ(gfx::Size(), GetNaturalSizeWithDAR(visible_size, gfx::Size(-1, 1)));
-
-  // Test normal sizes and ratios.
-  EXPECT_EQ(gfx::Size(320, 320),
-            GetNaturalSizeWithDAR(visible_size, gfx::Size(1, 1)));
-  EXPECT_EQ(gfx::Size(480, 240),
-            GetNaturalSizeWithDAR(visible_size, gfx::Size(2, 1)));
-  EXPECT_EQ(gfx::Size(320, 640),
-            GetNaturalSizeWithDAR(visible_size, gfx::Size(1, 2)));
-  EXPECT_EQ(gfx::Size(320, 240),
-            GetNaturalSizeWithDAR(visible_size, gfx::Size(4, 3)));
-  EXPECT_EQ(gfx::Size(320, 427),
-            GetNaturalSizeWithDAR(visible_size, gfx::Size(3, 4)));
-  EXPECT_EQ(gfx::Size(427, 240),
-            GetNaturalSizeWithDAR(visible_size, gfx::Size(16, 9)));
-  EXPECT_EQ(gfx::Size(320, 569),
-            GetNaturalSizeWithDAR(visible_size, gfx::Size(9, 16)));
-}
-
 namespace {
 
 uint8_t src6x4[] = {0,  1,  2,  3,  4,  5,  6,  7,  8,  9,  10, 11,

media/cdm/cdm_adapter.cc

@@ -32,6 +32,7 @@
 #include "media/base/video_decoder_config.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_types.h"
+#include "media/base/video_util.h"
 #include "media/cdm/cdm_auxiliary_helper.h"
 #include "media/cdm/cdm_helpers.h"
 #include "media/cdm/cdm_wrapper.h"
@@ -810,7 +811,7 @@ void CdmAdapter::InitializeVideoDecoder(const VideoDecoderConfig& config,
     return;
   }
 
-  natural_size_ = config.natural_size();
+  pixel_aspect_ratio_ = config.GetPixelAspectRatio();
 
   if (status == cdm::kDeferredInitialization) {
     DVLOG(1) << "Deferred initialization in " << __func__;
@@ -878,8 +879,9 @@ void CdmAdapter::DecryptAndDecodeVideo(scoped_refptr<DecoderBuffer> encrypted,
     return;
   }
 
-  scoped_refptr<VideoFrame> decoded_frame =
-      video_frame->TransformToVideoFrame(natural_size_);
+  gfx::Rect visible_rect(video_frame->Size().width, video_frame->Size().height);
+  scoped_refptr<VideoFrame> decoded_frame = video_frame->TransformToVideoFrame(
+      GetNaturalSize(visible_rect, pixel_aspect_ratio_));
   if (!decoded_frame) {
     DLOG(ERROR) << __func__ << ": TransformToVideoFrame failed.";
     video_decode_cb.Run(Decryptor::kError, nullptr);
@@ -905,7 +907,7 @@ void CdmAdapter::DeinitializeDecoder(StreamType stream_type) {
       audio_channel_layout_ = CHANNEL_LAYOUT_NONE;
       break;
     case Decryptor::kVideo:
-      natural_size_ = gfx::Size();
+      pixel_aspect_ratio_ = 0.0;
       break;
   }
 }

media/cdm/cdm_adapter.h

@@ -249,9 +249,8 @@ class MEDIA_EXPORT CdmAdapter : public ContentDecryptionModule,
   int audio_samples_per_second_ = 0;
   ChannelLayout audio_channel_layout_ = CHANNEL_LAYOUT_NONE;
 
-  // Keep track of video frame natural size from the latest configuration
-  // as the CDM doesn't provide it.
-  gfx::Size natural_size_;
+  // Keep track of aspect ratio from the latest configuration.
+  double pixel_aspect_ratio_ = 0.0;
 
   // Tracks whether an output protection query and a positive query result (no
   // unprotected external link) have been reported to UMA.

media/filters/aom_video_decoder.cc

@@ -14,6 +14,7 @@
 #include "media/base/decoder_buffer.h"
 #include "media/base/media_log.h"
 #include "media/base/media_switches.h"
+#include "media/base/video_util.h"
 #include "third_party/libyuv/include/libyuv/convert.h"
 
 // Include libaom header files.
@@ -358,9 +359,11 @@ scoped_refptr<VideoFrame> AomVideoDecoder::CopyImageToVideoFrame(
   }
 
   // Since we're making a copy, only copy the visible area.
-  const gfx::Size size(img->d_w, img->d_h);
-  auto frame = frame_pool_.CreateFrame(pixel_format, size, gfx::Rect(size),
-                                       config_.natural_size(), kNoTimestamp);
+  const gfx::Rect visible_rect(img->d_w, img->d_h);
+  auto frame = frame_pool_.CreateFrame(
+      pixel_format, visible_rect.size(), visible_rect,
+      GetNaturalSize(visible_rect, config_.GetPixelAspectRatio()),
+      kNoTimestamp);
   if (!frame)
     return nullptr;
 
@@ -380,8 +383,8 @@ scoped_refptr<VideoFrame> AomVideoDecoder::CopyImageToVideoFrame(
                      frame->visible_data(VideoFrame::kUPlane),
                      frame->stride(VideoFrame::kUPlane),
                      frame->visible_data(VideoFrame::kVPlane),
-                     frame->stride(VideoFrame::kVPlane), size.width(),
-                     size.height());
+                     frame->stride(VideoFrame::kVPlane), visible_rect.width(),
+                     visible_rect.height());
   }
 
   return frame;

media/filters/ffmpeg_video_decoder.cc

@@ -149,7 +149,8 @@ int FFmpegVideoDecoder::GetVideoBuffer(struct AVCodecContext* codec_context,
                                   codec_context->sample_aspect_ratio.num,
                                   codec_context->sample_aspect_ratio.den);
   } else {
-    natural_size = config_.natural_size();
+    natural_size =
+        GetNaturalSize(gfx::Rect(size), config_.GetPixelAspectRatio());
   }
 
   // FFmpeg has specific requirements on the allocation size of the frame.  The

media/filters/gpu_video_decoder.cc

@@ -653,7 +653,9 @@ void GpuVideoDecoder::PictureReady(const media::Picture& picture) {
           &GpuVideoDecoder::ReleaseMailbox, weak_factory_.GetWeakPtr(),
           factories_, picture.picture_buffer_id(), pb.client_texture_ids())),
       pb.size(), visible_rect,
-      GetNaturalSizeWithDAR(visible_rect.size(), natural_size), timestamp));
+      GetNaturalSize(visible_rect,
+                     GetPixelAspectRatio(visible_rect, natural_size)),
+      timestamp));
   if (!frame) {
     DLOG(ERROR) << "Create frame failed for: " << picture.picture_buffer_id();
     NotifyError(VideoDecodeAccelerator::PLATFORM_FAILURE);