the way it was done with timers in 2.3.1.
Using cRecordings list index as a recording id is a bad idea. If a recording is deleted between LSTR and DELR, the id passed to DELR might not be valid anymore. In VDR-2.3.2 (and, probably, in 2.3.1 too) even each subsequent DELR command in a series might invalidate the ids. For example:
# echo lstr | nc 172.17.0.1 6419 220 004f7f2b0687 SVDRP VideoDiskRecorder 2.3.2; Tue Mar 7 06:25:10 2017; UTF-8 250-1 07.03.17 02:00 0:00* TEST2 250-2 07.03.17 01:00 0:00* TEST1 250-3 07.03.17 04:00 0:00* TEST4 250 4 07.03.17 03:00 0:00* TEST3
# printf "delr 2\ndelr 3\nquit\n" | nc 172.17.0.1 6419 220 004f7f2b0687 SVDRP VideoDiskRecorder 2.3.2; Tue Mar 7 06:25:51 2017; UTF-8 250 Recording "2" deleted 250 Recording "3" deleted 221 004f7f2b0687 closing connection
# echo lstr | nc 172.17.0.1 6419 220 004f7f2b0687 SVDRP VideoDiskRecorder 2.3.2; Tue Mar 7 06:25:55 2017; UTF-8 250-1 07.03.17 02:00 0:00* TEST2 250 2 07.03.17 04:00 0:00* TEST4
Recording #4 (TEST3) was deleted instead of #3 (TEST4).
VDR-2.2.0 handles it as expected (as 'help DELR' says):
# echo lstr | nc 172.17.0.1 6419 220 004f7f2b0687 SVDRP VideoDiskRecorder 2.2.0; Tue Mar 7 06:35:13 2017; UTF-8 250-1 07.03.17 02:00 0:00* TEST2 250-2 07.03.17 01:00 0:00* TEST1 250-3 07.03.17 04:00 0:00* TEST4 250 4 07.03.17 03:00 0:00* TEST3
# printf "delr 2\ndelr 3\nquit\n" | nc 172.17.0.1 6419 220 004f7f2b0687 SVDRP VideoDiskRecorder 2.2.0; Tue Mar 7 06:35:30 2017; UTF-8 250 Recording "2" deleted 250 Recording "3" deleted 221 004f7f2b0687 closing connection
# echo lstr | nc 172.17.0.1 6419 220 004f7f2b0687 SVDRP VideoDiskRecorder 2.2.0; Tue Mar 7 06:35:34 2017; UTF-8 250-1 07.03.17 02:00 0:00* TEST2 250 2 07.03.17 03:00 0:00* TEST3
Those unique ids are not persistent and don't survive VDR restarts. The same is true about timers.
--- recording.c | 21 +++++++++++++++++++++ recording.h | 8 ++++++++ svdrp.c | 8 ++++---- 3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/recording.c b/recording.c index 5293459..e51b82e 100644 --- a/recording.c +++ b/recording.c @@ -753,6 +753,7 @@ char *LimitNameLengths(char *s, int PathMax, int NameMax)
cRecording::cRecording(cTimer *Timer, const cEvent *Event) { + id = 0; resume = RESUME_NOT_INITIALIZED; titleBuffer = NULL; sortBufferName = sortBufferTime = NULL; @@ -808,6 +809,7 @@ cRecording::cRecording(cTimer *Timer, const cEvent *Event)
cRecording::cRecording(const char *FileName) { + id = 0; resume = RESUME_NOT_INITIALIZED; fileSizeMB = -1; // unknown channel = -1; @@ -1463,6 +1465,8 @@ time_t cRecordings::lastUpdate = 0; cRecordings::cRecordings(bool Deleted) :cList<cRecording>(Deleted ? "DelRecs" : "Recordings") { + setRecordingId = !Deleted; + lastRecordingId = 0; }
cRecordings::~cRecordings() @@ -1507,6 +1511,15 @@ void cRecordings::Update(bool Wait) } }
+const cRecording *cRecordings::GetById(int Id) const +{ + for (const cRecording *r = First(); r; r = Next(r)) { + if (r->Id() == Id) + return r; + } + return NULL; +} + const cRecording *cRecordings::GetByName(const char *FileName) const { if (FileName) { @@ -1518,6 +1531,14 @@ const cRecording *cRecordings::GetByName(const char *FileName) const return NULL; }
+void cRecordings::Add(cRecording *Recording) +{ + if (setRecordingId) + // no need for locking, the caller must have a lock on the Recordigs list + Recording->SetId(++lastRecordingId); + cList<cRecording>::Add(Recording); +} + void cRecordings::AddByName(const char *FileName, bool TriggerUpdate) { if (!GetByName(FileName)) { diff --git a/recording.h b/recording.h index b649f6f..689f5de 100644 --- a/recording.h +++ b/recording.h @@ -97,6 +97,7 @@ public: class cRecording : public cListObject { friend class cRecordings; private: + int id; mutable int resume; mutable char *titleBuffer; mutable char *sortBufferName; @@ -116,6 +117,7 @@ private: static char *StripEpisodeName(char *s, bool Strip); char *SortName(void) const; void ClearSortName(void); + void SetId(int Id) { id = Id; } // should only be set by cRecordings time_t start; int priority; int lifetime; @@ -124,6 +126,7 @@ public: cRecording(cTimer *Timer, const cEvent *Event); cRecording(const char *FileName); virtual ~cRecording(); + int Id(void) const { return id; } time_t Start(void) const { return start; } int Priority(void) const { return priority; } int Lifetime(void) const { return lifetime; } @@ -220,6 +223,8 @@ class cVideoDirectoryScannerThread;
class cRecordings : public cList<cRecording> { private: + bool setRecordingId; + int lastRecordingId; static cRecordings recordings; static cRecordings deletedRecordings; static char *updateFileName; @@ -254,8 +259,11 @@ public: static bool NeedsUpdate(void); void ResetResume(const char *ResumeFileName = NULL); void ClearSortNames(void); + const cRecording *GetById(int Id) const; + cRecording *GetById(int Id) { return const_cast<cRecording *>(static_cast<const cRecordings *>(this)->GetById(Id)); }; const cRecording *GetByName(const char *FileName) const; cRecording *GetByName(const char *FileName) { return const_cast<cRecording *>(static_cast<const cRecordings *>(this)->GetByName(FileName)); } + void Add(cRecording *Recording); void AddByName(const char *FileName, bool TriggerUpdate = true); void DelByName(const char *FileName); void UpdateByName(const char *FileName); diff --git a/svdrp.c b/svdrp.c index 1697cc6..607201c 100644 --- a/svdrp.c +++ b/svdrp.c @@ -1280,7 +1280,7 @@ void cSVDRPServer::CmdDELR(const char *Option) if (isnumber(Option)) { LOCK_RECORDINGS_WRITE; Recordings->SetExplicitModify(); - if (cRecording *Recording = Recordings->Get(strtol(Option, NULL, 10) - 1)) { + if (cRecording *Recording = Recordings->GetById(strtol(Option, NULL, 10))) { if (int RecordingInUse = Recording->IsInUse()) Reply(550, "%s", *RecordingInUseMessage(RecordingInUse, Option, Recording)); else { @@ -1707,7 +1707,7 @@ void cSVDRPServer::CmdLSTR(const char *Option) p = strtok_r(NULL, delim, &strtok_next); } if (Number) { - if (const cRecording *Recording = Recordings->Get(strtol(Option, NULL, 10) - 1)) { + if (const cRecording *Recording = Recordings->GetById(strtol(Option, NULL, 10))) { FILE *f = fdopen(file, "w"); if (f) { if (Path) @@ -1729,7 +1729,7 @@ void cSVDRPServer::CmdLSTR(const char *Option) else if (Recordings->Count()) { const cRecording *Recording = Recordings->First(); while (Recording) { - Reply(Recording == Recordings->Last() ? 250 : -250, "%d %s", Recording->Index() + 1, Recording->Title(' ', true)); + Reply(Recording == Recordings->Last() ? 250 : -250, "%d %s", Recording->Id(), Recording->Title(' ', true)); Recording = Recordings->Next(Recording); } } @@ -1940,7 +1940,7 @@ void cSVDRPServer::CmdMOVR(const char *Option) if (isnumber(num)) { LOCK_RECORDINGS_WRITE; Recordings->SetExplicitModify(); - if (cRecording *Recording = Recordings->Get(strtol(num, NULL, 10) - 1)) { + if (cRecording *Recording = Recordings->GetById(strtol(num, NULL, 10))) { if (int RecordingInUse = Recording->IsInUse()) Reply(550, "%s", *RecordingInUseMessage(RecordingInUse, Option, Recording)); else {