| Message ID | 20250407085639.16180-7-mzamazal@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 04. 07. 10:56 keltezéssel, Milan Zamazal írta: > Currently, raw streams don't work in the simple pipeline and the > requested stream roles are ignored. In order to support raw streams, we > now track in SimpleCameraData whether raw and/or processed streams are > requested. We also check that at most one raw stream is requested, > there is no reason to have more. > > That information will be used in the followup patches. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 82d7a9a5..bf9d75f4 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -345,6 +345,8 @@ public: > }; > std::queue<RequestOutputs> conversionQueue_; > bool useConversion_; > + bool processedRequested_; > + bool rawRequested_; > > std::unique_ptr<Converter> converter_; > std::unique_ptr<SoftwareIsp> swIsp_; > @@ -1305,6 +1307,20 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo > if (roles.empty()) > return config; > > + data->processedRequested_ = false; > + data->rawRequested_ = false; I am not sure if `generateConfiguration()` should modify the state of the camera. It does not seem to be compatible with e.g an application caching multiple desired configurations. But crucially, the `generateConfiguration()` method, in contrast to most other similar methods, runs in the caller's thread, and does not run in the camera manager's private thread. (I am not sure if this is just an oversight or intentional.) So these two flags should probably be stored in `SimpleCameraConfiguration`, and copied into `SimpleCameraData` after a successful `configure()`. Similarly to `useConversion_`, as far as I can tell. Regards, Barnabás Pőcze > + for (auto &role : roles) > + if (role == StreamRole::Raw) { > + if (data->rawRequested_) { > + LOG(SimplePipeline, Error) > + << "Can't capture multiple raw streams"; > + return nullptr; > + } > + data->rawRequested_ = true; > + } else { > + data->processedRequested_ = true; > + } > + > /* Create the formats map. */ > std::map<PixelFormat, std::vector<SizeRange>> formats; >
Hi Barnabás, thank you for review. Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes: > Hi > > 2025. 04. 07. 10:56 keltezéssel, Milan Zamazal írta: >> Currently, raw streams don't work in the simple pipeline and the >> requested stream roles are ignored. In order to support raw streams, we >> now track in SimpleCameraData whether raw and/or processed streams are >> requested. We also check that at most one raw stream is requested, >> there is no reason to have more. >> That information will be used in the followup patches. >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >> --- >> src/libcamera/pipeline/simple/simple.cpp | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp >> index 82d7a9a5..bf9d75f4 100644 >> --- a/src/libcamera/pipeline/simple/simple.cpp >> +++ b/src/libcamera/pipeline/simple/simple.cpp >> @@ -345,6 +345,8 @@ public: >> }; >> std::queue<RequestOutputs> conversionQueue_; >> bool useConversion_; >> + bool processedRequested_; >> + bool rawRequested_; >> std::unique_ptr<Converter> converter_; >> std::unique_ptr<SoftwareIsp> swIsp_; >> @@ -1305,6 +1307,20 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo >> if (roles.empty()) >> return config; >> + data->processedRequested_ = false; >> + data->rawRequested_ = false; > > I am not sure if `generateConfiguration()` should modify the state of the > camera. It does not seem to be compatible with e.g an application caching > multiple desired configurations. But crucially, the `generateConfiguration()` > method, in contrast to most other similar methods, runs in the caller's thread, > and does not run in the camera manager's private thread. (I am not sure if this > is just an oversight or intentional.) > > So these two flags should probably be stored in `SimpleCameraConfiguration`, > and copied into `SimpleCameraData` after a successful `configure()`. Similarly > to `useConversion_`, as far as I can tell. Good point, I'll try to rework it. As for the thread, I don't know. > Regards, > Barnabás Pőcze > > >> + for (auto &role : roles) >> + if (role == StreamRole::Raw) { >> + if (data->rawRequested_) { >> + LOG(SimplePipeline, Error) >> + << "Can't capture multiple raw streams"; >> + return nullptr; >> + } >> + data->rawRequested_ = true; >> + } else { >> + data->processedRequested_ = true; >> + } >> + >> /* Create the formats map. */ >> std::map<PixelFormat, std::vector<SizeRange>> formats; >>
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 82d7a9a5..bf9d75f4 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -345,6 +345,8 @@ public: }; std::queue<RequestOutputs> conversionQueue_; bool useConversion_; + bool processedRequested_; + bool rawRequested_; std::unique_ptr<Converter> converter_; std::unique_ptr<SoftwareIsp> swIsp_; @@ -1305,6 +1307,20 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo if (roles.empty()) return config; + data->processedRequested_ = false; + data->rawRequested_ = false; + for (auto &role : roles) + if (role == StreamRole::Raw) { + if (data->rawRequested_) { + LOG(SimplePipeline, Error) + << "Can't capture multiple raw streams"; + return nullptr; + } + data->rawRequested_ = true; + } else { + data->processedRequested_ = true; + } + /* Create the formats map. */ std::map<PixelFormat, std::vector<SizeRange>> formats;
Currently, raw streams don't work in the simple pipeline and the requested stream roles are ignored. In order to support raw streams, we now track in SimpleCameraData whether raw and/or processed streams are requested. We also check that at most one raw stream is requested, there is no reason to have more. That information will be used in the followup patches. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/libcamera/pipeline/simple/simple.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)