[libcamera-devel] [RFC PATCH 0/2] android: Fix the issue with Type::Mapped only request

[Jacopo Mondi]

Hi Hiro,

On Mon, Sep 27, 2021 at 07:48:19PM +0900, Hirokazu Honda wrote:
> We have the problem that no buffer is not provided to a native camera
> if a request given in CameraDevice::processCaptureRequest() contains
> only streams that are resolved as CameraStream::Type::Mapped in
> CameraDevice::configureStreams(). See the example in below.
>
> ```
> [0:27:06.863311142] [15203] DEBUG HAL camera_device.cpp:980
> '\_SB_.PCI0.I2C2.CAM0': Queueing request 139090704005584 with 2
> streams
> [0:27:06.863389502] [15203] DEBUG HAL camera_device.cpp:1005
> '\_SB_.PCI0.I2C2.CAM0': 0 - (320x240)[0x00000022] -> (320x240)[NV12]
> (mapped)
> [0:27:06.863436331] [15203] DEBUG HAL camera_device.cpp:1005
> '\_SB_.PCI0.I2C2.CAM0': 1 - (320x240)[0x00000021] -> (320x240)[NV12]
> (mapped)
> [0:27:06.863487886] [15203] DEBUG HAL camera_device.cpp:1043
> '\_SB_.PCI0.I2C2.CAM0': buffers=0
> [0:27:06.863561912] [15218] ERROR Camera camera.cpp:1031 Request
> contains no buffers
> ```
>
> This patch series fixes the issue by looking up the stream
> alternatively sent to a native camera. To do that, we have to
> allocate a FrameBuffer for the stream. One possible way is to use
> FrameBufferAllocator. However, FrameBufferAllocator::allocate must
> be called when the state of Camera is Configured, but not Running.
> So we have to allocate buffers during configureStreams(). But
> allocating buffers for CameraStrems whose type is Direct wastes
> memory as they tends to not be used at all.
>
> I would rather have a new class that allocates FrameBuffers in a
> platform-dependent way, cros::CameraBufferManager on ChromeOS and
> gralloc API on other platforms.

Here is the reply to my previous question.

And I actually think the contrary: this series fixes a bug it
shouldn't introduce new features. I would make sure the minimum amount
of changes to get there, instead of piling more stuff on top that
requires more review, more test, more verifications and could
potentially introduce new bugs.

>
> With this patch, android.hardware.camera2.cts.SurfaceViewPreviewTest#testDeferredSurfaces passed.

Please add the full CTS results to the cover letters of series that
touch the HAL in non trivial ways.

I think we'll make this a requirement going forward.

>
> Hirokazu Honda (2):
>   android: Introduce PlatformFrameBufferAllocator
>   android: Send alternative stream configuration if no buffer to be sent
>     exists
>
>  src/android/camera_device.cpp           |  50 ++++++++--
>  src/android/camera_device.h             |   4 +
>  src/android/camera_stream.cpp           |  37 +++++++-
>  src/android/camera_stream.h             |  10 +-
>  src/android/frame_buffer.h              |  53 +++++++++++
>  src/android/mm/cros_frame_buffer.cpp    |  85 +++++++++++++++++
>  src/android/mm/generic_frame_buffer.cpp | 116 ++++++++++++++++++++++++
>  src/android/mm/meson.build              |   6 +-
>  8 files changed, 344 insertions(+), 17 deletions(-)
>  create mode 100644 src/android/frame_buffer.h
>  create mode 100644 src/android/mm/cros_frame_buffer.cpp
>  create mode 100644 src/android/mm/generic_frame_buffer.cpp
>
> --
> 2.33.0.685.g46640cef36-goog