Add unittest for VEA Flush() |
||||||||
Issue descriptionIn Issue 755889 , we implemented Flush function. We should also implement the corresponding check in unittest. At the first step, we check a simple situation: flush after all frames are encoded. Here is the step: 1. VEA client initializes VEA and encode frames, as usual. 2. After VEA client encodes all the frames, call VEA.Flush(callback) 3. Wait for callback is called with success in a period of timeout. 4. Validate the encoded frame, as usual.
,
Mar 30 2018
After the Flush unittest is completed, we need to check vaapi also pass the unitttest.
,
Apr 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/abcbc5356cc8da6a6511774c8ab8e20d6dec9ba2 commit abcbc5356cc8da6a6511774c8ab8e20d6dec9ba2 Author: Chih-Yu Huang <akahuang@google.com> Date: Thu Apr 05 01:47:43 2018 vea: Check flush behavior in VEA unittest. This CL checks VEA flush behavior by adding two states: CS_FLUSHING and CS_FLUSHED. Now the encoding flow becomes: - Set state from CS_ENCODING to CS_FLUSHING when the client sends the last frame and calls VEA::Flush(flush_cb) - Set state from CS_FLUSHING to CS_FINISHED when the client receives the last encoded frame - Set state from CS_FINISHED to CS_FLUSHED when flush_cb(success=true) is called - Set state to CS_ERROR if either of situation is true: - flush_cb(success=false) is called before last encoded frame is received - flush_cb(success=true) is not called in a period of time (e.g 200 ms) from VEA::Flush() is called. BUG= chromium:820313 TEST=Run video_encode_accelerator_unittest at scarlet device 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;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I2a83b0e7f902575b084ae4ccf85f3ba7d5f74819 Reviewed-on: https://chromium-review.googlesource.com/908370 Reviewed-by: Kuang-che Wu <kcwu@chromium.org> Commit-Queue: Chih-Yu Huang <akahuang@chromium.org> Cr-Commit-Position: refs/heads/master@{#548288} [modify] https://crrev.com/abcbc5356cc8da6a6511774c8ab8e20d6dec9ba2/media/gpu/video_encode_accelerator_unittest.cc
,
Apr 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d90e694fc6f5199e727200f6b36f361351497344 commit d90e694fc6f5199e727200f6b36f361351497344 Author: Chih-Yu Huang <akahuang@google.com> Date: Thu Apr 05 09:45:51 2018 vaapi vea: Fix flush functionality. In Vaapi VEA, We push the input frame at the queue |encoder_input_queue_| first, and then push to the second queue |submitted_encode_jobs_| when we prepare the submit the frame. To flush the frame correctly, we should push a "fence" (nullptr) into the first queue |encoder_input_queue_|, then pass the fence to the second queue. Bug: chromium:820313 Test: Pass video_encode_accelerator_unittest at Eve device 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;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: Id4ce02973297e678fd7fb1d35ae6946d667315d5 Reviewed-on: https://chromium-review.googlesource.com/997233 Reviewed-by: Ricky Liang <jcliang@chromium.org> Commit-Queue: Chih-Yu Huang <akahuang@chromium.org> Cr-Commit-Position: refs/heads/master@{#548377} [modify] https://crrev.com/d90e694fc6f5199e727200f6b36f361351497344/media/gpu/vaapi/vaapi_video_encode_accelerator.cc
,
Apr 8 2018
video_VideoEncodeAccelerator fails on several devices. Please take a look. https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/testDetails?testName=video_VideoEncodeAccelerato
,
Apr 9 2018
Discussed with hiroh@ and tfiga@ offline, there are some drivers not supporting flush yet. Currently we add a "--disable_flush" argument at vea unittest, and skip checking these unsupported devices first. Then we should fix them when we enable ARC++ HW encoder.
,
Apr 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e6c75d9f117a956605c8d11193167803c5e242d6 commit e6c75d9f117a956605c8d11193167803c5e242d6 Author: Chih-Yu Huang <akahuang@google.com> Date: Tue Apr 10 03:15:35 2018 vea_unittest: Set longer flush timeout. Originally we assume the FPS of all encoders should be larger than the minimum FPS 30. But the FPS for multiple encoder test case should be divided by the number of the encoders. This CL changes the timeout to 2 seconds assuming the FPS should be at least larger than 5. Bug: chromium:820313 Test: Pass video_encode_accelerator_unittest with 1920x1080 video at scarlet device 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;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: Ic91a6aa8b00ee0bdb73da407e4c435ea2979a2c4 Reviewed-on: https://chromium-review.googlesource.com/997253 Reviewed-by: Ricky Liang <jcliang@chromium.org> Commit-Queue: Chih-Yu Huang <akahuang@chromium.org> Cr-Commit-Position: refs/heads/master@{#549404} [modify] https://crrev.com/e6c75d9f117a956605c8d11193167803c5e242d6/media/gpu/video_encode_accelerator_unittest.cc
,
Apr 16 2018
,
Apr 16 2018
May I ask TPM to approve this CL? https://chromium-review.googlesource.com/c/chromiumos/third_party/autotest/+/1002335
,
Apr 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/49083c3c5a8a34595222c2009881b175b03422b8 commit 49083c3c5a8a34595222c2009881b175b03422b8 Author: Chih-Yu Huang <akahuang@google.com> Date: Mon Apr 16 16:20:44 2018 video: Add "--disable_flush" argument in VEA-related autotest In CL:999233, we add "--disable_flush" argument to skip checking the flush function. This CL added this argument for the devices which flush function is broken. BUG= chromium:820313 TEST=Run video_VideoEncodeAccelerator.h264 autotest and confirm "--disable_flush" argument is appended for device in blacklist Change-Id: Ifea62d2d784636d4412c0d24d6161f6eb14fd1dd Reviewed-on: https://chromium-review.googlesource.com/1002335 Commit-Ready: Chih-Yu Huang <akahuang@chromium.org> Tested-by: Chih-Yu Huang <akahuang@chromium.org> Reviewed-by: Chih-Yu Huang <akahuang@chromium.org> [modify] https://crrev.com/49083c3c5a8a34595222c2009881b175b03422b8/client/site_tests/video_VideoEncodeAccelerator/video_VideoEncodeAccelerator.py [add] https://crrev.com/49083c3c5a8a34595222c2009881b175b03422b8/client/cros/video/encoder_utils.py [modify] https://crrev.com/49083c3c5a8a34595222c2009881b175b03422b8/client/site_tests/video_VEAPerf/video_VEAPerf.py [modify] https://crrev.com/49083c3c5a8a34595222c2009881b175b03422b8/client/site_tests/video_HangoutHardwarePerf/video_HangoutHardwarePerf.py
,
Apr 16 2018
Pls apply appropriate OSs label. Thank you.
,
Apr 16 2018
Applying OS=Chrome per component "OS>Kernel>Video". Pls add any others OSs if applicable.
,
Apr 16 2018
Per #9 note that we (TPMs) typically review the CL when we consider the merge request but don't approve the CL directly. Assume the scope of this CL is testing and not customer facing?
,
Apr 16 2018
#13, yes this CL is for testing. To get tests on M67 passed, cherry-picking this CL is needed.
,
Apr 17 2018
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/64dd913eb0ab9ae20ee0e601a4914c98438b0adb commit 64dd913eb0ab9ae20ee0e601a4914c98438b0adb Author: Chih-Yu Huang <akahuang@google.com> Date: Tue Apr 17 05:43:34 2018 video: Add "--disable_flush" argument in VEA-related autotest In CL:999233, we add "--disable_flush" argument to skip checking the flush function. This CL added this argument for the devices which flush function is broken. BUG= chromium:820313 TEST=Run video_VideoEncodeAccelerator.h264 autotest and confirm "--disable_flush" argument is appended for device in blacklist Change-Id: Ifea62d2d784636d4412c0d24d6161f6eb14fd1dd Reviewed-on: https://chromium-review.googlesource.com/1002335 Commit-Ready: Chih-Yu Huang <akahuang@chromium.org> Tested-by: Chih-Yu Huang <akahuang@chromium.org> Reviewed-by: Chih-Yu Huang <akahuang@chromium.org> (cherry picked from commit 49083c3c5a8a34595222c2009881b175b03422b8) Reviewed-on: https://chromium-review.googlesource.com/1013446 Commit-Queue: Chih-Yu Huang <akahuang@chromium.org> [modify] https://crrev.com/64dd913eb0ab9ae20ee0e601a4914c98438b0adb/client/site_tests/video_VideoEncodeAccelerator/video_VideoEncodeAccelerator.py [add] https://crrev.com/64dd913eb0ab9ae20ee0e601a4914c98438b0adb/client/cros/video/encoder_utils.py [modify] https://crrev.com/64dd913eb0ab9ae20ee0e601a4914c98438b0adb/client/site_tests/video_VEAPerf/video_VEAPerf.py [modify] https://crrev.com/64dd913eb0ab9ae20ee0e601a4914c98438b0adb/client/site_tests/video_HangoutHardwarePerf/video_HangoutHardwarePerf.py
,
Apr 17 2018
,
Apr 20 2018
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 25 2018
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 25 2018
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8 commit 43b45aee7461d7c5ab8a256b741d2e4dc984e9d8 Author: Chih-Yu Huang <akahuang@google.com> Date: Tue Sep 04 05:50:32 2018 VEA: Remove "--disable_flush" argument at VEA unittest. Originally, we need to add "--disable_flush" argument when running the VEA unittest if the encoder does not support flush. This CL added a method IsFlushSupported() to check if the encoder support flush. BUG= 820313 TEST=Run vea unittest at eve and nyan_big without "--disable_flush" argument 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: Id6a59546b0a13df15ca4ff2825eb70c848eaf704 Reviewed-on: https://chromium-review.googlesource.com/1170671 Reviewed-by: Chrome Cunningham <chcunningham@chromium.org> Reviewed-by: Pawel Osciak <posciak@chromium.org> Reviewed-by: Hirokazu Honda <hiroh@chromium.org> Commit-Queue: Chih-Yu Huang <akahuang@chromium.org> Cr-Commit-Position: refs/heads/master@{#588457} [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/v4l2/v4l2_video_encode_accelerator.cc [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/v4l2/v4l2_video_encode_accelerator.h [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/vaapi/vaapi_video_encode_accelerator.cc [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/vaapi/vaapi_video_encode_accelerator.h [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/video_encode_accelerator_unittest.cc [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/video/video_encode_accelerator.cc [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/video/video_encode_accelerator.h
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8 commit 43b45aee7461d7c5ab8a256b741d2e4dc984e9d8 Author: Chih-Yu Huang <akahuang@google.com> Date: Tue Sep 04 05:50:32 2018 VEA: Remove "--disable_flush" argument at VEA unittest. Originally, we need to add "--disable_flush" argument when running the VEA unittest if the encoder does not support flush. This CL added a method IsFlushSupported() to check if the encoder support flush. BUG= 820313 TEST=Run vea unittest at eve and nyan_big without "--disable_flush" argument 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: Id6a59546b0a13df15ca4ff2825eb70c848eaf704 Reviewed-on: https://chromium-review.googlesource.com/1170671 Reviewed-by: Chrome Cunningham <chcunningham@chromium.org> Reviewed-by: Pawel Osciak <posciak@chromium.org> Reviewed-by: Hirokazu Honda <hiroh@chromium.org> Commit-Queue: Chih-Yu Huang <akahuang@chromium.org> Cr-Commit-Position: refs/heads/master@{#588457} [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/v4l2/v4l2_video_encode_accelerator.cc [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/v4l2/v4l2_video_encode_accelerator.h [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/vaapi/vaapi_video_encode_accelerator.cc [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/vaapi/vaapi_video_encode_accelerator.h [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/video_encode_accelerator_unittest.cc [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/video/video_encode_accelerator.cc [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/video/video_encode_accelerator.h
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8 commit 43b45aee7461d7c5ab8a256b741d2e4dc984e9d8 Author: Chih-Yu Huang <akahuang@google.com> Date: Tue Sep 04 05:50:32 2018 VEA: Remove "--disable_flush" argument at VEA unittest. Originally, we need to add "--disable_flush" argument when running the VEA unittest if the encoder does not support flush. This CL added a method IsFlushSupported() to check if the encoder support flush. BUG= 820313 TEST=Run vea unittest at eve and nyan_big without "--disable_flush" argument 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: Id6a59546b0a13df15ca4ff2825eb70c848eaf704 Reviewed-on: https://chromium-review.googlesource.com/1170671 Reviewed-by: Chrome Cunningham <chcunningham@chromium.org> Reviewed-by: Pawel Osciak <posciak@chromium.org> Reviewed-by: Hirokazu Honda <hiroh@chromium.org> Commit-Queue: Chih-Yu Huang <akahuang@chromium.org> Cr-Commit-Position: refs/heads/master@{#588457} [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/v4l2/v4l2_video_encode_accelerator.cc [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/v4l2/v4l2_video_encode_accelerator.h [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/vaapi/vaapi_video_encode_accelerator.cc [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/vaapi/vaapi_video_encode_accelerator.h [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/gpu/video_encode_accelerator_unittest.cc [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/video/video_encode_accelerator.cc [modify] https://crrev.com/43b45aee7461d7c5ab8a256b741d2e4dc984e9d8/media/video/video_encode_accelerator.h
,
Sep 10
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/248df8b553ff945fc9e1c872d1e014100f304d1f commit 248df8b553ff945fc9e1c872d1e014100f304d1f Author: Chih-Yu Huang <akahuang@google.com> Date: Mon Sep 10 08:13:27 2018 Revert "video: Add "--disable_flush" argument in VEA-related autotest" In CL:1170671 we removed the argument "--disable_flush". Revert the CL at autotest as well. This reverts commit 49083c3c5a8a34595222c2009881b175b03422b8. BUG= chromium:820313 TEST=none Change-Id: Ia8153f376e0faa124b2747f3a438051710e556b4 Reviewed-on: https://chromium-review.googlesource.com/1170676 Commit-Ready: Chih-Yu Huang <akahuang@chromium.org> Tested-by: Chih-Yu Huang <akahuang@chromium.org> Reviewed-by: Hirokazu Honda <hiroh@chromium.org> [modify] https://crrev.com/248df8b553ff945fc9e1c872d1e014100f304d1f/client/site_tests/video_VideoEncodeAccelerator/video_VideoEncodeAccelerator.py [delete] https://crrev.com/9244d9837eec43e28a74315f40c169bc8a8940dc/client/cros/video/encoder_utils.py [modify] https://crrev.com/248df8b553ff945fc9e1c872d1e014100f304d1f/client/site_tests/video_VEAPerf/video_VEAPerf.py [modify] https://crrev.com/248df8b553ff945fc9e1c872d1e014100f304d1f/client/site_tests/video_HangoutHardwarePerf/video_HangoutHardwarePerf.py |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by posciak@chromium.org
, Mar 9 2018Status: Assigned (was: Untriaged)