Skip to content

Commit

Permalink
[video_frame] Fix a few incorrect usages of VideoFrame::writable_data
Browse files Browse the repository at this point in the history
The changed locations violated the CHECK in
https://source.chromium.org/chromium/chromium/src/+/main:media/base/video_frame.h;l=577
which only allows usage of `writable_data` for owned shared memory.
These encoders don't actually modify the frame, so the the const
`visible_data` method is more appropriate. The only sticking point is
that the APIs unfortunately take non-const uint8_t* so the pointer
returned from `visible_data` is const_cast to make the type match up.

(cherry picked from commit f475257)

Fixed: 1458242
Change-Id: I2042c38a9c6f08e7a4cb7d3dadbba41924f5bf53
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4654869
Commit-Queue: Bryant Chandler <bryantchandler@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Derek Schuff <dschuff@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1164154}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4658989
Auto-Submit: Bryant Chandler <bryantchandler@chromium.org>
Commit-Queue: Derek Schuff <dschuff@chromium.org>
Cr-Commit-Position: refs/branch-heads/5845@{#272}
Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}
  • Loading branch information
Bryant Chandler authored and Chromium LUCI CQ committed Jun 30, 2023
1 parent e230f86 commit d5b7ead
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 12 deletions.
15 changes: 8 additions & 7 deletions content/renderer/pepper/video_encoder_shim.cc
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,15 @@ void VideoEncoderShim::EncoderImpl::DoEncode() {
vpx_image_t* const result = vpx_img_wrap(
&vpx_image, VPX_IMG_FMT_I420, frame.frame->visible_rect().width(),
frame.frame->visible_rect().height(), 1,
frame.frame->writable_data(media::VideoFrame::kYPlane));
const_cast<uint8_t*>(
frame.frame->visible_data(media::VideoFrame::kYPlane)));
DCHECK_EQ(result, &vpx_image);
vpx_image.planes[VPX_PLANE_Y] =
frame.frame->GetWritableVisibleData(media::VideoFrame::kYPlane);
vpx_image.planes[VPX_PLANE_U] =
frame.frame->GetWritableVisibleData(media::VideoFrame::kUPlane);
vpx_image.planes[VPX_PLANE_V] =
frame.frame->GetWritableVisibleData(media::VideoFrame::kVPlane);
vpx_image.planes[VPX_PLANE_Y] = const_cast<uint8_t*>(
frame.frame->visible_data(media::VideoFrame::kYPlane));
vpx_image.planes[VPX_PLANE_U] = const_cast<uint8_t*>(
frame.frame->visible_data(media::VideoFrame::kUPlane));
vpx_image.planes[VPX_PLANE_V] = const_cast<uint8_t*>(
frame.frame->visible_data(media::VideoFrame::kVPlane));
vpx_image.stride[VPX_PLANE_Y] =
frame.frame->stride(media::VideoFrame::kYPlane);
vpx_image.stride[VPX_PLANE_U] =
Expand Down
2 changes: 1 addition & 1 deletion media/cast/encoding/av1_encoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ void Av1Encoder::Encode(scoped_refptr<media::VideoFrame> video_frame,
aom_image_t aom_image;
aom_image_t* const result = aom_img_wrap(
&aom_image, aom_format, frame_size.width(), frame_size.height(), 1,
video_frame->writable_data(VideoFrame::kYPlane));
const_cast<uint8_t*>(video_frame->visible_data(VideoFrame::kYPlane)));
DCHECK_EQ(result, &aom_image);

aom_image.planes[AOM_PLANE_Y] =
Expand Down
2 changes: 1 addition & 1 deletion media/cast/encoding/vpx_encoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ void VpxEncoder::Encode(scoped_refptr<media::VideoFrame> video_frame,
vpx_image_t vpx_image;
vpx_image_t* const result = vpx_img_wrap(
&vpx_image, vpx_format, frame_size.width(), frame_size.height(), 1,
video_frame->writable_data(VideoFrame::kYPlane));
const_cast<uint8_t*>(video_frame->visible_data(VideoFrame::kYPlane)));
DCHECK_EQ(result, &vpx_image);
switch (vpx_format) {
case VPX_IMG_FMT_I420:
Expand Down
6 changes: 3 additions & 3 deletions media/video/av1_video_encoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,9 @@ void Av1VideoEncoder::Encode(scoped_refptr<VideoFrame> frame,

aom_img_fmt fmt = frame->format() == PIXEL_FORMAT_NV12 ? AOM_IMG_FMT_NV12
: AOM_IMG_FMT_I420;
aom_image_t* image = aom_img_wrap(&image_, fmt, options_.frame_size.width(),
options_.frame_size.height(), 1,
frame->writable_data(VideoFrame::kYPlane));
aom_image_t* image = aom_img_wrap(
&image_, fmt, options_.frame_size.width(), options_.frame_size.height(),
1, const_cast<uint8_t*>(frame->visible_data(VideoFrame::kYPlane)));
DCHECK_EQ(image, &image_);

switch (frame->format()) {
Expand Down

0 comments on commit d5b7ead

Please sign in to comment.