Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions src/gsmenu/colmenu.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ extern lv_indev_t * indev_drv;
extern int audio_get_enabled(void);
extern void audio_set_enabled(int enabled);

/* Restream C API (gstrtpreceiver.cpp on device, stubs in simulator.c) — declared
* here rather than including gstrtpreceiver.h, which drags in gst/gst.h. */
extern bool restream_get_enabled(void);
extern void restream_set_enabled(bool enabled);
extern void restream_scan_clients(char * buf, size_t buf_len);
extern const char * restream_get_manual_ip(void);
extern void restream_set_manual_ip(const char * ip);
Comment on lines +20 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Informational

2. Duplicated restream api decls 🐞 Bug ⚙ Maintainability

colmenu.c manually re-declares the restream_* API instead of including the canonical declarations,
so future signature/linkage changes can silently desync and become hard to detect. This introduces
unnecessary interface drift risk across device implementation and simulator stubs.
Agent Prompt
## Issue description
`src/gsmenu/colmenu.c` adds manual `extern` declarations for the Restream API to avoid including `gstrtpreceiver.h` (which pulls in GStreamer headers). This duplicates the interface already declared in `gstrtpreceiver.h`, creating a long-term risk of signature drift.

## Issue Context
The PR comment explains the motivation (avoiding heavy GStreamer includes), but duplication is still avoidable by splitting the lightweight C API into a dedicated header.

## Fix Focus Areas
- src/gsmenu/colmenu.c[17-23]
- src/gstrtpreceiver.h[113-124]

## Suggested fix approach
- Create a small header (e.g., `src/restream_api.h`) containing only the `extern \"C\"` C API declarations (restream_* and any related functions) and minimal includes (`<stdbool.h>`, `<stddef.h>`).
- Include this new header from both `gstrtpreceiver.h` and `colmenu.c` (and keep simulator stubs aligned), removing the duplicated declarations from `colmenu.c`.

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


static colstack_t * g_cs;

/* True while the command-error dialog is up, so async teardowns (e.g. the text
Expand Down Expand Up @@ -103,6 +111,24 @@ char * colmenu_get(const char * type, const char * page, const char * param, cha
return strdup(audio_get_enabled() ? "1" : "0");
}

/* Restream is configured from pixelpilot.yaml and lives entirely inside the
* app — gsmenu.sh has no say in it. Serve its rows from the C API, here on
* read and in do_set() on write, so no shell round-trip happens at all. */
if(strcmp(param, "restream_enabled") == 0) {
return strdup(restream_get_enabled() ? "1" : "0");
}
if(strcmp(param, "restream_target") == 0) {
/* scan_clients already returns the dropdown's option list: "Auto" followed
* by one discovered/pinned IP per line. Empty manual ip == auto-discover. */
if(opts) {
char clients[1024] = {0};
restream_scan_clients(clients, sizeof(clients));
*opts = strdup(clients);
}
Comment on lines +124 to +127

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Truncated dropdown options 🐞 Bug ≡ Correctness

colmenu_get() copies the restream client option list through a fixed 1024-byte buffer, so a long
discovered/pinned list can be truncated mid-line and produce incomplete dropdown entries. Selecting
a truncated entry can set an invalid manual target string and prevent restream from reaching the
intended client.
Agent Prompt
## Issue description
`colmenu_get()` builds the Restream Target dropdown options by calling `restream_scan_clients()` into a fixed 1024-byte stack buffer and then `strdup()`s it. If the generated options string exceeds 1024 bytes, it will be truncated without preserving line boundaries, which can result in a partial/invalid last option.

## Issue Context
The device implementation (`restream_scan_clients`) generates a newline-delimited list and copies it into the provided buffer with bounded copy semantics; truncation can occur when many ARP entries exist or when pinned/manual IPs add additional lines.

## Fix Focus Areas
- src/gsmenu/colmenu.c[111-121]
- src/gstrtpreceiver.cpp[1572-1605]

## Suggested fix approach
- Increase the buffer substantially (e.g., 4096/8192) **and** defensively trim any trailing partial line by cutting back to the last `\n` when the buffer fills.
- Preferably, change the API to return the required size (or return an allocated string) so callers never guess buffer sizes.

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

const char * ip = restream_get_manual_ip();
return strdup(ip && ip[0] ? ip : "Auto");
}

char errf[] = "/tmp/gsmenu_gerr_XXXXXX";
int efd = mkstemp(errf);
if(efd >= 0) close(efd);
Expand Down Expand Up @@ -373,6 +399,18 @@ static void do_set_done(void * ctx, int rc)
static void do_set(void * ctx, const char * value)
{
bind_ctx_t * b = ctx;

/* Restream applies to the running app only (see colmenu_get) — apply it
* straight away and skip the gsmenu.sh set entirely. */
if(b->param && strncmp(b->param, "restream_", 9) == 0) {
if(strcmp(b->param, "restream_enabled") == 0)
restream_set_enabled(value && strcmp(value, "on") == 0);
else if(strcmp(b->param, "restream_target") == 0)
restream_set_manual_ip(value ? value : ""); /* "Auto" → auto-discover */
if(b->on_change) b->on_change(value ? value : "");
return;
}

char cmd[320];
snprintf(cmd, sizeof(cmd), "gsmenu.sh set %s %s %s \"%s\"",
b->type, b->page, b->param, value ? value : "");
Expand Down
14 changes: 13 additions & 1 deletion src/gsmenu/colmenu_pages.c
Original file line number Diff line number Diff line change
Expand Up @@ -442,13 +442,25 @@ static const colmenu_item_t sys_dvr_items[] = {
{ .kind=COLMENU_SWITCH, .label="Record OSD in DVR", .param="dvr_osd", .on_change=on_dvr_osd },
};
static const colmenu_page_t sys_dvr_page = { "DVR", "gs", "system", sys_dvr_items, 8 };

/* Restream to phone/laptop over the local WiFi. Both rows are served by the app
* itself, not gsmenu.sh — colmenu.c intercepts the "restream_" params on read
* and write. The Target dropdown's options are the discovered clients, with
* "Auto" (pick whoever shows up) as the first entry. */
static const colmenu_item_t restream_items[] = {
{ .kind=COLMENU_SWITCH, .icon=LV_SYMBOL_WIFI, .label="Enabled", .param="restream_enabled" },
{ .kind=COLMENU_DROPDOWN, .icon=LV_SYMBOL_WIFI, .label="Target", .param="restream_target" },
};
static const colmenu_page_t sys_restream_page = { "Restream", "gs", "system", restream_items, 2 };

static const colmenu_item_t system_items[] = {
{ .kind=COLMENU_SUBMENU, .icon=LV_SYMBOL_WIFI, .label="Receiver", .sub=&sys_receiver_page },
{ .kind=COLMENU_SUBMENU, .icon=LV_SYMBOL_AUDIO, .label="Audio", .sub=&sys_audio_page },
{ .kind=COLMENU_SUBMENU, .icon=LV_SYMBOL_IMAGE, .label="Display", .sub=&sys_display_page },
{ .kind=COLMENU_SUBMENU, .icon=LV_SYMBOL_VIDEO, .label="DVR", .sub=&sys_dvr_page },
{ .kind=COLMENU_SUBMENU, .icon=LV_SYMBOL_WIFI, .label="Restream", .sub=&sys_restream_page },
};
static const colmenu_page_t system_page = { "System", "gs", "system", system_items, 4 };
static const colmenu_page_t system_page = { "System", "gs", "system", system_items, 5 };

/* WiFi. The WiFi page shows the live connection (get gs wifi ssid) — entering the
* connected network gives Disconnect / Forget. "Networks" lists only AVAILABLE
Expand Down
10 changes: 10 additions & 0 deletions src/gstrtpreceiver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2476,6 +2476,16 @@ void restream_scan_clients(char* buf, size_t buf_len) {
combined += '\n';
combined += manual_ip;
}
// Callers hand this straight to an LVGL dropdown, which splits on '\n', so a
// buffer too small to hold every entry must still end on a line boundary —
// a half-copied address would show up as a selectable, bogus target. Drop
// the partial tail rather than offering it (no trailing '\n' either, which
// would render as an empty option).
if (combined.size() >= buf_len) {
combined.resize(buf_len - 1);
const size_t last_nl = combined.find_last_of('\n');
combined.resize(last_nl == std::string::npos ? 0 : last_nl);
}
strncpy(buf, combined.c_str(), buf_len - 1);
buf[buf_len - 1] = '\0';
}
Expand Down
22 changes: 16 additions & 6 deletions src/simulator.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "lvgl/lvgl.h"
#include "menu.h"
#include "input.h"
Expand Down Expand Up @@ -38,16 +39,25 @@ void my_log_cb(lv_log_level_t level, const char * buf)
}

// Simulator stubs for restream API (real impl lives in gstrtpreceiver.cpp,
// which is not part of the simulator build)
bool restream_get_enabled() { return false; }
void restream_set_enabled(bool enabled) { (void)enabled; }
// which is not part of the simulator build). Stateful so the menu rows can
// actually be exercised; the client list is faked.
static bool sim_restream_enabled = false;
static char sim_restream_ip[64] = "";
bool restream_get_enabled() { return sim_restream_enabled; }
void restream_set_enabled(bool enabled) { sim_restream_enabled = enabled; }
void restream_scan_clients(char* buf, size_t buf_len) {
if (buf && buf_len) snprintf(buf, buf_len, "Auto\n192.168.1.23\n192.168.1.47");
}
const char* restream_get_manual_ip() { return sim_restream_ip; }
void restream_set_manual_ip(const char* ip) {
if (ip && ip[0] && strcmp(ip, "Auto") != 0) snprintf(sim_restream_ip, sizeof(sim_restream_ip), "%s", ip);
else sim_restream_ip[0] = '\0';
}

int audio_get_enabled(void) { return 0; }
void audio_set_enabled(int enabled) { (void)enabled; }
void audio_set_device(const char* device) { (void)device; }
void audio_set_volume(int percent) { (void)percent; }
void restream_scan_clients(char* buf, size_t buf_len) { if (buf && buf_len) buf[0] = '\0'; }
const char* restream_get_manual_ip() { return ""; }
void restream_set_manual_ip(const char* ip) { (void)ip; }

int main(int argc, char **argv)
{
Expand Down