Take the unique pointer to the `Fence` object by rvalue reference
so that it is not destroyed if the function returns an error code
and does not take ownership of the unique pointer.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
In the near future we will add a SyncAdjustment control for adjusting
the frame duration via the sync algorithm. This control needs to be able
to take on a negative value, since the frame duration can be shortened
in addition to being extended. While the control is an int, it would be
convenient to be able to clamp it to frame duration limits, which are
usually handled as utils::Duration values internally. To allow this
using utils::Duration, add a unary negation operation to
utils::Duration. Also add a test for the operator.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
In current implementation, the sink pad counter of the crossbar is not
incremented if the pad is not connected to any subdevice. This would lead
to incorrect routing and format configuration if CSI is not connected
to first sink pad.
To avoid such issue, every sink pads must be taken into account. Then if
CSI and sensor are present, current counter is used for routing at match(),
and stored in camera data to be reused during configure().
Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
Tested-by: Pavel Löbl <pavel@loebl.cz>
Tested-by: Julien Vuillaumier <julien.vuillaumier@nxp.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
In `Semaphore::release()`, unlocking the mutex before signalling the condition
variable can be problematic, especially with "temporary" objects such as the
ones `BoundMethodBase::activatePack()` uses to handle `ConnectionTypeBlocking`.
Specifically, `Semaphore::acquire()` might lock the mutex after `Semaphore::release()`
has unlocked it, but before it had the chance to notify the condition variable.
In that case `Semaphore::acquire()` can succeed, and execution may proceed to
destroy the `Semaphore` object while the other thread is in the process of
running `std::condition_variable::notify_all()`.
Bug: https://bugs.libcamera.org/show_bug.cgi?id=225
Fixes: 66e7c5b774 ("libcamera: Add Semaphore class")
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The function is not actually thread safe contrary to its documentation.
Since it is currently not used in an unsafe context, simply remove the
mention of thread safety from the documentation.
The variable must still remain atomic because it is accessed internally
from different threads, e.g. `Thread::postMessage()`.
Also add an assertion to enforce this.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
It is often helpful to know which tuning file is used. Add a log
statement with INFO level for that.
As the core logic has multiple return paths, adding the log statement is
not straight forward. Extract finder logic into a ipaConfigurationFile()
function and call that with the name and optionally the fallbackName
from the configurationFile() function.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
There is no reason to allocate the frame info objects dynamically,
and then store raw pointers in the `std::map` in the rkisp1
and ipu3 pipeline handler.
Instead, store the objects directly in the map. This removes
the need for manually calling new/delete, simplifies the code,
and eliminates one memory allocation per frame.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Top-level `const` qualifiers are not useful, so avoid them. This is done
either by simply removing the top-level `const`, or making the function
return a reference to const where that is appropriate.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
This reverts commit 10cdc914da.
The constructors introduced by that commit are not used anywhere,
and they do not match the existing practice for boolean controls.
Specifically, every single boolean control is described by calling
the `ControlInfo(ControlValue, ControlValue, ControlValue)`
constructor. Crucially, that constructor does not set `values_`,
while the two removed constructors do. And whether or not `values_`
has any elements is currently used as an implicit sign to decide
whether or not the control is "enum-like", and those are assumed
to have type `int32_t`.
For example, any boolean control described using any of the two
removed constructors would cause an assertion in failure in
`CameraSession::listControls()` when calling `value.get<int32_t>()`.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
Add unit cell size from the 'pixel size' element in the datasheet.
Delays are set to 2 in case a setting is entered at the very end of the
N frame, the N+1 frame will miss it and only the N+2 frame will use this
new setting.
Note that vd56g3 has a diagonal color test pattern, but does not match
any description in specified test patterns and therefore is ignored.
Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The `ProcessManager` is a singleton class to handle `SIGCHLD` signals
and report the exit status to the particular `Process` instance.
However, having a singleton in a library is not favourable and it is
even less favourable if it installs a signal handler.
Using pidfd it is possible to avoid the need for the signal handler;
and the `Process` objects can watch their pidfd themselves, eliminating
the need for the `ProcessManager` class altogether.
`P_PIDFD` for `waitid()` was introduced in Linux 5.4, so this change
raises the minimum supported kernel version. `clone3()`, `CLONE_PIDFD`,
`pidfd_send_signal()` were all introduced earlier.
Furthermore, the call to the `unshare()` system call can be removed
as those options can be passed to `clone3()` directly.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Instead of creating a new vector, take the vector by value to make it
possible for the caller to use move construction when calling the function.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Instead of using a separate member variable, use `pid_ > 0` to determine
if the process is still running. Previously the value of `pid_` was not
reset to -1 when the process terminated, but since it is only meaningful
while the process is running, reset it to -1 in `Process::died()`.
Neither `pid_` nor `running_` are exposed, so this change has no effect
on the public interface or observable behaviour.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
When no log category is specified, `nullptr` is passed, and
then the `_log()` function implementations replace that with
`LogCategory::defaultCategory()`. But since the call site always
knows the log category, this condition can be removed and the
`_LOG1()` macro can use `LogCategory::defaultCategory()`.
So remove the condition from the `_log()` implementations and
use references to refer to log categories.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Most pipeline handler methods are dispatched in the internal `CameraManager`
thread. `PipelineHandler::generateConfiguration()` is called directly,
however, which likely goes against the expectations, so call it on the
internal thread.
Before 14618cdd0c ("libcamera: base: bound_method: Move return value")
this could not be done because the function returns `std::unique_ptr`, and
that type cannot be copied.
Bug: https://bugs.libcamera.org/show_bug.cgi?id=273
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Accept empty `ControlList`s without an info map. Serialization
supports `ControlList`s without an associated `ControlInfoMap`
object. However, for deserialization, a "v4l2" control list
without an info map resulted in a fatal error.
After 30114cadd8 ("ipa: rpi: Defer initialising AF LensPosition ControlInfo and value")
the `lensControls` member of `ipa::RPi::ConfigResult` is left empty
and without an info map in every case, not just when a lens is not present.
This causes the above assertion to trigger every single time.
Also fix the indefinite article of "a ControlInfoMap" in the
error message.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Header comment blocks used to contain the file name. Considered as
useless information, the names have been removed, with the last ones
supposed to be dropped in commit d3bf27180e ("libcamera: Drop
remaining file names from header comment blocks"). A few have however
been forgotten, and more crept back since. Remove them.
While at it, fix one typo in a header comment block by replacing
'MaliC55 with Mali-C55', and add a missing blank line in
src/ipa/rpi/pisp/pisp.cpp.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
To debug color issues in a pipeline it is necessary to know the color
space used. Therefore add the color space to the string representation
of V4L2DeviceFormat that is returned by toString() and operator<<(). The
format gets for example printed as
[0:05:52.981684000] [230] DEBUG RkISP1 rkisp1_path.cpp:468 Configured main resizer video device with 1920x1080-NV12/sYCC
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <uajain@igalia.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Camera creation is one of the most important events generated by
libcamera, but we are completely silent about it. The lack of a log
message makes it more difficult to identify problems and provide
support. Fix it by adding an Info message that reports the camera id and
its pipeline handler when the camera is added.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Umang Jain <uajain@igalia.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The match() function currently reports that it is not possible to create
any cameras if it encounters an empty media graph.
Fix this by looping over all media graphs and only returning false when
all of them fail to create a camera.
It is worth noting that an issue does exist when on a partial match that
ends in an invalid match, any media devices that were acquired will stay
acquired. This is not a new issue though, as any acquired media devices
in general are not released until pipeline handler deconstruction. This
requires a rework of how we do matching and pipeline handler
construction, so it is captured in a comment.
In the meantime, this fix fixes a problem without increasing the net
number of problems.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Hui Fang <hui.fang@nxp.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
In Camera::queueRequest() the control list is updated transparently by
converting AeEnable into ExposureTimeMode and AnalogueGainMode
controls.
However, this was not happening during Camera::start(), meaning that
setting AeEnable there was having no effect. It should behave the same
here too.
Fixes: 7abd413905 ("libcamera: camera: Pre-process AeEnable control")
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
ISI can be used to support multiple cameras at the same time in the media
device. A dedicated pad is assigned to each camera by HW (dts). We then
assign one (or more) source pad(s) depending on ISI constraints in the SoC.
Since ISI may have different number of pipes depending on SoC, the number
of cameras is computed _before_ going through all pipeline's components,
to limit number of pipes that could be assigned to a camera.
For instance, 3 is targeted as maximum amount of streams per camera as
defined by 'kNumStreams' constant. If 2 cameras are connected, with
only 5 ISI pipes, this target cannot be achieved. In such case, 2 streams
are assigned to each camera.
On the other hand, if ISI has 8 source pads (from index 6 to 13) and 2
cameras, first three source pads [6:8] are assigned to first camera, and
the three next source pads [9:11] are assigned to the second camera. All
these pads (sink and sources) must have same format to prevent configuration
mismatch during start.
However, since ISI routing must be performed _before_ any stream becomes
active, 'setRouting' is now executed during the 'match' step, instead of
'configure'. Indeed, it is up to the application to decide when a camera
'start' is executed: this could happen before or after the other camera
configuration is executed. Moving routing into the 'match' step makes sure
the routing is correctly applied before any 'start' is executed.
Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
This patch adds number of streams per camera as constructor parameter, and
limits stream count to 3. Some applications may need up to 3 streams for
preview + capture + video record. Currently, imx8-isi pipeline only supports
up to 2. Increase constant parameter to 3 to match these applications'
requirements.
Minimum value between default value 3, and total amount of ISI's pipes is
now applied. For SOCs which only have 1 ISI pipe (ie i.MX93), available
stream count becomes 1.
Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The mali-c55 pipeline handler currently looks for a media entity with
the function MEDIA_ENT_F_IO_V4L to recognise a memory input subdevice.
This is apparently intended for video device entities, and we should
be looking for MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER. Correct the
entity function that the pipeline handler looks for.
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
At the moment the mali-c55 pipeline handler sets bytesused for a
buffer to be the maximum possible size (i.e. the size of a struct
mali_c55_params_buffer). This is not really in keeping with the goal
of the extensible parameters formats, and will not work with the new
framework for those formats. Update the IPA module and pipeline
handler to set bytesused to the size of the parameters that were
actually supplied rather than the maximum possible size.
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Type casting from unsigned int to int performed in stats computation is
unnecessary, window_.width is unsigned and the array index is always
non-negative. Let's simply use unsigned int in all the
SwStatsCpu:stats* methods.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Umang Jain <uajain@igalia.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
To debug color issues in a pipeline it is necessary to know the color
space used. Therefore add the color space to the string representation
of V4L2SubdeviceFormat that is returned by toString() and operator<<().
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
Currently the rkisp1 pipeline handler relies on bufferCount to decide on
the number of parameter and statistics buffers to allocate internally.
Instead, the number of internal buffers should be the minimum required
by the pipeline to keep the requests flowing, in order to avoid wasting
memory.
Stop relying on bufferCount for these numbers and instead set them to
kRkISP1MinBufferCount.
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Sven Püschel <s.pueschel@pengutronix.de>
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Umang Jain <uajain@igalia.com>
The StreamConfiguration::bufferCount is reset to a hardcoded value of 4
in RkISP1Path::validate(). Keep the minimum value of 4 but do not reset
it, if it was set to a larger value. This allows the user to set
bufferCount to an arbitrary number of buffers which then can be
allocated for example by the FrameBufferAllocator. If the bufferCount is
set to a smaller value it gets reset to kRkISP1MinBufferCount again and
the configuration is marked as adjusted.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Tested-by: Sven Püschel <s.pueschel@pengutronix.de>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Umang Jain <uajain@igalia.com>
