Message ID | 44640281.3030702@gmail.com |
---|---|
State | New |
Headers |
Received: from pne-smtpout4-sn2.hy.skanova.net ([81.228.8.154]) by www.linuxtv.org with esmtp (Exim 4.50) id 1FeORn-0007ZD-7K for vdr@linuxtv.org; Fri, 12 May 2006 05:36:11 +0200 Received: from mail.onse.fi (80.223.77.223) by pne-smtpout4-sn2.hy.skanova.net (7.2.070) id 444499DE00140017 for vdr@linuxtv.org; Fri, 12 May 2006 05:35:40 +0200 Received: from [10.0.0.4] (kannettava [10.0.0.4]) by mail.onse.fi (Postfix) with ESMTP id E51E946C41F4 for <vdr@linuxtv.org>; Fri, 12 May 2006 06:35:35 +0300 (EEST) Message-ID: <44640281.3030702@gmail.com> Date: Fri, 12 May 2006 06:35:29 +0300 From: Anssi Hannula <anssi.hannula@gmail.com> User-Agent: Mozilla Thunderbird 1.0.6 (Windows/20050716) X-Accept-Language: en-us, en MIME-Version: 1.0 To: VDR Mailing List <vdr@linuxtv.org> Subject: Re: [vdr] [PATCH] Priority of transfer-mode should not be -1 References: <JJEEJFMIKKDPLKGCNPNCEEFDCKAA.bergwinkl.thomas@vr-web.de> <4306F832.9030407@cadsoft.de> <4311D492.8040602@gmail.com> <43122CB3.30105@cadsoft.de> <446255A9.2010304@gmail.com> In-Reply-To: <446255A9.2010304@gmail.com> Content-Type: multipart/mixed; boundary="------------080106060004060504020904" X-BeenThere: vdr@linuxtv.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: VDR Mailing List <vdr@linuxtv.org> List-Id: VDR Mailing List <vdr.linuxtv.org> List-Unsubscribe: <http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr>, <mailto:vdr-request@linuxtv.org?subject=unsubscribe> List-Archive: <http://www.linuxtv.org/pipermail/vdr> List-Post: <mailto:vdr@linuxtv.org> List-Help: <mailto:vdr-request@linuxtv.org?subject=help> List-Subscribe: <http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr>, <mailto:vdr-request@linuxtv.org?subject=subscribe> X-List-Received-Date: Fri, 12 May 2006 03:36:11 -0000 Status: O X-Status: X-Keywords: X-UID: 9416 |
Commit Message
Anssi Hannula
May 12, 2006, 3:35 a.m. UTC
Anssi Hannula wrote: > The Receiving(bool CheckAny) is modified so that if it is called on the > transfer-moded device, it returns true if receivers are found with > priority < 0 even if CheckAny == false. > bool cDevice::Receiving(bool CheckAny) const > { > + if (this == ActualDevice()) > + CheckAny = true; No, what if primary device is not receiving, but has -1 priority receivers? This reports true then, when it shouldn't. It should be this instead: + if (this == cTransferControl::ReceiverDevice()) + return true; Attached is a revised patch.
Comments
Anssi Hannula wrote: > Anssi Hannula wrote: >> The Receiving(bool CheckAny) is modified so that if it is called on the >> transfer-moded device, it returns true if receivers are found with >> priority < 0 even if CheckAny == false. > >> bool cDevice::Receiving(bool CheckAny) const >> { >> + if (this == ActualDevice()) >> + CheckAny = true; > > No, what if primary device is not receiving, but has -1 priority > receivers? This reports true then, when it shouldn't. > > It should be this instead: > + if (this == cTransferControl::ReceiverDevice()) > + return true; > > Attached is a revised patch. > > > ------------------------------------------------------------------------ > > 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-12 06:31:20.000000000 +0300 > @@ -627,7 +627,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 +1158,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); > @@ -1183,6 +1183,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; 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. Klaus
Klaus Schmidinger wrote: > Anssi Hannula wrote: >> 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-12 06:31:20.000000000 +0300 >> @@ -627,7 +627,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 +1158,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); >> @@ -1183,6 +1183,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; > > > 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. BTW, isn't the PrimaryLimit there so that liveview would not be distracted when a recording with (priority < primarylimit) is trying to start? Why is it used for primary device exclusively, then? If the liveview is in transfer-mode, shouldn't that be protected as well? If you think it should be used for primary device only, only the change in Receiving() in my patch is necessary. If you think it should indeed be used for whatever device the user currently is watching, there are some more places where transfermode-liveview priority is assumed 0, when it should be PrimaryLimit (at least in device.c line 591 call to GetDevice()). Or if you still think the PrimaryLimit adds only complexity for not much of real value, then remove it ;) and assume 0 for it's value everywhere.
Anssi Hannula wrote: > Klaus Schmidinger wrote: >> Anssi Hannula wrote: > >>> 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-12 06:31:20.000000000 +0300 >>> @@ -627,7 +627,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 +1158,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); >>> @@ -1183,6 +1183,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; >> >> 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. > BTW, isn't the PrimaryLimit there so that liveview would not be > distracted when a recording with (priority < primarylimit) is trying to > start? Why is it used for primary device exclusively, then? If the > liveview is in transfer-mode, shouldn't that be protected as well? This parameter was introduced at a time when it was not yet possible to record and replay on the FF cards at the same time, so that a timer wouldn't prevent the user from live viewing. Now that the FF cards can record and replay at the same time, the primary device actually wouldn't need to be "protected" any more, because if it has to be used for recording (for instance because it is the only device that can provide a certain channel), it could just fetch the live channel from a different device and show it in Transfer Mode. > If you think it should be used for primary device only, only the change > in Receiving() in my patch is necessary. If you think it should indeed > be used for whatever device the user currently is watching, there are > some more places where transfermode-liveview priority is assumed 0, when > it should be PrimaryLimit (at least in device.c line 591 call to > GetDevice()). > Or if you still think the PrimaryLimit adds only complexity for not much > of real value, then remove it ;) and assume 0 for it's value everywhere. I'd really prefer to drop the whole PrimaryLimit thing, because, as we see in this thread, it makes things rather complex. If users program so many concurrent timers that the system needs all the devices to record them, well, then that's apparently what they wanted. 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? 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: for (int i = 0; i < numDevices; i++) { if (AvoidActualDevice && device[i] == ActualDevice()) continue; bool ndr; ... Finally, streamdev would have to be modified so that it sets AvoidActualDevice to 'true' before calling cDevice::GetDevice(), and back to 'false' afterwards. If this turns out to be feasible, we could change cDevice::GetDevice() in version 1.5.x so that it has an additional parameter for this. Klaus
Klaus Schmidinger wrote: > Anssi Hannula wrote: > >> Klaus Schmidinger wrote: >> >>> Anssi Hannula wrote: >> >> >>>> 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-12 06:31:20.000000000 +0300 >>>> @@ -627,7 +627,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 +1158,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); >>>> @@ -1183,6 +1183,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; >>> >>> >>> 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 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 > 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.
Anssi Hannula wrote: > Klaus Schmidinger wrote: >> Anssi Hannula wrote: >> >>> Klaus Schmidinger wrote: >>> >>>> Anssi Hannula wrote: >>> >>>>> 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-12 06:31:20.000000000 +0300 >>>>> @@ -627,7 +627,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 +1158,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); >>>>> @@ -1183,6 +1183,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; >>>> >>>> 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 ;-) >> 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: Device *cDevice::GetDevice(const cChannel *Channel, int Priority, bool *NeedsDetachReceivers) { ... 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. Klaus
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-12 06:31:20.000000000 +0300 @@ -627,7 +627,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 +1158,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); @@ -1183,6 +1183,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;