fix a stale channel state, truncated audio settings and an RTCP send to fd 0

[kasperiio]

Four independent defects, found while bringing up a new platform but none of them platform-specific. Split out of a larger branch so they can be reviewed on their own.

• media — a failed create_channel, video_create or bind_channel returned through HAL_ERROR leaving chnState[index].enable true, so the video thread kept polling a channel that was never created.
• server — audio bitrate, gain and srate were parsed into a short. 48000 does not fit, so POST /api/audio?srate=48000 silently stored −17536.
• server — a POST with no Content-Type header dereferenced NULL inside STARTS_WITH.
• rtcp — __rtcp_send_sr() sent on con->trans[track_id].server_port_rtp without checking the track had ever been SETUP. A client that sets up video only leaves the audio track at fd 0, so the sender report went to stdin.

Testing

Built for fh8856v100_lite (ARMv6). Runs as part of the combined branch this was split from, on three Fullhan FH8856 cameras. This branch on its own is build-tested, not separately run on hardware.

Two follow-ups are prepared on top of this: RTSP interleaved-send/timestamp work, and a Fullhan FH8852/FH8856 HAL.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Fix media state, audio parsing, POST handling, and RTCP transport

🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Clear enabled media-channel state when channel, encoder, or binding setup fails.
• Preserve full-width audio values and tolerate POST requests without Content-Type.
• Skip RTCP sender reports for tracks without initialized UDP or TCP transport.

Diagram

graph TD
  API["Server API"] -->|stores integers| CFG["Audio Config"]
  API -->|checks header| OSD["OSD Upload"]
  MEDIA["Media Enable"] -->|creates| HAL["HAL Setup"] -->|failure clears| STATE["Channel State"]
  RTCP["RTCP Sender"] -->|checks setup| TRANS["Track Transport"]

Loading

High-Level Assessment

The targeted guards are appropriate for these independent defects and minimize regression risk. A centralized media-initialization rollback helper was considered, but it would broaden this focused fix; clearing state immediately before HAL_ERROR is consistent with the macro’s immediate-return behavior.

Files changed (3) +25 / -4

Bug fix (3) +25 / -4

media.cClear channel enable state after media setup failures +18/-0

Clear channel enable state after media setup failures

• Marks MJPEG and MP4 channels disabled when channel creation, encoder creation, or channel binding fails. This prevents the video thread from polling channels that were never successfully initialized.

src/media.c

rtcp.hSkip RTCP reports for tracks without transport setup +3/-0

Skip RTCP reports for tracks without transport setup

• Returns successfully without sending a sender report when a track has neither a UDP server port nor interleaved TCP transport. This prevents unconfigured tracks from using the zero-initialized file descriptor.

src/rtsp/rtcp.h

server.cPreserve audio values and guard missing Content-Type +4/-4

Preserve audio values and guard missing Content-Type

• Parses audio bitrate, gain, and sample rate into full-width integers, preventing values such as 48000 from being truncated. Also checks that Content-Type exists before identifying multipart OSD uploads.

src/server.c

[qodo-free-for-open-source-projects]

Code Review by Qodo

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

1. Failure cleanup is skipped ✓ Resolved 🐞 Bug ☼ Reliability

Description

On encoder or bind failure, the new assignment clears chnState[index].enable before HAL_ERROR
returns, causing both disable functions to skip cleanup of the channel and any successfully created
encoder. Subsequent API retries can reuse the slot while its previous HAL resources remain allocated
or bound.

Code

src/media.c[R587-590]

      if (ret)
+        {
+            chnState[index].enable = false;
          HAL_ERROR("media", "Creating encoder %d failed with %#x!\n%s\n",

Evidence

take_next_free_channel reserves a slot by setting enable=true, while the disable functions
immediately skip slots where it is false. HAL_ERROR returns from the function, so the changed
assignments prevent the only normal unbind/destroy paths from running; the HTTP reconfiguration
paths subsequently call disable and enable again while ignoring the enable result.

src/media.c[303-313]
src/media.c[525-539]
src/media.c[605-620]
src/hal/macros.h[35-41]
src/server.c[1064-1067]
src/server.c[1144-1147]

Agent prompt

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

## Issue description
Failed media initialization clears the channel's enabled state before releasing resources created by earlier initialization stages. Because `HAL_ERROR` returns immediately and disable functions skip disabled slots, retries can encounter leaked or stale HAL resources.
## Issue Context
Apply stage-aware rollback to both MJPEG and MP4 initialization. Destroy or unbind only resources whose creation succeeded, then mark the slot disabled and return the failure.
## Fix Focus Areas
- src/media.c[525-603]
- src/media.c[605-691]
- src/media.c[303-409]

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

---

2. Track check reads out-of-bounds ✓ Resolved 🐞 Bug ≡ Correctness

Description

__rtcp_send_sr indexes con->trans[track_id] in the new early-return condition before validating
that track_id is within the two-element array. An invalid ID therefore causes an out-of-bounds
read before the existing assertion can return FAILURE.

Code

src/rtsp/rtcp.h[R23-24]

+    /* A track without a transport (never SETUP) has fd 0, which is stdin */
+    if (!con->trans[track_id].server_port_rtp && !con->trans[track_id].is_tcp) return SUCCESS;

Evidence

The changed line dereferences trans at line 24, whereas the bounds assertion is not executed until
lines 32–34. The connection structure defines trans as an array of exactly two elements.

src/rtsp/rtcp.h[21-35]
src/rtsp/rtsp.h[98-106]

Agent prompt

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

## Issue description
The newly added transport-state guard dereferences the transport array before the function's track bounds assertion. Move validation ahead of every `con->trans[track_id]` access.
## Issue Context
`connection_item_t::trans` contains exactly two tracks, and the existing assertion already defines the required bounds.
## Fix Focus Areas
- src/rtsp/rtcp.h[21-35]

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

---

3. Audio ranges remain unchecked ✓ Resolved 🐞 Bug ≡ Correctness

Description

The widened strtol results are still accepted after any partial numeric conversion without
overflow or supported-range validation, allowing values such as negative sample rates or bitrates to
wrap when assigned to unsigned configuration fields. The invalid settings are immediately passed to
audio reinitialization even though configuration loading restricts bitrate, gain, and sample rate to
defined ranges.

Code

src/server.c[R889-891]

+                    int result = strtol(value, &remain, 10);
                  if (remain != value)
                      app_config.audio_srate = result;

Evidence

The API checks only whether parsing consumed at least one character, then assigns directly to the
configuration and restarts audio. The destination declarations make bitrate and sample rate
unsigned, while the configuration loader proves the intended accepted ranges.

src/server.c[866-897]
src/app_config.h[77-81]
src/app_config.c[454-459]
src/media.c[452-481]

Agent prompt

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

## Issue description
The audio API directly stores unchecked `strtol` conversions. Reject overflow, trailing nonnumeric characters, and values outside the same ranges enforced by configuration loading before modifying or reinitializing audio state.
## Issue Context
The supported ranges are bitrate 32–320, gain -60–30, and sample rate 8000–96000. Bitrate and sample rate destinations are unsigned.
## Fix Focus Areas
- src/server.c[866-897]
- src/app_config.c[454-459]
- src/app_config.h[77-81]

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

---

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[wberube]

Thank you very much for the fixes! The stale channel state and RTCP-to-stdin issues were good catches, I refined a few things on top, mainly moving the ranged-value parser to hal/tools so it can be reused by other modules.

Apologies for the delay in reviewing this, I’ll make sure to get to the other two PRs you opened promptly as well.