From dd13b8a2ae1d7a883958aecd964b90fae0fec263 Mon Sep 17 00:00:00 2001
From: Dale Curtis <dalecurtis@chromium.org>
Date: Fri, 29 Aug 2025 20:13:53 -0700
Subject: [PATCH] Use input visible size when calling CopyPlane()

The output coded size may be adjusted which causes the row_bytes()
and rows() methods on VideoFrame to return values which may be
outside the range of source.

Fixed: 441917796
Change-Id: I8bb6c7e70060717b9b28688a6424e0b032fb99ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6900663
Commit-Queue: Eugene Zemtsov <eugene@chromium.org>
Reviewed-by: Eugene Zemtsov <eugene@chromium.org>
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1508722}
---

diff --git a/media/base/video_frame.cc b/media/base/video_frame.cc
index 8fb80072..ac36168 100644
--- a/media/base/video_frame.cc
+++ b/media/base/video_frame.cc
@@ -1270,20 +1270,26 @@
 
 // static
 void VideoFrame::UpdateHashWithFrameForTesting(crypto::hash::Hasher& hasher,
-                                               const VideoFrame& frame) {
+                                               const VideoFrame& frame,
+                                               bool visible_data_only) {
   for (size_t plane = 0; plane < NumPlanes(frame.format()); ++plane) {
-    for (int row = 0; row < frame.rows(plane); ++row) {
-      hasher.Update(frame.data_[plane].subspan(
-          base::checked_cast<size_t>(frame.stride(plane) * row),
-          base::checked_cast<size_t>(frame.row_bytes(plane))));
+    const size_t rows = base::checked_cast<size_t>(
+        visible_data_only ? frame.GetVisibleRows(plane) : frame.rows(plane));
+    const size_t row_bytes = base::checked_cast<size_t>(
+        visible_data_only ? frame.GetVisibleRowBytes(plane)
+                          : frame.row_bytes(plane));
+    const auto& plane_data = frame.data_[plane];
+    for (size_t row = 0; row < rows; ++row) {
+      hasher.Update(plane_data.subspan(frame.stride(plane) * row, row_bytes));
     }
   }
 }
 
 // static
-std::string VideoFrame::HexHashOfFrameForTesting(const VideoFrame& frame) {
+std::string VideoFrame::HexHashOfFrameForTesting(const VideoFrame& frame,
+                                                 bool visible_data_only) {
   crypto::hash::Hasher hasher(crypto::hash::HashKind::kSha256);
-  UpdateHashWithFrameForTesting(hasher, frame);  // IN-TEST
+  UpdateHashWithFrameForTesting(hasher, frame, visible_data_only);  // IN-TEST
   std::array<uint8_t, crypto::hash::kSha256Size> hash;
   hasher.Finish(hash);
   return base::ToLowerASCII(base::HexEncode(hash));
@@ -1462,6 +1468,14 @@
   return Rows(plane, format(), coded_size().height());
 }
 
+int VideoFrame::GetVisibleRowBytes(size_t plane) const {
+  return RowBytes(plane, format(), visible_rect().width());
+}
+
+int VideoFrame::GetVisibleRows(size_t plane) const {
+  return Rows(plane, format(), visible_rect().height());
+}
+
 int VideoFrame::columns(size_t plane) const {
   return Columns(plane, format(), coded_size().width());
 }
@@ -1482,7 +1496,6 @@
                           base::bits::AlignDownDeprecatedDoNotUse(
                               visible_rect_.y(), alignment.height()));
 
-  const int visible_plane_rows = Rows(plane, format(), visible_rect_.height());
   const int plane_stride = stride(plane);
   const gfx::Size subsample = SampleSize(format(), plane);
   DCHECK(offset.x() % subsample.width() == 0);
@@ -1494,8 +1507,8 @@
       BytesPerElement(format(), plane) * (offset.x() / subsample.width()));
   // In the last row, bytes between visible width and the full stride are not
   // the part of the visible plane.
-  size_t visible_plane_size = plane_stride * (visible_plane_rows - 1) +
-                              RowBytes(plane, format(), visible_rect_.width());
+  size_t visible_plane_size =
+      plane_stride * (GetVisibleRows(plane) - 1) + GetVisibleRowBytes(plane);
   return data.subspan(visible_plane_offset, visible_plane_size);
 }
 
diff --git a/media/base/video_frame.h b/media/base/video_frame.h
index 72b870c..56d47377 100644
--- a/media/base/video_frame.h
+++ b/media/base/video_frame.h
@@ -478,12 +478,16 @@
   static size_t Columns(size_t plane, VideoPixelFormat format, int width);
 
   // Given a crypto/hash Hasher, hash in the pixels from a single VideoFrame.
+  // If `visible_data_only` is true only the frame's visible area will be
+  // hashed, if false then the entire coded frame area will be hashed.
   static void UpdateHashWithFrameForTesting(crypto::hash::Hasher& hasher,
-                                            const VideoFrame& frame);
+                                            const VideoFrame& frame,
+                                            bool visible_data_only = true);
 
   // Convenience wrapper around UpdateHashWithFrameForTesting(): produces the
   // SHA-256 hash of a single video frame's pixels, as a lowercase hex string.
-  static std::string HexHashOfFrameForTesting(const VideoFrame& frame);
+  static std::string HexHashOfFrameForTesting(const VideoFrame& frame,
+                                              bool visible_data_only = true);
 
   // Returns true if |frame| is accessible mapped in the VideoFrame memory
   // space.
@@ -627,6 +631,10 @@
   int row_bytes(size_t plane) const;
   int rows(size_t plane) const;
 
+  // Similar to row_bytes() and rows(), but instead refers to the visible area.
+  int GetVisibleRowBytes(size_t plane) const;
+  int GetVisibleRows(size_t plane) const;
+
   // Returns the number of columns for a given plane.
   int columns(size_t plane) const;
 
diff --git a/media/base/video_frame_unittest.cc b/media/base/video_frame_unittest.cc
index 2b906fa..01280fe0 100644
--- a/media/base/video_frame_unittest.cc
+++ b/media/base/video_frame_unittest.cc
@@ -240,7 +240,9 @@
            frame->stride(plane) * frame->rows(plane));
   }
 
-  EXPECT_EQ(VideoFrame::HexHashOfFrameForTesting(*frame.get()), expected_hash);
+  EXPECT_EQ(VideoFrame::HexHashOfFrameForTesting(*frame,
+                                                 /*visible_data_only=*/false),
+            expected_hash);
 }
 
 TEST(VideoFrame, CreateFrame) {
@@ -785,6 +787,75 @@
   EXPECT_EQ(677, frame->coded_size().width());
 }
 
+TEST(VideoFrame, CreateFrame_OddSize) {
+  constexpr gfx::Size odd_size(677, 287);
+  constexpr auto kTimestamp = base::TimeDelta();
+
+  // I420A frames should have the Y, A planes aligned upward to an even size,
+  // since it's required by the subsampled U,V planes.
+  auto frame = VideoFrame::CreateFrame(
+      PIXEL_FORMAT_I420A, odd_size, gfx::Rect(odd_size), odd_size, kTimestamp);
+  ASSERT_TRUE(frame);
+  EXPECT_EQ(gfx::Size(678, 288), frame->coded_size());
+  for (int plane = 0; plane < 4; plane++) {
+    // CreateFrame() will always allocate to an even coded size for non-sampled
+    // planes, so the visible row data should be less than the coded row data.
+    if (plane == VideoFrame::Plane::kY || plane == VideoFrame::Plane::kA) {
+      EXPECT_LT(frame->GetVisibleRowBytes(plane), frame->row_bytes(plane));
+      EXPECT_LT(frame->GetVisibleRows(plane), frame->rows(plane));
+    } else {
+      EXPECT_EQ(frame->GetVisibleRowBytes(plane), frame->row_bytes(plane));
+      EXPECT_EQ(frame->GetVisibleRows(plane), frame->rows(plane));
+    }
+  }
+
+  // I444A frames don't have subsampling, so no adjustment is needed.
+  frame = VideoFrame::CreateFrame(PIXEL_FORMAT_I444A, odd_size,
+                                  gfx::Rect(odd_size), odd_size, kTimestamp);
+  ASSERT_TRUE(frame);
+  EXPECT_EQ(odd_size, frame->coded_size());
+  for (int plane = 0; plane < 4; plane++) {
+    EXPECT_EQ(frame->GetVisibleRowBytes(plane), frame->row_bytes(plane));
+    EXPECT_EQ(frame->GetVisibleRows(plane), frame->rows(plane));
+  }
+}
+
+TEST(VideoFrame, RowBytes) {
+  constexpr gfx::Size kCodedSize(16, 14);
+  constexpr gfx::Rect kVisibleRect(4, 4, 8, 8);
+
+  // Just spot test I420A for simplicity.
+  scoped_refptr<VideoFrame> frame =
+      VideoFrame::CreateFrame(PIXEL_FORMAT_I420A, kCodedSize, kVisibleRect,
+                              kVisibleRect.size(), base::TimeDelta());
+  ASSERT_TRUE(frame);
+  ASSERT_EQ(frame->row_bytes(VideoFrame::Plane::kY), kCodedSize.width());
+  ASSERT_EQ(frame->row_bytes(VideoFrame::Plane::kU), kCodedSize.width() / 2);
+  ASSERT_EQ(frame->row_bytes(VideoFrame::Plane::kV), kCodedSize.width() / 2);
+  ASSERT_EQ(frame->row_bytes(VideoFrame::Plane::kA), kCodedSize.width());
+  ASSERT_EQ(frame->rows(VideoFrame::Plane::kY), kCodedSize.height());
+  ASSERT_EQ(frame->rows(VideoFrame::Plane::kU), kCodedSize.height() / 2);
+  ASSERT_EQ(frame->rows(VideoFrame::Plane::kV), kCodedSize.height() / 2);
+  ASSERT_EQ(frame->rows(VideoFrame::Plane::kA), kCodedSize.height());
+
+  ASSERT_EQ(frame->GetVisibleRowBytes(VideoFrame::Plane::kY),
+            kVisibleRect.width());
+  ASSERT_EQ(frame->GetVisibleRowBytes(VideoFrame::Plane::kU),
+            kVisibleRect.width() / 2);
+  ASSERT_EQ(frame->GetVisibleRowBytes(VideoFrame::Plane::kV),
+            kVisibleRect.width() / 2);

diff --git a/media/base/video_frame_unittest.cc b/media/base/video_frame_unittest.cc
index 2b906fa..01280fe0 100644
--- a/media/base/video_frame_unittest.cc
+++ b/media/base/video_frame_unittest.cc
@@ -240,7 +240,9 @@
            frame->stride(plane) * frame->rows(plane));
   }
 
-  EXPECT_EQ(VideoFrame::HexHashOfFrameForTesting(*frame.get()), expected_hash);
+  EXPECT_EQ(VideoFrame::HexHashOfFrameForTesting(*frame,
+                                                 /*visible_data_only=*/false),
+            expected_hash);
 }
 
 TEST(VideoFrame, CreateFrame) {
@@ -785,6 +787,75 @@
   EXPECT_EQ(677, frame->coded_size().width());
 }
 
+TEST(VideoFrame, CreateFrame_OddSize) {
+  constexpr gfx::Size odd_size(677, 287);
+  constexpr auto kTimestamp = base::TimeDelta();
+
+  // I420A frames should have the Y, A planes aligned upward to an even size,
+  // since it's required by the subsampled U,V planes.
+  auto frame = VideoFrame::CreateFrame(
+      PIXEL_FORMAT_I420A, odd_size, gfx::Rect(odd_size), odd_size, kTimestamp);
+  ASSERT_TRUE(frame);
+  EXPECT_EQ(gfx::Size(678, 288), frame->coded_size());
+  for (int plane = 0; plane < 4; plane++) {
+    // CreateFrame() will always allocate to an even coded size for non-sampled
+    // planes, so the visible row data should be less than the coded row data.
+    if (plane == VideoFrame::Plane::kY || plane == VideoFrame::Plane::kA) {
+      EXPECT_LT(frame->GetVisibleRowBytes(plane), frame->row_bytes(plane));
+      EXPECT_LT(frame->GetVisibleRows(plane), frame->rows(plane));
+    } else {
+      EXPECT_EQ(frame->GetVisibleRowBytes(plane), frame->row_bytes(plane));
+      EXPECT_EQ(frame->GetVisibleRows(plane), frame->rows(plane));
+    }
+  }
+
+  // I444A frames don't have subsampling, so no adjustment is needed.
+  frame = VideoFrame::CreateFrame(PIXEL_FORMAT_I444A, odd_size,
+                                  gfx::Rect(odd_size), odd_size, kTimestamp);
+  ASSERT_TRUE(frame);
+  EXPECT_EQ(odd_size, frame->coded_size());
+  for (int plane = 0; plane < 4; plane++) {
+    EXPECT_EQ(frame->GetVisibleRowBytes(plane), frame->row_bytes(plane));
+    EXPECT_EQ(frame->GetVisibleRows(plane), frame->rows(plane));
+  }
+}
+
+TEST(VideoFrame, RowBytes) {
+  constexpr gfx::Size kCodedSize(16, 14);
+  constexpr gfx::Rect kVisibleRect(4, 4, 8, 8);
+
+  // Just spot test I420A for simplicity.
+  scoped_refptr<VideoFrame> frame =
+      VideoFrame::CreateFrame(PIXEL_FORMAT_I420A, kCodedSize, kVisibleRect,
+                              kVisibleRect.size(), base::TimeDelta());
+  ASSERT_TRUE(frame);
+  ASSERT_EQ(frame->row_bytes(VideoFrame::Plane::kY), kCodedSize.width());
+  ASSERT_EQ(frame->row_bytes(VideoFrame::Plane::kU), kCodedSize.width() / 2);
+  ASSERT_EQ(frame->row_bytes(VideoFrame::Plane::kV), kCodedSize.width() / 2);
+  ASSERT_EQ(frame->row_bytes(VideoFrame::Plane::kA), kCodedSize.width());
+  ASSERT_EQ(frame->rows(VideoFrame::Plane::kY), kCodedSize.height());
+  ASSERT_EQ(frame->rows(VideoFrame::Plane::kU), kCodedSize.height() / 2);
+  ASSERT_EQ(frame->rows(VideoFrame::Plane::kV), kCodedSize.height() / 2);
+  ASSERT_EQ(frame->rows(VideoFrame::Plane::kA), kCodedSize.height());
+
+  ASSERT_EQ(frame->GetVisibleRowBytes(VideoFrame::Plane::kY),
+            kVisibleRect.width());
+  ASSERT_EQ(frame->GetVisibleRowBytes(VideoFrame::Plane::kU),
+            kVisibleRect.width() / 2);
+  ASSERT_EQ(frame->GetVisibleRowBytes(VideoFrame::Plane::kV),
+            kVisibleRect.width() / 2);
+  ASSERT_EQ(frame->GetVisibleRowBytes(VideoFrame::Plane::kA),
+            kVisibleRect.width());
+  ASSERT_EQ(frame->GetVisibleRows(VideoFrame::Plane::kY),
+            kVisibleRect.height());
+  ASSERT_EQ(frame->GetVisibleRows(VideoFrame::Plane::kU),
+            kVisibleRect.height() / 2);
+  ASSERT_EQ(frame->GetVisibleRows(VideoFrame::Plane::kV),
+            kVisibleRect.height() / 2);
+  ASSERT_EQ(frame->GetVisibleRows(VideoFrame::Plane::kA),
+            kVisibleRect.height());
+}
+
 TEST(VideoFrame, AllocationSize_OddSize) {
   const gfx::Size size(3, 5);
diff --git a/media/test/pipeline_integration_test.cc b/media/test/pipeline_integration_test.cc
index fb0c86b..a69a08b 100644
--- a/media/test/pipeline_integration_test.cc
+++ b/media/test/pipeline_integration_test.cc
@@ -3035,7 +3035,8 @@
   ASSERT_EQ(PIPELINE_OK, StartPipelineWithHlsManifest("hls/mp_ts_avc1.m3u8"));
   Play();
   ASSERT_TRUE(WaitUntilOnEnded());
-  EXPECT_EQ("00df0aa6796123f535402c39c20baa3fe1c2ad02fe1f015f84878977b30931d2",
+  // 320x192 video of which only 320x180 is visible.
+  EXPECT_EQ("9537d9d2592aa801cff8fceb2af9f6e3c5226df089e16f8f789d43e1fdec7ba2",
             GetVideoHash());
 }
 #endif
diff --git a/third_party/blink/renderer/modules/webcodecs/video_frame_test.cc b/third_party/blink/renderer/modules/webcodecs/video_frame_test.cc
index 7fa9293d..15030a2f 100644
--- a/third_party/blink/renderer/modules/webcodecs/video_frame_test.cc
+++ b/third_party/blink/renderer/modules/webcodecs/video_frame_test.cc
@@ -18,6 +18,7 @@
 #include "third_party/blink/renderer/bindings/modules/v8/v8_union_blob_htmlcanvaselement_htmlimageelement_htmlvideoelement_imagebitmap_imagedata_offscreencanvas_svgimageelement_videoframe.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_union_cssimagevalue_htmlcanvaselement_htmlimageelement_htmlvideoelement_imagebitmap_offscreencanvas_svgimageelement_videoframe.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_video_decoder_config.h"
+#include "third_party/blink/renderer/bindings/modules/v8/v8_video_frame_buffer_init.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_video_frame_copy_to_options.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_video_frame_init.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_video_frame_metadata.h"
@@ -114,6 +115,78 @@
   EXPECT_EQ(nullptr, blink_frame->frame());
 }
 
+TEST_F(VideoFrameTest, ConstructorOddSize) {
+  V8TestingScope scope;
+
+  constexpr auto kOddSize = gfx::Size(61, 21);
+  const auto kOddUVSize = gfx::Size(std::ceil(kOddSize.width() / 2.0),
+                                    std::ceil(kOddSize.height() / 2.0));
+  const size_t allocation_size =
+      kOddSize.Area64() * 2 + kOddUVSize.Area64() * 2;
+
+  auto* array_buffer = DOMArrayBuffer::Create(allocation_size, 1);
+
+  // Fill buffer with random data for hash and extents testing.
+  base::RandBytes(array_buffer->ByteSpan());
+
+  std::string media_frame_hash;
+  {
+    const size_t kYAPlaneByteSize = kOddSize.Area64();
+    const size_t kUVPlaneByteSize = kOddUVSize.Area64();
+    auto src_media_frame = media::VideoFrame::WrapExternalYuvaData(
+        media::PIXEL_FORMAT_I420A, kOddSize, gfx::Rect(kOddSize), kOddSize,
+        kOddSize.width(), kOddUVSize.width(), kOddUVSize.width(),
+        kOddSize.width(), array_buffer->ByteSpan().first(kYAPlaneByteSize),
+        array_buffer->ByteSpan().subspan(kYAPlaneByteSize, kUVPlaneByteSize),
+        array_buffer->ByteSpan().subspan(kYAPlaneByteSize + kUVPlaneByteSize,
+                                         kUVPlaneByteSize),
+        array_buffer->ByteSpan().subspan(
+            kYAPlaneByteSize + kUVPlaneByteSize * 2, kYAPlaneByteSize),
+        base::TimeDelta());
+    ASSERT_TRUE(src_media_frame);
+    media_frame_hash =
+        media::VideoFrame::HexHashOfFrameForTesting(*src_media_frame);
+  }
+
+  auto* init = VideoFrameBufferInit::Create();
+  init->setTimestamp(0);
+  init->setCodedWidth(kOddSize.width());
+  init->setCodedHeight(kOddSize.height());
+  init->setFormat("I420A");
+  init->setDisplayWidth(kOddSize.width());
+  init->setDisplayHeight(kOddSize.height());
+
+  // Test non-transfer constructor first then the transfer constructor.
+  for (bool test_transfer : {false, true}) {
+    SCOPED_TRACE(test_transfer);
+    if (test_transfer) {
+      HeapVector<Member<DOMArrayBuffer>> transfer;
+      transfer.push_back(Member<DOMArrayBuffer>(array_buffer));
+      init->setTransfer(std::move(transfer));
+    }
+
+    VideoFrame* blink_frame = VideoFrame::Create(
+        scope.GetScriptState(),
+        MakeGarbageCollected<V8AllowSharedBufferSource>(array_buffer), init,
+        scope.GetExceptionState());
+    ASSERT_TRUE(blink_frame);
+
+    EXPECT_LE(static_cast<unsigned>(kOddSize.width()),
+              blink_frame->codedWidth());
+    EXPECT_LE(static_cast<unsigned>(kOddSize.height()),
+              blink_frame->codedHeight());
+    EXPECT_EQ(static_cast<unsigned>(kOddSize.width()),
+              blink_frame->displayWidth());
+    EXPECT_EQ(static_cast<unsigned>(kOddSize.height()),
+              blink_frame->displayHeight());
+
+    auto blink_media_frame = blink_frame->frame();
+    EXPECT_EQ(media_frame_hash,
+              media::VideoFrame::HexHashOfFrameForTesting(*blink_media_frame));
+    blink_frame->close();
+  }
+}
+
 TEST_F(VideoFrameTest, CopyToRGB) {
   V8TestingScope scope;