From 8ed8f211000b63886700f0dbc270ea3b0129579b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Sat, 26 Jul 2025 15:13:26 +0200 Subject: [PATCH] spa: libcamera: source: process requests on data loop Since `impl::requestComplete()` runs in an internal libcamera thread, extra care would need to be taken to validate all accesses to common data structures. For example, the function might call `spa_libcamera_buffer_recycle()`, which accesses `impl::ctrls`, which would be unsafe because it could read or modified at the same time on the data thread. So move the processing of requests to the data loop. (cherry picked from commit 019a5c130f486994f22caebb66d8e53f691c9a6f) --- spa/plugins/libcamera/libcamera-source.cpp | 121 +++++++++++---------- 1 file changed, 62 insertions(+), 59 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 9f665c523..0fb40688d 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -1020,6 +1020,63 @@ void handle_completed_request(struct impl *impl, libcamera::Request *request) impl, request, request_id, static_cast(request->status()), request->sequence()); + if (request->status() == libcamera::Request::Status::RequestCancelled) { + spa_log_trace(impl->log, "%p: request %p[%" PRIu64 "] cancelled", + impl, request, request_id); + request->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); + SPA_FLAG_SET(b->flags, BUFFER_FLAG_OUTSTANDING); + spa_libcamera_buffer_recycle(impl, port, b->id); + return; + } + + const FrameBuffer *buffer = request->findBuffer(port->streamConfig.stream()); + if (buffer == nullptr) { + spa_log_warn(impl->log, "%p: request %p[%" PRIu64 "] has no buffer for stream %p", + impl, request, request_id, port->streamConfig.stream()); + return; + } + + const FrameMetadata &fmd = buffer->metadata(); + + if (impl->clock) { + double target = (double)port->info.rate.num / port->info.rate.denom; + double corr; + + if (impl->dll.bw == 0.0) { + spa_dll_set_bw(&impl->dll, SPA_DLL_BW_MAX, port->info.rate.denom, port->info.rate.denom); + impl->clock->next_nsec = fmd.timestamp; + corr = 1.0; + } else { + double diff = ((double)impl->clock->next_nsec - (double)fmd.timestamp) / SPA_NSEC_PER_SEC; + double error = port->info.rate.denom * (diff - target); + corr = spa_dll_update(&impl->dll, SPA_CLAMPD(error, -128., 128.)); + } + /* FIXME, we should follow the driver clock and target_ values. + * for now we ignore and use our own. */ + impl->clock->target_rate = port->rate; + impl->clock->target_duration = 1; + + impl->clock->nsec = fmd.timestamp; + impl->clock->rate = port->rate; + impl->clock->position = fmd.sequence; + impl->clock->duration = 1; + impl->clock->delay = 0; + impl->clock->rate_diff = corr; + impl->clock->next_nsec += (uint64_t) (target * SPA_NSEC_PER_SEC * corr); + } + + if (b->h) { + b->h->flags = 0; + if (fmd.status != libcamera::FrameMetadata::Status::FrameSuccess) + b->h->flags |= SPA_META_HEADER_FLAG_CORRUPTED; + b->h->offset = 0; + b->h->seq = fmd.sequence; + b->h->pts = fmd.timestamp; + b->h->dts_offset = 0; + } + + request->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); + spa_list_append(&port->queue, &b->link); spa_io_buffers *io = port->io; @@ -1231,66 +1288,12 @@ spa_libcamera_alloc_buffers(struct impl *impl, struct port *port, void impl::requestComplete(libcamera::Request *request) { struct impl *impl = this; - struct port *port = &impl->out_ports[0]; - Stream *stream = port->streamConfig.stream(); - uint32_t index, buffer_id; - struct buffer *b; + uint32_t index; - spa_log_debug(impl->log, "request complete"); - - buffer_id = request->cookie(); - b = &port->buffers[buffer_id]; - - if ((request->status() == Request::RequestCancelled)) { - spa_log_debug(impl->log, "Request was cancelled"); - request->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); - SPA_FLAG_SET(b->flags, BUFFER_FLAG_OUTSTANDING); - spa_libcamera_buffer_recycle(impl, port, b->id); - return; - } - FrameBuffer *buffer = request->findBuffer(stream); - if (buffer == nullptr) { - spa_log_warn(impl->log, "unknown buffer"); - return; - } - const FrameMetadata &fmd = buffer->metadata(); - - if (impl->clock) { - double target = (double)port->info.rate.num / port->info.rate.denom; - double corr; - - if (impl->dll.bw == 0.0) { - spa_dll_set_bw(&impl->dll, SPA_DLL_BW_MAX, port->info.rate.denom, port->info.rate.denom); - impl->clock->next_nsec = fmd.timestamp; - corr = 1.0; - } else { - double diff = ((double)impl->clock->next_nsec - (double)fmd.timestamp) / SPA_NSEC_PER_SEC; - double error = port->info.rate.denom * (diff - target); - corr = spa_dll_update(&impl->dll, SPA_CLAMPD(error, -128., 128.)); - } - /* FIXME, we should follow the driver clock and target_ values. - * for now we ignore and use our own. */ - impl->clock->target_rate = port->rate; - impl->clock->target_duration = 1; - - impl->clock->nsec = fmd.timestamp; - impl->clock->rate = port->rate; - impl->clock->position = fmd.sequence; - impl->clock->duration = 1; - impl->clock->delay = 0; - impl->clock->rate_diff = corr; - impl->clock->next_nsec += (uint64_t) (target * SPA_NSEC_PER_SEC * corr); - } - if (b->h) { - b->h->flags = 0; - if (fmd.status != FrameMetadata::Status::FrameSuccess) - b->h->flags |= SPA_META_HEADER_FLAG_CORRUPTED; - b->h->offset = 0; - b->h->seq = fmd.sequence; - b->h->pts = fmd.timestamp; - b->h->dts_offset = 0; - } - request->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); + spa_log_trace(impl->log, "%p: request %p[%" PRIu64 "] completed status:%u seq:%" PRIu32, + impl, request, request->cookie(), + static_cast(request->status()), + request->sequence()); spa_ringbuffer_get_write_index(&impl->completed_requests_rb, &index); impl->completed_requests[index & MASK_BUFFERS] = request;