VIMC pipeline handler has dmabuf-backed mock FrameBuffers which are
specifically targetted mimicking IPA buffers (parameter and statistics).
Map these mock buffers to the VIMC IPA that would enable exercising IPA
IPC code paths. This will provide leverage to our test suite to test
IPA IPC code paths, which are common to various platforms.
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
VIMC is a virtual test driver that doesn't have statistics or
parameters buffers that are typically passed from a pipeline
handler to its platform IPA. To increase the test coverage going
forward, we can at least mimick the typical interaction of how
a pipeline handler and IPA interact, and use it to increase the
test coverage.
Hence, create simple (single plane) dmabuf-backed FrameBuffers,
which can act as mock IPA buffers and can be memory mapped (mmap)
to VIMC IPA. To create these buffers, temporarily hijack the output
video node and configure it with a V4L2DeviceFormat. Buffers then
can be exported from the output video node using
V4L2VideoDevice::exportBuffers(). These buffers will be mimicked as
IPA buffers in subsequent commits.
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
All the IPU3 Camera controls are currently initialized by the pipeline
handler which initializes them using the camera sensor configuration and
platform specific requirements.
However, some controls are better initialized by the IPA, which might,
in example, cap the exposure times and frame duration to the constraints
of its algorithms implementation.
Also, moving forward, the IPA should register controls to report its
capabilities, in example the ability to enable/disable 3A algorithms on
request.
Move the existing controls initialization to the IPA, by providing
the sensor configuration and its controls to the IPU3IPA::init()
function, which initializes controls and returns them to the pipeline
through an output parameter.
The existing controls initialization has been copied verbatim from the
pipeline handler to the IPA, if not a for few line breaks adjustments
and the resulting Camera controls values are not changed.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
ControlInfoMap does not have a ControlId map associated, but rather
creates one with the generateIdMap() function at creation time.
As a consequence, when in the need to de-serialize a ControlInfoMap all
the ControlId it contains are created by the deserializer instance, not
being able to discern if the controls the ControlIdMap refers to are the
global libcamera controls (and properties) or instances local to the
V4L2 device that has first initialized the controls.
As a consequence the ControlId stored in a de-serialized map will always
be newly created entities, preventing lookup by ControlId reference on a
de-serialized ControlInfoMap.
In order to make it possible to use globally available ControlId
instances whenever possible, create ControlInfoMap with a reference to
an externally allocated ControlIdMap instead of generating one
internally.
As a consequence the class constructors take and additional argument,
which might be not pleasant to type in, but enforces the concepts that
ControlInfoMap should be created with controls part of the same id map.
As the ControlIdMap the ControlInfoMap refers to needs to be allocated
externally:
- Use the globally available controls::controls (or
properties::properties) id map when referring to libcamera controls
- The V4L2 device that creates ControlInfoMap by parsing the device's
controls has to allocate a ControlIdMap
- The ControlSerializer that de-serializes a ControlInfoMap has to
create and store the ControlIdMap the de-serialized info map refers to
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Usage of 'method' to refer to member functions comes from Java. The C++
standard uses the term 'function' only. Replace 'method' with 'function'
or 'member function' through the whole code base and documentation.
While at it, fix two typos (s/backeng/backend/).
The BoundMethod and Object::invokeMethod() are left as-is here, and will
be addressed separately.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
To capture raw frames, the ImgU isn't needed. However, to implement
auto-exposure, we do need to configure the IPA since it shall setup
the sensor controls (exposure, vblank and so on) for the capture.
One cannot simply configure the IPA, without the ImgU as the
parameters and statistics buffer passed to the IPA are actually
managed by the ImgU.
Until we prepare and setup the ImgU to run an internal queue for
raw-only camera configuration, disallow this configuration and
report it as invalid.
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
When RPiCameraData::clearIncompleteRequests() clears out the request
queue during a stop condition, it unconditionally calls completeBuffer()
on all buffers in each request. This is wrong, as a buffer could have
already been completed as part of the current request, but the request
itself may not yet have completed.
Fix this by checking if the buffers in the request have been completed
before cancelling them.
Fixes: d372aaa10d ("pipeline: raspberrypi: Simplify RPiCameraData::clearIncompleteRequests()")
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Implement the D-Pointer design pattern in the FrameBuffer class to allow
changing internal data without affecting the public ABI.
Move the request_ field and the setRequest() function to the
FrameBuffer::Private class. This allows hiding the setRequest() function
from the public API, removing one todo item. More fields may be moved
later.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
When the configuration of the converter fails due to format mismatch,
the error messages only indicates that a failure occurred. Improve it to
ease debugging by printing the requested and obtained formats.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Move the functionality for the following components to the new
base support library:
- BoundMethod
- EventDispatcher
- EventDispatcherPoll
- Log
- Message
- Object
- Signal
- Semaphore
- Thread
- Timer
While it would be preferable to see these split to move one component
per commit, these components are all interdependent upon each other,
which leaves us with one big change performing the move for all of them.
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
With the addition of FrameBuffer::cancel(), the logic to clear and return
pending requests can be simplified by not having to queue all the request
buffers to the device before calling streamOff().
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Commit 922833f774 ("libcamera: simple: Report sensor timestamp")
unconditionally tries to access the request through the capture buffer
to store the capture timestamp in the metadata. This causes a null
pointer dereference when using a converter, as the capture buffers are
free-wheeling in that case, and not associated with a request.
Fix this by getting the request from the user-facing buffer, which can
be the capture buffer when no converter is used.
Fixes: 922833f774 ("libcamera: simple: Report sensor timestamp")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
If the IPA fails to configure, this can now be caught by the pipeline handler
but the cause may not be clear.
If the IPA is isolated, then reports from that IPA will not be visible
in the libcamera logs directly.
Print the return error value to help identify any issue that has arisen.
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The IPU3 IPA interface does not define a return value from configure().
This prevents errors from being reported back to the pipeline handler
when they occur in the IPA.
Update the IPU3 IPA interface and add return values to the checks in
IPAIPU3::configure() accordingly
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
When compiling with optimization, gcc 9 and newer throw an unitialized
variable warning:
../../src/libcamera/pipeline/ipu3/imgu.cpp: In function ‘void libcamera::{anonymous}::calculateBDSHeight(libcamera::ImgUDevice::Pipe*, const libcamera::Size&, const libcamera::Size&, unsigned int, float)’:
../../src/libcamera/pipeline/ipu3/imgu.cpp:172:17: error: ‘bdsHeight’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
172 | unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
Neither clang not gcc versions older than 9 complain. This seems to be
a false positive.
However, there's an obvious error in the code. The second while () loop
in the first part of calculateBDSHeight() modifies the bdsHeight
variable set by the first loop even if the second loop doesn't find a
suitable height. This can result in an incorrect bdsHeight value. Fix
this, which also gets rid of the compiler warning.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
This reverts commit 5b015e96cc.
The ImgU pipe configuration debug is useful to test the correctness
of the parameters computation against the Intel Python script.
However, the number of debug messages which is printed out by the
configuration procedure is so high it floods the logs, up to the point
that starting the Android camera3 HAL, which tests several configurations
at startup, becomes so slow it is barely usable.
Revert the patch that adds the excessive debug statements, which are mostly
useful only when testing the configuration procedure.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Acked-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
We need a separate control to report the nominal frame duration, but
it's also useful to report the min/max frame duration values that will
be used. Split the FrameDurations control into FrameDuration and
FrameDurationLimits respectively to support both of these.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
The supportedDevices may contain entries which have the same driver
but different converters. For example, if we add these two entries:
{ "mtk-seninf", "mtk-mdp", 3 },
{ "mtk-seninf", "mtk-mdp3", 3 },
the simple pipeline handler will always take the first one where it
can acquire the driver and skip the rest.
So, make the changes to support this usecase.
Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
IPU3CameraData stores requests that have been failed due to a
buffer shortage. The requests should be retried once enough
buffers are available. This sets the retry function as signal to
CIO2Device and IPU3Frame, and invokes it from
CIO2Device::tryReturnBuffer() and IPU3Frame::remove().
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
PipelineHandlerIPU3 returns -ENOBUFS and -ENOMEM on queueing a
request when there are not sufficient buffers for the request.
Since the request will be successful if it is queued later when
enough buffers are available. The requests failed due to a buffer
shortage should be stored and retried later in the FIFO order.
This introduces the queue in IPU3CameraData to do that.
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Update the revision of the ImgU Python configuration tool the
libcamera implementation is based on to commit 243d134
("Fix some bug for some resolutions").
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org
The IF rectangle height is iteratively computed by first subtracting
the scaling factor to the estimated height, then in a successive loop
by adding the same scaling factor until the maximum IF size is not
reached.
As the computed IF height is not cached in any variable, the second
loop over-writes the result of the first one, even if the BDS alignment
condition is not satisfied.
Fix this by caching the result of the two iterations, and use the one
that produced any result, with a preference for the one produced by the
second loop, as implemented in the reference python script.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
As reported in https://bugs.libcamera.org/show_bug.cgi?id=32
allowing resolutions < IF_CROP_MAX leads to a not manageable number
configurations to be tested, slowing down the ImgU pipe configuration
to a point which is not acceptable for production devices.
Filter all resolutions < IF_CROP_MAX to maintain the run-time complexity
acceptable and remove the safety check that was meant to avoid overflows
when computing the IF rectangle sizes.
Bug: https://bugs.libcamera.org/show_bug.cgi?id=32
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
With the recent change to merge existing Request metadata with the
ControlList provided by the IPA in commit fcfb1dc02a ("libcamera:
raspberry: Report sensor timestamp"), we can now write the
controls::ScalerCrop value at the start of the pipeline instead of at
the end.
This change simplifies the logic slightly, and allows us to write all
metadata items to the Request in one place.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Write the controls::SensorTimestamp value in the Request metadata when
the request is popped from the queue ready to run the pipeline. This
ensures that the timestamp is written to the correct Request item,
which may not be at the top of the queue when the Unicam buffer dequeue
occurs.
Fixes: fcfb1dc02a ("libcamera: raspberry: Report sensor timestamp")
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The controller algorithms currently run on every frame provided to the
IPA by the pipeline handler. This may be undesirable for very fast fps
operating modes where it could significantly increase the computation
cycles (per unit time) without providing any significant changes to the
IQ parameters. The added latencies could also cause dropped frames.
Pass the FrameBuffer timestamp to the IPA through the controls. This
timestamp will be used to rate-limit the controller algorithms to run
with a minimum inter-frame time given by a compile time constant,
currently set to 16.66ms. On startup, we don't rate-limit the algorithms
until after the number of frames required for convergence.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Report the sensor's timestamp in the Request metadata by using the
Unicam::Image buffer timestamp as an initial approximation.
The buffer's timestamp is recorded at DMA-transfer time, and it does not
theoretically matches the 'start of exposure' definition, but when used
to compare two consecutive frames it gives an acceptable estimation of
the sensor frame period duration.
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
When a Request is completed upon receiving the IPA produced metadata,
the metadata associated with the Request are over-written, deleting
the information set at output buffer completion, such as the
SensorTimestamp.
This commit applies to the RaspberryPi pipeline handler the same change
applied to IPU3 in commit 2ee3dd57e2fc ("libcamera: ipu3: Merge IPA
metadata controls").
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Report the sensor's timestamp in the Request metadata using the
completed buffer timestamp.
The buffer's timestamp reports the video capture buffer processing time,
and it does not theoretically matches the 'start of exposure'
definition.
VIMC being a testing platform and the test driver completes the buffers
for each media entity connected in the pipeline one after the other, the
current solution is acceptable for the pipeline.
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Umang Jain
Laurent Pinchart
Jacopo Mondi
Kieran Bingham
Naushir Patuck
David Plowman
Hirokazu Honda
Paul Elder
Phi-Bang Nguyen