[Camera] Start the video stream using the parameters from the allocated stream #40658
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the video stream allocation process to ensure the video stream starts using the final allocated parameters rather than the originally requested parameters. The key improvement is adding a new callback mechanism that provides the server-finalized stream parameters to the implementation layer.
- Adds a new
OnVideoStreamAllocatedcallback to provide finalized stream parameters after server allocation - Moves video stream startup logic from allocation to the new callback to use final parameters
- Updates video pipeline creation to use allocated stream parameters instead of stored stream parameters
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| camera-av-stream-management-server.h | Adds new OnVideoStreamAllocated virtual method to delegate interface |
| camera-av-stream-management-server.cpp | Calls the new callback with finalized stream parameters after allocation |
| camera-av-stream-manager.cpp | Implements the callback and moves stream startup logic from allocation method |
| camera-av-stream-manager.h | Declares the new callback method override |
| camera-device.cpp | Updates StartVideoStream to accept and use allocated stream parameters |
| camera-device.h | Updates method signature to accept VideoStreamStruct instead of stream ID |
| camera-device-interface.h | Updates interface method signature to match implementation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the video stream startup process to use the final allocated stream parameters, which is a good improvement for correctness and flexibility. The changes are well-contained and follow a clear logic of moving the stream start call to a new delegate method OnVideoStreamAllocated, which is triggered after the server finalizes the stream parameters. I've found one minor issue with a log message that reports incorrect stream parameters, which could be misleading. Otherwise, the changes look solid.
yufengwangca
force-pushed
the
pr/camera/stream
branch
2 times, most recently
from
b117f5d to
dede2ba
Compare
|
PR #40658: Size comparison from 6e50acf to dede2ba Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #40658: Size comparison from 6e50acf to 37e23d0 Full report (33 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
yufengwangca
force-pushed
the
pr/camera/stream
branch
from
37e23d0 to
70be118
Compare
|
PR #40658: Size comparison from 4050ccd to 70be118 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #40658: Size comparison from 4050ccd to e1c42ca Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
pidarped
reviewed
Aug 21, 2025
| void CameraAVStreamManager::OnVideoStreamAllocated(const VideoStreamStruct & allocatedStream, bool shouldStartNewVideo) | ||
| { | ||
| // Only start the video stream if the flag indicates we should (new allocation or configuration changed) | ||
| if (shouldStartNewVideo) |
There was a problem hiding this comment.
If the stream is getting re-used, then there is an already active stream. If that was modified, then the stream needs to be stopped and restarted with the new parameters.
| following command: | ||
|
|
||
| ``` | ||
| v4l2-ctl -d /dev/video0 --list-formats-ext |
There was a problem hiding this comment.
Would recommend adding a v4l2-ctl --list-devices command to list the device that is connected before using it in the command above to list the formats.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Currently, there are two different stream representations:
CameraDevice HAL: Contains "available streams" (VideoStream structs) that represent the hardware capabilities and current allocation status
CameraAVStreamMgmtServer: Contains "allocated streams" (mAllocatedVideoStreams) that represent the Matter cluster view of allocated streams
How Stream Allocation Works
Here's how the flow works when a user requests to allocate a stream:
Stream Allocation Request Flow:
CameraAVStreamMgmtServer::HandleVideoStreamAllocate() receives the request
It calls mDelegate.VideoStreamAllocate() (which is CameraAVStreamManager::VideoStreamAllocate())
CameraAVStreamManager::VideoStreamAllocate() searches through mCameraDeviceHAL->GetCameraHALInterface().GetAvailableVideoStreams()
If a compatible stream is found, it marks stream.isAllocated = true in the HAL
It calls mCameraDeviceHAL->GetCameraHALInterface().StartVideoStream(outStreamID) to start the actual stream
The server then either calls AddVideoStream() or UpdateVideoStreamRangeParams() to update its mAllocatedVideoStreams
The Issue You Identified:
You're absolutely right that UpdateVideoStreamRangeParams() only updates the mAllocatedVideoStreams in the server, not the HAL streams. However, looking at the code, this is actually by design.
HAL "Available Streams" = Reference templates with wide parameter ranges
Server "Allocated Streams" = Narrowed parameter ranges representing actual allocated streams
When starting a video stream, you should use the allocated stream parameters from the server, not the original HAL parameters
Related issues
N/A
Testing
To start a live video stream from your camera, use the liveview start command
followed by the nodeID you set during pairing. For example, if your nodeID is
1, you can request a stream with a minimum resolution of 800x600 pixels and a
minimum frame rate of 30 frames per second using the command below.
To see what video formats and resolutions your camera supports, use the
following command: