Skip to content

Refactor EventStream::sendFrame to use reusable filepath member #4616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
connortechnology merged 4 commits into from
Mar 4, 2026
Merged

Refactor EventStream::sendFrame to use reusable filepath member #4616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
678562a
Initial plan
Copilot Feb 11, 2026
a843684
refactor: use reusable reuse_filepath_ member in EventStream::sendFrame
Copilot Feb 11, 2026
a335da1
refactor: remove unused reuse_image_ member to keep changes minimal
Copilot Feb 11, 2026
f61a2e0
Merge branch 'master' into copilot/refactor-filepath-handling
connortechnology Mar 4, 2026
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: 20 additions & 18 deletions src/zm_eventstream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,27 +845,29 @@ bool EventStream::sendFrame(Microseconds delta_us) {
curr_frame_id = std::clamp(curr_frame_id, 1, (int)event_data->frames.size());
}

std::string filepath;
// Reusable string member avoids per-frame heap allocations.
// After the first frame, the string's buffer is reused (unless path exceeds capacity).
reuse_filepath_.clear();
struct stat filestat = {};

// This needs to be abstracted. If we are saving jpgs, then load the capture file.
// If we are only saving analysis frames, then send that.
if ((frame_type == FRAME_ANALYSIS) && (event_data->SaveJPEGs & 2)) {
filepath = stringtf(staticConfig.analyse_file_format.c_str(), event_data->path.c_str(), curr_frame_id);
if (stat(filepath.c_str(), &filestat) < 0) {
Debug(1, "analyze file %s not found will try to stream from other", filepath.c_str());
filepath = stringtf(staticConfig.capture_file_format.c_str(), event_data->path.c_str(), curr_frame_id);
if (stat(filepath.c_str(), &filestat) < 0) {
Debug(1, "capture file %s not found either", filepath.c_str());
filepath = "";
reuse_filepath_ = stringtf(staticConfig.analyse_file_format.c_str(), event_data->path.c_str(), curr_frame_id);
if (stat(reuse_filepath_.c_str(), &filestat) < 0) {
Comment on lines +848 to +857
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

The new comments claim this avoids per-frame heap allocations, but stringtf() allocates a heap buffer (unique_ptr<char[]>) on every call. Reusing the destination std::string capacity only avoids reallocation of reuse_filepath_ itself, not the allocations inside stringtf(). To actually eliminate per-frame heap allocations, keep the prior snprintf/stack-buffer approach or introduce a formatter that writes directly into reuse_filepath_ (e.g., vsnprintf into a resized string buffer).

Copilot uses AI. Check for mistakes.
Debug(1, "analyze file %s not found will try to stream from other", reuse_filepath_.c_str());
reuse_filepath_ = stringtf(staticConfig.capture_file_format.c_str(), event_data->path.c_str(), curr_frame_id);
if (stat(reuse_filepath_.c_str(), &filestat) < 0) {
Debug(1, "capture file %s not found either", reuse_filepath_.c_str());
reuse_filepath_.clear();
}
}
} else if (event_data->SaveJPEGs & 1) {
filepath = stringtf(staticConfig.capture_file_format.c_str(), event_data->path.c_str(), curr_frame_id);
if (stat(filepath.c_str(), &filestat) < 0) {
reuse_filepath_ = stringtf(staticConfig.capture_file_format.c_str(), event_data->path.c_str(), curr_frame_id);
if (stat(reuse_filepath.c_str(), &filestat) < 0) {
Comment on lines 851 to +867
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

filestat is only used as an out-param to stat() for existence checks, and none of its fields are read afterwards. This adds unnecessary work in the hot path. Consider using access(path, R_OK) (as in PR #4609) and drop filestat entirely, or otherwise avoid filling a struct stat when you only need to know if the file is readable.

Copilot uses AI. Check for mistakes.
Debug(1, "Capture file %s not found (bulk/interpolated frame %d), trying ffmpeg_input",
filepath.c_str(), curr_frame_id);
filepath = "";
reuse_filepath.c_str(), curr_frame_id);
reuse_filepathi.clear();
// Fall through — ffmpeg_input will be tried below if available
Comment on lines +866 to 871
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

Typo/identifier mismatches here will not compile: reuse_filepath should be reuse_filepath_, and reuse_filepathi is an undefined identifier. Also the .c_str() call in the Debug uses the wrong variable name. Rename these consistently to reuse_filepath_ and clear via reuse_filepath_.clear().

Copilot uses AI. Check for mistakes.
}
} else if (!ffmpeg_input) {
Expand All @@ -874,7 +876,7 @@ bool EventStream::sendFrame(Microseconds delta_us) {
}

if ( type == STREAM_MPEG ) {
Image image(filepath.c_str());
Image image(reuse_filepath_.c_str());

Image *send_image = prepareImage(&image);

Expand All @@ -889,19 +891,19 @@ bool EventStream::sendFrame(Microseconds delta_us) {
config.mpeg_timed_frames,
delta_us.count() * 1000);
} else {
bool send_raw = (type == STREAM_JPEG) && ((scale >= ZM_SCALE_BASE) && (zoom == ZM_SCALE_BASE)) && !filepath.empty();
bool send_raw = (type == STREAM_JPEG) && ((scale >= ZM_SCALE_BASE) && (zoom == ZM_SCALE_BASE)) && !reuse_filepath_.empty();

if (send_raw) {
fprintf(stdout, "--" BOUNDARY "\r\n");
if (!send_file(filepath)) {
Error("Can't send %s: %s", filepath.c_str(), strerror(errno));
if (!send_file(reuse_filepath_)) {
Error("Can't send %s: %s", reuse_filepath_.c_str(), strerror(errno));
return false;
}
} else {
Image *image = nullptr;

if (!filepath.empty()) {
image = new Image(filepath.c_str());
if (!reuse_filepath_.empty()) {
image = new Image(reuse_filepath_.c_str());
} else if (ffmpeg_input) {
// Get the frame from the mp4 input
const FrameData *frame_data = &event_data->frames[curr_frame_id-1];
Expand Down
1 change: 1 addition & 0 deletions src/zm_eventstream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ class EventStream : public StreamBase {
bool send_buffer(uint8_t * buffer, int size);
Storage *storage;
FFmpeg_Input *ffmpeg_input;
std::string reuse_filepath_; // reused across sendFrame calls to avoid per-frame heap alloc
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

The member comment says it avoids per-frame heap allocations, but with the current stringtf() usage there are still heap allocations per call. Consider rewording to reflect that this primarily reuses the destination string’s capacity (and/or changing the formatting approach so the comment becomes true).

Suggested change
std::string reuse_filepath_; // reused across sendFrame calls to avoid per-frame heap alloc
std::string reuse_filepath_; // reused across sendFrame calls to reuse capacity and reduce per-frame heap allocations

Copilot uses AI. Check for mistakes.
};

#endif // ZM_EVENTSTREAM_H
Loading