Message ID | 20200112010212.2609025-33-niklas.soderlund@ragnatech.se |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Hi Niklas, Thank you for the patch. On Sun, Jan 12, 2020 at 02:02:12AM +0100, Niklas Söderlund wrote: > With the FrameBuffer rework completed there is no reason to keep the > camera prepared state around as buffer allocations are now decoupled > from the camera state. Remove the camera state simplifying the API. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/camera.h | 4 - > src/android/camera_device.cpp | 11 +-- > src/cam/capture.cpp | 8 -- > src/libcamera/camera.cpp | 100 +++++------------------- > src/libcamera/framebuffer_allocator.cpp | 5 +- > src/libcamera/pipeline_handler.cpp | 5 ++ > src/qcam/main_window.cpp | 9 --- > src/v4l2/v4l2_camera.cpp | 5 -- > test/camera/buffer_import.cpp | 10 --- > test/camera/capture.cpp | 10 --- > test/camera/statemachine.cpp | 83 -------------------- > 11 files changed, 27 insertions(+), 223 deletions(-) > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index 28655bec9ebc89ce..6597ade83288a170 100644 > --- a/include/libcamera/camera.h > +++ b/include/libcamera/camera.h > @@ -91,9 +91,6 @@ public: > std::unique_ptr<CameraConfiguration> generateConfiguration(const StreamRoles &roles); > int configure(CameraConfiguration *config); > > - int allocateBuffers(); > - int freeBuffers(); > - > Request *createRequest(uint64_t cookie = 0); > int queueRequest(Request *request); > > @@ -105,7 +102,6 @@ private: > CameraAvailable, > CameraAcquired, > CameraConfigured, > - CameraPrepared, > CameraRunning, > }; > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index 49321db07a2c93d5..a98fd744f5347432 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -77,8 +77,6 @@ int CameraDevice::open() > void CameraDevice::close() > { > camera_->stop(); > - > - camera_->freeBuffers(); > camera_->release(); > > running_ = false; > @@ -690,16 +688,9 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque > > /* Start the camera if that's the first request we handle. */ > if (!running_) { > - int ret = camera_->allocateBuffers(); > - if (ret) { > - LOG(HAL, Error) << "Failed to allocate buffers"; > - return; > - } > - > - ret = camera_->start(); > + int ret = camera_->start(); > if (ret) { > LOG(HAL, Error) << "Failed to start camera"; > - camera_->freeBuffers(); > return; > } > > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp > index 738fa1c267eb6e36..7d970f991d3aaf1a 100644 > --- a/src/cam/capture.cpp > +++ b/src/cam/capture.cpp > @@ -42,12 +42,6 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) > return ret; > } > > - ret = camera_->allocateBuffers(); > - if (ret) { > - std::cerr << "Failed to allocate buffers" << std::endl; > - return ret; > - } > - > camera_->requestCompleted.connect(this, &Capture::requestComplete); > > if (options.isSet(OptFile)) { > @@ -67,8 +61,6 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) > writer_ = nullptr; > } > > - camera_->freeBuffers(); > - > delete allocator; > > return ret; > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index f3a7578d0834a9d6..79a5f994f9bbc8c1 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -275,15 +275,13 @@ std::size_t CameraConfiguration::size() const > * \section camera_operation Operating the Camera > * > * An application needs to perform a sequence of operations on a camera before > - * it is ready to process requests. The camera needs to be acquired, configured > - * and resources allocated or imported to prepare the camera for capture. Once > - * started the camera can process requests until it is stopped. When an > - * application is done with a camera all resources allocated need to be freed > - * and the camera released. > + * it is ready to process requests. The camera needs to be acquired and > + * configured to prepare the camera for capture. Once started the camera can > + * process requests until it is stopped. When an application is done with a > + * camera, the camera needs to be released. > * > * An application may start and stop a camera multiple times as long as it is > - * not released. The camera may also be reconfigured provided that all > - * resources allocated are freed prior to the reconfiguration. > + * not released. The camera may also be reconfigured. > * > * \subsection Camera States > * > @@ -297,7 +295,6 @@ std::size_t CameraConfiguration::size() const > * node [shape = doublecircle ]; Available; > * node [shape = circle ]; Acquired; > * node [shape = circle ]; Configured; > - * node [shape = circle ]; Prepared; > * node [shape = circle ]; Running; > * > * Available -> Available [label = "release()"]; > @@ -307,14 +304,10 @@ std::size_t CameraConfiguration::size() const > * Acquired -> Configured [label = "configure()"]; > * > * Configured -> Available [label = "release()"]; > - * Configured -> Configured [label = "configure()"]; > - * Configured -> Prepared [label = "allocateBuffers()"]; > + * Configured -> Configured [label = "configure(), createRequest()"]; > + * Configured -> Running [label = "start()"]; > * > - * Prepared -> Configured [label = "freeBuffers()"]; > - * Prepared -> Prepared [label = "createRequest()"]; > - * Prepared -> Running [label = "start()"]; > - * > - * Running -> Prepared [label = "stop()"]; > + * Running -> Configured [label = "stop()"]; > * Running -> Running [label = "createRequest(), queueRequest()"]; > * } > * \enddot > @@ -330,19 +323,14 @@ std::size_t CameraConfiguration::size() const > * Configured state. > * > * \subsubsection Configured > - * The camera is configured and ready for the application to prepare it with > - * resources. The camera may be reconfigured multiple times until resources > - * are provided and the state progresses to Prepared. > - * > - * \subsubsection Prepared > - * The camera has been configured and provided with resources and is ready to be > - * started. The application may free the camera's resources to get back to the > - * Configured state or start() it to progress to the Running state. > + * The camera is configured and ready to be started. The application may > + * release() the camera and to get back to the Available state or start() > + * it to progress to the Running state. > * > * \subsubsection Running > * The camera is running and ready to process requests queued by the > * application. The camera remains in this state until it is stopped and moved > - * to the Prepared state. > + * to the Configured state. > */ > > /** > @@ -420,7 +408,6 @@ static const char *const camera_state_names[] = { > "Available", > "Acquired", > "Configured", > - "Prepared", > "Running", > }; > > @@ -465,8 +452,6 @@ bool Camera::stateIs(State state) const > * > * \todo Deal with pending requests if the camera is disconnected in a > * running state. > - * \todo Update comment about Running state when importing buffers as well as > - * allocating them are supported. > */ > void Camera::disconnect() > { > @@ -474,11 +459,11 @@ void Camera::disconnect() > > /* > * If the camera was running when the hardware was removed force the > - * state to Prepared to allow applications to call freeBuffers() and > - * release() before deleting the camera. > + * state to Configured state to allow applications to free resources > + * and call release() before deleting the camera. > */ > if (state_ == CameraRunning) > - state_ = CameraPrepared; > + state_ = CameraConfigured; > > disconnected_ = true; > disconnected.emit(this); > @@ -702,53 +687,6 @@ int Camera::configure(CameraConfiguration *config) > return 0; > } > > -/** > - * \brief Allocate buffers for all configured streams > - * > - * This function affects the state of the camera, see \ref camera_operation. > - * > - * \return 0 on success or a negative error code otherwise > - * \retval -ENODEV The camera has been disconnected from the system > - * \retval -EACCES The camera is not in a state where buffers can be allocated > - * \retval -EINVAL The configuration is not valid > - */ > -int Camera::allocateBuffers() > -{ > - if (disconnected_) > - return -ENODEV; > - > - if (!stateIs(CameraConfigured)) > - return -EACCES; > - > - if (activeStreams_.empty()) { > - LOG(Camera, Error) > - << "Can't allocate buffers without streams"; > - return -EINVAL; > - } > - > - state_ = CameraPrepared; > - > - return 0; > -} > - > -/** > - * \brief Release all buffers from allocated pools in each stream > - * > - * This function affects the state of the camera, see \ref camera_operation. > - * > - * \return 0 on success or a negative error code otherwise > - * \retval -EACCES The camera is not in a state where buffers can be freed > - */ > -int Camera::freeBuffers() > -{ > - if (!stateIs(CameraPrepared)) > - return -EACCES; > - > - state_ = CameraConfigured; > - > - return 0; > -} > - > /** > * \brief Create a request object for the camera > * \param[in] cookie Opaque cookie for application use > @@ -764,14 +702,14 @@ int Camera::freeBuffers() > * The ownership of the returned request is passed to the caller, which is > * responsible for either queueing the request or deleting it. > * > - * This function shall only be called when the camera is in the Prepared > + * This function shall only be called when the camera is in the Configured > * or Running state, see \ref camera_operation. > * > * \return A pointer to the newly created request, or nullptr on error > */ > Request *Camera::createRequest(uint64_t cookie) > { > - if (disconnected_ || !stateBetween(CameraPrepared, CameraRunning)) > + if (disconnected_ || !stateBetween(CameraConfigured, CameraRunning)) > return nullptr; > > return new Request(this, cookie); > @@ -842,7 +780,7 @@ int Camera::start() > if (disconnected_) > return -ENODEV; > > - if (!stateIs(CameraPrepared)) > + if (!stateIs(CameraConfigured)) > return -EACCES; > > LOG(Camera, Debug) << "Starting capture"; > @@ -885,7 +823,7 @@ int Camera::stop() > > LOG(Camera, Debug) << "Stopping capture"; > > - state_ = CameraPrepared; > + state_ = CameraConfigured; > > pipe_->stop(this); > > diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp > index 57789b24e9615fa6..207a13bd841da2b8 100644 > --- a/src/libcamera/framebuffer_allocator.cpp > +++ b/src/libcamera/framebuffer_allocator.cpp > @@ -116,8 +116,7 @@ FrameBufferAllocator::~FrameBufferAllocator() > */ > int FrameBufferAllocator::allocate(Stream *stream) > { > - if (camera_->state_ != Camera::CameraConfigured && > - camera_->state_ != Camera::CameraPrepared) { > + if (camera_->state_ != Camera::CameraConfigured) { > LOG(Allocator, Error) > << "Camera must be in the configured state to allocate buffers"; > return -EACCES; > @@ -163,7 +162,7 @@ int FrameBufferAllocator::allocate(Stream *stream) > */ > int FrameBufferAllocator::free(Stream *stream) > { > - if (camera_->state_ != Camera::CameraConfigured && camera_->state_ != Camera::CameraPrepared) { > + if (camera_->state_ != Camera::CameraConfigured) { > LOG(Allocator, Error) > << "Camera must be in the configured state to free buffers"; > return -EACCES; > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 2fd65b468b9cd84f..669097f609ab7168 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -301,6 +301,11 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera) > * suitable to be added to a Request for the stream, and shall be mappable to > * the CPU through their associated dmabufs with mmap(). > * > + * The method may only be called after the Camera has been configured and before > + * it gets started, or after it gets stopped. It shall be called only for > + * streams that are part of the active camera configuration, and at most once > + * per stream until buffers for the stream are freed with freeFrameBuffers(). > + * Ah, that's where the documentation went :-) You just need to move it to the right patch. All the rest is fine here. > * exportFrameBuffers() shall also allocate all other resources required by > * the pipeline handler for the stream to prepare for starting the Camera. This > * responsibility is shared with importFrameBuffers(), and one and only one of > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index 047bf15ea7c188f7..1d9c756f147a59f7 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -172,13 +172,6 @@ int MainWindow::startCapture() > > adjustSize(); > > - ret = camera_->allocateBuffers(); > - if (ret) { > - std::cerr << "Failed to allocate buffers" > - << std::endl; > - return ret; > - } > - > ret = allocator_->allocate(stream); > if (ret < 0) { > std::cerr << "Failed to allocate capture buffers" << std::endl; > @@ -244,7 +237,6 @@ error: > } > mappedBuffers_.clear(); > > - camera_->freeBuffers(); > return ret; > } > > @@ -264,7 +256,6 @@ void MainWindow::stopCapture() > } > mappedBuffers_.clear(); > > - camera_->freeBuffers(); > isCapturing_ = false; > > config_.reset(); > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp > index 10db15d6276d9bf3..44cb4e7c551be759 100644 > --- a/src/v4l2/v4l2_camera.cpp > +++ b/src/v4l2/v4l2_camera.cpp > @@ -121,10 +121,6 @@ int V4L2Camera::configure(StreamConfiguration *streamConfigOut, > > int V4L2Camera::allocBuffers(unsigned int count) > { > - int ret = camera_->allocateBuffers(); > - if (ret) > - return ret == -EACCES ? -EBUSY : ret; > - > Stream *stream = *camera_->streams().begin(); > > return bufferAllocator_->allocate(stream); > @@ -134,7 +130,6 @@ void V4L2Camera::freeBuffers() > { > Stream *stream = *camera_->streams().begin(); > bufferAllocator_->free(stream); > - camera_->freeBuffers(); > } > > FileDescriptor V4L2Camera::getBufferFd(unsigned int index) > diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp > index f506d1b221e568ad..e7048335e0317703 100644 > --- a/test/camera/buffer_import.cpp > +++ b/test/camera/buffer_import.cpp > @@ -174,11 +174,6 @@ protected: > return TestFail; > } > > - if (camera_->allocateBuffers()) { > - std::cout << "Failed to allocate buffers" << std::endl; > - return TestFail; > - } > - > Stream *stream = cfg.stream(); > > BufferSource source; > @@ -244,11 +239,6 @@ protected: > return TestFail; > } > > - if (camera_->freeBuffers()) { > - std::cout << "Failed to free buffers" << std::endl; > - return TestFail; > - } > - > return TestPass; > } > > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp > index de879ee4eb1420a6..b304d59c1c2aa9e2 100644 > --- a/test/camera/capture.cpp > +++ b/test/camera/capture.cpp > @@ -87,11 +87,6 @@ protected: > return TestFail; > } > > - if (camera_->allocateBuffers()) { > - cout << "Failed to allocate buffers" << endl; > - return TestFail; > - } > - > Stream *stream = cfg.stream(); > > int ret = allocator_->allocate(stream); > @@ -158,11 +153,6 @@ protected: > return TestFail; > } > > - if (camera_->freeBuffers()) { > - cout << "Failed to free buffers" << endl; > - return TestFail; > - } > - > return TestPass; > } > > diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp > index f3a7ca7c32a5ec97..20541b3e4752dc81 100644 > --- a/test/camera/statemachine.cpp > +++ b/test/camera/statemachine.cpp > @@ -29,12 +29,6 @@ protected: > if (camera_->configure(defconf_.get()) != -EACCES) > return TestFail; > > - if (camera_->allocateBuffers() != -EACCES) > - return TestFail; > - > - if (camera_->freeBuffers() != -EACCES) > - return TestFail; > - > if (camera_->createRequest()) > return TestFail; > > @@ -65,12 +59,6 @@ protected: > if (camera_->acquire() != -EBUSY) > return TestFail; > > - if (camera_->allocateBuffers() != -EACCES) > - return TestFail; > - > - if (camera_->freeBuffers() != -EACCES) > - return TestFail; > - > if (camera_->createRequest()) > return TestFail; > > @@ -103,57 +91,6 @@ protected: > if (camera_->acquire() != -EBUSY) > return TestFail; > > - if (camera_->freeBuffers() != -EACCES) > - return TestFail; > - > - if (camera_->createRequest()) > - return TestFail; > - > - Request request(camera_.get()); > - if (camera_->queueRequest(&request) != -EACCES) > - return TestFail; > - > - if (camera_->start() != -EACCES) > - return TestFail; > - > - if (camera_->stop() != -EACCES) > - return TestFail; > - > - /* Test operations which should pass. */ > - if (camera_->configure(defconf_.get())) > - return TestFail; > - > - /* Test valid state transitions, end in Prepared state. */ > - if (camera_->release()) > - return TestFail; > - > - if (camera_->acquire()) > - return TestFail; > - > - if (camera_->configure(defconf_.get())) > - return TestFail; > - > - if (camera_->allocateBuffers()) > - return TestFail; > - > - return TestPass; > - } > - > - int testPrepared() > - { > - /* Test operations which should fail. */ > - if (camera_->acquire() != -EBUSY) > - return TestFail; > - > - if (camera_->release() != -EBUSY) > - return TestFail; > - > - if (camera_->configure(defconf_.get()) != -EACCES) > - return TestFail; > - > - if (camera_->allocateBuffers() != -EACCES) > - return TestFail; > - > Request request1(camera_.get()); > if (camera_->queueRequest(&request1) != -EACCES) > return TestFail; > @@ -170,9 +107,6 @@ protected: > delete request2; > > /* Test valid state transitions, end in Running state. */ > - if (camera_->freeBuffers()) > - return TestFail; > - > if (camera_->release()) > return TestFail; > > @@ -182,9 +116,6 @@ protected: > if (camera_->configure(defconf_.get())) > return TestFail; > > - if (camera_->allocateBuffers()) > - return TestFail; > - > /* Use internally allocated buffers. */ > allocator_ = FrameBufferAllocator::create(camera_); > Stream *stream = *camera_->streams().begin(); > @@ -209,12 +140,6 @@ protected: > if (camera_->configure(defconf_.get()) != -EACCES) > return TestFail; > > - if (camera_->allocateBuffers() != -EACCES) > - return TestFail; > - > - if (camera_->freeBuffers() != -EACCES) > - return TestFail; > - > if (camera_->start() != -EACCES) > return TestFail; > > @@ -236,9 +161,6 @@ protected: > > delete allocator_; > > - if (camera_->freeBuffers()) > - return TestFail; > - > if (camera_->release()) > return TestFail; > > @@ -276,11 +198,6 @@ protected: > return TestFail; > } > > - if (testPrepared() != TestPass) { > - cout << "State machine in Prepared state failed" << endl; > - return TestFail; > - } > - > if (testRuning() != TestPass) { > cout << "State machine in Running state failed" << endl; > return TestFail;
diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 28655bec9ebc89ce..6597ade83288a170 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -91,9 +91,6 @@ public: std::unique_ptr<CameraConfiguration> generateConfiguration(const StreamRoles &roles); int configure(CameraConfiguration *config); - int allocateBuffers(); - int freeBuffers(); - Request *createRequest(uint64_t cookie = 0); int queueRequest(Request *request); @@ -105,7 +102,6 @@ private: CameraAvailable, CameraAcquired, CameraConfigured, - CameraPrepared, CameraRunning, }; diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 49321db07a2c93d5..a98fd744f5347432 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -77,8 +77,6 @@ int CameraDevice::open() void CameraDevice::close() { camera_->stop(); - - camera_->freeBuffers(); camera_->release(); running_ = false; @@ -690,16 +688,9 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque /* Start the camera if that's the first request we handle. */ if (!running_) { - int ret = camera_->allocateBuffers(); - if (ret) { - LOG(HAL, Error) << "Failed to allocate buffers"; - return; - } - - ret = camera_->start(); + int ret = camera_->start(); if (ret) { LOG(HAL, Error) << "Failed to start camera"; - camera_->freeBuffers(); return; } diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 738fa1c267eb6e36..7d970f991d3aaf1a 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -42,12 +42,6 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) return ret; } - ret = camera_->allocateBuffers(); - if (ret) { - std::cerr << "Failed to allocate buffers" << std::endl; - return ret; - } - camera_->requestCompleted.connect(this, &Capture::requestComplete); if (options.isSet(OptFile)) { @@ -67,8 +61,6 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) writer_ = nullptr; } - camera_->freeBuffers(); - delete allocator; return ret; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index f3a7578d0834a9d6..79a5f994f9bbc8c1 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -275,15 +275,13 @@ std::size_t CameraConfiguration::size() const * \section camera_operation Operating the Camera * * An application needs to perform a sequence of operations on a camera before - * it is ready to process requests. The camera needs to be acquired, configured - * and resources allocated or imported to prepare the camera for capture. Once - * started the camera can process requests until it is stopped. When an - * application is done with a camera all resources allocated need to be freed - * and the camera released. + * it is ready to process requests. The camera needs to be acquired and + * configured to prepare the camera for capture. Once started the camera can + * process requests until it is stopped. When an application is done with a + * camera, the camera needs to be released. * * An application may start and stop a camera multiple times as long as it is - * not released. The camera may also be reconfigured provided that all - * resources allocated are freed prior to the reconfiguration. + * not released. The camera may also be reconfigured. * * \subsection Camera States * @@ -297,7 +295,6 @@ std::size_t CameraConfiguration::size() const * node [shape = doublecircle ]; Available; * node [shape = circle ]; Acquired; * node [shape = circle ]; Configured; - * node [shape = circle ]; Prepared; * node [shape = circle ]; Running; * * Available -> Available [label = "release()"]; @@ -307,14 +304,10 @@ std::size_t CameraConfiguration::size() const * Acquired -> Configured [label = "configure()"]; * * Configured -> Available [label = "release()"]; - * Configured -> Configured [label = "configure()"]; - * Configured -> Prepared [label = "allocateBuffers()"]; + * Configured -> Configured [label = "configure(), createRequest()"]; + * Configured -> Running [label = "start()"]; * - * Prepared -> Configured [label = "freeBuffers()"]; - * Prepared -> Prepared [label = "createRequest()"]; - * Prepared -> Running [label = "start()"]; - * - * Running -> Prepared [label = "stop()"]; + * Running -> Configured [label = "stop()"]; * Running -> Running [label = "createRequest(), queueRequest()"]; * } * \enddot @@ -330,19 +323,14 @@ std::size_t CameraConfiguration::size() const * Configured state. * * \subsubsection Configured - * The camera is configured and ready for the application to prepare it with - * resources. The camera may be reconfigured multiple times until resources - * are provided and the state progresses to Prepared. - * - * \subsubsection Prepared - * The camera has been configured and provided with resources and is ready to be - * started. The application may free the camera's resources to get back to the - * Configured state or start() it to progress to the Running state. + * The camera is configured and ready to be started. The application may + * release() the camera and to get back to the Available state or start() + * it to progress to the Running state. * * \subsubsection Running * The camera is running and ready to process requests queued by the * application. The camera remains in this state until it is stopped and moved - * to the Prepared state. + * to the Configured state. */ /** @@ -420,7 +408,6 @@ static const char *const camera_state_names[] = { "Available", "Acquired", "Configured", - "Prepared", "Running", }; @@ -465,8 +452,6 @@ bool Camera::stateIs(State state) const * * \todo Deal with pending requests if the camera is disconnected in a * running state. - * \todo Update comment about Running state when importing buffers as well as - * allocating them are supported. */ void Camera::disconnect() { @@ -474,11 +459,11 @@ void Camera::disconnect() /* * If the camera was running when the hardware was removed force the - * state to Prepared to allow applications to call freeBuffers() and - * release() before deleting the camera. + * state to Configured state to allow applications to free resources + * and call release() before deleting the camera. */ if (state_ == CameraRunning) - state_ = CameraPrepared; + state_ = CameraConfigured; disconnected_ = true; disconnected.emit(this); @@ -702,53 +687,6 @@ int Camera::configure(CameraConfiguration *config) return 0; } -/** - * \brief Allocate buffers for all configured streams - * - * This function affects the state of the camera, see \ref camera_operation. - * - * \return 0 on success or a negative error code otherwise - * \retval -ENODEV The camera has been disconnected from the system - * \retval -EACCES The camera is not in a state where buffers can be allocated - * \retval -EINVAL The configuration is not valid - */ -int Camera::allocateBuffers() -{ - if (disconnected_) - return -ENODEV; - - if (!stateIs(CameraConfigured)) - return -EACCES; - - if (activeStreams_.empty()) { - LOG(Camera, Error) - << "Can't allocate buffers without streams"; - return -EINVAL; - } - - state_ = CameraPrepared; - - return 0; -} - -/** - * \brief Release all buffers from allocated pools in each stream - * - * This function affects the state of the camera, see \ref camera_operation. - * - * \return 0 on success or a negative error code otherwise - * \retval -EACCES The camera is not in a state where buffers can be freed - */ -int Camera::freeBuffers() -{ - if (!stateIs(CameraPrepared)) - return -EACCES; - - state_ = CameraConfigured; - - return 0; -} - /** * \brief Create a request object for the camera * \param[in] cookie Opaque cookie for application use @@ -764,14 +702,14 @@ int Camera::freeBuffers() * The ownership of the returned request is passed to the caller, which is * responsible for either queueing the request or deleting it. * - * This function shall only be called when the camera is in the Prepared + * This function shall only be called when the camera is in the Configured * or Running state, see \ref camera_operation. * * \return A pointer to the newly created request, or nullptr on error */ Request *Camera::createRequest(uint64_t cookie) { - if (disconnected_ || !stateBetween(CameraPrepared, CameraRunning)) + if (disconnected_ || !stateBetween(CameraConfigured, CameraRunning)) return nullptr; return new Request(this, cookie); @@ -842,7 +780,7 @@ int Camera::start() if (disconnected_) return -ENODEV; - if (!stateIs(CameraPrepared)) + if (!stateIs(CameraConfigured)) return -EACCES; LOG(Camera, Debug) << "Starting capture"; @@ -885,7 +823,7 @@ int Camera::stop() LOG(Camera, Debug) << "Stopping capture"; - state_ = CameraPrepared; + state_ = CameraConfigured; pipe_->stop(this); diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp index 57789b24e9615fa6..207a13bd841da2b8 100644 --- a/src/libcamera/framebuffer_allocator.cpp +++ b/src/libcamera/framebuffer_allocator.cpp @@ -116,8 +116,7 @@ FrameBufferAllocator::~FrameBufferAllocator() */ int FrameBufferAllocator::allocate(Stream *stream) { - if (camera_->state_ != Camera::CameraConfigured && - camera_->state_ != Camera::CameraPrepared) { + if (camera_->state_ != Camera::CameraConfigured) { LOG(Allocator, Error) << "Camera must be in the configured state to allocate buffers"; return -EACCES; @@ -163,7 +162,7 @@ int FrameBufferAllocator::allocate(Stream *stream) */ int FrameBufferAllocator::free(Stream *stream) { - if (camera_->state_ != Camera::CameraConfigured && camera_->state_ != Camera::CameraPrepared) { + if (camera_->state_ != Camera::CameraConfigured) { LOG(Allocator, Error) << "Camera must be in the configured state to free buffers"; return -EACCES; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 2fd65b468b9cd84f..669097f609ab7168 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -301,6 +301,11 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera) * suitable to be added to a Request for the stream, and shall be mappable to * the CPU through their associated dmabufs with mmap(). * + * The method may only be called after the Camera has been configured and before + * it gets started, or after it gets stopped. It shall be called only for + * streams that are part of the active camera configuration, and at most once + * per stream until buffers for the stream are freed with freeFrameBuffers(). + * * exportFrameBuffers() shall also allocate all other resources required by * the pipeline handler for the stream to prepare for starting the Camera. This * responsibility is shared with importFrameBuffers(), and one and only one of diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 047bf15ea7c188f7..1d9c756f147a59f7 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -172,13 +172,6 @@ int MainWindow::startCapture() adjustSize(); - ret = camera_->allocateBuffers(); - if (ret) { - std::cerr << "Failed to allocate buffers" - << std::endl; - return ret; - } - ret = allocator_->allocate(stream); if (ret < 0) { std::cerr << "Failed to allocate capture buffers" << std::endl; @@ -244,7 +237,6 @@ error: } mappedBuffers_.clear(); - camera_->freeBuffers(); return ret; } @@ -264,7 +256,6 @@ void MainWindow::stopCapture() } mappedBuffers_.clear(); - camera_->freeBuffers(); isCapturing_ = false; config_.reset(); diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 10db15d6276d9bf3..44cb4e7c551be759 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -121,10 +121,6 @@ int V4L2Camera::configure(StreamConfiguration *streamConfigOut, int V4L2Camera::allocBuffers(unsigned int count) { - int ret = camera_->allocateBuffers(); - if (ret) - return ret == -EACCES ? -EBUSY : ret; - Stream *stream = *camera_->streams().begin(); return bufferAllocator_->allocate(stream); @@ -134,7 +130,6 @@ void V4L2Camera::freeBuffers() { Stream *stream = *camera_->streams().begin(); bufferAllocator_->free(stream); - camera_->freeBuffers(); } FileDescriptor V4L2Camera::getBufferFd(unsigned int index) diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index f506d1b221e568ad..e7048335e0317703 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -174,11 +174,6 @@ protected: return TestFail; } - if (camera_->allocateBuffers()) { - std::cout << "Failed to allocate buffers" << std::endl; - return TestFail; - } - Stream *stream = cfg.stream(); BufferSource source; @@ -244,11 +239,6 @@ protected: return TestFail; } - if (camera_->freeBuffers()) { - std::cout << "Failed to free buffers" << std::endl; - return TestFail; - } - return TestPass; } diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index de879ee4eb1420a6..b304d59c1c2aa9e2 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -87,11 +87,6 @@ protected: return TestFail; } - if (camera_->allocateBuffers()) { - cout << "Failed to allocate buffers" << endl; - return TestFail; - } - Stream *stream = cfg.stream(); int ret = allocator_->allocate(stream); @@ -158,11 +153,6 @@ protected: return TestFail; } - if (camera_->freeBuffers()) { - cout << "Failed to free buffers" << endl; - return TestFail; - } - return TestPass; } diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index f3a7ca7c32a5ec97..20541b3e4752dc81 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -29,12 +29,6 @@ protected: if (camera_->configure(defconf_.get()) != -EACCES) return TestFail; - if (camera_->allocateBuffers() != -EACCES) - return TestFail; - - if (camera_->freeBuffers() != -EACCES) - return TestFail; - if (camera_->createRequest()) return TestFail; @@ -65,12 +59,6 @@ protected: if (camera_->acquire() != -EBUSY) return TestFail; - if (camera_->allocateBuffers() != -EACCES) - return TestFail; - - if (camera_->freeBuffers() != -EACCES) - return TestFail; - if (camera_->createRequest()) return TestFail; @@ -103,57 +91,6 @@ protected: if (camera_->acquire() != -EBUSY) return TestFail; - if (camera_->freeBuffers() != -EACCES) - return TestFail; - - if (camera_->createRequest()) - return TestFail; - - Request request(camera_.get()); - if (camera_->queueRequest(&request) != -EACCES) - return TestFail; - - if (camera_->start() != -EACCES) - return TestFail; - - if (camera_->stop() != -EACCES) - return TestFail; - - /* Test operations which should pass. */ - if (camera_->configure(defconf_.get())) - return TestFail; - - /* Test valid state transitions, end in Prepared state. */ - if (camera_->release()) - return TestFail; - - if (camera_->acquire()) - return TestFail; - - if (camera_->configure(defconf_.get())) - return TestFail; - - if (camera_->allocateBuffers()) - return TestFail; - - return TestPass; - } - - int testPrepared() - { - /* Test operations which should fail. */ - if (camera_->acquire() != -EBUSY) - return TestFail; - - if (camera_->release() != -EBUSY) - return TestFail; - - if (camera_->configure(defconf_.get()) != -EACCES) - return TestFail; - - if (camera_->allocateBuffers() != -EACCES) - return TestFail; - Request request1(camera_.get()); if (camera_->queueRequest(&request1) != -EACCES) return TestFail; @@ -170,9 +107,6 @@ protected: delete request2; /* Test valid state transitions, end in Running state. */ - if (camera_->freeBuffers()) - return TestFail; - if (camera_->release()) return TestFail; @@ -182,9 +116,6 @@ protected: if (camera_->configure(defconf_.get())) return TestFail; - if (camera_->allocateBuffers()) - return TestFail; - /* Use internally allocated buffers. */ allocator_ = FrameBufferAllocator::create(camera_); Stream *stream = *camera_->streams().begin(); @@ -209,12 +140,6 @@ protected: if (camera_->configure(defconf_.get()) != -EACCES) return TestFail; - if (camera_->allocateBuffers() != -EACCES) - return TestFail; - - if (camera_->freeBuffers() != -EACCES) - return TestFail; - if (camera_->start() != -EACCES) return TestFail; @@ -236,9 +161,6 @@ protected: delete allocator_; - if (camera_->freeBuffers()) - return TestFail; - if (camera_->release()) return TestFail; @@ -276,11 +198,6 @@ protected: return TestFail; } - if (testPrepared() != TestPass) { - cout << "State machine in Prepared state failed" << endl; - return TestFail; - } - if (testRuning() != TestPass) { cout << "State machine in Running state failed" << endl; return TestFail;