search

catalinii / minisatip

minisatip is an SATIP server for linux using local DVB-S2, DVB-C, DVB-T or ATSC cards
https://minisatip.org
332 stars 81 forks source link

Handle occupied remote SAT>IP server #1190

Open lars18th opened 2 months ago

lars18th commented 2 months ago

Within the satipc module, when a remote SAT>IP server sends a 404 "Not Found" response instead of retrying the Request it stays quiet.

lars18th commented 2 months ago

This fixes #1118

lars18th commented 2 months ago

Hi, I've updated the patch to be more generic. Instead of retry forever in cases of 454, 503 or 405 (and now 404 from minisatip as well) the adapter goes to the inactive state because the request can't be served.

Pending: How to change to other free adapters.

catalinii commented 2 months ago

Why not returning 503 (on the rtsp code path) when there are no adapters?

lars18th commented 2 months ago

Hi @catalinii ,

1) I'll improve a bit this PR with a simple enhancement: "configure the behaviour when the SAT>IP server is occupied". The idea is simple: add the parameter --satipc-busy-timer [msec]. So don't merge it now.

2) I don't know what signifies "Why not returning 503 (on the rtsp code path) when there are no adapters". At time minisatip returns 404 when no tuner is available. And this return code is multiple files/lines. I don't think this is a problem. We can handle it as it's implemented in this PR. Or you want to say something different?

lars18th commented 2 months ago

Hi @catalinii ,

I need you help to implement the --satipc-busy-timer [msec] option. The user functionality is quite simple:

I feel this has sense and it's easy to understand. However, to implement I don't know something: the current satipc->last_setup member is not really used. I can use sip->last_setup = getTick(); to set it when we need to set the timer for retrying the request. But I don't know how I can use the timeouts.

Any idea to set another timer inside the satipc module to trigger some action?

catalinii commented 2 months ago

Hi,

why not adding a new structure member with this specific name (or rename last_setup). About timeout, you can add a new sock like this and set the timeout in the next line: https://github.com/catalinii/minisatip/blob/master/src/satipc.c#L586

Also do u have some logs on what happens ? Basically if get 404/503 from the server when you tune first time, you should return basically the same code to the upstream client and this should be the default. This would ensure the correct behavior from other clients (like TVH) which could use another tuner to process the request

lars18th commented 2 months ago

why not adding a new structure member with this specific name (or rename last_setup).

My idea is to reuse last_setup because in the current code is not used.

About timeout, you can add a new sock like this and set the timeout in the next line: https://github.com/catalinii/minisatip/blob/master/src/satipc.c#L586

Thank you. That's the point that I need. 😉

Also do u have some logs on what happens ? Basically if get 404/503 from the server when you tune first time, you should return basically the same code to the upstream client and this should be the default. This would ensure the correct behavior from other clients (like TVH) which could use another tuner to process the request

With the current implementation of the satipc module the "tuner" (aka adapter) status is handled locally by de module. Therefore any request from the upper layers is not waiting for the response. And then when you receive the 404/503 response from the remote SAT>IP you've already sended to the upper layer (the client) an OK to the request that has originated the "busy tuner" response (actually a PLAY/SETUP command or a simple HTTP GET).

With this architecture what the current code is doing is this (checked with logs): when the 404/503 response is received the local state is changed to send a new SETUP/PLAY request and retry the last command. This is more or less useful if you not overload the remote SAT>IP server. Because when the tuner will be free the request will be served. But for the problem of the efficiency this PR is necessary. Without it an infinite loop appears... until the client closes the connection. And this infinite loop could generate troubles in the network and/or the remote server. So, the easy solution is to almost "disable any retry" and not serve the request.

But my further proposal is to leave the user the option to fine-tune this behaviour. With the parameter --satipc-busy-timer [msec] we enable the option to disable all retries or control the time between them. I feel it's sufficient flexible and easy to add to the current code. You agree with that?

But in any case, before the implementation of this new parameter I recommend to merge this PR. Without it it would be very easy to overload the server. So please merge it now. And I'll provide another one later witht he --satipc-busy-timer [msec] option.