[libcamera-devel] [PATCH] utils: ipc, raspberrypi: Make first output parameter direct return if int32
Paul Elder
paul.elder at ideasonboard.com
Fri Mar 5 10:31:46 CET 2021
To make it more convenient for synchronous IPA calls to return a status,
convert the first output into a direct return if it is an int32.
Convert the raspberrypi IPA interface and usage appropriately.
Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
---
This is still an RFC until we decide what we want to do.
This patch depends on:
- utils: ipc: Support custom parameters to init()
- DEMO: raspberrypi: Use custom parameters to init()
I'm expecting to squash with the first patch. If we drop the second
patch then we can remove the raspberrypi components from this patch.
I still need to update the IPA guide appropriately, but I decided to get
approval on this RFC first.
Basically the only question (besides standard review) I want to ask is,
should we squash this with "utils: ipc: Support custom parameters to
init()"?
Naush, does this (and "utils: ipc: Support custom parameters to init()")
fit what you want?
---
include/libcamera/ipa/raspberrypi.mojom | 2 +-
src/ipa/raspberrypi/raspberrypi.cpp | 44 +++++++++---------
.../pipeline/raspberrypi/raspberrypi.cpp | 7 ++-
.../module_ipa_proxy.cpp.tmpl | 7 ++-
.../module_ipa_proxy_worker.cpp.tmpl | 8 ++--
.../libcamera_templates/proxy_functions.tmpl | 4 +-
.../generators/mojom_libcamera_generator.py | 45 ++++++++++++-------
7 files changed, 64 insertions(+), 53 deletions(-)
diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
index b8944227..99a62c02 100644
--- a/include/libcamera/ipa/raspberrypi.mojom
+++ b/include/libcamera/ipa/raspberrypi.mojom
@@ -78,7 +78,7 @@ interface IPARPiInterface {
map<uint32, IPAStream> streamConfig,
map<uint32, ControlInfoMap> entityControls,
ConfigInput ipaConfig)
- => (ConfigOutput results, int32 ret);
+ => (int32 ret, ConfigOutput results);
/**
* \fn mapBuffers()
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 6a9aba6f..ac18dcbd 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -79,17 +79,17 @@ public:
munmap(lsTable_, ipa::RPi::MaxLsGridSize);
}
- void init(const IPASettings &settings, const std::string &sensorName,
- int *ret, bool *metadataSupport) override;
+ int init(const IPASettings &settings, const std::string &sensorName,
+ bool *metadataSupport) override;
void start(const ipa::RPi::StartControls &data,
ipa::RPi::StartControls *result) override;
void stop() override {}
- void configure(const CameraSensorInfo &sensorInfo,
- const std::map<unsigned int, IPAStream> &streamConfig,
- const std::map<unsigned int, ControlInfoMap> &entityControls,
- const ipa::RPi::ConfigInput &data,
- ipa::RPi::ConfigOutput *response, int32_t *ret) override;
+ int configure(const CameraSensorInfo &sensorInfo,
+ const std::map<unsigned int, IPAStream> &streamConfig,
+ const std::map<unsigned int, ControlInfoMap> &entityControls,
+ const ipa::RPi::ConfigInput &data,
+ ipa::RPi::ConfigOutput *response) override;
void mapBuffers(const std::vector<IPABuffer> &buffers) override;
void unmapBuffers(const std::vector<unsigned int> &ids) override;
void signalStatReady(const uint32_t bufferId) override;
@@ -165,15 +165,15 @@ private:
double maxFrameDuration_;
};
-void IPARPi::init(const IPASettings &settings, const std::string &sensorName,
- int *ret, bool *metadataSupport)
+int IPARPi::init(const IPASettings &settings, const std::string &sensorName,
+ bool *metadataSupport)
{
LOG(IPARPI, Debug) << "sensor name is " << sensorName;
tuningFile_ = settings.configurationFile;
*metadataSupport = true;
- *ret = 0;
+ return 0;
}
void IPARPi::start(const ipa::RPi::StartControls &data,
@@ -296,16 +296,15 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
mode_.max_frame_length = sensorInfo.maxFrameLength;
}
-void IPARPi::configure(const CameraSensorInfo &sensorInfo,
- [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
- const std::map<unsigned int, ControlInfoMap> &entityControls,
- const ipa::RPi::ConfigInput &ipaConfig,
- ipa::RPi::ConfigOutput *result, int32_t *ret)
+int IPARPi::configure(const CameraSensorInfo &sensorInfo,
+ [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
+ const std::map<unsigned int, ControlInfoMap> &entityControls,
+ const ipa::RPi::ConfigInput &ipaConfig,
+ ipa::RPi::ConfigOutput *result)
{
if (entityControls.size() != 2) {
LOG(IPARPI, Error) << "No ISP or sensor controls found.";
- *ret = -1;
- return;
+ return -1;
}
result->params = 0;
@@ -315,14 +314,12 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
if (!validateSensorControls()) {
LOG(IPARPI, Error) << "Sensor control validation failed.";
- *ret = -1;
- return;
+ return -1;
}
if (!validateIspControls()) {
LOG(IPARPI, Error) << "ISP control validation failed.";
- *ret = -1;
- return;
+ return -1;
}
/* Setup a metadata ControlList to output metadata. */
@@ -340,8 +337,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
if (!helper_) {
LOG(IPARPI, Error) << "Could not create camera helper for "
<< cameraName;
- *ret = -1;
- return;
+ return -1;
}
/*
@@ -409,7 +405,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
lastMode_ = mode_;
- *ret = 0;
+ return 0;
}
void IPARPi::mapBuffers(const std::vector<IPABuffer> &buffers)
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index a1c90028..106318ed 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -1194,9 +1194,8 @@ int RPiCameraData::loadIPA()
IPASettings settings(ipa_->configurationFile(sensor_->model() + ".json"));
- int ret;
bool metadataSupport;
- ipa_->init(settings, "sensor name", &ret, &metadataSupport);
+ int ret = ipa_->init(settings, "sensor name", &metadataSupport);
LOG(RPI, Debug) << "metadata support " << (metadataSupport ? "yes" : "no");
return ret;
}
@@ -1254,8 +1253,8 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
/* Ready the IPA - it must know about the sensor resolution. */
ipa::RPi::ConfigOutput result;
- ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,
- &result, &ret);
+ ret = ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,
+ &result);
if (ret < 0) {
LOG(RPI, Error) << "IPA configuration failed!";
diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
index ff667ce0..167aaabd 100644
--- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
+++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
@@ -212,7 +212,12 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
{%- endif %}
}
{% if method|method_return_value != "void" %}
- return IPADataSerializer<{{method.response_parameters|first|name}}>::deserialize(_ipcOutputBuf.data(), 0);
+ {{method|method_return_value}} _finalRet = IPADataSerializer<{{method|method_return_value}}>::deserialize(_ipcOutputBuf.data(), 0);
+
+{{proxy_funcs.deserialize_call(method|method_param_outputs, '_ipcOutputBuf.data()', '_ipcOutputBuf.fds()', init_offset = method|method_return_value|byte_width|int)}}
+
+ return _finalRet;
+
{% elif method|method_param_outputs|length > 0 %}
{{proxy_funcs.deserialize_call(method|method_param_outputs, '_ipcOutputBuf.data()', '_ipcOutputBuf.fds()')}}
{% endif -%}
diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
index d08af76d..c4206162 100644
--- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
+++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
@@ -85,15 +85,14 @@ public:
{{param|name}} {{param.mojom_name}};
{% endfor %}
{%- if method|method_return_value != "void" %}
- {{method|method_return_value}} _callRet = ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});
-{%- else %}
+ {{method|method_return_value}} _callRet =
+{%- endif -%}
ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}}
{{- ", " if method|method_param_outputs|params_comma_sep -}}
{%- for param in method|method_param_outputs -%}
&{{param.mojom_name}}{{", " if not loop.last}}
{%- endfor -%}
);
-{%- endif %}
{% if not method|is_async %}
IPCMessage::Header header = { _ipcMessage.header().cmd, _ipcMessage.header().cookie };
IPCMessage _response(header);
@@ -102,9 +101,8 @@ public:
std::tie(_callRetBuf, std::ignore) =
IPADataSerializer<{{method|method_return_value}}>::serialize(_callRet);
_response.data().insert(_response.data().end(), _callRetBuf.cbegin(), _callRetBuf.cend());
-{%- else %}
- {{proxy_funcs.serialize_call(method|method_param_outputs, "_response.data()", "_response.fds()")|indent(16, true)}}
{%- endif %}
+ {{proxy_funcs.serialize_call(method|method_param_outputs, "_response.data()", "_response.fds()")|indent(16, true)}}
int _ret = socket_.send(_response.payload());
if (_ret < 0) {
LOG({{proxy_worker_name}}, Error)
diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
index f2d86b67..c2ac42fc 100644
--- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
+++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
@@ -135,8 +135,8 @@
#
# \todo Avoid intermediate vectors
#}
-{%- macro deserialize_call(params, buf, fds, pointer = true, declare = false, iter = false, data_size = '') -%}
-{% set ns = namespace(size_offset = 0) %}
+{%- macro deserialize_call(params, buf, fds, pointer = true, declare = false, iter = false, data_size = '', init_offset = 0) -%}
+{% set ns = namespace(size_offset = init_offset) %}
{%- if params|length > 1 %}
{%- for param in params %}
[[maybe_unused]] const size_t {{param.mojom_name}}BufSize = readPOD<uint32_t>({{buf}}, {{ns.size_offset}}
diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py
index fa0c21a2..e72a05ff 100644
--- a/utils/ipc/generators/mojom_libcamera_generator.py
+++ b/utils/ipc/generators/mojom_libcamera_generator.py
@@ -149,10 +149,16 @@ def MethodParamInputs(method):
return method.parameters
def MethodParamOutputs(method):
- if (MethodReturnValue(method) != 'void' or
- method.response_parameters is None):
+ if method.response_parameters is None:
+ return []
+
+ if MethodReturnValue(method) == 'void':
+ return method.response_parameters
+
+ if len(method.response_parameters) <= 1:
return []
- return method.response_parameters
+
+ return method.response_parameters[1:]
def MethodParamsHaveFd(parameters):
return len([x for x in parameters if HasFd(x)]) > 0
@@ -167,11 +173,8 @@ def MethodParamNames(method):
params = []
for param in method.parameters:
params.append(param.mojom_name)
- if MethodReturnValue(method) == 'void':
- if method.response_parameters is None:
- return params
- for param in method.response_parameters:
- params.append(param.mojom_name)
+ for param in MethodParamOutputs(method):
+ params.append(param.mojom_name)
return params
def MethodParameters(method):
@@ -180,18 +183,17 @@ def MethodParameters(method):
params.append('const %s %s%s' % (GetNameForElement(param),
'&' if not IsPod(param) else '',
param.mojom_name))
- if MethodReturnValue(method) == 'void':
- if method.response_parameters is None:
- return params
- for param in method.response_parameters:
- params.append(f'{GetNameForElement(param)} *{param.mojom_name}')
+ for param in MethodParamOutputs(method):
+ params.append(f'{GetNameForElement(param)} *{param.mojom_name}')
return params
def MethodReturnValue(method):
- if method.response_parameters is None:
+ if method.response_parameters is None or len(method.response_parameters) == 0:
return 'void'
- if len(method.response_parameters) == 1 and IsPod(method.response_parameters[0]):
- return GetNameForElement(method.response_parameters[0])
+ first_output = method.response_parameters[0]
+ if ((len(method.response_parameters) == 1 and IsPod(first_output)) or
+ first_output.kind == mojom.INT32):
+ return GetNameForElement(first_output)
return 'void'
def IsAsync(method):
@@ -237,6 +239,16 @@ def BitWidth(element):
return '32'
return ''
+def ByteWidthFromCppType(t):
+ key = None
+ for mojo_type, cpp_type in _kind_to_cpp_type.items():
+ if t == cpp_type:
+ key = mojo_type
+ if key is None:
+ raise Exception('invalid type')
+ return str(int(int(_bit_widths[key]) / 8))
+
+
# Get the type name for a given element
def GetNameForElement(element):
# structs
@@ -373,6 +385,7 @@ class Generator(generator.Generator):
libcamera_filters = {
'all_types': GetAllTypes,
'bit_width': BitWidth,
+ 'byte_width' : ByteWidthFromCppType,
'cap': Capitalize,
'choose': Choose,
'comma_sep': CommaSep,
--
2.27.0
More information about the libcamera-devel
mailing list