tequilaOS/platform_hardware_libhardware
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Implement flushing and improve request tracking.

Browse source 
BUG: https://b/29937783, https://b/32941326
TEST: unit tests pass, fixes 8 failing cts tests
Change-Id: Idc390bdc1e51ba2bdbda91d90acc459a09a5f174
This commit is contained in:
Ari Hausman-Cohen 2016-11-16 10:53:52 -08:00
parent ad6fe2b033
commit c5a4852451
10 changed files with 204 additions and 93 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
modules/camera/3_4/camera.cpp
View file

@ -163,6 +163,7 @@ int Camera::close()
}
disconnect();
flush();
mBusy = false;
return 0;
}
@ -393,6 +394,9 @@ const camera_metadata_t* Camera::constructDefaultRequestSettings(int type)
int Camera::processCaptureRequest(camera3_capture_request_t *temp_request)
{
int res;
// TODO(b/32917568): A capture request submitted or ongoing during a flush
// should be returned with an error; for now they are mutually exclusive.
android::Mutex::Autolock al(mFlushLock);
ATRACE_CALL();
@ -460,7 +464,8 @@ int Camera::processCaptureRequest(camera3_capture_request_t *temp_request)
void Camera::completeRequest(std::shared_ptr<CaptureRequest> request, int err)
{
if (!mInFlightTracker->Remove(request)) {
ALOGE("%s:%d: Completed request %p is not being tracked.",
ALOGE("%s:%d: Completed request %p is not being tracked. "
"It may have been cleared out during a flush.",
__func__, mId, request.get());
return;
}
@ -498,6 +503,30 @@ void Camera::completeRequest(std::shared_ptr<CaptureRequest> request, int err)
sendResult(request);
}
int Camera::flush()
{
ALOGV("%s:%d: Flushing.", __func__, mId);
// TODO(b/32917568): Synchronization. Behave "appropriately"
// (i.e. according to camera3.h) if process_capture_request()
// is called concurrently with this (in either order).
// Since the callback to completeRequest also may happen on a separate
// thread, this function should behave nicely concurrently with that too.
android::Mutex::Autolock al(mFlushLock);
std::set<std::shared_ptr<CaptureRequest>> requests;
mInFlightTracker->Clear(&requests);
for (auto& request : requests) {
// TODO(b/31653322): See camera3.h. Should return different error
// depending on status of the request.
completeRequestWithError(request);
}
ALOGV("%s:%d: Flushed %u requests.", __func__, mId, requests.size());
// Call down into the device flushing.
return flushBuffers();
}
int Camera::preprocessCaptureBuffer(camera3_stream_buffer_t *buffer)
{
int res;
@ -638,11 +667,9 @@ static void dump(const camera3_device_t *dev, int fd)
camdev_to_camera(dev)->dump(fd);
}
static int flush(const camera3_device_t*)
static int flush(const camera3_device_t *dev)
{
// TODO(b/29937783)
ALOGE("%s: unimplemented.", __func__);
return -1;
return camdev_to_camera(dev)->flush();
}
} // extern "C"

6
modules/camera/3_4/camera.h
View file

@ -55,7 +55,7 @@ class Camera {
const camera_metadata_t *constructDefaultRequestSettings(int type);
int processCaptureRequest(camera3_capture_request_t *temp_request);
void dump(int fd);
int flush();
protected:
// Connect to the device: open dev nodes, etc.
@ -82,6 +82,9 @@ class Camera {
// Enqueue a request to receive data from the camera
virtual int enqueueRequest(
std::shared_ptr<CaptureRequest> request) = 0;
// Flush in flight buffers.
virtual int flushBuffers() = 0;
// Callback for when the device has filled in the requested data.
// Fills in the result struct, validates the data, sends appropriate
@ -133,6 +136,7 @@ class Camera {
// Lock protecting only static camera characteristics, which may
// be accessed without the camera device open
android::Mutex mStaticInfoLock;
android::Mutex mFlushLock;
// Array of handles to streams currently in use by the device
Stream **mStreams;
// Number of streams in mStreams

12
modules/camera/3_4/request_tracker.cpp
View file

@ -70,8 +70,13 @@ bool RequestTracker::Add(std::shared_ptr<CaptureRequest> request) {
}
bool RequestTracker::Remove(std::shared_ptr<CaptureRequest> request) {
if (!request) {
return false;
}
// Get the request.
const auto frame_number_request = frames_in_flight_.find(request->frame_number);
const auto frame_number_request =
frames_in_flight_.find(request->frame_number);
if (frame_number_request == frames_in_flight_.end()) {
ALOGE("%s: Frame %u is not in flight.", __func__, request->frame_number);
return false;
@ -105,7 +110,10 @@ void RequestTracker::Clear(
// Clear out all tracking.
frames_in_flight_.clear();
buffers_in_flight_.clear();
// Maintain the configuration, but reset counts.
for (auto& stream_count : buffers_in_flight_) {
stream_count.second = 0;
}
}
bool RequestTracker::CanAddRequest(const CaptureRequest& request) const {

3
modules/camera/3_4/request_tracker_test.cpp
View file

@ -230,6 +230,9 @@ TEST_F(RequestTrackerTest, ClearRequests) {
dut_->Clear(&actual);
EXPECT_TRUE(dut_->Empty());
EXPECT_EQ(actual, expected);
// Configuration (max values) should not have been cleared.
EXPECT_TRUE(dut_->Add(request1));
}
TEST_F(RequestTrackerTest, ClearRequestsNoResult) {

122
modules/camera/3_4/v4l2_camera.cpp
View file

@ -121,6 +121,20 @@ void V4L2Camera::disconnect() {
// because this camera is no longer in use.
}
int V4L2Camera::flushBuffers() {
HAL_LOG_ENTER();
int res = device_->StreamOff();
// This is not strictly necessary, but prevents a buildup of aborted
// requests in the in_flight_ map. These should be cleared
// whenever the stream is turned off.
std::lock_guard<std::mutex> guard(in_flight_lock_);
in_flight_.clear();
return res;
}
int V4L2Camera::initStaticInfo(android::CameraMetadata* out) {
HAL_LOG_ENTER();
@ -191,12 +205,11 @@ int V4L2Camera::enqueueRequest(
// Assume request validated before calling this function.
// (For now, always exactly 1 output buffer, no inputs).
{
std::lock_guard<std::mutex> guard(request_queue_lock_);
request_queue_.push(request);
requests_available_.notify_one();
}
requests_available_.notify_one();
return 0;
}
@ -235,68 +248,80 @@ bool V4L2Camera::enqueueRequestBuffers() {
return true;
}
// Actually enqueue the buffer for capture.
res = device_->EnqueueBuffer(&request->output_buffers[0]);
if (res) {
HAL_LOGE("Device failed to enqueue buffer.");
completeRequest(request, res);
return true;
}
// Replace the requested settings with a snapshot of
// the used settings/state immediately after enqueue.
// the used settings/state immediately before enqueue.
res = metadata_->FillResultMetadata(&request->settings);
if (res) {
// Note: since request is a shared pointer, this may happen if another
// thread has already decided to complete the request (e.g. via flushing),
// since that locks the metadata (in that case, this failing is fine,
// and completeRequest will simply do nothing).
HAL_LOGE("Failed to fill result metadata.");
completeRequest(request, res);
return true;
}
// Make sure the stream is on (no effect if already on).
res = device_->StreamOn();
if (res) {
HAL_LOGE("Device failed to turn on stream.");
completeRequest(request, res);
return true;
}
// Actually enqueue the buffer for capture.
{
std::lock_guard<std::mutex> guard(in_flight_lock_);
in_flight_.push(request);
uint32_t index;
res = device_->EnqueueBuffer(&request->output_buffers[0], &index);
if (res) {
HAL_LOGE("Device failed to enqueue buffer.");
completeRequest(request, res);
return true;
}
// Make sure the stream is on (no effect if already on).
res = device_->StreamOn();
if (res) {
HAL_LOGE("Device failed to turn on stream.");
// Don't really want to send an error for only the request here,
// since this is a full device error.
// TODO: Should trigger full flush.
return true;
}
// Note: the request should be dequeued/flushed from the device
// before removal from in_flight_.
in_flight_.emplace(index, request);
buffers_in_flight_.notify_one();
}
buffers_in_flight_.notify_one();
return true;
}
bool V4L2Camera::dequeueRequestBuffers() {
std::unique_lock<std::mutex> lock(in_flight_lock_);
while (in_flight_.empty()) {
buffers_in_flight_.wait(lock);
}
std::shared_ptr<default_camera_hal::CaptureRequest> request =
in_flight_.front();
in_flight_.pop();
lock.unlock();
// Dequeue the buffer. For now, since each request has only 1 buffer,
// and the in_flight_lock_ is held while enqueueing and dequeuing
// from the device, just assume that the dequeued buffer corresponds
// to the dequeued request.
// TODO(b/31657008): in_flight_ should map buffer handles to requests;
// this consumer should just dequeue whatever it gets, then match
// that to a handle.
v4l2_buffer result_buffer;
int res = device_->DequeueBuffer(&result_buffer);
// Dequeue a buffer.
uint32_t result_index;
int res = device_->DequeueBuffer(&result_index);
if (res) {
HAL_LOGE("Device failed to dequeue buffer.");
completeRequest(request, res);
if (res == -EAGAIN) {
// EAGAIN just means nothing to dequeue right now.
// Wait until something is available before looping again.
std::unique_lock<std::mutex> lock(in_flight_lock_);
while (in_flight_.empty()) {
buffers_in_flight_.wait(lock);
}
} else {
HAL_LOGW("Device failed to dequeue buffer: %d", res);
}
return true;
}
completeRequest(request, 0);
// Find the associated request and complete it.
std::lock_guard<std::mutex> guard(in_flight_lock_);
auto index_request = in_flight_.find(result_index);
if (index_request != in_flight_.end()) {
completeRequest(index_request->second, 0);
in_flight_.erase(index_request);
} else {
HAL_LOGW(
"Dequeued non in-flight buffer index %d. "
"This buffer may have been flushed from the HAL but not the device.",
index_request->first);
}
return true;
}
@ -411,6 +436,14 @@ int V4L2Camera::setupStream(default_camera_hal::Stream* stream,
// claims it's underdocumented; the implementation lets the HAL overwrite it.
stream->setDataSpace(HAL_DATASPACE_V0_JFIF);
std::lock_guard<std::mutex> guard(in_flight_lock_);
// The framework should be enforcing this, but doesn't hurt to be safe.
if (!in_flight_.empty()) {
HAL_LOGE("Can't set device format while frames are in flight.");
return -EINVAL;
}
// Ensure the stream is off.
int res = device_->StreamOff();
if (res) {
HAL_LOGE("Device failed to turn off stream for reconfiguration.");
@ -422,6 +455,7 @@ int V4L2Camera::setupStream(default_camera_hal::Stream* stream,
HAL_LOGE("Failed to set device to correct format for stream.");
return res;
}
// Sanity check.
if (*max_buffers < 1) {
HAL_LOGE("Setting format resulted in an invalid maximum of %u buffers.",

7
modules/camera/3_4/v4l2_camera.h
View file

@ -21,6 +21,7 @@
#include <array>
#include <condition_variable>
#include <map>
#include <queue>
#include <string>
@ -80,6 +81,8 @@ class V4L2Camera : public default_camera_hal::Camera {
// Enqueue a request to receive data from the camera.
int enqueueRequest(
std::shared_ptr<default_camera_hal::CaptureRequest> request) override;
// Flush in flight buffers.
int flushBuffers() override;
// Async request processing helpers.
// Dequeue a request from the waiting queue.
@ -100,7 +103,9 @@ class V4L2Camera : public default_camera_hal::Camera {
std::queue<std::shared_ptr<default_camera_hal::CaptureRequest>>
request_queue_;
std::mutex in_flight_lock_;
std::queue<std::shared_ptr<default_camera_hal::CaptureRequest>> in_flight_;
// Maps buffer index : request.
std::map<uint32_t, std::shared_ptr<default_camera_hal::CaptureRequest>>
in_flight_;
// Threads require holding an Android strong pointer.
android::sp<android::Thread> buffer_enqueuer_;
android::sp<android::Thread> buffer_dequeuer_;

1
modules/camera/3_4/v4l2_gralloc.cpp
View file

@ -321,6 +321,7 @@ int V4L2Gralloc::unlockAllBuffers() {
// The BufferData entry is always dynamically allocated in lock().
delete entry.second;
}
mBufferMap.clear();
// If any unlock failed, return error.
if (failed) {

90
modules/camera/3_4/v4l2_wrapper.cpp
View file

@ -68,7 +68,8 @@ int V4L2Wrapper::Connect() {
return 0;
}
int fd = TEMP_FAILURE_RETRY(open(device_path_.c_str(), O_RDWR));
// Open in nonblocking mode (DQBUF may return EAGAIN).
int fd = TEMP_FAILURE_RETRY(open(device_path_.c_str(), O_RDWR | O_NONBLOCK));
if (fd < 0) {
HAL_LOGE("failed to open %s (%s)", device_path_.c_str(), strerror(errno));
return -ENODEV;
@ -155,6 +156,10 @@ int V4L2Wrapper::StreamOff() {
int res = IoctlLocked(VIDIOC_STREAMOFF, &type);
// Calling STREAMOFF releases all queued buffers back to the user.
int gralloc_res = gralloc_->unlockAllBuffers();
// No buffers in flight.
for (size_t i = 0; i < buffers_.size(); ++i) {
buffers_[i] = false;
}
if (res < 0) {
HAL_LOGE("STREAMOFF fails: %s", strerror(errno));
return -ENODEV;
@ -350,9 +355,7 @@ int V4L2Wrapper::GetFormatFrameSizes(uint32_t v4l2_format,
// Continuous/Step-wise: based on the stepwise struct returned by the query.
// Fully listing all possible sizes, with large enough range/small enough
// step size, may produce far too many potential sizes. Instead, find the
// closest to a set of standard sizes plus largest possible.
sizes->insert({{{static_cast<int32_t>(size_query.stepwise.max_width),
static_cast<int32_t>(size_query.stepwise.max_height)}}});
// closest to a set of standard sizes.
for (const auto size : kStandardSizes) {
// Find the closest size, rounding up.
uint32_t desired_width = size[0];
@ -453,11 +456,26 @@ int V4L2Wrapper::SetFormat(const default_camera_hal::Stream& stream,
return 0;
}
// Not in the correct format, set our format.
// Not in the correct format, set the new one.
if (format_) {
// If we had an old format, first request 0 buffers to inform the device
// we're no longer using any previously "allocated" buffers from the old
// format. This seems like it shouldn't be necessary for USERPTR memory,
// and/or should happen from turning the stream off, but the driver
// complained. May be a driver issue, or may be intended behavior.
int res = RequestBuffers(0);
if (res) {
return res;
}
}
// Set the camera to the new format.
v4l2_format new_format;
desired_format.FillFormatRequest(&new_format);
// TODO(b/29334616): When async, this will need to check if the stream
// is on, and if so, lock it off while setting format.
if (IoctlLocked(VIDIOC_S_FMT, &new_format) < 0) {
HAL_LOGE("S_FMT failed: %s", strerror(errno));
return -ENODEV;
@ -472,31 +490,22 @@ int V4L2Wrapper::SetFormat(const default_camera_hal::Stream& stream,
// Keep track of our new format.
format_.reset(new StreamFormat(new_format));
// Format changed, setup new buffers.
int res = SetupBuffers();
// Format changed, request new buffers.
int res = RequestBuffers(1);
if (res) {
HAL_LOGE("Failed to set up buffers for new format.");
HAL_LOGE("Requesting buffers for new format failed.");
return res;
}
*result_max_buffers = buffers_.size();
return 0;
}
int V4L2Wrapper::SetupBuffers() {
HAL_LOG_ENTER();
if (!format_) {
HAL_LOGE("Stream format must be set before setting up buffers.");
return -ENODEV;
}
// "Request" a buffer (since we're using a userspace buffer, this just
// tells V4L2 to switch into userspace buffer mode).
int V4L2Wrapper::RequestBuffers(uint32_t num_requested) {
v4l2_requestbuffers req_buffers;
memset(&req_buffers, 0, sizeof(req_buffers));
req_buffers.type = format_->type();
req_buffers.memory = V4L2_MEMORY_USERPTR;
req_buffers.count = 1;
req_buffers.count = num_requested;
int res = IoctlLocked(VIDIOC_REQBUFS, &req_buffers);
// Calling REQBUFS releases all queued buffers back to the user.
@ -511,7 +520,7 @@ int V4L2Wrapper::SetupBuffers() {
}
// V4L2 will set req_buffers.count to a number of buffers it can handle.
if (req_buffers.count < 1) {
if (num_requested > 0 && req_buffers.count < 1) {
HAL_LOGE("REQBUFS claims it can't handle any buffers.");
return -ENODEV;
}
@ -520,7 +529,8 @@ int V4L2Wrapper::SetupBuffers() {
return 0;
}
int V4L2Wrapper::EnqueueBuffer(const camera3_stream_buffer_t* camera_buffer) {
int V4L2Wrapper::EnqueueBuffer(const camera3_stream_buffer_t* camera_buffer,
uint32_t* enqueued_index) {
if (!format_) {
HAL_LOGE("Stream format must be set before enqueuing buffers.");
return -ENODEV;
@ -576,36 +586,52 @@ int V4L2Wrapper::EnqueueBuffer(const camera3_stream_buffer_t* camera_buffer) {
std::lock_guard<std::mutex> guard(buffer_queue_lock_);
buffers_[index] = true;
if (enqueued_index) {
*enqueued_index = index;
}
return 0;
}
int V4L2Wrapper::DequeueBuffer(v4l2_buffer* buffer) {
int V4L2Wrapper::DequeueBuffer(uint32_t* dequeued_index) {
if (!format_) {
HAL_LOGE("Stream format must be set before dequeueing buffers.");
return -ENODEV;
HAL_LOGV(
"Format not set, so stream can't be on, "
"so no buffers available for dequeueing");
return -EAGAIN;
}
memset(buffer, 0, sizeof(*buffer));
buffer->type = format_->type();
buffer->memory = V4L2_MEMORY_USERPTR;
if (IoctlLocked(VIDIOC_DQBUF, buffer) < 0) {
HAL_LOGE("DQBUF fails: %s", strerror(errno));
return -ENODEV;
v4l2_buffer buffer;
memset(&buffer, 0, sizeof(buffer));
buffer.type = format_->type();
buffer.memory = V4L2_MEMORY_USERPTR;
int res = IoctlLocked(VIDIOC_DQBUF, &buffer);
if (res) {
if (errno == EAGAIN) {
// Expected failure.
return -EAGAIN;
} else {
// Unexpected failure.
HAL_LOGE("DQBUF fails: %s", strerror(errno));
return -ENODEV;
}
}
// Mark the buffer as no longer in flight.
{
std::lock_guard<std::mutex> guard(buffer_queue_lock_);
buffers_[buffer->index] = false;
buffers_[buffer.index] = false;
}
// Now that we're done painting the buffer, we can unlock it.
int res = gralloc_->unlock(buffer);
res = gralloc_->unlock(&buffer);
if (res) {
HAL_LOGE("Gralloc failed to unlock buffer after dequeueing.");
return res;
}
if (dequeued_index) {
*dequeued_index = buffer.index;
}
return 0;
}

12
modules/camera/3_4/v4l2_wrapper.h
View file

@ -45,8 +45,9 @@ class V4L2Wrapper {
Connection(std::shared_ptr<V4L2Wrapper> device)
: device_(std::move(device)), connect_result_(device_->Connect()) {}
~Connection() {
if (connect_result_ == 0)
if (connect_result_ == 0) {
device_->Disconnect();
}
}
// Check whether the connection succeeded or not.
inline int status() const { return connect_result_; }
@ -77,8 +78,9 @@ class V4L2Wrapper {
virtual int SetFormat(const default_camera_hal::Stream& stream,
uint32_t* result_max_buffers);
// Manage buffers.
virtual int EnqueueBuffer(const camera3_stream_buffer_t* camera_buffer);
virtual int DequeueBuffer(v4l2_buffer* buffer);
virtual int EnqueueBuffer(const camera3_stream_buffer_t* camera_buffer,
uint32_t* enqueued_index = nullptr);
virtual int DequeueBuffer(uint32_t* dequeued_index = nullptr);
private:
// Constructor is private to allow failing on bad input.
@ -93,8 +95,8 @@ class V4L2Wrapper {
// Perform an ioctl call in a thread-safe fashion.
template <typename T>
int IoctlLocked(int request, T data);
// Adjust buffers any time a device is connected/reformatted.
int SetupBuffers();
// Request/release userspace buffer mode via VIDIOC_REQBUFS.
int RequestBuffers(uint32_t num_buffers);
inline bool connected() { return device_fd_.get() >= 0; }

7
modules/camera/3_4/v4l2_wrapper_mock.h
View file

@ -45,9 +45,10 @@ class V4L2WrapperMock : public V4L2Wrapper {
MOCK_METHOD2(SetFormat,
int(const default_camera_hal::Stream& stream,
uint32_t* result_max_buffers));
MOCK_METHOD1(EnqueueBuffer,
int(const camera3_stream_buffer_t* camera_buffer));
MOCK_METHOD1(DequeueBuffer, int(v4l2_buffer* buffer));
MOCK_METHOD2(EnqueueBuffer,
int(const camera3_stream_buffer_t* camera_buffer,
uint32_t* enqueued_index));
MOCK_METHOD1(DequeueBuffer, int(uint32_t* dequeued_index));
};
} // namespace v4l2_camera_hal