From b2b50aba5d3e26d6459e03ab0e58521f7127401f Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 29 Apr 2026 19:39:04 -0400 Subject: [PATCH 1/2] perf: skip clearPackets early-returns and allow drop-to-iterator-keyframe Two related changes to PacketQueue::clearPackets, called by the analysis thread on every video packet: 1. Lock-free call-site gate (should_try_clear) on the analysis path. In keep_keyframes mode the existing early-return at the top of clearPackets discards most non-keyframe video packets after acquiring the queue mutex. Add an inline lock-free check at the call site so non-keyframe packets skip the mutex acquire entirely. clear_packets_pending_ is now std::atomic so it can be read without the lock; a stale read is harmless (at worst we make one extra cheap early-returning call). The !keep_keyframes path always returns true from the gate because that mode pops one packet at a time on every video packet. 2. Iterator boundary in the scan loop changed from >= to >. Setting next_front to a packet that an iterator points at is safe because clearPackets deletes strictly before next_front, so the iterator's own packet stays in the queue. Previously, an event-start (or other) iterator landing exactly on a keyframe blocked the leading GOP from being dropped until the iterator advanced; now we can include that keyframe as next_front while the iterator continues to point at it. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/zm_monitor.cpp | 4 +++- src/zm_packetqueue.cpp | 11 ++++++++--- src/zm_packetqueue.h | 14 +++++++++++++- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index f40623f14..4027127e5 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -2464,7 +2464,9 @@ bool Monitor::Analyse() { } // end if ( trigger_data->trigger_state != TRIGGER_OFF ) if (packet->codec_type == AVMEDIA_TYPE_VIDEO) { - packetqueue.clearPackets(packet); + if (packetqueue.should_try_clear(packet->keyframe)) { + packetqueue.clearPackets(packet); + } // Only do these if it's a video packet. shared_data->last_read_index = packet->image_index; analysis_image_count++; diff --git a/src/zm_packetqueue.cpp b/src/zm_packetqueue.cpp index 20bb701ea..290dda7b3 100644 --- a/src/zm_packetqueue.cpp +++ b/src/zm_packetqueue.cpp @@ -259,7 +259,8 @@ bool PacketQueue::clearPackets(const std::shared_ptr &add_packet) { ) ) { Debug(3, "stream index %d ?= video_stream_id %d, keyframe %d, keep_keyframes %d, pending %d, counts %d > pre_event_count %d at begin %d", - add_packet->packet->stream_index, video_stream_id, add_packet->keyframe, keep_keyframes, clear_packets_pending_, + add_packet->packet->stream_index, video_stream_id, add_packet->keyframe, keep_keyframes, + clear_packets_pending_.load(std::memory_order_relaxed), packet_counts[video_stream_id], pre_event_video_packet_count, ( *(pktQueue.begin()) != add_packet ) ); @@ -348,8 +349,12 @@ bool PacketQueue::clearPackets(const std::shared_ptr &add_packet) { while (*it != add_packet) { zm_packet = *it; - if (zm_packet->queue_index >= min_iterator_queue_index) { - Debug(3, "Found iterator Counted %d video packets. Which would leave %d in packetqueue tail count is %d", + // Use > (not >=) so we still consider the iterator's own packet as a + // next_front candidate. Setting next_front to a packet that an iterator + // points at is safe: we delete strictly before next_front, so the + // iterator's packet stays in the queue. + if (zm_packet->queue_index > min_iterator_queue_index) { + Debug(3, "Past iterator. Counted %d video packets. Which would leave %d in packetqueue tail count is %d", video_packets_to_delete, packet_counts[video_stream_id]-video_packets_to_delete, tail_count); break; } diff --git a/src/zm_packetqueue.h b/src/zm_packetqueue.h index 6c149aef0..bd485b480 100644 --- a/src/zm_packetqueue.h +++ b/src/zm_packetqueue.h @@ -21,6 +21,7 @@ #include "zm_time.h" +#include #include #include #include @@ -54,7 +55,7 @@ class PacketQueue { bool has_out_of_order_packets_; int max_keyframe_interval_; int frames_since_last_keyframe_; - bool clear_packets_pending_; + std::atomic clear_packets_pending_; uint64_t next_queue_index_; Monitor *monitor_; @@ -83,6 +84,17 @@ class PacketQueue { int get_max_keyframe_interval() const { return max_keyframe_interval_; }; bool clearPackets(const std::shared_ptr &packet); + + // Lock-free gate for callers: returns false when a clearPackets() call would + // certainly early-return. When keep_keyframes is on we only have work to do + // on a keyframe (start of a droppable GOP) or when a previous attempt was + // deferred via clear_packets_pending_. A stale read of pending is harmless; + // worst case we make one extra (cheap) early-returning call. + bool should_try_clear(bool is_keyframe) const { + if (!keep_keyframes) return true; + return is_keyframe || clear_packets_pending_.load(std::memory_order_relaxed); + } + int packet_count(int stream_id); bool increment_it(packetqueue_iterator *it, bool wait); From 23060048e777bd93ae8d7ed489b4a4a1c45cee46 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 30 Apr 2026 09:09:53 -0400 Subject: [PATCH 2/2] fix: parse audioMotionAnalyzer.js as module so ESLint succeeds The file uses ES module import/export syntax but the global ESLint config sets sourceType: "script", which caused CI to fail with a parse error. Add a per-file override that parses it as a module, run eslint --fix for the now-visible whitespace/indent/semi issues, and drop two unused locals in createMotionAnalyzer that --fix could not resolve. Co-Authored-By: Claude Opus 4.7 (1M context) --- eslint.config.js | 6 + web/skins/classic/js/audioMotionAnalyzer.js | 132 ++++++++++---------- 2 files changed, 71 insertions(+), 67 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index ba5d64269..dc26d5f32 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -86,6 +86,12 @@ module.exports = defineConfig([{ "eol-last": "off", "indent": "off", }, +}, { + "files": ["web/skins/classic/js/audioMotionAnalyzer.js"], + + "languageOptions": { + sourceType: "module", + }, }, globalIgnores([ "**/*.min.js", "web/api/lib", diff --git a/web/skins/classic/js/audioMotionAnalyzer.js b/web/skins/classic/js/audioMotionAnalyzer.js index bbab8df51..c59b574d3 100644 --- a/web/skins/classic/js/audioMotionAnalyzer.js +++ b/web/skins/classic/js/audioMotionAnalyzer.js @@ -10,7 +10,7 @@ function checkAudioMotionEnabled() { } if (checkAudioMotionEnabled()) { - import('../assets/audioMotion-analyzer/src/audioMotion-analyzer.js').then(module => { + import('../assets/audioMotion-analyzer/src/audioMotion-analyzer.js').then((module) => { if (module.AudioMotionAnalyzer) { AudioMotionAnalyzer = module.AudioMotionAnalyzer; } else { @@ -35,12 +35,12 @@ export class _AudioMotionAnalyzer extends HTMLElement { } this.mid = stringToNumber(this.id); this.gainNode = null; // This is required for controlling the signal level, as we're using a separate stream. This is because when using WebRTC, we can't get the audio stream from