When compiling with optimization, gcc 9 and newer throw an unitialized
variable warning:
../../src/libcamera/pipeline/ipu3/imgu.cpp: In function ‘void libcamera::{anonymous}::calculateBDSHeight(libcamera::ImgUDevice::Pipe*, const libcamera::Size&, const libcamera::Size&, unsigned int, float)’:
../../src/libcamera/pipeline/ipu3/imgu.cpp:172:17: error: ‘bdsHeight’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
172 | unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
Neither clang not gcc versions older than 9 complain. This seems to be
a false positive.
However, there's an obvious error in the code. The second while () loop
in the first part of calculateBDSHeight() modifies the bdsHeight
variable set by the first loop even if the second loop doesn't find a
suitable height. This can result in an incorrect bdsHeight value. Fix
this, which also gets rid of the compiler warning.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
GLib 2.62 has deprecated g_memdup(), which is used inline in GStreamer's
gstbytereader.h header. This results in a compilation warning:
In file included from /usr/include/gstreamer-1.0/gst/base/base.h:35,
from ../../src/gstreamer/gstlibcamerasrc.cpp:39:
/usr/include/gstreamer-1.0/gst/base/gstbytereader.h: In function ‘guint8* gst_byte_reader_dup_data_unchecked(GstByteReader*, guint)’:
/usr/include/gstreamer-1.0/gst/base/gstbytereader.h:365:41: error: ‘void* g_memdup(gconstpointer, guint)’ is deprecated: Use 'g_memdup2' instead [-Werror=deprecated-declarations]
365 | return (guint8 *) g_memdup (data, size);
GLib has a mechanism to silence warnings introduced in newer versions,
to allow compilation without warnings on multiple versions of the
library. This is done by setting the GLIB_VERSION_MIN_REQUIRED macro to
the minimum GLib version required.
As we depend on GStreamer >= 1.14, set the minimum GLib version to 2.40.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
The standard C library close() and lseek() functions require inclusion
of the unistd.h header. Include it explicitly where needed instead of
relying on indirect inclusion.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Tested-by: Paul Elder <paul.elder@ideasonboard.com>
This reverts commit 5b015e96cc.
The ImgU pipe configuration debug is useful to test the correctness
of the parameters computation against the Intel Python script.
However, the number of debug messages which is printed out by the
configuration procedure is so high it floods the logs, up to the point
that starting the Android camera3 HAL, which tests several configurations
at startup, becomes so slow it is barely usable.
Revert the patch that adds the excessive debug statements, which are mostly
useful only when testing the configuration procedure.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Acked-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Move the documentation back to the mojom file from the cpp file. While
at it, move the documentation for IPAInterface::init() and
IPAInterface::stop() to the IPA guide.
While at it, update the todo comment in all of the mojom files
accordingly.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
[umang.jain@ideasonboard.com: Update todos]
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
We need a separate control to report the nominal frame duration, but
it's also useful to report the min/max frame duration values that will
be used. Split the FrameDurations control into FrameDuration and
FrameDurationLimits respectively to support both of these.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Originally CameraHalManager is created in the libcamera start up
and destroyed in the libcamera termination. However,
CameraHalManager destructor can access other static objects that
has been destroyed.
Avoid this issue by destroying CameraHalManager when tear_down() is
called in ChromeOS or leaking it in other platforms.
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>
On gcc versions older than 9, the file system library, used by the
Android camera HAL configuration file parser, is implemented in a
separate static library. Furthermore, on gcc 7, it's provided in the
std::experimental namespace. This breaks compilation of the HAL on gcc
7, and linking on gcc 8.
Fix the compilation issue by conditionally including
<experimental/filesystem> and creating a namespace alias in std, and the
link issue by linking to libstdc++fs on gcc versions older than 9.
The inclusion of <experimental/filesystem> is a bit of a hack, and when
we'll start using the file system library in another compilation unit,
we should then move all this to an internal helper to abstract the
compiler version.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Add utility functions to CameraMetadata to check if it has been resized,
and for outputting the actual entry and data count. This is meant to be
used to output information on resizing, to assist developers in
choosing proper initial sizes to avoid resizing. Also make CameraDevice
use these functions for static and result metadata.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Open the HAL configuration file in the Camera HAL manager and get
the camera properties for each created CameraDevice and initialize it
with them.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
The HAL layer was getting the min and max frame durations from the
camera, then rounding it to fps to report as available fps ranges. The
same min and max frame durations were then being reported as min and max
frame durations. Since the fps are integer values while the frame
durations are in ns, this caused a rounding error making it seem like we
were reporting an available max fps that was higher than what was
allowed by the minimum frame duration.
An example is if the minimum frame duration is reported as 33366700ns.
The HAL layer would then convert it to fps, which is 29.97, but it would
be rounded and reported as 30 fps. When 30 fps is converted to a frame
duration it is 33333333ns, which is less than the minimum frame duration
that we report. Thus the minimum frame duration that we report
contradicts the fps range that we report.
Fix this by recalculating the frame durations based on the rounded fps
values.
This allows the following CTS test to pass:
- android.hardware.camera2.cts.SurfaceViewPreviewTest#testPreviewFpsRange
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
The supportedDevices may contain entries which have the same driver
but different converters. For example, if we add these two entries:
{ "mtk-seninf", "mtk-mdp", 3 },
{ "mtk-seninf", "mtk-mdp3", 3 },
the simple pipeline handler will always take the first one where it
can acquire the driver and skip the rest.
So, make the changes to support this usecase.
Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The original updateControls() has the assumption that ctrls and
v4l2Ctrls lists are in the same order. It is dependent on the
caller implementation though. This changes updateControls()
implementation so that it works without the assumption.
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>
When stopping the camera worker, queuedRequest() calls may have queued
asynchronous function invocation messages to the worker thread, and some
of those messages may not have been processed yet. The messages will
stay in the thread's queue until the camera worker is restarted (when
the camera service will start a new capture session). At that point,
they will be dispatched, which will cause a crash due to the
CaptureRequest passed to processRequest() having been deleted by
CameraDevice::stop() calling descriptors_.clear().
Fix this by forcing dispatching of all function invocation messages when
stopping the camera worker thread. Note that this is inherently racy, as
more queueRequest() calls may arrive from the camera service while we're
stopping. This race condition will be addressed by a subsequent patch
series.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
There can be multiple IPAs per pipeline-handler or platform.
They can live in-tree or externally linked. It is up to the vendor
whether to use in-tree IPA for a platform or provide it externally.
In the case of IPAs being external, building in-tree IPAs might be
futile hence, provide a clear and explicit mechanism to choose whether
to build the in-tree IPA for the platform or not.
By default, all in-tree IPAs are built when a matching Pipeline handler
is also enabled.
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
CameraSensorInfo structure is designed to pass in camera sensor related
information from pipeline-handler to IPA. Since the pipeline-handler
and IPA are connected via mojom IPC IPA interface, the interface
itself provides a more suitable placement of CameraSensorInfo,
instead of camera_sensor.h (which is a libcamera internal header
ultimately, at this point).
As CameraSensorInfo is already defined in core.mojom, it is just
a matter of removing [skipHeader] tag to allow code-generation
of CameraSensorInfo.
Finally, update header paths to include CameraSensorInfo definition
from IPA interfaces instead of "libcamera/internal/camera_sensor.h".
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Moving the core.mojom documentation to its corresponding .cpp file
(core_ipa_interface.cpp). This will allow Doxygen to generate the
documentation for IPABuffer, IPASettings and IPAStream structures.
Since the .mojom files are placed in include/ directory, the .cpp file
will live in $sourcedir/src/libcamera/ipa/ - which can also contain
documentation for other mojom generated IPA interfaces in subsequent
commit.
Also hide the constructors in generated IPA interface from doxygen,
via #ifndef __DOXYGEN__. These constructors provide no major value in
documenting them, instead will spew out doxygen warnings during the
build.
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The asynchronous signal delivery mechanism can't be used by application,
as it requires libcamera to be aware of the application event loop,
which has been dropped a long time ago. Drop the corresponding
documentation paragraph.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
V4L2ControlId and V4L2ControlInfo are just convenience classes to
create ControlId and ControlInfo from v4l2_query_ext_control.
Therefore, there is no need of being a class. It is used only
from V4L2Device. This removes the classes and put the equivalent
functions of creating ControlId and ControlInfo in
v4l2_device.cpp.
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
IPU3CameraData stores requests that have been failed due to a
buffer shortage. The requests should be retried once enough
buffers are available. This sets the retry function as signal to
CIO2Device and IPU3Frame, and invokes it from
CIO2Device::tryReturnBuffer() and IPU3Frame::remove().
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
PipelineHandlerIPU3 returns -ENOBUFS and -ENOMEM on queueing a
request when there are not sufficient buffers for the request.
Since the request will be successful if it is queued later when
enough buffers are available. The requests failed due to a buffer
shortage should be stored and retried later in the FIFO order.
This introduces the queue in IPU3CameraData to do that.
Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The LOG() macro uses the global Logger and LogCategory objects
internally. This can result in crashes or other undefined behaviour when
logging from destructors of global objects, as the Logger and/or the
LogCategory instances may have been destroyed.
This isn't ideal and should eventually be fixed. For the time being,
document the restriction, and add a todo list item to fix the problem.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Update the revision of the ImgU Python configuration tool the
libcamera implementation is based on to commit 243d134
("Fix some bug for some resolutions").
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org
The IF rectangle height is iteratively computed by first subtracting
the scaling factor to the estimated height, then in a successive loop
by adding the same scaling factor until the maximum IF size is not
reached.
As the computed IF height is not cached in any variable, the second
loop over-writes the result of the first one, even if the BDS alignment
condition is not satisfied.
Fix this by caching the result of the two iterations, and use the one
that produced any result, with a preference for the one produced by the
second loop, as implemented in the reference python script.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
As reported in https://bugs.libcamera.org/show_bug.cgi?id=32
allowing resolutions < IF_CROP_MAX leads to a not manageable number
configurations to be tested, slowing down the ImgU pipe configuration
to a point which is not acceptable for production devices.
Filter all resolutions < IF_CROP_MAX to maintain the run-time complexity
acceptable and remove the safety check that was meant to avoid overflows
when computing the IF rectangle sizes.
Bug: https://bugs.libcamera.org/show_bug.cgi?id=32
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
The CameraMetadata::updateEntry() functions cast the data pointer to a
void pointer, which is then used internally to call
update_camera_metadata_entry(). If the caller passes a pointer to an
incorrect data type, the behaviour is undefined, with possible crashes
if the incorrect data type is smaller than expected by the Android
metadata library.
To avoid crashes, make all public updateEntry() functions take typed
pointers, and pass the element size to the internal function. The
element size is then checked against the expected size, and an error
message logged if they don't match. This won't catch incorrect data
types that have the same size as the correct type, but will at least
avoid potential crashes.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Tested-by: Paul Elder <paul.elder@ideasonboard.com>
