Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]: "satipc" does not work properly when the tuner is in use. #1118

Open
lars18th opened this issue Jun 7, 2023 · 15 comments
Open

[BUG]: "satipc" does not work properly when the tuner is in use. #1118

lars18th opened this issue Jun 7, 2023 · 15 comments
Labels

Comments

@lars18th
Copy link
Contributor

lars18th commented Jun 7, 2023

Hi @catalinii ,

When using the satipc module if you "share" the source adapter then you will be in troubles when the tuner is in use.

Here a simple tcpdump log with the problem:

18:48:29.137741 IP 192.168.1.51.56302 > 192.168.1.82.rtsp: Flags [P.], seq 3586096098:3586096289, ack 2841071189, win 501, length 191: RTSP: PLAY rtsp://192.168.1.82:554/?freq=... RTSP/1.0
18:48:29.138591 IP 192.168.1.82.rtsp > 192.168.1.51.56302: Flags [P.], seq 1:127, ack 191, win 473, length 126: RTSP: RTSP/1.0 404 Not Found
18:48:29.138756 IP 192.168.1.37.54071 > 192.168.1.82.rtsp: Flags [.], ack 2298112, win 21, length 0
18:48:29.138823 IP 192.168.1.82.rtsp > 192.168.1.37.54071: Flags [P.], seq 2298112:2299324, ack 1, win 490, length 1212: RTSP
18:48:29.140357 IP 192.168.1.51.56302 > 192.168.1.82.rtsp: Flags [.], ack 127, win 501, length 0
18:48:29.140519 IP 192.168.1.51.56302 > 192.168.1.82.rtsp: Flags [P.], seq 191:274, ack 127, win 501, length 83: RTSP: DESCRIBE rtsp://192.168.1.82:554/ RTSP/1.0

As you can see the problem is this:

  • the minisatip satipc requests a SETUP with new tunning parameters.
  • The SAT>IP server (that is another minisatip process) has the adapter in use, therefore it responds with 404 error.
  • The minisatip satipc receives the response and starts to process a restart of the adapter.
  • And this loop will continue until the adapter will be free.

However, in the minisatip server using the satipc module I've more adapters. And the problem is that instead of searching for another (next) free adapter it will continue reopening (aka restarting) the same adapter.

Any idea to fix this?

@Jalle19
Copy link
Collaborator

Jalle19 commented Jun 7, 2023

Doesn't this only happen if fe= is specified in the URL?

@lars18th
Copy link
Contributor Author

lars18th commented Jun 7, 2023

Hi @catalinii ,

The minisatip LOG when the problem appears:

[07/06 19:28:42.648]: satipc: commit for adapter 3 freq 11347000, pids to add 15, pids to delete 0, expect_reply 0, force_commit 0 want_tune 1 do_tune 1, force_pids 0, sent_transport 0, sleep 0, closed_rtsp 0
[07/06 19:28:42.648]: satipc_http_request (adapter 3): sending to sock 12
[07/06 19:28:42.648]: Reply (handle 11) [192.168.1.82:53387] content_len:0, sock 15
[07/06 19:28:42.656]: satipc_reply (adapter 3): receiving from handle 9, sock 12
[07/06 19:28:42.656]: marking device 0 as error, rc = 404
[07/06 19:28:42.656]: satipc 0 wp 0 qp 0, expect_reply 1, want_tune 0, force_commit 0, want commit 0
[07/06 19:28:42.656]: satipc_http_request (adapter 3): sending to sock 12
[07/06 19:28:42.657]: satipc_reply (adapter 3): receiving from handle 9, sock 12
[07/06 19:28:42.657]: satipc: Received signal status from the server for adapter 3, stength=240 status=1 quality=15
[07/06 19:28:42.657]: satipc 0 wp 0 qp 0, expect_reply 1, want_tune 0, force_commit 0, want commit 0
[07/06 19:28:54.679]: select_and_execute[15]: Close on socket 11 (sid:0) from 192.168.1.82:53387 - type http (3) errno 0
[07/06 19:28:54.679]: Requested sid close 0 timeout 30000 type 1, sock 15, handle 11, timeout 0

@lars18th
Copy link
Contributor Author

lars18th commented Jun 7, 2023

Doesn't this only happen if fe= is specified in the URL?

No. It appears with and without the fe= parameter. But without it the correct behaviour is to close the adpater and search for another one. Try yourself to do the test:

  • With a minisatip satipc client, configure it to use another remote minisatip server. With a SAT>IP client open a service with the remote minisatip server. While streaming this service request from the local minisatip a new service in a different transponder... What happens?

@lars18th
Copy link
Contributor Author

lars18th commented Jun 7, 2023

More or less the bug has the root cause in these lines:

minisatip/src/satipc.c

Lines 243 to 244 in 8d33e93

LOG("marking device %d as error, rc = %d", sip->id, rc);
ad->err = 1;

The error of the adapter it's set, but the code will never checks it.

@Jalle19
Copy link
Collaborator

Jalle19 commented Jun 7, 2023

I'm pretty sure this works for me. I have one minisatip instance proxying two Digibit R1 tuners.

@Jalle19
Copy link
Collaborator

Jalle19 commented Jun 7, 2023

Maybe I'm misunderstanding the issue

@catalinii
Copy link
Owner

Can u check how next branch is behaving?
There are some refactorings done for the code ro be managed easier there.
The master branch will get only important fixes and will not get any new features

@lars18th
Copy link
Contributor Author

lars18th commented Jun 8, 2023

Can u check how next branch is behaving?
There are some refactorings done for the code ro be managed easier there.

I'll check it. However, reading the code of satipc.c...

    if (rc == 404) {
        sip->state = SATIP_STATE_OPTIONS;
    }
[...]
        case SATIP_STATE_OPTIONS:
            sip->state = SATIP_STATE_SETUP;

So, I feel the behaviour is the same: re-request when receiving the 404 error. And the correct behaviour is to close the adapter and try the next.

The master branch will get only important fixes and will not get any new features

Then, please commit my PR #1117 with the next branch. This new functionality is very useful. 😉

@Jalle19
Copy link
Collaborator

Jalle19 commented Jun 8, 2023

Shouldn't the server be choosing a free adapter? Why does the client have to do that?

@lars18th
Copy link
Contributor Author

Hi @Jalle19 ,

Shouldn't the server be choosing a free adapter? Why does the client have to do that?

The scenario is this:

Client 1 ------------------------> +--> minisatip A (1 tuner)
                                   |
Client 2 ---> minisatip C (satipc) +--> minisatip B (1 tuner)

And the problem appears in the "minisatip C". This is a central server that has configured with 2 adapters (with the satipc module). The problem here is that when the client 1 is using the minisatip A (that only has 1 tuner), then the adapter 1 in the minisatip C is BUSY. But the current implementation marks it as FREE. Therefore if the Client 2 requests some channel then the minisatip C will selects the adapter 1... and this is BUSY because the minisatip A is in use by the client 1. And then the current code enters in a loop re-requesting to SETUP to the minisatip A, instead of try with the minisatip B that it's free.

So the current implementation of the satipc module requires to be changed. The code at time doesn't care about BUSY adapters. And this functionality requires to be incorporated. So please @catalinii any idea about how to do it?

Regards.

@Jalle19
Copy link
Collaborator

Jalle19 commented Jun 12, 2023

Yeah I think I understand the problem now.

@lars18th
Copy link
Contributor Author

Yeah I think I understand the problem now.

Nice! So to fix it, I feel @catalinii needs to help us to indicate how to close an adapter from inside the submodule (satipc in this case) and reopen another one from the upper layer. My suggestion is that the get_free_adapter(...) function will check for a new adapter parameter (ad->busy) ; and the satipc module will set/cleans this flag.

What you think @catalinii ?

@Jalle19
Copy link
Collaborator

Jalle19 commented Jun 28, 2024

It seems to be that in @lars18th scenario, "minisatip A" that has one physical tuner should return error code 503 with the body No-More: frontends. Currently we just return 404:

        a_id = get_free_adapter(&sid->tp);
        LOG("Got adapter %d on sid %d socket %d", a_id, sid->sid, s->id);
        if (a_id < 0)
            return -404;

This would just be the first step, next would be to handle this return code correctly. Currently the path taken is the same - state gets reset to SATIP_STATE_OPTIONS.

@Jalle19 Jalle19 added the bug label Jun 28, 2024
@lars18th
Copy link
Contributor Author

Hi @Jalle19 ,

Yes, the "remote SAT>IP server" (the one with the physical tuner) requires to return the correct response.
And then, the "local SAT>IP server" (the one using the satipc module) needs to process it in acordance.

@lars18th
Copy link
Contributor Author

Hi @catalinii (and @Jalle19 ),

My PR #1190 fixes the problem "in the single case":

  • When using one tuner in the local minisatip it works like a charm... if the remote SAT>IP server is busy, then the request is not served. And any new connection works if the remote server has a free tuner.
  • However, when using two (or more) tuners this is not working in the best way: if the first remote tuner is busy it have sense to try with the second tuner, that perhaps is free.

But I don't know how to handle this case.
Any idea?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants