LinuxTV Patchwork Use unique recording ids for LSTR/DELR/MOVR

login
register
mail settings
Submitter glenvt18
Date March 14, 2017, 1:13 a.m.
Message ID <1489454026-17819-1-git-send-email-glenvt18@gmail.com>
Download mbox | patch
Permalink /patch/40025/
State New
Headers show

Comments

glenvt18 - March 14, 2017, 1:13 a.m.
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(-)

Patch

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 {

Privacy Policy