From 6b70066fea6b1d02233f603c25e407d43a49f098 Mon Sep 17 00:00:00 2001 From: Klaus Schmidinger Date: Sun, 6 Jul 2025 15:06:55 +0200 Subject: [PATCH] Reworked cTimer::Matches() --- CONTRIBUTORS | 2 + HISTORY | 21 ++++++++- config.h | 6 +-- menu.c | 6 +-- timers.c | 131 ++++++++++++++++++++++++++++++++++++--------------- timers.h | 34 +++++++++++-- vdr.c | 6 +-- 7 files changed, 153 insertions(+), 53 deletions(-) diff --git a/CONTRIBUTORS b/CONTRIBUTORS index bff40c9a..bb8f9d77 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -2601,6 +2601,8 @@ Markus Ehrnsperger cStatus::OsdStatusMessage2() with the type of the message for suggesting to add mutex locks to protect creating/deleting cStatus and cEpgHandler objects + for reporting that cTimer::StartTime() may return different values, depending on the + parameters of a previous call to cTimer::Matches() Werner Färber for reporting a bug in handling the cPluginManager::Active() result when pressing diff --git a/HISTORY b/HISTORY index e91f52a5..8e88d05f 100644 --- a/HISTORY +++ b/HISTORY @@ -10138,10 +10138,29 @@ Video Disk Recorder Revision History - Fixed an invalid lock sequence when pressing the Channel+/Channel- keys while in the "What's on..." menu in live view. -2025-06-27: +2025-07-06: - Fixed cPoller::Poll() to allow negative timeout values again. - When regenerating the index of a recording, PID changes are now taken into account (reported by Christoph Haubrich). - In the "Timers" menu the '0' key now toggles between showing all timers and only the active ones (thanks to Matthias Senzel). +- Reworked cTimer::Matches() (triggered by Markus Ehrnsperger): + + Calculating the raw start/stop time of a timer has been moved to the new function + cTimer::CalcStartStopTime(). If a plugin calls cTimer::Matches() with Directly==true, + it should use cTimer::CalcStartStopTime() instead. If it calls cTimer::Matches() with + Directly==false, it should call cTimer::Matches(time_t t, int Margin) instead. + + The versions of cTimer::Matches() with 'bool Directly' are deprecated. Existing calls + with Directly==true are redirected to cTimer::CalcStartStopTime(), and a log message + is issued. + + cTimer::Matches() now reports an error if it is called with a time parameter that is + not the current time. + + The "first day" parameter of a repeating timer is now only reset if cTimer::Matches() + is called with t==0 and Margin==0 and it has been exceeded. + + Access to the cached cTimer::start-/stopTime members is now protected via a mutex. + + Plugins that use cTimer should continue to work as before. However, the author should + react accordingly to compile time and log messages regarding these modifications. + To test whether the plugin code will compile once the deprecated functions are + removed in a future version, the macro DEPRECATED_TIMER_MATCHES can be set to 0 in + timers.h. + APIVERSNUM is now 30009. diff --git a/config.h b/config.h index dce389ba..9bb4c5d5 100644 --- a/config.h +++ b/config.h @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: config.h 5.32 2025/06/20 14:13:54 kls Exp $ + * $Id: config.h 5.33 2025/07/06 15:06:55 kls Exp $ */ #ifndef __CONFIG_H @@ -27,8 +27,8 @@ // The plugin API's version number: -#define APIVERSION "8" -#define APIVERSNUM 30008 +#define APIVERSION "9" +#define APIVERSNUM 30009 // When loading plugins, VDR searches files by their APIVERSION, which // is different from VDRVERSION. APIVERSION is a plain number, incremented diff --git a/menu.c b/menu.c index dca61aa9..d03b2c0d 100644 --- a/menu.c +++ b/menu.c @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: menu.c 5.30 2025/06/27 09:05:20 kls Exp $ + * $Id: menu.c 5.31 2025/07/06 15:06:55 kls Exp $ */ #include "menu.h" @@ -1785,7 +1785,7 @@ eOSState cMenuWhatsOn::Record(void) Timer = t; return AddSubMenu(new cMenuEditTimer(Timer)); } - if (Timer->Matches(0, false, NEWTIMERLIMIT)) + if (Timer->Matches(0, NEWTIMERLIMIT)) return AddSubMenu(new cMenuEditTimer(Timer, true)); Timers->Add(Timer); Timers->SetModified(); @@ -2092,7 +2092,7 @@ eOSState cMenuSchedule::Record(void) Timer = t; return AddSubMenu(new cMenuEditTimer(Timer)); } - if (Timer->Matches(0, false, NEWTIMERLIMIT)) + if (Timer->Matches(0, NEWTIMERLIMIT)) return AddSubMenu(new cMenuEditTimer(Timer, true)); Timers->Add(Timer); Timers->SetModified(); diff --git a/timers.c b/timers.c index 122a9473..35bf73a4 100644 --- a/timers.c +++ b/timers.c @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: timers.c 5.25 2025/01/13 14:44:18 kls Exp $ + * $Id: timers.c 5.26 2025/07/06 15:06:55 kls Exp $ */ #include "timers.h" @@ -570,7 +570,7 @@ void cTimer::SetFile(const char *File) #define EITPRESENTFOLLOWINGRATE 10 // max. seconds between two occurrences of the "EIT present/following table for the actual multiplex" (2s by the standard, using some more for safety) #define EITPRESENTFOLLOWINGGRACE 60 // max. seconds before reporting a loss of VPS control of an ongoing recording -bool cTimer::Matches(time_t t, bool Directly, int Margin) const +void cTimer::CalcStartStopTime(time_t &startTime, time_t &stopTime, time_t t) const { startTime = stopTime = 0; if (t == 0) @@ -605,9 +605,60 @@ bool cTimer::Matches(time_t t, bool Directly, int Margin) const } if (!startTime) startTime = IncDay(t, 7); // just to have something that's more than a week in the future - else if (!Directly && (t > startTime || t > day + SECSINDAY + 3600)) // +3600 in case of DST change - day = 0; } +} + +#if DEPRECATED_TIMER_MATCHES +bool cTimer::Matches(time_t t, bool Directly) const +{ + if (Directly) { + static bool MatchesDirectlyReported = false; + if (!MatchesDirectlyReported) { + esyslog("ERROR: cTimer::Matches() called with Directly==true - use cTimer::CalcStartStopTime() instead"); + cBackTrace::BackTrace(); + MatchesDirectlyReported = true; + } + cMutexLock MutexLock(&mutex); + CalcStartStopTime(startTime, stopTime, t); + return startTime <= t && t < stopTime; + } + return Matches(t, 0); +} + +bool cTimer::Matches(time_t t, bool Directly, int Margin) const +{ + if (Directly) { + static bool MatchesDirectlyReported = false; + if (!MatchesDirectlyReported) { + esyslog("ERROR: cTimer::Matches() called with Directly==true - use cTimer::CalcStartStopTime() instead"); + cBackTrace::BackTrace(); + MatchesDirectlyReported = true; + } + cMutexLock MutexLock(&mutex); + CalcStartStopTime(startTime, stopTime, t); + return startTime <= t && t < stopTime; + } + return Matches(t, Margin); +} +#endif + +bool cTimer::Matches(time_t t, int Margin) const +{ + static bool TimeDiffReported = false; + bool IsNow = false; + if (t == 0) { + t = time(NULL); + IsNow = Margin == 0; + } + else if (!TimeDiffReported && abs(t - time(NULL)) > 10) { + esyslog("ERROR: cTimer::Matches() called with invalid time - use cTimer::CalcStartStopTime() instead"); + cBackTrace::BackTrace(); + TimeDiffReported = true; + } + cMutexLock MutexLock(&mutex); + CalcStartStopTime(startTime, stopTime, t); + if (IsNow && !IsSingleEvent() && (t > startTime || t > day + SECSINDAY + 3600)) // +3600 in case of DST change + day = 0; if (IsPatternTimer()) return false; // we only need to have start/stopTime initialized @@ -620,43 +671,41 @@ bool cTimer::Matches(time_t t, bool Directly, int Margin) const if (event) { if (HasFlags(tfVps)) { if (event->Vps()) { - if (Margin || !Directly) { - startTime = event->StartTime(); - stopTime = event->EndTime(); - if (!Margin) { // this is an actual check - const cSchedule *Schedule = event->Schedule(); - if (Schedule && Schedule->PresentSeenWithin(EITPRESENTFOLLOWINGRATE)) { // VPS control can only work with up-to-date events... - if (!vpsActive) { - vpsActive = true; - if (Recording()) - dsyslog("timer %s regained VPS control", *ToDescr()); - } - bool running = event->IsRunning(true); - if (!running) { - if (Recording() && vpsNotRunning == 0) - vpsNotRunning = time(NULL); - } - else - vpsNotRunning = 0; - return running || time(NULL) < vpsNotRunning + VPSGRACE; + startTime = event->StartTime(); + stopTime = event->EndTime(); + if (!Margin) { // this is an actual check + const cSchedule *Schedule = event->Schedule(); + if (Schedule && Schedule->PresentSeenWithin(EITPRESENTFOLLOWINGRATE)) { // VPS control can only work with up-to-date events... + if (!vpsActive) { + vpsActive = true; + if (Recording()) + dsyslog("timer %s regained VPS control", *ToDescr()); } - if (Recording()) { - if (Schedule && Schedule->PresentSeenWithin(EITPRESENTFOLLOWINGGRACE)) - return event->IsRunning(true); // give it a chance to recover - worst case: the recording will be 60 seconds too long - if (vpsActive) { - vpsActive = false; - esyslog("ERROR: timer %s lost VPS control", *ToDescr()); - } - // ...otherwise we fall back to normal timer handling below (note: Margin == 0!) + bool running = event->IsRunning(true); + if (!running) { + if (Recording() && vpsNotRunning == 0) + vpsNotRunning = time(NULL); } else - return false; // relying on vdr.c to ensure that a transponder is tuned to this channel + vpsNotRunning = 0; + return running || time(NULL) < vpsNotRunning + VPSGRACE; } + if (Recording()) { + if (Schedule && Schedule->PresentSeenWithin(EITPRESENTFOLLOWINGGRACE)) + return event->IsRunning(true); // give it a chance to recover - worst case: the recording will be 60 seconds too long + if (vpsActive) { + vpsActive = false; + esyslog("ERROR: timer %s lost VPS control", *ToDescr()); + } + // ...otherwise we fall back to normal timer handling below (note: Margin == 0!) + } + else + return false; // relying on vdr.c to ensure that a transponder is tuned to this channel } } } else if (HasFlags(tfSpawned)) { - if (!Margin && !Directly) { // this is an actual check + if (!Margin) { // this is an actual check // The spawned timer's start-/stopTimes are adjusted to the event's times in AdjustSpawnedTimer(). // However, in order to make sure the timer is set to the correct event, the margins at begin // end end are limited by the durations of the events before and after this timer's event. @@ -696,7 +745,8 @@ eTimerMatch cTimer::Matches(const cEvent *Event, int *Overlap) const return tmNone; UseVps = false; } - Matches(UseVps ? Event->Vps() : Event->StartTime(), true); + time_t startTime, stopTime; // not modifying the class members here! + CalcStartStopTime(startTime, stopTime, UseVps ? Event->Vps() : Event->StartTime()); int overlap = 0; if (UseVps) { if (startTime == Event->Vps()) { @@ -718,7 +768,6 @@ eTimerMatch cTimer::Matches(const cEvent *Event, int *Overlap) const overlap = FULLMATCH; } } - startTime = stopTime = 0; if (Overlap) *Overlap = overlap; return overlap >= FULLMATCH ? tmFull : overlap > 0 ? tmPartial : tmNone; @@ -749,7 +798,7 @@ bool cTimer::Expired(void) const if (FirstEvent) { if (Schedule) { for (const cEvent *e = FirstEvent; e; e = Schedule->Events()->Next(e)) { - if (e->Vps() == startTime) { + if (e->Vps() == StartTime()) { ExpireTime = e->EndTime() + EXPIRELATENCY; dsyslog("timer %s is waiting for next VPS event %s", *ToDescr(), *e->ToDescr()); // no break here - let's play it safe and look at *all* events @@ -770,6 +819,7 @@ bool cTimer::Expired(void) const time_t cTimer::StartTime(void) const { + cMutexLock MutexLock(&mutex); if (!startTime) Matches(); return startTime; @@ -777,6 +827,7 @@ time_t cTimer::StartTime(void) const time_t cTimer::StopTime(void) const { + cMutexLock MutexLock(&mutex); if (!stopTime) Matches(); return stopTime; @@ -944,9 +995,10 @@ bool cTimer::SetEventFromSchedule(const cSchedules *Schedules) // Normal timers match the event they have the most overlap with: int Overlap = 0; // Set up the time frame within which to check events: - Matches(0, true); - time_t TimeFrameBegin = StartTime() - EPGLIMITBEFORE; - time_t TimeFrameEnd = StopTime() + EPGLIMITAFTER; + time_t startTime, stopTime; // not modifying the class members here! + CalcStartStopTime(startTime, stopTime); + time_t TimeFrameBegin = startTime - EPGLIMITBEFORE; + time_t TimeFrameEnd = stopTime + EPGLIMITAFTER; for (const cEvent *e = Schedule->Events()->First(); e; e = Schedule->Events()->Next(e)) { if (e->EndTime() < TimeFrameBegin) continue; // skip events way before the timer starts @@ -1079,6 +1131,7 @@ bool cTimer::HasFlags(uint Flags) const void cTimer::Skip(void) { + cMutexLock MutexLock(&mutex); day = IncDay(SetTime(StartTime(), 0), 1); startTime = 0; SetEvent(NULL); diff --git a/timers.h b/timers.h index bdc823dd..ced66fcb 100644 --- a/timers.h +++ b/timers.h @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: timers.h 5.12 2025/03/02 11:03:35 kls Exp $ + * $Id: timers.h 5.13 2025/07/06 15:06:55 kls Exp $ */ #ifndef __TIMERS_H @@ -31,6 +31,7 @@ class cTimers; class cTimer : public cListObject { friend class cMenuEditTimer; private: + mutable cMutex mutex; int id; mutable time_t startTime, stopTime; ///< the time_t value calculated from 'day', 'start' and 'stop' int scheduleStateSet; @@ -95,11 +96,36 @@ public: void SetPattern(const char *Pattern); void SetFile(const char *File); bool IsPatternTimer(void) const { return *pattern; } - bool Matches(time_t t = 0, bool Directly = false, int Margin = 0) const; + void CalcStartStopTime(time_t &startTime, time_t &stopTime, time_t t = 0) const; + ///< Calculates the raw start and stop time of this timer, as given by the user in the timer definition. + ///< If t is given, and this is a repeating timer, the start and stop times on that day are returned + ///< (default is "today"). t can be any time_t value on the given day. +#define DEPRECATED_TIMER_MATCHES 1 +#if DEPRECATED_TIMER_MATCHES + // for backwards compatibility, remove these functions once Matches(time_t ...) has default parameters: + bool Matches(void) const { return Matches(0, 0); } + bool Matches(time_t t) const { return Matches(t, 0); } + [[deprecated("use CalcStartStopTime() instead")]] bool Matches(time_t t, bool Directly) const; + [[deprecated("use CalcStartStopTime() instead")]] bool Matches(time_t t, bool Directly, int Margin) const; + bool Matches(time_t t, int Margin) const; +#else + bool Matches(time_t t = 0, int Margin = 0) const; + ///< Returns true if this timer matches now. If several timers need to be checked with the exact same time, t can be given + ///< (which needs to be close to the current time, otherwise an error is logged). If Margin is given, it is subtracted from + ///< the timer's actual start time. This can be used to check whether the timer will start within the next Margin seconds. + ///< If called with t==0 and Margin==0 (which is regularly done), the "first day" parameter of a repeating timer is reset + ///< if it has been exceeded. + ///< The actual start/stop times of the timer are stored for retrieval via the StartTime() and StopTime() functions. + ///< For VPS timers these are the times of the respective EPG event. +#endif eTimerMatch Matches(const cEvent *Event, int *Overlap = NULL) const; bool Expired(void) const; - time_t StartTime(void) const; ///< the start time as given by the user - time_t StopTime(void) const; ///< the stop time as given by the user + time_t StartTime(void) const; + ///< The start time of this timer, which is the time as given by the user (for normal timers) or the start time + ///< of the event that is assigned to this timer (for VPS timers). + time_t StopTime(void) const; + ///< The stop time of this timer, which is the time as given by the user (for normal timers) or the end time + ///< of the event that is assigned to this timer (for VPS timers). time_t StartTimeEvent(void) const; ///< the start/stop times as given by the event (for VPS timers), by event plus margins (for spawned non-VPS timers), time_t StopTimeEvent(void) const; ///< or by the user (for normal timers) void SetId(int Id); diff --git a/vdr.c b/vdr.c index 6229e8ee..f3d36f6b 100644 --- a/vdr.c +++ b/vdr.c @@ -22,7 +22,7 @@ * * The project's page is at https://www.tvdr.de * - * $Id: vdr.c 5.19 2025/06/20 13:44:13 kls Exp $ + * $Id: vdr.c 5.20 2025/07/06 15:06:55 kls Exp $ */ #include @@ -1137,7 +1137,7 @@ int main(int argc, char *argv[]) bool NeedsTransponder = false; if (Timer->HasFlags(tfActive) && !Timer->Recording()) { if (Timer->HasFlags(tfVps)) { - if (Timer->Matches(Now, true, Setup.VpsMargin)) + if (Timer->Matches(Now, Setup.VpsMargin)) InVpsMargin = true; else if (Timer->Event()) { LOCK_SCHEDULES_READ; @@ -1152,7 +1152,7 @@ int main(int argc, char *argv[]) } } else - NeedsTransponder = Timer->Matches(Now, true, TIMERLOOKAHEADTIME); + NeedsTransponder = Timer->Matches(Now, TIMERLOOKAHEADTIME); } if (!Timer->Recording()) Timer->SetInVpsMargin(InVpsMargin);