| Age | Commit message (Collapse) | Author |
|---|
|
The unit test associates a fence with a framebuffer and makes sure
the fence is correctly signalled.
Once a fence is correctly signalled, it is released by the core, and the
underlying file descriptor closed.
The unit test however tries to write on the fence file descriptor every
time the designated Request is queued, an error which is now visible
as the return value of the fence signalling write() is checked.
Fix that by associating a fence with a framebuffer only once.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
|
|
The ::write() function used to signal a framebuffer fence in the unit
test is marked with the 'warn_unused_result'.
When building in debugoptimized mode not checking for the return
value causes issues at build time:
/test/fence.cpp:254:2: error: ignoring return value of function declared with 'warn_unused_result' attribute
Fix that by checking the ::write() return value and emitting an
error message in case the write fails.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reported-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
|
|
Add a test for the Fence class by testing a Fence failure case, and
by testing a successfully signalled fence capture cycle.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
For consistency with UniqueFD, rename the fd() function to get().
Renaming UniqueFD::get() to fd() would have been another option, but was
rejected to keep as close as possible to the std::shared_ptr<> and
std::unique_ptr<> APIs.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
|
|
Now that we have a UniqueFD class, the name FileDescriptor is ambiguous.
Rename it to SharedFD.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
|
|
IPCUnixSocket::create() creates two file descriptors. One of
them is stored in IPCUnixSocket and the other is returned to a
caller. This clarifies the ownership using UniqueFD.
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
|
|
Add a unit test to exercise the API of the UniqueFD class.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
|
|
The FileDescriptor class is a generic helper that matches the criteria
for the base library. Move it there.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
|
|
gst_element_get_request_pad() is being replaced by gst_element_request_simple()
since 1.19.1, throwing a warning in case of use until its definitive
replacement on 1.20.
Hence, prefer using gst_element_request_simple() in case gstreamer
version is >= 1.19.1 to avoid the compilation error below (tested on f35):
[258/391] Compiling C++ object test/gstreamer/multi_stream_test.p/gstreamer_multi_stream_test.cpp.o
FAILED: test/gstreamer/multi_stream_test.p/gstreamer_multi_stream_test.cpp.o
c++ -Itest/gstreamer/multi_stream_test.p -Itest/gstreamer -I../test/gstreamer -Itest/libtest -I../test/libtest -Iinclude -I../include -Iinclude/libcamera/ipa -Iinclude/libcamera -I/usr/include/gstreamer-1.0 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -g -Wshadow -include config.h -pthread -DLIBCAMERA_BASE_PRIVATE -MD -MQ test/gstreamer/multi_stream_test.p/gstreamer_multi_stream_test.cpp.o -MF test/gstreamer/multi_stream_test.p/gstreamer_multi_stream_test.cpp.o.d -o test/gstreamer/multi_stream_test.p/gstreamer_multi_stream_test.cpp.o -c ../test/gstreamer/gstreamer_multi_stream_test.cpp
../test/gstreamer/gstreamer_multi_stream_test.cpp: In member function ‘virtual int GstreamerMultiStreamTest::run()’:
../test/gstreamer/gstreamer_multi_stream_test.cpp:90:76: error: ‘GstPad* gst_element_get_request_pad(GstElement*, const gchar*)’ is deprecated: Use 'gst_element_request_pad_simple' instead [-Werror=deprecated-declarations]
90 | g_autoptr(GstPad) request_pad = gst_element_get_request_pad(libcameraSrc_, "src_%u");
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/gstreamer-1.0/gst/gstbin.h:27,
from /usr/include/gstreamer-1.0/gst/gst.h:35,
from ../test/gstreamer/gstreamer_multi_stream_test.cpp:13:
/usr/include/gstreamer-1.0/gst/gstelement.h:1042:25: note: declared here
1042 | GstPad* gst_element_get_request_pad (GstElement *element, const gchar *name);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
When tests are run under valgrind, /proc/self/exe points to valgrind,
not to the test binary. This results in failures for tests that need to
fork processes. Fix it by replacing "/proc/self/exe" with the path to
the test binary.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
Store the path to the test executable, found in argv[0], in the Test
instance. This can be useful for tests that need to fork processes.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The error message should be:
"Failed to associate buffer with request"
Fix that.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
Remove the verbose #ifndef/#define/#endif pattern for maintaining
header idempotency, and replace it with a simple #pragma once.
This simplifies the headers, and prevents redundant changes when
header files get moved.
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
|
|
The unscoped enum BayerFormat::Packing leads to usage of the ambiguous
BayerFormat::None enumerator. Turn the enumeration into a scoped enum to
force usage of BayerFormat::Packing::None, and drop the now redundant
"Packed" suffix for the CSI2 and IPU3 packing.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
"using namespace" in a header file propagates the namespace to
the files including the header file. So it should be avoided.
This removes "using namespace" in header files in test.
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>
|
|
Add four new member functions to the Size class (two in-place and two
const) to grow and shrink a Size by given margins.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
|
|
Remove header files which were not being used in the test code.
The following headers were removed from the gstreamer_single_stream_test:
- libcamera/base/utils.h
- libcamera/internal/source_paths.h
Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
|
|
Two independent instances of the ControlSerializer class are in use at
the IPC boundaries, one in the Proxy class that serializes data from the
pipeline handler to the IPA, and one in the ProxyWorker which serializes
data in the opposite direction.
Each instance operates autonomously, without any centralized point of
control, and each one assigns a numerical handle to each ControlInfoMap
it serializes. This creates a risk of potential collision on the handle
values, as both instances will use the same numerical space and
are not aware of what handles has been already used by the instance "on
the other side".
To fix that, partition the handles numerical space by initializing the
control serializer with a seed according to the role of the component
that creates the serializer and increment the handle number by 2, to
avoid any collision risk.
While this is temporary and rather hacky solution, it solves an issue
with isolated IPA modules without too much complexity added.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
This patch adds a test to test if multi stream using libcamera's
gstreamer element works.
Test will run only on devices that support multistream capture, eg.,
devices that use IPU3 and raspberrypi pipeline. This was tested on
a Raspberry Pi 4B+.
Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
|
|
As the vimc scaler prevents us from passing v4l2-compliance, skip it
until the fix has been merged. Instead of removing it from the
supported_pipelines list, add an extra check, since we will have the
same construct later when we check for the kernel version.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
In gstreamer, when elements are created, usually a floating [1]
reference is returned which simply means, there is no ownership
transfer (yet). Once can simply check for NULL and return through
an error path, without bothering to clean up. Hence, g_autoptr is
not much of help here.
If the NULL checks have been passed successfully, elements are ready
to use. However, we must claim ownership/reference it before using
them via g_object_ref_sink().
This patch build upon this principle and removes the g_autoptr
from gstreamer test base class (gstreamer_test.cpp) whereever
necessary to tide up the code.
[1] https://gstreamer.freedesktop.org/documentation/additional/design/MT-refcounting.html?gi-language=c#refcounting1
Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
|
|
Simplify memory handling and complexity of the test by using
gst_parse_bin_from_description_full [1].
[1]: https://gstreamer.freedesktop.org/documentation/gstreamer/gstutils.html?gi-language=c#gst_parse_bin_from_description_full
Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
|
|
The test hold a valid reference to convert0_ and sink0_ but
not released. This results in a memory leak and can be checked
via valgrind. Drop the references with test cleanup() virtual
function.
Valgrind log (glib and gst suppression files were used):
==345380== LEAK SUMMARY:
==345380== definitely lost: 1,688 bytes in 2 blocks
==345380== indirectly lost: 7,069 bytes in 42 blocks
The patch fixes the leaks reported by valgrind above to:
==348870== LEAK SUMMARY:
==348870== definitely lost: 0 bytes in 0 blocks
==348870== indirectly lost: 0 bytes in 0 blocks
Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
|
|
The V4L2 compatibility layer test includes a list of supported devices,
as V4L2 compatibility isn't officially supported with all devices yet.
If no supported device is present, the test reports success, while it
actually hasn't run. Report it being skipped in that case.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
|
|
The ipa_data_serializer_test is configured to be able to run in parallel
with other tests.
If the test tries to acquire the VIMC sensor B while it is in use, it
would skip require skipping the test.
Instead, inform meson that it can not be run in parallel to ensure that
the test is able to obtain the resources it requires for execution.
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
v4l2-compliance changed their version string:
v4l2-compliance 1.21.0-4618
v4l2-compliance SHA: cc211b76476aca2c072ffa83a9b003957d5f3909, 64 bits, 64-bit time_t
v4l2-compliance 1.21.0-4838, 64 bits, 64-bit time_t
The current parsing takes the last result of split, which works for the
former, but not the latter. Take the second result of split instead, and
strip away any commas.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
The destructor tried to check if pipeline_ is a parent of libcameraSrc_.
This was needed to be checked as if it is, cleanup of libcameraSrc_
would be handled by pipeline itself.
Since, the destructor can be called anytime, even when pipeline_ hasn't
been created, the use of pipeline_ to check if libcameraSrc_ has an
ancestor as pipeline_ caused a segmentation fault.
Fixes: f58768092277 ("test: gstreamer: Fix the destructor of GstreamerTest base class")
Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
A lot of code used in the single stream test is boiler plate and common
across every gstreamer test. Factor out this code into a base class
called GstreamerTest.
Also update the gstreamer_single_stream_test to use the GstreamerTest
base class.
Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
|
|
The V4L2VideoDevice::toV4L2PixelFormat() function is incorrectly
implemented, as it will pick a multi-planar format if the device
supports the multi-planar API, even if only single-planar formats are
supported. This currently works because the implementation calls
V4L2PixelFormat::fromPixelFormat(), which ignores the multiplanar
argument and always returns a single-planar format.
Fixing this isn't trivial. As we don't need to support multi-planar V4L2
formats at this point, drop the function instead of pretending
everything is fine, and call V4L2PixelFormat::fromPixelFormat() directly
from pipeline handlers. As the single-planar case is the most common,
set the multiplanar argument to false by default to avoid long lines.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
|
|
The index generated by utils::enumerate() is an iteration counter, which
should thus be positive. Use std::size_t instead of the difference_type
of the container.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
|
|
Many signals used in internal and public APIs carry the emitter pointer
as a signal argument. This was done to allow slots connected to multiple
signal instances to differentiate between emitters. While starting from
a good intention of facilitating the implementation of slots, it turned
out to be a bad API design as the signal isn't meant to know what it
will be connected to, and thus shouldn't carry parameters that are
solely meant to support a use case specific to the connected slot.
These pointers turn out to be unused in all slots but one. In the only
case where it is needed, it can be obtained by wrapping the slot in a
lambda function when connecting the signal. Do so, and drop the emitter
pointer from all signals.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
|
|
In many cases, the emitter object passed as a pointer from signals to
slots is also available as a class member. Use the class member when
this occurs, to prepare for removal of the emitter object pointer from
signals.
In test/event.cpp, this additionally requires moving the EventNotifier
to a class member.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
|
|
It can be useful to connect a signal to a functor, and in particular a
lambda function, while still operating in the context of a receiver
object (to support both object-based disconnection and queued
connections to Object instances).
Add a BoundMethodFunctor class to bind a functor, and a corresponding
Signal::connect() function. There is no corresponding disconnect()
function, as a lambda passed to connect() can't be later passed to
disconnect(). Disconnection typically uses disconnect(T *object), which
will cover the vast majority of use cases.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
|
|
When disconnecting a signal from a receiver, it is usually not necessary
to specify the receiver's slot function explicitly, as the signal is
often connected to a single slot for a given receiver. We can thus use a
simpler version of Signal::disconnect() that takes a pointer to the
receiver object only. This reduces code size, as the disconnect()
function is a template function.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
|
|
The V4L2 compat test runs v4l2-ctl and v4l2-compliance with
v4l2-compat.so preloaded. If libcamera is compiled with the address
sanitizer enabled, the ASan library will be loaded due to preloading
v4l2-compat.so. This however doesn't occur early enough in the dynamic
linking process due to the v4l2 executables not being themselves linked
to the ASan runtime, which causes ASan to abort with
==2198==ASan runtime does not come first in initial library list; you should either link runtime to your application or manually preload it with LD_PRELOAD.
Using LD_PRELOAD to load the ASan runtime would fix this issue, but it
requires knowing the absolute path to the ASan shared object. This is
compiler-dependent and for clang, architecture-dependent as well.
Until we figure out how to safely retrieve that information, disable the
test when ASan is enabled as a quick fix.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
|
|
In CameraSensor, the mbusCodes() and sizes() accessor functions
retrieves all the supported media bus codes and the supported sizes
respectively. However, this is quite limiting since the caller
probably isn't in a position to match which range of sizes are
supported for a particular mbusCode.
Hence, the caller is most likely interested to know about the sizes
supported for a particular media bus code. This patch transforms the
existing CameraSensor::sizes() to CameraSensor::sizes(mbuscode) to
achieve that goal.
The patch also transforms existing CIO2Device::sizes() in IPU3 pipeline
handler to CIO2Device::sizes(PixelFormat) on a similar principle. The
function is then plumbed to CameraSensor::sizes(mbusCode) to enumerate
the per-format sizes as required in
PipelineHandlerIPU3::generateConfiguration().
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Tested-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
ASan needs to be loaded first before gstreamer is loaded. This was not
possible, so verify_asan_link_order was disabled. Better way to tackle
this issue was disabling forks on the gstreamer side.
verify_asan_link_order=0 disables the check on ASan side which checks if
ASan was loaded before any other shared objects. Since, gstreamer spawns
a child helper process while building the registry, we needed to disable
this check. But with gst_registry_fork_set_enabled() it is possible to
disable spawning this child helper process, so this ensures that ASan is
loaded before any other shared object is loaded.
Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
This patch simplifies memory management, i.e., by replacing bare
pointers with g_autoptr or g_autofree according to use case.
While at it also update time representation of timeout variable with
GST_SECOND.
Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Regarding (de)serialization in isolated IPA calls, we have four layers:
- struct
- byte vector + fd vector
- IPCMessage
- IPC payload
The proxy handles the upper three layers (with help from the
IPADataSerializer), and passes an IPCMessage to the IPC mechanism
(implemented as an IPCPipe), which sends an IPC payload to its worker
counterpart.
When a FileDescriptor is involved, previously it was only a
FileDescriptor in the first layer; in the lower three it was an int. To
reduce the risk of potential fd leaks in the future, keep the
FileDescriptor as-is throughout the upper three layers. Only the IPC
mechanism will deal with ints, if it so wishes, when it does the actual
IPC. IPCPipeUnixSocket does deal with ints for sending fds, so the
conversion between IPCMessage and IPCUnixSocket::Payload converts
between FileDescriptor and int.
Additionally, change the data portion of the serialized form of
FileDescriptor to a 32-bit unsigned integer, for alightnment purposes
and in preparation for conversion to an index into the fd array.
Also update the deserializer of FrameBuffer::Plane accordingly.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Tested-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
|
|
Development and usage of isolated IPA's has identified bugs in the
transmission of the file descriptors between the pipeline handler and
the IPA through.
Add tests to identify these bugs and prevent regressions.
This tests basically checks for two things:
- Camera reconfigurations without stopping CameraManager
- Fd leaks across IPA IPC boundary [1]
Currently, it uses vimc, but can be easily changed to using another
platform (for e.g. IPU3) by changing kCamId_ and kIpaProxyName_.
The test performs kNumOfReconfigures_ (currently set to 10)
reconfigurations of the camera. Each reconfiguration runs
start(), capture(100ms), stop() of the camera. Hence, the test
runs approximately for 1 second.
For checking the fd leaks, the test monitors the /proc/$PROXY_PID/fd
directory for open fds. It compares the number of open fds after each
run to the number of open fds before the first run. If those two are
found to be mis-matched, the test shall report failure.
The test validates IPA IPC code paths which are used when the IPA is run
in isolated mode. The CameraTest is constructed with the isolate flag
set to enforce this.
[1] https://bugs.libcamera.org/show_bug.cgi?id=63
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
Some tests might require to have LIBCAMERA_IPA_FORCE_ISOLATION set
to ensure they can test the IPA running in isolated mode. These tests
are likely to leverage CameraTest. The environment variable should
be set before CameraManager::start() call which happens in CameraTest's
constructor. Hence, plumb the constructor with a flag so that the
LIBCAMERA_IPA_FORCE_ISOLATION can be set before CameraManager::start().
Signed-off-by: Umang Jain <umang.jain@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>
|
|
This patch adds a test to test if single stream using libcamera's
gstreamer element works.
We need to work around two distinct issues with ASan when enabled in the
build:
- glib has a known leak at initialization time. This is covered by the
suppression file shipped with glib, but it's not clear how to use it
automatically. For now, disable leak detection to avoid test failures.
- GStreamer spawns a child process to scan plugins. If GStreamer is
compiled without ASan (which is likely) but libcamera is, dlopen()ing
the libcamera plugin will cause an ASan link order verification
failure. Disable the verification child processes to work around the
problem. This requires gcc 8 or newer.
Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
Reviewed-by: Paul Elder <paul.elder@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>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Test that lookup by ControlId reference works in the control
serialization test making sure that the control limits are not changed by
de-serialization.
The test currently fails and demonstates that lookup by ControlId is
currently not supported until the introduction of the next patch.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@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>
|
|
Remove the need for callers to reference PROT_READ/PROT_WRITE directly
from <sys/mman.h> by instead exposing the Read/Write mapping options as
flags from the MappedFrameBuffer class itself.
While here, introduce the <stdint.h> header which is required for the
uint8_t as part of the Plane.
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The MappedFrameBuffer is a convenience feature which sits on top of the
FrameBuffer and facilitates mapping it to CPU accessible memory with
mmap.
This implementation is internal and currently sits in the same internal
files as the internal FrameBuffer, thus exposing those internals to
users of the MappedFramebuffer implementation.
Move the MappedFrameBuffer and MappedBuffer implementation to its own
implementation files, and fix the sources throughout to use that
accordingly.
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@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>
|
|
Add type safety by turning the MapFlag and OpenModeFlag enum into enum
class.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
|
|
Add tests that exercise the whole API of the Flags class.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
It would be convenient to be able to iterate over available boolean
values, for example for controls that designate if some function can be
enabled/disabled. The current min/max/def constructor is insufficient,
as .values() is empty, so the values cannot be easily iterated over, and
creating a Span of booleans does not work for the values constructor.
Add new constructors to ControlInfo that takes a set of booleans (if
both booleans are valid values) plus a default, and another that takes
only one boolean (if only one boolean is a valid value).
Update the ControlInfo test accordingly.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
|
