catalinii / minisatip

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

WIP: FIFO refactoring #1060

Closed lars18th closed 1 year ago

lars18th commented 1 year ago

I was thinking about using fifo for the satipc module (for sending the messages instead of the hash table), but I think a better option is to refactor and prevent the satipc module from buffering responses. This would be better because of a message is buffered any update to the list of pids for example, requires a new message. If there is no buffering the satipc module can just share it's current state rather than an older state. I plan next to replace the socket buffering with FIFO, as based on current consumption it uses 2x-4x more memory that usable.

Originally posted by @catalinii in https://github.com/catalinii/minisatip/issues/1059#issuecomment-1437552298

lars18th commented 1 year ago

Hi @catalinii ,

I post here to continue discussing about next steps. I hope you agree!

I was thinking about using fifo for the satipc module (for sending the messages instead of the hash table), but I think a better option is to refactor and prevent the satipc module from buffering responses. This would be better because of a message is buffered any update to the list of pids for example, requires a new message. If there is no buffering the satipc module can just share it's current state rather than an older state.

I agree with the idea of doing some refactoring of the satipc module. Remember that we have pending the bug of the delayed TEARDOWN command (#817). And also the implemenation to handle the closing of the TCP RTSP socket during the PLAY (the Xoro behaviour). However, I'm not sure if the synchronous (aka not buffered) message handling is the best idea. What happens then with cases like fast pid open-close? In some cases, for example when restarting the CA, the same pid will be reopened in a very short time. And I detected for long time that some SAT>IP servers can't handle correctly very fast pid changes. The observed behaviour then is that the stream suffers for some (small) CC errors. So I prefer to overcome these cases, and IMO I think the message buffering is not a bad idea. What you think about this?

I plan next to replace the socket buffering with FIFO, as based on current consumption it uses 2x-4x more memory that usable.

This could be fantastic. Futhermore, perhaps this will open the scene to reincorporate the old idea of a small buffering for the decoding. The idea is to put the stream in a FIFO with a sort user configurable delay (around 250~500ms) and process it after. But processing all ECM filters when inserting TS packets in the FIFO instead of when reading from the FIFO. You think this could be feasible?

Regards.

catalinii commented 1 year ago

1) I am proposing to remove the buffering of requests to the satip server, but not a synchronous implementation (tbh I was considering that as well as some point). Removing buffering:

A sync connection would behave like you said: every adapter state change will trigger an update and wait for a response and only then it will update the client (tvh, vdr) about the change being committed. netceiver implementation does this. I am not sure how this would work over high RTT links

2) yes this is a possibility with FIFO (but probably a big/risky change)

lars18th commented 1 year ago

Hi @catalinii ,

  1. OK. Therefore the idea is to "buffer" all adapter changes until the last RTSP response is received. That's right? In this case, the RTT doesn't have a really impact. In fact, the RTT time could be in less order of magnitude than the processing time of the SAT>IP server. However, you need to consider the case of a TEARDOWN or error handling.
  2. For reimplementing the stream reading with FIFO perhaps we will need to wait some time. So, don't care about it now.

Regards.

catalinii commented 1 year ago

1) To not buffer, but cache all the changes (such as new pids, new freq, ... ) then when the reply comes from the server, one request is created with all those changes.

lars18th commented 1 year ago
  1. To not buffer, but cache all the changes (such as new pids, new freq, ... ) then when the reply comes from the server, one request is created with all those changes.

That's the correct way. I agree. 😉

catalinii commented 1 year ago

We can continue satip discussion here: https://github.com/catalinii/minisatip/pull/1080