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);
I'll need to give this more thought - won't have time for it today.
Klaus
