Repository navigation
Conversation
GC4653 follows the requested picture size and frame rate. The library accepts 1920x1080 at 25 or 30 fps and 2560x1440 at 20 or 25 fps. An exact size uses that mode and the nearest listed rate. A smaller picture uses the smallest mode that covers it. Anything larger uses 2560x1440. Other sensors in this HAL keep their previous single mode and format. The FH8852 H.264 constant-bitrate loop ignores its target, so the requested bitrate is applied as a fixed quantizer: 38 at 1024 kbps, 36 at 2048, 34 at 4096, and 32 at 8192, and never below 30. H.265 keeps the driver's constant-bitrate control. MJPEG uses its own scaled channel at the requested size. A higher quality selects a lower encoder quantizer, from 98 at quality 1 down to 0 at quality 99. The encoder does not use the MJPEG bitrate field.
PR Summary by QodoSelect GC4653 modes and correct FH8852 video controls
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Code Review by Qodo
1. Snapshots can switch to video dimensions
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+215
to
+218
| for (char i = 0; i < _f2_vpss_count; i++) { | ||
| if (_f2_vpss_open[i]) continue; | ||
| _f2_vpss_src[index] = i; | ||
| return f2_vpss_setup(i, width, height, framerate); |
There was a problem hiding this comment.
1. Snapshots can switch to video dimensions 🐞 Bug ≡ Correctness
f2_channel_create assigns a JPEG encoder any free picture channel, but a later video encoder can reconfigure that same channel by its index without checking whether the JPEG encoder uses it. When JPEG starts first at a size different from the pipeline picture, enabling video on that index changes the source used by subsequent snapshots.
Agent Prompt
## Issue description
A JPEG encoder can be assigned a free picture channel that a later video encoder reconfigures, changing the source of subsequent snapshots.
## Fix Focus Areas
- src/hal/fhan/f2_hal.c[201-223]
- src/hal/fhan/f2_hal.c[146-173]
## Recommended Fix
Track active users of each picture channel. Before assigning an indexed channel to a video encoder, preserve or relocate any active JPEG user; never reconfigure a source while another encoder depends on it.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+215
to
+220
| for (char i = 0; i < _f2_vpss_count; i++) { | ||
| if (_f2_vpss_open[i]) continue; | ||
| _f2_vpss_src[index] = i; | ||
| return f2_vpss_setup(i, width, height, framerate); | ||
| } | ||
| _f2_vpss_src[index] = -1; |
There was a problem hiding this comment.
2. Repeated snapshots exhaust picture channels 🐞 Bug ☼ Reliability
f2_channel_create opens a free picture channel for a new JPEG size, but f2_video_destroy never releases that channel when the JPEG encoder is destroyed. After JPEG is disabled and recreated at different sizes enough times, no free channel remains; creation falls back to the main picture, whose dimensions can differ or exceed the encoder’s previously allocated size.
Agent Prompt
## Issue description
JPEG reconfiguration opens picture channels that remain allocated after the encoder is destroyed, eventually forcing a wrong-size fallback.
## Fix Focus Areas
- src/hal/fhan/f2_hal.c[208-223]
- src/hal/fhan/f2_hal.c[805-813]
## Recommended Fix
Track picture-channel ownership or users and disable a JPEG-allocated channel when its last encoder is destroyed. Reuse or reclaim unused channels on reconfiguration, and do not silently substitute the main picture when no matching channel is available.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+381
to
+385
| mode = f2_snr_match(f2_snr.name, (unsigned int)width, (unsigned int)height, | ||
| (unsigned int)(unsigned char)framerate); | ||
| snrFmt = mode ? mode->format : F2_SNR_FMT; | ||
| if (mode) { | ||
| _f2_snr_dim = mode->dim; |
There was a problem hiding this comment.
3. Streams can advertise the wrong frame rate 🐞 Bug ≡ Correctness
f2_pipeline_create selects the nearest supported sensor rate but retains the requested rate for picture-channel setup and downstream stream configuration. A 2560×1440 request at 30 fps selects the 25-fps GC4653 mode while the encoder and MP4 configuration still use 30 fps, leaving the stream timing and reported rate inconsistent with its source.
Agent Prompt
## Issue description
Nearest-rate sensor selection does not update the frame rate used to configure picture channels, encoders, and stream metadata.
## Fix Focus Areas
- src/hal/fhan/f2_hal.c[375-392]
- src/hal/fhan/f2_hal.c[430-439]
- src/media.c[929-954]
- src/media.c[748-797]
## Recommended Fix
Expose the selected sensor frame rate to downstream setup. Configure picture-channel input timing from that actual rate, and set encoder and stream metadata rates to achievable output rates when a request is rounded to a supported sensor mode.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Author
|
Now the camera is on openwall: |
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
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
This is the FH8852V200 stream work for a GC4653 camera.
0x203,0x204) and 2560×1440 at 20 or 25 fps (0x602,0x603). An exact size selects that mode and the nearest listed rate. A smaller picture uses the smallest mode that covers it. Anything larger uses 2560×1440. The mode is chosen when the pipeline starts, so a resolution change needs a restart.0x203they already had. Their libraries were checked, not reflashed onto hardware.jxf32,mn34425, andovos02kdo accept0x203.ovos04c10andovos08do not list0x203; that request is unchanged from the previous HAL.ovos05lists0x203as 1080p, while the HAL size remains 2592×1944, also unchanged.gc5603is still in the table and is not shipped in this firmware package.FH_VENC_SetRCAttr.Test plan
Checked on an FH8852V200 with GC4653:
Using gc4653_mipi 2560x1440 at 25 fps (format 0x603)/image.jpgis 2560×1440