| Age | Commit message (Collapse) | Author |
|---|
|
Expecting pipeline handler implementations of queueRequest() to call
the base class queueRequest() at the correct point have led to different
behaviors between the pipelines.
Fix this by splitting queueRequest() into a base class implementation
which handles the bookkeeping and a new queueRequestDevice() that is
to be implemented by pipeline handlers and only deals with committing the
request to the device.
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Object instances receive messages dispatched from the event loop of the
thread they belong to. Deleting an object from a different thread is
thus dangerous, unless the caller ensures that no message delivery is in
progress. Document this in the Object class documentation.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
The Object class stores the number of pending messages that have been
posted for it, while the actual messages are stored in a per-thread list
in the ThreadData class. When dispatching messages, the message is
removed from the list, passed to the receiver, and the number of pending
messages is updated.
In order to avoid too much contention on the list lock, the lock is
dropped to pass the message to the receiver and then taken again. This
creates a race condition window with Thread::removeMessages(), as the
number of pending messages for a receiver is briefly out of sync with
the pending messages list. The assertion at the end of removeMessages()
thus sometimes fails.
Fix this by decrementing the pending messages counter before releasing
the lock in Thread::dispatchMessages(). This fixes the slow message
receiver test in MessageTest.
Fixes: 01b930964acd ("libcamera: thread: Add a messaging passing API")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
When moving an Object to a Thread, messages posted for the object are
move to the target thread's message queue. This requires locking the
message queues of the current and target threads, as the target thread
may (and is usually) running. The implementation is faulty as it locks
the thread data instead of the message queue. This creates a race
condition with a tiny but exploitable time window.
The issue was noticed by the event-thread test rarely but reproducibly
failing with the following assertion error:
[1:39:33.850878042]FATAL default thread.cpp:440 assertion "data_ == receiver->thread()->data_" failed
The issue only occurred when libcamera was compiled in release mode,
further hinting of a race condition.
Fixes: 01b930964acd ("libcamera: thread: Add a messaging passing API")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
When a fatal error occurs the program aborts, and all the logger
provides is the location of the line that caused the error. Extend this
with a full backtrace to help debugging.
The backtrace is generated using the backtrace() call, a GNU extension
to the C library. It is available in glibc and uClibc but not in musl.
Test for availability of the function to condition compilation of the
backtrace printing. Implementing backtrace support with musl is an
exercise left to the reader if desired.
The LogOutput class is extended to support writing string messages
directly to the output. Strings written directly will be considered as
LogDebug messages when written to the Syslog.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
gcc 8 and 9 complain about the OptionValue::integer_ member being
possibly used initialized when compiled in release mode. I haven't been
able to find where this could be the case, and the compiler error
message isn't helpful:
In file included from ../../src/cam/options.cpp:14:
../../src/cam/options.h: In member function ‘bool OptionsBase<T>::parseValue(const T&, const Option&, const char*) [with T = std::__cxx11::basic_string<char>]’:
../../src/cam/options.h:84:7: error: ‘<anonymous>.OptionValue::integer_’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
class OptionValue
^~~~~~~~~~~
../../src/cam/options.h: In member function ‘bool OptionsBase<T>::parseValue(const T&, const Option&, const char*) [with T = int]’:
../../src/cam/options.h:84:7: error: ‘<anonymous>.OptionValue::integer_’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
class OptionValue
^~~~~~~~~~~
Furthermore valgrind doesn't report any issue. This is likely a false
positive, but fix it nonetheless as the fix is cheap.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
Oxford English spells "serialize", not "serialise". Fix it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
When an IPA module is loaded without isolation and implements the
IPAInterface internally, going through ipa_context_ops is a waste of
time. Add an operation to retrieve the IPAInterface, and use it directly
in the IPAContextWrapper.
For debugging purpose, make it possible to forcing usage of the C API by
defining the LIBCAMERA_IPA_FORCE_C_API environment variable.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
IPA modules have to implement a public ipaCreate() function, but its
prototype isn't declared in any header file. This allows for modules to
get the prototype wrong without being warned by the compiler. Fix it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
Switch IPA communication to the plain C API. As the IPAInterface class
is easier to use for pipeline handlers than a plain C API, retain it and
add an IPAContextWrapper that translate between the C++ and the C APIs.
On the IPA module side usage of IPAInterface may be desired for IPAs
implemented in C++ that want to link to libcamera. For those IPAs, a new
IPAInterfaceWrapper helper class is introduced to wrap the IPAInterface
implemented internally by the IPA module into an ipa_context,
ipa_context_ops and ipa_callback_ops.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
The C++ objects that are expected to convey data through the IPA API
will have associated methods that would require IPAs to link to
libcamera. Even though the libcamera license allows this, suppliers of
closed-source IPAs may have a different interpretation. To ease their
mind and clearly separate vendor code and libcamera code, define a plain
C IPA API. The corresponding C objects are stored in plain C structures
or have their binary format documented, removing the need for linking to
libcamera code on the IPA side.
The C API adds three new C structures, ipa_context, ipa_context_ops and
ipa_callback_ops. The ipa_context_ops and ipa_callback_ops contain
function pointers for all the IPA interface methods and signals,
respectively. The ipa_context represents a context of operation for the
IPA, and is passed to the IPA oparations. The IPAInterface class is
retained as it is easier to use than a plain C API for pipeline
handlers, and wrappers will be developed to translate between the C and
C++ APIs.
Switching to the C API internally will be done in a second step.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
The IPAInterface::configure() operation receives a map of ControlInfoMap
instances. Pass const references instead to avoid copies when not
required (the callee can still make manual copies), and to allow for the
future serialization layer to keep references to the original object.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
Add a new ControlSerializer helper to serialize and deserialize
ControlInfoMap and ControlList instances. This will be used to implement
the C IPA protocol and the communication with IPA through IPC.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
The ByteStreamBuffer class wraps a memory area, expected to be allocated
by the user of the class and provides operations to perform sequential
access in read and write modes.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Define data structures to be used during interaction between IPA modules
and pipeline handlers to serialize control lists and control info maps.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
In order to inspect planes of a const Buffer, add a const accessor.
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>
|
|
V4L2 controls of type V4L2_CTRL_TYPE_BOOLEAN are incorrectly described
with a ControlRange of int32_t values. Fix it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
ControlInfoMap requires the ControlId and ControlRange of each entry to
have identical types. Check for this and log an error if a mismatch is
detected.
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>
|
|
Store a reference to the ControlInfoMap used to create a ControlList and
provide an operation to retrieve it. This will be used to implement
serialization of ControlList.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
We need to construct empty ControlList objects to serialization. Make
the constructor public.
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>
|
|
The ControlInfoMap class has a move assignment operator from a plain
map, but no corresponding move constructor. Add one.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
In preparation for serialization, index the ControlList by unsigned int.
This will allow deserializing a ControlList without requiring external
information.
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>
|
|
Allow comparision of control ranges by adding the required operators.
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>
|
|
The ControlInfoMap count() and find() methods use at() to lookup the
control numerical ID in the idmap_. This causes an exception to be
thrown if the ID doesn't exist in the map. Fix it by using the find()
method instead in find(), and rely on idmap_.count() in count().
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>
|
|
There is no need to forward declare BufferPool, drop it.
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Do not store the camera raw pointer in the capture class, this will
prevent forwarding the shared pointer in the future.
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
In completeRequest() the request argument is used as a local variable,
this is confusing. Add a separate local variable instead of reusing the
argument.
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
There is no point in explicitly checking the same error in the only call
sites for the internal function, centralize the check and simplify the
code.
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Other parts of the cam utility uses the libcamera namespace, do the same
in the buffer writer.
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
signal
The stream to buffer map in the requestCompleted signal is taken
directly from the request which is part of the same signal. Remove the
map as it can be fetched directly from the request.
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Neither uClibc nor uClibc-ng support O_TMPFILE. Define it in utils.h if
not defined.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
Remove spaces between empty curly brackets (replacing { } with {}) to
comply with the coding style. Fix one other coding style violation on
the lines touched by those fixes.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
Comply with the coding style by removing lots of unneeded semicolons.
Fix a few other coding style violations on the lines touched by those
fixes.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
Use the newly introduced InvocationTypeBlocking message type to replace
the blocking message delivery implemented with the ThreadRPC class in the
Android camera HAL.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
Allow specifying a different connection type than ConnectionTypeQueued
for Object::invokeMethod().
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
The BoundMethodBase::activatePack() and the internal
Object::invokeMethod() are duplicate implementation of the same
mechanism. Use the former to replace the latter.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
Support all connection types in the BoundMethodBase::activePack()
method. To support this, add a semaphore to the InvokeMessage to signal
delivery.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
Define an enumeration of connection types to describe the delivery
method of signals and method invocation.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
Add a general-purpose counting semaphore class.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
Use DRM FourCC values for the newly defined PixelFormat.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
In preparation for switching PixelFormat to DRM pixel formats, add
helper methods to the V4L2VideoDevice class to translate between DRM
pixel formats and V4L2 pixel formats.
More work is needed to properly support the V4L2 multi-planar
(NV[0-9]*M) formats.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
Use the newly defined PixelFormat type to define the image format
contained in the StreamFormats and StreamConfiguration classes.
Update the classes documentation accordingly.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Define a PixelFormat type as a simple typedef to an uint32_t. The usage
of a dedicated type creates a cleaner and more self-described API.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The libcamera name should never be capitalized. Fix the two incorrect
occurrences.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
Remove unused local map variable.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Fill the StreamConfiguration with all supported formats. The list of
supported formats is currently hardcoded based on the limits of the vimc
driver.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
|
|
If there is only one camera exposed by libcamera, there is little value
in asking the user to choose it.
Automatically select it, and remove the need to ask the user to select
'ok' from a Dialog box.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
Now that our usage of C compatibility header is documented, use them
consistently through the source code.
While at it, group the C and C++ include statements as defined in the
coding style, and fix a handful of #include ordering issues.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
|
Signaled is an American spelling.
s/signaled/signalled/
Fixes: b2c06cf40975 ("libcamera: Handle request completion explicitly in pipeline handlers")
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The QtEventDispatcher timers implementation depends on Qt 5.9 or newer
due to the use of QObject::startTimer(std::chrono::milliseconds).
Support earlier Qt versions by using the QObject::startTimer(int)
version instead.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
|
