[WiP] Changing channel using numbers does not work across group (3 Viewers)

[Stéphane Lenclud]

Only two additional comments (opinions! ):

Thanks for the feedback mate.

As far as the end result goes and once we make sure the current group is scanned first we don't need any special treatment for the 'all channels' group.
However we could end up checking each channel twice. Once through normal groups and once through 'all channels' group.
Therefore I would still try to either scan the 'all channels' group or scan the other groups but not both if it can be avoid.
The algorithm should go like that:

• Scan current group and stop there if it's the 'all channels' group.
• Scan all groups excluding 'all channels' and current group.
• Scan 'all channels' group if it's not the current one.

Then again we could decide performance is not a concern here and we can afford the overhead.
That whole channel to number mapping is largely unoptimized to start with anyway.
For best performance we ought to have a dictionary of channels indexed by number really.
However this is totally outside the scope of those changes.

I'll try and see what I can do with 'set current group'.

 

[Stéphane Lenclud]

I've had a look at the Radio channels part and it seems the whole logic there is totally different so I don't believe those changes which only apply to TV would mandate changes in the Radio part.
I seriously doubt channel zapping behaviour are currently consistent between TV and Radio.

If possible, I would only set the current group after trying to select the target channel, and only if zapping is successful.

I could not find any error checking mechanism there so I can't tell if the zapping was successful. However even if it fails which should be a rare occurence consequences should be minor anyway.

 

[Stéphane Lenclud]

I've published a new installer on the JIRA.

 

[mm1352000]

However we could end up checking each channel twice. Once through normal groups and once through 'all channels' group.

I don't think this is something you should have to care about. If people are bothered by performance issues related to the all channels group, they can use the "hide all channels group" setting or configure the all channels group to be the last group.

Scan current group and stop there if it's the 'all channels' group.

If the current group is the all channels group, the channel is guaranteed to be found. Therefore you don't need any special stop handling for the all channels group.

• Scan all groups excluding 'all channels' and current group.
• Scan 'all channels' group if it's not the current one.

As explained in my previous reply, I don't think that the all channels group should be given special treatment with regard to ordering either. Surely it's far better to be consistent with the [configurable] channel group ordering. That way people can configure their ordering to get the behaviour they want.

Then again we could decide performance is not a concern here and we can afford the overhead.

As above: I don't think you should be concerned about this.
Another way to look at this would be to consider that forcing the all channels group to be checked last actually adds overhead if the target channel is only present in the all channels group.

That whole channel to number mapping is largely unoptimized to start with anyway.
For best performance we ought to have a dictionary of channels indexed by number really.

Note that channel numbers are not guaranteed to be unique.

However this is totally outside the scope of those changes.

I'm not sure why you think that. Implementing zapping using a dictionary wouldn't take much more code. Less than 10 extra lines?

I've had a look at the Radio channels part and it seems the whole logic there is totally different so I don't believe those changes which only apply to TV would mandate changes in the Radio part.
I seriously doubt channel zapping behaviour are currently consistent between TV and Radio.

I had a look myself, and in doing so I discovered code in both the TV guide and radio guide:
https://github.com/MediaPortal/Medi...ibrary/TvPlugin/TvPlugin/TvGuideBase.cs#L4110
https://github.com/MediaPortal/Medi...ary/TvPlugin/TvPlugin/RadioGuideBase.cs#L3800

It looks like the purpose is to find a channel in the guide by its channel number.
Here the TV and radio code seems to be very similar.
Both of them will need to be extended in the same way as the navigator code.