capture_unittests's TakePhoto fails on ChromeOS VM |
|||||||
Issue descriptionVideoCaptureDeviceTest.UsingRealWebcam_TakePhoto failed on amd64-generic VM where vivid is enabled: The error is like the following: > Check failed: stream_context_[StreamType::kStillCapture] https://ci.chromium.org/p/chromium/builders/luci.chromium.try/chromeos-amd64-generic-rel/121949 This error is related to crrev.com/c/1295753. When I reverted this CL locally, capture_unittests passed.
,
Nov 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ccff286509941a955c566919b7755bdb9432f153 commit ccff286509941a955c566919b7755bdb9432f153 Author: Heng-Ruey Hsu <henryhsu@chromium.org> Date: Tue Nov 06 03:52:02 2018 Reconfigure camera stream in TakePhoto API doesn't ask user to call SetPhotoOptions before TakePhoto. Therefore TakePhoto should enable blob streams as well if SetPhotoOptions is not called. BUG=b:114676133, chromium:900900 TEST=./capture_unittest --gtest_filter="*VideoCaptureDeviceTests/*" and take photo in CCA Change-Id: I72d4f1394fe95aaca768cb366a341796f55fe336 Reviewed-on: https://chromium-review.googlesource.com/c/1314074 Reviewed-by: Miguel Casas <mcasas@chromium.org> Commit-Queue: Heng-ruey Hsu <henryhsu@chromium.org> Cr-Commit-Position: refs/heads/master@{#605588} [modify] https://crrev.com/ccff286509941a955c566919b7755bdb9432f153/media/capture/video/chromeos/camera_device_delegate.cc [modify] https://crrev.com/ccff286509941a955c566919b7755bdb9432f153/media/capture/video/chromeos/camera_device_delegate.h [modify] https://crrev.com/ccff286509941a955c566919b7755bdb9432f153/media/capture/video/video_capture_device_unittest.cc
,
Nov 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/69af66515d9b068996d7081eb921459476d1846c commit 69af66515d9b068996d7081eb921459476d1846c Author: Heng-Ruey Hsu <henryhsu@chromium.org> Date: Tue Nov 06 06:41:53 2018 Reconfigure camera stream in TakePhoto API doesn't ask user to call SetPhotoOptions before TakePhoto. Therefore TakePhoto should enable blob streams as well if SetPhotoOptions is not called. BUG=b:114676133, chromium:900900 TEST=./capture_unittest --gtest_filter="*VideoCaptureDeviceTests/*" and take photo in CCA TBR=henryhsu@chromium.org (cherry picked from commit ccff286509941a955c566919b7755bdb9432f153) Change-Id: I72d4f1394fe95aaca768cb366a341796f55fe336 Reviewed-on: https://chromium-review.googlesource.com/c/1314074 Reviewed-by: Miguel Casas <mcasas@chromium.org> Commit-Queue: Heng-ruey Hsu <henryhsu@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#605588} Reviewed-on: https://chromium-review.googlesource.com/c/1319400 Reviewed-by: Heng-ruey Hsu <henryhsu@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#537} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/69af66515d9b068996d7081eb921459476d1846c/media/capture/video/chromeos/camera_device_delegate.cc [modify] https://crrev.com/69af66515d9b068996d7081eb921459476d1846c/media/capture/video/chromeos/camera_device_delegate.h [modify] https://crrev.com/69af66515d9b068996d7081eb921459476d1846c/media/capture/video/video_capture_device_unittest.cc
,
Nov 6
,
Nov 9
It fails [1] on another DHECK: > [2947:3325:1108/194617.198867:89599898:FATAL:stream_buffer_manager.cc(84)] Check failed: !stream_context_[StreamType::kPreview]. It looks like we enter the StreamBufferManager::SetUpStreamsAndBuffers [2] again after reconfiguring the streams for still capture, but we didn't cleanup the |stream_context_| when stopping the previous streams. [1] https://ci.chromium.org/p/chromium/builders/luci.chromium.try/chromeos-amd64-generic-rel/127690 [2] https://cs.chromium.org/chromium/src/media/capture/video/chromeos/stream_buffer_manager.cc?l=84&rcl=4d281c8de3c433b6660a63af2b33977800d94aca
,
Nov 9
Thank you shik for the update. When we build capture_unittests as release-build, DCHECK is skipped and it says "PASSED" without any error message. I forgot enabling DCHECK when I verified the CL. Sorry.
,
Nov 9
We perhaps might want to make the test fail even when it's a release-build.
,
Nov 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/678fc40f10bb52e86d53f9f49ce936ecf98162b0 commit 678fc40f10bb52e86d53f9f49ce936ecf98162b0 Author: Heng-Ruey Hsu <henryhsu@chromium.org> Date: Wed Nov 14 09:54:43 2018 Remove unnecessary DCHECK in stream buffer manager SetUpStreamsAndBuffers may be called twice when enabling blob stream. For the second call, kPreview stream already exists. Also reset repeating_request_settings_ in StopPreview. BUG= chromium:900900 TEST=./capture_unittest --gtest_filter="*VideoCaptureDeviceTests/*" Pass TakePhoto cases with DCHECK enabled. Change-Id: Iac89964b95e7d14f592c779f11a92ef46dba930e Reviewed-on: https://chromium-review.googlesource.com/c/1332974 Reviewed-by: Ricky Liang <jcliang@chromium.org> Commit-Queue: Heng-ruey Hsu <henryhsu@chromium.org> Cr-Commit-Position: refs/heads/master@{#607943} [modify] https://crrev.com/678fc40f10bb52e86d53f9f49ce936ecf98162b0/media/capture/video/chromeos/stream_buffer_manager.cc
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/69ecf314612e3e0d43864aaedd888f8d6be41e81 commit 69ecf314612e3e0d43864aaedd888f8d6be41e81 Author: Heng-Ruey Hsu <henryhsu@chromium.org> Date: Thu Nov 15 05:54:31 2018 Remove unnecessary DCHECK in stream buffer manager SetUpStreamsAndBuffers may be called twice when enabling blob stream. For the second call, kPreview stream already exists. Also reset repeating_request_settings_ in StopPreview. BUG= chromium:900900 TEST=./capture_unittest --gtest_filter="*VideoCaptureDeviceTests/*" Pass TakePhoto cases with DCHECK enabled. TBR=henryhsu@chromium.org (cherry picked from commit 678fc40f10bb52e86d53f9f49ce936ecf98162b0) Change-Id: Iac89964b95e7d14f592c779f11a92ef46dba930e Reviewed-on: https://chromium-review.googlesource.com/c/1332974 Reviewed-by: Ricky Liang <jcliang@chromium.org> Commit-Queue: Heng-ruey Hsu <henryhsu@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#607943} Reviewed-on: https://chromium-review.googlesource.com/c/1337218 Reviewed-by: Heng-ruey Hsu <henryhsu@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#689} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/69ecf314612e3e0d43864aaedd888f8d6be41e81/media/capture/video/chromeos/stream_buffer_manager.cc
,
Nov 15
,
Nov 15
Here's a summary of the rules that were executed: - OnlyMergeApprovedChange: Rule Failed -- Revision 69ecf314612e3e0d43864aaedd888f8d6be41e81 was merged to refs/branch-heads/3578 branch with no merge approval from a TPM! Please explain why this change was merged to the branch! - AcknowledgeMerge: Notification Required --
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/69ecf314612e3e0d43864aaedd888f8d6be41e81 Commit: 69ecf314612e3e0d43864aaedd888f8d6be41e81 Author: henryhsu@chromium.org Commiter: henryhsu@chromium.org Date: 2018-11-15 05:54:31 +0000 UTC Remove unnecessary DCHECK in stream buffer manager SetUpStreamsAndBuffers may be called twice when enabling blob stream. For the second call, kPreview stream already exists. Also reset repeating_request_settings_ in StopPreview. BUG= chromium:900900 TEST=./capture_unittest --gtest_filter="*VideoCaptureDeviceTests/*" Pass TakePhoto cases with DCHECK enabled. TBR=henryhsu@chromium.org (cherry picked from commit 678fc40f10bb52e86d53f9f49ce936ecf98162b0) Change-Id: Iac89964b95e7d14f592c779f11a92ef46dba930e Reviewed-on: https://chromium-review.googlesource.com/c/1332974 Reviewed-by: Ricky Liang <jcliang@chromium.org> Commit-Queue: Heng-ruey Hsu <henryhsu@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#607943} Reviewed-on: https://chromium-review.googlesource.com/c/1337218 Reviewed-by: Heng-ruey Hsu <henryhsu@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#689} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by henryhsu@chromium.org
, Nov 2