From b238a2bc25389fd826bbd295e0a817cda2047e10 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 15 Mar 2021 15:08:43 -0400 Subject: [PATCH 01/72] Implement decoder thread, locking is broken --- src/CMakeLists.txt | 1 + src/zm_monitor.cpp | 177 +++++++++++++++++++++++++++++++---------- src/zm_monitor.h | 4 + src/zm_packet.cpp | 4 +- src/zm_packet.h | 49 +++++++++--- src/zm_packetqueue.cpp | 12 +-- 6 files changed, 185 insertions(+), 62 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c6a9c04c7..37b08ad58 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -16,6 +16,7 @@ set(ZM_BIN_SRC_FILES zm_crypt.cpp zm.cpp zm_db.cpp + zm_decoder_thread.cpp zm_logger.cpp zm_event.cpp zm_eventstream.cpp diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 88028ca32..73822502e 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -395,6 +395,8 @@ Monitor::Monitor() storage(nullptr), videoStore(nullptr), analysis_it(nullptr), + decoder_it(nullptr), + decoder(nullptr), n_zones(0), zones(nullptr), privacy_bitmask(nullptr), @@ -1802,21 +1804,20 @@ void Monitor::UpdateAnalysisFPS() { // If there isn't then we keep pre-event + alarm frames. = pre_event_count bool Monitor::Analyse() { - if ( !Enabled() ) { + if (!Enabled()) { Warning("Shouldn't be doing Analyse when not Enabled"); return false; } - if ( !analysis_it ) - analysis_it = packetqueue.get_video_it(true); + if (!analysis_it) analysis_it = packetqueue.get_video_it(true); // if have event, send frames until we find a video packet, at which point do analysis. Adaptive skip should only affect which frames we do analysis on. // get_analysis_packet will lock the packet and may wait if analysis_it is at the end ZMPacket *snap = packetqueue.get_packet(analysis_it); - if ( !snap ) return false; + if (!snap) return false; // Is it possible for snap->score to be ! -1 ? Not if everything is working correctly - if ( snap->score != -1 ) { + if (snap->score != -1) { snap->unlock(); packetqueue.increment_it(analysis_it); Error("skipping because score was %d", snap->score); @@ -1837,25 +1838,24 @@ bool Monitor::Analyse() { std::lock_guard lck(event_mutex); // if we have been told to be OFF, then we are off and don't do any processing. - if ( trigger_data->trigger_state != TriggerState::TRIGGER_OFF ) { + if (trigger_data->trigger_state != TriggerState::TRIGGER_OFF) { Debug(4, "Trigger not OFF state is (%d)", int(trigger_data->trigger_state)); int score = 0; // Ready means that we have captured the warmup # of frames - if ( !Ready() ) { + if (!Ready()) { Debug(3, "Not ready?"); snap->unlock(); return false; } - Debug(4, "Ready"); std::string cause; Event::StringSetMap noteSetMap; // Specifically told to be on. Setting the score here will trigger the alarm. - if ( trigger_data->trigger_state == TriggerState::TRIGGER_ON ) { + if (trigger_data->trigger_state == TriggerState::TRIGGER_ON) { score += trigger_data->trigger_score; Debug(1, "Triggered on score += %d => %d", trigger_data->trigger_score, score); - if ( !event ) { + if (!event) { cause += trigger_data->trigger_cause; } Event::StringSet noteSet; @@ -1863,12 +1863,13 @@ bool Monitor::Analyse() { noteSetMap[trigger_data->trigger_cause] = noteSet; } // end if trigger_on - if ( signal_change ) { + // FIXME this snap might not be the one that caused the signal change. Need to store that in the packet. + if (signal_change) { Debug(2, "Signal change, new signal is %d", signal); const char *signalText = "Unknown"; - if ( !signal ) { + if (!signal) { signalText = "Lost"; - if ( event ) { + if (event) { Info("%s: %03d - Closing event %" PRIu64 ", signal loss", name, analysis_image_count, event->Id()); closeEvent(); last_section_mod = 0; @@ -1877,9 +1878,8 @@ bool Monitor::Analyse() { signalText = "Reacquired"; score += 100; } - if ( !event ) { - if ( cause.length() ) - cause += ", "; + if (!event) { + if (cause.length()) cause += ", "; cause += SIGNAL_CAUSE; } Event::StringSet noteSet; @@ -1887,15 +1887,26 @@ bool Monitor::Analyse() { noteSetMap[SIGNAL_CAUSE] = noteSet; shared_data->state = state = IDLE; shared_data->active = signal; - if ( (function == MODECT or function == MOCORD) and snap->image ) + if ((function == MODECT or function == MOCORD) and snap->image) ref_image.Assign(*(snap->image)); }// else if (signal) { - if (snap->image or (snap->codec_type == AVMEDIA_TYPE_VIDEO)) { + if (snap->codec_type == AVMEDIA_TYPE_VIDEO) { + while (!snap->image and !snap->decoded) { + // Need to wait for the decoder thread. + Debug(1, "Waiting for decode"); + snap->wait(); + if (!snap->image and snap->decoded) { + Debug(1, "No image but was decoded, giving up"); + snap->unlock(); + return false; + } + } // end while ! decoded + struct timeval *timestamp = snap->timestamp; - if ( Active() and (function == MODECT or function == MOCORD) and snap->image ) { + if (Active() and (function == MODECT or function == MOCORD)) { Debug(3, "signal and active and modect"); Event::StringSet zoneSet; @@ -1999,7 +2010,7 @@ bool Monitor::Analyse() { ); // This gets a lock on the starting packet - ZMPacket *starting_packet = packetqueue.get_packet(start_it); + ZMPacket *starting_packet = ( *start_it == snap_it ) ? snap : packetqueue.get_packet(start_it); event = new Event(this, *(starting_packet->timestamp), "Continuous", noteSetMap); // Write out starting packets, do not modify packetqueue it will garbage collect itself @@ -2030,9 +2041,7 @@ bool Monitor::Analyse() { for ( int i=0; i < n_zones; i++ ) { if ( zones[i]->Alarmed() ) { alarm_cause += std::string(zones[i]->Label()); - if ( i < n_zones-1 ) { - alarm_cause += ","; - } + if (i < n_zones-1) alarm_cause += ","; } } alarm_cause = cause+" "+alarm_cause; @@ -2085,7 +2094,7 @@ bool Monitor::Analyse() { snap_it, (pre_event_count > alarm_frame_count ? pre_event_count : alarm_frame_count) ); - ZMPacket *starting_packet = *(*start_it); + ZMPacket *starting_packet = ( *start_it == snap_it ) ? snap : packetqueue.get_packet(start_it); event = new Event(this, *(starting_packet->timestamp), cause, noteSetMap); shared_data->last_event_id = event->Id(); @@ -2255,24 +2264,6 @@ bool Monitor::Analyse() { } // end if ( trigger_data->trigger_state != TRIGGER_OFF ) if (event) event->AddPacket(snap); -#if 0 - if (snap->packet.stream_index == video_stream_id) { - if (video_fifo) { - if ( snap->keyframe ) { - // avcodec strips out important nals that describe the stream and - // stick them in extradata. Need to send them along with keyframes - AVStream *stream = camera->getVideoStream(); - video_fifo->write( - static_cast(stream->codecpar->extradata), - stream->codecpar->extradata_size); - } - video_fifo->writePacket(*snap); - } - } else if (snap->packet.stream_index == audio_stream_id) { - if (audio_fifo) - audio_fifo->writePacket(*snap); - } -#endif // popPacket will have placed a second lock on snap, so release it here. snap->unlock(); @@ -2507,7 +2498,6 @@ int Monitor::Capture() { gettimeofday(packet->timestamp, nullptr); shared_data->zmc_heartbeat_time = packet->timestamp->tv_sec; - Image* capture_image = image_buffer[index].image; int captureResult = 0; if ( deinterlacing_value == 4 ) { @@ -2536,7 +2526,7 @@ int Monitor::Capture() { Rgb signalcolor; /* HTML colour code is actually BGR in memory, we want RGB */ signalcolor = rgb_convert(signal_check_colour, ZM_SUBPIX_ORDER_BGR); - capture_image = new Image(width, height, camera->Colours(), camera->SubpixelOrder()); + Image *capture_image = new Image(width, height, camera->Colours(), camera->SubpixelOrder()); capture_image->Fill(signalcolor); shared_data->signal = false; shared_data->last_write_index = index; @@ -2592,6 +2582,7 @@ int Monitor::Capture() { return 1; } // end if audio +#if 0 if ( !packet->image ) { if ( packet->packet.size and !packet->in_frame ) { if ( !decoding_enabled ) { @@ -2678,6 +2669,7 @@ int Monitor::Capture() { shared_data->signal = ( capture_image and signal_check_points ) ? CheckSignal(capture_image) : true; shared_data->last_write_index = index; shared_data->last_write_time = packet->timestamp->tv_sec; +#endif image_count++; // Will only be queued if there are iterators allocated in the queue. @@ -2710,6 +2702,100 @@ int Monitor::Capture() { return captureResult; } // end Monitor::Capture +bool Monitor::Decode() { + if (!decoder_it) decoder_it = packetqueue.get_video_it(true); + + ZMPacket *packet = packetqueue.get_packet(decoder_it); + if (!packet) return false; + packetqueue.increment_it(decoder_it); + if (packet->image or (packet->codec_type != AVMEDIA_TYPE_VIDEO)) { + packet->unlock(); + return true; // Don't need decode + } + + int ret = 0; + if (packet->packet.size and !packet->in_frame) { + // Allocate the image first so that it can be used by hwaccel + // We don't actually care about camera colours, pixel order etc. We care about the desired settings + // + //capture_image = packet->image = new Image(width, height, camera->Colours(), camera->SubpixelOrder()); + ret = packet->decode(camera->getVideoCodecContext()); + } else { + Debug(1, "No packet.size(%d) or packet->in_frame(%p). Not decoding", packet->packet.size, packet->in_frame); + } + Image* capture_image = nullptr; + unsigned int index = image_count % image_buffer_count; + if (ret > 0) { + if (packet->in_frame and !packet->image) { + packet->image = new Image(camera_width, camera_height, camera->Colours(), camera->SubpixelOrder()); + packet->get_image(); + } + + if (packet->image) { + capture_image = packet->image; + + /* Deinterlacing */ + if ( deinterlacing_value ) { + if ( deinterlacing_value == 1 ) { + capture_image->Deinterlace_Discard(); + } else if ( deinterlacing_value == 2 ) { + capture_image->Deinterlace_Linear(); + } else if ( deinterlacing_value == 3 ) { + capture_image->Deinterlace_Blend(); + } else if ( deinterlacing_value == 4 ) { + capture_image->Deinterlace_4Field(next_buffer.image, (deinterlacing>>8)&0xff); + } else if ( deinterlacing_value == 5 ) { + capture_image->Deinterlace_Blend_CustomRatio((deinterlacing>>8)&0xff); + } + } + + if ( orientation != ROTATE_0 ) { + Debug(2, "Doing rotation"); + switch ( orientation ) { + case ROTATE_0 : + // No action required + break; + case ROTATE_90 : + case ROTATE_180 : + case ROTATE_270 : + capture_image->Rotate((orientation-1)*90); + break; + case FLIP_HORI : + case FLIP_VERT : + capture_image->Flip(orientation==FLIP_HORI); + break; + } + } // end if have rotation + + if (privacy_bitmask) { + Debug(1, "Applying privacy"); + capture_image->MaskPrivacy(privacy_bitmask); + } + + if (config.timestamp_on_capture) { + Debug(1, "Timestampprivacy"); + TimestampImage(packet->image, packet->timestamp); + } + + if (!ref_image.Buffer()) { + // First image, so assign it to ref image + Debug(1, "Assigning ref image %dx%d size: %d", width, height, camera->ImageSize()); + ref_image.Assign(width, height, camera->Colours(), camera->SubpixelOrder(), + packet->image->Buffer(), camera->ImageSize()); + } + image_buffer[index].image->Assign(*(packet->image)); + *(image_buffer[index].timestamp) = *(packet->timestamp); + } // end if have image + } // end if did decoding + packet->decoded = true; + packet->unlock(); // unlock will also signal + + shared_data->signal = ( capture_image and signal_check_points ) ? CheckSignal(capture_image) : true; + shared_data->last_write_index = index; + shared_data->last_write_time = packet->timestamp->tv_sec; + return true; +} // end bool Monitor::Decode() + void Monitor::TimestampImage(Image *ts_image, const struct timeval *ts_time) const { if ( !label_format[0] ) return; @@ -3026,6 +3112,7 @@ int Monitor::PrimeCapture() { audio_fifo = new Fifo(shared_data->audio_fifo_path, true); } } // end if rtsp_server + decoder = new DecoderThread(this); } else { Debug(2, "Failed to prime %d", ret); } @@ -3035,6 +3122,8 @@ int Monitor::PrimeCapture() { int Monitor::PreCapture() const { return camera->PreCapture(); } int Monitor::PostCapture() const { return camera->PostCapture(); } int Monitor::Close() { + decoder->Stop(); + delete decoder; std::lock_guard lck(event_mutex); if (event) { Info("%s: image_count:%d - Closing event %" PRIu64 ", shutting down", name, image_count, event->Id()); diff --git a/src/zm_monitor.h b/src/zm_monitor.h index 20a59d79c..91d12dd24 100644 --- a/src/zm_monitor.h +++ b/src/zm_monitor.h @@ -22,6 +22,7 @@ #include "zm_define.h" #include "zm_camera.h" +#include "zm_decoder_thread.h" #include "zm_event.h" #include "zm_fifo.h" #include "zm_image.h" @@ -375,6 +376,8 @@ protected: VideoStore *videoStore; PacketQueue packetqueue; packetqueue_iterator *analysis_it; + packetqueue_iterator *decoder_it; + DecoderThread *decoder; int n_zones; @@ -549,6 +552,7 @@ public: //unsigned int DetectBlack( const Image &comp_image, Event::StringSet &zoneSet ); bool CheckSignal( const Image *image ); bool Analyse(); + bool Decode(); void DumpImage( Image *dump_image ) const; void TimestampImage( Image *ts_image, const struct timeval *ts_time ) const; bool closeEvent(); diff --git a/src/zm_packet.cpp b/src/zm_packet.cpp index 58c2bda8c..3a865fcc3 100644 --- a/src/zm_packet.cpp +++ b/src/zm_packet.cpp @@ -27,6 +27,7 @@ using namespace std; AVPixelFormat target_format = AV_PIX_FMT_NONE; ZMPacket::ZMPacket() : + lck(mutex,std::defer_lock), keyframe(0), in_frame(nullptr), out_frame(nullptr), @@ -37,7 +38,8 @@ ZMPacket::ZMPacket() : score(-1), codec_type(AVMEDIA_TYPE_UNKNOWN), image_index(-1), - codec_imgsize(0) + codec_imgsize(0), + decoded(0) { av_init_packet(&packet); packet.size = 0; // So we can detect whether it has been filled. diff --git a/src/zm_packet.h b/src/zm_packet.h index 118239860..f4032bf80 100644 --- a/src/zm_packet.h +++ b/src/zm_packet.h @@ -21,6 +21,7 @@ #define ZM_PACKET_H #include "zm_logger.h" +#include #include extern "C" { @@ -36,7 +37,7 @@ class Image; class ZMPacket { public: - std::recursive_mutex mutex; + int keyframe; AVStream *stream; // Input stream AVPacket packet; // Input packet, undecoded @@ -51,6 +52,7 @@ class ZMPacket { int image_index; int codec_imgsize; int64_t pts; // pts in the packet can be in another time base. This MUST be in AV_TIME_BASE_Q + bool decoded; public: AVPacket *av_packet() { return &packet; } @@ -65,21 +67,44 @@ class ZMPacket { explicit ZMPacket(ZMPacket &packet); ZMPacket(); ~ZMPacket(); - void lock() { - Debug(4,"Locking packet %d", this->image_index); - mutex.lock(); - Debug(4,"packet %d locked", this->image_index); + + std::unique_lock * lock() { + std::unique_lock *lck = new std::unique_lock(mutex); + Debug(4, "packet %d locked", this->image_index); + return lck; }; - bool trylock() { - Debug(4,"TryLocking packet %d", this->image_index); - return mutex.try_lock(); + std::unique_lock * trylock() { + std::unique_lock *lck = new std::unique_lock(mutex, std::defer_lock); + Debug(4, "TryLocking packet %d", this->image_index); + if ( lck.try_lock() ) + return lck; + delete lck; + return nullptr; }; - void unlock() { - Debug(4,"packet %d unlocked", this->image_index); - mutex.unlock(); + void unlock(std::unique_lock *lck) { + Debug(4, "packet %d unlocked", this->image_index); + lck->unlock(); + condition.notify_all(); }; - AVFrame *get_out_frame( const AVCodecContext *ctx ); + void wait(std::unique_lock *lck) { + Debug(4, "packet %d waiting", this->image_index); + // We already have a lock, but it's a recursive mutex.. so this may be ok + condition.wait(*lck); + } + AVFrame *get_out_frame(const AVCodecContext *ctx); int get_codec_imgsize() { return codec_imgsize; }; }; +class ZMLockedPacket : public ZMPacket { + public: + std::mutex mutex_; + std::condition_variable condition_; + std::unique_lock lck_; + ZMPacket *packet_; + + ZMLockedPacket(ZMPacket *p) : packet_(packet), lck_(mutex_) { + } + +} + #endif /* ZM_PACKET_H */ diff --git a/src/zm_packetqueue.cpp b/src/zm_packetqueue.cpp index 2ba04e71b..5f34b1f0d 100644 --- a/src/zm_packetqueue.cpp +++ b/src/zm_packetqueue.cpp @@ -478,8 +478,10 @@ ZMPacket *PacketQueue::get_packet(packetqueue_iterator *it) { return nullptr; } Debug(3, "get_packet %p image_index: %d, about to lock packet", p, p->image_index); - while ( !(zm_terminate or deleting) and !p->trylock() ) { - Debug(3, "waiting. Queue size %d it == end? %d", pktQueue.size(), ( *it == pktQueue.end() ) ); + while (!(zm_terminate or deleting) and !p->trylock()) { + Debug(3, "waiting on index %d. Queue size %d it == end? %d", + p->image_index, pktQueue.size(), ( *it == pktQueue.end() ) ); + ZM_DUMP_PACKET(p->packet, ""); condition.wait(lck); } Debug(2, "Locked packet, unlocking packetqueue mutex"); @@ -612,14 +614,14 @@ packetqueue_iterator * PacketQueue::get_video_it(bool wait) { while ( *it != pktQueue.end() ) { ZMPacket *zm_packet = *(*it); - if ( !zm_packet ) { + if (!zm_packet) { Error("Null zmpacket in queue!?"); free_it(it); return nullptr; } Debug(1, "Packet keyframe %d for stream %d, so returning the it to it", zm_packet->keyframe, zm_packet->packet.stream_index); - if ( zm_packet->keyframe and ( zm_packet->packet.stream_index == video_stream_id ) ) { + if (zm_packet->keyframe and ( zm_packet->packet.stream_index == video_stream_id )) { Debug(1, "Found a keyframe for stream %d, so returning the it to it", video_stream_id); return it; } @@ -627,7 +629,7 @@ packetqueue_iterator * PacketQueue::get_video_it(bool wait) { } Debug(1, "DIdn't Found a keyframe for stream %d, so returning the it to it", video_stream_id); return it; -} +} // get video_it void PacketQueue::free_it(packetqueue_iterator *it) { for ( From 6a11b23aafc401fb336257599ab99ccccec5a61a Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 15 Mar 2021 15:08:59 -0400 Subject: [PATCH 02/72] Add decoder thread --- src/zm_decoder_thread.cpp | 54 +++++++++++++++++++++++++++++++++++++++ src/zm_decoder_thread.h | 29 +++++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 src/zm_decoder_thread.cpp create mode 100644 src/zm_decoder_thread.h diff --git a/src/zm_decoder_thread.cpp b/src/zm_decoder_thread.cpp new file mode 100644 index 000000000..e8023e905 --- /dev/null +++ b/src/zm_decoder_thread.cpp @@ -0,0 +1,54 @@ +#include "zm_decoder_thread.h" + +#include "zm_monitor.h" +#include "zm_signal.h" +#include "zm_utils.h" + +//DecoderThread::DecoderThread(std::shared_ptr monitor) : +DecoderThread::DecoderThread(Monitor * monitor) : + monitor_(monitor), terminate_(false) { + //monitor_(std::move(monitor)), terminate_(false) { + thread_ = std::thread(&DecoderThread::Run, this); +} + +DecoderThread::~DecoderThread() { + Stop(); + if (thread_.joinable()) + thread_.join(); +} + +void DecoderThread::Run() { + Debug(2, "DecoderThread::Run() for %d", monitor_->Id()); + + //Microseconds decoder_rate = Microseconds(monitor_->GetDecoderRate()); + //Seconds decoder_update_delay = Seconds(monitor_->GetDecoderUpdateDelay()); + //Debug(2, "DecoderThread::Run() have update delay %d", decoder_update_delay); + + //TimePoint last_decoder_update_time = std::chrono::steady_clock::now(); + //TimePoint cur_time; + + while (!(terminate_ or zm_terminate)) { + // Some periodic updates are required for variable capturing framerate + //if (decoder_update_delay != Seconds::zero()) { + //cur_time = std::chrono::steady_clock::now(); + //Debug(2, "Updating adaptive skip"); + //if ((cur_time - last_decoder_update_time) > decoder_update_delay) { + //decoder_rate = Microseconds(monitor_->GetDecoderRate()); + //last_decoder_update_time = cur_time; + //} + //} + + if (!monitor_->Decode()) { + //if ( !(terminate_ or zm_terminate) ) { + //Microseconds sleep_for = monitor_->Active() ? Microseconds(ZM_SAMPLE_RATE) : Microseconds(ZM_SUSPENDED_RATE); + //Debug(2, "Sleeping for %" PRId64 "us", int64(sleep_for.count())); + //std::this_thread::sleep_for(sleep_for); + //} + //} else if (decoder_rate != Microseconds::zero()) { + //Debug(2, "Sleeping for %" PRId64 " us", int64(decoder_rate.count())); + //std::this_thread::sleep_for(decoder_rate); + //} else { + //Debug(2, "Not sleeping"); + } + } +} diff --git a/src/zm_decoder_thread.h b/src/zm_decoder_thread.h new file mode 100644 index 000000000..703a7e1e1 --- /dev/null +++ b/src/zm_decoder_thread.h @@ -0,0 +1,29 @@ +#ifndef ZM_DECODER_THREAD_H +#define ZM_DECODER_THREAD_H + +#include +#include +#include + +class Monitor; + +class DecoderThread { + public: + explicit DecoderThread(Monitor* monitor); + //explicit DecoderThread(std::shared_ptr monitor); + ~DecoderThread(); + DecoderThread(DecoderThread &rhs) = delete; + DecoderThread(DecoderThread &&rhs) = delete; + + void Stop() { terminate_ = true; } + + private: + void Run(); + + Monitor* monitor_; + //std::shared_ptr monitor_; + std::atomic terminate_; + std::thread thread_; +}; + +#endif From baf73fea7be90a307925ee479e253d51d47b40f1 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 15 Mar 2021 15:11:12 -0400 Subject: [PATCH 03/72] Ensure that we disconnect when ShmValid fails --- src/zm_rtsp_server.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/zm_rtsp_server.cpp b/src/zm_rtsp_server.cpp index 0fb5a8d15..7fe8e916a 100644 --- a/src/zm_rtsp_server.cpp +++ b/src/zm_rtsp_server.cpp @@ -187,14 +187,16 @@ int main(int argc, char *argv[]) { for (size_t i = 0; i < monitors.size(); i++) { std::shared_ptr monitor = monitors[i]; - if (!(monitor->ShmValid() or monitor->connect())) { - Warning("Couldn't connect to monitor %d", monitor->Id()); - if (sessions[i]) { - rtspServer->RemoveSession(sessions[i]->GetMediaSessionId()); - sessions[i] = nullptr; - } - monitor->disconnect(); - continue; + if (!monitor->ShmValid()) { + monitor->disconnect(); + if (!monitor->connect()) { + Warning("Couldn't connect to monitor %d", monitor->Id()); + if (sessions[i]) { + rtspServer->RemoveSession(sessions[i]->GetMediaSessionId()); + sessions[i] = nullptr; + } + continue; + } } if (!sessions[i]) { From 9903e909af93b628c558c8ec34de5fb0e9790236 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 15 Mar 2021 17:05:30 -0400 Subject: [PATCH 04/72] Rework locking in ZMPacket by using a new class called ZMLockedPacket. --- src/zm_monitor.cpp | 83 +++++++++++++++++++++++++++--------------- src/zm_packet.cpp | 1 - src/zm_packet.h | 61 ++++++++++++++++--------------- src/zm_packetqueue.cpp | 30 ++++++++------- src/zm_packetqueue.h | 5 ++- src/zm_videostore.cpp | 23 ------------ 6 files changed, 105 insertions(+), 98 deletions(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 73822502e..0ea6b5fb6 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -1813,12 +1813,13 @@ bool Monitor::Analyse() { // if have event, send frames until we find a video packet, at which point do analysis. Adaptive skip should only affect which frames we do analysis on. // get_analysis_packet will lock the packet and may wait if analysis_it is at the end - ZMPacket *snap = packetqueue.get_packet(analysis_it); - if (!snap) return false; + ZMLockedPacket *packet_lock = packetqueue.get_packet(analysis_it); + if (!packet_lock) return false; + ZMPacket *snap = packet_lock->packet_; // Is it possible for snap->score to be ! -1 ? Not if everything is working correctly if (snap->score != -1) { - snap->unlock(); + delete packet_lock; packetqueue.increment_it(analysis_it); Error("skipping because score was %d", snap->score); return false; @@ -1844,7 +1845,7 @@ bool Monitor::Analyse() { // Ready means that we have captured the warmup # of frames if (!Ready()) { Debug(3, "Not ready?"); - snap->unlock(); + delete packet_lock; return false; } @@ -1896,10 +1897,10 @@ bool Monitor::Analyse() { while (!snap->image and !snap->decoded) { // Need to wait for the decoder thread. Debug(1, "Waiting for decode"); - snap->wait(); + packet_lock->wait(); if (!snap->image and snap->decoded) { Debug(1, "No image but was decoded, giving up"); - snap->unlock(); + delete packet_lock; return false; } } // end while ! decoded @@ -2010,22 +2011,32 @@ bool Monitor::Analyse() { ); // This gets a lock on the starting packet - ZMPacket *starting_packet = ( *start_it == snap_it ) ? snap : packetqueue.get_packet(start_it); + + ZMLockedPacket *starting_packet_lock = nullptr; + ZMPacket *starting_packet = nullptr; + if ( *start_it != snap_it ) { + starting_packet_lock = packetqueue.get_packet(start_it); + if (!starting_packet_lock) return false; + starting_packet = starting_packet_lock->packet_; + } else { + starting_packet = snap; + } event = new Event(this, *(starting_packet->timestamp), "Continuous", noteSetMap); // Write out starting packets, do not modify packetqueue it will garbage collect itself - while ( starting_packet and (*start_it) != snap_it ) { + while ( starting_packet and ((*start_it) != snap_it) ) { event->AddPacket(starting_packet); // Have added the packet, don't want to unlock it until we have locked the next packetqueue.increment_it(start_it); if ( (*start_it) == snap_it ) { - starting_packet->unlock(); + if (starting_packet_lock) delete starting_packet_lock; break; } - ZMPacket *p = packetqueue.get_packet(start_it); - starting_packet->unlock(); - starting_packet = p; + ZMLockedPacket *lp = packetqueue.get_packet(start_it); + delete starting_packet_lock; + starting_packet_lock = lp; + starting_packet = lp->packet_; } packetqueue.free_it(start_it); delete start_it; @@ -2094,7 +2105,16 @@ bool Monitor::Analyse() { snap_it, (pre_event_count > alarm_frame_count ? pre_event_count : alarm_frame_count) ); - ZMPacket *starting_packet = ( *start_it == snap_it ) ? snap : packetqueue.get_packet(start_it); + + ZMLockedPacket *starting_packet_lock = nullptr; + ZMPacket *starting_packet = nullptr; + if ( *start_it != snap_it ) { + starting_packet_lock = packetqueue.get_packet(start_it); + if (!starting_packet_lock) return false; + starting_packet = starting_packet_lock->packet_; + } else { + starting_packet = snap; + } event = new Event(this, *(starting_packet->timestamp), cause, noteSetMap); shared_data->last_event_id = event->Id(); @@ -2108,12 +2128,13 @@ bool Monitor::Analyse() { packetqueue.increment_it(start_it); if ( (*start_it) == snap_it ) { - starting_packet->unlock(); + if (starting_packet_lock) delete starting_packet_lock; break; } - ZMPacket *p = packetqueue.get_packet(start_it); - starting_packet->unlock(); - starting_packet = p; + ZMLockedPacket *lp = packetqueue.get_packet(start_it); + delete starting_packet_lock; + starting_packet_lock = lp; + starting_packet = lp->packet_; } packetqueue.free_it(start_it); delete start_it; @@ -2266,7 +2287,7 @@ bool Monitor::Analyse() { if (event) event->AddPacket(snap); // popPacket will have placed a second lock on snap, so release it here. - snap->unlock(); + delete packet_lock; if ( snap->image_index > 0 ) { // Only do these if it's a video packet. @@ -2705,11 +2726,12 @@ int Monitor::Capture() { bool Monitor::Decode() { if (!decoder_it) decoder_it = packetqueue.get_video_it(true); - ZMPacket *packet = packetqueue.get_packet(decoder_it); - if (!packet) return false; + ZMLockedPacket *packet_lock = packetqueue.get_packet(decoder_it); + if (!packet_lock) return false; + ZMPacket *packet = packet_lock->packet_; packetqueue.increment_it(decoder_it); if (packet->image or (packet->codec_type != AVMEDIA_TYPE_VIDEO)) { - packet->unlock(); + delete packet_lock; return true; // Don't need decode } @@ -2788,7 +2810,7 @@ bool Monitor::Decode() { } // end if have image } // end if did decoding packet->decoded = true; - packet->unlock(); // unlock will also signal + delete packet_lock; shared_data->signal = ( capture_image and signal_check_points ) ? CheckSignal(capture_image) : true; shared_data->last_write_index = index; @@ -3112,7 +3134,7 @@ int Monitor::PrimeCapture() { audio_fifo = new Fifo(shared_data->audio_fifo_path, true); } } // end if rtsp_server - decoder = new DecoderThread(this); + if (decoding_enabled) decoder = new DecoderThread(this); } else { Debug(2, "Failed to prime %d", ret); } @@ -3140,25 +3162,25 @@ Monitor::Orientation Monitor::getOrientation() const { return orientation; } // So this should be done as the first task in the analysis thread startup. // This function is deprecated. void Monitor::get_ref_image() { - ZMPacket *snap = nullptr; + ZMLockedPacket *snap_lock = nullptr; if ( !analysis_it ) analysis_it = packetqueue.get_video_it(true); while ( ( - !( snap = packetqueue.get_packet(analysis_it)) + !( snap_lock = packetqueue.get_packet(analysis_it)) or - ( snap->codec_type != AVMEDIA_TYPE_VIDEO ) + ( snap_lock->packet_->codec_type != AVMEDIA_TYPE_VIDEO ) or - ! snap->image + ! snap_lock->packet_->image ) and !zm_terminate) { Debug(1, "Waiting for capture daemon lastwriteindex(%d) lastwritetime(%d)", shared_data->last_write_index, shared_data->last_write_time); - if ( snap and ! snap->image ) { - snap->unlock(); + if ( snap_lock and ! snap_lock->packet_->image ) { + delete snap_lock; // can't analyse it anyways, incremement packetqueue.increment_it(analysis_it); } @@ -3167,6 +3189,7 @@ void Monitor::get_ref_image() { if ( zm_terminate ) return; + ZMPacket *snap = snap_lock->packet_; Debug(1, "get_ref_image: packet.stream %d ?= video_stream %d, packet image id %d packet image %p", snap->packet.stream_index, video_stream_id, snap->image_index, snap->image ); // Might not have been decoded yet FIXME @@ -3177,7 +3200,7 @@ void Monitor::get_ref_image() { } else { Debug(2, "Have no ref image about to unlock"); } - snap->unlock(); + delete snap_lock; } std::vector Monitor::Groups() { diff --git a/src/zm_packet.cpp b/src/zm_packet.cpp index 3a865fcc3..cb37558b3 100644 --- a/src/zm_packet.cpp +++ b/src/zm_packet.cpp @@ -27,7 +27,6 @@ using namespace std; AVPixelFormat target_format = AV_PIX_FMT_NONE; ZMPacket::ZMPacket() : - lck(mutex,std::defer_lock), keyframe(0), in_frame(nullptr), out_frame(nullptr), diff --git a/src/zm_packet.h b/src/zm_packet.h index f4032bf80..507547f99 100644 --- a/src/zm_packet.h +++ b/src/zm_packet.h @@ -37,6 +37,8 @@ class Image; class ZMPacket { public: + std::mutex mutex_; + std::condition_variable condition_; int keyframe; AVStream *stream; // Input stream @@ -68,43 +70,44 @@ class ZMPacket { ZMPacket(); ~ZMPacket(); - std::unique_lock * lock() { - std::unique_lock *lck = new std::unique_lock(mutex); - Debug(4, "packet %d locked", this->image_index); - return lck; - }; - std::unique_lock * trylock() { - std::unique_lock *lck = new std::unique_lock(mutex, std::defer_lock); - Debug(4, "TryLocking packet %d", this->image_index); - if ( lck.try_lock() ) - return lck; - delete lck; - return nullptr; - }; - void unlock(std::unique_lock *lck) { - Debug(4, "packet %d unlocked", this->image_index); - lck->unlock(); - condition.notify_all(); - }; - void wait(std::unique_lock *lck) { - Debug(4, "packet %d waiting", this->image_index); - // We already have a lock, but it's a recursive mutex.. so this may be ok - condition.wait(*lck); - } AVFrame *get_out_frame(const AVCodecContext *ctx); int get_codec_imgsize() { return codec_imgsize; }; }; -class ZMLockedPacket : public ZMPacket { +class ZMLockedPacket { public: - std::mutex mutex_; - std::condition_variable condition_; - std::unique_lock lck_; ZMPacket *packet_; + std::unique_lock lck_; - ZMLockedPacket(ZMPacket *p) : packet_(packet), lck_(mutex_) { + ZMLockedPacket(ZMPacket *p) : + packet_(p), + lck_(packet_->mutex_, std::defer_lock) { + } + ~ZMLockedPacket() { + unlock(); } -} + void lock() { + Debug(4, "locking packet %d", packet_->image_index); + lck_.lock(); + Debug(4, "packet %d locked", packet_->image_index); + }; + bool trylock() { + Debug(4, "TryLocking packet %d", packet_->image_index); + return lck_.try_lock(); + }; + void unlock() { + Debug(4, "packet %d unlocked", packet_->image_index); + lck_.unlock(); + packet_->condition_.notify_all(); + }; + + void wait() { + Debug(4, "packet %d waiting", packet_->image_index); + // We already have a lock, but it's a recursive mutex.. so this may be ok + packet_->condition_.wait(lck_); + } + +}; #endif /* ZM_PACKET_H */ diff --git a/src/zm_packetqueue.cpp b/src/zm_packetqueue.cpp index 5f34b1f0d..fe1c6f5b2 100644 --- a/src/zm_packetqueue.cpp +++ b/src/zm_packetqueue.cpp @@ -144,17 +144,19 @@ void PacketQueue::clearPackets(ZMPacket *add_packet) { // First packet is special because we know it is a video keyframe and only need to check for lock ZMPacket *zm_packet = *it; - if ( zm_packet->trylock() ) { + ZMLockedPacket *lp = new ZMLockedPacket(zm_packet); + if ( lp->trylock() ) { ++it; - zm_packet->unlock(); + delete lp; // Since we have many packets in the queue, we should NOT be pointing at end so don't need to test for that while ( *it != add_packet ) { zm_packet = *it; - if ( !zm_packet->trylock() ) { + lp = new ZMLockedPacket(zm_packet); + if ( !lp->trylock() ) { break; } - zm_packet->unlock(); + delete lp; if ( is_there_an_iterator_pointing_to_packet(zm_packet) ) { Warning("Found iterator at beginning of queue. Some thread isn't keeping up"); @@ -200,7 +202,7 @@ void PacketQueue::clearPackets(ZMPacket *add_packet) { return; } // end voidPacketQueue::clearPackets(ZMPacket* zm_packet) -ZMPacket* PacketQueue::popPacket( ) { +ZMLockedPacket* PacketQueue::popPacket( ) { Debug(4, "pktQueue size %d", pktQueue.size()); if ( pktQueue.empty() ) { return nullptr; @@ -222,14 +224,15 @@ ZMPacket* PacketQueue::popPacket( ) { } } // end foreach iterator - zm_packet->lock(); + ZMLockedPacket *lp = new ZMLockedPacket (zm_packet); + lp->lock(); pktQueue.pop_front(); packet_counts[zm_packet->packet.stream_index] -= 1; mutex.unlock(); - return zm_packet; + return lp; } // popPacket @@ -325,9 +328,9 @@ void PacketQueue::clear() { while (!pktQueue.empty()) { ZMPacket *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 - packet->lock(); + ZMLockedPacket lp(packet); + lp.lock(); pktQueue.pop_front(); - packet->unlock(); delete packet; } @@ -452,7 +455,7 @@ int PacketQueue::packet_count(int stream_id) { // Returns a packet. Packet will be locked -ZMPacket *PacketQueue::get_packet(packetqueue_iterator *it) { +ZMLockedPacket *PacketQueue::get_packet(packetqueue_iterator *it) { if ( deleting or zm_terminate ) return nullptr; @@ -477,16 +480,17 @@ ZMPacket *PacketQueue::get_packet(packetqueue_iterator *it) { Error("Null p?!"); return nullptr; } + ZMLockedPacket *lp = new ZMLockedPacket(p); Debug(3, "get_packet %p image_index: %d, about to lock packet", p, p->image_index); - while (!(zm_terminate or deleting) and !p->trylock()) { + while (!(zm_terminate or deleting) and !lp->trylock()) { Debug(3, "waiting on index %d. Queue size %d it == end? %d", p->image_index, pktQueue.size(), ( *it == pktQueue.end() ) ); ZM_DUMP_PACKET(p->packet, ""); condition.wait(lck); } Debug(2, "Locked packet, unlocking packetqueue mutex"); - return p; -} // end ZMPacket *PacketQueue::get_packet(it) + return lp; +} // end ZMLockedPacket *PacketQueue::get_packet(it) bool PacketQueue::increment_it(packetqueue_iterator *it) { Debug(2, "Incrementing %p, queue size %d, end? %d", it, pktQueue.size(), ((*it) == pktQueue.end())); diff --git a/src/zm_packetqueue.h b/src/zm_packetqueue.h index c0ba045b8..e759dc777 100644 --- a/src/zm_packetqueue.h +++ b/src/zm_packetqueue.h @@ -24,6 +24,7 @@ #include class ZMPacket; +class ZMLockedPacket; typedef std::list::iterator packetqueue_iterator; @@ -52,7 +53,7 @@ class PacketQueue { void setMaxVideoPackets(int p); bool queuePacket(ZMPacket* packet); - ZMPacket * popPacket(); + ZMLockedPacket * popPacket(); bool popVideoPacket(ZMPacket* packet); bool popAudioPacket(ZMPacket* packet); unsigned int clear(unsigned int video_frames_to_keep, int stream_id); @@ -68,7 +69,7 @@ class PacketQueue { bool increment_it(packetqueue_iterator *it); bool increment_it(packetqueue_iterator *it, int stream_id); - ZMPacket *get_packet(packetqueue_iterator *); + ZMLockedPacket *get_packet(packetqueue_iterator *); packetqueue_iterator *get_video_it(bool wait); packetqueue_iterator *get_stream_it(int stream_id); void free_it(packetqueue_iterator *); diff --git a/src/zm_videostore.cpp b/src/zm_videostore.cpp index 1ffc69e90..ba19f6f57 100644 --- a/src/zm_videostore.cpp +++ b/src/zm_videostore.cpp @@ -1211,29 +1211,6 @@ int VideoStore::writeAudioFramePacket(ZMPacket *zm_packet) { return 0; } // end int VideoStore::writeAudioFramePacket(AVPacket *ipkt) -int VideoStore::write_packets(PacketQueue &queue) { - // Need to write out all the frames from the last keyframe? - // No... need to write out all frames from when the event began. Due to PreEventFrames, this could be more than since the last keyframe. - unsigned int packet_count = 0; - ZMPacket *queued_packet; - - while ( ( queued_packet = queue.popPacket() ) ) { - AVPacket *avp = queued_packet->av_packet(); - - packet_count += 1; - //Write the packet to our video store - Debug(2, "Writing queued packet stream: %d KEY %d, remaining (%d)", - avp->stream_index, avp->flags & AV_PKT_FLAG_KEY, queue.size() ); - int ret = this->writePacket( queued_packet ); - if ( ret < 0 ) { - //Less than zero and we skipped a frame - } - delete queued_packet; - } // end while packets in the packetqueue - Debug(2, "Wrote %d queued packets", packet_count ); - return packet_count; -} // end int VideoStore::write_packets( PacketQueue &queue ) { - int VideoStore::write_packet(AVPacket *pkt, AVStream *stream) { pkt->pos = -1; pkt->stream_index = stream->index; From 76267bc57f48ec4ab4fda7cf506aeff25db6d35b Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Tue, 16 Mar 2021 12:08:11 -0400 Subject: [PATCH 05/72] put back deleting the raw image when not saving jpegs. We only need it for the snapshot and that should be the alarmed image anyways. --- src/zm_monitor.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 0ea6b5fb6..4097036ae 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -2286,6 +2286,15 @@ bool Monitor::Analyse() { if (event) event->AddPacket(snap); + // In the case where people have pre-alarm frames, the web ui will generate the frame images + // from the mp4. So no one will notice anyways. + if ((videowriter == PASSTHROUGH) and !savejpegs) { + Debug(1, "Deleting image data for %d", snap->image_index); + // Don't need raw images anymore + delete snap->image; + snap->image = nullptr; + } + // popPacket will have placed a second lock on snap, so release it here. delete packet_lock; From 9d239219de5c6a0bf06030ff8fc513c64ac2dd81 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Tue, 16 Mar 2021 12:08:45 -0400 Subject: [PATCH 06/72] Break out early if no more buffer. Saves a couple cycles --- src/zm_rtsp_server_fifo_h264_source.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/zm_rtsp_server_fifo_h264_source.cpp b/src/zm_rtsp_server_fifo_h264_source.cpp index 6d7dda1b1..3d32f1809 100644 --- a/src/zm_rtsp_server_fifo_h264_source.cpp +++ b/src/zm_rtsp_server_fifo_h264_source.cpp @@ -83,6 +83,7 @@ std::list< std::pair > H264_ZoneMinderFifoSource::splitF } #endif frameList.push_back(std::pair(buffer, size)); + if (!bufSize) break; buffer = this->extractFrame(&buffer[size], bufSize, size); } // end while buffer From 28700fd56b94345b2c9651ca3a8bfab2cc68133c Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Tue, 16 Mar 2021 12:09:14 -0400 Subject: [PATCH 07/72] Implement saving DecodingEnabled from function view --- web/includes/actions/function.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/web/includes/actions/function.php b/web/includes/actions/function.php index 338361883..fc7f7703b 100644 --- a/web/includes/actions/function.php +++ b/web/includes/actions/function.php @@ -40,10 +40,12 @@ if ( $action == 'function' ) { $newFunction = validStr($_REQUEST['newFunction']); # Because we use a checkbox, it won't get passed in the request. So not being in _REQUEST means 0 $newEnabled = ( !isset($_REQUEST['newEnabled']) or $_REQUEST['newEnabled'] != '1' ) ? '0' : '1'; + $newDecodingEnabled = ( !isset($_REQUEST['newDecodingEnabled']) or $_REQUEST['newDecodingEnabled'] != '1' ) ? '0' : '1'; $oldFunction = $monitor->Function(); $oldEnabled = $monitor->Enabled(); - if ( $newFunction != $oldFunction || $newEnabled != $oldEnabled ) { - $monitor->save(array('Function'=>$newFunction, 'Enabled'=>$newEnabled)); + $oldDecodingEnabled = $monitor->DecodingEnabled(); + if ( $newFunction != $oldFunction || $newEnabled != $oldEnabled || $newDecodingEnabled != $oldDecodingEnabled ) { + $monitor->save(array('Function'=>$newFunction, 'Enabled'=>$newEnabled, 'DecodingEnabled'=>$newDecodingEnabled)); if ( daemonCheck() && ($monitor->Type() != 'WebSite') ) { $monitor->zmcControl(($newFunction != 'None') ? 'restart' : 'stop'); From ebf1b7cbdc05a5b952c6aebebfcc40b01a27ff3f Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Tue, 16 Mar 2021 13:26:06 -0400 Subject: [PATCH 08/72] Only output to stdout if mTerminalLevel is something. zms for example SHOULD not output to stdout, ever except maybe when running from terminal to debug --- src/zm_logger.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/zm_logger.cpp b/src/zm_logger.cpp index 75d4d4865..3d0c0c2c5 100644 --- a/src/zm_logger.cpp +++ b/src/zm_logger.cpp @@ -524,8 +524,8 @@ void Logger::logPrint(bool hex, const char * const filepath, const int line, con if (mLogFileFP) { fputs(logString, mLogFileFP); if (mFlush) fflush(mLogFileFP); - } else { - puts("Logging to file, but failed to open it\n"); + } else if (mTerminalLevel != NOLOG) { + puts("Logging to file but failed to open it\n"); } } // end if level <= mFileLevel From 5e54a63bd5f4a83a879f7559c28ed146f60dedbc Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Tue, 16 Mar 2021 13:26:40 -0400 Subject: [PATCH 09/72] Only load zones if doing something other than QUERY. Only delete decoder if there is one. --- src/zm_monitor.cpp | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 4097036ae..bc91de2bf 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -446,8 +446,7 @@ void Monitor::Load(MYSQL_ROW dbrow, bool load_zones=true, Purpose p = QUERY) { server_id = dbrow[col] ? atoi(dbrow[col]) : 0; col++; storage_id = atoi(dbrow[col]); col++; - if ( storage ) - delete storage; + if (storage) delete storage; storage = new Storage(storage_id); if ( ! strcmp(dbrow[col], "Local") ) { @@ -636,16 +635,16 @@ void Monitor::Load(MYSQL_ROW dbrow, bool load_zones=true, Purpose p = QUERY) { image_buffer_count, image_size, (image_buffer_count*image_size), mem_size); - Zone **zones = 0; - int n_zones = Zone::Load(this, zones); - this->AddZones(n_zones, zones); - this->AddPrivacyBitmask(zones); - // Should maybe store this for later use std::string monitor_dir = stringtf("%s/%d", storage->Path(), id); LoadCamera(); if ( purpose != QUERY ) { + Zone **zones = 0; + int n_zones = Zone::Load(this, zones); + this->AddZones(n_zones, zones); + this->AddPrivacyBitmask(zones); + if ( mkdir(monitor_dir.c_str(), 0755) && ( errno != EEXIST ) ) { Error("Can't mkdir %s: %s", monitor_dir.c_str(), strerror(errno)); } @@ -2733,7 +2732,6 @@ int Monitor::Capture() { } // end Monitor::Capture bool Monitor::Decode() { - if (!decoder_it) decoder_it = packetqueue.get_video_it(true); ZMLockedPacket *packet_lock = packetqueue.get_packet(decoder_it); if (!packet_lock) return false; @@ -3143,7 +3141,10 @@ int Monitor::PrimeCapture() { audio_fifo = new Fifo(shared_data->audio_fifo_path, true); } } // end if rtsp_server - if (decoding_enabled) decoder = new DecoderThread(this); + if (decoding_enabled) { + if (!decoder_it) decoder_it = packetqueue.get_video_it(true); + decoder = new DecoderThread(this); + } } else { Debug(2, "Failed to prime %d", ret); } @@ -3153,8 +3154,10 @@ int Monitor::PrimeCapture() { int Monitor::PreCapture() const { return camera->PreCapture(); } int Monitor::PostCapture() const { return camera->PostCapture(); } int Monitor::Close() { - decoder->Stop(); - delete decoder; + if ( decoder ) { + decoder->Stop(); + delete decoder; + } std::lock_guard lck(event_mutex); if (event) { Info("%s: image_count:%d - Closing event %" PRIu64 ", shutting down", name, image_count, event->Id()); From 028f2dd626eecb62c5f4371aa80e58a931ff556b Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Tue, 16 Mar 2021 13:27:27 -0400 Subject: [PATCH 10/72] Debug extra error log and code style --- src/zm_monitorstream.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/zm_monitorstream.cpp b/src/zm_monitorstream.cpp index fd618800e..b31c8e642 100644 --- a/src/zm_monitorstream.cpp +++ b/src/zm_monitorstream.cpp @@ -467,7 +467,7 @@ bool MonitorStream::sendFrame(Image *image, struct timeval *timestamp) { } // end bool MonitorStream::sendFrame( Image *image, struct timeval *timestamp ) void MonitorStream::runStream() { - if ( type == STREAM_SINGLE ) { + if (type == STREAM_SINGLE) { // Not yet migrated over to stream class SingleImage(scale); return; @@ -479,15 +479,13 @@ void MonitorStream::runStream() { fputs("Content-Type: multipart/x-mixed-replace; boundary=" BOUNDARY "\r\n\r\n", stdout); if ( !checkInitialised() ) { - Error("Not initialized"); - while ( !(loadMonitor(monitor_id) || zm_terminate) ) { + while (!(loadMonitor(monitor_id) || zm_terminate)) { sendTextFrame("Not connected"); - if ( connkey ) + if (connkey) checkCommandQueue(); sleep(1); } - if ( zm_terminate ) - return; + if (zm_terminate) return; } updateFrameRate(monitor->GetFPS()); @@ -562,7 +560,7 @@ void MonitorStream::runStream() { capture_fps = capture_max_fps; } - while ( !zm_terminate ) { + while (!zm_terminate) { bool got_command = false; if ( feof(stdout) ) { Debug(2, "feof stdout"); @@ -570,7 +568,7 @@ void MonitorStream::runStream() { } else if ( ferror(stdout) ) { Debug(2, "ferror stdout"); break; - } else if ( !monitor->ShmValid() ) { + } else if (!monitor->ShmValid()) { Debug(2, "monitor not valid.... maybe we should wait until it comes back."); break; } From c0242e736970a6d5d534a9f7618d5fc807acc76c Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Tue, 16 Mar 2021 13:28:00 -0400 Subject: [PATCH 11/72] Fix memleak when connect fails --- src/zm_stream.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/zm_stream.cpp b/src/zm_stream.cpp index 1f141ec65..e309a14bc 100644 --- a/src/zm_stream.cpp +++ b/src/zm_stream.cpp @@ -40,7 +40,7 @@ StreamBase::~StreamBase() { bool StreamBase::loadMonitor(int p_monitor_id) { monitor_id = p_monitor_id; - if ( !(monitor = Monitor::Load(monitor_id, false, Monitor::QUERY)) ) { + if ( !(monitor or (monitor = Monitor::Load(monitor_id, false, Monitor::QUERY))) ) { Error("Unable to load monitor id %d for streaming", monitor_id); return false; } @@ -52,6 +52,7 @@ bool StreamBase::loadMonitor(int p_monitor_id) { if ( !monitor->connect() ) { Error("Unable to connect to monitor id %d for streaming", monitor_id); + monitor->disconnect(); return false; } From 8fa989f8e9452c14d436b6c907d5bdfc15192699 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Tue, 16 Mar 2021 20:07:06 -0400 Subject: [PATCH 12/72] Increase debug level of input selection --- src/zm_ffmpeg_camera.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/zm_ffmpeg_camera.cpp b/src/zm_ffmpeg_camera.cpp index a1d8fb418..2f89197cb 100644 --- a/src/zm_ffmpeg_camera.cpp +++ b/src/zm_ffmpeg_camera.cpp @@ -200,10 +200,10 @@ int FfmpegCamera::Capture(ZMPacket &zm_packet) { ) ) { // if audio stream is behind video stream, then read from audio, otherwise video mFormatContextPtr = mSecondFormatContext; - Debug(1, "Using audio input"); + Debug(2, "Using audio input"); } else { mFormatContextPtr = mFormatContext; - Debug(1, "Using video input because %" PRId64 " >= %" PRId64, + Debug(2, "Using video input because %" PRId64 " >= %" PRId64, (mAudioStream?av_rescale_q(mLastAudioPTS, mAudioStream->time_base, AV_TIME_BASE_Q):0), av_rescale_q(mLastVideoPTS, mVideoStream->time_base, AV_TIME_BASE_Q) ); From 12ed02a5b092ef7aaaf5d514b82593ca793a472f Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Tue, 16 Mar 2021 20:07:59 -0400 Subject: [PATCH 13/72] Move trigger detection before motion detection. Only wait for decoding if decoding is enabled --- src/zm_monitor.cpp | 99 ++++++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 47 deletions(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index bc91de2bf..091d2278e 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -1893,16 +1893,55 @@ bool Monitor::Analyse() { if (signal) { if (snap->codec_type == AVMEDIA_TYPE_VIDEO) { - while (!snap->image and !snap->decoded) { - // Need to wait for the decoder thread. + // Check to see if linked monitors are triggering. + if (n_linked_monitors > 0) { + Debug(1, "Checking linked monitors"); + // FIXME improve logic here + bool first_link = true; + Event::StringSet noteSet; + for ( int i = 0; i < n_linked_monitors; i++ ) { + // TODO: Shouldn't we try to connect? + if ( linked_monitors[i]->isConnected() ) { + Debug(1, "Linked monitor %d %s is connected", + linked_monitors[i]->Id(), linked_monitors[i]->Name()); + if ( linked_monitors[i]->hasAlarmed() ) { + Debug(1, "Linked monitor %d %s is alarmed", + linked_monitors[i]->Id(), linked_monitors[i]->Name()); + if ( !event ) { + if ( first_link ) { + if ( cause.length() ) + cause += ", "; + cause += LINKED_CAUSE; + first_link = false; + } + } + noteSet.insert(linked_monitors[i]->Name()); + score += linked_monitors[i]->lastFrameScore(); // 50; + } else { + Debug(1, "Linked monitor %d %s is not alarmed", + linked_monitors[i]->Id(), linked_monitors[i]->Name()); + } + } else { + Debug(1, "Linked monitor %d %d is not connected. Connecting.", i, linked_monitors[i]->Id()); + linked_monitors[i]->connect(); + } + } // end foreach linked_monitor + if ( noteSet.size() > 0 ) + noteSetMap[LINKED_CAUSE] = noteSet; + } // end if linked_monitors + + if ( decoding_enabled ) { + while (!snap->image and !snap->decoded and !zm_terminate) { + // Need to wait for the decoder thread. Debug(1, "Waiting for decode"); - packet_lock->wait(); - if (!snap->image and snap->decoded) { - Debug(1, "No image but was decoded, giving up"); - delete packet_lock; - return false; - } - } // end while ! decoded + packet_lock->wait(); + if (!snap->image and snap->decoded) { + Debug(1, "No image but was decoded, giving up"); + delete packet_lock; + return false; + } + } // end while ! decoded + } struct timeval *timestamp = snap->timestamp; @@ -1943,42 +1982,6 @@ bool Monitor::Analyse() { } // end if motion_score } // end if active and doing motion detection - // Check to see if linked monitors are triggering. - if (n_linked_monitors > 0) { - Debug(4, "Checking linked monitors"); - // FIXME improve logic here - bool first_link = true; - Event::StringSet noteSet; - for ( int i = 0; i < n_linked_monitors; i++ ) { - // TODO: Shouldn't we try to connect? - if ( linked_monitors[i]->isConnected() ) { - Debug(4, "Linked monitor %d %s is connected", - linked_monitors[i]->Id(), linked_monitors[i]->Name()); - if ( linked_monitors[i]->hasAlarmed() ) { - Debug(4, "Linked monitor %d %s is alarmed", - linked_monitors[i]->Id(), linked_monitors[i]->Name()); - if ( !event ) { - if ( first_link ) { - if ( cause.length() ) - cause += ", "; - cause += LINKED_CAUSE; - first_link = false; - } - } - noteSet.insert(linked_monitors[i]->Name()); - score += linked_monitors[i]->lastFrameScore(); // 50; - } else { - Debug(4, "Linked monitor %d %s is not alarmed", - linked_monitors[i]->Id(), linked_monitors[i]->Name()); - } - } else { - Debug(1, "Linked monitor %d %d is not connected. Connecting.", i, linked_monitors[i]->Id()); - linked_monitors[i]->connect(); - } - } // end foreach linked_monitor - if ( noteSet.size() > 0 ) - noteSetMap[LINKED_CAUSE] = noteSet; - } // end if linked_monitors if (function == RECORD or function == MOCORD) { // If doing record, check to see if we need to close the event or not. @@ -2287,7 +2290,7 @@ bool Monitor::Analyse() { // In the case where people have pre-alarm frames, the web ui will generate the frame images // from the mp4. So no one will notice anyways. - if ((videowriter == PASSTHROUGH) and !savejpegs) { + if (snap->image and (videowriter == PASSTHROUGH) and !savejpegs) { Debug(1, "Deleting image data for %d", snap->image_index); // Don't need raw images anymore delete snap->image; @@ -2569,6 +2572,8 @@ int Monitor::Capture() { // Don't want to do analysis on it, but we won't due to signal return -1; } else if ( captureResult > 0 ) { + // If we captured, let's assume signal, ::Decode will detect further + shared_data->signal = true; Debug(2, "Have packet stream_index:%d ?= videostream_id:(%d) q.vpktcount(%d) event?(%d) ", packet->packet.stream_index, video_stream_id, packetqueue.packet_count(video_stream_id), ( event ? 1 : 0 ) ); @@ -2732,6 +2737,7 @@ int Monitor::Capture() { } // end Monitor::Capture bool Monitor::Decode() { + if (!decoder_it) decoder_it = packetqueue.get_video_it(true); ZMLockedPacket *packet_lock = packetqueue.get_packet(decoder_it); if (!packet_lock) return false; @@ -3142,7 +3148,6 @@ int Monitor::PrimeCapture() { } } // end if rtsp_server if (decoding_enabled) { - if (!decoder_it) decoder_it = packetqueue.get_video_it(true); decoder = new DecoderThread(this); } } else { From 1daafd7f851ff6b336a973160036b98805d01dc3 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Tue, 16 Mar 2021 20:08:08 -0400 Subject: [PATCH 14/72] add GetType --- src/zm_monitor.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/zm_monitor.h b/src/zm_monitor.h index 91d12dd24..dbd54d6cd 100644 --- a/src/zm_monitor.h +++ b/src/zm_monitor.h @@ -414,6 +414,8 @@ public: if ( shared_data && shared_data->valid ) { struct timeval now; gettimeofday(&now, nullptr); + Debug(3, "Shared data is valid, checking heartbeat %u - %u = %d < %f", + now.tv_sec, shared_data->zmc_heartbeat_time, (now.tv_sec - shared_data->zmc_heartbeat_time), config.watch_max_delay); if ((now.tv_sec - shared_data->zmc_heartbeat_time) < config.watch_max_delay) return true; } @@ -429,6 +431,7 @@ public: } return storage; } + inline CameraType GetType() const { return type; } inline Function GetFunction() const { return function; } inline PacketQueue * GetPacketQueue() { return &packetqueue; } inline bool Enabled() const { From fe17d7bb2320e233e1c1ed94842c6d7599e4b16a Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Tue, 16 Mar 2021 20:09:14 -0400 Subject: [PATCH 15/72] Add checks for aliveness of monitor in streaming. If decoding disabled can't view stream. --- src/zm_monitorstream.cpp | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/zm_monitorstream.cpp b/src/zm_monitorstream.cpp index b31c8e642..491019a3a 100644 --- a/src/zm_monitorstream.cpp +++ b/src/zm_monitorstream.cpp @@ -469,24 +469,33 @@ bool MonitorStream::sendFrame(Image *image, struct timeval *timestamp) { void MonitorStream::runStream() { if (type == STREAM_SINGLE) { // Not yet migrated over to stream class - SingleImage(scale); + if (checkInitialised()) + SingleImage(scale); + else + sendTextFrame("Unable to send image"); + return; } openComms(); - if ( type == STREAM_JPEG ) + if (type == STREAM_JPEG) fputs("Content-Type: multipart/x-mixed-replace; boundary=" BOUNDARY "\r\n\r\n", stdout); - if ( !checkInitialised() ) { - while (!(loadMonitor(monitor_id) || zm_terminate)) { - sendTextFrame("Not connected"); - if (connkey) - checkCommandQueue(); - sleep(1); - } - if (zm_terminate) return; + while (!(loadMonitor(monitor_id) || zm_terminate)) { + sendTextFrame("Not connected"); + if (connkey) + checkCommandQueue(); + sleep(1); } + if (zm_terminate) return; + while (!checkInitialised() and !zm_terminate) { + sendTextFrame("Unable to stream"); + if (connkey) + checkCommandQueue(); + sleep(1); + } + if (zm_terminate) return; updateFrameRate(monitor->GetFPS()); @@ -854,7 +863,7 @@ void MonitorStream::SingleImage(int scale) { int img_buffer_size = 0; static JOCTET img_buffer[ZM_MAX_IMAGE_SIZE]; Image scaled_image; - while ( monitor->shared_data->last_write_index >= monitor->image_buffer_count ) { + while ((monitor->shared_data->last_write_index >= monitor->image_buffer_count) and !zm_terminate) { Debug(1, "Waiting for capture to begin"); usleep(100000); } From 14ef8336b9c4d5380e1c355b79997b2799f6cfa5 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Tue, 16 Mar 2021 20:10:00 -0400 Subject: [PATCH 16/72] If ffmpeg and decoding Disabled don't stream --- src/zm_stream.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/zm_stream.cpp b/src/zm_stream.cpp index e309a14bc..869341114 100644 --- a/src/zm_stream.cpp +++ b/src/zm_stream.cpp @@ -72,6 +72,10 @@ bool StreamBase::checkInitialised() { Error("Monitor shm is not connected"); return false; } + if ((monitor->GetType() == Monitor::FFMPEG) and !monitor->DecodingEnabled() ) { + Error("Monitor is not decoding."); + return false; + } return true; } From 74616d106192a7579a7b3272bf4083cda5f30cc3 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 10:06:45 -0400 Subject: [PATCH 17/72] Update CaptureFPS SQL to just do an update and don't use static sql. TThis may fix a signal 6 crash that we have been seeing --- src/zm_monitor.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 091d2278e..2835146ff 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -1734,16 +1734,10 @@ void Monitor::UpdateCaptureFPS() { last_fps_time = now_double; last_capture_image_count = image_count; - static char sql[ZM_SQL_SML_BUFSIZ]; - // The reason we update the Status as well is because if mysql restarts, the Monitor_Status table is lost, - // and nothing else will update the status until zmc restarts. Since we are successfully capturing we can - // assume that we are connected - snprintf(sql, sizeof(sql), - "INSERT INTO Monitor_Status (MonitorId,CaptureFPS,CaptureBandwidth,Status) " - "VALUES (%d, %.2lf, %u, 'Connected') ON DUPLICATE KEY UPDATE " - "CaptureFPS = %.2lf, CaptureBandwidth=%u, Status='Connected'", - id, new_capture_fps, new_capture_bandwidth, new_capture_fps, new_capture_bandwidth); - dbQueue.push(sql); + std::string sql = stringtf( + "UPDATE Monitor_Status SET CaptureFPS = %.2lf, CaptureBandwidth=%u WHERE MonitorId=%u", + new_capture_fps, new_capture_bandwidth, id); + dbQueue.push(sql.c_str()); } // now != last_fps_time } // end if report fps } // void Monitor::UpdateCaptureFPS() From 6a2e237902a57306875ab0aeac18eb0b737f3045 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 10:07:03 -0400 Subject: [PATCH 18/72] Fix delete packet before deleting lock on packet --- src/zm_packetqueue.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/zm_packetqueue.cpp b/src/zm_packetqueue.cpp index fe1c6f5b2..4cbf23910 100644 --- a/src/zm_packetqueue.cpp +++ b/src/zm_packetqueue.cpp @@ -87,7 +87,7 @@ bool PacketQueue::queuePacket(ZMPacket* add_packet) { pktQueue.push_back(add_packet); packet_counts[add_packet->packet.stream_index] += 1; - Debug(1, "packet counts for %d is %d", + Debug(2, "packet counts for %d is %d", add_packet->packet.stream_index, packet_counts[add_packet->packet.stream_index]); @@ -328,9 +328,10 @@ void PacketQueue::clear() { while (!pktQueue.empty()) { ZMPacket *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 - ZMLockedPacket lp(packet); - lp.lock(); + ZMLockedPacket *lp = new ZMLockedPacket(packet); + lp->lock(); pktQueue.pop_front(); + delete lp; delete packet; } @@ -605,7 +606,7 @@ packetqueue_iterator * PacketQueue::get_video_it(bool wait) { if ( wait ) { while ( ((! pktQueue.size()) or (*it == pktQueue.end())) and !zm_terminate and !deleting ) { - Debug(2, "waiting. Queue size %d it == end? %d", pktQueue.size(), ( *it == pktQueue.end() ) ); + Debug(2, "waiting for packets in queue. Queue size %d it == end? %d", pktQueue.size(), ( *it == pktQueue.end() ) ); condition.wait(lck); *it = pktQueue.begin(); } From 657a5edda4ce0135672a2f0a124e024f06b02c0b Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 10:11:06 -0400 Subject: [PATCH 19/72] If decoding disabled, set signal and last_write_time in the Capture thread. So that zm_watch knows we are alive --- src/zm_monitor.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 2835146ff..86d1774f4 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -2567,7 +2567,11 @@ int Monitor::Capture() { return -1; } else if ( captureResult > 0 ) { // If we captured, let's assume signal, ::Decode will detect further - shared_data->signal = true; + if (!decoding_enabled) { + shared_data->last_write_index = index; + shared_data->last_write_time = packet->timestamp->tv_sec; + shared_data->signal = true; + } Debug(2, "Have packet stream_index:%d ?= videostream_id:(%d) q.vpktcount(%d) event?(%d) ", packet->packet.stream_index, video_stream_id, packetqueue.packet_count(video_stream_id), ( event ? 1 : 0 ) ); From a8d31ca6862405dcfb34a961e7424ceaf665d062 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 12:46:55 -0400 Subject: [PATCH 20/72] After moving analysis thread into monitor, I don't know how to handle the shared_ptr stuff --- src/zm_analysis_thread.cpp | 7 +++++-- src/zm_analysis_thread.h | 8 +++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/zm_analysis_thread.cpp b/src/zm_analysis_thread.cpp index 3e66940a0..7a6b01bbb 100644 --- a/src/zm_analysis_thread.cpp +++ b/src/zm_analysis_thread.cpp @@ -1,10 +1,13 @@ #include "zm_analysis_thread.h" +#include "zm_monitor.h" #include "zm_signal.h" #include "zm_utils.h" -AnalysisThread::AnalysisThread(std::shared_ptr monitor) : - monitor_(std::move(monitor)), terminate_(false) { +//AnalysisThread::AnalysisThread(std::shared_ptr monitor) : +AnalysisThread::AnalysisThread(Monitor* monitor) : + monitor_(monitor), terminate_(false) { + //monitor_(std::move(monitor)), terminate_(false) { thread_ = std::thread(&AnalysisThread::Run, this); } diff --git a/src/zm_analysis_thread.h b/src/zm_analysis_thread.h index 8cf389e93..7fc0fd8c9 100644 --- a/src/zm_analysis_thread.h +++ b/src/zm_analysis_thread.h @@ -1,14 +1,15 @@ #ifndef ZM_ANALYSIS_THREAD_H #define ZM_ANALYSIS_THREAD_H -#include "zm_monitor.h" +class Monitor; #include #include #include class AnalysisThread { public: - explicit AnalysisThread(std::shared_ptr monitor); + explicit AnalysisThread(Monitor* monitor); + //explicit AnalysisThread(std::shared_ptr monitor); ~AnalysisThread(); AnalysisThread(AnalysisThread &rhs) = delete; AnalysisThread(AnalysisThread &&rhs) = delete; @@ -18,7 +19,8 @@ class AnalysisThread { private: void Run(); - std::shared_ptr monitor_; + Monitor* monitor_; + //std::shared_ptr monitor_; std::atomic terminate_; std::thread thread_; }; From dca34544ec5d9bac52ecfe7e6942ea0bd80b98b1 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 12:47:52 -0400 Subject: [PATCH 21/72] move analysis thread into monitor. populate analysis_it and decoder_it in Prime instead of constantly checking for them. Handle cases where LockedPacket are null due to shutdown --- src/zm_monitor.cpp | 120 +++++++++++++++++++++++++++------------------ 1 file changed, 71 insertions(+), 49 deletions(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 86d1774f4..38a2814a3 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -395,6 +395,7 @@ Monitor::Monitor() storage(nullptr), videoStore(nullptr), analysis_it(nullptr), + analysis_thread(nullptr), decoder_it(nullptr), decoder(nullptr), n_zones(0), @@ -1126,12 +1127,11 @@ Monitor::~Monitor() { disconnect(); } // end if mem_ptr - if (analysis_it) { - packetqueue.free_it(analysis_it); - analysis_it = nullptr; - } + // Will be free by packetqueue destructor + analysis_it = nullptr; + decoder_it = nullptr; - for ( int i = 0; i < n_zones; i++ ) { + for (int i = 0; i < n_zones; i++) { delete zones[i]; } delete[] zones; @@ -1801,7 +1801,6 @@ bool Monitor::Analyse() { Warning("Shouldn't be doing Analyse when not Enabled"); return false; } - if (!analysis_it) analysis_it = packetqueue.get_video_it(true); // if have event, send frames until we find a video packet, at which point do analysis. Adaptive skip should only affect which frames we do analysis on. @@ -1959,12 +1958,12 @@ bool Monitor::Analyse() { Debug(3, "After motion detection, score:%d last_motion_score(%d), new motion score(%d)", score, last_motion_score, motion_score); + motion_frame_count += 1; } else { - Warning("No image in snap"); + Debug(1, "No image in snap, codec likely not ready"); } // Why are we updating the last_motion_score too? last_motion_score = motion_score; - motion_frame_count += 1; } else { Debug(1, "Skipped motion detection"); } @@ -2031,6 +2030,7 @@ bool Monitor::Analyse() { } ZMLockedPacket *lp = packetqueue.get_packet(start_it); delete starting_packet_lock; + if (!lp) return false; starting_packet_lock = lp; starting_packet = lp->packet_; } @@ -2129,6 +2129,11 @@ bool Monitor::Analyse() { } ZMLockedPacket *lp = packetqueue.get_packet(start_it); delete starting_packet_lock; + if (!lp) { + // Shutting down event will be closed by ~Monitor() + // Perhaps we shouldn't do this. + return false; + } starting_packet_lock = lp; starting_packet = lp->packet_; } @@ -2164,7 +2169,7 @@ bool Monitor::Analyse() { Debug(1, "Staying in %s", State_Strings[state].c_str()); } - if ( state == ALARM ) { + if (state == ALARM) { last_alarm_count = analysis_image_count; } // This is needed so post_event_count counts after last alarmed frames while in ALARM not single alarmed frames while ALERT } else { // no score? @@ -2172,7 +2177,7 @@ bool Monitor::Analyse() { if (state == ALARM) { Info("%s: %03d - Gone into alert state", name, analysis_image_count); shared_data->state = state = ALERT; - } else if ( state == ALERT ) { + } else if (state == ALERT) { if ( ( analysis_image_count-last_alarm_count > post_event_count ) && @@ -2190,7 +2195,7 @@ bool Monitor::Analyse() { shared_data->state = state = TAPE; } } - } else if ( state == PREALARM ) { + } else if (state == PREALARM) { // Back to IDLE shared_data->state = state = ((function != MOCORD) ? IDLE : TAPE); } else { @@ -2198,19 +2203,19 @@ bool Monitor::Analyse() { State_Strings[state].c_str(), analysis_image_count, last_alarm_count, post_event_count, timestamp->tv_sec, video_store_data->recording.tv_sec, min_section_length); } - if ( Event::PreAlarmCount() ) + if (Event::PreAlarmCount()) Event::EmptyPreAlarmFrames(); } // end if score or not snap->score = score; - if ( state == PREALARM ) { + if (state == PREALARM) { // Generate analysis images if necessary - if ( (savejpegs > 1) and snap->image ) { - for ( int i = 0; i < n_zones; i++ ) { - if ( zones[i]->Alarmed() ) { - if ( zones[i]->AlarmImage() ) { - if ( ! snap->analysis_image ) + if ((savejpegs > 1) and snap->image) { + for (int i = 0; i < n_zones; i++) { + if (zones[i]->Alarmed()) { + if (zones[i]->AlarmImage()) { + if (!snap->analysis_image) snap->analysis_image = new Image(*(snap->image)); snap->analysis_image->Overlay(*(zones[i]->AlarmImage())); } @@ -2221,38 +2226,42 @@ bool Monitor::Analyse() { // incremement pre alarm image count //have_pre_alarmed_frames ++; Event::AddPreAlarmFrame(snap->image, *timestamp, score, nullptr); - } else if ( state == ALARM ) { - if ( ( savejpegs > 1 ) and snap->image ) { - for ( int i = 0; i < n_zones; i++ ) { - if ( zones[i]->Alarmed() ) { - if ( zones[i]->AlarmImage() ) { - if ( ! snap->analysis_image ) + } else if (state == ALARM) { + if ((savejpegs > 1 ) and snap->image) { + for (int i = 0; i < n_zones; i++) { + if (zones[i]->Alarmed()) { + if (zones[i]->AlarmImage()) { + if (!snap->analysis_image) snap->analysis_image = new Image(*(snap->image)); snap->analysis_image->Overlay(*(zones[i]->AlarmImage())); } - if ( config.record_event_stats ) + if (config.record_event_stats) zones[i]->RecordStats(event); } // end if zone is alarmed } // end foreach zone - } - if ( noteSetMap.size() > 0 ) - event->updateNotes(noteSetMap); - if ( section_length - && ( ( timestamp->tv_sec - video_store_data->recording.tv_sec ) >= section_length ) - && ! (image_count % fps_report_interval) - ) { - Warning("%s: %03d - event %" PRIu64 ", has exceeded desired section length. %d - %d = %d >= %d", - name, image_count, event->Id(), - timestamp->tv_sec, video_store_data->recording.tv_sec, - timestamp->tv_sec - video_store_data->recording.tv_sec, - section_length - ); - closeEvent(); - event = new Event(this, *timestamp, cause, noteSetMap); - shared_data->last_event_id = event->Id(); - //set up video store data - snprintf(video_store_data->event_file, sizeof(video_store_data->event_file), "%s", event->getEventFile()); - video_store_data->recording = event->StartTime(); + } + if (event) { + if (noteSetMap.size() > 0) + event->updateNotes(noteSetMap); + if ( section_length + && ( ( timestamp->tv_sec - video_store_data->recording.tv_sec ) >= section_length ) + && ! (image_count % fps_report_interval) + ) { + Warning("%s: %03d - event %" PRIu64 ", has exceeded desired section length. %d - %d = %d >= %d", + name, image_count, event->Id(), + timestamp->tv_sec, video_store_data->recording.tv_sec, + timestamp->tv_sec - video_store_data->recording.tv_sec, + section_length + ); + closeEvent(); + event = new Event(this, *timestamp, cause, noteSetMap); + shared_data->last_event_id = event->Id(); + //set up video store data + snprintf(video_store_data->event_file, sizeof(video_store_data->event_file), "%s", event->getEventFile()); + video_store_data->recording = event->StartTime(); + } + } else { + Error("ALARM but no event"); } } else if ( state == ALERT ) { @@ -2735,7 +2744,6 @@ int Monitor::Capture() { } // end Monitor::Capture bool Monitor::Decode() { - if (!decoder_it) decoder_it = packetqueue.get_video_it(true); ZMLockedPacket *packet_lock = packetqueue.get_packet(decoder_it); if (!packet_lock) return false; @@ -3146,8 +3154,17 @@ int Monitor::PrimeCapture() { } } // end if rtsp_server if (decoding_enabled) { - decoder = new DecoderThread(this); + if (!decoder_it) decoder_it = packetqueue.get_video_it(false); + if (!decoder) decoder = new DecoderThread(this); } + if (function != MONITOR) { + if (!analysis_it) analysis_it = packetqueue.get_video_it(false); + if (!analysis_thread) { + Debug(1, "Starting an analysis thread for monitor (%d)", id); + analysis_thread = new AnalysisThread(this); + } + } + } else { Debug(2, "Failed to prime %d", ret); } @@ -3157,17 +3174,22 @@ int Monitor::PrimeCapture() { int Monitor::PreCapture() const { return camera->PreCapture(); } int Monitor::PostCapture() const { return camera->PostCapture(); } int Monitor::Close() { - if ( decoder ) { - decoder->Stop(); + // Because the stream indexes may change we have to clear out the packetqueue + if (decoder) decoder->Stop(); + if (analysis_thread) analysis_thread->Stop(); + packetqueue.clear(); + if (decoder) { delete decoder; } + if (analysis_thread) { + delete analysis_thread; + } std::lock_guard lck(event_mutex); if (event) { Info("%s: image_count:%d - Closing event %" PRIu64 ", shutting down", name, image_count, event->Id()); closeEvent(); } if (camera) camera->Close(); - packetqueue.clear(); return 1; } From 9ca5f49d82072e54e0666ce5daa3aca0d684843e Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 12:48:08 -0400 Subject: [PATCH 22/72] Move analysis_thread into Monitor --- src/zm_monitor.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/zm_monitor.h b/src/zm_monitor.h index dbd54d6cd..7969d2eef 100644 --- a/src/zm_monitor.h +++ b/src/zm_monitor.h @@ -22,6 +22,7 @@ #include "zm_define.h" #include "zm_camera.h" +#include "zm_analysis_thread.h" #include "zm_decoder_thread.h" #include "zm_event.h" #include "zm_fifo.h" @@ -376,9 +377,9 @@ protected: VideoStore *videoStore; PacketQueue packetqueue; packetqueue_iterator *analysis_it; + AnalysisThread *analysis_thread; packetqueue_iterator *decoder_it; - DecoderThread *decoder; - + DecoderThread *decoder; int n_zones; Zone **zones; From 0b4f04c4d5bb776725e77531e54d8b380154d82d Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 12:48:42 -0400 Subject: [PATCH 23/72] notify in clear before taking lock to increase chance of other threads exiting. Handle terminate case in get_packet --- src/zm_packetqueue.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/zm_packetqueue.cpp b/src/zm_packetqueue.cpp index 4cbf23910..5dbc4ea00 100644 --- a/src/zm_packetqueue.cpp +++ b/src/zm_packetqueue.cpp @@ -56,11 +56,11 @@ int PacketQueue::addStream() { PacketQueue::~PacketQueue() { clear(); - if ( packet_counts ) { + if (packet_counts) { delete[] packet_counts; packet_counts = nullptr; } - while ( !iterators.empty() ) { + while (!iterators.empty()) { packetqueue_iterator *it = iterators.front(); iterators.pop_front(); delete it; @@ -322,6 +322,7 @@ unsigned int PacketQueue::clear(unsigned int frames_to_keep, int stream_id) { void PacketQueue::clear() { deleting = true; + condition.notify_all(); std::unique_lock lck(mutex); @@ -457,7 +458,7 @@ int PacketQueue::packet_count(int stream_id) { // Returns a packet. Packet will be locked ZMLockedPacket *PacketQueue::get_packet(packetqueue_iterator *it) { - if ( deleting or zm_terminate ) + if (deleting or zm_terminate) return nullptr; Debug(4, "Locking in get_packet using it %p queue end? %d, packet %p", @@ -471,7 +472,7 @@ ZMLockedPacket *PacketQueue::get_packet(packetqueue_iterator *it) { Debug(2, "waiting. Queue size %d it == end? %d", pktQueue.size(), (*it == pktQueue.end())); condition.wait(lck); } - if ( deleting or zm_terminate ) + if (deleting or zm_terminate) return nullptr; Debug(4, "get_packet using it %p queue end? %d, packet %p", @@ -489,6 +490,10 @@ ZMLockedPacket *PacketQueue::get_packet(packetqueue_iterator *it) { ZM_DUMP_PACKET(p->packet, ""); condition.wait(lck); } + if (deleting or zm_terminate) { + // packet may have been deleted so we can't delete the lp FIXME + return nullptr; + } Debug(2, "Locked packet, unlocking packetqueue mutex"); return lp; } // end ZMLockedPacket *PacketQueue::get_packet(it) From 6ab8bee581908e4f36c989109125f69fe441f146 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 12:48:59 -0400 Subject: [PATCH 24/72] Increase debug level for fifo writing --- src/zm_fifo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/zm_fifo.cpp b/src/zm_fifo.cpp index e7ff145af..35867cfc9 100644 --- a/src/zm_fifo.cpp +++ b/src/zm_fifo.cpp @@ -104,7 +104,7 @@ bool Fifo::close() { bool Fifo::writePacket(ZMPacket &packet) { if (!(outfile or open())) return false; - Debug(1, "Writing header ZM %u %" PRId64, packet.packet.size, packet.pts); + Debug(2, "Writing header ZM %u %" PRId64, packet.packet.size, packet.pts); // Going to write a brief header if ( fprintf(outfile, "ZM %u %" PRId64 "\n", packet.packet.size, packet.pts) < 0 ) { if (errno != EAGAIN) { From feafaa29bfc08094f4a4c9b065599b0f594007c8 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 12:49:12 -0400 Subject: [PATCH 25/72] improve debug logging --- src/zm_rtsp_server_fifo_source.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/zm_rtsp_server_fifo_source.cpp b/src/zm_rtsp_server_fifo_source.cpp index cc3d443b1..1b3b4f73d 100644 --- a/src/zm_rtsp_server_fifo_source.cpp +++ b/src/zm_rtsp_server_fifo_source.cpp @@ -84,7 +84,7 @@ int ZoneMinderFifoSource::getNextFrame() { return -1; } - Debug(4, "%s bytes read %d bytes, buffer size %u", m_fifo.c_str(), bytes_read, m_buffer.size()); + Debug(1, "%s bytes read %d bytes, buffer size %u", m_fifo.c_str(), bytes_read, m_buffer.size()); while (m_buffer.size()) { unsigned int data_size = 0; int64_t pts; @@ -96,14 +96,14 @@ int ZoneMinderFifoSource::getNextFrame() { header_end = (unsigned char *)memchr(header_start, '\n', m_buffer.tail()-header_start); if (!header_end) { // Must not have enough data. So... keep all. - Debug(1, "Didn't find newline"); + Debug(1, "Didn't find newline buffer size is %d", m_buffer.size()); return 0; } unsigned int header_size = header_end-header_start; char *header = new char[header_size+1]; + strncpy(header, reinterpret_cast(header_start), header_size); header[header_size] = '\0'; - strncpy(header, reinterpret_cast(header_start), header_end-header_start); char *content_length_ptr = strchr(header, ' '); if (!content_length_ptr) { @@ -125,12 +125,12 @@ int ZoneMinderFifoSource::getNextFrame() { pts_ptr ++; data_size = atoi(content_length_ptr); pts = strtoll(pts_ptr, nullptr, 10); + Debug(4, "ZM Packet %s header_size %d packet size %u pts %" PRId64, header, header_size, data_size, pts); delete[] header; } else { - Debug(1, "ZM header not found %s.",m_buffer.head()); + Debug(1, "ZM header not found in %d of buffer:%s.", m_buffer.size(), m_buffer.head()); return 0; } - Debug(4, "ZM Packet size %u pts %" PRId64, data_size, pts); if (header_start != m_buffer) { Debug(4, "ZM Packet didn't start at beginning of buffer %u. %c%c", header_start-m_buffer.head(), m_buffer[0], m_buffer[1]); @@ -142,21 +142,21 @@ int ZoneMinderFifoSource::getNextFrame() { int bytes_needed = data_size - (m_buffer.size() - header_size); if (bytes_needed > 0) { Debug(4, "Need another %d bytes. Trying to read them", bytes_needed); - int bytes_read = m_buffer.read_into(m_fd, bytes_needed, {1,0}); + int bytes_read = m_buffer.read_into(m_fd, bytes_needed); if ( bytes_read != bytes_needed ) { - Debug(4, "Failed to read another %d bytes.", bytes_needed); + Debug(1, "Failed to read another %d bytes, got %d.", bytes_needed, bytes_read); return -1; } } - unsigned char *packet_start = m_buffer.head() + header_size; + m_buffer.consume(header_size); + unsigned char *packet_start = m_buffer.head(); size_t bytes_remaining = data_size; std::list< std::pair > framesList = this->splitFrames(packet_start, bytes_remaining); Debug(3, "Got %d frames, consuming %d bytes, remaining %d", framesList.size(), header_size + data_size, bytes_remaining); - m_buffer.consume(header_size + data_size); + m_buffer.consume(data_size); while (framesList.size()) { std::pair nal = framesList.front(); framesList.pop_front(); - PushFrame(nal.first, nal.second, pts); } } // end while m_buffer.size() From 2b34d09b846cfb3db16d3a34ad3db8961452acde Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 12:49:50 -0400 Subject: [PATCH 26/72] Move analysis_thread into Monitor. Don't do extra gettimeofday if no delays are set. Fix status update on terminate --- src/zmc.cpp | 59 +++++++++++++++++++++-------------------------------- 1 file changed, 23 insertions(+), 36 deletions(-) diff --git a/src/zmc.cpp b/src/zmc.cpp index 19c8891c7..6a9a37c14 100644 --- a/src/zmc.cpp +++ b/src/zmc.cpp @@ -54,7 +54,6 @@ possible, this should run at more or less constant speed. */ #include "zm.h" -#include "zm_analysis_thread.h" #include "zm_camera.h" #include "zm_db.h" #include "zm_define.h" @@ -272,8 +271,6 @@ int main(int argc, char *argv[]) { } // end foreach monitor if (zm_terminate) break; - std::vector> analysis_threads = std::vector>(); - int *capture_delays = new int[monitors.size()]; int *alarm_capture_delays = new int[monitors.size()]; struct timeval * last_capture_times = new struct timeval[monitors.size()]; @@ -284,12 +281,6 @@ int main(int argc, char *argv[]) { alarm_capture_delays[i] = monitors[i]->GetAlarmCaptureDelay(); Debug(2, "capture delay(%u mSecs 1000/capture_fps) alarm delay(%u)", capture_delays[i], alarm_capture_delays[i]); - - Monitor::Function function = monitors[0]->GetFunction(); - if (function != Monitor::MONITOR) { - Debug(1, "Starting an analysis thread for monitor (%d)", monitors[i]->Id()); - analysis_threads.emplace_back(ZM::make_unique(monitors[i])); - } } struct timeval now; @@ -320,29 +311,31 @@ int main(int argc, char *argv[]) { break; } - gettimeofday(&now, nullptr); // capture_delay is the amount of time we should sleep in useconds to achieve the desired framerate. int delay = (monitors[i]->GetState() == Monitor::ALARM) ? alarm_capture_delays[i] : capture_delays[i]; - if (delay && last_capture_times[i].tv_sec) { - // DT_PREC_3 means that the value will be in thousands of a second - DELTA_TIMEVAL(delta_time, now, last_capture_times[i], DT_PREC_6); + if (delay) { + gettimeofday(&now, nullptr); + if (last_capture_times[i].tv_sec) { + // DT_PREC_3 means that the value will be in thousands of a second + DELTA_TIMEVAL(delta_time, now, last_capture_times[i], DT_PREC_6); - // You have to add back in the previous sleep time - sleep_time = delay - (delta_time.delta - sleep_time); - Debug(4, "Sleep time is %d from now:%d.%d last:%d.%d delta %d delay: %d", - sleep_time, - now.tv_sec, now.tv_usec, - last_capture_times[i].tv_sec, last_capture_times[i].tv_usec, - delta_time.delta, - delay - ); - - if (sleep_time > 0) { - Debug(4, "usleeping (%d)", sleep_time); - usleep(sleep_time); - } - } // end if has a last_capture time - last_capture_times[i] = now; + // You have to add back in the previous sleep time + sleep_time = delay - (delta_time.delta - sleep_time); + Debug(4, "Sleep time is %d from now:%d.%d last:%d.%d delta %d delay: %d", + sleep_time, + now.tv_sec, now.tv_usec, + last_capture_times[i].tv_sec, last_capture_times[i].tv_usec, + delta_time.delta, + delay + ); + + if (sleep_time > 0) { + Debug(4, "usleeping (%d)", sleep_time); + usleep(sleep_time); + } + } // end if has a last_capture time + last_capture_times[i] = now; + } // end if delay } // end foreach n_monitors if ((result < 0) or zm_reload) { @@ -351,16 +344,10 @@ int main(int argc, char *argv[]) { } } // end while ! zm_terminate and connected - for (std::unique_ptr &analysis_thread: analysis_threads) - analysis_thread->Stop(); - for (size_t i = 0; i < monitors.size(); i++) { monitors[i]->Close(); } - // Killoff the analysis threads. Don't need them spinning while we try to reconnect - analysis_threads.clear(); - delete [] alarm_capture_delays; delete [] capture_delays; delete [] last_capture_times; @@ -385,7 +372,7 @@ int main(int argc, char *argv[]) { for (std::shared_ptr &monitor : monitors) { static char sql[ZM_SQL_SML_BUFSIZ]; snprintf(sql, sizeof(sql), - "INSERT INTO Monitor_Status (MonitorId,Status) VALUES (%d, 'Connected') ON DUPLICATE KEY UPDATE Status='NotRunning'", + "INSERT INTO Monitor_Status (MonitorId,Status) VALUES (%d, 'NotRunning') ON DUPLICATE KEY UPDATE Status='NotRunning'", monitor->Id()); zmDbDo(sql); } From c39ec5873bcd2b310a4d21daef4cf76d4c000f71 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 12:50:13 -0400 Subject: [PATCH 27/72] don't include zm_utils in decoder_thread --- src/zm_decoder_thread.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/zm_decoder_thread.cpp b/src/zm_decoder_thread.cpp index e8023e905..3d522830d 100644 --- a/src/zm_decoder_thread.cpp +++ b/src/zm_decoder_thread.cpp @@ -2,7 +2,6 @@ #include "zm_monitor.h" #include "zm_signal.h" -#include "zm_utils.h" //DecoderThread::DecoderThread(std::shared_ptr monitor) : DecoderThread::DecoderThread(Monitor * monitor) : From 284fe52b5f9da0c02d49da6849a1d710c477894b Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 12:57:45 -0400 Subject: [PATCH 28/72] fix double stop/free of decoder and analysis threads --- src/zm_monitor.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 38a2814a3..b8d2fa4fc 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -3180,9 +3180,11 @@ int Monitor::Close() { packetqueue.clear(); if (decoder) { delete decoder; + decoder = nullptr; } if (analysis_thread) { delete analysis_thread; + analysis_thread = nullptr; } std::lock_guard lck(event_mutex); if (event) { From ec8e0f5997ca6790888e52de275044ad3c393991 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 13:09:54 -0400 Subject: [PATCH 29/72] replace while(1) with while(not zm_terminate) so that these scripts exit cleanly --- scripts/zmcontrol.pl.in | 12 ++++++++++-- scripts/zmstats.pl.in | 11 +++++++++-- scripts/zmtrigger.pl.in | 11 +++++++++-- scripts/zmwatch.pl.in | 11 +++++++++-- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/scripts/zmcontrol.pl.in b/scripts/zmcontrol.pl.in index 474148df3..d1f50cf43 100644 --- a/scripts/zmcontrol.pl.in +++ b/scripts/zmcontrol.pl.in @@ -138,6 +138,14 @@ if ( $options{command} ) { Fatal("Can't load ZoneMinder::Control::$protocol\n$Module::Load::Conditional::ERROR"); } + my $zm_terminate = 0; + sub TermHandler { + Info('Received TERM, exiting'); + $zm_terminate = 1; + } + $SIG{TERM} = \&TermHandler; + $SIG{INT} = \&TermHandler; + Info("Control server $id/$protocol starting at " .strftime('%y/%m/%d %H:%M:%S', localtime()) ); @@ -166,7 +174,7 @@ if ( $options{command} ) { my $win = $rin; my $ein = $win; my $timeout = MAX_COMMAND_WAIT; - while ( 1 ) { + while (!$zm_terminate) { my $nfound = select(my $rout = $rin, undef, undef, $timeout); if ( $nfound > 0 ) { if ( vec($rout, fileno(SERVER), 1) ) { @@ -202,7 +210,7 @@ if ( $options{command} ) { } else { Debug('Select timed out'); } - } # end while forever + } # end while !$zm_terminate Info("Control server $id/$protocol exiting"); unlink($sock_file); $control->close(); diff --git a/scripts/zmstats.pl.in b/scripts/zmstats.pl.in index 81fdd3129..7cdf93b5a 100644 --- a/scripts/zmstats.pl.in +++ b/scripts/zmstats.pl.in @@ -28,13 +28,20 @@ delete @ENV{qw(IFS CDPATH ENV BASH_ENV)}; logInit(); logSetSignal(); +my $zm_terminate = 0; +sub TermHandler { + Info('Received TERM, exiting'); + $zm_terminate = 1; +} +$SIG{TERM} = \&TermHandler; +$SIG{INT} = \&TermHandler; Info('Stats Daemon starting in '.START_DELAY.' seconds'); sleep(START_DELAY); my $dbh = zmDbConnect(); -while( 1 ) { +while (!$zm_terminate) { while ( ! ( $dbh and $dbh->ping() ) ) { Info('Reconnecting to db'); if ( !($dbh = zmDbConnect()) ) { @@ -95,7 +102,7 @@ while( 1 ) { zmDbDo('DELETE FROM Sessions WHERE access < ? LIMIT 100', time - (60*60*24*7)); sleep($Config{ZM_STATS_UPDATE_INTERVAL}); -} # end while (1) +} # end while (!$zm_terminate) Info('Stats Daemon exiting'); exit(); diff --git a/scripts/zmtrigger.pl.in b/scripts/zmtrigger.pl.in index 5fc632165..c409bde33 100644 --- a/scripts/zmtrigger.pl.in +++ b/scripts/zmtrigger.pl.in @@ -87,6 +87,13 @@ delete @ENV{qw(IFS CDPATH ENV BASH_ENV)}; logInit(); logSetSignal(); +my $zm_terminate = 0; +sub TermHandler { + Info('Received TERM, exiting'); + $zm_terminate = 1; +} +$SIG{TERM} = \&TermHandler; +$SIG{INT} = \&TermHandler; Info('Trigger daemon starting'); @@ -118,7 +125,7 @@ my $win = $rin; my $ein = $win; my $timeout = SELECT_TIMEOUT; my %actions; -while (1) { +while (!$zm_terminate) { $rin = $base_rin; # Add the file descriptors of any spawned connections foreach my $fileno ( keys %spawned_connections ) { @@ -312,7 +319,7 @@ while (1) { # zmDbConnect will ping and reconnect if neccessary $dbh = zmDbConnect(); -} # end while ( 1 ) +} # end while (!$zm_terminate) Info('Trigger daemon exiting'); exit; diff --git a/scripts/zmwatch.pl.in b/scripts/zmwatch.pl.in index d82c74b99..f1fbd16b4 100644 --- a/scripts/zmwatch.pl.in +++ b/scripts/zmwatch.pl.in @@ -68,6 +68,13 @@ delete @ENV{qw(IFS CDPATH ENV BASH_ENV)}; logInit(); logSetSignal(); +my $zm_terminate = 0; +sub TermHandler { + Info('Received TERM, exiting'); + $zm_terminate = 1; +} +$SIG{TERM} = \&TermHandler; +$SIG{INT} = \&TermHandler; Info('Watchdog starting, pausing for '.START_DELAY.' seconds'); sleep(START_DELAY); @@ -77,7 +84,7 @@ my $sql = $Config{ZM_SERVER_ID} ? 'SELECT * FROM Monitors WHERE ServerId=?' : 'S my $sth = $dbh->prepare_cached($sql) or Fatal("Can't prepare '$sql': ".$dbh->errstr()); -while( 1 ) { +while (!$zm_terminate) { while ( ! ( $dbh and $dbh->ping() ) ) { if ( ! ( $dbh = zmDbConnect() ) ) { sleep($Config{ZM_WATCH_CHECK_INTERVAL}); @@ -192,7 +199,7 @@ while( 1 ) { } # end foreach monitor sleep($Config{ZM_WATCH_CHECK_INTERVAL}); -} # end while (1) +} # end while (!$zm_terminate) Info("Watchdog exiting"); exit(); From 079d3361a2beeb8fe317b90532c791feead5cec2 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 15:52:55 -0400 Subject: [PATCH 30/72] Rework to read content_length bytes at once. Micro-optimisation --- src/zm_remote_camera_http.cpp | 292 +++++++++++++++++----------------- 1 file changed, 146 insertions(+), 146 deletions(-) diff --git a/src/zm_remote_camera_http.cpp b/src/zm_remote_camera_http.cpp index d3ba95df0..3cfdc7412 100644 --- a/src/zm_remote_camera_http.cpp +++ b/src/zm_remote_camera_http.cpp @@ -202,7 +202,7 @@ int RemoteCameraHttp::SendRequest() { * > 0 is the # of bytes read. */ -int RemoteCameraHttp::ReadData( Buffer &buffer, unsigned int bytes_expected ) { +int RemoteCameraHttp::ReadData(Buffer &buffer, unsigned int bytes_expected) { fd_set rfds; FD_ZERO(&rfds); FD_SET(sd, &rfds); @@ -210,44 +210,44 @@ int RemoteCameraHttp::ReadData( Buffer &buffer, unsigned int bytes_expected ) { struct timeval temp_timeout = timeout; int n_found = select(sd+1, &rfds, nullptr, nullptr, &temp_timeout); - if( n_found == 0 ) { - Debug( 1, "Select timed out timeout was %d secs %d usecs", temp_timeout.tv_sec, temp_timeout.tv_usec ); + if (n_found == 0) { + Debug(1, "Select timed out timeout was %d secs %d usecs", temp_timeout.tv_sec, temp_timeout.tv_usec); int error = 0; - socklen_t len = sizeof (error); - int retval = getsockopt (sd, SOL_SOCKET, SO_ERROR, &error, &len); - if(retval != 0 ) { - Debug( 1, "error getting socket error code %s", strerror(retval) ); + socklen_t len = sizeof(error); + int retval = getsockopt(sd, SOL_SOCKET, SO_ERROR, &error, &len); + if (retval != 0) { + Debug(1, "error getting socket error code %s", strerror(retval)); } - if (error != 0 ) { + if (error != 0) { return -1; } // Why are we disconnecting? It's just a timeout, meaning that data wasn't available. //Disconnect(); return 0; - } else if ( n_found < 0) { + } else if (n_found < 0) { Error("Select error: %s", strerror(errno)); return -1; } unsigned int total_bytes_to_read = 0; - if ( bytes_expected ) { + if (bytes_expected) { total_bytes_to_read = bytes_expected; } else { - if ( ioctl( sd, FIONREAD, &total_bytes_to_read ) < 0 ) { - Error( "Can't ioctl(): %s", strerror(errno) ); + if (ioctl(sd, FIONREAD, &total_bytes_to_read) < 0) { + Error("Can't ioctl(): %s", strerror(errno) ); return -1; } - if ( total_bytes_to_read == 0 ) { - if ( mode == SINGLE_IMAGE ) { + if (total_bytes_to_read == 0) { + if (mode == SINGLE_IMAGE) { int error = 0; - socklen_t len = sizeof (error); - int retval = getsockopt( sd, SOL_SOCKET, SO_ERROR, &error, &len ); - if ( retval != 0 ) { - Debug( 1, "error getting socket error code %s", strerror(retval) ); + socklen_t len = sizeof(error); + int retval = getsockopt(sd, SOL_SOCKET, SO_ERROR, &error, &len); + if (retval != 0) { + Debug(1, "error getting socket error code %s", strerror(retval)); } - if ( error != 0 ) { + if (error != 0) { return -1; } // Case where we are grabbing a single jpg, but no content-length was given, so the expectation is that we read until close. @@ -261,38 +261,38 @@ int RemoteCameraHttp::ReadData( Buffer &buffer, unsigned int bytes_expected ) { } // There can be lots of bytes available. I've seen 4MB or more. This will vastly inflate our buffer size unnecessarily. - if ( total_bytes_to_read > ZM_NETWORK_BUFSIZ ) { + if (total_bytes_to_read > ZM_NETWORK_BUFSIZ) { total_bytes_to_read = ZM_NETWORK_BUFSIZ; - Debug(4, "Just getting 32K" ); + Debug(4, "Just getting 32K"); } else { - Debug(4, "Just getting %d", total_bytes_to_read ); + Debug(4, "Just getting %d", total_bytes_to_read); } } // end if bytes_expected or not - Debug( 4, "Expecting %d bytes", total_bytes_to_read ); + Debug(4, "Expecting %d bytes", total_bytes_to_read); int total_bytes_read = 0; do { - int bytes_read = buffer.read_into( sd, total_bytes_to_read ); - if ( bytes_read < 0 ) { - Error( "Read error: %s", strerror(errno) ); - return( -1 ); - } else if ( bytes_read == 0 ) { - Debug( 2, "Socket closed" ); + int bytes_read = buffer.read_into(sd, total_bytes_to_read); + if (bytes_read < 0) { + Error("Read error: %s", strerror(errno)); + return -1; + } else if (bytes_read == 0) { + Debug(2, "Socket closed"); //Disconnect(); // Disconnect is done outside of ReadData now. - return( -1 ); - } else if ( (unsigned int)bytes_read < total_bytes_to_read ) { - Error( "Incomplete read, expected %d, got %d", total_bytes_to_read, bytes_read ); - return( -1 ); + return -1; + } else if ((unsigned int)bytes_read < total_bytes_to_read) { + Debug(1, "Incomplete read, expected %d, got %d", total_bytes_to_read, bytes_read); + } else { + Debug(3, "Read %d bytes", bytes_read); } - Debug( 3, "Read %d bytes", bytes_read ); total_bytes_read += bytes_read; total_bytes_to_read -= bytes_read; - } while ( total_bytes_to_read ); + } while (total_bytes_to_read); - Debug(4, buffer); + Debug(4, "buffer: %s", buffer); return total_bytes_read; -} +} // end readData int RemoteCameraHttp::GetData() { time_t start_time = time(nullptr); @@ -497,36 +497,36 @@ int RemoteCameraHttp::GetResponse() { return( -1 ); } - if ( content_length ) { - while ( ((long)buffer.size() < content_length ) && ! zm_terminate ) { - Debug(3, "Need more data buffer %d < content length %d", buffer.size(), content_length ); - int bytes_read = GetData(); + if (content_length) { + while (((long)buffer.size() < content_length ) and !zm_terminate) { + Debug(3, "Need more data buffer %d < content length %d", buffer.size(), content_length); + int bytes_read = ReadData(buffer, content_length - buffer.size()); - if ( bytes_read < 0 ) { - Error( "Unable to read content" ); - return( -1 ); + if (bytes_read < 0) { + Error("Unable to read content"); + return -1; } bytes += bytes_read; } - Debug( 3, "Got end of image by length, content-length = %d", content_length ); + Debug(3, "Got end of image by length, content-length = %d", content_length); } else { - while ( !content_length ) { + while (!content_length) { buffer_len = GetData(); - if ( buffer_len < 0 ) { - Error( "Unable to read content" ); - return( -1 ); + if (buffer_len < 0) { + Error("Unable to read content"); + return -1; } bytes += buffer_len; static RegExpr *content_expr = 0; - if ( mode == MULTI_IMAGE ) { - if ( !content_expr ) { + if (mode == MULTI_IMAGE) { + if (!content_expr) { char content_pattern[256] = ""; - snprintf( content_pattern, sizeof(content_pattern), "^(.+?)(?:\r?\n)*(?:--)?%s\r?\n", content_boundary ); - content_expr = new RegExpr( content_pattern, PCRE_DOTALL ); + snprintf(content_pattern, sizeof(content_pattern), "^(.+?)(?:\r?\n)*(?:--)?%s\r?\n", content_boundary); + content_expr = new RegExpr(content_pattern, PCRE_DOTALL); } - if ( content_expr->Match( buffer, buffer.size() ) == 2 ) { + if (content_expr->Match( buffer, buffer.size()) == 2) { content_length = content_expr->MatchLength( 1 ); - Debug( 3, "Got end of image by pattern, content-length = %d", content_length ); + Debug(3, "Got end of image by pattern, content-length = %d", content_length); } } } @@ -623,7 +623,7 @@ int RemoteCameraHttp::GetResponse() { case HEADERCONT : { buffer_len = GetData(); - if ( buffer_len < 0 ) { + if (buffer_len < 0) { Error("Unable to read header"); return -1; } @@ -634,13 +634,13 @@ int RemoteCameraHttp::GetResponse() { int header_len = buffer.size(); bool all_headers = false; - while ( true ) { + while (true and !zm_terminate) { int crlf_len = memspn(header_ptr, "\r\n", header_len); - if ( n_headers ) { + if (n_headers) { if ( - (crlf_len == 2 && !strncmp(header_ptr, "\n\n", crlf_len )) + (crlf_len == 2 && !strncmp(header_ptr, "\n\n", crlf_len)) || - (crlf_len == 4 && !strncmp( header_ptr, "\r\n\r\n", crlf_len )) + (crlf_len == 4 && !strncmp(header_ptr, "\r\n\r\n", crlf_len)) ) { Debug(3, "Have double linefeed, done headers"); *header_ptr = '\0'; @@ -650,8 +650,8 @@ int RemoteCameraHttp::GetResponse() { break; } } - if ( crlf_len ) { - if ( header_len == crlf_len ) { + if (crlf_len) { + if (header_len == crlf_len) { break; } else { *header_ptr = '\0'; @@ -661,22 +661,22 @@ int RemoteCameraHttp::GetResponse() { } Debug(6, "%s", header_ptr); - if ( (crlf = mempbrk(header_ptr, "\r\n", header_len)) ) { + if ((crlf = mempbrk(header_ptr, "\r\n", header_len))) { //headers[n_headers++] = header_ptr; n_headers++; - if ( !http_header && (strncasecmp(header_ptr, http_match, http_match_len) == 0) ) { + if (!http_header && (strncasecmp(header_ptr, http_match, http_match_len) == 0)) { http_header = header_ptr+http_match_len; - Debug( 6, "Got http header '%s'", header_ptr ); + Debug(6, "Got http header '%s'", header_ptr); } else if ( !connection_header && (strncasecmp(header_ptr, connection_match, connection_match_len) == 0) ) { connection_header = header_ptr+connection_match_len; - Debug( 6, "Got connection header '%s'", header_ptr ); + Debug(6, "Got connection header '%s'", header_ptr); } else if ( !content_length_header && (strncasecmp(header_ptr, content_length_match, content_length_match_len) == 0) ) { content_length_header = header_ptr+content_length_match_len; - Debug( 6, "Got content length header '%s'", header_ptr ); + Debug(6, "Got content length header '%s'", header_ptr); } else if ( !authenticate_header && (strncasecmp(header_ptr, authenticate_match, authenticate_match_len) == 0) ) { authenticate_header = header_ptr; - Debug( 6, "Got authenticate header '%s'", header_ptr ); + Debug(6, "Got authenticate header '%s'", header_ptr); } else if ( !content_type_header && (strncasecmp(header_ptr, content_type_match, content_type_match_len) == 0) ) { content_type_header = header_ptr+content_type_match_len; Debug(6, "Got content type header '%s'", header_ptr); @@ -691,10 +691,10 @@ int RemoteCameraHttp::GetResponse() { } } // end while search for headers - if ( all_headers ) { + if (all_headers) { char *start_ptr, *end_ptr; - if ( !http_header ) { + if (!http_header) { Error("Unable to extract HTTP status from header"); return -1; } @@ -703,7 +703,7 @@ int RemoteCameraHttp::GetResponse() { end_ptr = start_ptr+strspn(start_ptr, "10."); // FIXME Why are we memsetting every time? Can we not do it once? - memset(http_version, 0, sizeof(http_version)); + //memset(http_version, 0, sizeof(http_version)); strncpy(http_version, start_ptr, end_ptr-start_ptr); start_ptr = end_ptr; @@ -718,12 +718,12 @@ int RemoteCameraHttp::GetResponse() { start_ptr += strspn(start_ptr, " "); strcpy(status_mesg, start_ptr); - if ( status == 401 ) { - if ( mNeedAuth ) { + if (status == 401) { + if (mNeedAuth) { Error("Failed authentication"); return -1; } - if ( !authenticate_header ) { + if (!authenticate_header) { Error("Failed authentication, but don't have an authentication header."); return -1; } @@ -732,7 +732,7 @@ int RemoteCameraHttp::GetResponse() { Debug(2, "Checking for digest auth in %s", authenticate_header); mAuthenticator->checkAuthResponse(Header); - if ( mAuthenticator->auth_method() == zm::AUTH_DIGEST ) { + if (mAuthenticator->auth_method() == zm::AUTH_DIGEST) { Debug(2, "Need Digest Authentication"); request = stringtf("GET %s HTTP/%s\r\n", path.c_str(), config.http_version); request += stringtf("User-Agent: %s/%s\r\n", config.http_ua, ZM_VERSION); @@ -747,26 +747,26 @@ int RemoteCameraHttp::GetResponse() { } else { Debug(2, "Need some other kind of Authentication"); } - } else if ( status < 200 || status > 299 ) { + } else if (status < 200 || status > 299) { Error("Invalid response status %s: %s", status_code, status_mesg); return -1; } Debug(3, "Got status '%d' (%s), http version %s", status, status_mesg, http_version); - if ( connection_header ) { + if (connection_header) { memset(connection_type, 0, sizeof(connection_type)); start_ptr = connection_header + strspn(connection_header, " "); // FIXME Should we not use strncpy? strcpy(connection_type, start_ptr); Debug(3, "Got connection '%s'", connection_type); } - if ( content_length_header ) { + if (content_length_header) { start_ptr = content_length_header + strspn(content_length_header, " "); - content_length = atoi( start_ptr ); + content_length = atoi(start_ptr); Debug(3, "Got content length '%d'", content_length); } - if ( content_type_header ) { - memset(content_type, 0, sizeof(content_type)); + if (content_type_header) { + //memset(content_type, 0, sizeof(content_type)); start_ptr = content_type_header + strspn(content_type_header, " "); if ( (end_ptr = strchr(start_ptr, ';')) ) { strncpy(content_type, start_ptr, end_ptr-start_ptr); @@ -774,7 +774,7 @@ int RemoteCameraHttp::GetResponse() { start_ptr = end_ptr + strspn(end_ptr, "; "); - if ( strncasecmp(start_ptr, boundary_match, boundary_match_len) == 0 ) { + if (strncasecmp(start_ptr, boundary_match, boundary_match_len) == 0) { start_ptr += boundary_match_len; start_ptr += strspn(start_ptr, "-"); content_boundary_len = sprintf(content_boundary, "--%s", start_ptr); @@ -788,24 +788,24 @@ int RemoteCameraHttp::GetResponse() { } } // end if content_type_header - if ( !strcasecmp(content_type, "image/jpeg") || !strcasecmp(content_type, "image/jpg") ) { + if (!strcasecmp(content_type, "image/jpeg") || !strcasecmp(content_type, "image/jpg")) { // Single image mode = SINGLE_IMAGE; format = JPEG; state = CONTENT; - } else if ( !strcasecmp(content_type, "image/x-rgb") ) { + } else if (!strcasecmp(content_type, "image/x-rgb")) { // Single image mode = SINGLE_IMAGE; format = X_RGB; state = CONTENT; - } else if ( !strcasecmp(content_type, "image/x-rgbz") ) { + } else if (!strcasecmp(content_type, "image/x-rgbz")) { // Single image mode = SINGLE_IMAGE; format = X_RGBZ; state = CONTENT; - } else if ( !strcasecmp(content_type, "multipart/x-mixed-replace") ) { + } else if (!strcasecmp(content_type, "multipart/x-mixed-replace")) { // Image stream, so start processing - if ( !content_boundary[0] ) { + if (!content_boundary[0]) { Error("No content boundary found in header '%s'", content_type_header); return -1; } @@ -817,8 +817,8 @@ int RemoteCameraHttp::GetResponse() { //// MPEG stream, coming soon! //} else { - Error( "Unrecognised content type '%s'", content_type ); - return( -1 ); + Error("Unrecognised content type '%s'", content_type); + return -1; } } else { Debug(3, "Unable to extract entire header from stream, continuing"); @@ -844,24 +844,24 @@ int RemoteCameraHttp::GetResponse() { int subheader_len = buffer.size(); bool all_headers = false; - while( true ) { - int crlf_len = memspn( subheader_ptr, "\r\n", subheader_len ); - if ( n_subheaders ) { - if ( (crlf_len == 2 && !strncmp( subheader_ptr, "\n\n", crlf_len )) || (crlf_len == 4 && !strncmp( subheader_ptr, "\r\n\r\n", crlf_len )) ) { + while (true and !zm_terminate) { + int crlf_len = memspn(subheader_ptr, "\r\n", subheader_len); + if (n_subheaders) { + if ( (crlf_len == 2 && !strncmp(subheader_ptr, "\n\n", crlf_len)) || (crlf_len == 4 && !strncmp( subheader_ptr, "\r\n\r\n", crlf_len )) ) { *subheader_ptr = '\0'; subheader_ptr += crlf_len; - subheader_len -= buffer.consume( subheader_ptr-(char *)buffer ); + subheader_len -= buffer.consume(subheader_ptr-(char *)buffer); all_headers = true; break; } } - if ( crlf_len ) { - if ( subheader_len == crlf_len ) { + if (crlf_len) { + if (subheader_len == crlf_len) { break; } else { *subheader_ptr = '\0'; subheader_ptr += crlf_len; - subheader_len -= buffer.consume( subheader_ptr-(char *)buffer ); + subheader_len -= buffer.consume(subheader_ptr-(char *)buffer); } } @@ -905,29 +905,29 @@ int RemoteCameraHttp::GetResponse() { } } - if ( all_headers && boundary_header ) { + if (all_headers && boundary_header) { char *start_ptr/*, *end_ptr*/; - Debug( 3, "Got boundary '%s'", boundary_header ); + Debug(3, "Got boundary '%s'", boundary_header); - if ( subcontent_length_header[0] ) { - start_ptr = subcontent_length_header + strspn( subcontent_length_header, " " ); - content_length = atoi( start_ptr ); - Debug( 3, "Got subcontent length '%d'", content_length ); + if (subcontent_length_header[0]) { + start_ptr = subcontent_length_header + strspn(subcontent_length_header, " "); + content_length = atoi(start_ptr); + Debug(3, "Got subcontent length '%d'", content_length); } - if ( subcontent_type_header[0] ) { - memset( content_type, 0, sizeof(content_type) ); - start_ptr = subcontent_type_header + strspn( subcontent_type_header, " " ); - strcpy( content_type, start_ptr ); - Debug( 3, "Got subcontent type '%s'", content_type ); + if (subcontent_type_header[0]) { + //memset(content_type, 0, sizeof(content_type)); + start_ptr = subcontent_type_header + strspn(subcontent_type_header, " "); + strcpy(content_type, start_ptr); + Debug(3, "Got subcontent type '%s'", content_type); } state = CONTENT; } else { - Debug( 3, "Unable to extract subheader from stream, retrying" ); + Debug(3, "Unable to extract subheader from stream, retrying"); buffer_len = GetData(); - if ( buffer_len < 0 ) { - Error( "Unable to read subheader" ); - return( -1 ); + if (buffer_len < 0) { + Error("Unable to read subheader"); + return -1; } bytes += buffer_len; state = SUBHEADERCONT; @@ -942,90 +942,90 @@ int RemoteCameraHttp::GetResponse() { *semicolon = '\0'; } - if ( !strcasecmp( content_type, "image/jpeg" ) || !strcasecmp( content_type, "image/jpg" ) ) { + if (!strcasecmp(content_type, "image/jpeg") || !strcasecmp(content_type, "image/jpg")) { format = JPEG; - } else if ( !strcasecmp( content_type, "image/x-rgb" ) ) { + } else if (!strcasecmp(content_type, "image/x-rgb")) { format = X_RGB; - } else if ( !strcasecmp( content_type, "image/x-rgbz" ) ) { + } else if (!strcasecmp(content_type, "image/x-rgbz")) { format = X_RGBZ; } else { - Error( "Found unsupported content type '%s'", content_type ); - return( -1 ); + Error("Found unsupported content type '%s'", content_type); + return -1; } // This is an early test for jpeg content, so we can bail early - if ( format == JPEG && buffer.size() >= 2 ) { - if ( buffer[0] != 0xff || buffer[1] != 0xd8 ) { - Error( "Found bogus jpeg header '%02x%02x'", buffer[0], buffer[1] ); - return( -1 ); + if (format == JPEG and buffer.size() >= 2) { + if (buffer[0] != 0xff or buffer[1] != 0xd8) { + Error("Found bogus jpeg header '%02x%02x'", buffer[0], buffer[1]); + return -1; } } - if ( content_length ) { - while ( ( (long)buffer.size() < content_length ) && ! zm_terminate ) { + if (content_length) { + while (((long)buffer.size() < content_length) and !zm_terminate) { Debug(4, "getting more data"); - int bytes_read = GetData(); - if ( bytes_read < 0 ) { + int bytes_read = ReadData(buffer, content_length-buffer.size()); + if (bytes_read < 0) { Error("Unable to read content"); return -1; } bytes += bytes_read; } - Debug( 3, "Got end of image by length, content-length = %d", content_length ); + Debug(3, "Got end of image by length, content-length = %d", content_length); } else { // Read until we find the end of image or the stream closes. - while ( !content_length && !zm_terminate ) { + while (!content_length && !zm_terminate) { Debug(4, "!content_length, ReadData"); - buffer_len = ReadData( buffer ); - if ( buffer_len < 0 ) { - Error( "Unable to read content" ); - return( -1 ); + buffer_len = ReadData(buffer); + if (buffer_len < 0) { + Error("Unable to read content"); + return -1; } bytes += buffer_len; int buffer_size = buffer.size(); - if ( buffer_len ) { + if (buffer_len) { // Got some data - if ( mode == MULTI_IMAGE ) { + if (mode == MULTI_IMAGE) { // Look for the boundary marker, determine content length using it's position - if ( char *start_ptr = (char *)memstr( (char *)buffer, "\r\n--", buffer_size ) ) { + if (char *start_ptr = (char *)memstr( (char *)buffer, "\r\n--", buffer_size)) { content_length = start_ptr - (char *)buffer; - Debug( 2, "Got end of image by pattern (crlf--), content-length = %d", content_length ); + Debug(2, "Got end of image by pattern (crlf--), content-length = %d", content_length); } else { - Debug( 2, "Did not find end of image by patten (crlf--) yet, content-length = %d", content_length ); + Debug(2, "Did not find end of image by patten (crlf--) yet, content-length = %d", content_length); } } // end if MULTI_IMAGE } else { content_length = buffer_size; - Debug( 2, "Got end of image by closure, content-length = %d", content_length ); - if ( mode == SINGLE_IMAGE ) { + Debug(2, "Got end of image by closure, content-length = %d", content_length); + if (mode == SINGLE_IMAGE) { char *end_ptr = (char *)buffer+buffer_size; // strip off any last line feeds - while( *end_ptr == '\r' || *end_ptr == '\n' ) { + while (*end_ptr == '\r' || *end_ptr == '\n') { content_length--; end_ptr--; } - if ( end_ptr != ((char *)buffer+buffer_size) ) { - Debug( 2, "Trimmed end of image, new content-length = %d", content_length ); + if (end_ptr != ((char *)buffer+buffer_size)) { + Debug(2, "Trimmed end of image, new content-length = %d", content_length); } } // end if SINGLE_IMAGE } // end if read some data } // end while ! content_length } // end if content_length - if ( mode == SINGLE_IMAGE ) { + if (mode == SINGLE_IMAGE) { state = HEADER; Disconnect(); } else { state = SUBHEADER; } - if ( format == JPEG && buffer.size() >= 2 ) { - if ( buffer[0] != 0xff || buffer[1] != 0xd8 ) { - Error( "Found bogus jpeg header '%02x%02x'", buffer[0], buffer[1] ); - return( -1 ); + if (format == JPEG && buffer.size() >= 2) { + if (buffer[0] != 0xff || buffer[1] != 0xd8) { + Error("Found bogus jpeg header '%02x%02x'", buffer[0], buffer[1]); + return -1; } } From fb28c6b365314ec35442bf3dc9f56add39ef4976 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 15:53:14 -0400 Subject: [PATCH 31/72] Fix login in Decode for non-ffmpeg monitors --- src/zm_monitor.cpp | 122 ++++++++++++++++++++++----------------------- 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index b8d2fa4fc..46b5014a5 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -2551,7 +2551,7 @@ int Monitor::Capture() { } } else { captureResult = camera->Capture(*packet); - Debug(4, "Back from capture result=%d image %d", captureResult, image_count); + Debug(4, "Back from capture result=%d image count %d", captureResult, image_count); if ( captureResult < 0 ) { Debug(2, "failed capture"); @@ -2749,85 +2749,85 @@ bool Monitor::Decode() { if (!packet_lock) return false; ZMPacket *packet = packet_lock->packet_; packetqueue.increment_it(decoder_it); - if (packet->image or (packet->codec_type != AVMEDIA_TYPE_VIDEO)) { + if (packet->codec_type != AVMEDIA_TYPE_VIDEO) { delete packet_lock; return true; // Don't need decode } int ret = 0; - if (packet->packet.size and !packet->in_frame) { + if ((!packet->image) and packet->packet.size and !packet->in_frame) { // Allocate the image first so that it can be used by hwaccel // We don't actually care about camera colours, pixel order etc. We care about the desired settings // //capture_image = packet->image = new Image(width, height, camera->Colours(), camera->SubpixelOrder()); ret = packet->decode(camera->getVideoCodecContext()); - } else { - Debug(1, "No packet.size(%d) or packet->in_frame(%p). Not decoding", packet->packet.size, packet->in_frame); + if (ret > 0) { + if (packet->in_frame and !packet->image) { + packet->image = new Image(camera_width, camera_height, camera->Colours(), camera->SubpixelOrder()); + packet->get_image(); + } + } else { + Debug(1, "No packet.size(%d) or packet->in_frame(%p). Not decoding", packet->packet.size, packet->in_frame); + } } Image* capture_image = nullptr; unsigned int index = image_count % image_buffer_count; - if (ret > 0) { - if (packet->in_frame and !packet->image) { - packet->image = new Image(camera_width, camera_height, camera->Colours(), camera->SubpixelOrder()); - packet->get_image(); + + if (packet->image) { + capture_image = packet->image; + + /* Deinterlacing */ + if ( deinterlacing_value ) { + if ( deinterlacing_value == 1 ) { + capture_image->Deinterlace_Discard(); + } else if ( deinterlacing_value == 2 ) { + capture_image->Deinterlace_Linear(); + } else if ( deinterlacing_value == 3 ) { + capture_image->Deinterlace_Blend(); + } else if ( deinterlacing_value == 4 ) { + capture_image->Deinterlace_4Field(next_buffer.image, (deinterlacing>>8)&0xff); + } else if ( deinterlacing_value == 5 ) { + capture_image->Deinterlace_Blend_CustomRatio((deinterlacing>>8)&0xff); + } } - if (packet->image) { - capture_image = packet->image; - - /* Deinterlacing */ - if ( deinterlacing_value ) { - if ( deinterlacing_value == 1 ) { - capture_image->Deinterlace_Discard(); - } else if ( deinterlacing_value == 2 ) { - capture_image->Deinterlace_Linear(); - } else if ( deinterlacing_value == 3 ) { - capture_image->Deinterlace_Blend(); - } else if ( deinterlacing_value == 4 ) { - capture_image->Deinterlace_4Field(next_buffer.image, (deinterlacing>>8)&0xff); - } else if ( deinterlacing_value == 5 ) { - capture_image->Deinterlace_Blend_CustomRatio((deinterlacing>>8)&0xff); - } + if ( orientation != ROTATE_0 ) { + Debug(2, "Doing rotation"); + switch ( orientation ) { + case ROTATE_0 : + // No action required + break; + case ROTATE_90 : + case ROTATE_180 : + case ROTATE_270 : + capture_image->Rotate((orientation-1)*90); + break; + case FLIP_HORI : + case FLIP_VERT : + capture_image->Flip(orientation==FLIP_HORI); + break; } + } // end if have rotation - if ( orientation != ROTATE_0 ) { - Debug(2, "Doing rotation"); - switch ( orientation ) { - case ROTATE_0 : - // No action required - break; - case ROTATE_90 : - case ROTATE_180 : - case ROTATE_270 : - capture_image->Rotate((orientation-1)*90); - break; - case FLIP_HORI : - case FLIP_VERT : - capture_image->Flip(orientation==FLIP_HORI); - break; - } - } // end if have rotation + if (privacy_bitmask) { + Debug(1, "Applying privacy"); + capture_image->MaskPrivacy(privacy_bitmask); + } - if (privacy_bitmask) { - Debug(1, "Applying privacy"); - capture_image->MaskPrivacy(privacy_bitmask); - } + if (config.timestamp_on_capture) { + Debug(1, "Timestampprivacy"); + TimestampImage(packet->image, packet->timestamp); + } - if (config.timestamp_on_capture) { - Debug(1, "Timestampprivacy"); - TimestampImage(packet->image, packet->timestamp); - } - - if (!ref_image.Buffer()) { - // First image, so assign it to ref image - Debug(1, "Assigning ref image %dx%d size: %d", width, height, camera->ImageSize()); - ref_image.Assign(width, height, camera->Colours(), camera->SubpixelOrder(), - packet->image->Buffer(), camera->ImageSize()); - } - image_buffer[index].image->Assign(*(packet->image)); - *(image_buffer[index].timestamp) = *(packet->timestamp); - } // end if have image - } // end if did decoding + if (!ref_image.Buffer()) { + // First image, so assign it to ref image + Debug(1, "Assigning ref image %dx%d size: %d", width, height, camera->ImageSize()); + ref_image.Assign(width, height, camera->Colours(), camera->SubpixelOrder(), + packet->image->Buffer(), camera->ImageSize()); + } + image_buffer[index].image->Assign(*(packet->image)); + *(image_buffer[index].timestamp) = *(packet->timestamp); + } // end if have image packet->decoded = true; delete packet_lock; From ccb1bc1a7dbafa38b9693a5b31f6d96f9852940a Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 16:11:31 -0400 Subject: [PATCH 32/72] Have to wait until we are finished with the packet before unlocking. --- src/zm_monitor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 46b5014a5..bb7eda5e4 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -2829,11 +2829,11 @@ bool Monitor::Decode() { *(image_buffer[index].timestamp) = *(packet->timestamp); } // end if have image packet->decoded = true; - delete packet_lock; shared_data->signal = ( capture_image and signal_check_points ) ? CheckSignal(capture_image) : true; shared_data->last_write_index = index; shared_data->last_write_time = packet->timestamp->tv_sec; + delete packet_lock; return true; } // end bool Monitor::Decode() From 1b876f24f9d91ce86167beeee3f73bc0f34091f1 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 17:14:46 -0400 Subject: [PATCH 33/72] Must have Id as well in order to know which monitor to control --- web/api/app/Controller/MonitorsController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/api/app/Controller/MonitorsController.php b/web/api/app/Controller/MonitorsController.php index 2a6374222..184e17bb3 100644 --- a/web/api/app/Controller/MonitorsController.php +++ b/web/api/app/Controller/MonitorsController.php @@ -344,7 +344,7 @@ class MonitorsController extends AppController { public function daemonControl($id, $command, $daemon=null) { // Need to see if it is local or remote $monitor = $this->Monitor->find('first', array( - 'fields' => array('Type', 'Function', 'Device', 'ServerId'), + 'fields' => array('Id', 'Type', 'Function', 'Device', 'ServerId'), 'conditions' => array('Id' => $id) )); $monitor = $monitor['Monitor']; From f4506a8f350f7fbd7e4b785735d5a2a04e9d62ff Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Mar 2021 23:41:00 -0400 Subject: [PATCH 34/72] We always need an analysis thread. --- src/zm_monitor.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index bb7eda5e4..b17bf97b1 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -3157,12 +3157,10 @@ int Monitor::PrimeCapture() { if (!decoder_it) decoder_it = packetqueue.get_video_it(false); if (!decoder) decoder = new DecoderThread(this); } - if (function != MONITOR) { - if (!analysis_it) analysis_it = packetqueue.get_video_it(false); - if (!analysis_thread) { - Debug(1, "Starting an analysis thread for monitor (%d)", id); - analysis_thread = new AnalysisThread(this); - } + if (!analysis_it) analysis_it = packetqueue.get_video_it(false); + if (!analysis_thread) { + Debug(1, "Starting an analysis thread for monitor (%d)", id); + analysis_thread = new AnalysisThread(this); } } else { From edefbfcad68bdf1c8ece4da0ac04f2e0ffd123bc Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Mar 2021 09:24:27 -0400 Subject: [PATCH 35/72] Remove assumptions about Analysis being about motion detection. Fixes mem leaks in Monitor mode --- src/zm_monitor.cpp | 6 ++---- src/zm_monitor.h | 4 ---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index b17bf97b1..8fc0a507a 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -1797,10 +1797,6 @@ void Monitor::UpdateAnalysisFPS() { // If there isn't then we keep pre-event + alarm frames. = pre_event_count bool Monitor::Analyse() { - if (!Enabled()) { - Warning("Shouldn't be doing Analyse when not Enabled"); - return false; - } // if have event, send frames until we find a video packet, at which point do analysis. Adaptive skip should only affect which frames we do analysis on. @@ -3153,10 +3149,12 @@ int Monitor::PrimeCapture() { audio_fifo = new Fifo(shared_data->audio_fifo_path, true); } } // end if rtsp_server + if (decoding_enabled) { if (!decoder_it) decoder_it = packetqueue.get_video_it(false); if (!decoder) decoder = new DecoderThread(this); } + if (!analysis_it) analysis_it = packetqueue.get_video_it(false); if (!analysis_thread) { Debug(1, "Starting an analysis thread for monitor (%d)", id); diff --git a/src/zm_monitor.h b/src/zm_monitor.h index 7969d2eef..f12910597 100644 --- a/src/zm_monitor.h +++ b/src/zm_monitor.h @@ -445,10 +445,6 @@ public: } inline const char *EventPrefix() const { return event_prefix; } inline bool Ready() const { - if ( function <= MONITOR ) { - Error("Should not be calling Ready if the function doesn't include motion detection"); - return false; - } if ( image_count >= ready_count ) { return true; } From 4cb38a119e16e06c943df113d1606450ee658198 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Mar 2021 14:09:15 -0400 Subject: [PATCH 36/72] Fix saving Filters and other objects. Apparently comparing 0 to NOW() doesn't work. --- web/includes/Object.php | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/web/includes/Object.php b/web/includes/Object.php index 9c9a3c095..da104e5db 100644 --- a/web/includes/Object.php +++ b/web/includes/Object.php @@ -302,14 +302,20 @@ class ZM_Object { # Set defaults. Note that we only replace "" with null, not other values # because for example if we want to clear TimestampFormat, we clear it, but the default is a string value foreach ( $this->defaults as $field => $default ) { - if ( (!property_exists($this, $field)) or ($this->{$field} === '') ) { - if ( is_array($default) ) { + if (!property_exists($this, $field)) { + if (is_array($default)) { $this->{$field} = $default['default']; - } else if ( $default == null ) { + } else { + $this->{$field} = $default; + } + } else if ($this->{$field} === '') { + if (is_array($default)) { + $this->{$field} = $default['default']; + } else if ($default == null) { $this->{$field} = $default; } } - } + } # end foreach default $fields = array_filter( $this->defaults, @@ -339,10 +345,10 @@ class ZM_Object { ') VALUES ('. implode(', ', array_map(function($field){return $this->$field() == 'NOW()' ? 'NOW()' : '?';}, $fields)).')'; - $values = array_values(array_map( - function($field){return $this->$field();}, - array_filter($fields, function($field){ return $this->$field() != 'NOW()';}) - )); + # For some reason comparing 0 to 'NOW()' returns false; So we do this. + $filtered = array_filter($fields, function($field){ return ( (!$this->$field()) or ($this->$field() != 'NOW()'));}); + $mapped = array_map(function($field){return $this->$field();}, $filtered); + $values = array_values($mapped); if ( dbQuery($sql, $values) ) { $this->{'Id'} = dbInsertId(); return true; From 7a4c34ec7e7c5d4a11f375553594e89af2d0cb92 Mon Sep 17 00:00:00 2001 From: Peter Keresztes Schmidt Date: Thu, 18 Mar 2021 12:59:14 +0100 Subject: [PATCH 37/72] RemoteCameraHttp: Fix a log message --- src/zm_remote_camera_http.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/zm_remote_camera_http.cpp b/src/zm_remote_camera_http.cpp index 3cfdc7412..010fc4fbc 100644 --- a/src/zm_remote_camera_http.cpp +++ b/src/zm_remote_camera_http.cpp @@ -289,7 +289,7 @@ int RemoteCameraHttp::ReadData(Buffer &buffer, unsigned int bytes_expected) { total_bytes_to_read -= bytes_read; } while (total_bytes_to_read); - Debug(4, "buffer: %s", buffer); + Debug(4, "buffer size: %d", static_cast(buffer)); return total_bytes_read; } // end readData From d1bbfdaf6b3067985a6669dedb9906fd4d1b8cb9 Mon Sep 17 00:00:00 2001 From: Peter Keresztes Schmidt Date: Thu, 18 Mar 2021 13:00:36 +0100 Subject: [PATCH 38/72] Build: Enable -Wconditionally-supported on GCC We want to have warnings if we use some implementation-specific features of GCC to be able to keep compatibility with clang. --- cmake/compiler/gcc/settings.cmake | 1 + 1 file changed, 1 insertion(+) diff --git a/cmake/compiler/gcc/settings.cmake b/cmake/compiler/gcc/settings.cmake index 87ff6e939..497db5a17 100644 --- a/cmake/compiler/gcc/settings.cmake +++ b/cmake/compiler/gcc/settings.cmake @@ -1,6 +1,7 @@ target_compile_options(zm-warning-interface INTERFACE -Wall + -Wconditionally-supported -Wextra -Wformat-security -Wno-cast-function-type From 7889b515d306f3ddaceac8a166463e6e2594df8f Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Mar 2021 14:46:01 -0400 Subject: [PATCH 39/72] Add groovy and hirsuit builds --- utils/packpack/startpackpack.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/packpack/startpackpack.sh b/utils/packpack/startpackpack.sh index d464089d3..75aee96f3 100755 --- a/utils/packpack/startpackpack.sh +++ b/utils/packpack/startpackpack.sh @@ -347,7 +347,7 @@ elif [ "${OS}" == "debian" ] || [ "${OS}" == "ubuntu" ] || [ "${OS}" == "raspbia setdebpkgname movecrud - if [ "${DIST}" == "focal" ] || [ "${DIST}" == "buster" ]; then + if [ "${DIST}" == "focal" ] || "${DIST}" == "groovy" || "${DIST}" == "hirsuit" || [ "${DIST}" == "buster" ]; then ln -sfT distros/ubuntu2004 debian elif [ "${DIST}" == "beowulf" ]; then ln -sfT distros/beowulf debian From e39c293a772ac62e1e99b0202113c7a7b4d9b1da Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Mar 2021 15:58:45 -0400 Subject: [PATCH 40/72] fix eslint --- web/skins/classic/views/js/snapshot.js | 1 - 1 file changed, 1 deletion(-) diff --git a/web/skins/classic/views/js/snapshot.js b/web/skins/classic/views/js/snapshot.js index 9db096277..9ddefe0f7 100644 --- a/web/skins/classic/views/js/snapshot.js +++ b/web/skins/classic/views/js/snapshot.js @@ -27,7 +27,6 @@ function manageDelConfirmModalBtns() { } function initPage() { - // enable or disable buttons based on current selection and user rights /* renameBtn.prop('disabled', !canEdit.Events); From dd714a018fc7ac50adce3ad22440f1c56d417917 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Mar 2021 16:18:21 -0400 Subject: [PATCH 41/72] add populating RtspServer --- utils/packpack/startpackpack.sh | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/utils/packpack/startpackpack.sh b/utils/packpack/startpackpack.sh index 75aee96f3..e18dbd127 100755 --- a/utils/packpack/startpackpack.sh +++ b/utils/packpack/startpackpack.sh @@ -117,6 +117,17 @@ commonprep () { exit 1 fi fi + + if [ -e "build/RtspServer" ]; then + echo "Found existing RtspServer..." + else + echo "Cloning RtspServer ..." + git clone https://github.com/ZoneMinder/RtspServer + if [ $? -ne 0 ]; then + echo "ERROR: CakePHP-Enum-Behavior tarball retreival failed..." + exit 1 + fi + fi } # Uncompress the submodule tarballs and move them into place @@ -137,6 +148,13 @@ movecrud () { rmdir web/api/app/Plugin/CakePHP-Enum-Behavior mv -f CakePHP-Enum-Behavior-${CEBVER} web/api/app/Plugin/CakePHP-Enum-Behavior fi + if [ -e "dep/RtspServer/CMakeLists.txt" ]; then + echo "RtspServer already installed..." + else + echo "Copying RtspServer..." + rmdir dep/RtspServer + cp -Rpd build/RtspServer dep/RtspServer + fi } # previsouly part of installzm.sh From 86e7c440872a3ed416b9338b39a126eef815962e Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Mar 2021 16:20:19 -0400 Subject: [PATCH 42/72] add missing [] --- utils/packpack/startpackpack.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/packpack/startpackpack.sh b/utils/packpack/startpackpack.sh index e18dbd127..950f5fcf2 100755 --- a/utils/packpack/startpackpack.sh +++ b/utils/packpack/startpackpack.sh @@ -365,7 +365,7 @@ elif [ "${OS}" == "debian" ] || [ "${OS}" == "ubuntu" ] || [ "${OS}" == "raspbia setdebpkgname movecrud - if [ "${DIST}" == "focal" ] || "${DIST}" == "groovy" || "${DIST}" == "hirsuit" || [ "${DIST}" == "buster" ]; then + if [ "${DIST}" == "focal" ] || [ "${DIST}" == "groovy" ] || [ "${DIST}" == "hirsuit" ] || [ "${DIST}" == "buster" ]; then ln -sfT distros/ubuntu2004 debian elif [ "${DIST}" == "beowulf" ]; then ln -sfT distros/beowulf debian From 70e61740d3438901d7128707765bd7a1207b995d Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Mar 2021 16:41:12 -0400 Subject: [PATCH 43/72] Fix eslint --- web/skins/classic/views/js/montage.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/skins/classic/views/js/montage.js b/web/skins/classic/views/js/montage.js index 1a3c901d4..a1260effc 100644 --- a/web/skins/classic/views/js/montage.js +++ b/web/skins/classic/views/js/montage.js @@ -271,7 +271,9 @@ function reloadWebSite(ndx) { } function takeSnapshot() { - monitor_ids = monitorData.map( monitor=> { return 'monitor_ids[]='+monitor.id; }); + monitor_ids = monitorData.map((monitor)=>{ + return 'monitor_ids[]='+monitor.id; + }); window.location = '?view=snapshot&action=create&'+monitor_ids.join('&'); } From 53133ba0519ece3ff637ee454af1afa2f4f7deb8 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Mar 2021 17:43:31 -0400 Subject: [PATCH 44/72] add token as an alternative to jwt_token --- src/zm_rtsp_server_authenticator.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/zm_rtsp_server_authenticator.h b/src/zm_rtsp_server_authenticator.h index 55a7784e7..9908327d3 100644 --- a/src/zm_rtsp_server_authenticator.h +++ b/src/zm_rtsp_server_authenticator.h @@ -46,6 +46,9 @@ class ZM_RtspServer_Authenticator : public xop::Authenticator { if (query.has("jwt_token")) { const QueryParameter *jwt_token = query.get("jwt_token"); user = zmLoadTokenUser(jwt_token->firstValue().c_str(), false); + } else if (query.has("token")) { + const QueryParameter *jwt_token = query.get("token"); + user = zmLoadTokenUser(jwt_token->firstValue().c_str(), false); } else if (strcmp(config.auth_relay, "none") == 0) { if (query.has("username")) { std::string username = query.get("username")->firstValue(); From 65ed17ab76c15ef70b48eac2f8d2895d28f1b8e8 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Mar 2021 17:46:00 -0400 Subject: [PATCH 45/72] fix dest of clone of RtspServer --- utils/packpack/startpackpack.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/packpack/startpackpack.sh b/utils/packpack/startpackpack.sh index 950f5fcf2..ef370aea5 100755 --- a/utils/packpack/startpackpack.sh +++ b/utils/packpack/startpackpack.sh @@ -122,7 +122,7 @@ commonprep () { echo "Found existing RtspServer..." else echo "Cloning RtspServer ..." - git clone https://github.com/ZoneMinder/RtspServer + git clone https://github.com/ZoneMinder/RtspServer build/RtspServer if [ $? -ne 0 ]; then echo "ERROR: CakePHP-Enum-Behavior tarball retreival failed..." exit 1 From b71db319a029ad775a2c5881b3d0b31a81a56ad3 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Mar 2021 17:56:39 -0400 Subject: [PATCH 46/72] need to rm -r RtspServer --- utils/packpack/startpackpack.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/packpack/startpackpack.sh b/utils/packpack/startpackpack.sh index ef370aea5..0ba1d2d07 100755 --- a/utils/packpack/startpackpack.sh +++ b/utils/packpack/startpackpack.sh @@ -124,7 +124,7 @@ commonprep () { echo "Cloning RtspServer ..." git clone https://github.com/ZoneMinder/RtspServer build/RtspServer if [ $? -ne 0 ]; then - echo "ERROR: CakePHP-Enum-Behavior tarball retreival failed..." + echo "ERROR: RtspServer clone failed..." exit 1 fi fi @@ -152,7 +152,7 @@ movecrud () { echo "RtspServer already installed..." else echo "Copying RtspServer..." - rmdir dep/RtspServer + rm -r dep/RtspServer cp -Rpd build/RtspServer dep/RtspServer fi } From 33f98a0d40e5867984526a6156bc6f1ca6b13098 Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sat, 20 Mar 2021 20:02:52 +0100 Subject: [PATCH 47/72] Issue #3197 Add RECORD to Event_Close_Mode time --- src/zm_monitor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 8fc0a507a..9fb398bdb 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -1979,7 +1979,7 @@ bool Monitor::Analyse() { Debug(2, "Have event %" PRIu64 " in mocord", event->Id()); if (section_length && ( ( timestamp->tv_sec - video_store_data->recording.tv_sec ) >= section_length ) - && ( (function == MOCORD && (event_close_mode != CLOSE_TIME)) || ! ( timestamp->tv_sec % section_length ) ) + && ( ( ( (function == MOCORD) || (function == RECORD) ) && (event_close_mode != CLOSE_TIME)) || ! ( timestamp->tv_sec % section_length ) ) ) { Info("%s: %03d - Closing event %" PRIu64 ", section end forced %d - %d = %d >= %d", From db7e9edcab43b5adae6550f2a590a7cc471f24ba Mon Sep 17 00:00:00 2001 From: SirLouen Date: Sun, 21 Mar 2021 00:00:36 +0100 Subject: [PATCH 48/72] Issue #3197 Improvement --- src/zm_monitor.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 9fb398bdb..58977dc2c 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -1977,11 +1977,10 @@ bool Monitor::Analyse() { if (event) { Debug(2, "Have event %" PRIu64 " in mocord", event->Id()); - if (section_length - && ( ( timestamp->tv_sec - video_store_data->recording.tv_sec ) >= section_length ) - && ( ( ( (function == MOCORD) || (function == RECORD) ) && (event_close_mode != CLOSE_TIME)) || ! ( timestamp->tv_sec % section_length ) ) - ) { - + if (section_length && ( ( timestamp->tv_sec - video_store_data->recording.tv_sec ) >= section_length ) + && ( ( (function == MOCORD) && (event_close_mode != CLOSE_TIME) ) || ( (function == RECORD) && (event_close_mode == CLOSE_TIME) ) + || ! ( timestamp->tv_sec % section_length ) ) ) + { Info("%s: %03d - Closing event %" PRIu64 ", section end forced %d - %d = %d >= %d", name, image_count, event->Id(), timestamp->tv_sec, video_store_data->recording.tv_sec, From a042e4bf77557bd5f59b9bbed6849e7b37070cb0 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sat, 20 Mar 2021 19:27:18 -0400 Subject: [PATCH 49/72] spacing --- src/zm_monitor.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 8fc0a507a..1c47f59e7 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -1947,8 +1947,8 @@ bool Monitor::Analyse() { motion_frame_skip, capture_fps, analysis_fps_limit); } - if ( !(analysis_image_count % (motion_frame_skip+1)) ) { - if ( snap->image ) { + if (!(analysis_image_count % (motion_frame_skip+1))) { + if (snap->image) { // Get new score. motion_score = DetectMotion(*(snap->image), zoneSet); @@ -1971,17 +1971,14 @@ bool Monitor::Analyse() { } // end if motion_score } // end if active and doing motion detection - if (function == RECORD or function == MOCORD) { // If doing record, check to see if we need to close the event or not. - if (event) { Debug(2, "Have event %" PRIu64 " in mocord", event->Id()); if (section_length && ( ( timestamp->tv_sec - video_store_data->recording.tv_sec ) >= section_length ) && ( (function == MOCORD && (event_close_mode != CLOSE_TIME)) || ! ( timestamp->tv_sec % section_length ) ) ) { - Info("%s: %03d - Closing event %" PRIu64 ", section end forced %d - %d = %d >= %d", name, image_count, event->Id(), timestamp->tv_sec, video_store_data->recording.tv_sec, From 68f9c7c9e67a34a123e20d4ef11b09e17a53352a Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sat, 20 Mar 2021 19:27:53 -0400 Subject: [PATCH 50/72] introduce a _last_error member to the object for reporting errors saving. --- web/includes/Object.php | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/web/includes/Object.php b/web/includes/Object.php index da104e5db..4ed4fadf0 100644 --- a/web/includes/Object.php +++ b/web/includes/Object.php @@ -5,6 +5,7 @@ require_once('database.php'); $object_cache = array(); class ZM_Object { + protected $_last_error; public function __construct($IdOrRow = NULL) { $class = get_class($this); @@ -207,8 +208,8 @@ class ZM_Object { public function changes($new_values, $defaults=null) { $changes = array(); - if ( $defaults ) { - foreach ( $defaults as $field => $type ) { + if ($defaults) { + foreach ($defaults as $field => $type) { if ( isset($new_values[$field]) ) { # Will have already been handled above continue; @@ -247,47 +248,40 @@ class ZM_Object { } else if ( $this->$field() != $value ) { $changes[$field] = $value; } - } else if ( property_exists($this, $field) ) { + } else if (property_exists($this, $field)) { $type = (array_key_exists($field, $this->defaults) && is_array($this->defaults[$field])) ? $this->defaults[$field]['type'] : 'scalar'; - if ( $type == 'set' ) { + if ($type == 'set') { $old_value = is_array($this->$field) ? $this->$field : ($this->$field ? explode(',', $this->$field) : array()); $new_value = is_array($value) ? $value : ($value ? explode(',', $value) : array()); $diff = array_recursive_diff($old_value, $new_value); - if ( count($diff) ) { - $changes[$field] = $new_value; - } + if (count($diff)) $changes[$field] = $new_value; # Input might be a command separated string, or an array } else { - if ( array_key_exists($field, $this->defaults) && is_array($this->defaults[$field]) && isset($this->defaults[$field]['filter_regexp']) ) { - if ( is_array($this->defaults[$field]['filter_regexp']) ) { - foreach ( $this->defaults[$field]['filter_regexp'] as $regexp ) { + if (array_key_exists($field, $this->defaults) && is_array($this->defaults[$field]) && isset($this->defaults[$field]['filter_regexp'])) { + if (is_array($this->defaults[$field]['filter_regexp'])) { + foreach ($this->defaults[$field]['filter_regexp'] as $regexp) { $value = preg_replace($regexp, '', trim($value)); } } else { $value = preg_replace($this->defaults[$field]['filter_regexp'], '', trim($value)); } } - if ( $this->{$field} != $value ) { - $changes[$field] = $value; - } + if ($this->{$field} != $value) $changes[$field] = $value; } - } else if ( array_key_exists($field, $this->defaults) ) { - if ( is_array($this->defaults[$field]) and isset($this->defaults[$field]['default']) ) { + } else if (array_key_exists($field, $this->defaults)) { + if (is_array($this->defaults[$field]) and isset($this->defaults[$field]['default'])) { $default = $this->defaults[$field]['default']; } else { $default = $this->defaults[$field]; } - if ( $default != $value ) { - $changes[$field] = $value; - } + if ($default != $value) $changes[$field] = $value; } } # end foreach newvalue - return $changes; } # end public function changes @@ -335,8 +329,7 @@ class ZM_Object { $sql = 'UPDATE `'.$table.'` SET '.implode(', ', array_map(function($field) {return '`'.$field.'`=?';}, $fields)).' WHERE Id=?'; $values = array_map(function($field){ return $this->{$field};}, $fields); $values[] = $this->{'Id'}; - if ( dbQuery($sql, $values) ) - return true; + if (dbQuery($sql, $values)) return true; } else { unset($fields['Id']); @@ -349,11 +342,12 @@ class ZM_Object { $filtered = array_filter($fields, function($field){ return ( (!$this->$field()) or ($this->$field() != 'NOW()'));}); $mapped = array_map(function($field){return $this->$field();}, $filtered); $values = array_values($mapped); - if ( dbQuery($sql, $values) ) { + if (dbQuery($sql, $values)) { $this->{'Id'} = dbInsertId(); return true; } } + $this->_last_error = dbError($sql); return false; } // end function save @@ -427,5 +421,8 @@ class ZM_Object { public function remove_from_cache() { return ZM_Object::_remove_from_cache(get_class(), $this); } + public function get_last_error() { + return $this->_last_error; + } } # end class Object ?> From 8a1d13b6cd132aad8fe5c09d4e4c0d405cfb9865 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sun, 21 Mar 2021 09:17:03 -0400 Subject: [PATCH 51/72] perror => Error() --- src/zm_fifo.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/zm_fifo.cpp b/src/zm_fifo.cpp index 35867cfc9..10a8cc365 100644 --- a/src/zm_fifo.cpp +++ b/src/zm_fifo.cpp @@ -86,7 +86,7 @@ bool Fifo::open() { } long pipe_size = (long)fcntl(raw_fd, F_GETPIPE_SZ); if (pipe_size == -1) { - perror("get pipe size failed."); + Error("get pipe size failed."); } Debug(1, "default pipe size: %ld\n", pipe_size); #endif @@ -106,7 +106,7 @@ bool Fifo::writePacket(ZMPacket &packet) { Debug(2, "Writing header ZM %u %" PRId64, packet.packet.size, packet.pts); // Going to write a brief header - if ( fprintf(outfile, "ZM %u %" PRId64 "\n", packet.packet.size, packet.pts) < 0 ) { + if (fprintf(outfile, "ZM %u %" PRId64 "\n", packet.packet.size, packet.pts) < 0) { if (errno != EAGAIN) { Error("Problem during writing: %s", strerror(errno)); } else { @@ -121,6 +121,7 @@ bool Fifo::writePacket(ZMPacket &packet) { } return true; } + bool Fifo::writePacket(std::string filename, ZMPacket &packet) { bool on_blocking_abort = true; FILE *outfile = nullptr; From d0adaeaabe47aabb5867178a5b07fa21b7881fde Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sun, 21 Mar 2021 09:18:12 -0400 Subject: [PATCH 52/72] rework user saving action to use User object. Implement a duplicate username check. Deprecate php < 5.3 due to lack of bcrypt password hashing functions. Hence deprecate the use of mysql PASSWORD() --- web/includes/actions/user.php | 101 ++++++++++++++++------------------ 1 file changed, 48 insertions(+), 53 deletions(-) diff --git a/web/includes/actions/user.php b/web/includes/actions/user.php index 372108355..b13471231 100644 --- a/web/includes/actions/user.php +++ b/web/includes/actions/user.php @@ -18,77 +18,72 @@ // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. // -if ( $action == 'Save' ) { - if ( canEdit('System') ) { - if ( !empty($_REQUEST['uid']) ) { - $dbUser = dbFetchOne('SELECT * FROM Users WHERE Id=?', NULL, array($_REQUEST['uid'])); - } else { - $dbUser = array(); - } +global $error_message; - $types = array(); - if ( isset($_REQUEST['newUser']['MonitorIds']) and is_array($_REQUEST['newUser']['MonitorIds']) ) +if ($action == 'Save') { + require_once('includes/User.php'); + $uid = isset($_REQUEST['uid']) ? validInt($_REQUEST['uid']) : 0; + $dbUser = new ZM\User($uid); + + if (canEdit('System')) { + # Need to check for uniqueness of Username + $user_with_my_username = ZM\User::find_one(array('Username'=>$_REQUEST['newUser']['Username'])); + if ($user_with_my_username and + ( ( $uid and ($user_with_my_username->Id() != $uid) ) or !$uid) + ) { + $error_message = 'There already exists a user with this Username
'; + unset($_REQUEST['redirect']); + return; + } + # What other tests should we do? + + if (isset($_REQUEST['newUser']['MonitorIds']) and is_array($_REQUEST['newUser']['MonitorIds'])) $_REQUEST['newUser']['MonitorIds'] = implode(',', $_REQUEST['newUser']['MonitorIds']); - if ( !$_REQUEST['newUser']['Password'] ) + if (!empty($_REQUEST['newUser']['Password'])) { + $_REQUEST['newUser']['Password'] = password_hash($_REQUEST['newUser']['Password'], PASSWORD_BCRYPT); + } else { unset($_REQUEST['newUser']['Password']); - - $changes = getFormChanges($dbUser, $_REQUEST['newUser'], $types); - - if ( isset($_REQUEST['newUser']['Password']) ) { - if ( function_exists('password_hash') ) { - $pass_hash = '"'.password_hash($_REQUEST['newUser']['Password'], PASSWORD_BCRYPT).'"'; - } else { - $pass_hash = ' PASSWORD('.dbEscape($_REQUEST['newUser']['Password']).') '; - ZM\Info('Cannot use bcrypt as you are using PHP < 5.3'); - } - - if ( $_REQUEST['newUser']['Password'] ) { - $changes['Password'] = 'Password = '.$pass_hash; - } else { - unset($changes['Password']); - } } + $changes = $dbUser->changes($_REQUEST['newUser']); + ZM\Debug("Changes: " . print_r($changes, true)); - if ( count($changes) ) { - if ( !empty($_REQUEST['uid']) ) { - dbQuery('UPDATE Users SET '.implode(', ', $changes).' WHERE Id = ?', array($_REQUEST['uid'])); - # If we are updating the logged in user, then update our session user data. - if ( $user and ( $dbUser['Username'] == $user['Username'] ) ) { + if (count($changes)) { + if (!$dbUser->save($changes)) { + $error_message = $dbUser->get_last_error(); + unset($_REQUEST['redirect']); + return; + } + + if ($uid) { + if ($user and ($dbUser->Username() == $user['Username'])) { # We are the logged in user, need to update the $user object and generate a new auth_hash $sql = 'SELECT * FROM Users WHERE Enabled=1 AND Id=?'; - $user = dbFetchOne($sql, NULL, array($_REQUEST['uid'])); + $user = dbFetchOne($sql, NULL, array($uid)); # Have to update auth hash in session zm_session_start(); generateAuthHash(ZM_AUTH_HASH_IPS, true); session_write_close(); } - } else { - dbQuery('INSERT INTO Users SET '.implode(', ', $changes)); } } # end if changes - } else if ( ZM_USER_SELF_EDIT and ( $_REQUEST['uid'] == $user['Id'] ) ) { - $uid = $user['Id']; - - $dbUser = dbFetchOne('SELECT Id, Password, Language FROM Users WHERE Id = ?', NULL, array($uid)); - - $types = array(); - $changes = getFormChanges($dbUser, $_REQUEST['newUser'], $types); - - if ( function_exists('password_hash') ) { - $pass_hash = '"'.password_hash($_REQUEST['newUser']['Password'], PASSWORD_BCRYPT).'"'; + } else if (ZM_USER_SELF_EDIT and ($uid == $user['Id'])) { + if (!empty($_REQUEST['newUser']['Password'])) { + $_REQUEST['newUser']['Password'] = password_hash($_REQUEST['newUser']['Password'], PASSWORD_BCRYPT); } else { - $pass_hash = ' PASSWORD('.dbEscape($_REQUEST['newUser']['Password']).') '; - ZM\Info ('Cannot use bcrypt as you are using PHP < 5.3'); + unset($_REQUEST['newUser']['Password']); } + $fields = array('Password'=>'', 'Language'=>'', 'HomeView'=>''); + ZM\Debug("changes: ".print_r(array_intersect_key($_REQUEST['newUser'], $fields),true)); + $changes = $dbUser->changes(array_intersect_key($_REQUEST['newUser'], $fields)); + ZM\Debug("changes: ".print_r($changes, true)); - if ( !empty($_REQUEST['newUser']['Password']) ) { - $changes['Password'] = 'Password = '.$pass_hash; - } else { - unset($changes['Password']); - } - if ( count($changes) ) { - dbQuery('UPDATE Users SET '.implode(', ', $changes).' WHERE Id=?', array($uid)); + if (count($changes)) { + if (!$dbUser->save($changes)) { + $error_message = $dbUser->get_last_error(); + unset($_REQUEST['redirect']); + return; + } # We are the logged in user, need to update the $user object and generate a new auth_hash $sql = 'SELECT * FROM Users WHERE Enabled=1 AND Id=?'; From cc455e5d7402999f037bbfee4ed1b4e07fdfa7c9 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sun, 21 Mar 2021 09:18:47 -0400 Subject: [PATCH 53/72] fix require=>require_once for User.php. Use getBodyTopHTML so that we get the error reporting --- web/skins/classic/views/user.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/web/skins/classic/views/user.php b/web/skins/classic/views/user.php index 981b205e2..e14c2e1f2 100644 --- a/web/skins/classic/views/user.php +++ b/web/skins/classic/views/user.php @@ -25,7 +25,7 @@ if ( !canEdit('System') && !$selfEdit ) { return; } -require('includes/User.php'); +require_once('includes/User.php'); if ( $_REQUEST['uid'] ) { if ( !($newUser = new ZM\User($_REQUEST['uid'])) ) { @@ -54,9 +54,9 @@ foreach ( dbFetchAll($sql) as $monitor ) { $focusWindow = true; xhtmlHeaders(__FILE__, translate('User').' - '.$newUser->Username()); +echo getBodyTopHTML(); +echo getNavBarHTML(); ?> - -
From 284837d53669df5b3f75b88b7651c552875a24c9 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sun, 21 Mar 2021 09:19:21 -0400 Subject: [PATCH 54/72] quotes, spaces. Also move setting redirect to to where we actually do the redirect so that actions can remove the redirect if there was an error to report. --- web/index.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/web/index.php b/web/index.php index bf71b059c..9683e5b31 100644 --- a/web/index.php +++ b/web/index.php @@ -98,7 +98,7 @@ if ( isset($_GET['skin']) ) { $skin = 'classic'; } -if ( ! is_dir("skins/$skin") ) { +if (!is_dir('skins/'.$skin) ) { $skins = array_map('basename', glob('skins/*', GLOB_ONLYDIR)); if ( !in_array($skin, $skins) ) { @@ -117,10 +117,10 @@ if ( isset($_GET['css']) ) { $css = 'classic'; } -if ( !is_dir("skins/$skin/css/$css") ) { +if (!is_dir("skins/$skin/css/$css")) { $css_skins = array_map('basename', glob('skins/'.$skin.'/css/*', GLOB_ONLYDIR)); - if ( count($css_skins) ) { - if ( !in_array($css, $css_skins) ) { + if (count($css_skins)) { + if (!in_array($css, $css_skins)) { ZM\Error("Invalid skin css '$css' setting to " . $css_skins[0]); $css = $css_skins[0]; } else { @@ -137,7 +137,7 @@ define('ZM_SKIN_PATH', "skins/$skin"); define('ZM_SKIN_NAME', $skin); $skinBase = array(); // To allow for inheritance of skins -if ( !file_exists(ZM_SKIN_PATH) ) +if (!file_exists(ZM_SKIN_PATH)) ZM\Fatal("Invalid skin '$skin'"); $skinBase[] = $skin; @@ -183,9 +183,6 @@ $user = null; if ( isset($_REQUEST['view']) ) $view = detaintPath($_REQUEST['view']); -if ( isset($_REQUEST['redirect']) ) - $redirect = '?view='.detaintPath($_REQUEST['redirect']); - # Add CSP Headers $cspNonce = bin2hex(zm_random_bytes(16)); @@ -265,6 +262,8 @@ if ( ZM_OPT_USE_AUTH and (!isset($user)) and ($view != 'login') and ($view != 'n $request = null; } +if ( isset($_REQUEST['redirect']) ) + $redirect = '?view='.detaintPath($_REQUEST['redirect']); if ( $redirect ) { ZM\Debug("Redirecting to $redirect"); From a57473a146b6222cd281fe5d0fe295ee81519ba1 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sun, 21 Mar 2021 09:19:31 -0400 Subject: [PATCH 55/72] remove debug --- web/skins/classic/views/options.php | 1 - 1 file changed, 1 deletion(-) diff --git a/web/skins/classic/views/options.php b/web/skins/classic/views/options.php index 2be9e9b52..2745979b7 100644 --- a/web/skins/classic/views/options.php +++ b/web/skins/classic/views/options.php @@ -156,7 +156,6 @@ foreach ( array_map('basename', glob('skins/'.$skin.'/css/*', GLOB_ONLYDIR)) as $userMonitors[] = $monitors[$monitorId]['Name']; } } - ZM\Debug("monitors: ".$user_row['Username'] . ' ' . $user_row['MonitorIds']. ' :' . print_r($userMonitors, true)); ?> From 07cf4697148b6f9a96fb7f869d0308be5bbb57e4 Mon Sep 17 00:00:00 2001 From: Peter Keresztes Schmidt Date: Sun, 21 Mar 2021 14:33:20 +0100 Subject: [PATCH 56/72] dep: Update RtspServer --- dep/RtspServer | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dep/RtspServer b/dep/RtspServer index 20846b25f..c81ec629f 160000 --- a/dep/RtspServer +++ b/dep/RtspServer @@ -1 +1 @@ -Subproject commit 20846b25ffc0c9a1de1b6701ca99425ef39e9f3f +Subproject commit c81ec629f091b77e2948b1e6113064de7e9bc9e3 From 6d9a4ed661b94300865ab48f7ed08835d50b31f8 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sun, 21 Mar 2021 12:28:33 -0400 Subject: [PATCH 57/72] If the analysis thread is falling behind, we can't count the packets after it in the number of packets to keep in queue. So figure out how many there are and add that to the max_video_packet count to keep so that we always have enough to satisfy pre_event_count --- src/zm_packetqueue.cpp | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/zm_packetqueue.cpp b/src/zm_packetqueue.cpp index 5dbc4ea00..7cc19debf 100644 --- a/src/zm_packetqueue.cpp +++ b/src/zm_packetqueue.cpp @@ -120,7 +120,7 @@ void PacketQueue::clearPackets(ZMPacket *add_packet) { // // So start at the beginning, counting video packets until the next keyframe. // Then if deleting those packets doesn't break 1 and 2, then go ahead and delete them. - if ( ! ( + if (! ( add_packet->packet.stream_index == video_stream_id and add_packet->keyframe @@ -138,6 +138,20 @@ void PacketQueue::clearPackets(ZMPacket *add_packet) { } std::unique_lock lck(mutex); + // If ananlysis_it isn't at the end, we need to keep that many additional packets + int tail_count = 0; + if (pktQueue.back() != add_packet) { + Debug(1, "Ours is not the back"); + packetqueue_iterator it = pktQueue.end(); + --it; + while (*it != add_packet) { + if ((*it)->packet.stream_index == video_stream_id) + ++tail_count; + --it; + } + } + Debug(1, "Tail count is %d", tail_count); + packetqueue_iterator it = pktQueue.begin(); packetqueue_iterator next_front = pktQueue.begin(); int video_packets_to_delete = 0; // This is a count of how many packets we will delete so we know when to stop looking @@ -145,33 +159,31 @@ void PacketQueue::clearPackets(ZMPacket *add_packet) { // First packet is special because we know it is a video keyframe and only need to check for lock ZMPacket *zm_packet = *it; ZMLockedPacket *lp = new ZMLockedPacket(zm_packet); - if ( lp->trylock() ) { + if (lp->trylock()) { ++it; delete lp; // Since we have many packets in the queue, we should NOT be pointing at end so don't need to test for that - while ( *it != add_packet ) { + while (*it != add_packet) { zm_packet = *it; lp = new ZMLockedPacket(zm_packet); - if ( !lp->trylock() ) { - break; - } + if (!lp->trylock()) break; delete lp; - if ( is_there_an_iterator_pointing_to_packet(zm_packet) ) { + if (is_there_an_iterator_pointing_to_packet(zm_packet)) { Warning("Found iterator at beginning of queue. Some thread isn't keeping up"); break; } - if ( zm_packet->packet.stream_index == video_stream_id ) { - if ( zm_packet->keyframe ) { + if (zm_packet->packet.stream_index == video_stream_id) { + if (zm_packet->keyframe) { Debug(3, "Have a video keyframe so setting next front to it"); next_front = it; } ++video_packets_to_delete; - Debug(4, "Counted %d video packets. Which would leave %d in packetqueue", - video_packets_to_delete, packet_counts[video_stream_id]-video_packets_to_delete); - if (packet_counts[video_stream_id] - video_packets_to_delete <= max_video_packet_count) { + Debug(4, "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); + if (packet_counts[video_stream_id] - video_packets_to_delete <= max_video_packet_count + tail_count) { break; } } From 8a1284e2fa2a7c3ff09f27bc53d377044fadccf4 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sun, 21 Mar 2021 12:30:56 -0400 Subject: [PATCH 58/72] Can't use a decimal step. Has to be any because browsers suck. --- web/skins/classic/views/js/zone.js | 2 +- web/skins/classic/views/zone.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/web/skins/classic/views/js/zone.js b/web/skins/classic/views/js/zone.js index 67ff28d1d..6282cbf8a 100644 --- a/web/skins/classic/views/js/zone.js +++ b/web/skins/classic/views/js/zone.js @@ -202,7 +202,7 @@ function toPercent(field, maxValue) { field.value = 100; } } - field.setAttribute('step', 0.01); + field.setAttribute('step', 'any'); field.setAttribute('max', 100); } diff --git a/web/skins/classic/views/zone.php b/web/skins/classic/views/zone.php index 2c41a885c..7a4761a50 100644 --- a/web/skins/classic/views/zone.php +++ b/web/skins/classic/views/zone.php @@ -206,7 +206,7 @@ if ( count($other_zones) ) { array('data-on-change'=>'applyZoneUnits', 'id'=>'newZone[Units]') ); # Used later for number inputs - $step = $newZone['Units'] == 'Percent' ? ' step="0.01" max="100"' : ''; + $step = $newZone['Units'] == 'Percent' ? ' step="any" max="100"' : ''; ?> From 7f9c9c6624b4ac3cf74fd986ea1d831a651362f3 Mon Sep 17 00:00:00 2001 From: Peter Keresztes Schmidt Date: Sun, 21 Mar 2021 21:40:41 +0100 Subject: [PATCH 59/72] web: make eslint happy --- web/skins/classic/views/js/events.js | 4 ++-- web/skins/classic/views/js/filter.js | 6 ++---- web/skins/classic/views/js/snapshots.js | 3 +-- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/web/skins/classic/views/js/events.js b/web/skins/classic/views/js/events.js index 40adaba20..9f6d34f75 100644 --- a/web/skins/classic/views/js/events.js +++ b/web/skins/classic/views/js/events.js @@ -58,8 +58,8 @@ function processRows(rows) { var emailed = row.Emailed == yesString ? emailedString : ''; row.Id = '' + eid + ''; - row.Name = '' + row.Name + '' - + '
' + archived + emailed + '
'; + row.Name = '' + row.Name + '' + + '
' + archived + emailed + '
'; if ( canEdit.Monitors ) row.Monitor = '' + row.Monitor + ''; if ( canEdit.Events ) row.Cause = '' + row.Cause + ''; if ( row.Notes.indexOf('detected:') >= 0 ) { diff --git a/web/skins/classic/views/js/filter.js b/web/skins/classic/views/js/filter.js index c59e755ff..61c01a2f2 100644 --- a/web/skins/classic/views/js/filter.js +++ b/web/skins/classic/views/js/filter.js @@ -40,10 +40,8 @@ function validateForm(form) { var have_endtime_term = false; for ( var i = 0; i < rows.length; i++ ) { if ( - ( form.elements['filter[Query][terms][' + i + '][attr]'].value == 'EndDateTime' ) - || - ( form.elements['filter[Query][terms][' + i + '][attr]'].value == 'EndTime' ) - || + ( form.elements['filter[Query][terms][' + i + '][attr]'].value == 'EndDateTime' ) || + ( form.elements['filter[Query][terms][' + i + '][attr]'].value == 'EndTime' ) || ( form.elements['filter[Query][terms][' + i + '][attr]'].value == 'EndDate' ) ) { have_endtime_term = true; diff --git a/web/skins/classic/views/js/snapshots.js b/web/skins/classic/views/js/snapshots.js index dff843374..b0e83001d 100644 --- a/web/skins/classic/views/js/snapshots.js +++ b/web/skins/classic/views/js/snapshots.js @@ -48,7 +48,6 @@ function ajaxRequest(params) { function processRows(rows) { $j.each(rows, function(ndx, row) { - var id = row.Id; row.Id = '' + id + ''; row.Name = '' + row.Name + ''; @@ -192,7 +191,7 @@ function initPage() { window.location.reload(true); }); -/* + /* // Manage the ARCHIVE button document.getElementById("archiveBtn").addEventListener("click", function onArchiveClick(evt) { var selections = getIdSelections(); From 7e86e1ef401cdf17193739ecf64afd5368231daa Mon Sep 17 00:00:00 2001 From: Peter Keresztes Schmidt Date: Sun, 7 Mar 2021 16:17:17 +0100 Subject: [PATCH 60/72] utils: Make TimevalToString thread-safe --- src/zm_utils.cpp | 31 ++++++++++++++++--------------- src/zm_utils.h | 2 +- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/zm_utils.cpp b/src/zm_utils.cpp index 3fce11d1c..71f99ef9e 100644 --- a/src/zm_utils.cpp +++ b/src/zm_utils.cpp @@ -22,7 +22,7 @@ #include "zm_config.h" #include "zm_logger.h" #include -#include +#include #include #include /* Definition of AT_* constants */ #include @@ -43,7 +43,7 @@ std::string trimSet(std::string str, std::string trimset) { // Trim Both leading and trailing sets size_t startpos = str.find_first_not_of(trimset); // Find the first character position after excluding leading blank spaces size_t endpos = str.find_last_not_of(trimset); // Find the first character position from reverse af - + // if all spaces or empty return an empty string if ( ( std::string::npos == startpos ) || ( std::string::npos == endpos ) ) return std::string(""); @@ -148,7 +148,7 @@ const std::string base64Encode(const std::string &inString) { selection = remainder | (*inPtr >> 4); remainder = (*inPtr++ & 0x0f) << 2; outString += base64_table[selection]; - + if ( *inPtr ) { selection = remainder | (*inPtr >> 6); outString += base64_table[selection]; @@ -175,7 +175,7 @@ int split(const char* string, const char delim, std::vector& items) return -2; std::string str(string); - + while ( true ) { size_t pos = str.find(delim); items.push_back(str.substr(0, pos)); @@ -242,7 +242,7 @@ void hwcaps_detect() { } else { sse_version = 0; Debug(1, "Detected a x86\\x86-64 processor"); - } + } #elif defined(__arm__) // ARM processor in 32bit mode // To see if it supports NEON, we need to get that information from the kernel @@ -279,7 +279,7 @@ void* sse2_aligned_memcpy(void* dest, const void* src, size_t bytes) { "sse2_copy_iter:\n\t" "movdqa (%0),%%xmm0\n\t" "movdqa 0x10(%0),%%xmm1\n\t" - "movdqa 0x20(%0),%%xmm2\n\t" + "movdqa 0x20(%0),%%xmm2\n\t" "movdqa 0x30(%0),%%xmm3\n\t" "movdqa 0x40(%0),%%xmm4\n\t" "movdqa 0x50(%0),%%xmm5\n\t" @@ -328,16 +328,17 @@ void timespec_diff(struct timespec *start, struct timespec *end, struct timespec } } -char *timeval_to_string( struct timeval tv ) { - time_t nowtime; - struct tm *nowtm; - static char tmbuf[20], buf[28]; +std::string TimevalToString(timeval tv) { + tm now = {}; + std::array tm_buf = {}; - nowtime = tv.tv_sec; - nowtm = localtime(&nowtime); - strftime(tmbuf, sizeof tmbuf, "%Y-%m-%d %H:%M:%S", nowtm); - snprintf(buf, sizeof buf-1, "%s.%06ld", tmbuf, tv.tv_usec); - return buf; + localtime_r(&tv.tv_sec, &now); + size_t tm_buf_len = strftime(tm_buf.data(), tm_buf.size(), "%Y-%m-%d %H:%M:%S", &now); + if (tm_buf_len == 0) { + return ""; + } + + return stringtf("%s.%06ld", tm_buf.data(), tv.tv_usec); } std::string UriDecode( const std::string &encoded ) { diff --git a/src/zm_utils.h b/src/zm_utils.h index 4eba32b6a..41b6789fe 100644 --- a/src/zm_utils.h +++ b/src/zm_utils.h @@ -63,7 +63,7 @@ void hwcaps_detect(); extern unsigned int sse_version; extern unsigned int neonversion; -char *timeval_to_string( struct timeval tv ); +std::string TimevalToString(timeval tv); std::string UriDecode( const std::string &encoded ); void touch( const char *pathname ); From 4e8c7d1f7cac1936c08ae6932726895c5079cfd1 Mon Sep 17 00:00:00 2001 From: Peter Keresztes Schmidt Date: Sun, 7 Mar 2021 16:35:05 +0100 Subject: [PATCH 61/72] Eliminate non-thread-safe calls to localtime localtime uses an internal static storage to which a pointer is given as return value. Due to this it is not safe to call localtime from multiple threads since the same static storage is used. Use localtime_r instead which allows to pass in a tm struct. Fixes: https://github.com/ZoneMinder/zoneminder/security/code-scanning/24 https://github.com/ZoneMinder/zoneminder/security/code-scanning/25 https://github.com/ZoneMinder/zoneminder/security/code-scanning/26 https://github.com/ZoneMinder/zoneminder/security/code-scanning/27 https://github.com/ZoneMinder/zoneminder/security/code-scanning/28 https://github.com/ZoneMinder/zoneminder/security/code-scanning/30 https://github.com/ZoneMinder/zoneminder/security/code-scanning/31 https://github.com/ZoneMinder/zoneminder/security/code-scanning/33 https://github.com/ZoneMinder/zoneminder/security/code-scanning/58 https://github.com/ZoneMinder/zoneminder/security/code-scanning/59 https://github.com/ZoneMinder/zoneminder/security/code-scanning/63 https://github.com/ZoneMinder/zoneminder/security/code-scanning/64 https://github.com/ZoneMinder/zoneminder/security/code-scanning/65 --- src/zm_event.cpp | 33 +++++++++++++++++---------------- src/zm_event.h | 12 +++++++----- src/zm_eventstream.cpp | 18 ++++++++++-------- src/zm_image.cpp | 6 ++++-- src/zm_logger.cpp | 3 ++- src/zm_monitor.cpp | 3 ++- src/zm_user.cpp | 11 ++++++----- src/zmu.cpp | 6 ++++-- 8 files changed, 52 insertions(+), 40 deletions(-) diff --git a/src/zm_event.cpp b/src/zm_event.cpp index f0b2da694..9ab13fafc 100644 --- a/src/zm_event.cpp +++ b/src/zm_event.cpp @@ -71,8 +71,8 @@ Event::Event( std::string notes; createNotes(notes); - struct timeval now; - gettimeofday(&now, 0); + timeval now = {}; + gettimeofday(&now, nullptr); if ( !start_time.tv_sec ) { Warning("Event has zero time, setting to now"); @@ -80,12 +80,12 @@ Event::Event( } else if ( start_time.tv_sec > now.tv_sec ) { char buffer[26]; char buffer_now[26]; - struct tm* tm_info; + tm tm_info = {}; - tm_info = localtime(&start_time.tv_sec); - strftime(buffer, 26, "%Y:%m:%d %H:%M:%S", tm_info); - tm_info = localtime(&now.tv_sec); - strftime(buffer_now, 26, "%Y:%m:%d %H:%M:%S", tm_info); + localtime_r(&start_time.tv_sec, &tm_info); + strftime(buffer, 26, "%Y:%m:%d %H:%M:%S", &tm_info); + localtime_r(&now.tv_sec, &tm_info); + strftime(buffer_now, 26, "%Y:%m:%d %H:%M:%S", &tm_info); Error( "StartDateTime in the future starttime %u.%u >? now %u.%u difference %d\n%s\n%s", @@ -661,16 +661,17 @@ bool Event::SetPath(Storage *storage) { return false; } - struct tm *stime = localtime(&start_time.tv_sec); + tm stime = {}; + localtime_r(&start_time.tv_sec, &stime); if ( scheme == Storage::DEEP ) { int dt_parts[6]; - dt_parts[0] = stime->tm_year-100; - dt_parts[1] = stime->tm_mon+1; - dt_parts[2] = stime->tm_mday; - dt_parts[3] = stime->tm_hour; - dt_parts[4] = stime->tm_min; - dt_parts[5] = stime->tm_sec; + dt_parts[0] = stime.tm_year-100; + dt_parts[1] = stime.tm_mon+1; + dt_parts[2] = stime.tm_mday; + dt_parts[3] = stime.tm_hour; + dt_parts[4] = stime.tm_min; + dt_parts[5] = stime.tm_sec; std::string date_path; std::string time_path; @@ -685,7 +686,7 @@ bool Event::SetPath(Storage *storage) { if ( i == 2 ) date_path = path; } - time_path = stringtf("%02d/%02d/%02d", stime->tm_hour, stime->tm_min, stime->tm_sec); + time_path = stringtf("%02d/%02d/%02d", stime.tm_hour, stime.tm_min, stime.tm_sec); // Create event id symlink std::string id_file = stringtf("%s/.%" PRIu64, date_path.c_str(), id); @@ -695,7 +696,7 @@ bool Event::SetPath(Storage *storage) { } } else if ( scheme == Storage::MEDIUM ) { path += stringtf("/%04d-%02d-%02d", - stime->tm_year+1900, stime->tm_mon+1, stime->tm_mday + stime.tm_year+1900, stime.tm_mon+1, stime.tm_mday ); if ( mkdir(path.c_str(), 0755) and ( errno != EEXIST ) ) { Error("Can't mkdir %s: %s", path.c_str(), strerror(errno)); diff --git a/src/zm_event.h b/src/zm_event.h index 1eb3f2a7f..5e972eca2 100644 --- a/src/zm_event.h +++ b/src/zm_event.h @@ -138,18 +138,20 @@ class Event { bool SetPath(Storage *storage); public: - static const char *getSubPath(struct tm *time) { + static const char *getSubPath(tm time) { static char subpath[PATH_MAX] = ""; snprintf(subpath, sizeof(subpath), "%02d/%02d/%02d/%02d/%02d/%02d", - time->tm_year-100, time->tm_mon+1, time->tm_mday, - time->tm_hour, time->tm_min, time->tm_sec); + time.tm_year-100, time.tm_mon+1, time.tm_mday, + time.tm_hour, time.tm_min, time.tm_sec); return subpath; } static const char *getSubPath(time_t *time) { - return Event::getSubPath(localtime(time)); + tm time_tm = {}; + localtime_r(time, &time_tm); + return Event::getSubPath(time_tm); } - const char* getEventFile(void) const { + const char* getEventFile() const { return video_file.c_str(); } diff --git a/src/zm_eventstream.cpp b/src/zm_eventstream.cpp index 5ed424b7f..8610f2f04 100644 --- a/src/zm_eventstream.cpp +++ b/src/zm_eventstream.cpp @@ -171,33 +171,35 @@ bool EventStream::loadEventData(uint64_t event_id) { const char *storage_path = storage->Path(); if ( event_data->scheme == Storage::DEEP ) { - struct tm *event_time = localtime(&event_data->start_time); + tm event_time = {}; + localtime_r(&event_data->start_time, &event_time); if ( storage_path[0] == '/' ) snprintf(event_data->path, sizeof(event_data->path), "%s/%u/%02d/%02d/%02d/%02d/%02d/%02d", storage_path, event_data->monitor_id, - event_time->tm_year-100, event_time->tm_mon+1, event_time->tm_mday, - event_time->tm_hour, event_time->tm_min, event_time->tm_sec); + event_time.tm_year-100, event_time.tm_mon+1, event_time.tm_mday, + event_time.tm_hour, event_time.tm_min, event_time.tm_sec); else snprintf(event_data->path, sizeof(event_data->path), "%s/%s/%u/%02d/%02d/%02d/%02d/%02d/%02d", staticConfig.PATH_WEB.c_str(), storage_path, event_data->monitor_id, - event_time->tm_year-100, event_time->tm_mon+1, event_time->tm_mday, - event_time->tm_hour, event_time->tm_min, event_time->tm_sec); + event_time.tm_year-100, event_time.tm_mon+1, event_time.tm_mday, + event_time.tm_hour, event_time.tm_min, event_time.tm_sec); } else if ( event_data->scheme == Storage::MEDIUM ) { - struct tm *event_time = localtime(&event_data->start_time); + tm event_time = {}; + localtime_r(&event_data->start_time, &event_time); if ( storage_path[0] == '/' ) snprintf(event_data->path, sizeof(event_data->path), "%s/%u/%04d-%02d-%02d/%" PRIu64, storage_path, event_data->monitor_id, - event_time->tm_year+1900, event_time->tm_mon+1, event_time->tm_mday, + event_time.tm_year+1900, event_time.tm_mon+1, event_time.tm_mday, event_data->event_id); else snprintf(event_data->path, sizeof(event_data->path), "%s/%s/%u/%04d-%02d-%02d/%" PRIu64, staticConfig.PATH_WEB.c_str(), storage_path, event_data->monitor_id, - event_time->tm_year+1900, event_time->tm_mon+1, event_time->tm_mday, + event_time.tm_year+1900, event_time.tm_mon+1, event_time.tm_mday, event_data->event_id); } else { diff --git a/src/zm_image.cpp b/src/zm_image.cpp index 014ae783c..2e0c82670 100644 --- a/src/zm_image.cpp +++ b/src/zm_image.cpp @@ -1186,7 +1186,8 @@ cinfo->out_color_space = JCS_RGB; // This is a lot of stuff to allocate on the stack. Recommend char *timebuf[64]; char timebuf[64], msbuf[64]; - strftime(timebuf, sizeof timebuf, "%Y:%m:%d %H:%M:%S", localtime(&(timestamp.tv_sec))); + tm timestamp_tm = {}; + strftime(timebuf, sizeof timebuf, "%Y:%m:%d %H:%M:%S", localtime_r(×tamp.tv_sec, ×tamp_tm)); snprintf(msbuf, sizeof msbuf, "%06d",(int)(timestamp.tv_usec)); // we only use milliseconds because that's all defined in exif, but this is the whole microseconds because we have it unsigned char exiftimes[82] = { 0x45, 0x78, 0x69, 0x66, 0x00, 0x00, 0x49, 0x49, 0x2A, 0x00, 0x08, 0x00, 0x00, 0x00, 0x01, 0x00, @@ -2132,7 +2133,8 @@ void Image::Annotate( void Image::Timestamp( const char *label, const time_t when, const Coord &coord, const int size ) { char time_text[64]; - strftime(time_text, sizeof(time_text), "%y/%m/%d %H:%M:%S", localtime(&when)); + tm when_tm = {}; + strftime(time_text, sizeof(time_text), "%y/%m/%d %H:%M:%S", localtime_r(&when, &when_tm)); if ( label ) { // Assume label is max 64, + ' - ' + 64 chars of time_text char text[132]; diff --git a/src/zm_logger.cpp b/src/zm_logger.cpp index 3d0c0c2c5..d425764ae 100644 --- a/src/zm_logger.cpp +++ b/src/zm_logger.cpp @@ -447,7 +447,8 @@ void Logger::logPrint(bool hex, const char * const filepath, const int line, con } else { #endif char *timePtr = timeString; - timePtr += strftime(timePtr, sizeof(timeString), "%x %H:%M:%S", localtime(&timeVal.tv_sec)); + tm now_tm = {}; + timePtr += strftime(timePtr, sizeof(timeString), "%x %H:%M:%S", localtime_r(&timeVal.tv_sec, &now_tm)); snprintf(timePtr, sizeof(timeString)-(timePtr-timeString), ".%06ld", timeVal.tv_usec); #if 0 } diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 1ca51bdb9..f846e569f 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -2842,7 +2842,8 @@ void Monitor::TimestampImage(Image *ts_image, const struct timeval *ts_time) con // Expand the strftime macros first char label_time_text[256]; - strftime(label_time_text, sizeof(label_time_text), label_format, localtime(&ts_time->tv_sec)); + tm ts_tm = {}; + strftime(label_time_text, sizeof(label_time_text), label_format, localtime_r(&ts_time->tv_sec, &ts_tm)); char label_text[1024]; const char *s_ptr = label_time_text; char *d_ptr = label_text; diff --git a/src/zm_user.cpp b/src/zm_user.cpp index 512718ccb..13cf7c1ac 100644 --- a/src/zm_user.cpp +++ b/src/zm_user.cpp @@ -245,18 +245,19 @@ User *zmLoadAuthUser(const char *auth, bool use_remote_addr) { const char *pass = dbrow[2]; time_t our_now = now; + tm now_tm = {}; for ( unsigned int i = 0; i < hours; i++, our_now -= 3600 ) { - struct tm *now_tm = localtime(&our_now); + localtime_r(&our_now, &now_tm); snprintf(auth_key, sizeof(auth_key)-1, "%s%s%s%s%d%d%d%d", config.auth_hash_secret, user, pass, remote_addr, - now_tm->tm_hour, - now_tm->tm_mday, - now_tm->tm_mon, - now_tm->tm_year); + now_tm.tm_hour, + now_tm.tm_mday, + now_tm.tm_mon, + now_tm.tm_year); #if HAVE_DECL_MD5 MD5((unsigned char *)auth_key, strlen(auth_key), md5sum); diff --git a/src/zmu.cpp b/src/zmu.cpp index 0bfe90b3b..384af99a3 100644 --- a/src/zmu.cpp +++ b/src/zmu.cpp @@ -506,8 +506,10 @@ int main(int argc, char *argv[]) { struct timeval timestamp = monitor->GetTimestamp(image_idx); if ( verbose ) { char timestamp_str[64] = "None"; - if ( timestamp.tv_sec ) - strftime(timestamp_str, sizeof(timestamp_str), "%Y-%m-%d %H:%M:%S", localtime(×tamp.tv_sec)); + if ( timestamp.tv_sec ) { + tm tm_info = {}; + strftime(timestamp_str, sizeof(timestamp_str), "%Y-%m-%d %H:%M:%S", localtime_r(×tamp.tv_sec, &tm_info)); + } if ( image_idx == -1 ) printf("Time of last image capture: %s.%02ld\n", timestamp_str, timestamp.tv_usec/10000); else From 67d7872e9a3a8db8fc550d1a9f5f7eedceb9f018 Mon Sep 17 00:00:00 2001 From: Peter Keresztes Schmidt Date: Sun, 7 Mar 2021 16:44:08 +0100 Subject: [PATCH 62/72] Eliminate non-thread-safe calls to gmtime gmtime uses an internal static storage to which a pointer is given as return value. Due to this it is not safe to call gmtime from multiple threads since the same static storage is used. Use gmtime_r instead which allows to pass in a tm struct. Fixes: https://github.com/ZoneMinder/zoneminder/security/code-scanning/32 --- src/zms.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/zms.cpp b/src/zms.cpp index 89b20e30c..70ec38fdc 100644 --- a/src/zms.cpp +++ b/src/zms.cpp @@ -241,8 +241,9 @@ int main(int argc, const char *argv[], char **envp) { time_t now = time(nullptr); char date_string[64]; + tm now_tm = {}; strftime(date_string, sizeof(date_string)-1, - "%a, %d %b %Y %H:%M:%S GMT", gmtime(&now)); + "%a, %d %b %Y %H:%M:%S GMT", gmtime_r(&now, &now_tm)); fputs("Last-Modified: ", stdout); fputs(date_string, stdout); From 858ae8b11fa283816db1f04d853265ca092ab692 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sun, 21 Mar 2021 13:16:55 -0400 Subject: [PATCH 63/72] fix alignment and min width of datetime column in logs view --- web/skins/classic/css/base/views/log.css | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/web/skins/classic/css/base/views/log.css b/web/skins/classic/css/base/views/log.css index 918c1097c..0c6f618c8 100644 --- a/web/skins/classic/css/base/views/log.css +++ b/web/skins/classic/css/base/views/log.css @@ -16,3 +16,14 @@ tr.log-dbg td { font-style: italic; } +th[data-field="DateTime"], +th[data-field="Component"], +th[data-field="Code"], +th[data-field="Message"], +th[data-field="File"], +th[data-field="Line"] { + text-align: left; +} +th[data-field="DateTime"] { + min-width: 135px; +} From be653980f34d2dc41a53b7ac19033edd707c57f3 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sun, 21 Mar 2021 18:17:10 -0400 Subject: [PATCH 64/72] fix eslint --- web/skins/classic/views/js/snapshots.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/skins/classic/views/js/snapshots.js b/web/skins/classic/views/js/snapshots.js index b0e83001d..8df31d5c3 100644 --- a/web/skins/classic/views/js/snapshots.js +++ b/web/skins/classic/views/js/snapshots.js @@ -267,7 +267,7 @@ function initPage() { }) .fail(logAjaxFail); }); -*/ + */ // Manage the DELETE button document.getElementById("deleteBtn").addEventListener("click", function onDeleteClick(evt) { if ( ! canEdit.Events ) { From 6d5cbe258365b53785c0ee3c45a97e7c0026632a Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 22 Mar 2021 11:02:32 -0400 Subject: [PATCH 65/72] Make incorrect dimensions non-fatal if the monitor dimensions are larger than what is expected, so at least there is enough ram to store the image --- src/zm_local_camera.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/zm_local_camera.cpp b/src/zm_local_camera.cpp index b5c0ab665..b464c4d96 100644 --- a/src/zm_local_camera.cpp +++ b/src/zm_local_camera.cpp @@ -798,10 +798,10 @@ void LocalCamera::Initialise() { ); if ( v4l2_data.fmt.fmt.pix.width != width ) { - Warning("Failed to set requested width"); + Warning("Failed to set requested width"); } if ( v4l2_data.fmt.fmt.pix.height != height ) { - Warning("Failed to set requested height"); + Warning("Failed to set requested height"); } /* Buggy driver paranoia. */ @@ -2087,8 +2087,11 @@ int LocalCamera::Capture(ZMPacket &zm_packet) { buffer_bytesused = v4l2_data.bufptr->bytesused; bytes += buffer_bytesused; - if ( (v4l2_data.fmt.fmt.pix.width * v4l2_data.fmt.fmt.pix.height) != (width * height) ) { - Fatal("Captured image dimensions differ: V4L2: %dx%d monitor: %dx%d", + if ( (v4l2_data.fmt.fmt.pix.width * v4l2_data.fmt.fmt.pix.height) > (width * height) ) { + Fatal("Captured image dimensions larger than image buffer: V4L2: %dx%d monitor: %dx%d", + v4l2_data.fmt.fmt.pix.width, v4l2_data.fmt.fmt.pix.height, width, height); + } else if ( (v4l2_data.fmt.fmt.pix.width * v4l2_data.fmt.fmt.pix.height) != (width * height) ) { + Error("Captured image dimensions differ: V4L2: %dx%d monitor: %dx%d", v4l2_data.fmt.fmt.pix.width, v4l2_data.fmt.fmt.pix.height, width, height); } } // end if v4l2 From 3f3bc50acb9cea70e901aae0649319b37eb91d3a Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 22 Mar 2021 12:04:32 -0400 Subject: [PATCH 66/72] Add keep_keyframes setting. When NOT doing passthrough we don't actually have to store all packets since last keyframe, so don't do it. SImplifies clearPackets() logic a lot and will save ram for those people. --- src/zm_packetqueue.cpp | 40 +++++++++++++++++++++++++++++----------- src/zm_packetqueue.h | 2 ++ 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/zm_packetqueue.cpp b/src/zm_packetqueue.cpp index 7cc19debf..152f51972 100644 --- a/src/zm_packetqueue.cpp +++ b/src/zm_packetqueue.cpp @@ -31,7 +31,8 @@ PacketQueue::PacketQueue(): max_video_packet_count(-1), max_stream_id(-1), packet_counts(nullptr), - deleting(false) + deleting(false), + keep_keyframes(false) { } @@ -120,23 +121,39 @@ void PacketQueue::clearPackets(ZMPacket *add_packet) { // // So start at the beginning, counting video packets until the next keyframe. // Then if deleting those packets doesn't break 1 and 2, then go ahead and delete them. - if (! ( + if (keep_keyframes and ! ( add_packet->packet.stream_index == video_stream_id - and - add_packet->keyframe - and - (packet_counts[video_stream_id] > max_video_packet_count) - and - *(pktQueue.begin()) != add_packet - ) + and + add_packet->keyframe + and + (packet_counts[video_stream_id] > max_video_packet_count) + and + *(pktQueue.begin()) != add_packet + ) ) { - Debug(3, "stream index %d ?= video_stream_id %d, keyframe %d, counts %d > max %d at begin %d", - add_packet->packet.stream_index, video_stream_id, add_packet->keyframe, packet_counts[video_stream_id], max_video_packet_count, + Debug(3, "stream index %d ?= video_stream_id %d, keyframe %d, keep_keyframes %d, counts %d > max %d at begin %d", + add_packet->packet.stream_index, video_stream_id, add_packet->keyframe, keep_keyframes, packet_counts[video_stream_id], max_video_packet_count, ( *(pktQueue.begin()) != add_packet ) ); return; } std::unique_lock lck(mutex); + if (!keep_keyframes) { + // If not doing passthrough, we don't care about starting with a keyframe so logic is simpler + while ((*pktQueue.begin() != add_packet) and (packet_counts[video_stream_id] > max_video_packet_count)) { + ZMPacket *zm_packet = *pktQueue.begin(); + ZMLockedPacket *lp = new ZMLockedPacket(zm_packet); + if (!lp->trylock()) break; + delete lp; + + pktQueue.pop_front(); + packet_counts[zm_packet->packet.stream_index] -= 1; + Debug(1, "Deleting a packet with stream index:%d image_index:%d with keyframe:%d, video frames in queue:%d max: %d, queuesize:%d", + zm_packet->packet.stream_index, zm_packet->image_index, zm_packet->keyframe, packet_counts[video_stream_id], max_video_packet_count, pktQueue.size()); + delete zm_packet; + } // end while + return; + } // If ananlysis_it isn't at the end, we need to keep that many additional packets int tail_count = 0; @@ -156,6 +173,7 @@ void PacketQueue::clearPackets(ZMPacket *add_packet) { packetqueue_iterator next_front = pktQueue.begin(); int video_packets_to_delete = 0; // This is a count of how many packets we will delete so we know when to stop looking + // First packet is special because we know it is a video keyframe and only need to check for lock ZMPacket *zm_packet = *it; ZMLockedPacket *lp = new ZMLockedPacket(zm_packet); diff --git a/src/zm_packetqueue.h b/src/zm_packetqueue.h index e759dc777..8caa0a527 100644 --- a/src/zm_packetqueue.h +++ b/src/zm_packetqueue.h @@ -38,6 +38,7 @@ class PacketQueue { int max_stream_id; int *packet_counts; /* packet count for each stream_id, to keep track of how many video vs audio packets are in the queue */ bool deleting; + bool keep_keyframes; std::list iterators; std::mutex mutex; @@ -51,6 +52,7 @@ class PacketQueue { int addStream(); void setMaxVideoPackets(int p); + void setKeepKeyframes(bool k) { keep_keyframes = k; }; bool queuePacket(ZMPacket* packet); ZMLockedPacket * popPacket(); From 2d4b4b60223c142a027fdb09374d2f42518dcdc6 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 22 Mar 2021 12:05:05 -0400 Subject: [PATCH 67/72] If we already tried decoding a packet, don't try again. Also we really shouldn't be decoding in videostore. --- src/zm_videostore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/zm_videostore.cpp b/src/zm_videostore.cpp index ba19f6f57..201192678 100644 --- a/src/zm_videostore.cpp +++ b/src/zm_videostore.cpp @@ -979,7 +979,7 @@ int VideoStore::writeVideoFramePacket(ZMPacket *zm_packet) { ); } else if ( !zm_packet->in_frame ) { Debug(4, "Have no in_frame"); - if ( zm_packet->packet.size ) { + if (zm_packet->packet.size and !zm_packet->decoded) { Debug(4, "Decoding"); if ( !zm_packet->decode(video_in_ctx) ) { Debug(2, "unable to decode yet."); From b8b20917be8e41435a1ac4daab2681ebed18d570 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 22 Mar 2021 12:05:22 -0400 Subject: [PATCH 68/72] setKeepKeyframes when not PASSTHROUGH --- src/zm_monitor.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index f846e569f..83c041ed4 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -582,6 +582,7 @@ void Monitor::Load(MYSQL_ROW dbrow, bool load_zones=true, Purpose p = QUERY) { warmup_count = atoi(dbrow[col]); col++; pre_event_count = atoi(dbrow[col]); col++; packetqueue.setMaxVideoPackets(pre_event_count); + packetqueue.setKeepKeyframes(videowriter == PASSTHROUGH); post_event_count = atoi(dbrow[col]); col++; stream_replay_buffer = atoi(dbrow[col]); col++; alarm_frame_count = atoi(dbrow[col]); col++; @@ -2574,7 +2575,7 @@ int Monitor::Capture() { // Don't want to do analysis on it, but we won't due to signal return -1; } else if ( captureResult > 0 ) { - // If we captured, let's assume signal, ::Decode will detect further + // If we captured, let's assume signal, Decode will detect further if (!decoding_enabled) { shared_data->last_write_index = index; shared_data->last_write_time = packet->timestamp->tv_sec; From c347261e19688731f024088638405676ff1d92d9 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 22 Mar 2021 12:05:36 -0400 Subject: [PATCH 69/72] Change default of ImageBufferCount to 3 --- web/includes/Monitor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/includes/Monitor.php b/web/includes/Monitor.php index 9470c68f3..eb9efed29 100644 --- a/web/includes/Monitor.php +++ b/web/includes/Monitor.php @@ -83,7 +83,7 @@ class Monitor extends ZM_Object { 'LabelX' => 0, 'LabelY' => 0, 'LabelSize' => 1, - 'ImageBufferCount' => 20, + 'ImageBufferCount' => 3, 'WarmupCount' => 0, 'PreEventCount' => 5, 'PostEventCount' => 5, From fa08240a4d3c2d12a9d01e2ad8a79ca676ae5153 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 22 Mar 2021 12:06:25 -0400 Subject: [PATCH 70/72] Fix set() and __call to use the default value when set value is ''. Fixes issues in monitor view when changing type --- web/includes/Object.php | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/web/includes/Object.php b/web/includes/Object.php index 4ed4fadf0..337136836 100644 --- a/web/includes/Object.php +++ b/web/includes/Object.php @@ -39,6 +39,7 @@ class ZM_Object { public function __call($fn, array $args){ $type = (array_key_exists($fn, $this->defaults) && is_array($this->defaults[$fn])) ? $this->defaults[$fn]['type'] : 'scalar'; + if ( count($args) ) { if ( $type == 'set' and is_array($args[0]) ) { $this->{$fn} = implode(',', $args[0]); @@ -51,7 +52,15 @@ class ZM_Object { $this->{$fn} = preg_replace($this->defaults[$fn]['filter_regexp'], '', $args[0]); } } else { - $this->{$fn} = $args[0]; + if ( $args[0] == '' and array_key_exists($fn, $this->defaults) ) { + if ( is_array($this->defaults[$fn]) ) { + $this->{$fn} = $this->defaults[$fn]['default']; + } else { + $this->{$fn} = $this->defaults[$fn]; + } + } else { + $this->{$fn} = $args[0]; + } } } @@ -175,7 +184,7 @@ class ZM_Object { $this->$field($value); } else { if ( is_array($value) ) { -# perhaps should turn into a comma-separated string + # perhaps should turn into a comma-separated string $this->{$field} = implode(',', $value); } else if ( is_string($value) ) { if ( array_key_exists($field, $this->defaults) && is_array($this->defaults[$field]) && isset($this->defaults[$field]['filter_regexp']) ) { @@ -186,8 +195,14 @@ class ZM_Object { } else { $this->{$field} = preg_replace($this->defaults[$field]['filter_regexp'], '', trim($value)); } + } else if ( $value == '' and array_key_exists($field, $this->defaults) ) { + if ( is_array($this->defaults[$field]) ) { + $this->{$field} = $this->defaults[$field]['default']; + } else { + $this->{$field} = $this->defaults[$field]; + } } else { - $this->{$field} = trim($value); + $this->{$field} = $value; } } else if ( is_integer($value) ) { $this->{$field} = $value; From 613ed1faf2295d38ef08df0adadb17cff8543f71 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 22 Mar 2021 12:06:48 -0400 Subject: [PATCH 71/72] Update estimated ram use when we use the dropdown to change resolution --- web/skins/classic/views/js/monitor.js | 1 + 1 file changed, 1 insertion(+) diff --git a/web/skins/classic/views/js/monitor.js b/web/skins/classic/views/js/monitor.js index 1603df783..854ba627b 100644 --- a/web/skins/classic/views/js/monitor.js +++ b/web/skins/classic/views/js/monitor.js @@ -48,6 +48,7 @@ function updateMonitorDimensions(element) { form.elements['newMonitor[Height]'].value = dimensions[1]; } } + update_estimated_ram_use(); return false; } From c7b22dae818654a557061ffc743d08e745f97d78 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Mon, 22 Mar 2021 12:07:23 -0400 Subject: [PATCH 72/72] get rid of nextId entirely. Don't want to use it anywhere other than setting the name --- web/skins/classic/views/monitor.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/web/skins/classic/views/monitor.php b/web/skins/classic/views/monitor.php index a2402b8bc..fb7e2bde8 100644 --- a/web/skins/classic/views/monitor.php +++ b/web/skins/classic/views/monitor.php @@ -37,16 +37,14 @@ $mid = null; $monitor = null; if ( !empty($_REQUEST['mid']) ) { $mid = validInt($_REQUEST['mid']); - $monitor = new ZM\Monitor($mid); if ( $monitor and ZM_OPT_X10 ) $x10Monitor = dbFetchOne('SELECT * FROM TriggersX10 WHERE MonitorId = ?', NULL, array($mid)); } if ( !$monitor ) { - $nextId = getTableAutoInc('Monitors'); $monitor = new ZM\Monitor(); - $monitor->Name(translate('Monitor').'-'.$nextId); + $monitor->Name(translate('Monitor').'-'.getTableAutoInc('Monitors')); $monitor->WebColour(random_colour()); } # end if $_REQUEST['mid']