| Message ID | 20260618123844.656396-18-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On Thu, Jun 18, 2026 at 02:38:34PM +0200, Barnabás Pőcze wrote: > `std::queue` already uses `std::deque` as its implementation, so there is > no difference there. In behaviour, indeed, but in semantics, it makes it clear that the queue is, well, a queue. Unless this change is needed to support further work, I'd drop it. > And getting a specific index of `std::list` is a linear > operation, so replace it with an `std::deque`, to enable pipeline handlers > to look into the list more easily. Will they need that ? If so, please briefly explain the reason in the commit message. > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > include/libcamera/internal/camera.h | 7 +++---- > src/libcamera/pipeline_handler.cpp | 8 ++++---- > 2 files changed, 7 insertions(+), 8 deletions(-) > > diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h > index 8a2e9ed589..be3e5ff97e 100644 > --- a/include/libcamera/internal/camera.h > +++ b/include/libcamera/internal/camera.h > @@ -8,9 +8,8 @@ > #pragma once > > #include <atomic> > -#include <list> > +#include <deque> > #include <memory> > -#include <queue> > #include <set> > #include <stdint.h> > #include <string> > @@ -36,8 +35,8 @@ public: > PipelineHandler *pipe() { return pipe_.get(); } > const PipelineHandler *pipe() const { return pipe_.get(); } > > - std::list<Request *> queuedRequests_; > - std::queue<Request *> waitingRequests_; > + std::deque<Request *> queuedRequests_; > + std::deque<Request *> waitingRequests_; > ControlInfoMap controlInfo_; > ControlList properties_; > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 99f35d1e42..5376fee299 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -372,7 +372,7 @@ void PipelineHandler::stop(Camera *camera) > * after the device to keep them in order. > */ > Camera::Private *data = camera->_d(); > - std::queue<Request *> waitingRequests; > + std::deque<Request *> waitingRequests; > waitingRequests.swap(data->waitingRequests_); > > /* Stop the pipeline handler and let the queued requests complete. */ > @@ -381,7 +381,7 @@ void PipelineHandler::stop(Camera *camera) > /* Cancel and signal as complete all waiting requests. */ > while (!waitingRequests.empty()) { > Request *request = waitingRequests.front(); > - waitingRequests.pop(); > + waitingRequests.pop_front(); > > /* > * Cancel all requests by marking them as cancelled and calling > @@ -472,7 +472,7 @@ void PipelineHandler::queueRequest(Request *request) > > Camera *camera = request->_d()->camera(); > Camera::Private *data = camera->_d(); > - data->waitingRequests_.push(request); > + data->waitingRequests_.push_back(request); > > request->_d()->prepare(300ms); > } > @@ -521,7 +521,7 @@ void PipelineHandler::doQueueRequests(Camera *camera) > * Pop the request first, in case doQueueRequests() is called > * recursively from within doQueueRequest() > */ > - data->waitingRequests_.pop(); > + data->waitingRequests_.pop_front(); > doQueueRequest(request); > } > }
2026. 06. 23. 23:49 keltezéssel, Laurent Pinchart írta: > On Thu, Jun 18, 2026 at 02:38:34PM +0200, Barnabás Pőcze wrote: >> `std::queue` already uses `std::deque` as its implementation, so there is >> no difference there. > > In behaviour, indeed, but in semantics, it makes it clear that the queue > is, well, a queue. Unless this change is needed to support further work, > I'd drop it. > >> And getting a specific index of `std::list` is a linear >> operation, so replace it with an `std::deque`, to enable pipeline handlers >> to look into the list more easily. > > Will they need that ? If so, please briefly explain the reason in the > commit message. It's not part of this change set, but the way I have converted the uvcvideo pipeline hander, it needs to "index" into the queue to access the least recent request for which no buffer has been queued yet. Essentially it keeps track of many requests have buffers queued into the v4l2 device, and when new buffers appear, it processes the requests that don't have any buffers yet. This change is here to facilitate that. But now that I think about it, it might need some refinement. Maybe it's not the best approach, I don't know. > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> include/libcamera/internal/camera.h | 7 +++---- >> src/libcamera/pipeline_handler.cpp | 8 ++++---- >> 2 files changed, 7 insertions(+), 8 deletions(-) >> >> diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h >> index 8a2e9ed589..be3e5ff97e 100644 >> --- a/include/libcamera/internal/camera.h >> +++ b/include/libcamera/internal/camera.h >> @@ -8,9 +8,8 @@ >> #pragma once >> >> #include <atomic> >> -#include <list> >> +#include <deque> >> #include <memory> >> -#include <queue> >> #include <set> >> #include <stdint.h> >> #include <string> >> @@ -36,8 +35,8 @@ public: >> PipelineHandler *pipe() { return pipe_.get(); } >> const PipelineHandler *pipe() const { return pipe_.get(); } >> >> - std::list<Request *> queuedRequests_; >> - std::queue<Request *> waitingRequests_; >> + std::deque<Request *> queuedRequests_; >> + std::deque<Request *> waitingRequests_; >> ControlInfoMap controlInfo_; >> ControlList properties_; >> >> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp >> index 99f35d1e42..5376fee299 100644 >> --- a/src/libcamera/pipeline_handler.cpp >> +++ b/src/libcamera/pipeline_handler.cpp >> @@ -372,7 +372,7 @@ void PipelineHandler::stop(Camera *camera) >> * after the device to keep them in order. >> */ >> Camera::Private *data = camera->_d(); >> - std::queue<Request *> waitingRequests; >> + std::deque<Request *> waitingRequests; >> waitingRequests.swap(data->waitingRequests_); >> >> /* Stop the pipeline handler and let the queued requests complete. */ >> @@ -381,7 +381,7 @@ void PipelineHandler::stop(Camera *camera) >> /* Cancel and signal as complete all waiting requests. */ >> while (!waitingRequests.empty()) { >> Request *request = waitingRequests.front(); >> - waitingRequests.pop(); >> + waitingRequests.pop_front(); >> >> /* >> * Cancel all requests by marking them as cancelled and calling >> @@ -472,7 +472,7 @@ void PipelineHandler::queueRequest(Request *request) >> >> Camera *camera = request->_d()->camera(); >> Camera::Private *data = camera->_d(); >> - data->waitingRequests_.push(request); >> + data->waitingRequests_.push_back(request); >> >> request->_d()->prepare(300ms); >> } >> @@ -521,7 +521,7 @@ void PipelineHandler::doQueueRequests(Camera *camera) >> * Pop the request first, in case doQueueRequests() is called >> * recursively from within doQueueRequest() >> */ >> - data->waitingRequests_.pop(); >> + data->waitingRequests_.pop_front(); >> doQueueRequest(request); >> } >> } > > -- > Regards, > > Laurent Pinchart
On Wed, Jun 24, 2026 at 10:17:51AM +0200, Barnabás Pőcze wrote: > 2026. 06. 23. 23:49 keltezéssel, Laurent Pinchart írta: > > On Thu, Jun 18, 2026 at 02:38:34PM +0200, Barnabás Pőcze wrote: > >> `std::queue` already uses `std::deque` as its implementation, so there is > >> no difference there. > > > > In behaviour, indeed, but in semantics, it makes it clear that the queue > > is, well, a queue. Unless this change is needed to support further work, > > I'd drop it. > > > >> And getting a specific index of `std::list` is a linear > >> operation, so replace it with an `std::deque`, to enable pipeline handlers > >> to look into the list more easily. > > > > Will they need that ? If so, please briefly explain the reason in the > > commit message. > > It's not part of this change set, but the way I have converted the uvcvideo > pipeline hander, it needs to "index" into the queue to access the least recent > request for which no buffer has been queued yet. Essentially it keeps track of > many requests have buffers queued into the v4l2 device, and when new buffers > appear, it processes the requests that don't have any buffers yet. > > This change is here to facilitate that. But now that I think about it, it might need > some refinement. Maybe it's not the best approach, I don't know. Then I'd rather exclude this from the preparation series and review it with the uvcvideo changes if that's fine with you. > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >> --- > >> include/libcamera/internal/camera.h | 7 +++---- > >> src/libcamera/pipeline_handler.cpp | 8 ++++---- > >> 2 files changed, 7 insertions(+), 8 deletions(-) > >> > >> diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h > >> index 8a2e9ed589..be3e5ff97e 100644 > >> --- a/include/libcamera/internal/camera.h > >> +++ b/include/libcamera/internal/camera.h > >> @@ -8,9 +8,8 @@ > >> #pragma once > >> > >> #include <atomic> > >> -#include <list> > >> +#include <deque> > >> #include <memory> > >> -#include <queue> > >> #include <set> > >> #include <stdint.h> > >> #include <string> > >> @@ -36,8 +35,8 @@ public: > >> PipelineHandler *pipe() { return pipe_.get(); } > >> const PipelineHandler *pipe() const { return pipe_.get(); } > >> > >> - std::list<Request *> queuedRequests_; > >> - std::queue<Request *> waitingRequests_; > >> + std::deque<Request *> queuedRequests_; > >> + std::deque<Request *> waitingRequests_; > >> ControlInfoMap controlInfo_; > >> ControlList properties_; > >> > >> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > >> index 99f35d1e42..5376fee299 100644 > >> --- a/src/libcamera/pipeline_handler.cpp > >> +++ b/src/libcamera/pipeline_handler.cpp > >> @@ -372,7 +372,7 @@ void PipelineHandler::stop(Camera *camera) > >> * after the device to keep them in order. > >> */ > >> Camera::Private *data = camera->_d(); > >> - std::queue<Request *> waitingRequests; > >> + std::deque<Request *> waitingRequests; > >> waitingRequests.swap(data->waitingRequests_); > >> > >> /* Stop the pipeline handler and let the queued requests complete. */ > >> @@ -381,7 +381,7 @@ void PipelineHandler::stop(Camera *camera) > >> /* Cancel and signal as complete all waiting requests. */ > >> while (!waitingRequests.empty()) { > >> Request *request = waitingRequests.front(); > >> - waitingRequests.pop(); > >> + waitingRequests.pop_front(); > >> > >> /* > >> * Cancel all requests by marking them as cancelled and calling > >> @@ -472,7 +472,7 @@ void PipelineHandler::queueRequest(Request *request) > >> > >> Camera *camera = request->_d()->camera(); > >> Camera::Private *data = camera->_d(); > >> - data->waitingRequests_.push(request); > >> + data->waitingRequests_.push_back(request); > >> > >> request->_d()->prepare(300ms); > >> } > >> @@ -521,7 +521,7 @@ void PipelineHandler::doQueueRequests(Camera *camera) > >> * Pop the request first, in case doQueueRequests() is called > >> * recursively from within doQueueRequest() > >> */ > >> - data->waitingRequests_.pop(); > >> + data->waitingRequests_.pop_front(); > >> doQueueRequest(request); > >> } > >> }
diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h index 8a2e9ed589..be3e5ff97e 100644 --- a/include/libcamera/internal/camera.h +++ b/include/libcamera/internal/camera.h @@ -8,9 +8,8 @@ #pragma once #include <atomic> -#include <list> +#include <deque> #include <memory> -#include <queue> #include <set> #include <stdint.h> #include <string> @@ -36,8 +35,8 @@ public: PipelineHandler *pipe() { return pipe_.get(); } const PipelineHandler *pipe() const { return pipe_.get(); } - std::list<Request *> queuedRequests_; - std::queue<Request *> waitingRequests_; + std::deque<Request *> queuedRequests_; + std::deque<Request *> waitingRequests_; ControlInfoMap controlInfo_; ControlList properties_; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 99f35d1e42..5376fee299 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -372,7 +372,7 @@ void PipelineHandler::stop(Camera *camera) * after the device to keep them in order. */ Camera::Private *data = camera->_d(); - std::queue<Request *> waitingRequests; + std::deque<Request *> waitingRequests; waitingRequests.swap(data->waitingRequests_); /* Stop the pipeline handler and let the queued requests complete. */ @@ -381,7 +381,7 @@ void PipelineHandler::stop(Camera *camera) /* Cancel and signal as complete all waiting requests. */ while (!waitingRequests.empty()) { Request *request = waitingRequests.front(); - waitingRequests.pop(); + waitingRequests.pop_front(); /* * Cancel all requests by marking them as cancelled and calling @@ -472,7 +472,7 @@ void PipelineHandler::queueRequest(Request *request) Camera *camera = request->_d()->camera(); Camera::Private *data = camera->_d(); - data->waitingRequests_.push(request); + data->waitingRequests_.push_back(request); request->_d()->prepare(300ms); } @@ -521,7 +521,7 @@ void PipelineHandler::doQueueRequests(Camera *camera) * Pop the request first, in case doQueueRequests() is called * recursively from within doQueueRequest() */ - data->waitingRequests_.pop(); + data->waitingRequests_.pop_front(); doQueueRequest(request); } }
`std::queue` already uses `std::deque` as its implementation, so there is no difference there. And getting a specific index of `std::list` is a linear operation, so replace it with an `std::deque`, to enable pipeline handlers to look into the list more easily. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- include/libcamera/internal/camera.h | 7 +++---- src/libcamera/pipeline_handler.cpp | 8 ++++---- 2 files changed, 7 insertions(+), 8 deletions(-)