Prevent duplicate buffer indices
Track which buffer indices are in use to prevent duplicates.
BUG: 31831808
TEST: Test app runs, enqueue errors no longer showing up in logs.
Change-Id: I319f9a1906170c861e194e357bdb57fab81d8c34
diff --git a/modules/camera/3_4/v4l2_wrapper.cpp b/modules/camera/3_4/v4l2_wrapper.cpp
index 40db4f6..a0c5f04 100644
--- a/modules/camera/3_4/v4l2_wrapper.cpp
+++ b/modules/camera/3_4/v4l2_wrapper.cpp
@@ -56,7 +56,6 @@
std::unique_ptr<V4L2Gralloc> gralloc)
: device_path_(std::move(device_path)),
gralloc_(std::move(gralloc)),
- max_buffers_(0),
connection_count_(0) {
HAL_LOG_ENTER();
}
@@ -117,7 +116,7 @@
device_fd_.reset(); // Includes close().
format_.reset();
- max_buffers_ = 0;
+ buffers_.clear();
// Closing the device releases all queued buffers back to the user.
gralloc_->unlockAllBuffers();
}
@@ -493,7 +492,7 @@
HAL_LOGE("Failed to set up buffers for new format.");
return res;
}
- *result_max_buffers = max_buffers_;
+ *result_max_buffers = buffers_.size();
return 0;
}
@@ -526,12 +525,12 @@
}
// V4L2 will set req_buffers.count to a number of buffers it can handle.
- max_buffers_ = req_buffers.count;
- // Sanity check.
- if (max_buffers_ < 1) {
+ if (req_buffers.count < 1) {
HAL_LOGE("REQBUFS claims it can't handle any buffers.");
return -ENODEV;
}
+ buffers_.resize(req_buffers.count, false);
+
return 0;
}
@@ -543,14 +542,31 @@
return -ENODEV;
}
+ // Find a free buffer index. Could use some sort of persistent hinting
+ // here to improve expected efficiency, but buffers_.size() is expected
+ // to be low enough (<10 experimentally) that it's not worth it.
+ int index = -1;
+ {
+ std::lock_guard<std::mutex> guard(buffer_queue_lock_);
+ for (int i = 0; i < buffers_.size(); ++i) {
+ if (!buffers_[i]) {
+ index = i;
+ break;
+ }
+ }
+ }
+ if (index < 0) {
+ // Note: The HAL should be tracking the number of buffers in flight
+ // for each stream, and should never overflow the device.
+ HAL_LOGE("Cannot enqueue buffer: stream is already full.");
+ return -ENODEV;
+ }
+
// Set up a v4l2 buffer struct.
v4l2_buffer device_buffer;
memset(&device_buffer, 0, sizeof(device_buffer));
device_buffer.type = format_->type();
- // TODO(b/29334616): when this is async, actually limit the number
- // of buffers used to the known max, and set this according to the
- // queue length.
- device_buffer.index = 0;
+ device_buffer.index = index;
// Use QUERYBUF to ensure our buffer/device is in good shape,
// and fill out remaining fields.
@@ -572,6 +588,10 @@
return -ENODEV;
}
+ // Mark the buffer as in flight.
+ std::lock_guard<std::mutex> guard(buffer_queue_lock_);
+ buffers_[index] = true;
+
return 0;
}
@@ -591,6 +611,12 @@
return -ENODEV;
}
+ // Mark the buffer as no longer in flight.
+ {
+ std::lock_guard<std::mutex> guard(buffer_queue_lock_);
+ buffers_[buffer->index] = false;
+ }
+
// Now that we're done painting the buffer, we can unlock it.
int res = gralloc_->unlock(buffer);
if (res) {
diff --git a/modules/camera/3_4/v4l2_wrapper.h b/modules/camera/3_4/v4l2_wrapper.h
index 3798c14..f25f80e 100644
--- a/modules/camera/3_4/v4l2_wrapper.h
+++ b/modules/camera/3_4/v4l2_wrapper.h
@@ -22,6 +22,7 @@
#include <mutex>
#include <set>
#include <string>
+#include <vector>
#include <nativehelper/ScopedFd.h>
@@ -107,8 +108,12 @@
bool extended_query_supported_;
// The format this device is set up for.
std::unique_ptr<StreamFormat> format_;
- // The maximum number of buffers this device can handle in its current format.
- uint32_t max_buffers_;
+ // Map indecies to buffer status. True if the index is in-flight.
+ // |buffers_.size()| will always be the maximum number of buffers this device
+ // can handle in its current format.
+ std::vector<bool> buffers_;
+ // Lock protecting use of the buffer tracker.
+ std::mutex buffer_queue_lock_;
// Lock protecting use of the device.
std::mutex device_lock_;
// Lock protecting connecting/disconnecting the device.