EVS HAL with multi-buffer support via BufferDesc
HAL changes required in order to enable required functionality.
Specifically, support for use of supplied buffers as OpenGL textures.
Addition of getDisplayState is needed in order to meet original API
spec (allow secondary clients to monitor display state). Expected to be
needed specifically by Car Service. Note that this HAL is not used on
the phone and only relevant to the Android Auto at this point.
Test: compile & run evs_test. Still issues with buffer locking.
Added getDisplayState() to IEvsEnumerator to allow secondary clients to
query for display state without having to acquire the display.
Introduced the use of weak pointers in the service to detect when
clients holding an object disappear unexpectedly.
Plus squashed commit of the following:
commit 26c685b430028384157f573b31264ff03ea4c37b
Author: Scott Randolph <randolphs@google.com>
Date: Thu Jan 19 17:29:54 2017 -0800
Convert EVS HAL to use BufferDesc (vs raw handles)
Use a structure that includes the buffer properties in addition to the
memory handle itself to make it easier/safer for the HAL client code to
reconstruct ANativeWindowBuffer objects (as required when using the
buffer as a GL texture, for example)
Change-Id: Ifebf0c03b9ad167d61152b85f8229d5970cc41af
commit 0286385694282e017c12f238f5aaa86d8d1bd600
Author: Scott Randolph <randolphs@google.com>
Date: Tue Jan 17 12:01:09 2017 -0800
Incremental cleanup of EVS driver
Change-Id: Ibba2b7d9a1c55629d76ee6faf2a9f36c715e7bd7
commit 71e7b85f3919a60c5a194028a586ce81846c6c0d
Author: Scott Randolph <randolphs@google.com>
Date: Thu Jan 12 10:40:45 2017 -0800
Fix error check bug that cause stream start to fail
Also fixes a minor warning related to log message string formatting.
Change-Id: I74edf9a02db87c3632b4883ca40207fe861fd75d
commit 164e7ef048eba9bc184319ea928d231a26b695bc
Author: Scott Randolph <randolphs@google.com>
Date: Wed Jan 11 17:27:46 2017 -0800
Fix HAL update related compile bugs
Minor changes to accomodate recent HIDL/HAL changes.
Change-Id: I466d8979e8e56424442840aff8d17762eac0a590
commit 67391db6a04b2cbbda1a0b5c8b717fe79994f5b9
Author: Scott Randolph <randolphs@google.com>
Date: Wed Jan 11 13:56:23 2017 -0800
Avoid mutex reentrancy when setting frame count
At initialization time, we need to be able to set the number of frames
available for capture. This change makes the code to do this a private
function called from within a mutex in order to avoid having one mutex
protected call trying to make another mutex protected call as would
happen if it called directly into the public API.
Change-Id: I05c383976dd8db1acf5874bd47b2a3c3521d7032
commit 993f6e7e1e43f2bc4a48f6aae32d9ba2c3125d23
Author: Scott Randolph <randolphs@google.com>
Date: Mon Dec 19 18:39:38 2016 -0800
Avoid forced exit if frame delivery fails
If we don't read the error code from the frame delivery call, the
underlying code forces an application exit if an error was returned.
Even when we don't care about the result, we have to read it to avoid
this.
Change-Id: Ie3695faa10d1c5e340f66c369bfc2595ac516434
commit 08298db0f2d5d0ae7cd0781fde834fc7e63375d0
Author: Scott Randolph <randolphs@google.com>
Date: Mon Dec 19 18:21:16 2016 -0800
Support adjustable number of frames in flight
Add enhanced support for SetMaxFramesInFlight which is required to
support concurrent camera clients at the EVS Manager level.
Change-Id: I23b606a017529514370a03cd2eca3ee286af8fec
commit 198050b2cff70d1a232745ce8e7f35e87581562e
Author: Scott Randolph <randolphs@google.com>
Date: Fri Dec 16 16:27:19 2016 -0800
Add frame contents validation to EvsDisplay
Update the default "mock" implementation of EvsDisplay hardware layer to
provide a frame contents validation based on knowledge of what the
"mock" EvsCamera implementation will emit.
Change-Id: If0624f855a440f52a47af7751ed3b09e8b21ff74
commit c830b7ca466155049d6eee047f6c323521451e8b
Author: Scott Randolph <randolphs@google.com>
Date: Fri Dec 16 15:25:48 2016 -0800
Add multi-buffer support to EvsCamera
Extend the EvsCamera implmentation to support more than one buffer in
flight at a time. This is necessary in order to support concurrent
client access through the EvsManager proxy layer.
Change-Id: I0c32336be40c7bedd797140569650906458d27c1
commit 1bdbb9d641f95d494ec2a398b6d1f5202276d17b
Author: Scott Randolph <randolphs@google.com>
Date: Fri Dec 16 15:24:28 2016 -0800
Add additional validation on round trip pointers
Now that HIDL support returning the same pointer to an interface object
after a round trip through the transport, we can add a bit more
validation.
Change-Id: Ia9a3a6ec7552f5e5523c0f9ad66ceb02b46b2db4
Change-Id: I47468932d6a7b2b5eab47819917cdb20154d8f18
diff --git a/evs/1.0/default/Android.bp b/evs/1.0/default/Android.bp
index 3bda250..e3bff25 100644
--- a/evs/1.0/default/Android.bp
+++ b/evs/1.0/default/Android.bp
@@ -11,7 +11,6 @@
shared_libs: [
"android.hardware.evs@1.0",
- "android.hardware.graphics.allocator@2.0",
"libui",
"libbase",
"libbinder",
diff --git a/evs/1.0/default/EvsCamera.cpp b/evs/1.0/default/EvsCamera.cpp
index 6715a2e..c62f7b6 100644
--- a/evs/1.0/default/EvsCamera.cpp
+++ b/evs/1.0/default/EvsCamera.cpp
@@ -33,18 +33,22 @@
const char EvsCamera::kCameraName_Backup[] = "backup";
const char EvsCamera::kCameraName_RightTurn[] = "Right Turn";
+// Arbitrary limit on number of graphics buffers allowed to be allocated
+// Safeguards against unreasonable resource consumption and provides a testable limit
+const unsigned MAX_BUFFERS_IN_FLIGHT = 100;
+
// TODO(b/31632518): Need to get notification when our client dies so we can close the camera.
-// As it stands, if the client dies suddently, the buffer may be stranded.
-// As possible work around would be to give the client a HIDL object to exclusively hold
-// and use it's destructor to perform some work in the server side.
+// As it stands, if the client dies suddenly, the buffer may be stranded.
-EvsCamera::EvsCamera(const char *id) {
+EvsCamera::EvsCamera(const char *id) :
+ mFramesAllowed(0),
+ mFramesInUse(0),
+ mStreamState(STOPPED) {
+
ALOGD("EvsCamera instantiated");
mDescription.cameraId = id;
- mFrameBusy = false;
- mStreamState = STOPPED;
// Set up dummy data for testing
if (mDescription.cameraId == kCameraName_Backup) {
@@ -52,16 +56,23 @@
mDescription.vendorFlags = 0xFFFFFFFF; // Arbitrary value
mDescription.defaultHorResolution = 320; // 1/2 NTSC/VGA
mDescription.defaultVerResolution = 240; // 1/2 NTSC/VGA
- }
- else if (mDescription.cameraId == kCameraName_RightTurn) {
+ } else if (mDescription.cameraId == kCameraName_RightTurn) {
// Nothing but the name and the usage hint
mDescription.hints = static_cast<uint32_t>(UsageHint::USAGE_HINT_RIGHT_TURN);
- }
- else {
+ } else {
// Leave empty for a minimalist camera description without even a hint
}
+
+
+ // Set our buffer properties
+ mWidth = (mDescription.defaultHorResolution) ? mDescription.defaultHorResolution : 640;
+ mHeight = (mDescription.defaultVerResolution) ? mDescription.defaultVerResolution : 480;
+
+ mFormat = HAL_PIXEL_FORMAT_RGBA_8888;
+ mUsage = GRALLOC_USAGE_HW_TEXTURE | GRALLOC_USAGE_HW_CAMERA_WRITE;
}
+
EvsCamera::~EvsCamera() {
ALOGD("EvsCamera being destroyed");
std::lock_guard<std::mutex> lock(mAccessLock);
@@ -70,11 +81,14 @@
// (It really should be already)
stopVideoStream();
- // Drop the graphics buffer we've been using
- if (mBuffer) {
- // Drop the graphics buffer we've been using
- GraphicBufferAllocator& alloc(GraphicBufferAllocator::get());
- alloc.free(mBuffer);
+ // Drop all the graphics buffers we've been using
+ GraphicBufferAllocator& alloc(GraphicBufferAllocator::get());
+ for (auto&& rec : mBuffers) {
+ if (rec.inUse) {
+ ALOGE("Error - releasing buffer despite remote ownership");
+ }
+ alloc.free(rec.handle);
+ rec.handle = nullptr;
}
ALOGD("EvsCamera destroyed");
@@ -95,113 +109,109 @@
ALOGD("setMaxFramesInFlight");
std::lock_guard<std::mutex> lock(mAccessLock);
- // TODO: Update our stored value
-
- // TODO: Adjust our buffer count right now if we can. Otherwise, it'll adjust in doneWithFrame
-
- // For now we support only one!
- if (bufferCount != 1) {
- return EvsResult::BUFFER_NOT_AVAILABLE;
+ // We cannot function without at least one video buffer to send data
+ if (bufferCount < 1) {
+ ALOGE("Ignoring setMaxFramesInFlight with less than one buffer requested");
+ return EvsResult::INVALID_ARG;
}
- return EvsResult::OK;
+ // Update our internal state
+ if (setAvailableFrames_Locked(bufferCount)) {
+ return EvsResult::OK;
+ } else {
+ return EvsResult::BUFFER_NOT_AVAILABLE;
+ }
}
+
Return<EvsResult> EvsCamera::startVideoStream(const ::android::sp<IEvsCameraStream>& stream) {
ALOGD("startVideoStream");
std::lock_guard<std::mutex> lock(mAccessLock);
- // We only support a single stream at a time
if (mStreamState != STOPPED) {
ALOGE("ignoring startVideoStream call when a stream is already running.");
return EvsResult::STREAM_ALREADY_RUNNING;
}
+ // If the client never indicated otherwise, configure ourselves for a single streaming buffer
+ if (mFramesAllowed < 1) {
+ if (!setAvailableFrames_Locked(1)) {
+ ALOGE("Failed to start stream because we couldn't get a graphics buffer");
+ return EvsResult::BUFFER_NOT_AVAILABLE;
+ }
+ }
+
// Record the user's callback for use when we have a frame ready
mStream = stream;
- // Allocate a graphics buffer into which we'll put our test images
- if (!mBuffer) {
- mWidth = (mDescription.defaultHorResolution) ? mDescription.defaultHorResolution : 640;
- mHeight = (mDescription.defaultVerResolution) ? mDescription.defaultVerResolution : 480;
- // TODO: What about stride? Assume no padding for now...
- mStride = 4* mWidth; // Special cased to assume 4 byte pixels with no padding for now
-
- ALOGD("Allocating buffer for camera frame");
- GraphicBufferAllocator &alloc(GraphicBufferAllocator::get());
- status_t result = alloc.allocate(mWidth, mHeight,
- HAL_PIXEL_FORMAT_RGBA_8888, 1, GRALLOC_USAGE_HW_TEXTURE,
- &mBuffer, &mStride, 0, "EvsCamera");
- if (result != NO_ERROR) {
- ALOGE("Error %d allocating %d x %d graphics buffer", result, mWidth, mHeight);
- return EvsResult::BUFFER_NOT_AVAILABLE;
- }
- if (!mBuffer) {
- ALOGE("We didn't get a buffer handle back from the allocator");
- return EvsResult::BUFFER_NOT_AVAILABLE;
- }
- }
-
// Start the frame generation thread
mStreamState = RUNNING;
- mCaptureThread = std::thread([this](){GenerateFrames();});
+ mCaptureThread = std::thread([this](){ generateFrames(); });
return EvsResult::OK;
}
-Return<EvsResult> EvsCamera::doneWithFrame(uint32_t /* frameId */, const hidl_handle& bufferHandle) {
+
+Return<void> EvsCamera::doneWithFrame(const BufferDesc& buffer) {
ALOGD("doneWithFrame");
- std::lock_guard<std::mutex> lock(mAccessLock);
-
- if (!bufferHandle)
- {
- ALOGE("ignoring doneWithFrame called with invalid handle");
- return EvsResult::INVALID_ARG;
- }
-
- // TODO: Track which frames we've delivered and validate this is one of them
-
- // Mark the frame buffer as available for a new frame
- mFrameBusy = false;
-
- // TODO: If we currently have too many buffers, drop this one
-
- return EvsResult::OK;
-}
-
-Return<void> EvsCamera::stopVideoStream() {
- ALOGD("stopVideoStream");
-
- bool waitForJoin = false;
- // Lock scope
- {
+ { // lock context
std::lock_guard <std::mutex> lock(mAccessLock);
- if (mStreamState == RUNNING) {
- // Tell the GenerateFrames loop we want it to stop
- mStreamState = STOPPING;
+ if (buffer.memHandle == nullptr) {
+ ALOGE("ignoring doneWithFrame called with null handle");
+ } else if (buffer.bufferId >= mBuffers.size()) {
+ ALOGE("ignoring doneWithFrame called with invalid bufferId %d (max is %lu)",
+ buffer.bufferId, mBuffers.size()-1);
+ } else if (!mBuffers[buffer.bufferId].inUse) {
+ ALOGE("ignoring doneWithFrame called on frame %d which is already free",
+ buffer.bufferId);
+ } else {
+ // Mark the frame as available
+ mBuffers[buffer.bufferId].inUse = false;
+ mFramesInUse--;
- // Note that we asked the thread to stop and should wait for it do so
- waitForJoin = true;
- }
- }
-
- if (waitForJoin) {
- // Block outside the mutex until the "stop" flag has been acknowledged
- // NOTE: We won't send any more frames, but the client might still get one already in flight
- ALOGD("Waiting for stream thread to end...");
- mCaptureThread.join();
-
- // Lock scope
- {
- std::lock_guard <std::mutex> lock(mAccessLock);
- mStreamState = STOPPED;
+ // If this frame's index is high in the array, try to move it down
+ // to improve locality after mFramesAllowed has been reduced.
+ if (buffer.bufferId >= mFramesAllowed) {
+ // Find an empty slot lower in the array (which should always exist in this case)
+ for (auto&& rec : mBuffers) {
+ if (rec.handle == nullptr) {
+ rec.handle = mBuffers[buffer.bufferId].handle;
+ mBuffers[buffer.bufferId].handle = nullptr;
+ break;
+ }
+ }
+ }
}
}
return Void();
}
+
+Return<void> EvsCamera::stopVideoStream() {
+ ALOGD("stopVideoStream");
+ std::unique_lock <std::mutex> lock(mAccessLock);
+
+ if (mStreamState == RUNNING) {
+ // Tell the GenerateFrames loop we want it to stop
+ mStreamState = STOPPING;
+
+ // Block outside the mutex until the "stop" flag has been acknowledged
+ // We won't send any more frames, but the client might still get some already in flight
+ ALOGD("Waiting for stream thread to end...");
+ lock.unlock();
+ mCaptureThread.join();
+ lock.lock();
+
+ mStreamState = STOPPED;
+ ALOGD("Stream marked STOPPED.");
+ }
+
+ return Void();
+}
+
+
Return<int32_t> EvsCamera::getExtendedInfo(uint32_t opaqueIdentifier) {
ALOGD("getExtendedInfo");
std::lock_guard<std::mutex> lock(mAccessLock);
@@ -215,6 +225,7 @@
return 0;
}
+
Return<EvsResult> EvsCamera::setExtendedInfo(uint32_t /*opaqueIdentifier*/, int32_t /*opaqueValue*/) {
ALOGD("setExtendedInfo");
std::lock_guard<std::mutex> lock(mAccessLock);
@@ -224,10 +235,124 @@
}
-void EvsCamera::GenerateFrames() {
- ALOGD("Frame generate loop started");
+bool EvsCamera::setAvailableFrames_Locked(unsigned bufferCount) {
+ if (bufferCount < 1) {
+ ALOGE("Ignoring request to set buffer count to zero");
+ return false;
+ }
+ if (bufferCount > MAX_BUFFERS_IN_FLIGHT) {
+ ALOGE("Rejecting buffer request in excess of internal limit");
+ return false;
+ }
- uint32_t frameNumber;
+ // Is an increase required?
+ if (mFramesAllowed < bufferCount) {
+ // An increase is required
+ unsigned needed = bufferCount - mFramesAllowed;
+ ALOGI("Allocating %d buffers for camera frames", needed);
+
+ unsigned added = increaseAvailableFrames_Locked(needed);
+ if (added != needed) {
+ // If we didn't add all the frames we needed, then roll back to the previous state
+ ALOGE("Rolling back to previous frame queue size");
+ decreaseAvailableFrames_Locked(added);
+ return false;
+ }
+ } else if (mFramesAllowed > bufferCount) {
+ // A decrease is required
+ unsigned framesToRelease = mFramesAllowed - bufferCount;
+ ALOGI("Returning %d camera frame buffers", framesToRelease);
+
+ unsigned released = decreaseAvailableFrames_Locked(framesToRelease);
+ if (released != framesToRelease) {
+ // This shouldn't happen with a properly behaving client because the client
+ // should only make this call after returning sufficient outstanding buffers
+ // to allow a clean resize.
+ ALOGE("Buffer queue shrink failed -- too many buffers currently in use?");
+ }
+ }
+
+ return true;
+}
+
+
+unsigned EvsCamera::increaseAvailableFrames_Locked(unsigned numToAdd) {
+ // Acquire the graphics buffer allocator
+ GraphicBufferAllocator &alloc(GraphicBufferAllocator::get());
+
+ unsigned added = 0;
+
+ while (added < numToAdd) {
+ buffer_handle_t memHandle = nullptr;
+ status_t result = alloc.allocate(mWidth, mHeight,
+ mFormat, 1,
+ mUsage,
+ &memHandle, &mStride, 0, "EvsCamera");
+ if (result != NO_ERROR) {
+ ALOGE("Error %d allocating %d x %d graphics buffer", result, mWidth, mHeight);
+ break;
+ }
+ if (!memHandle) {
+ ALOGE("We didn't get a buffer handle back from the allocator");
+ break;
+ }
+
+ // Find a place to store the new buffer
+ bool stored = false;
+ for (auto&& rec : mBuffers) {
+ if (rec.handle == nullptr) {
+ // Use this existing entry
+ rec.handle = memHandle;
+ rec.inUse = false;
+ stored = true;
+ break;
+ }
+ }
+ if (!stored) {
+ // Add a BufferRecord wrapping this handle to our set of available buffers
+ mBuffers.emplace_back(memHandle);
+ }
+
+ mFramesAllowed++;
+ added++;
+ }
+
+ return added;
+}
+
+
+unsigned EvsCamera::decreaseAvailableFrames_Locked(unsigned numToRemove) {
+ // Acquire the graphics buffer allocator
+ GraphicBufferAllocator &alloc(GraphicBufferAllocator::get());
+
+ unsigned removed = 0;
+
+ for (auto&& rec : mBuffers) {
+ // Is this record not in use, but holding a buffer that we can free?
+ if ((rec.inUse == false) && (rec.handle != nullptr)) {
+ // Release buffer and update the record so we can recognize it as "empty"
+ alloc.free(rec.handle);
+ rec.handle = nullptr;
+
+ mFramesAllowed--;
+ removed++;
+
+ if (removed == numToRemove) {
+ break;
+ }
+ }
+ }
+
+ return removed;
+}
+
+
+// This is the asynchronous frame generation thread that runs in parallel with the
+// main serving thread. There is one for each active camera instance.
+void EvsCamera::generateFrames() {
+ ALOGD("Frame generation loop started");
+
+ unsigned idx;
while (true) {
bool timeForFrame = false;
@@ -235,57 +360,69 @@
{
std::lock_guard<std::mutex> lock(mAccessLock);
- // Tick the frame counter -- rollover is tolerated
- frameNumber = mFrameId++;
-
if (mStreamState != RUNNING) {
// Break out of our main thread loop
break;
}
- if (mFrameBusy) {
+ // Are we allowed to issue another buffer?
+ if (mFramesInUse >= mFramesAllowed) {
// Can't do anything right now -- skip this frame
- ALOGW("Skipped a frame because client hasn't returned a buffer\n");
- }
- else {
- // We're going to make the frame busy
- mFrameBusy = true;
- timeForFrame = true;
+ ALOGW("Skipped a frame because too many are in flight\n");
+ } else {
+ // Identify an available buffer to fill
+ for (idx = 0; idx < mBuffers.size(); idx++) {
+ if (!mBuffers[idx].inUse) {
+ if (mBuffers[idx].handle != nullptr) {
+ // Found an available record, so stop looking
+ break;
+ }
+ }
+ }
+ if (idx >= mBuffers.size()) {
+ // This shouldn't happen since we already checked mFramesInUse vs mFramesAllowed
+ ALOGE("Failed to find an available buffer slot\n");
+ } else {
+ // We're going to make the frame busy
+ mBuffers[idx].inUse = true;
+ mFramesInUse++;
+ timeForFrame = true;
+ }
}
}
if (timeForFrame) {
- // Lock our output buffer for writing
- uint32_t *pixels = nullptr;
- GraphicBufferMapper &mapper = GraphicBufferMapper::get();
- mapper.lock(mBuffer,
- GRALLOC_USAGE_SW_WRITE_OFTEN,
- android::Rect(mWidth, mHeight),
- (void **) &pixels);
+ // Assemble the buffer description we'll transmit below
+ BufferDesc buff = {};
+ buff.width = mWidth;
+ buff.height = mHeight;
+ buff.stride = mStride;
+ buff.format = mFormat;
+ buff.usage = mUsage;
+ buff.bufferId = idx;
+ buff.memHandle = mBuffers[idx].handle;
- // If we failed to lock the pixel buffer, we're about to crash, but log it first
- if (!pixels) {
- ALOGE("Camera failed to gain access to image buffer for writing");
+ // Write test data into the image buffer
+ fillTestFrame(buff);
+
+ // Issue the (asynchronous) callback to the client -- can't be holding the lock
+ auto result = mStream->deliverFrame(buff);
+ if (result.isOk()) {
+ ALOGD("Delivered %p as id %d", buff.memHandle.getNativeHandle(), buff.bufferId);
+ } else {
+ // This can happen if the client dies and is likely unrecoverable.
+ // To avoid consuming resources generating failing calls, we stop sending
+ // frames. Note, however, that the stream remains in the "STREAMING" state
+ // until cleaned up on the main thread.
+ ALOGE("Frame delivery call failed in the transport layer.");
+
+ // Since we didn't actually deliver it, mark the frame as available
+ std::lock_guard<std::mutex> lock(mAccessLock);
+ mBuffers[idx].inUse = false;
+ mFramesInUse--;
+
+ break;
}
-
- // Fill in the test pixels
- for (unsigned row = 0; row < mHeight; row++) {
- for (unsigned col = 0; col < mWidth; col++) {
- // Index into the row to set the pixel at this column
- // (We're making vertical gradient in the green channel, and
- // horitzontal gradient in the blue channel)
- pixels[col] = 0xFF0000FF | ((row & 0xFF) << 16) | ((col & 0xFF) << 8);
- }
- // Point to the next row
- pixels = pixels + (mStride / sizeof(*pixels));
- }
-
- // Release our output buffer
- mapper.unlock(mBuffer);
-
- // Issue the (asynchronous) callback to the client
- mStream->deliverFrame(frameNumber, mBuffer);
- ALOGD("Delivered %p as frame %d", mBuffer, frameNumber);
}
// We arbitrarily choose to generate frames at 10 fps (1/10 * uSecPerSec)
@@ -293,11 +430,58 @@
}
// If we've been asked to stop, send one last NULL frame to signal the actual end of stream
- mStream->deliverFrame(frameNumber, nullptr);
+ BufferDesc nullBuff = {};
+ auto result = mStream->deliverFrame(nullBuff);
+ if (!result.isOk()) {
+ ALOGE("Error delivering end of stream marker");
+ }
return;
}
+
+void EvsCamera::fillTestFrame(BufferDesc buff) {
+ // Lock our output buffer for writing
+ uint32_t *pixels = nullptr;
+ GraphicBufferMapper &mapper = GraphicBufferMapper::get();
+ mapper.lock(buff.memHandle,
+ GRALLOC_USAGE_SW_WRITE_OFTEN | GRALLOC_USAGE_SW_READ_NEVER,
+ android::Rect(buff.width, buff.height),
+ (void **) &pixels);
+
+ // If we failed to lock the pixel buffer, we're about to crash, but log it first
+ if (!pixels) {
+ ALOGE("Camera failed to gain access to image buffer for writing");
+ }
+
+ // Fill in the test pixels
+ for (unsigned row = 0; row < buff.height; row++) {
+ for (unsigned col = 0; col < buff.width; col++) {
+ // Index into the row to check the pixel at this column.
+ // We expect 0xFF in the LSB channel, a vertical gradient in the
+ // second channel, a horitzontal gradient in the third channel, and
+ // 0xFF in the MSB.
+ // The exception is the very first 32 bits which is used for the
+ // time varying frame signature to avoid getting fooled by a static image.
+ uint32_t expectedPixel = 0xFF0000FF | // MSB and LSB
+ ((row & 0xFF) << 8) | // vertical gradient
+ ((col & 0xFF) << 16); // horizontal gradient
+ if ((row | col) == 0) {
+ static uint32_t sFrameTicker = 0;
+ expectedPixel = (sFrameTicker) & 0xFF;
+ sFrameTicker++;
+ }
+ pixels[col] = expectedPixel;
+ }
+ // Point to the next row
+ pixels = pixels + (buff.stride / sizeof(*pixels));
+ }
+
+ // Release our output buffer
+ mapper.unlock(buff.memHandle);
+}
+
+
} // namespace implementation
} // namespace V1_0
} // namespace evs
diff --git a/evs/1.0/default/EvsCamera.h b/evs/1.0/default/EvsCamera.h
index 5d29125..8d644a0 100644
--- a/evs/1.0/default/EvsCamera.h
+++ b/evs/1.0/default/EvsCamera.h
@@ -23,19 +23,21 @@
#include <thread>
+
namespace android {
namespace hardware {
namespace evs {
namespace V1_0 {
namespace implementation {
+
class EvsCamera : public IEvsCamera {
public:
// Methods from ::android::hardware::evs::V1_0::IEvsCamera follow.
Return<void> getId(getId_cb id_cb) override;
Return<EvsResult> setMaxFramesInFlight(uint32_t bufferCount) override;
Return<EvsResult> startVideoStream(const ::android::sp<IEvsCameraStream>& stream) override;
- Return<EvsResult> doneWithFrame(uint32_t frameId, const hidl_handle& bufferHandle) override;
+ Return<void> doneWithFrame(const BufferDesc& buffer) override;
Return<void> stopVideoStream() override;
Return<int32_t> getExtendedInfo(uint32_t opaqueIdentifier) override;
Return<EvsResult> setExtendedInfo(uint32_t opaqueIdentifier, int32_t opaqueValue) override;
@@ -45,34 +47,49 @@
virtual ~EvsCamera() override;
const CameraDesc& getDesc() { return mDescription; };
- void GenerateFrames();
static const char kCameraName_Backup[];
static const char kCameraName_RightTurn[];
private:
- CameraDesc mDescription = {}; // The properties of this camera
+ // These three functions are expected to be called while mAccessLock is held
+ bool setAvailableFrames_Locked(unsigned bufferCount);
+ unsigned increaseAvailableFrames_Locked(unsigned numToAdd);
+ unsigned decreaseAvailableFrames_Locked(unsigned numToRemove);
- buffer_handle_t mBuffer = nullptr; // A graphics buffer into which we'll store images
- uint32_t mWidth = 0; // number of pixels across the buffer
- uint32_t mHeight = 0; // number of pixels vertically in the buffer
- uint32_t mStride = 0; // Bytes per line in the buffer
+ void generateFrames();
+ void fillTestFrame(BufferDesc buff);
- sp<IEvsCameraStream> mStream = nullptr; // The callback the user expects when a frame is ready
+ CameraDesc mDescription = {}; // The properties of this camera
- std::thread mCaptureThread; // The thread we'll use to synthesize frames
+ std::thread mCaptureThread; // The thread we'll use to synthesize frames
- uint32_t mFrameId; // A frame counter used to identify specific frames
+ uint32_t mWidth = 0; // Horizontal pixel count in the buffers
+ uint32_t mHeight = 0; // Vertical pixel count in the buffers
+ uint32_t mFormat = 0; // Values from android_pixel_format_t [TODO: YUV? Leave opaque?]
+ uint32_t mUsage = 0; // Values from from Gralloc.h
+ uint32_t mStride = 0; // Bytes per line in the buffers
+
+ sp<IEvsCameraStream> mStream = nullptr; // The callback used to deliver each frame
+
+ struct BufferRecord {
+ buffer_handle_t handle;
+ bool inUse;
+ explicit BufferRecord(buffer_handle_t h) : handle(h), inUse(false) {};
+ };
+ std::vector<BufferRecord> mBuffers; // Graphics buffers to transfer images
+ unsigned mFramesAllowed; // How many buffers are we currently using
+ unsigned mFramesInUse; // How many buffers are currently outstanding
enum StreamStateValues {
STOPPED,
RUNNING,
STOPPING,
};
- StreamStateValues mStreamState;
- bool mFrameBusy; // A flag telling us our one buffer is in use
+ StreamStateValues mStreamState;
- std::mutex mAccessLock;
+ // Syncrhonization necessary to deconflict mCaptureThread from the main service thread
+ std::mutex mAccessLock;
};
} // namespace implementation
diff --git a/evs/1.0/default/EvsDisplay.cpp b/evs/1.0/default/EvsDisplay.cpp
index 9dba6fc..dff4c49 100644
--- a/evs/1.0/default/EvsDisplay.cpp
+++ b/evs/1.0/default/EvsDisplay.cpp
@@ -39,6 +39,7 @@
ALOGD("EvsDisplay instantiated");
// Set up our self description
+ // NOTE: These are arbitrary values chosen for testing
mInfo.displayId = "Mock Display";
mInfo.vendorFlags = 3870;
mInfo.defaultHorResolution = 320;
@@ -50,16 +51,17 @@
ALOGD("EvsDisplay being destroyed");
std::lock_guard<std::mutex> lock(mAccessLock);
- // Report if we're going away while a buffer is outstanding. This could be bad.
+ // Report if we're going away while a buffer is outstanding
if (mFrameBusy) {
- ALOGE("EvsDisplay going down while client is holding a buffer\n");
+ ALOGE("EvsDisplay going down while client is holding a buffer");
}
// Make sure we release our frame buffer
- if (mBuffer) {
+ if (mBuffer.memHandle) {
// Drop the graphics buffer we've been using
GraphicBufferAllocator& alloc(GraphicBufferAllocator::get());
- alloc.free(mBuffer);
+ alloc.free(mBuffer.memHandle);
+ mBuffer.memHandle = nullptr;
}
ALOGD("EvsDisplay destroyed");
}
@@ -135,36 +137,60 @@
std::lock_guard<std::mutex> lock(mAccessLock);
// If we don't already have a buffer, allocate one now
- if (!mBuffer) {
+ if (!mBuffer.memHandle) {
+ // Assemble the buffer description we'll use for our render target
+ mBuffer.width = mInfo.defaultHorResolution;
+ mBuffer.height = mInfo.defaultVerResolution;
+ mBuffer.format = HAL_PIXEL_FORMAT_RGBA_8888;
+ mBuffer.usage = GRALLOC_USAGE_HW_RENDER | GRALLOC_USAGE_HW_COMPOSER;
+ mBuffer.bufferId = 0x3870; // Arbitrary magic number for self recognition
+
+ buffer_handle_t handle = nullptr;
GraphicBufferAllocator& alloc(GraphicBufferAllocator::get());
- status_t result = alloc.allocate(mInfo.defaultHorResolution, mInfo.defaultVerResolution,
- HAL_PIXEL_FORMAT_RGBA_8888, 1,
- GRALLOC_USAGE_HW_FB | GRALLOC_USAGE_HW_COMPOSER,
- &mBuffer, &mStride, 0, "EvsDisplay");
+ status_t result = alloc.allocate(mBuffer.width, mBuffer.height,
+ mBuffer.format, 1, mBuffer.usage,
+ &handle, &mBuffer.stride,
+ 0, "EvsDisplay");
+ if (result != NO_ERROR) {
+ ALOGE("Error %d allocating %d x %d graphics buffer",
+ result, mBuffer.width, mBuffer.height);
+ BufferDesc nullBuff = {};
+ _hidl_cb(nullBuff);
+ return Void();
+ }
+ if (!handle) {
+ ALOGE("We didn't get a buffer handle back from the allocator");
+ BufferDesc nullBuff = {};
+ _hidl_cb(nullBuff);
+ return Void();
+ }
+
+ mBuffer.memHandle = handle;
mFrameBusy = false;
- ALOGD("Allocated new buffer %p with stride %u", mBuffer, mStride);
+ ALOGD("Allocated new buffer %p with stride %u",
+ mBuffer.memHandle.getNativeHandle(), mStride);
}
// Do we have a frame available?
if (mFrameBusy) {
// This means either we have a 2nd client trying to compete for buffers
// (an unsupported mode of operation) or else the client hasn't returned
- // a previously issues buffer yet (they're behaving badly).
- // NOTE: We have to make callback even if we have nothing to provide
+ // a previously issued buffer yet (they're behaving badly).
+ // NOTE: We have to make the callback even if we have nothing to provide
ALOGE("getTargetBuffer called while no buffers available.");
- _hidl_cb(nullptr);
- }
- else {
+ BufferDesc nullBuff = {};
+ _hidl_cb(nullBuff);
+ return Void();
+ } else {
// Mark our buffer as busy
mFrameBusy = true;
// Send the buffer to the client
- ALOGD("Providing display buffer %p", mBuffer);
+ ALOGD("Providing display buffer handle %p as id %d",
+ mBuffer.memHandle.getNativeHandle(), mBuffer.bufferId);
_hidl_cb(mBuffer);
+ return Void();
}
-
- // All done
- return Void();
}
@@ -172,22 +198,19 @@
* This call tells the display that the buffer is ready for display.
* The buffer is no longer valid for use by the client after this call.
*/
-Return<EvsResult> EvsDisplay::returnTargetBufferForDisplay(const hidl_handle& bufferHandle) {
- ALOGD("returnTargetBufferForDisplay %p", bufferHandle.getNativeHandle());
+Return<EvsResult> EvsDisplay::returnTargetBufferForDisplay(const BufferDesc& buffer) {
+ ALOGD("returnTargetBufferForDisplay %p", buffer.memHandle.getNativeHandle());
std::lock_guard<std::mutex> lock(mAccessLock);
- // This shouldn't happen if we haven't issued the buffer!
- if (!bufferHandle) {
+ // Nobody should call us with a null handle
+ if (!buffer.memHandle.getNativeHandle()) {
ALOGE ("returnTargetBufferForDisplay called without a valid buffer handle.\n");
return EvsResult::INVALID_ARG;
}
- /* TODO(b/33492405): It would be nice to validate we got back the buffer we expect,
- * but HIDL doesn't support that (yet?)
- if (bufferHandle != mBuffer) {
+ if (buffer.bufferId != mBuffer.bufferId) {
ALOGE ("Got an unrecognized frame returned.\n");
return EvsResult::INVALID_ARG;
}
- */
if (!mFrameBusy) {
ALOGE ("A frame was returned with no outstanding frames.\n");
return EvsResult::BUFFER_NOT_AVAILABLE;
@@ -204,10 +227,71 @@
if (mRequestedState != DisplayState::VISIBLE) {
// We shouldn't get frames back when we're not visible.
ALOGE ("Got an unexpected frame returned while not visible - ignoring.\n");
- }
- else {
- // Make this buffer visible
- // TODO: Add code to put this image on the screen (or validate it somehow?)
+ } else {
+ // This is where the buffer would be made visible.
+ // For now we simply validate it has the data we expect in it by reading it back
+
+ // Lock our display buffer for reading
+ uint32_t* pixels = nullptr;
+ GraphicBufferMapper &mapper = GraphicBufferMapper::get();
+ mapper.lock(mBuffer.memHandle,
+ GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_NEVER,
+ android::Rect(mBuffer.width, mBuffer.height),
+ (void **)&pixels);
+
+ // If we failed to lock the pixel buffer, we're about to crash, but log it first
+ if (!pixels) {
+ ALOGE("Camera failed to gain access to image buffer for reading");
+ }
+
+ // Check the test pixels
+ bool frameLooksGood = true;
+ for (unsigned row = 0; row < mInfo.defaultVerResolution; row++) {
+ for (unsigned col = 0; col < mInfo.defaultHorResolution; col++) {
+ // Index into the row to check the pixel at this column.
+ // We expect 0xFF in the LSB channel, a vertical gradient in the
+ // second channel, a horitzontal gradient in the third channel, and
+ // 0xFF in the MSB.
+ // The exception is the very first 32 bits which is used for the
+ // time varying frame signature to avoid getting fooled by a static image.
+ uint32_t expectedPixel = 0xFF0000FF | // MSB and LSB
+ ((row & 0xFF) << 8) | // vertical gradient
+ ((col & 0xFF) << 16); // horizontal gradient
+ if ((row | col) == 0) {
+ // we'll check the "uniqueness" of the frame signature below
+ continue;
+ }
+ // Walk across this row (we'll step rows below)
+ if (pixels[col] != expectedPixel) {
+ ALOGE("Pixel check mismatch in frame buffer");
+ frameLooksGood = false;
+ break;
+ }
+ }
+
+ if (!frameLooksGood) {
+ break;
+ }
+
+ // Point to the next row
+ pixels = pixels + (mStride / sizeof(*pixels));
+ }
+
+ // Ensure we don't see the same buffer twice without it being rewritten
+ static uint32_t prevSignature = ~0;
+ uint32_t signature = pixels[0] & 0xFF;
+ if (prevSignature == signature) {
+ frameLooksGood = false;
+ ALOGE("Duplicate, likely stale frame buffer detected");
+ }
+
+
+ // Release our output buffer
+ mapper.unlock(mBuffer.memHandle);
+
+ if (!frameLooksGood) {
+ return EvsResult::UNDERLYING_SERVICE_ERROR;
+ }
}
return EvsResult::OK;
diff --git a/evs/1.0/default/EvsDisplay.h b/evs/1.0/default/EvsDisplay.h
index a2d5d3e..6e0111e 100644
--- a/evs/1.0/default/EvsDisplay.h
+++ b/evs/1.0/default/EvsDisplay.h
@@ -33,7 +33,7 @@
Return<EvsResult> setDisplayState(DisplayState state) override;
Return<DisplayState> getDisplayState() override;
Return<void> getTargetBuffer(getTargetBuffer_cb _hidl_cb) override;
- Return<EvsResult> returnTargetBufferForDisplay(const hidl_handle& bufferHandle) override;
+ Return<EvsResult> returnTargetBufferForDisplay(const BufferDesc& buffer) override;
// Implementation details
EvsDisplay();
@@ -41,10 +41,10 @@
private:
DisplayDesc mInfo = {};
- buffer_handle_t mBuffer = nullptr; // A graphics buffer into which we'll store images
- uint32_t mStride = 0; // Bytes per line in the buffer
+ BufferDesc mBuffer = {}; // A graphics buffer into which we'll store images
+ uint32_t mStride = 0; // Bytes per line in the buffer
- bool mFrameBusy = false; // A flag telling us our buffer is in use
+ bool mFrameBusy = false; // A flag telling us our buffer is in use
DisplayState mRequestedState = DisplayState::NOT_VISIBLE;
std::mutex mAccessLock;
diff --git a/evs/1.0/default/EvsEnumerator.cpp b/evs/1.0/default/EvsEnumerator.cpp
index ba8da00..4bf77f3 100644
--- a/evs/1.0/default/EvsEnumerator.cpp
+++ b/evs/1.0/default/EvsEnumerator.cpp
@@ -27,6 +27,11 @@
namespace implementation {
+// TODO(b/31632518): Need to get notification when our client dies so we can close the camera.
+// As it stands, if the client dies suddenly, the camera will be stuck "open".
+// NOTE: Display should already be safe by virtue of holding only a weak pointer.
+
+
EvsEnumerator::EvsEnumerator() {
ALOGD("EvsEnumerator created");
@@ -78,15 +83,11 @@
if (!pRecord) {
ALOGE("Requested camera %s not found", cameraId.c_str());
return nullptr;
- }
- else if (pRecord->inUse) {
+ } else if (pRecord->inUse) {
ALOGE("Cannot open camera %s which is already in use", cameraId.c_str());
return nullptr;
- }
- else {
- /* TODO(b/33492405): Do this, When HIDL can give us back a recognizable pointer
+ } else {
pRecord->inUse = true;
- */
return(pRecord->pCamera);
}
}
@@ -96,14 +97,21 @@
if (camera == nullptr) {
ALOGE("Ignoring call to closeCamera with null camera pointer");
- }
- else {
- // Make sure the camera has stopped streaming
- camera->stopVideoStream();
+ } else {
+ // Find this camera in our list
+ auto it = std::find_if(mCameraList.begin(),
+ mCameraList.end(),
+ [camera](const CameraRecord& rec) {
+ return (rec.pCamera == camera);
+ });
+ if (it == mCameraList.end()) {
+ ALOGE("Ignoring close on unrecognized camera");
+ } else {
+ // Make sure the camera has stopped streaming
+ camera->stopVideoStream();
- /* TODO(b/33492405): Do this, When HIDL can give us back a recognizable pointer
- pRecord->inUse = false;
- */
+ it->inUse = false;
+ }
}
return Void();
@@ -113,41 +121,49 @@
ALOGD("openDisplay");
// If we already have a display active, then this request must be denied
- if (mActiveDisplay != nullptr) {
+ sp<IEvsDisplay> pActiveDisplay = mActiveDisplay.promote();
+ if (pActiveDisplay != nullptr) {
ALOGW("Rejecting openDisplay request the display is already in use.");
return nullptr;
- }
- else {
+ } else {
// Create a new display interface and return it
- mActiveDisplay = new EvsDisplay();
- ALOGD("Returning new EvsDisplay object %p", mActiveDisplay.get());
- return mActiveDisplay;
+ pActiveDisplay = new EvsDisplay();
+ mActiveDisplay = pActiveDisplay;
+ ALOGD("Returning new EvsDisplay object %p", pActiveDisplay.get());
+ return pActiveDisplay;
}
}
Return<void> EvsEnumerator::closeDisplay(const ::android::sp<IEvsDisplay>& display) {
ALOGD("closeDisplay");
- if (mActiveDisplay == nullptr) {
- ALOGE("Ignoring closeDisplay when display is not active");
- }
- else if (display == nullptr) {
- ALOGE("Ignoring closeDisplay with null display pointer");
- }
- else {
+ // Do we still have a display object we think should be active?
+ sp<IEvsDisplay> pActiveDisplay = mActiveDisplay.promote();
+
+ if (pActiveDisplay == nullptr) {
+ ALOGE("Ignoring closeDisplay when there is no active display.");
+ } else if (display != pActiveDisplay) {
+ ALOGE("Ignoring closeDisplay on a display we didn't issue");
+ ALOGI("Got %p while active display is %p.", display.get(), pActiveDisplay.get());
+ } else {
// Drop the active display
- // TODO(b/33492405): When HIDL provides recognizable pointers, add validation here.
mActiveDisplay = nullptr;
}
return Void();
}
+Return<DisplayState> EvsEnumerator::getDisplayState() {
+ ALOGD("getDisplayState");
-// TODO(b/31632518): Need to get notification when our client dies so we can close the camera.
-// As possible work around would be to give the client a HIDL object to exclusively hold
-// and use it's destructor to perform some work in the server side.
-
+ // Do we still have a display object we think should be active?
+ sp<IEvsDisplay> pActiveDisplay = mActiveDisplay.promote();
+ if (pActiveDisplay != nullptr) {
+ return pActiveDisplay->getDisplayState();
+ } else {
+ return DisplayState::NOT_OPEN;
+ }
+}
} // namespace implementation
} // namespace V1_0
diff --git a/evs/1.0/default/EvsEnumerator.h b/evs/1.0/default/EvsEnumerator.h
index 90df837..0e719bd 100644
--- a/evs/1.0/default/EvsEnumerator.h
+++ b/evs/1.0/default/EvsEnumerator.h
@@ -38,6 +38,7 @@
Return<void> closeCamera(const ::android::sp<IEvsCamera>& carCamera) override;
Return<sp<IEvsDisplay>> openDisplay() override;
Return<void> closeDisplay(const ::android::sp<IEvsDisplay>& display) override;
+ Return<DisplayState> getDisplayState() override;
// Implementation details
EvsEnumerator();
@@ -50,7 +51,7 @@
};
std::list<CameraRecord> mCameraList;
- sp<IEvsDisplay> mActiveDisplay;
+ wp<IEvsDisplay> mActiveDisplay; // Weak pointer -> object destructs if client dies
};
} // namespace implementation