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 {
--
Sergey Chernyavskiy
