diff --git a/src/zm_packetqueue.cpp b/src/zm_packetqueue.cpp index 290dda7b3..145243e35 100644 --- a/src/zm_packetqueue.cpp +++ b/src/zm_packetqueue.cpp @@ -424,6 +424,12 @@ void PacketQueue::stop() { void PacketQueue::clear() { Debug(1, "Clearing packetqueue"); + // Move packets out under the lock, then let their destructors run after + // the mutex is released. Mirrors queuePacket()/clearPackets() so we don't + // stall other threads on expensive ZMPacket teardown (Image, AVPacket). + std::vector> packets_to_destroy; + + { std::lock_guard lck(mutex); deleting = true; // Why are we notifying? @@ -433,8 +439,6 @@ void PacketQueue::clear() { while (!pktQueue.empty()) { std::shared_ptr packet = pktQueue.front(); - // Someone might have this packet, but not for very long and since we have locked the queue they won't be able to get another one - // Deleting this packet, doesn't require a lock. We only need a lock if we are modifying the packet. Debug(1, "Deleting a packet with stream index:%d image_index:%d with keyframe:%d, video frames in queue:%d max: %d, queuesize:%zu", packet->packet->stream_index, @@ -445,6 +449,7 @@ void PacketQueue::clear() { pktQueue.size()); packet_counts[packet->packet->stream_index] -= 1; pktQueue.pop_front(); + packets_to_destroy.push_back(std::move(packet)); } Debug(1, "Packetqueue is clear, deleting iterators"); @@ -463,6 +468,8 @@ void PacketQueue::clear() { Debug(1, "Packetqueue is clear, notifying"); condition.notify_all(); + } // end scope for lock_guard — mutex released here + // packets_to_destroy goes out of scope here, destroying packets without holding the mutex } // end void PacketQueue::clear() unsigned int PacketQueue::size() { @@ -585,11 +592,6 @@ ZMPacketLock PacketQueue::get_packet_and_increment_it(packetqueue_iterator *it) return ZMPacketLock(); } // end ZMPacketLock *PacketQueue::get_packet_and_increment_it(it) -void PacketQueue::unlock(ZMPacketLock *lp) { - delete lp; - condition.notify_all(); -} - bool PacketQueue::increment_it(packetqueue_iterator *it, bool wait) { std::unique_lock lck(mutex); Debug(2, "Incrementing %p, queue size %zu, end? %d, deleting %d", it, pktQueue.size(), ((*it) == pktQueue.end()), deleting); diff --git a/src/zm_packetqueue.h b/src/zm_packetqueue.h index bd485b480..cbd945f66 100644 --- a/src/zm_packetqueue.h +++ b/src/zm_packetqueue.h @@ -28,7 +28,6 @@ #include #include -class Monitor; class ZMPacket; class ZMPacketLock; @@ -37,10 +36,9 @@ typedef std::list>::iterator packetqueue_iterator; class PacketQueue { private: // For now just to ease development std::list> pktQueue; - std::list>::iterator analysis_it; int video_stream_id; - int max_video_packet_count; // allow a negative value to someday mean unlimited + int max_video_packet_count; // 0 means unlimited // This is now a hard limit on the # of video packets to keep in the queue so that we can limit ram int pre_event_video_packet_count; // Was max_video_packet_count int max_stream_id; @@ -57,7 +55,6 @@ class PacketQueue { int frames_since_last_keyframe_; std::atomic clear_packets_pending_; uint64_t next_queue_index_; - Monitor *monitor_; public: PacketQueue(); @@ -69,7 +66,6 @@ class PacketQueue { void setMaxVideoPackets(int p); void setPreEventVideoPackets(int p); void setKeepKeyframes(bool k) { keep_keyframes = k; }; - void setMonitor(Monitor *m) { monitor_ = m; }; bool queuePacket(std::shared_ptr packet); void stop(); @@ -103,7 +99,6 @@ class PacketQueue { ZMPacketLock get_packet_no_wait(packetqueue_iterator *); ZMPacketLock get_packet_and_increment_it(packetqueue_iterator *); packetqueue_iterator *get_video_it(bool wait); - packetqueue_iterator *get_stream_it(int stream_id); void free_it(packetqueue_iterator *); packetqueue_iterator *get_event_start_packet_it( @@ -111,7 +106,6 @@ class PacketQueue { unsigned int pre_event_count ); bool is_there_an_iterator_pointing_to_packet(const std::shared_ptr zm_packet); - void unlock(ZMPacketLock *lp); void notify_all(); void wait(); void wait_for(Microseconds duration);