Commit fff73016 authored by Findit's avatar Findit

Revert "Enable camera blob stream when needed"

This reverts commit 10f4b936.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 601492 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzEwZjRiOTM2MzVlMTJmOWZhMGNiYTE2NDFhMTA5MzhjYTM4ZWQ0NDgM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20ChromiumOS%20MSan%20Tests/9190

Sample Failed Step: capture_unittests

Original change's description:
> Enable camera blob stream when needed
> 
> Since blob stream needs higher resolution, it causes higher cpu loading
> to require higher resolution and resize to smaller resolution.
> In hangout app, we don't need blob stream. Enabling blob stream when
> needed can save a lot of cpu usage.
> 
> BUG=b:114676133
> TEST=manually test in apprtc and CCA. make sure picture taking still
> works in CCA.
> 
> Change-Id: I9144461bc76627903d0b3b359ce9cf962ff3628c
> Reviewed-on: https://chromium-review.googlesource.com/c/1261242
> Commit-Queue: Heng-ruey Hsu <henryhsu@chromium.org>
> Reviewed-by: Ricky Liang <jcliang@chromium.org>
> Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#601492}

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
BUG=b:114676133

Change-Id: If173ffe9259f7eca849b184806bd56e2a9fbaac4
Reviewed-on: https://chromium-review.googlesource.com/c/1292256
Cr-Commit-Position: refs/heads/master@{#601538}
parent 3b62417e
......@@ -205,8 +205,7 @@ enum VideoCaptureError {
kMacDeckLinkCouldNotStartCapturing,
kMacDeckLinkUnsupportedPixelFormat,
kMacAvFoundationReceivedAVCaptureSessionRuntimeErrorNotification,
kAndroidApi2ErrorConfiguringCamera,
kCrosHalV3DeviceDelegateFailedToFlush
kAndroidApi2ErrorConfiguringCamera
};
enum VideoCaptureFrameDropReason {
......
......@@ -658,9 +658,6 @@ EnumTraits<media::mojom::VideoCaptureError, media::VideoCaptureError>::ToMojom(
case media::VideoCaptureError::kAndroidApi2ErrorConfiguringCamera:
return media::mojom::VideoCaptureError::
kAndroidApi2ErrorConfiguringCamera;
case media::VideoCaptureError::kCrosHalV3DeviceDelegateFailedToFlush:
return media::mojom::VideoCaptureError::
kCrosHalV3DeviceDelegateFailedToFlush;
}
NOTREACHED();
return media::mojom::VideoCaptureError::kNone;
......@@ -1179,9 +1176,6 @@ bool EnumTraits<media::mojom::VideoCaptureError, media::VideoCaptureError>::
case media::mojom::VideoCaptureError::kAndroidApi2ErrorConfiguringCamera:
*output = media::VideoCaptureError::kAndroidApi2ErrorConfiguringCamera;
return true;
case media::mojom::VideoCaptureError::kCrosHalV3DeviceDelegateFailedToFlush:
*output = media::VideoCaptureError::kCrosHalV3DeviceDelegateFailedToFlush;
return true;
}
NOTREACHED();
return false;
......
......@@ -133,12 +133,6 @@ class CameraDeviceDelegate::StreamCaptureInterfaceImpl final
}
}
void Flush(base::OnceCallback<void(int32_t)> callback) final {
if (camera_device_delegate_) {
camera_device_delegate_->Flush(std::move(callback));
}
}
private:
const base::WeakPtr<CameraDeviceDelegate> camera_device_delegate_;
};
......@@ -195,7 +189,7 @@ void CameraDeviceDelegate::StopAndDeAllocate(
// The device delegate is in the process of opening the camera device.
return;
}
stream_buffer_manager_->StopPreview(base::NullCallback());
stream_buffer_manager_->StopPreview();
device_ops_->Close(
base::BindOnce(&CameraDeviceDelegate::OnClosed, GetWeakPtr()));
}
......@@ -232,17 +226,18 @@ void CameraDeviceDelegate::GetPhotoState(
return;
}
int32_t max_blob_width = 0, max_blob_height = 0;
GetMaxBlobStreamResolution(static_metadata_, &max_blob_width,
&max_blob_height);
photo_state->width->current = max_blob_width;
photo_state->width->min = max_blob_width;
photo_state->width->max = max_blob_width;
photo_state->width->step = 0.0;
photo_state->height->current = max_blob_height;
photo_state->height->min = max_blob_height;
photo_state->height->max = max_blob_height;
photo_state->height->step = 0.0;
auto stream_config =
stream_buffer_manager_->GetStreamConfiguration(StreamType::kStillCapture);
if (stream_config) {
photo_state->width->current = stream_config->width;
photo_state->width->min = stream_config->width;
photo_state->width->max = stream_config->width;
photo_state->width->step = 0.0;
photo_state->height->current = stream_config->height;
photo_state->height->min = stream_config->height;
photo_state->height->max = stream_config->height;
photo_state->height->step = 0.0;
}
std::move(callback).Run(std::move(photo_state));
}
......@@ -263,14 +258,7 @@ void CameraDeviceDelegate::SetPhotoOptions(
return;
}
if (stream_buffer_manager_->GetStreamNumber() < kMaxConfiguredStreams) {
stream_buffer_manager_->StopPreview(
base::BindOnce(&CameraDeviceDelegate::OnFlushed, GetWeakPtr()));
set_photo_option_callback_ = std::move(callback);
} else {
set_photo_option_callback_.Reset();
std::move(callback).Run(true);
}
std::move(callback).Run(true);
}
void CameraDeviceDelegate::SetRotation(int rotation) {
......@@ -294,7 +282,7 @@ void CameraDeviceDelegate::OnMojoConnectionError() {
} else {
// The Mojo channel terminated unexpectedly.
if (stream_buffer_manager_) {
stream_buffer_manager_->StopPreview(base::NullCallback());
stream_buffer_manager_->StopPreview();
}
device_context_->SetState(CameraDeviceContext::State::kStopped);
device_context_->SetErrorState(
......@@ -307,19 +295,6 @@ void CameraDeviceDelegate::OnMojoConnectionError() {
}
}
void CameraDeviceDelegate::OnFlushed(int32_t result) {
DCHECK(ipc_task_runner_->BelongsToCurrentThread());
if (result) {
device_context_->SetErrorState(
media::VideoCaptureError::kCrosHalV3DeviceDelegateFailedToFlush,
FROM_HERE,
std::string("Flush failed: ") + base::safe_strerror(-result));
return;
}
device_context_->SetState(CameraDeviceContext::State::kInitialized);
ConfigureStreams(true);
}
void CameraDeviceDelegate::OnClosed(int32_t result) {
DCHECK(ipc_task_runner_->BelongsToCurrentThread());
DCHECK_EQ(device_context_->GetState(), CameraDeviceContext::State::kStopping);
......@@ -451,10 +426,10 @@ void CameraDeviceDelegate::OnInitialized(int32_t result) {
return;
}
device_context_->SetState(CameraDeviceContext::State::kInitialized);
ConfigureStreams(false);
ConfigureStreams();
}
void CameraDeviceDelegate::ConfigureStreams(bool require_photo) {
void CameraDeviceDelegate::ConfigureStreams() {
DCHECK(ipc_task_runner_->BelongsToCurrentThread());
DCHECK_EQ(device_context_->GetState(),
CameraDeviceContext::State::kInitialized);
......@@ -475,34 +450,31 @@ void CameraDeviceDelegate::ConfigureStreams(bool require_photo) {
preview_stream->rotation =
cros::mojom::Camera3StreamRotation::CAMERA3_STREAM_ROTATION_0;
cros::mojom::Camera3StreamConfigurationPtr stream_config =
cros::mojom::Camera3StreamConfiguration::New();
stream_config->streams.push_back(std::move(preview_stream));
// Set up context for still capture stream. We set still capture stream to the
// JPEG stream configuration with maximum supported resolution.
// TODO(jcliang): Once we support SetPhotoOptions() the still capture stream
// should be configured dynamically per the photo options.
if (require_photo) {
int32_t max_blob_width = 0, max_blob_height = 0;
GetMaxBlobStreamResolution(static_metadata_, &max_blob_width,
&max_blob_height);
cros::mojom::Camera3StreamPtr still_capture_stream =
cros::mojom::Camera3Stream::New();
still_capture_stream->id = static_cast<uint64_t>(StreamType::kStillCapture);
still_capture_stream->stream_type =
cros::mojom::Camera3StreamType::CAMERA3_STREAM_OUTPUT;
still_capture_stream->width = max_blob_width;
still_capture_stream->height = max_blob_height;
still_capture_stream->format =
cros::mojom::HalPixelFormat::HAL_PIXEL_FORMAT_BLOB;
still_capture_stream->data_space = 0;
still_capture_stream->rotation =
cros::mojom::Camera3StreamRotation::CAMERA3_STREAM_ROTATION_0;
stream_config->streams.push_back(std::move(still_capture_stream));
}
int32_t max_blob_width = 0, max_blob_height = 0;
GetMaxBlobStreamResolution(static_metadata_, &max_blob_width,
&max_blob_height);
cros::mojom::Camera3StreamPtr still_capture_stream =
cros::mojom::Camera3Stream::New();
still_capture_stream->id = static_cast<uint64_t>(StreamType::kStillCapture);
still_capture_stream->stream_type =
cros::mojom::Camera3StreamType::CAMERA3_STREAM_OUTPUT;
still_capture_stream->width = max_blob_width;
still_capture_stream->height = max_blob_height;
still_capture_stream->format =
cros::mojom::HalPixelFormat::HAL_PIXEL_FORMAT_BLOB;
still_capture_stream->data_space = 0;
still_capture_stream->rotation =
cros::mojom::Camera3StreamRotation::CAMERA3_STREAM_ROTATION_0;
cros::mojom::Camera3StreamConfigurationPtr stream_config =
cros::mojom::Camera3StreamConfiguration::New();
stream_config->streams.push_back(std::move(preview_stream));
stream_config->streams.push_back(std::move(still_capture_stream));
stream_config->operation_mode = cros::mojom::Camera3StreamConfigurationMode::
CAMERA3_STREAM_CONFIGURATION_NORMAL_MODE;
device_ops_->ConfigureStreams(
......@@ -530,8 +502,7 @@ void CameraDeviceDelegate::OnConfiguredStreams(
return;
}
if (!updated_config ||
updated_config->streams.size() > kMaxConfiguredStreams ||
updated_config->streams.size() < 1) {
updated_config->streams.size() != kMaxConfiguredStreams) {
device_context_->SetErrorState(
media::VideoCaptureError::
kCrosHalV3DeviceDelegateWrongNumberOfStreamsConfigured,
......@@ -598,10 +569,6 @@ void CameraDeviceDelegate::OnConstructedDefaultPreviewRequestSettings(
base::BindOnce(&CameraDeviceDelegate::ConstructDefaultRequestSettings,
GetWeakPtr(), StreamType::kStillCapture));
}
if (set_photo_option_callback_) {
std::move(set_photo_option_callback_).Run(true);
}
}
void CameraDeviceDelegate::OnConstructedDefaultStillCaptureRequestSettings(
......@@ -666,11 +633,6 @@ void CameraDeviceDelegate::ProcessCaptureRequest(
device_ops_->ProcessCaptureRequest(std::move(request), std::move(callback));
}
void CameraDeviceDelegate::Flush(base::OnceCallback<void(int32_t)> callback) {
DCHECK(ipc_task_runner_->BelongsToCurrentThread());
device_ops_->Flush(std::move(callback));
}
bool CameraDeviceDelegate::SetPointsOfInterest(
const std::vector<mojom::Point2DPtr>& points_of_interest) {
if (points_of_interest.empty()) {
......
......@@ -60,9 +60,6 @@ class CAPTURE_EXPORT StreamCaptureInterface {
virtual void ProcessCaptureRequest(
cros::mojom::Camera3CaptureRequestPtr request,
base::OnceCallback<void(int32_t)> callback) = 0;
// Send flush to cancel all pending requests to the camera HAL.
virtual void Flush(base::OnceCallback<void(int32_t)> callback) = 0;
};
// CameraDeviceDelegate is instantiated on the capture thread where
......@@ -102,9 +99,6 @@ class CAPTURE_EXPORT CameraDeviceDelegate final {
// Mojo connection error handler.
void OnMojoConnectionError();
// Reconfigure streams for picture taking.
void OnFlushed(int32_t result);
// Callback method for the Close Mojo IPC call. This method resets the Mojo
// connection and closes the camera device.
void OnClosed(int32_t result);
......@@ -130,7 +124,7 @@ class CAPTURE_EXPORT CameraDeviceDelegate final {
// indicates. If there's no error OnConfiguredStreams notifies
// |client_| the capture has started by calling OnStarted, and proceeds to
// ConstructDefaultRequestSettings.
void ConfigureStreams(bool require_photo);
void ConfigureStreams();
void OnConfiguredStreams(
int32_t result,
cros::mojom::Camera3StreamConfigurationPtr updated_config);
......@@ -161,7 +155,6 @@ class CAPTURE_EXPORT CameraDeviceDelegate final {
base::OnceCallback<void(int32_t)> callback);
void ProcessCaptureRequest(cros::mojom::Camera3CaptureRequestPtr request,
base::OnceCallback<void(int32_t)> callback);
void Flush(base::OnceCallback<void(int32_t)> callback);
bool SetPointsOfInterest(
const std::vector<mojom::Point2DPtr>& points_of_interest);
......@@ -194,8 +187,6 @@ class CAPTURE_EXPORT CameraDeviceDelegate final {
base::OnceClosure device_close_callback_;
VideoCaptureDevice::SetPhotoOptionsCallback set_photo_option_callback_;
base::WeakPtrFactory<CameraDeviceDelegate> weak_ptr_factory_;
DISALLOW_IMPLICIT_CONSTRUCTORS(CameraDeviceDelegate);
......
......@@ -242,13 +242,11 @@ class CameraDeviceDelegateTest : public ::testing::Test {
base::OnceCallback<void(int32_t,
cros::mojom::Camera3StreamConfigurationPtr)>&
callback) {
ASSERT_GE(2u, config->streams.size());
ASSERT_LT(0u, config->streams.size());
ASSERT_EQ(2u, config->streams.size());
for (size_t i = 0; i < config->streams.size(); ++i) {
config->streams[i]->usage = 0;
config->streams[i]->max_buffers = 1;
}
num_streams_ = config->streams.size();
std::move(callback).Run(0, std::move(config));
}
......@@ -334,16 +332,14 @@ class CameraDeviceDelegateTest : public ::testing::Test {
.Times(1)
.WillOnce(Invoke(&unittest_internal::MockGpuMemoryBufferManager::
CreateFakeGpuMemoryBuffer));
if (num_streams_ == 2) {
EXPECT_CALL(
mock_gpu_memory_buffer_manager_,
CreateGpuMemoryBuffer(_, gfx::BufferFormat::R_8,
gfx::BufferUsage::CAMERA_AND_CPU_READ_WRITE,
gpu::kNullSurfaceHandle))
.Times(1)
.WillOnce(Invoke(&unittest_internal::MockGpuMemoryBufferManager::
CreateFakeGpuMemoryBuffer));
}
EXPECT_CALL(
mock_gpu_memory_buffer_manager_,
CreateGpuMemoryBuffer(_, gfx::BufferFormat::R_8,
gfx::BufferUsage::CAMERA_AND_CPU_READ_WRITE,
gpu::kNullSurfaceHandle))
.Times(1)
.WillOnce(Invoke(&unittest_internal::MockGpuMemoryBufferManager::
CreateFakeGpuMemoryBuffer));
EXPECT_CALL(
mock_gpu_memory_buffer_manager_,
CreateGpuMemoryBuffer(gfx::Size(kDefaultWidth, kDefaultHeight),
......@@ -353,16 +349,14 @@ class CameraDeviceDelegateTest : public ::testing::Test {
.Times(1)
.WillOnce(Invoke(&unittest_internal::MockGpuMemoryBufferManager::
CreateFakeGpuMemoryBuffer));
if (num_streams_ == 2) {
EXPECT_CALL(mock_gpu_memory_buffer_manager_,
CreateGpuMemoryBuffer(
gfx::Size(kJpegMaxBufferSize, 1), gfx::BufferFormat::R_8,
gfx::BufferUsage::CAMERA_AND_CPU_READ_WRITE,
gpu::kNullSurfaceHandle))
.Times(1)
.WillOnce(Invoke(&unittest_internal::MockGpuMemoryBufferManager::
CreateFakeGpuMemoryBuffer));
}
EXPECT_CALL(mock_gpu_memory_buffer_manager_,
CreateGpuMemoryBuffer(
gfx::Size(kJpegMaxBufferSize, 1), gfx::BufferFormat::R_8,
gfx::BufferUsage::CAMERA_AND_CPU_READ_WRITE,
gpu::kNullSurfaceHandle))
.Times(1)
.WillOnce(Invoke(&unittest_internal::MockGpuMemoryBufferManager::
CreateFakeGpuMemoryBuffer));
}
void SetUpExpectationUntilCapturing(
......@@ -428,7 +422,6 @@ class CameraDeviceDelegateTest : public ::testing::Test {
ASSERT_TRUE(camera_device_delegate_);
device_delegate_thread_.Stop();
camera_device_delegate_.reset();
num_streams_ = 0;
}
void DoLoop() {
......@@ -468,8 +461,6 @@ class CameraDeviceDelegateTest : public ::testing::Test {
std::unique_ptr<CameraDeviceContext> device_context_;
size_t num_streams_;
private:
base::test::ScopedTaskEnvironment scoped_task_environment_;
base::Thread hal_delegate_thread_;
......@@ -612,7 +603,7 @@ TEST_F(CameraDeviceDelegateTest, FailToOpenDevice) {
};
EXPECT_CALL(*mock_client, OnError(_, _, _))
.Times(AtLeast(1))
.WillRepeatedly(InvokeWithoutArgs(stop_on_error));
.WillOnce(InvokeWithoutArgs(stop_on_error));
EXPECT_CALL(mock_camera_module_, DoGetCameraInfo(0, _))
.Times(1)
......
......@@ -187,13 +187,9 @@ void StreamBufferManager::StartPreview(
RegisterBuffer(StreamType::kPreview);
}
void StreamBufferManager::StopPreview(
base::OnceCallback<void(int32_t)> callback) {
void StreamBufferManager::StopPreview() {
DCHECK(ipc_task_runner_->BelongsToCurrentThread());
capturing_ = false;
if (callback) {
capture_interface_->Flush(std::move(callback));
}
}
cros::mojom::Camera3StreamPtr StreamBufferManager::GetStreamConfiguration(
......@@ -227,10 +223,6 @@ void StreamBufferManager::TakePhoto(
RegisterBuffer(StreamType::kStillCapture);
}
size_t StreamBufferManager::GetStreamNumber() {
return stream_context_.size();
}
void StreamBufferManager::AddResultMetadataObserver(
ResultMetadataObserver* observer) {
DCHECK(ipc_task_runner_->BelongsToCurrentThread());
......@@ -424,6 +416,7 @@ void StreamBufferManager::OnRegisteredBuffer(StreamType stream_type,
void StreamBufferManager::ProcessCaptureRequest() {
DCHECK(ipc_task_runner_->BelongsToCurrentThread());
DCHECK(stream_context_[StreamType::kPreview]);
DCHECK(stream_context_[StreamType::kStillCapture]);
cros::mojom::Camera3CaptureRequestPtr request =
cros::mojom::Camera3CaptureRequest::New();
......@@ -446,8 +439,7 @@ void StreamBufferManager::ProcessCaptureRequest() {
request->output_buffers.push_back(std::move(buffer));
}
if (stream_context_.count(StreamType::kStillCapture) &&
!stream_context_[StreamType::kStillCapture]->registered_buffers.empty()) {
if (!stream_context_[StreamType::kStillCapture]->registered_buffers.empty()) {
DCHECK(!still_capture_callbacks_currently_processing_.empty());
cros::mojom::Camera3StreamBufferPtr buffer =
cros::mojom::Camera3StreamBuffer::New();
......
......@@ -116,15 +116,13 @@ class CAPTURE_EXPORT StreamBufferManager final
// Stops the capture loop. After StopPreview is called |callback_ops_| is
// unbound, so no new capture request or result will be processed.
void StopPreview(base::OnceCallback<void(int32_t)> callback);
void StopPreview();
cros::mojom::Camera3StreamPtr GetStreamConfiguration(StreamType stream_type);
void TakePhoto(cros::mojom::CameraMetadataPtr settings,
VideoCaptureDevice::TakePhotoCallback callback);
size_t GetStreamNumber();
// CaptureMetadataDispatcher implementations.
void AddResultMetadataObserver(ResultMetadataObserver* observer) override;
void RemoveResultMetadataObserver(ResultMetadataObserver* observer) override;
......
......@@ -63,9 +63,6 @@ class MockStreamCaptureInterface : public StreamCaptureInterface {
MOCK_METHOD2(DoProcessCaptureRequest,
void(cros::mojom::Camera3CaptureRequestPtr& request,
base::OnceCallback<void(int32_t)>& callback));
void Flush(base::OnceCallback<void(int32_t)> callback) { DoFlush(callback); }
MOCK_METHOD1(DoFlush, void(base::OnceCallback<void(int32_t)>& callback));
};
const VideoCaptureFormat kDefaultCaptureFormat(gfx::Size(1280, 720),
......
......@@ -178,8 +178,7 @@ enum class VideoCaptureError {
kMacDeckLinkUnsupportedPixelFormat = 112,
kMacAvFoundationReceivedAVCaptureSessionRuntimeErrorNotification = 113,
kAndroidApi2ErrorConfiguringCamera = 114,
kCrosHalV3DeviceDelegateFailedToFlush = 115,
kMaxValue = 115
kMaxValue = 114
};
// WARNING: Do not change the values assigned to the entries. They are used for
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment