| Age | Commit message (Collapse) | Author |
|---|
|
buffer_handle_t doesn't provide sufficient info to map a buffer
properly. cros::CameraBufferManager enables handling the buffer on
ChromeOS, but no way is provided for other platforms.
Therefore, we put the assumption that planes are in the same buffer
and they are consecutive. This modifies the way of mapping in
generic_camera_buffer with the assumption.
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Pipeline managers sets a default value to StreamConfiguration::size. The
original fixation code was attempting to use it, but as it was truncating
the caps to its first structure it would never actually find a best match.
In this patch, instead of truncating, we weight various matches using the
product of the width and height delta. We also split delta from ranges
apart and prefer fixed size over them as ranges are not reliable.
This patch also removes the related todo, as it seems that libcamera core
won't go further then providing this default value and won't be sorting the
format and size lists.
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
It's not allowed in GStreamer to push events while holding the object
lock. This reduce the scope into which we hold the object lock. In
fact we don't need to protect against gst_task_resume() concurrency
when we stop the task as resume only do something if the task is paused.
This fixes a deadlock when running multiple instances of libcamerasrc
and closing one of the streaming window.
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
It's not allowed to have multiple instances of CameraManager. This
requirement is not easy for GStreamer were the device monitor and
the camerasrc, or two camerasrc instances don't usually have any
interaction between each other. Fix this by implementing a minimalist
singleton around CameraManager constructor and start()/stop()
operations.
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
This deadlock occurs when a buffer is holding the last reference on
the allocator. In gst_libcamera_allocator_release() we must drop the
object lock before dropping the last ref of that object since the
destructor will lock it again causing deadlock.
This was notice while switching camera or resolution in Cheese software.
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@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>
|
|
Fix a minor spelling error.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The BufferWriter class has been replaced by the FileSink.
Adapt the reference, to match the source code.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The CameraManager object should be deleted when it is no longer used to
prevent it from leaking.
When the application closes, the memory will be released, but it would
show up in reports from memory validation tools such as valgrind if not
handled correctly.
Recommend best-practices in the guide and ensure it is automatically
cleaned up when the CameraManager goes out of scope.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The build and run instructions read as if meson is the only way to
compile the application. Although this is recommended, it is not
required - adapt the language to fit better.
While here, fix the naming of the simple_cam meson object to match
the other usages of 'simple-cam'.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
This TODO has no real meaning, and isn't an action. Remove it.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The Gstreamer gstlibcamerasrc element operates better with the
glimagesink, as it can perform direct renders or hardware accellerated
image format conversion.
Further extend the sample commands to demonstrate how to specify a fixed
width and height parameter to the pipeline.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The compiler generated constructor does not initialize the
ControlInfoMap::idmap_ field.
Fix this by explicitly initializing the field in the class
declaration.
Reported-by: Coverity CID=354657
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 IPASessionConfiguration now has the grid configuration stored. Use
it it at prepare() and process() calls in AWB and pass it as a reference
to the private functions when needed.
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The IPASessionConfiguration now has the grid configuration stored. Use
it at process() call in AGC and pass it as a reference to the private
functions when needed.
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The Doxygen LATEX_SOURCE_CODE, RTF_SOURCE_CODE and
DOCBOOK_PROGRAMLISTING options are deprecated, and generate warnings
with the Doxygen's master branch. Drop them.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The MappedBuffer structure is a custom container that binds a data
pointer with a length. This is exactly what Span is. Use it instead.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
Now that the interface is properly used by the AGC class, move it into
ipa::ipu3::algorithms and let the loops do the calls.
As we need to exchange the exposure_ and gain_ by passing them through the
FrameContext, use the calculated values in setControls() function to
ease the reading.
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Now that the interface is properly used by the AWB class, move it into
ipa::ipu3::algorithms and let the loops do the calls.
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
In preparation for using the AGC through the new algorithm interfaces,
convert the existing code to use the new function types.
Now that the process call is rewritten, re-enable the compiler flag to
warn when a function declaration hides virtual functions from a base class
(-Woverloaded-virtual).
We never use converged_ so remove its declaration. The controls may not
need to be updated at each call, but it should be decided on the context
side and not by a specific call by using a lock status in the Agc
structure for instance.
As the params_ local variable is not useful anymore, remove it here
too.
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
When the stats are received, pass them with the context to the existing
AWB algorithm. IPAFrameContext now has a new structure to store the
gains calculated by the AWB algorithm.
When an EventFillParams event is received, call prepare() and set the new
gains accordingly in the params structure.
There is no more a need for the IPU3Awb::initialise() function, as the
params are always set in prepare().
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Introduce a new algorithm to manage the tone mapping handling of the
IPU3.
The initial algorithm is chosen to configure the gamma contrast curve
which moves the implementation out of AWB for simplicity. As it is
initialised with a default gamma value of 1.1, there is no need to use
the default table at initialisation anymore.
This demonstrates the way to use process() call when the EventStatReady
comes in. The function calculates the LUT in the context of a frame, and
when prepare() is called, the parameters are filled with the updated
values.
AGC is modified to take the new process interface into account.
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Implement a new modular framework for algorithms with a common context
structure that is passed to each algorithm through a common API.
This patch:
- removes all the local references from IPAIPU3 and uses IPAContext
- implements the list of pointers and the loop at configure call on each
algorithm
- loops in fillParams on each prepare() call on the algorithm list
- loops in prepareStats on each process() call on the algorithm list
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Introduce three functions in the Algorithm class to manage algorithms:
- configure which is called when IPA is configured only
- prepare called on EventFillParams event at each frame when the request
is queued
- process called on EventStatReady event at each frame completion when
the statistics have been generated.
The existing AGC implementation already has a function named process(),
though it has different arguments. Adding the new virtual process()
interface causes a compiler warning due to the AGC implementation
overloading a virtual function, even though the overload can be resolved
correctly.
Temporarily disable the warning in this commit to maintain bisection
until the AGC is converted to the new interface.
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
An increasing amount of data and information needs to be shared between
the components that build up to implement image processing algorithms.
Create a context structure which will allow us to work towards calling
algorithms in a modular way, and sharing information between the modules.
The IPA context is a global context set at configure time
(IPASessionConfiguration) and a per-frame context (IPAFrameContext) used
while streaming.
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
The abstract Algorithm class was originally placed in libipa as an
attempt define a generic algorithm container. This was a little
optimistic and pushed a bit far too early.
Move the Algorithm class into the IPU3 which is the only user of the
class, as we adapt it to support modular algorithm components for the
IPU3.
Not documenting the namespace may cause issues with Doxygen in libipa.
The file libipa.cpp is thus created as an empty file for now, but we
can leverage it in the future to add more global libipa documentation,
and possibly code too.
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
IPCMessage::payload() converts the IPCMessage into an IPCUnixSocket
payload. However, if IPCMessage is constructed with one of the
following constructors -
IPCMessage::IPCMessage(),
IPCMessage::IPCMessage(uint32_t cmd)
IPCMessage::IPCMessage(const Header &header)
The data_ vector of IPCMessage is empty and uninitialised. In that
case, IPCMessage::payload will try to memcpy() an empty data_ vector
which can lead to invoking memcpy() with a nullptr parameter, which
is then identified by the address sanity checker.. Add a non-empty
data_ vector check to avoid it.
The issue is noticed by running a test manually, testing the vimc
IPA code paths in isolated mode. It is only noticed when the test
is compiled with -Db_sanitize=address,undefined meson built-in option.
ipc_pipe.cpp:110:8: runtime error: null pointer passed as argument 2, which is declared to never be null
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
In IPCUnixSocket, a payload can be sent/received with empty fd vector,
which leads to passing a nullptr in memcpy() in both sendData()
and recvData(). Add a null check for fd vector's data pointer
to avoid invoking memcpy() with nullptr.
The issue is noticed by running a test manually testing the vimc
IPA code paths in isolated mode. It is only noticed when the test
is compiled with -Db_sanitize=address,undefined meson built-in option.
ipc_unixsocket.cpp:268:8: runtime error: null pointer passed as argument 2, which is declared to never be null
ipc_unixsocket.cpp:312:8: runtime error: null pointer passed as argument 1, which is declared to never be null
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@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>
|
|
The IPCUnixSocket::send() function may fail, in which case it can be
useful for debugging to log an error message that tells which event was
affected. Do so.
Reported-by: Coverity CID=35483[6-9]
Reported-by: Coverity CID=35484[01]
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The bcm2835-isp.h is included with quotes rather than
angle brackets.
Quoted includes is reserved for internal includes, while the
linux/bcm2835-isp.h header is exported from the Linux kernel.
Fix the inclusion type.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
There is no point in recording sensor's timestamp when V4L2VideoDevice
has marked the frame buffers with FrameMetadata::FrameCancelled
(happens when the streams are stopped). The metadata is mostly invalid
for cancelled buffers hence, setting timestamp on invalid metadata
does not make sense (however some metadata can be considered valid
for e.g. returning cause of failure through metadata on cancelled
buffers).
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Replace manual static casts from the PipelineHandler pointer to a
derived class pointer with helper functions in the camera data classes.
This simplifies code accessing the pipeline from the camera data.
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 PipelineHandler controls() and properties() functions are only used
by the Camera class. Now that the controls and properties are stored in
the Camera::Private class, we can drop those functions and access the
private data directly in Camera::controls() and Camera::properties().
Suggested-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
|
|
Update the pipeline handler guide following the migration from the
CameraData class to the Camera::Private class.
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 CameraData class isn't used anymore. Drop it.
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>
|
|
As part of the effort to remove the CameraData class, migrate the
pipeline handler-specific camera data from CameraData to the
Camera::Private class.
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>
|
|
As part of the effort to remove the CameraData class, migrate the
pipeline handler-specific camera data from CameraData to the
Camera::Private class.
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>
|
|
As part of the effort to remove the CameraData class, migrate the
pipeline handler-specific camera data from CameraData to the
Camera::Private class.
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>
|
|
As part of the effort to remove the CameraData class, migrate the
pipeline handler-specific camera data from CameraData to the
Camera::Private class.
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>
|
|
As part of the effort to remove the CameraData class, migrate the
pipeline handler-specific camera data from CameraData to the
Camera::Private class.
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>
|
|
As part of the effort to remove the CameraData class, migrate the
pipeline handler-specific camera data from CameraData to the
Camera::Private class.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
Improve the Camera::Private documentation by fixing minor spelling or
style issues.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
With pipeline handlers now being able to subclass Camera::Private, start
the migration from CameraData to Camera::Private by moving the members
of the base CameraData class. The controlInfo_, properties_ and pipe_
members are duplicated for now, to allow migrating pipeline handlers one
by one.
The Camera::Private class is now properly documented, don't exclude it
from documentation generation.
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>
|
|
In order to allow subclassing Camera::Private in pipeline handlers, pass
the pointer to the private data to the Camera constructor, and to the
Camera::createCamera() function.
The Camera::Private id_ and streams_ members now need to be initialized
by the Camera constructor instead of the Camera::Private constructor, to
allow storage of the streams in a pipeline handler-specific subclass of
Camera::Private.
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 Extensible constructor takes a pointer to a Private instance, whose
lifetime it then manages. Make this explicit in the API by passing the
pointer as a std::unique_ptr<Private>.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
|
|
When the stream is stopped, the V4L2VideoDevice sends back all
the queued buffers with FrameMetadata::FrameCancelled status.
It is the responsibility of the pipeline handler to handle
these buffers with FrameMetadata::FrameCancelled. VIMC is
currently missing this handling path.
As the FrameMetadata::FrameCancelled is set when the stream is
stopped, we can be sure that no more queued and re-use of request
shall happen. Hence, cancel all the requests' buffers force a
complete with completeBuffer().
The issue is caught by the gstreamer_single_stream_test.cpp running
with vimc. During the check with meson built-in option
'-Db_sanitize=address,undefined'
it was observed:
==118003==ERROR: AddressSanitizer: heap-use-after-free on address 0x60e000037108 at pc 0x7f225160c9ac bp 0x7f224a47b620 sp 0x7f224a47b618
READ of size 4 at 0x60e000037108 thread T1
#0 0x7f225160c9ab in libcamera::Request::sequence() const ../include/libcamera/request.h:55
#1 0x7f22518297aa in libcamera::VimcCameraData::bufferReady(libcamera::FrameBuffer*) ../src/libcamera/pipeline/vimc/vimc.cpp:577
#2 0x7f225183b1ef in libcamera::BoundMethodMember<libcamera::VimcCameraData, void, libcamera::FrameBuffer*>::activate(libcamera::FrameBuffer*, bool) ../include/libcamera/base/bound_method.h:194
#3 0x7f22515cc91f in libcamera::Signal<libcamera::FrameBuffer*>::emit(libcamera::FrameBuffer*) ../include/libcamera/base/signal.h:126
#4 0x7f22515c3305 in libcamera::V4L2VideoDevice::streamOff() ../src/libcamera/v4l2_videodevice.cpp:1605
#5 0x7f225181f345 in libcamera::PipelineHandlerVimc::stop(libcamera::Camera*) ../src/libcamera/pipeline/vimc/vimc.cpp:365
The VimcCameraData::bufferReady seems to emit even after the stream
is stopped. It's primarily due to vimc's lack of handling
FrameMetadata::FrameCancelled in its pipeline handler.
Signed-off-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: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
