Skip to content

osd: keep the main stream at full rate on SSC32x, add background boxes and centering - #52

Open
Jamp wants to merge 13 commits into
OpenIPC:masterfrom
Jamp:osd-regions
Open

Jamp wants to merge 13 commits into
OpenIPC:masterfrom
Jamp:osd-regions

Conversation

@Jamp

@Jamp Jamp commented Sep 30, 2026

Copy link
Copy Markdown
Contributor
  • Absent regN_* keys wiped their defaults (opacity 0, no font), so a region with only a text was invisible; the defaults are now seeded before parsing.
  • On i6 the region was attached again every second, and recreated whenever the text width changed; attach once and keep canvases stable.
  • On SSC323 a canvas made from the region thread lands in the MMA gap the encoder's frame buffers come and go from, and with 3DNR on the main stream drops from 20 to 13 fps. Canvases are now reserved in sdk_start, before the pipeline, like majestic's fixed ones.
  • regN_bgcolor draws the text over a box (0x8000 is opaque black, with a small margin), also through /api/osd/N as bgcolor=#RRGGBB or none; regN_posx: -1 centers the region.
  • Single-quoted YAML values are unquoted like double-quoted ones (yaml-cli writes them).
  • doc/overlays.md and doc/endpoints.md cover the new options.

Tested on SSC323 with 3DNR, a JPEG channel and a second VPE consumer: date top left and a name centered, both over black boxes, 20 fps and no MMA allocation failures.

The HAL changes are kept inside src/hal/star/i6_* and conditioned on the series where they are specific to SSC32x; glad to rebase once the HAL naming refactor you mentioned in #43 lands.

A clock rendered with a proportional font changes its width every few
seconds and the region got destroyed and recreated each time, freeing
and reallocating its MMA blocks between the VPE frame buffers. The
canvas is now rounded up to 32x16 and only grows, and the bitmap is
padded with transparent pixels to its size.
Made from the region thread, a canvas lands in the MMA gap the encoder's
frame buffers are taken from and returned to, and on SSC323 the main
stream then drops to 13 fps with 3DNR on. Created once at startup, with a
quarter more width than the text needs, it sits below them: 20 fps with
OSD, 3DNR, JPEG snapshots and motion detection.
regX_bgcolor fills the text canvas with a color, with a small margin on
either side, so white text stays readable over bright walls; it is also
set and reported through /api/osd/X as bgcolor=#RRGGBB or none. A regX_posx
of -1 centers the region on the main stream.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Stabilize i6 OSD canvases and add background boxes and centering

🐞 Bug fix ✨ Enhancement 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Reserve i6 text canvases before pipeline startup to prevent OSD allocation churn and frame drops.
• Preserve OSD defaults and add background boxes and horizontally centered regions through
 configuration and API.
• Accept single-quoted configuration values and document the new overlay options.
Diagram

graph TD
  Config["OSD config"] --> State["Region state"] --> Reserve["Early reservation"] --> Pipeline["Video pipeline"] --> Thread["Region thread"] --> Renderer["Text renderer"] --> Canvas["i6 canvas"]
  API["OSD API"] --> State
  State --> Thread
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Preallocate fixed canvases for every OSD slot
  • ➕ Avoids later region allocation when an overlay is added through the API or text outgrows its initial canvas.
  • ➖ Reserves more scarce MMA memory, including for unused regions, and requires a practical maximum canvas size.

Recommendation: The PR's measured, early reservation with width headroom is a reasonable balance for configured text overlays and limits MMA use. Reviewers should weigh that against the remaining allocation risk when text exceeds its reserved dimensions or a region is created after startup.

Files changed (13) +209 / -57

Enhancement (6) +89 / -38
i6_hal.hDeclare the i6 region reservation entry point +1/-0

Declare the i6 region reservation entry point

• Exposes 'i6_region_prepare' for reserving a text canvas before pipeline startup.

src/hal/star/i6_hal.h

region.cPrepare OSD regions and apply centered text rendering +67/-29

Prepare OSD regions and apply centered text rendering

• Extracts region defaults and font lookup, pre-renders configured i6 text to size startup reservations, and computes centered X positions for text and images. Passes background colors to the text renderer.

src/region.c

region.hExtend OSD state with background color +3/-1

Extend OSD state with background color

• Adds 'bgcolor' to each OSD region and declares the defaults and startup preparation functions.

src/region.h

server.cExpose OSD background colors through the API +10/-2

Expose OSD background colors through the API

• Accepts 'bgcolor' as a color or 'none' and includes its current value in the OSD JSON response.

src/server.c

text.cRender text over optional padded background boxes +6/-4

Render text over optional padded background boxes

• Fills the text bitmap with the requested background color and adds horizontal padding when a background is present.

src/text.c

text.hExtend the text renderer interface for backgrounds +2/-2

Extend the text renderer interface for backgrounds

• Adds a background-color argument to the rendered-text function declaration.

src/text.h

Bug fix (4) +108 / -16
app_config.cPreserve OSD defaults and persist background colors +6/-2

Preserve OSD defaults and persist background colors

• Seeds region defaults before parsing, accepts centered X positions, and parses and saves background colors. Marks configured text or image regions for an initial update.

src/app_config.c

config.cUnquote single-quoted configuration values +2/-1

Unquote single-quoted configuration values

• Treats matching single quotes like double quotes when extracting configuration values, supporting values written by yaml-cli.

src/hal/config.c

i6_hal.cReserve and retain aligned i6 OSD canvases +97/-13

Reserve and retain aligned i6 OSD canvases

• Adds early region reservation, rounds canvas dimensions, and avoids recreation when rendered text shrinks. Prevents redundant attachment, pads smaller bitmaps with transparent pixels, and reports attachment and bitmap failures.

src/hal/star/i6_hal.c

media.cReserve overlay regions before creating the pipeline +3/-0

Reserve overlay regions before creating the pipeline

• Calls 'region_prepare' after system initialization and before video pipeline creation, placing configured i6 canvases earlier in MMA memory.

src/media.c

Documentation (3) +12 / -3
config.mdDocument background color and centered X configuration +2/-1

Document background color and centered X configuration

• Describes 'regX_bgcolor', its transparency bit and default, and 'regX_posx: -1' for centering.

doc/config.md

endpoints.mdDocument OSD API background and centering options +4/-2

Document OSD API background and centering options

• Adds the 'bgcolor' parameter and response field, and notes that 'posx=-1' centers an overlay.

doc/endpoints.md

overlays.mdAdd examples for boxed and centered overlays +6/-0

Add examples for boxed and centered overlays

• Shows API requests that set a background box, center text, and remove the background.

doc/overlays.md

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Centered overlays miss smaller streams 🐞 Bug ≡ Correctness
Description
region_posx() calculates one X coordinate from the MP4 width whenever MP4 is enabled, and passes
it to the shared region attachment. If MP4 and MJPEG have different widths, an overlay centered on
MP4 is displaced or clipped on the MJPEG output.
Code

src/region.c[R352-353]

+    short frame = app_config.mp4_enable ? app_config.mp4_width : app_config.mjpeg_width;
+    return MAX(frame - width, 0) / 2 & ~1;
Evidence
The new helper chooses a single configured width, but MP4 and MJPEG have independent dimensions and
the I6 region is attached to all enabled VPE ports.

src/region.c[348-354]
src/region.c[424-436]
src/media.c[621-637]
src/media.c[707-723]
src/hal/star/i6_hal.c[442-460]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A negative X position is resolved using only one configured stream width even though regions attach to multiple outputs.
## Fix Focus Areas
- src/region.c[348-354]
- src/hal/star/i6_hal.c[442-460]
## Recommended Fix
Resolve centered positions per output using that output's dimensions, and apply the corresponding attachment coordinates on each supported multi-output backend.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Short hex backgrounds render transparent ✓ Resolved
Description
text_create_rendered() fills the canvas directly with the new background value, but
color_parse() does not set the ARGB1555 alpha bit for #RGB colors. Setting bgcolor=#f00, for
example, leaves the box pixels transparent even though the text renders.
Code

src/text.c[183]

+    text_new_rendered(&canvas, (CEILING(width) + 2 * pad + 3) & ~3, CEILING(height), background);
Evidence
The API accepts color_parse() output; its short-hex branch returns bit 16 rather than alpha bit
15. The canvas fill copies that value unchanged, whereas glyph composition explicitly sets bit 15.

src/server.c[1457-1458]
src/hal/tools.c[90-97]
src/text.c[131-141]
src/text.c[78-96]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Short-form hex background colors lack the alpha bit when copied directly into canvas pixels.
## Fix Focus Areas
- src/server.c[1457-1458]
- src/text.c[181-183]
## Recommended Fix
Normalize accepted background colors to opaque ARGB1555 before filling the canvas, or reject unsupported color forms in the API.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Growing text recreates video canvases 🐞 Bug ➹ Performance
Description
i6_region_prepare() reserves only 25% beyond the text's startup width, after which
i6_region_create() recreates a region at its exact newly required width. A dynamic value that
crosses that reserve and continues growing can therefore repeatedly recreate its canvas after the
video pipeline starts, bringing back the allocation churn this change is intended to prevent.
Code

src/hal/star/i6_hal.c[R506-507]

+    region.size.width = (width + width / 4 + I6_RGN_CANVAS_W - 1) & ~(I6_RGN_CANVAS_W - 1);
+    region.size.height = (height + I6_RGN_CANVAS_H - 1) & ~(I6_RGN_CANVAS_H - 1);
Evidence
Startup reservation adds one-quarter of the initial width. Runtime formatting can change the
rendered width, and the grow path destroys and recreates a region whenever its existing width is
insufficient.

src/hal/star/i6_hal.c[401-418]
src/hal/star/i6_hal.c[505-511]
src/region.c[408-436]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Dynamic text that exceeds the startup reserve is recreated at its exact current size, allowing repeated post-pipeline canvas allocations.
## Fix Focus Areas
- src/hal/star/i6_hal.c[401-418]
- src/hal/star/i6_hal.c[505-511]
## Recommended Fix
When a region must grow, allocate additional headroom using a consistent growth policy rather than recreating it at only the current required size.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View review recommended (2)
4. Re-created streams lose their OSD ✓ Resolved
Description
i6_region_create calls fnGetChannelConfig only for the default dest.port (port 0) and returns
if that port is attached, skipping the enabled-port loop and its fnAttachChannel calls. When
MJPEG, MP4, or JPEG channels are rebuilt or another stream is enabled after OSD startup, its VPE
port remains unattached through subsequent OSD updates.
Code

src/hal/star/i6_hal.c[R439-440]

+    if (!attach)
+        return EXIT_SUCCESS;
Evidence
The attachment check initially targets port 0, and its early return bypasses the loop that attaches
every enabled port. Channels can be recreated at runtime, and channel destruction disables the
corresponding VPE port, so an attachment on port 0 does not establish that a newly enabled port is
attached.

src/hal/star/i6_hal.c[424-463]
src/server.c[1025-1026]
src/server.c[1085-1086]
src/hal/star/i6_hal.c[717-745]
src/hal/star/i6_hal.c[394-395]
src/hal/star/i6_hal.c[424-460]
src/media.c[702-769]
src/region.c[418-436]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`i6_region_create` bases its attachment decision on port 0, so an existing attachment there prevents newly enabled or recreated stream ports from being attached.
## Fix Focus Areas
- src/hal/star/i6_hal.c[424-463]
## Recommended Fix
Loop over enabled ports and call `fnGetChannelConfig` with `dest.port = i` for each one. Check each port's attachment and point/alpha attributes individually, detaching and attaching that port only when its attachment is missing or its attributes differ; do not return based on another port's attachment.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Invalid background values show a white box ✓ Resolved
Description
The bgcolor handler stores color_parse(value) directly, and color_parse returns 0xFFFF
(opaque white) for unparseable input such as 0 or transparent, while #RGB returns a value
without bit 15. The result is a white box behind the text for values the docs say mean none, and for
short hex a padded box that the API reports as none.
Code

src/server.c[R1457-1458]

+                else if (EQUALS(key, "bgcolor"))
+                    osds[id].bgcolor = EQUALS(value, "none") ? 0 : color_parse(value);
Evidence
color_parse returns 0xFFFF on failure and a #RGB value without the 0x8000 bit. text_new_rendered
fills the canvas with any nonzero background.

src/hal/tools.c[85-124]
src/text.c[131-142]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
bgcolor accepts color_parse's 0xFFFF fallback and the #RGB format without bit 15, producing white or inconsistent boxes.
## Fix Focus Areas
- src/server.c[1457-1458]
## Recommended Fix
Map "none", "0" and empty input to 0. Otherwise parse the value and reject 0xFFFF when the input was not an explicit white. Set 0x8000 consistently for every parsed format, or leave bgcolor unchanged on invalid input.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

6. A centered clock jitters and re-attaches ✓ Resolved
Description
region_posx recomputes x from the rendered bitmap width, which changes every second for a
proportional-font clock. i6_region_create then sees attribCurr.point.x != rect.x and detaches
and re-attaches the region on all ports each second, bringing back the churn this PR set out to
remove.
Code

src/region.c[R349-354]

+static short region_posx(char id, short width) {
+    if (osds[id].posx >= 0) return osds[id].posx;
+
+    short frame = app_config.mp4_enable ? app_config.mp4_width : app_config.mjpeg_width;
+    return MAX(frame - width, 0) / 2 & ~1;
+}
Evidence
A change in x triggers the detach/reattach path in i6_region_create.

src/hal/star/i6_hal.c[427-437]
src/region.c[426-427]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Centering uses the per-frame text width, so x changes each second and triggers a reattach.
## Fix Focus Areas
- src/region.c[349-354]
## Recommended Fix
Center using the widest width seen so far for the region, or the reserved canvas width, so x only changes when the canvas grows.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Enlarged canvases can overflow the frame ✓ Resolved
Description
Canvases are now rounded up to 32 pixels, and reserved ones get another 25% of width, with
transparent padding on the right. A region whose text ends near the frame's right edge, or one
attached to a narrower JPEG port, may extend past the frame so that fnAttachChannel fails.
Code

src/hal/star/i6_hal.c[R506-507]

+    region.size.width = (width + width / 4 + I6_RGN_CANVAS_W - 1) & ~(I6_RGN_CANVAS_W - 1);
+    region.size.height = (height + I6_RGN_CANVAS_H - 1) & ~(I6_RGN_CANVAS_H - 1);
Evidence
Rounding and the 25% growth enlarge the canvas beyond the text. The attach call is now
error-checked, which suggests it can fail.

src/hal/star/i6_hal.c[401-402]
src/hal/star/i6_hal.c[451-463]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Padded canvases may exceed the port width at the given x.
## Fix Focus Areas
- src/hal/star/i6_hal.c[401-402]
- src/hal/star/i6_hal.c[506-507]
## Recommended Fix
Limit the canvas width so that x + width stays within the smallest enabled port width, falling back to the exact bitmap width when the padded size would overflow.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. Uploaded overlay images vanish after restart 🐞 Bug ≡ Correctness
Description
app_config_parse() leaves updt false when neither regN_text nor regN_img is configured, even
though the region thread can load an image from the default /tmp/osdN.bmp path when img is
empty. After an API upload clears the text, a bitmap that survives a service restart is not loaded
until another OSD update occurs.
Code

src/app_config.c[415]

+            osds[i].updt = !EMPTY(osds[i].text) || !EMPTY(osds[i].img);
Evidence
The upload handler writes to the default /tmp/osdN.bmp path and clears the text without setting
osds[id].img. The region thread checks that fallback path only when updt is set, but the changed
startup condition does not set it for a region with neither configured text nor image.

src/region.c[475-481]
src/server.c[1375-1387]
src/server.c[1373-1387]
src/app_config.c[415-415]
src/region.c[477-486]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Startup skips regions backed only by an existing bitmap at the default upload path when no text or image path is configured.
## Fix Focus Areas
- src/app_config.c[415-415]
- src/region.c[477-486]
## Recommended Fix
At startup, set `updt` when `access()` finds `/tmp/osd%d.bmp` or `/tmp/osd%d.png` for a region, even if neither text nor an image path is configured. Keep regions without configured content or an existing default image inactive.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can route each severity your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/region.c Outdated
Comment thread src/text.c
Comment thread src/hal/star/i6_hal.c Outdated
Comment thread src/hal/star/i6_hal.c Outdated
Comment thread src/server.c Outdated
Comment thread src/region.c
Comment thread src/hal/star/i6_hal.c Outdated
Comment thread src/app_config.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant