Source files in libcamera start by a comment block header, which
includes the file name and a one-line description of the file contents.
While the latter is useful to get a quick overview of the file contents
at a glance, the former is mostly a source of inconvenience. The name in
the comments can easily get out of sync with the file name when files
are renamed, and copy & paste during development have often lead to
incorrect names being used to start with.
Readers of the source code are expected to know which file they're
looking it. Drop the file name from the header comment block.
The change was generated with the following script:
----------------------------------------
dirs="include/libcamera src test utils"
declare -rA patterns=(
['c']=' \* '
['cpp']=' \* '
['h']=' \* '
['py']='# '
['sh']='# '
)
for ext in ${!patterns[@]} ; do
files=$(for dir in $dirs ; do find $dir -name "*.${ext}" ; done)
pattern=${patterns[${ext}]}
for file in $files ; do
name=$(basename ${file})
sed -i "s/^\(${pattern}\)${name} - /\1/" "$file"
done
done
----------------------------------------
This misses several files that are out of sync with the comment block
header. Those will be addressed separately and manually.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Several functions in libcamera classes are marked as thread-bound,
restricting the contexts in which those functions can be called. There
is no infrastructure to enforce these restrictions, causing difficult to
debug race conditions when they are not met by callers.
As a first step to solve this, add an assertThreadBound() protected
function to the Object class to test if the calling thread context is
valid, and use it in member functions of Object subclasses marked as
thread-bound. This replaces manual tests in a few locations.
The thread-bound member functions of classes that do not inherit from
Object are not checked, and neither are the functions of classes marked
as thread-bound at the class level. These issue should be addressed in
the future.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Object instances are meant to be deleted from the thread they are bound
to, and this requirement is documented. There are however exceptions to
the rule, as threads may be stopped and restarted, with objects bound to
them not being deleted and recreated for every stop/restart cycle. Bound
objects may therefore need to be deleted after the thread has stopped,
making it impossible to use Object::deleteLater().
Document the lifetime requirements more precisely, and enforce them with
an assertion.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Objects can be scheduled for deletion with Object::deleteLater(), which
queues a deferred deletion to the thread's event loop. As the
deleteLater() function is meant to be called from a different thread,
this may race with thread termination, and deferred deletions queued
just before calling Thread::exit() may not be processed by the event
loop. Make sure they get processed when finishing the thread, before
stopping.
This eliminates the race condition that occurs when calling
Object::deleteLater() followed by Thread::exit() from the same thread.
Calling deleteLater() from neither the thread the object is bound to or
the thread calling Thread::exit() is still inherently racy.
The change fixes a failure in the object-delete unit test.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
The Object::message() function is documented as thread-bound without
using the correct \threadbound reference. Fix it to ensure it gets
included in the thread safety context list.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
gcc 10 and 11 produce an error when compiling libcamera in release mode:
In file included from ../../src/libcamera/base/object.cpp:13:
../../include/libcamera/base/message.h: In member function ‘void libcamera::Object::notifyThreadMove()’:
../../include/libcamera/base/message.h:58:47: error: array subscript ‘const libcamera::InvokeMessage[0]’ is partly outside array bounds of ‘libcamera::Message [1]’ [-Werror=array-bounds]
58 | Semaphore *semaphore() const { return semaphore_; }
| ^~~~~~~~~~
../../src/libcamera/base/object.cpp:280:17: note: while referencing ‘msg’
280 | Message msg(Message::ThreadMoveMessage);
| ^~~
This seems to be a false positive, given that msg->type() can never be
equal to Message::InvokeMessage in Object::message() when called from
Object::notifyThreadMove(), as the message is created there with the
Message::ThreadMoveMessage type. The problem as been reported in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105400, but the error
nonetheless needs to be fixed without waiting for a new gcc release, and
a dynamic_cast does the job with a small additional runtime cost that
shouldn't be a big issue, given that moving objects between threads is a
rare operation.
Bug: https://bugs.libcamera.org/show_bug.cgi?id=125
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Objects are not expected to be connected to the same signal more than
once. Doing so likely indicates a bug in the code, and can be
highlighted in debug builds with an assert that performs a lookup on the
signals_ list.
While it is possible to allow the implementation to let objects connect
to a specific signal multiple times, there are no expected use cases for
this in libcamera and this behaviour is restricted to favour defensive
programming by raising an error when this occurs.
Remove the support in the test framework which uses multiple Signal
connections on the same object, and update the test to use a second
Signal.
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Usage of 'method' to refer to member functions comes from Java. The C++
standard uses the term 'function' only. Replace 'method' with 'function'
or 'member function' through the whole code base and documentation.
While at it, fix two typos (s/backeng/backend/).
The BoundMethod and Object::invokeMethod() are left as-is here, and will
be addressed separately.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Move the functionality for the following components to the new
base support library:
- BoundMethod
- EventDispatcher
- EventDispatcherPoll
- Log
- Message
- Object
- Signal
- Semaphore
- Thread
- Timer
While it would be preferable to see these split to move one component
per commit, these components are all interdependent upon each other,
which leaves us with one big change performing the move for all of them.
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
