New issue
Advanced search Search tips

Issue 876986 link

Starred by 1 user
Status: Fixed
Owner:
acourbot@chromium.org
Closed: Sep 13
Cc:
hiroh@chromium.org
deanliao@chromium.org
posciak@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

VideoFrameLayout should be a constant class.

Project Member Reported by acourbot@chromium.org, Aug 23 
VideoFrameLayout describes the layout of a video frame, i.e. a property that is not supposed to change at runtime.

Yet this class provides two methods that can change its state dynamically:

* void set_strides(std::vector<int32_t> strides)
* void set_buffer_sizes(std::vector<size_t> buffer_sizes)

This bug tracks the update of the users of VideoFrameLayout in such a way that these methods are not needed anymore and can be removed.
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 24 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ff63341da9a78bf18bb0755ed94696a54075ec5c

commit ff63341da9a78bf18bb0755ed94696a54075ec5c
Author: Alexandre Courbot <acourbot@chromium.org>
Date: Fri Aug 24 08:18:43 2018

media/base/video_frame_layout: remove set_buffer_sizes() method

This method has no user besides its own unit test, and we want to remove
all non-const methods from VideoFrameLayout.

BUG= 876986 
TEST=built vdatest and Chromium.

Change-Id: I5f332416c25a747685f2194989a61df225bb1159
Reviewed-on: https://chromium-review.googlesource.com/1186286
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585758}
[modify] https://crrev.com/ff63341da9a78bf18bb0755ed94696a54075ec5c/media/base/video_frame_layout.h
[modify] https://crrev.com/ff63341da9a78bf18bb0755ed94696a54075ec5c/media/base/video_frame_layout_unittest.cc
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 3 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/53905008c025284479f4c79a0032f58443f8e946

commit 53905008c025284479f4c79a0032f58443f8e946
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Mon Sep 03 08:28:38 2018

Nuke set_strides in VideoFrame and VideoFrameLayout

This removes set_strides from VideoFrameLayout and VideoFrame.
VideoFrameLayout in VideoFrame becomes constant.

BUG= chromium:876986 
TEST=media_unittests and VDA unittests on eve

Change-Id: I24143321cb553e5e598df6574eb16059e9252be5
Reviewed-on: https://chromium-review.googlesource.com/1196623
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588349}
[modify] https://crrev.com/53905008c025284479f4c79a0032f58443f8e946/media/base/video_frame.cc
[modify] https://crrev.com/53905008c025284479f4c79a0032f58443f8e946/media/base/video_frame.h
[modify] https://crrev.com/53905008c025284479f4c79a0032f58443f8e946/media/base/video_frame_layout.h
[modify] https://crrev.com/53905008c025284479f4c79a0032f58443f8e946/media/base/video_frame_layout_unittest.cc
[modify] https://crrev.com/53905008c025284479f4c79a0032f58443f8e946/media/mojo/common/mojo_shared_buffer_video_frame.cc
[modify] https://crrev.com/53905008c025284479f4c79a0032f58443f8e946/media/mojo/common/mojo_shared_buffer_video_frame.h
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 11 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76f221244448793c3ab0346bc94e77e8d5a3b666

commit 76f221244448793c3ab0346bc94e77e8d5a3b666
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Tue Sep 11 08:54:27 2018

VideoFrameLayout: Add offsets and aggregate strides and offsets as Plane

VideoFrameLayout should have information about offset per plane, not only
stride. The number of stride should be the same as the number of offsets. To
guarantee this, they are stored in the vector of struct, Plane.

BUG= chromium:876986 ,  chromium:856562 
TEST=VDA unittest and media_unittest

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: Ie9d9afa5db7dabaee2f48fbcd355c13d29be8035
Reviewed-on: https://chromium-review.googlesource.com/1188730
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590242}
[modify] https://crrev.com/76f221244448793c3ab0346bc94e77e8d5a3b666/media/base/video_frame.cc
[modify] https://crrev.com/76f221244448793c3ab0346bc94e77e8d5a3b666/media/base/video_frame.h
[modify] https://crrev.com/76f221244448793c3ab0346bc94e77e8d5a3b666/media/base/video_frame_layout.cc
[modify] https://crrev.com/76f221244448793c3ab0346bc94e77e8d5a3b666/media/base/video_frame_layout.h
[modify] https://crrev.com/76f221244448793c3ab0346bc94e77e8d5a3b666/media/base/video_frame_layout_unittest.cc
[modify] https://crrev.com/76f221244448793c3ab0346bc94e77e8d5a3b666/media/base/video_frame_unittest.cc
[modify] https://crrev.com/76f221244448793c3ab0346bc94e77e8d5a3b666/media/cast/test/utility/video_utility.cc
[modify] https://crrev.com/76f221244448793c3ab0346bc94e77e8d5a3b666/media/gpu/v4l2/v4l2_image_processor.cc
[modify] https://crrev.com/76f221244448793c3ab0346bc94e77e8d5a3b666/media/gpu/v4l2/v4l2_video_decode_accelerator.cc

Comment 4 by hiroh@chromium.org, Sep 13

Status: Fixed (was: Assigned)
Now all VideoFrameLayout's function are const. This work is done.
Let's close!
►

Sign in to add a comment