Message ID | 20250203104318.135628-3-pobrn@protonmail.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Quoting Barnabás Pőcze (2025-02-03 10:43:31) > Fill the `FrameMetadata` object of the `FrameBuffer`s because it > should not be left uninitialized as users expect to be able to > access it and find reasonable data there. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=245 > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> Aha, also looks good. Does this get caught by lc-compliance now ? Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/pipeline/virtual/virtual.cpp | 18 +++++++++++++++++- > src/libcamera/pipeline/virtual/virtual.h | 1 + > 2 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp > index 94c2bd91e..1a75f35aa 100644 > --- a/src/libcamera/pipeline/virtual/virtual.cpp > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > @@ -286,6 +286,11 @@ int PipelineHandlerVirtual::exportFrameBuffers([[maybe_unused]] Camera *camera, > int PipelineHandlerVirtual::start([[maybe_unused]] Camera *camera, > [[maybe_unused]] const ControlList *controls) > { > + VirtualCameraData *data = cameraData(camera); > + > + for (auto &s : data->streamConfigs_) > + s.seq = 0; > + > return 0; > } > > @@ -297,13 +302,24 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > Request *request) > { > VirtualCameraData *data = cameraData(camera); > + const auto timestamp = currentTimestamp(); > > for (auto const &[stream, buffer] : request->buffers()) { > bool found = false; > /* map buffer and fill test patterns */ > for (auto &streamConfig : data->streamConfigs_) { > if (stream == &streamConfig.stream) { > + FrameMetadata &fmd = buffer->_d()->metadata(); > + > + fmd.status = FrameMetadata::Status::FrameSuccess; > + fmd.sequence = streamConfig.seq++; > + fmd.timestamp = timestamp; > + > + for (const auto [i, p] : utils::enumerate(buffer->planes())) > + fmd.planes()[i].bytesused = p.length; > + > found = true; > + > if (streamConfig.frameGenerator->generateFrame( > stream->configuration().size, buffer)) > buffer->_d()->cancel(); > @@ -315,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > ASSERT(found); > } > > - request->metadata().set(controls::SensorTimestamp, currentTimestamp()); > + request->metadata().set(controls::SensorTimestamp, timestamp); > completeRequest(request); > > return 0; > diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h > index 92ad7d4a9..683cb82b4 100644 > --- a/src/libcamera/pipeline/virtual/virtual.h > +++ b/src/libcamera/pipeline/virtual/virtual.h > @@ -37,6 +37,7 @@ public: > struct StreamConfig { > Stream stream; > std::unique_ptr<FrameGenerator> frameGenerator; > + unsigned int seq = 0; > }; > /* The config file is parsed to the Configuration struct */ > struct Configuration { > -- > 2.48.1 > >
2025. február 3., hétfő 12:04 keltezéssel, Kieran Bingham <kieran.bingham@ideasonboard.com> írta: > Quoting Barnabás Pőcze (2025-02-03 10:43:31) > > Fill the `FrameMetadata` object of the `FrameBuffer`s because it > > should not be left uninitialized as users expect to be able to > > access it and find reasonable data there. > > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=245 > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > Aha, also looks good. Does this get caught by lc-compliance now ? Not yet. > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > --- > > src/libcamera/pipeline/virtual/virtual.cpp | 18 +++++++++++++++++- > > src/libcamera/pipeline/virtual/virtual.h | 1 + > > 2 files changed, 18 insertions(+), 1 deletion(-) > > > > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp > > index 94c2bd91e..1a75f35aa 100644 > > --- a/src/libcamera/pipeline/virtual/virtual.cpp > > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > > @@ -286,6 +286,11 @@ int PipelineHandlerVirtual::exportFrameBuffers([[maybe_unused]] Camera *camera, > > int PipelineHandlerVirtual::start([[maybe_unused]] Camera *camera, > > [[maybe_unused]] const ControlList *controls) > > { > > + VirtualCameraData *data = cameraData(camera); > > + > > + for (auto &s : data->streamConfigs_) > > + s.seq = 0; > > + > > return 0; > > } > > > > @@ -297,13 +302,24 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > > Request *request) > > { > > VirtualCameraData *data = cameraData(camera); > > + const auto timestamp = currentTimestamp(); > > > > for (auto const &[stream, buffer] : request->buffers()) { > > bool found = false; > > /* map buffer and fill test patterns */ > > for (auto &streamConfig : data->streamConfigs_) { > > if (stream == &streamConfig.stream) { > > + FrameMetadata &fmd = buffer->_d()->metadata(); > > + > > + fmd.status = FrameMetadata::Status::FrameSuccess; > > + fmd.sequence = streamConfig.seq++; > > + fmd.timestamp = timestamp; > > + > > + for (const auto [i, p] : utils::enumerate(buffer->planes())) > > + fmd.planes()[i].bytesused = p.length; > > + > > found = true; > > + > > if (streamConfig.frameGenerator->generateFrame( > > stream->configuration().size, buffer)) > > buffer->_d()->cancel(); > > @@ -315,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > > ASSERT(found); > > } > > > > - request->metadata().set(controls::SensorTimestamp, currentTimestamp()); > > + request->metadata().set(controls::SensorTimestamp, timestamp); > > completeRequest(request); > > > > return 0; > > diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h > > index 92ad7d4a9..683cb82b4 100644 > > --- a/src/libcamera/pipeline/virtual/virtual.h > > +++ b/src/libcamera/pipeline/virtual/virtual.h > > @@ -37,6 +37,7 @@ public: > > struct StreamConfig { > > Stream stream; > > std::unique_ptr<FrameGenerator> frameGenerator; > > + unsigned int seq = 0; > > }; > > /* The config file is parsed to the Configuration struct */ > > struct Configuration { > > -- > > 2.48.1 > > > > >
Hi Barnabás On Mon, Feb 03, 2025 at 10:43:31AM +0000, Barnabás Pőcze wrote: > Fill the `FrameMetadata` object of the `FrameBuffer`s because it > should not be left uninitialized as users expect to be able to > access it and find reasonable data there. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=245 > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > --- > src/libcamera/pipeline/virtual/virtual.cpp | 18 +++++++++++++++++- > src/libcamera/pipeline/virtual/virtual.h | 1 + > 2 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp > index 94c2bd91e..1a75f35aa 100644 > --- a/src/libcamera/pipeline/virtual/virtual.cpp > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > @@ -286,6 +286,11 @@ int PipelineHandlerVirtual::exportFrameBuffers([[maybe_unused]] Camera *camera, > int PipelineHandlerVirtual::start([[maybe_unused]] Camera *camera, > [[maybe_unused]] const ControlList *controls) > { > + VirtualCameraData *data = cameraData(camera); > + > + for (auto &s : data->streamConfigs_) > + s.seq = 0; > + > return 0; > } > > @@ -297,13 +302,24 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > Request *request) > { > VirtualCameraData *data = cameraData(camera); > + const auto timestamp = currentTimestamp(); > > for (auto const &[stream, buffer] : request->buffers()) { > bool found = false; > /* map buffer and fill test patterns */ > for (auto &streamConfig : data->streamConfigs_) { > if (stream == &streamConfig.stream) { > + FrameMetadata &fmd = buffer->_d()->metadata(); > + > + fmd.status = FrameMetadata::Status::FrameSuccess; > + fmd.sequence = streamConfig.seq++; > + fmd.timestamp = timestamp; > + > + for (const auto [i, p] : utils::enumerate(buffer->planes())) > + fmd.planes()[i].bytesused = p.length; > + > found = true; > + > if (streamConfig.frameGenerator->generateFrame( > stream->configuration().size, buffer)) > buffer->_d()->cancel(); > @@ -315,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > ASSERT(found); > } > > - request->metadata().set(controls::SensorTimestamp, currentTimestamp()); > + request->metadata().set(controls::SensorTimestamp, timestamp); > completeRequest(request); > > return 0; > diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h > index 92ad7d4a9..683cb82b4 100644 > --- a/src/libcamera/pipeline/virtual/virtual.h > +++ b/src/libcamera/pipeline/virtual/virtual.h > @@ -37,6 +37,7 @@ public: > struct StreamConfig { > Stream stream; > std::unique_ptr<FrameGenerator> frameGenerator; > + unsigned int seq = 0; > }; > /* The config file is parsed to the Configuration struct */ > struct Configuration { > -- > 2.48.1 > >
diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp index 94c2bd91e..1a75f35aa 100644 --- a/src/libcamera/pipeline/virtual/virtual.cpp +++ b/src/libcamera/pipeline/virtual/virtual.cpp @@ -286,6 +286,11 @@ int PipelineHandlerVirtual::exportFrameBuffers([[maybe_unused]] Camera *camera, int PipelineHandlerVirtual::start([[maybe_unused]] Camera *camera, [[maybe_unused]] const ControlList *controls) { + VirtualCameraData *data = cameraData(camera); + + for (auto &s : data->streamConfigs_) + s.seq = 0; + return 0; } @@ -297,13 +302,24 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, Request *request) { VirtualCameraData *data = cameraData(camera); + const auto timestamp = currentTimestamp(); for (auto const &[stream, buffer] : request->buffers()) { bool found = false; /* map buffer and fill test patterns */ for (auto &streamConfig : data->streamConfigs_) { if (stream == &streamConfig.stream) { + FrameMetadata &fmd = buffer->_d()->metadata(); + + fmd.status = FrameMetadata::Status::FrameSuccess; + fmd.sequence = streamConfig.seq++; + fmd.timestamp = timestamp; + + for (const auto [i, p] : utils::enumerate(buffer->planes())) + fmd.planes()[i].bytesused = p.length; + found = true; + if (streamConfig.frameGenerator->generateFrame( stream->configuration().size, buffer)) buffer->_d()->cancel(); @@ -315,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, ASSERT(found); } - request->metadata().set(controls::SensorTimestamp, currentTimestamp()); + request->metadata().set(controls::SensorTimestamp, timestamp); completeRequest(request); return 0; diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h index 92ad7d4a9..683cb82b4 100644 --- a/src/libcamera/pipeline/virtual/virtual.h +++ b/src/libcamera/pipeline/virtual/virtual.h @@ -37,6 +37,7 @@ public: struct StreamConfig { Stream stream; std::unique_ptr<FrameGenerator> frameGenerator; + unsigned int seq = 0; }; /* The config file is parsed to the Configuration struct */ struct Configuration {
Fill the `FrameMetadata` object of the `FrameBuffer`s because it should not be left uninitialized as users expect to be able to access it and find reasonable data there. Bug: https://bugs.libcamera.org/show_bug.cgi?id=245 Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> --- src/libcamera/pipeline/virtual/virtual.cpp | 18 +++++++++++++++++- src/libcamera/pipeline/virtual/virtual.h | 1 + 2 files changed, 18 insertions(+), 1 deletion(-)