clear() was destroying ZMPackets while holding the queue mutex, unlike
queuePacket() and clearPackets() which defer destruction to a stack
vector that goes out of scope after the lock is released. ZMPacket
teardown frees Image, AVFrame, and AVPacket data and is expensive,
especially on PASSTHROUGH queues that may hold hundreds of MB. Doing it
under the mutex risked stalling capture and analysis threads during
shutdown or monitor reconfigure. Adopt the same deferred-destroy
pattern.
Also remove dead code identified in the audit:
- PacketQueue::unlock(ZMPacketLock*): defined but no callers; would
UAF if anyone passed a non-heap pointer.
- monitor_ field and setMonitor(): set never read, no callers.
- analysis_it field: declared, never used (Monitor::analysis_it is
the live one).
- get_stream_it(int) declaration: never defined.
- max_video_packet_count comment: clarify 0 means unlimited (the
setter clamps negatives to 0 and the limit check is > 0).
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<bool> 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) <noreply@anthropic.com>
Remove per-packet ZMPacketLock trylock() from clearPackets() scan loops
and queuePacket() GOP deletion — the iterator check is sufficient since
threads only access packets through their own shared_ptr.
Add a monotonic uint64_t queue_index to ZMPacket, assigned on enqueue.
clearPackets() now finds the earliest iterator position with a single
min() over the 2-3 iterators, then uses one integer comparison per
scanned packet instead of walking all iterators per packet.
Defer packet destruction outside the mutex in both clearPackets() and
queuePacket() by collecting removed shared_ptrs into a local vector
and releasing the lock before they are destroyed.
Raise per-packet deletion Debug(1) to Debug(4) in both paths.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Event::Run could block indefinitely in PacketQueue methods during normal
event closing (closeEvent from analysis thread), because their wait
predicates only check deleting/zm_terminate, not Event's terminate_ flag.
Three changes fix this:
- get_packet_no_wait: return immediately when iterator at end instead of
blocking on condition variable (makes it truly non-blocking)
- Event::Run: use increment_it(wait=false) since deletePacket can advance
the iterator to end() during AddPacket_ without the queue lock
- Event::Stop: call packetqueue->notify_all() to wake timed waits so
Run() checks terminate_ promptly
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a uint64_t queue_index field to ZMPacket, assigned by PacketQueue
on enqueue via a monotonic counter. This lets clearPackets() find the
earliest iterator-pointed packet with a single min() over the 2-3
iterators, then check each scanned packet with one integer comparison
instead of searching a vector of shared_ptrs.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove per-packet ZMPacketLock trylock() from clearPackets() scan loops
and queuePacket() GOP deletion — the iterator check is sufficient since
threads only access packets through their own shared_ptr after obtaining
it from the queue.
Pre-compute the set of iterator-pointed packets once before scanning
instead of calling is_there_an_iterator_pointing_to_packet() per packet,
reducing O(packets * iterators) to O(iterators) for the lookup setup.
Batch packet destruction outside the mutex by collecting removed
shared_ptrs into a local vector and releasing the lock before they are
destroyed, so expensive ZMPacket destructors don't block queuePacket()
and get_packet_and_increment_it().
Raise per-packet deletion Debug(1) to Debug(4) to cut string formatting
on the hot path.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The backward walk loop exits when the iterator reaches pktQueue.begin()
without counting that packet as a video frame. This off-by-one causes
the "Hit end of packetqueue before satisfying pre_event_count" warning
even when the queue has enough packets. Check the begin packet after
the loop exits.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Monitor::Decode():
- Reorganize into 5 clear phases with descriptive comments
- Phase 1: Receive decoded frame from decoder
- Phase 2: Get and send new packet to decoder
- Phase 3: Convert decoded frame to Image
- Phase 4: Prepare Y-channel for analysis
- Phase 5: Process RGB image (deinterlace, rotate, privacy, timestamp)
- Extract applyOrientation() and applyDeinterlacing() helper functions
- Keep slow send_packet detection timing for diagnostics
PacketQueue locking fixes:
- Move lock acquisition before accessing shared state in queuePacket()
- Keep lock held while iterating in stop()
- Add lock to addStream()
- Remove duplicate packet_counts allocation in clear()
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Isaac Connor