Klaus Schmidinger wrote:
> Anssi Hannula wrote:
>> Klaus Schmidinger wrote:
>>> Anssi Hannula wrote:
>>>
>>>> Klaus Schmidinger wrote:
>>>>
>>>>> Anssi Hannula wrote:
>>>>>
>>>>>> Klaus Schmidinger wrote:
>>>>>>
>>>>>>> I'm afraid this patch has a nasty side effect.
>>>>>>>
>>>>>>> Asssume the following scenario:
>>>>>>>
>>>>>>> - two devices, primary is 1
>>>>>>> - all channels are available on both devices, except for channels
>>>>>>> 10, 11 and 12 which can only be received by device 2
>>>>>>> - channels 10 and 11 are on the same transponder, channel 12 is on a
>>>>>>> different one
>>>>>>> - switch directly to channel 10 -> transfer mode starts
>>>>>>> - press "Up" to switch to channel 11, but that channel is skipped
>>>>>>> and
>>>>>>> it switches to channel 12
>>>>>>>
>>>>>>> So I guess I can't accept this patch.
>>>>>>
>>>>>> Thank you for taking a look.
>>>>>>
>>>>>> However, I don't see why there is such a problem in the above
>>>>>> scenario:
>>>>>>
>>>>>> - let's consider PrimaryLimit is 0 (the default)
>>>>>> - SwitchChannel() gets called (device.c line 581)
>>>>>> - PrimaryDevice()->ProvidesChannel() returns false (line 591)
>>>>>> - GetDevice() is called (line 281)
>>>>>> - for the device 1 ProvidesChannel() returns false (line 288)
>>>>>> - ProvidesChannel() is called for device 2 (dvbdevice.c line 767)
>>>>>> - Priority = 0, this->Priority() = -1, so hasPriority = true
>>>>>> - ProvidesChannel() result = true is set in line 785
>>>>>> - ProvidesChannel() return true and ndr = false (device.c line 288)
>>>>>> - (device[i]->Receiving() && !ndr) evaluates true (line 290)
>>>>>> - pri = 0 so device 2 is selected
>>>>>> - SwitchChannel() calls another SwitchChannel (line 601)
>>>>>> - SwitchChannel() calls SetChannel (line 568)
>>>>>> - and so on
>>>>>>
>>>>>> Hopefully I'll have time to test this scenario later today.
>>>>>
>>>>> Well, just try it - it did behave here as described.
>>>>
>>>> Well, I tried it in the following scenario:
>>>> - two devices, primary is 1
>>>> - all channels available on device 2 only
>>>> - channels 1 and 2 are on the same transponder, 3 and 4 on a different
>>>> one
>>>> - manually switch to channel 1
>>>> - press up -> vdr switches to 2
>>>> - up again -> vdr switches to 3
>>>> - up again -> vdr switches to 4
>>>>
>>>> It works ok.
>>>> Do you have PrimaryLimit != 0? As it turns out, there is indeed a
>>>> bug in
>>>
>>> Well, I normally have PrimaryLimit=0, but I've set it to 10 for testing.
>>> Sorry, forgot to mention that.
>>>
>>>> the patch which appears when PrimaryLimit is something else than 0 and
>>>> affects the channelup & channeldown.
>>>> If you do, try one of the following:
>>>> 1. The above patch with only the Receiving() change.
>>>> 2. The above patch fully and additionally Priority of GetDevice() call
>>>> in device.c line 591 changed from 0 to Setup.PrimaryLimit.
>>>> 3. PrimaryLimit = 0
>>>
>>> Sorry, it's late today - but see below, maybe this is all no longer
>>> a problem, anyway ;-)
>>
>> It is, but hopefully not for long :)
>>
>>>>> Looking back at your original posting, I believe the actual problem
>>>>> that
>>>>> triggered all this was that streamdev would interrupt live viewing in
>>>>> Transfer Mode, right?
>>>>
>>>> Yes.
>>>>
>>>>> Well, what if streamdev, when selecting the device
>>>>> to use, could avoid the ActualDevice()?
>>>>>
>>>>> To test this (without modifying the cDevice API) you could introduce a
>>>>>
>>>>> bool AvoidActualDevice = false;
>>>>>
>>>>> right before
>>>>>
>>>>> cDevice *cDevice::GetDevice(const cChannel *Channel, int Priority,
>>>>> bool
>>>>> *NeedsDetachReceivers)
>>>>>
>>>>> and inside that function check that variable, and if it is true and
>>>>> the
>>>>> checked device is the ActualDevice(), continue the 'for' loop:
>>>>
>>>> I think it would be much better to fix this bug (VDR doesn't prefer
>>>> free
>>>> devices over transfer-moded) rather than add this kind of workaround.
>>>
>>> VDR does prefer free devices over ones used for Transfer Mode:
>>
>> No, it doesn't, if the transfer-moded device is a budget card:
>>
>>> Device *cDevice::GetDevice(const cChannel *Channel, int Priority, bool
>>> *NeedsDetachReceivers)
>>> {
>>> ...
>>> else if (!device[i]->Receiving() && !device[i]->HasDecoder())
>>> pri = 2; // free and not a full featured card
>>
>> This causes the device to be selected no matter if it is in transfer
>> mode or not.
>
> Ok, you're right about that.
>
>> It seems to me that the above "2" case and the below "3" case should be
>> swapped.
>>
>> However, Receiving() is supposed to be "Returns true if we are currently
>> receiving." so I think it should still check for transfermode, as it is
>> then definitely receiving. Then the "free and not the actual device"
>> case could be dropped from GetDevice().
>>
>>
>>> else if (!device[i]->Receiving() && device[i] !=
>>> ActualDevice())
>>> pri = 3; // free and not the actual device
>>> else if (!device[i]->Receiving() &&
>>> !device[i]->IsPrimaryDevice())
>>> pri = 4; // free and not the primary device
>>> ...
>>> }
>>>
>>> Hmm, come to think of it: could it be that your original problem
>>> no longer exists since VDR 1.3.46?
>>>
>>> 2006-04-09: Version 1.3.46
>>>
>>> ...
>>> - Now avoiding the 'actual' device when starting a recording, so that a
>>> Transfer
>>> Mode for live tv isn't interrupted.
>>
>> Nope, I'm running 1.4.0 and I also confirmed the problem still exists
>> with vanilla VDR.
>>
>> I've now attached two patches. One adds a check for transfer mode in
>> Receiving() and drops the then-useless "free and not the actual device"
>> case from GetDevice(). The other one is optional: it changes the
>> effective transfer-mode receiver priority from fixed 0 to PrimaryLimit,
>> so that the transfer-mode is also "protected" by PrimaryLimit.
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>> diff -Nurp -x '*~' vdr-1.4.0/device.c vdr-1.4.0-fix2/device.c
>> --- vdr-1.4.0/device.c 2006-04-14 17:34:43.000000000 +0300
>> +++ vdr-1.4.0-fix2/device.c 2006-05-14 01:10:25.000000000 +0300
>> @@ -293,18 +293,16 @@ cDevice *cDevice::GetDevice(const cChann
>> pri = 1; // free and fewer Ca's
>> else if (!device[i]->Receiving() && !device[i]->HasDecoder())
>> pri = 2; // free and not a full featured card
>> - else if (!device[i]->Receiving() && device[i] !=
>> ActualDevice())
>> - pri = 3; // free and not the actual device
>> else if (!device[i]->Receiving() &&
>> !device[i]->IsPrimaryDevice())
>> - pri = 4; // free and not the primary device
>> + pri = 3; // free and not the primary device
>> else if (!device[i]->Receiving())
>> - pri = 5; // free
>> + pri = 4; // free
>> else if (d && device[i]->Priority() < d->Priority())
>> - pri = 6; // receiving but priority is lower
>> + pri = 5; // 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 = 6; // receiving with same priority but fewer Ca's
>> else
>> - pri = 8; // all others
>> + pri = 7; // all others
>> if (pri <= select) {
>> select = pri;
>> d = device[i];
>> @@ -1183,6 +1181,8 @@ int cDevice::ProvidesCa(const cChannel *
>>
>> bool cDevice::Receiving(bool CheckAny) const
>> {
>> + if (this == cTransferControl::ReceiverDevice())
>> + return true;
>> for (int i = 0; i < MAXRECEIVERS; i++) {
>> if (receiver[i] && (CheckAny || receiver[i]->priority >= 0)) //
>> cReceiver with priority < 0 doesn't count
>> return true;
>>
>>
>> ------------------------------------------------------------------------
>>
>> diff -Nurp -x '*~' vdr-1.4.0/device.c vdr-1.4.0-fix/device.c
>> --- vdr-1.4.0/device.c 2006-04-14 17:34:43.000000000 +0300
>> +++ vdr-1.4.0-fix/device.c 2006-05-14 01:06:32.000000000 +0300
>> @@ -588,7 +586,7 @@ bool cDevice::SwitchChannel(int Directio
>> cChannel *channel;
>> while ((channel = Channels.GetByNumber(n, Direction)) != NULL) {
>> // try only channels which are currently available
>> - if (PrimaryDevice()->ProvidesChannel(channel,
>> Setup.PrimaryLimit) || PrimaryDevice()->CanReplay() &&
>> GetDevice(channel, 0))
>> + if (PrimaryDevice()->ProvidesChannel(channel,
>> Setup.PrimaryLimit) || PrimaryDevice()->CanReplay() &&
>> GetDevice(channel, Setup.PrimaryLimit))
>> break;
>> n = channel->Number() + Direction;
>> }
>> @@ -627,7 +625,7 @@ eSetChannelResult cDevice::SetChannel(co
>> // use the card that actually can receive it and transfer data from
>> there:
>>
>> if (NeedsTransferMode) {
>> - cDevice *CaDevice = GetDevice(Channel, 0, &NeedsDetachReceivers);
>> + cDevice *CaDevice = GetDevice(Channel, Setup.PrimaryLimit,
>> &NeedsDetachReceivers);
>> if (CaDevice && CanReplay()) {
>> cStatus::MsgChannelSwitch(this, 0); // only report status if
>> we are actually going to switch the channel
>> if (CaDevice->SetChannel(Channel, false) == scrOk) { //
>> calling SetChannel() directly, not SwitchChannel()!
>> @@ -1158,7 +1156,7 @@ int cDevice::Ca(void) const
>>
>> int cDevice::Priority(void) const
>> {
>> - int priority = IsPrimaryDevice() ? Setup.PrimaryLimit - 1 :
>> DEFAULTPRIORITY;
>> + int priority = ActualDevice() == this ? Setup.PrimaryLimit - 1 :
>> DEFAULTPRIORITY;
>> for (int i = 0; i < MAXRECEIVERS; i++) {
>> if (receiver[i])
>> priority = max(receiver[i]->priority, priority);
@Anssi: can you please check whether my latest changes to cDevice::GetDevice()
have also fixed this problem? See the patch vdr-1.4.0-getdevice-2.diff posted
under "RFC: recording strategy on timer conflicts".
Klaus
