RFC: recording strategy on timer conflicts

Message ID 4477000B.60408@cadsoft.de
State New
Headers

Commit Message

Klaus Schmidinger May 26, 2006, 1:18 p.m. UTC
  Christian Wieninger wrote:
> Am Dienstag, 23. Mai 2006 22:01 schrieb Andreas Brugger:
>> This isn't as easy as I thought ...
> 
> no it isn't ;-) But I think my patch should solve the problem. At least it 
> works fine here. So let's see what Klaus is thinking of it (if he is reading 
> this).

I believe the main problem with the current GetDevice() function
is that it doesn't query *all* devices, but rather selects the "first best"
device.

The attached patch implements a new way of selecting the device.
It makes sure all devices are queried and selects the one with
the least "impact".

The various criteria are represented as parts of an integer number,
and in the end the device with the smallest value wins. The most
important criteria are pushed into the integer first, resulting in
the highest order bits, thus making the most difference.

In this patch the new calculation is done in parallel to the old one,
and the result is logged in case the decision would be different.
The final result is still taken from the old calculation, but once
you find the new one stable enough, you can enable the 'return dd'
line.

There is a lot of debug output in there, so that we can see the
individual parts of the decision making. This will be removed
in the final version.

Please give this a try, and let me know whether it fixes all known
problems.

Klaus
  

Comments

Christian Wieninger May 27, 2006, 5:58 a.m. UTC | #1
Am Freitag, 26. Mai 2006 15:18 schrieb Klaus Schmidinger:
> Please give this a try, and let me know whether it fixes all known
> problems.
>

A quick test of the problem described previously seems to work fine:
(I'm listing only the relevant log entries, dd as return value was not active)

// Start of recording with prioritiy 45 at 00:27 on device 2
Mai 27 00:27:00 linux vdr: [5506] switching device 2 to channel 2
Mai 27 00:27:00 linux vdr: [5506] timer 65 (2 0027-0100 'Blond am Freitag') start
Mai 27 00:27:00 linux vdr: [5506] record /video0/Blond_am_Freitag/2006-05-27.00.27.45.99.rec
...
// Start of recording with prioritiy 5 at 00:27 on device 1
Mai 27 00:27:01 linux vdr: [5506] switching device 1 to channel 1
Mai 27 00:27:02 linux vdr: [5506] timer 66 (1 0027-0125 'Nachtmagazin') start
Mai 27 00:27:02 linux vdr: [5506] record /video0/Nachtmagazin/2006-05-27.00.27.05.99.rec
...
// Start of recording with priority 50 at 00:29
Mai 27 00:29:00 linux vdr: [5506] ******* new GetDevice would select device 1 in
stead of 2
Mai 27 00:29:00 linux vdr: [5506] stopping recording on DVB device 2 due to higher priority
Mai 27 00:29:03 linux vdr: [5506] switching device 2 to channel 6
Mai 27 00:29:03 linux vdr: [5506] timer 67 (6 0029-0220 'Nich' mit Leo') start
Mai 27 00:29:03 linux vdr: [5506] record /video0/Nich'_mit_Leo/2006-05-27.00.29.50.99.rec
...
Mai 27 00:29:03 linux vdr: [5506] stopping recording on DVB device 1 due to higher priority
...
Mai 27 00:29:04 linux vdr: [5506] switching device 1 to channel 2
Mai 27 00:29:04 linux vdr: [5506] timer 65 (2 0027-0100 'Blond am Freitag') start
Mai 27 00:29:04 linux vdr: [5506] record /video0/Blond_am_Freitag/2006-05-27.00.27.45.99.rec

> //XXX what if Priority() is < -1? -> limit to -1..MAXPRIORITY???

how can one set a priority below -1 and what would this mean? As far as 
understood the cDevice class, this can only be done in a cDevice-derived 
class. So perhaps cDevice::Priority should limit its return value to -1 ... MAXPRIORITY?

Christian
  
Klaus Schmidinger May 27, 2006, 9:02 a.m. UTC | #2
Christian Wieninger wrote:
> ...
>> //XXX what if Priority() is < -1? -> limit to -1..MAXPRIORITY???
> 
> how can one set a priority below -1 and what would this mean? As far as 
> understood the cDevice class, this can only be done in a cDevice-derived 
> class. So perhaps cDevice::Priority should limit its return value to -1 ... MAXPRIORITY?

The priority comes from the cReveicers attached to the device:

   cReceiver(int Ca, int Priority, int Pid, const int *Pids1 = NULL, const int *Pids2 = NULL, const int *Pids3 = NULL);
       ...
       ///< Priority may be any value in the range 0..99. Negative values indicate
       ///< that this cReceiver may be detached at any time (without blocking the
       ///< cDevice it is attached to).

So officially the value can't be greater than 99, but it may well
be any negative value. However, VDR itself only uses '-1', and I guess
it's fair to assume that no plugin will be using too large negative
values. To be on the safe side I guess I'll do

       imp <<= 8; imp |= min(max(device[i]->Priority() + MAXPRIORITY, 0), 0xFF);

which would allow values in the range -99..99, and in any case limit the
result to 0..255.

Klaus
  

Patch

--- device.c	2006/04/14 14:34:43	1.128
+++ device.c	2006/05/26 13:07:12
@@ -313,6 +313,47 @@ 
             }
          }
       }
+  //XXX
+  {
+  cDevice *dd = NULL;//XXX
+  uint Impact = 0xFFFFFFFF;
+  for (int i = 0; i < numDevices; i++) {
+      printf("%d  ", i + 1);//XXX
+      bool ndr;
+      if (device[i]->ProvidesChannel(Channel, Priority, &ndr)) { // this device is basicly able to do the job
+         uint imp = 0;
+         imp <<= 1; imp |= !device[i]->Receiving() || ndr;
+         imp <<= 1; imp |= device[i]->Receiving();
+         imp <<= 1; imp |= device[i] == ActualDevice();
+         imp <<= 1; imp |= device[i]->IsPrimaryDevice();
+         imp <<= 1; imp |= device[i]->HasDecoder();
+         imp <<= 8; imp |= device[i]->Priority() + 1; //XXX what if Priority() is < -1? -> limit to -1..MAXPRIORITY???
+         imp <<= 8; imp |= device[i]->ProvidesCa(Channel); //XXX limit to 0..255???
+         if (imp < Impact) {
+            Impact = imp;
+            dd = device[i];
+            }
+         printf(" %d",  !device[i]->Receiving() || ndr);//XXX
+         printf(" %d",  device[i]->Receiving());//XXX
+         printf(" %d",  device[i] == ActualDevice());//XXX
+         printf(" %d",  device[i]->IsPrimaryDevice());//XXX
+         printf(" %d",  device[i]->HasDecoder());//XXX
+         printf(" %3d", device[i]->Priority() + 1);//XXX
+         printf(" %d",  device[i]->ProvidesCa(Channel));//XXX
+         printf(" - %08X ", imp);//XXX
+         }
+      else {//XXX
+         printf(" -");//XXX
+         }//XXX
+      printf("\n");//XXX
+      }
+  if (dd && d)//XXX
+     printf("--> %d  (%d)\n", dd->CardIndex() + 1, d->CardIndex() + 1);//XXX
+  if (dd && d && dd != d)//XXX
+     dsyslog("******* new GetDevice would select device %d instead of %d", dd->CardIndex() + 1, d->CardIndex() + 1);//XXX
+  //return dd; //XXX activate this if you trust it
+  }
+  //XXX
   return d;
 }