RFC: recording strategy on timer conflicts
Commit Message
Hi,
I'm currently working on a tool that checks for timer conflicts and became
aware of the following problem:
Suppose you have the following 3 timers on a system with two DVB cards:
nr. transponder - start - priority
1. A - 20:10 - 45
2. B - 20:10 - 5
3. C - 20:11 - 50
A,B,C are different transponders. VDR will start recording from transponder A
at 20:10 with device 2. The same for transponder B with device 1.
At 20:11 we have a conflict and IMHO VDR should stop timer 2 and start timer 3
on device 1.
But when I check the logs the following occures:
VDR first stops timer 1 on device 2 and starts timer 3 on device 2. Then it
recognizes the pending timer 1 with higher priority than the running timer 2
and stops this one. So in the end all is fine, but there is a small break
about 1-2 seconds in recording 1.
The problem is, that cDevice::GetDevice handles the two devices in this case
with the same priority and therefore returns the device with the highest
number. I think one could fix this with a small change (patch against 1.4.0
is attached), but the whole thing is really complicated and I do not have a
real deep insight in the side effects ;-) , so perhaps you could give a
comment on this.
BR,
Christian
Comments
Am Samstag, 20. Mai 2006 08:40 schrieb Christian Wieninger:
> The problem is, that cDevice::GetDevice handles the two devices in this
> case with the same priority and therefore returns the device with the
> highest number. I think one could fix this with a small change (patch
> against 1.4.0 is attached), but the whole thing is really complicated and I
> do not have a real deep insight in the side effects ;-) , so perhaps you
> could give a comment on this.
>
no comment on this? ;-)
Christian
Christian Wieninger schrieb:
>Hi,
>
>I'm currently working on a tool that checks for timer conflicts and became
>aware of the following problem:
>
>Suppose you have the following 3 timers on a system with two DVB cards:
>
>nr. transponder - start - priority
>1. A - 20:10 - 45
>2. B - 20:10 - 5
>3. C - 20:11 - 50
>
>A,B,C are different transponders. VDR will start recording from transponder A
>at 20:10 with device 2. The same for transponder B with device 1.
>At 20:11 we have a conflict and IMHO VDR should stop timer 2 and start timer 3
>on device 1.
>But when I check the logs the following occures:
>VDR first stops timer 1 on device 2 and starts timer 3 on device 2. Then it
>recognizes the pending timer 1 with higher priority than the running timer 2
>and stops this one. So in the end all is fine, but there is a small break
>about 1-2 seconds in recording 1.
>The problem is, that cDevice::GetDevice handles the two devices in this case
>with the same priority and therefore returns the device with the highest
>number. I think one could fix this with a small change (patch against 1.4.0
>is attached), but the whole thing is really complicated and I do not have a
>real deep insight in the side effects ;-) , so perhaps you could give a
>comment on this.
>
Was this the case also with older VDR-versions? I always thought that
VDR behaves the way you also expected and avoids interrupting a
recording ...
I don't have a in depth look into this code either, but shouldn't
cDevice::GetDevice already only prefer device 1 (the one recording timer
2). As it loops through the devices it should give device 1 the priority
5 and as it comes to device 2 it should evaluate it to priority 7, as
Priority of the device 2 is not smaller than the Priority of device 1
(which is one of the conditions for priority 5).
Is that right, or am I missing something here? But why does VDR stop
both timers?
Bye,
Andreas Brugger
Am Montag, 22. Mai 2006 20:58 schrieb Andreas Brugger:
> Was this the case also with older VDR-versions? I always thought that
> VDR behaves the way you also expected and avoids interrupting a
> recording ...
Atleast with 1.3.44 and above the problem exists.
> I don't have a in depth look into this code either, but shouldn't
> cDevice::GetDevice already only prefer device 1 (the one recording timer
> 2). As it loops through the devices it should give device 1 the priority
> 5 and as it comes to device 2 it should evaluate it to priority 7, as
> Priority of the device 2 is not smaller than the Priority of device 1
> (which is one of the conditions for priority 5).
With 'Priority' you probably mean the variable 'pri' used in GetDevice?
In the case described before, GetDevice (1.4.0) sets 'pri' to 8 for both
devices: The first loop for device 1 has no previously selected device ('d')
and so 'pri' is set to 8, since device 1 is receiving. In the second loop
device 2 has priority bigger than device 1 and so again 'pri' is set to 8.
So device 2 will be returned resulting in a short break of recording 1 with
priority 45.
The idea of my patch is to stretch the pri-value to MAXPRIORITY+1 and to add
the current priority of the receiving device in this case. Perhaps it would
even be a better approach to add the device priority in any case if a device
is currently receiving.
Christian
Christian Wieninger schrieb:
>>I don't have a in depth look into this code either, but shouldn't
>>cDevice::GetDevice already only prefer device 1 (the one recording timer
>>2). As it loops through the devices it should give device 1 the priority
>>5 and as it comes to device 2 it should evaluate it to priority 7, as
>>Priority of the device 2 is not smaller than the Priority of device 1
>>(which is one of the conditions for priority 5).
>>
>>
>
>With 'Priority' you probably mean the variable 'pri' used in GetDevice?
>
>
Yes thats right.
>In the case described before, GetDevice (1.4.0) sets 'pri' to 8 for both
>devices: The first loop for device 1 has no previously selected device ('d')
>and so 'pri' is set to 8, since device 1 is receiving. In the second loop
>device 2 has priority bigger than device 1 and so again 'pri' is set to 8.
>So device 2 will be returned resulting in a short break of recording 1 with
>priority 45.
>
>The idea of my patch is to stretch the pri-value to MAXPRIORITY+1 and to add
>the current priority of the receiving device in this case. Perhaps it would
>even be a better approach to add the device priority in any case if a device
>is currently receiving.
>
>
Oh, that was a stupid misinterpretation from me. You are right of course.
I think there is the general problem of the cDevice::GetDevice-Routine
that in the first run there is no selected device. This should effect
all the conditions where there is a check for d != NULL, so maybe the
easiest fix is to set the Device in the beginning of the routine to one
of the devices (first or last):
> - cDevice *d = NULL;
> + cDevice *d = device[0]; // or device[numDevices]
So the two timers should evaluate to different priorities.
Any thoughts on that? Do I miss something again?
Am Dienstag, 23. Mai 2006 18:30 schrieb Andreas Brugger:
> of the devices (first or last):
> > - cDevice *d = NULL;
> > + cDevice *d = device[0]; // or device[numDevices]
>
> So the two timers should evaluate to different priorities.
>
> Any thoughts on that? Do I miss something again?
I think so ;-) because if d is set to first device then
else if (d && device[i]->Priority() < d->Priority())
pri = 6; // receiving but priority is lower
the first loop does not match the if condition. It would work if d was
initially set to the last device. But I don't know if this would make
problems in any other cases.
Christian
Christian Wieninger schrieb:
>Am Dienstag, 23. Mai 2006 18:30 schrieb Andreas Brugger:
>
>
>>of the devices (first or last):
>>
>>
>>>- cDevice *d = NULL;
>>>+ cDevice *d = device[0]; // or device[numDevices]
>>>
>>>
>>So the two timers should evaluate to different priorities.
>>
>>Any thoughts on that? Do I miss something again?
>>
>>
>
>I think so ;-) because if d is set to first device then
>
> else if (d && device[i]->Priority() < d->Priority())
> pri = 6; // receiving but priority is lower
>
>the first loop does not match the if condition. It would work if d was
>initially set to the last device. But I don't know if this would make
>problems in any other cases.
>
>
Ohhh ... right! I haven't thought that through completely it seems ...
there is also a problem that the first (or last) device would be
returned even if the devices doesn't provide the channel.
This isn't as easy as I thought ...
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).
Christian
@@ -282,6 +282,7 @@
{
cDevice *d = NULL;
int select = INT_MAX;
+ int priStep = MAXPRIORITY+1;
for (int i = 0; i < numDevices; i++) {
bool ndr;
@@ -290,21 +291,21 @@
if (device[i]->Receiving() && !ndr)
pri = 0; // receiving and allows additional receivers
else if (!device[i]->Receiving(true) && d && device[i]->ProvidesCa(Channel) < d->ProvidesCa(Channel))
- pri = 1; // free and fewer Ca's
+ pri = priStep; // free and fewer Ca's
else if (!device[i]->Receiving() && !device[i]->HasDecoder())
- pri = 2; // free and not a full featured card
+ pri = 2*priStep; // free and not a full featured card
else if (!device[i]->Receiving() && device[i] != ActualDevice())
- pri = 3; // free and not the actual device
+ pri = 3*priStep; // free and not the actual device
else if (!device[i]->Receiving() && !device[i]->IsPrimaryDevice())
- pri = 4; // free and not the primary device
+ pri = 4*priStep; // free and not the primary device
else if (!device[i]->Receiving())
- pri = 5; // free
- else if (d && device[i]->Priority() < d->Priority())
- pri = 6; // receiving but priority is lower
+ pri = 5*priStep; // free
+ else if ((d && device[i]->Priority() < d->Priority()) || device[i]->Priority() < Priority)
+ pri = 6*priStep + device[i]->Priority(); // receiving but priority is lower
else if (d && device[i]->Priority() == d->Priority() && device[i]->ProvidesCa(Channel) < d->ProvidesCa(Channel))
- pri = 7; // receiving with same priority but fewer Ca's
+ pri = 7*priStep; // receiving with same priority but fewer Ca's
else
- pri = 8; // all others
+ pri = 8*priStep; // all others
if (pri <= select) {
select = pri;
d = device[i];