Add adapter restrictions

[lars18th]

A new parameter called "--adapter-restrictions" is added to enforce some adapter limitations when selecting a free tuner. The three limits are (in order of checking): number of used services, number of total pids, consumed bandwith. By default for all adapters the limits are "-1 : -1 : -1". But if you want to limit the adapter #1 to three simultaneous services only use "1:3:-1:-1".

Note: The calculation of the current bandwith is not currently implemented. Therefore it really doesn't work.

[lars18th]

Hi @catalinii and @Jalle19 ,

This PR implements the request #1114. Please, note that the code is simple. The most large function is the parsing of the parameters. The data of the used pids and services it's already calculated. The only missing is the bandwith. All the code is here, but I don't know how to calculate it for each individual adapter. So I request your help to complete this.

All my tests limiting the number of services (2 or 3 for example) and the pids (10, 15, 20, etc.) work without troubles.
I feel this will useful for some users that have multiple tuners with some hardware limits. You can define the restrictions at adapter level, so it's very powerful.

Regards.

[Jalle19]

Clearly we have different opinions on what "simple" means 😄

[catalinii]

@lars18th there is another point that is not obvious:
the bandwidth at the adapter level is different than the bandwidth that is being sent over the network:
For example if 1 channel is requested by 2 or more clients independently:

• all pids will be set once on the adapter
• the stream.c code will send the same pids across the network to 2 independent clients

So even if the BW at adapter level is <15Mbits, you will still send > 15Mbits over the network.

[lars18th]

@lars18th there is another point that is not obvious:

Hi @catalinii ,

Thank you for the response. Regarding your question, what I want to implement regarding the limit of the bandwith is the "adapter bandwith" and not the network bandwith. The problem is that not all adapters/drivers/devices can handle the full bandwith of a transponder. And then if you put a limit, then you have the option to not serve more requests over this limit. For sure the limit is reliable for CBR and not true for VBR. However, with a single 10Mbps limit you will not get add more services if the current bandwith is going from 10-15Mbps for example. That's the simple idea. And the limit have more sense if you are using the satipc module because in this case the "network bandwith" will be the same as the "adapter bandwith". So you can help me to calculate this value?

Futhermore, I'll try to simplify the code a bit more.
Regards.

Note: If you want, after committing this we could try to add a "client network bandwidth limit". In this case, the idea would be to limit the "total output network bandwidth" to a maximum, regardless of the number of adapters.

[lars18th]

Hi @Jalle19 ,

Clearly we have different opinions on what "simple" means 😄

If you remove the parsing of the command line parameter the code is very simple. Just a few lines of code.

[lars18th]

Hi @catalinii ,

I've implemented the bandwith adapter calculation. Therfore the check is complete.

Please, check it and merge.
Regards.

[lars18th]

Hi @Jalle19 and @catalinii ,

If you aren't testing this PR because the erros in CI, then the problem is not with my code. It's because this commit 8d33e93 that has broken the binaries build.

So please fix this, and check my PR. I'm waiting for merging it.
Regards.

[catalinii]

@lars18th this PR takes the current get_adapter_code to a place where is really too complex and hard to reason about. The motivation for that as I said is to support devices that are not supported by their vendor. As I said is not worth merging it.

[lars18th]

Hi @catalinii ,

@lars18th this PR takes the current get_adapter_code to a place where is really too complex and hard to reason about. The motivation for that as I said is to support devices that are not supported by their vendor. As I said is not worth merging it.

Sorry, but I don't agree with that.

Let me to comment these key points about devices:

• Some SAT>IP servers with certification (https://www.satip.info/resources/products/) can't handle more than N programs at the same time.
• A lot of SAT>IP hardware devices have closed source firmwares with last dates of more than five years. And these devices continue in production. So we can't consider them as not supported by the vendor.

Let me to comment other points regarding the PR too:

• The code inside this patch is very simple and clean. Futhermore it provides functionality to "debug" the current state of an adapter related to number of programs, bandwith and number of pids in use. Did you notice?
• The target of this code when I've created it isn't with the idea of solving some troubles with low cost Enigma boxes. The idea is to limit the number of channels processed by one tuner. Based on three areas: bandwith, pids and programs. A know a lot of different devices that has one of these limitations. I've asked/proposed this several years ago if you remember it.

So please, don't be wrong. This patch has sense for several users. And because it doesn't change anything by default I feel it's preferable to merge it.

Anyway, please comment if you want about point per point of the discussed points. Perhaps I'm really missing something.
Regards.

[Jalle19]

Some SAT>IP servers with certification (https://www.satip.info/resources/products/) can't handle more than N programs at the same time.

I feel it would be better to document which these devices are so people don't buy them, instead of working around their shortcomings.

[lars18th]

Some SAT>IP servers with certification (https://www.satip.info/resources/products/) can't handle more than N programs at the same time.

I feel it would be better to document which these devices are so people don't buy them, instead of working around their shortcomings.

Sorry? You can recommend whatever you want. However, manufacturers will continue to provide what they want. That's the current situation. And minisatip can handle any type of devices. And this PR is proof that this is true.

Sorry @Jalle19 and @catalinii , but I'm very confused about your reluctance to merge a simple/clear/useful/non-intrusive patch that provides very interesting functionality. Just because you don't seem to want to boost "underpowered and poorly supported hardware". My intention is to enhance this software, and in my environment the physical tuner is not very relevant.... I'm using from Digital Devices to low cost ALi based STB (very, very, very bad and closed source). But with a central minisatip I can manage everything together.

And in case you are still thinking about it, I don't recommend buying certain low-cost Enigma2-based products with outdated, poorly implemented, unsupported, absurdly limited and closed-source drivers. But what's wrong with adding this functionality?

Regards.

[Jalle19]

I think we have different opinions on what "clean and non-intrusive" means. Also, "useful" is a matter of opinion since minisatip has been around for at least 7 years without this coming up as a feature request.

I just want to keep the code simple and not support odd usecases.

[lars18th]

Hi @Jalle19 ,

It's clear that we share different point of views. However, let me to add some comments:

I think we have different opinions on what "clean and non-intrusive" means.

Well, how many lines are in this PR? Please, take note that the functionality to limit the tuners are only in the function source_enabled_for_adapter() (18 lines with comments, blanks and LOG) and the three checks inside get_free_adapter() that are only an added new AND block. Anything else are only to calculate the values (bandwith, programs and pids) at adapter level that are now printed in dump_adapters() for convenience. And the parsing the new parameter --adapter-restrictions. You say it's complex? Well, it's your respectable opinion.

It's non-intrusive? If you apply the patch and restart minisatip with identical parameters nothing changes. So we can considerer non-intrusive... or not?

Also, "useful" is a matter of opinion since minisatip has been around for at least 7 years without this coming up as a feature request.

I not agree 100% with this sentence. A lot of time ago I've requested to @catalinii to support something more smart when selecting between multiple tuners. Futhermore, I've help in implementing the source mapping of adapters, more complex that this functionality and only used by some users (my included). Plus other changes too. Anyway, I can understand that perhaps you will not need/use this functionality, but for other users it has sense.

I just want to keep the code simple and not support odd usecases.

It will continue to have the same simplicity. The code of the patch reflects it.
And regarding the "odd use cases", I repeat will all respect that it's your particular opinion. Only a small part of SAT>IP servers can handle the full transport stream independently of the bandwith of the transponder.

So I finally see that I'm not going to convince you. I just hope that my effort and time invested in this project will not be questioned and thrown away.
Regards.

[Jalle19]

As a compromise, could we consider just having a limiter for the number of services per adapter?

[lars18th]

Hi @catalinii and @Jalle19 ,

I've resolved the conflicts with this patch, but now the tests fail because the out-of-sync with the current master branch. I feel I need to return to create a PR to provide this change. However, BEFORE THAT I need to know your current point of view. Please, take note of the effort to contribute to the project and be as open-minded as you can:

• I've created this enhancement to improve the user experience and make the tool more flexible. Please, belive me that the root cause is not to add support for buggy devices. At the beginnig my objective was to improve this debug line that prints the current state of the used tuners:
  

  minisatip/src/adapter.c

  Lines 562 to 567 in 80578a2

  	LOG("%d|f: %d sid_cnt:%d master_sid:%d master_source:%d del_sys: "
  	"%s,%s,%s %s",
  	i, ad->tp.freq, ad->sid_cnt, ad->master_sid, ad->master_source,
  	get_delsys(ad->sys[0]), get_delsys(ad->sys[1]),
  	get_delsys(ad->sys[2]),
  	dump_absolute_table(ad, buf, sizeof(buf)));

• The reason is because I need to know the number of services and used pids when a new tuner will be selected.
• Therefore I've implementing this in the code without much effort. But after I see that this could be enhanced with more data and can be used to tweak the selection of a new tuner. So finally, I've added three new data to the print: services, bandwith and pids. And I've added also code to check these values when a new tuner will be select. And at the end I've added the command line parameters to use different values for each adapter.

Said that, let me to explain the use cases of this new functionality:

1. In the debug now you can see in the dump of the current used adapters new relevant information: number of services, total input bandwith and the number of pids enabled. This info is provided without any overhead. And it's very useful to detect some interferencies that could be created by other changes or unsupported hardware.
2. When you have more than one remote SAT>IP servers providing the same frequencies now you can limit the total used bandwith. Imagine that you have a central minisatip, connected to TWO different SAT>IP servers in a remote location. Each one can connect to the same transponders, and the network to reach each of them has around 8Mbps. Then you can stream 1 service of around ~6Mbps without problems. OK, but what happens if you request 2 services of the same transponder? In this case, only one remote SAT>IP will be used. But then the output bandwith could exceed the available bandwith (12 versus 8). And therefore you'll in troubles. One solution is to limit each adapter with this PR to 8Mbps. Then you could request anything on them until the bandwith will be less. For example, one HD video service and 4 radios perhaps could be handled by one SAT>IP server, or two SD services, etc. But if you request two HD services of the same transponder then the two adapter will be used. Nice, right?
3. Another different use case: you have multiple SAT>IP servers that handle CA in hardware. But you can decrypt only 2 services with one CA. In this case, you want to request 1 or 2 services only for each server. And this is true also in case of multiple services in the same transponder. Therefore with this patch you can now set the maximun number of services that you want to request to an individual adapter. Futhermore with a different value for each one. Nice too, right?
4. Finally the limit of pids could be another different use case. For this the only limit is the hardware pid filtering of the server. In this case we need to remember that a lot of hardware SAT>IP servers have a limit of the number of pids. It's common a large number like 32, 64 or more. But the limit exist in several devices. This is not the case with enigma2 boxes. As we can run minisatip on them, and overcome any hardware limit with a software pid filtering if the tuner driver supports the full transport stream. Anyway, the functionality is here and we can limit the pids without any effort.

After this explanation about the sense of this enhancement, please review the code and check the real simplicity of it:

• Overall the code are simple lines or blocks of few lines. Except the function set_restrictions_adapters(). This is a new 50 lines of a parsing function. And it only handles the new parameter --adapter-restrictions. Nothing more, nothing less.
• This patch follows the KISS principle because it only adds a new functionality in a simple way.
• It also follows the main principles pointed by @catalinii bacause: a) It doesn't changes the current functionality. b) It's a non-intrussive new paramter that could be used or not. c) It doesn't breaks other modules. d) It doesn't depends on any other functionality.

Therefore, I feel this could be added without any problem. But I need your confirmation. My idea is that if you accept this then I'll remake the PR for the current master branch to be ready to merge it.

You agree?

[Jalle19]

From the use cases you listed, numbers 2 and 3 are IMO reasonable scenarios. Perhaps we could add just support for "max services" and skip "max pids" and "max bandwidth" to keep things simple?

[Jalle19]

Closing this for now, we can re-open it when you have time to work on it again 🙏

[lars18th]

Closing this for now, we can re-open it when you have time to work on it again 🙏

Hi @Jalle19 ,

No problem. I'm using it for a lot of time, but I've more "smart" options for the tuner selection. So I'll try to implement all-in-one in a new P.R. in the future when I'll have sufficient time.

Regards.