Just an observation. That code from vdr.c that calls
GetDeviceForTransponder() (for example, when a VPS timer is about to
start) can only interrupt tasks with priority < LIVEPRIORITY anyway. It
looks like disabling priority checking can't do much harm here. Is that
what you originally meant?
BTW. Device bonding itself is a nice VDR feature. Unfortunately I can't
test it (with 3+ devices) :(
On Tue, Dec 27, 2016 at 01:38:51PM +0100, Klaus Schmidinger wrote:
> On 22.12.2016 16:07, glenvt18 wrote:
> > I understand. But this code looks confusing (at least). And, just to
> > draw your attention, there is no priority checking here unless
> > MaySwitchTransponder() returns true on the second call (which would be
> > really ugly). Thus, for example, this code (vdr.c:1109)
> >
> > if (NeedsTransponder || InVpsMargin) {
> > // Find a device that provides the required transponder:
> > cDevice *Device = cDevice::GetDeviceForTransponder(Timer->Channel(), MINPRIORITY);
> > if (!Device && InVpsMargin)
> > Device = cDevice::GetDeviceForTransponder(Timer->Channel(), LIVEPRIORITY);
> > // Switch the device to the transponder:
> > if (Device) {
> > ...
> >
> > has no sense.
>
> I totally agree! And I have nothing to counter the logic in your
> arguments.
>
> However, before I revert the change that was introduced in version
> 1.7.29, I would need somebody with a setup that contains "bonded"
> devices to test whether recordings still work properly with the
> second call to d->MaySwitchTransponder(Channel) removed.
>
> Is anybody actually using "device bonding" (or "LNB-sharing", as it was
> named when this was still a patch)? Personally I never liked this and
> I still believe that it makes things unnecessarily complex and should
> be removed again...
>
> Klaus
>
> > On Thu, Dec 22, 2016 at 02:56:10PM +0100, Klaus Schmidinger wrote:
> > > On 26.05.2016 17:58, glenvt18 wrote:
> > > > d->MaySwitchTransponder(Channel) is always false here
> > > >
> > > > Please review,
> > > > Sergey Chernyavskiy.
> > > >
> > > > ---
> > > > device.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/device.c b/device.c
> > > > index 18867cd..542d120 100644
> > > > --- a/device.c
> > > > +++ b/device.c
> > > > @@ -342,7 +342,7 @@ cDevice *cDevice::GetDeviceForTransponder(const cChannel *Channel, int Priority)
> > > > if (d->ProvidesTransponder(Channel)) {
> > > > if (d->MaySwitchTransponder(Channel))
> > > > Device = d; // this device may switch to the transponder without disturbing any receiver or live view
> > > > - else if (!d->Occupied() && d->MaySwitchTransponder(Channel)) { // MaySwitchTransponder() implicitly calls Occupied()
> > > > + else if (!d->Occupied()) { // MaySwitchTransponder() implicitly calls Occupied()
> > > > if (d->Priority() < Priority && (!Device || d->Priority() < Device->Priority()))
> > > > Device = d; // use this one only if no other with less impact can be found
> > > > }
> > >
> > > This was introduced in version 1.7.29:
> > >
> > > --- device.c 2012/06/09 14:37:24 2.61
> > > +++ device.c 2012/06/10 13:13:18 2.62
> > > @@ -334,7 +334,7 @@
> > > if (d->ProvidesTransponder(Channel)) {
> > > if (d->MaySwitchTransponder(Channel))
> > > Device = d; // this device may switch to the transponder without disturbing any receiver or live view
> > > - else if (!d->Occupied()) {
> > > + else if (!d->Occupied() && d->MaySwitchTransponder(Channel)) { // MaySwitchTransponder() implicitly calls Occupied()
> > > if (d->Priority() < Priority && (!Device || d->Priority() < Device->Priority()))
> > > Device = d; // use this one only if no other with less impact can be found
> > > }
> > >
> > > with the change comment "Fixed handling recording with more than two bonded devices".
> > >
> > > While I tend to agree with you in that d->MaySwitchTransponder(Channel) is always false here,
> > > this must at least have fixed the problem at hand back then (it was the only file that
> > > has been modified for this fix). I'm therefore hesitant to remove this call and
> > > risk breaking something...
> > >
> > > Klaus
> > >
> > > _______________________________________________
> > > vdr mailing list
> > > vdr@xxxxxxxxxxx
> > > https://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
> >
>
> _______________________________________________
> vdr mailing list
> vdr@xxxxxxxxxxx
> https://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
--
glenvt18
_______________________________________________
vdr mailing list
vdr@xxxxxxxxxxx
https://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
