Provide a sequence number on Requests which are added by the pipeline
handler.
Each pipeline handler keeps a requestSequence per CameraData and
increments everytime a request is queued on that camera.
The sequence number is associated with the Request and can be utilised
for assisting with debugging, and printing the queueing sequence of in
flight requests.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The FrameBuffer class is only friends with Request so that the request
can be associated with the buffers.
FrameBuffer already has a helper to setRequest(), so let's use that
directly instead.
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Asynchronous tasks can only be submitted while the IPA is running.
Further more, the shutdown sequence can not be tracked with a simple
running flag. We can also be in the state 'Stopping' where we have not
yet completed all events, but we must not commence anything new.
Refactor the running_ boolean into a stateful enum to track this.
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
A range-based for loop whose range expression is an array of char
pointers and range variable declaration is a const reference to a
std::string creates a temporary string from the char pointer and binds
the range variable reference to it. This creates a const reference to a
temporary, which is valid in C++, and extends the lifetime of the
temporary to the lifetime of the reference.
However, lifetime extension in range-based for loops is considered as a
sign of a potential issue, as a temporary is created for every
iteration, which can be costly, and the usage of a reference in the
range declaration doesn't make it obvious that the code isn't simply
binding a reference to an existing object. gcc 11, with the
-Wrange-loop-construct option, flags this:
uvcvideo.cpp:432:33: error: loop variable 'name' of type 'const string&' {aka 'const std::__cxx11::basic_string<cha
r>&'} binds to a temporary constructed from type 'const char* const' [-Werror=range-loop-construct]
| 432 | for (const std::string &name : { "idVendor", "idProduct" }) {
| | ^~~~
To please the compiler, make the range variable a const char *. This may
bring a tiny performance improvement, as the name is only used once, in
a location where the compiler can use
operator+(const std::string &, const char *)
instead of
operator+(const std::string &, const std::string &)
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Use a const char * type instead of auto, and update the commit message
accordingly.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The exact same comment is used two times within 5 lines.
Remove one of them as it is redundant.
Signed-off-by: Sebastian Fricke <sebastian.fricke@posteo.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Do not send events back to the pipeline handler, while the IPA thread
has not been started.
If information needs to be returned from configure it should be returned
directly.
Move the initial call to setControls() out of configure() and into the
start() method which is called after the IPA running_ state is updated.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Sebastian Fricke <sebastian.fricke@posteo.net>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The destructor stops the camera manager if it is running. This is at the
moment an implementation detail, but makes sense to guarantee in the
API. Document it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sebastian Fricke <sebastian.fricke@posteo.net>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reference the issue tracker in the documentation, as relying on tribal
knowledge isn't a great option.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sebastian Fricke <sebastian.fricke@posteo.net>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
We return early with subdir_done() if android_enabled is false. There's
no need to check the variable later in the file.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
CameraDevice::getResultMetadata() doesn't change either
|descriptor| and member variables. It should be marked as a
const function and |descriptor| should be passed with const
lvalue reference.
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>
CameraMetadata stored in requestTemplates_ in CameraDevice is
not necessary to be a raw pointer. This reduces the manual
new/delete code by changing the type to std::unique_ptr.
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
staticMetadata_ in CameraDevice is not necessary to be a raw
pointer. This reduces the manual new/delete code by changing the
type to std::unique_ptr.
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Camera3RequestDescriptor has a length and an allocated buffer
for camera_stream_buffer_t array. This replaces the variables
with std::vector.
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
CameraDevice::camera() originally returns shared_ptr. It is
mandatory to make a copy by calling camera() in this way. There
is no need of copying if a caller needs the reference of the
camera like const shared_ptr<Camera> &cam = camera(). That is, it
is a caller that copying is required. This changes the return
type of camera() to const shared_ptr&, so that we are able to
reduce one redundant copy in the above case.
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
CameraDevice takes the ownership of Camera. Therefore,
shared_ptr would rather be used than const shared_ptr&.
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
libcamera::Camera::id() is called after std::move() in
cameraAdded(). This fixes the issue by not executing std::move().
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
CameraManager is owned by CameraHalManager. The ownership of the
object is not shared with other classes. So CameraHalManager
should manage CameraManager with std::unique_ptr.
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
CameraDevice is owned by CameraHalManager. The ownership of the
object is not shared with other classes. So CameraHalManager
should manage CameraDevice with std::unique_ptr.
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The V4l2 buffer cache allows us to map incoming buffers to an instance
of the V4L2 buffer required to actually queue.
If the cache_ is not available, then the buffers required to allow
queuing to a device have been released, and this indicates an issue at
the pipeline handler.
This could be a common mistake, as it could happen if a pipeline handler
always requeues buffers to the device after they complete, without
checking if they are cancelled.
Catch any invalid queueing of buffers to the V4L2 video device when
resources have been released by adding a Fatal log message to highlight
the error during development.
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The configure operation is synchronous and should not send events back
to the pipeline handler.
If information needs to be returned from configure it should be handled
through the interface directly.
Move the initial call to setControls() out of configure() and into the
start() method which is called after the IPA running_ state is updated.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Ensure that the buffers are associated with the request even if they are
used internally to be able to correctly map back to the resources they
are being used to fulfil.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Ensure that checks on resource availability are handled first, and then
operate on the queues only after the resources are confirmed as
available.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
When the imguOutputBufferReady() detects a cancelled frame, it is
reporting that the metadata has been processed in order to be able to
complete the cancelled request.
This causes the FrameInfo to be completed and deleted early, but then an
active buffer on the IMGU can complete and be unable to find the
FrameInfo for it to complete correctly.
Do not mark metadataProcessed early on the event that a single buffer is
detected as cancelled. The stopping of the V4L2 devices will ensure
that all queued buffers are returned to us and we can follow the normal
and expected shutdown sequence.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The IPA should be stopped before the hardware devices to ensure that
all asynchronous actions have completed within the IPA before resources
are removed and released.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Events may be queued to the pipeline handler between the pipeline
handler entering the ::stop() function, and before the call to stop the
IPA has completed.
Handle these events by dispatching all pending messages at the proxy
after the IPA has fully stopped.
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signals and calls from the IPA should not occur after the IPA has been
put into the stopped state.
Add assertions to catch and prevent any messages being processed after
this.
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Building the cam application isn't always desired, for instance in a
production environment that only needs the libcamera library. Add a
meson option to disable it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reduce the indentation in the gstreamer and qcam meson.build with
subdir_done(). This follows similar patterns in cam and v4l2.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Rename ConfigInput to IPAConfig to be more consistent with the naming,
and remove ConfigInput::op, as it is never used.
Replace ConfigOutput with a ControlList type, as that is the only return
type from ipa::configure().
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Separate out the in and out parameters in ipa::start() as they are not
the same. This function now takes in a ControlList and returns out a
struct StartConfig which holds a ControlList and drop frame count for
the pipeline handler to action.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The lastMode_ member variable is now unused, so remove it.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The controller initialise is a one-off operation, so move it from
ipa::configure() to ipa::init().
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Conditionally open the embedded data node in pipeline_handler::match()
based on whether the ipa::init() result reports if the sensor supports
embedded data or not.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Move the opening of the CamHelper from ipa::configure() to ipa::init().
This allows the pipeline handler to get the sensor specific parameters
in pipeline_handler::match() where the ipa is initialised.
Having the sensor parameters available earlier will allow selective
use of the embedded data node in a future change.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Pass the sensor model string to the IPA init() method through the
IPASettings structure.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The correct return value for the HAL for hal_dev_open() when trying to
open a camera that's already opened is EUSERS. Make hal_dev_open()
return -EUSERS, and plumb the logic for this through
CameraHalManager::open().
This allows the following CTS tests to pass:
- android.hardware.camera2.cts.CameraManagerTest#testCameraManagerOpenAllCameras
- android.hardware.camera2.cts.MultiViewTest#testDualCameraPreview
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>
There are std::unique_ptr rvalue reference arguments. They are
intended to pass the ownership to the functions. In the case,
it is right to let the argument be std::unique_ptr value and
pass by std::move().
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Now that the camera location is not defaulted anymore in the library but
doesn't get registered if the firmware interface does not provide the
information, do not default it to FRONT if LocationExternal is reported.
To maintain compatibility with CTS requirements, default location to
FRONT only if the camera property is not available.
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Do not register the Location property if not available from the firmware
interface instead of defaulting it to External.
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
In preparation to register the Location property only if the firware
interface provides it, do not assume it is available and build the
camera name using the camera sensor model as a fallback.
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
After commit 4671911df0 ("pipeline: simple: Use breadth-first search
to setup media pipeline"), the explanation in the SimplePipeline
documentation how the handler tries to find a valid path to capture
device does not reflect the reality anymore. Update the text to the new
situation.
Fixes: 4671911df0 ("pipeline: simple: Use breadth-first search to setup media pipeline")
Signed-off-by: Marian Cichy <m.cichy@pengutronix.de>
Reviewed-by: Sebastian Fricke <sebastian.fricke@posteo.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The number of frames passed to the -C flag of cam should come right
after it, without a space, otherwise the value is discarded.
The log below already showed the correct usage, but the command in the
code-block was wrong, so fix it.
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Reviewed-by: Sebastian Fricke <sebastian.fricke@posteo.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The pipeline-handler guide is a bit outdated:
- ControlList* in start() is now const.
- Registered pipeline handler log now comes from Camera log and the message is different.
Update the pipeline-handler guide to reflect these changes.
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Reviewed-by: Sebastian Fricke <sebastian.fricke@posteo.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Fix some links to the V4L2 documentation. The first one is indeed
pointing to the wrong page. The second one has a highlight embedded, so
not necessarily wrong, but not needed, so clean it up as well.
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Reviewed-by: Sebastian Fricke <sebastian.fricke@posteo.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The usage of # in embedded URIs for references to sections in
reStructuredText is HTML specific. Use the standard rST reference to
targets instead, to be compatible with other formats as well, should
this documentation ever be exported to PDF, for example.
See [1] for an equivalent commit in the Linux Kernel documentation.
[1] https://lore.kernel.org/linux-doc/20201228144537.135353-1-nfraprado@protonmail.com/
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Reviewed-by: Sebastian Fricke <sebastian.fricke@posteo.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Fix some typos and some literals using ` instead of ``.
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Reviewed-by: Sebastian Fricke <sebastian.fricke@posteo.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The documentation of the controls() method refers to the methods
ControlList::operator[]() and ControlList::update(), which do not exist.
Instead refer to ControlList::get() and ControlList::set() to achieve a
similar documentation.
Fixes: a8c40942b9 ("libcamera: controls: Improve the API towards applications")
Signed-off-by: Marian Cichy <m.cichy@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
