Bug #4499
closed
SAT>IP: Fix five different bugs
100%
Description
Hi Jaroslav,
After a lot of testing and debugging, we are glad to publish a fix for some SAT>IP server bugs.
We present the patches divided into 5 groups for a better understanding. In fact, the more relevant fix is the last one, but we present them in reverse order.
1) Add default short DESCRIBE:
At this time, when a session has incomplete configuration data or the delivery system is unsupported, in this case the session description is incomplete. This patch implements a short/neutral response. This is required by some clients that refuse to continue if the response is incomplete.
--- a/src/satip/rtp.c
+++ b/src/satip/rtp.c
@@ -811,7 +811,9 @@ satip_status_build(satip_rtp_session_t *rtp, char *buf, int len)
c2tft, ds, plp, specinv, pids);
break;
default:
- return 0;
+ // Send a short description (required by some SAT>IP clients)
+ r = snprintf(buf, len, "ver=1.0;src=%d;tuner=%d,%d,%d,%d,,,,,,,,;pids=%s",
+ rtp->source, rtp->frontend, level, lock, quality, pids);
}
return r >= len ? len - 1 : r;
2) Fix for incorrect delivery system in SESSION:
With the current implementation, when a new session is created the delivery session is not set correctly. Only the value for the MUX is set, but not the value in the SESSION.
--- a/src/satip/rtsp.c
+++ b/src/satip/rtsp.c
@@ -562,6 +562,7 @@ rtsp_start
&rs->dmc, 1);
if (mux) {
created = 1;
+ dmc = ((dvb_mux_t *)mux)->lm_tuning;
rs->perm_lock = 1;
}
}
@@ -1109,6 +1110,7 @@ rtsp_parse_cmd
dmc->dmc_fe_freq = freq;
dmc->dmc_fe_modulation = mtype;
+ dmc->dmc_fe_delsys = delsys;
rs->delsys = delsys;
rs->frontend = fe;
rs->findex = findex;
3) Fix incorrect Frontend & Source values in the DESCRIPTON:
The current implementation generates incorrect values for the feID and srcID in the description responses (for both, the RTCP header and for the DESCRIBE request). As documented in the “3.5.11 Query Syntax” section of the SAT>IP Standard Specification these values start with “1” and not “0”. The range for feID is [1..65535], and the range for srcID is [1..255]. It’s very easy to check the values generated by the TVH using the tcpdump command. Currently, “src=” and “tuner=” are often equal to “0”. The problem is in the initialization of the RTP queue. The call to the “rtp.c@satip_rtp_queue()” function has incorrect call parameters when called from “rtsp.c@rtsp_start()”. Furthermore, the current code uses three values: frontend, frontend-index, and source. However, only two are defined in the standard. The provided patch is sufficiently tested and it uses frontend-index and source as the parameters for creating the RTP stream. In addition, the initialization of the frontend-index is set to 1 instead of 0.
--- a/src/satip/rtsp.c
+++ b/src/satip/rtsp.c
@@ -626,7 +626,7 @@ pids:
&hc->hc_fd_lock, hc->hc_peer, rs->rtp_peer_port,
rs->udp_rtp ? rs->udp_rtp->fd : hc->hc_fd,
rs->udp_rtcp ? rs->udp_rtcp->fd : -1,
- rs->frontend, rs->findex, &rs->dmc_tuned,
+ rs->findex, rs->src, &rs->dmc_tuned,
&rs->pids,
ocmd == RTSP_CMD_PLAY || oldstate == STATE_PLAY,
rs->perm_lock);
@@ -881,7 +881,7 @@ rtsp_parse_cmd
session_t **rrs, int *valid, int *oldstate)
{
session_t *rs = NULL;
- int errcode = HTTP_STATUS_BAD_REQUEST, r, findex = 0, has_args, weight = 0;
+ int errcode = HTTP_STATUS_BAD_REQUEST, r, findex = 1, has_args, weight = 0;
int delsys = DVB_SYS_NONE, msys, fe, src, freq, pol, sr;
int fec, ro, plts, bw, tmode, mtype, gi, plp, t2id, sm, c2tft, ds, specinv;
char *s;
4) Fix response with a remote SAT>IP server returning BAD SERVICE:
This is a very obscure bug. The problem occurs when you use two instances of the TVH. One is the local server and the second is the remote server. When a client requests for a MUX to the local server, and the remote server responses with a 405-STATUS NOT ALLOWED (or a similar error), in this case the client never receives an error. Therefore, the client only continues after its timeout expires. The solution is recheck for every client request if the associated subscription is invalid. This is the way it works, since the asynchronous coupling between the SAT>IP server and client process inside the TVH makes it impossible to send back the response from the remote server. Fortunately, all SAT>IP clients resend the tuning command after a short period of time when the signal is unlocked. This patch fixes this problem with a very simple implementation.
--- a/src/satip/rtsp.c
+++ b/src/satip/rtsp.c
@@ -573,6 +573,12 @@ rtsp_start
(rtsp_muxcnf == MUXCNF_REJECT || rtsp_muxcnf == MUXCNF_REJECT_EXACT_MATCH ) ? " (configuration)" : "");
goto endclean;
}
+ if (mux && rs->subs && rs->subs->ths_state == SUBSCRIPTION_BAD_SERVICE) {
+ dvb_mux_conf_str(&rs->dmc, buf, sizeof(buf));
+ tvhwarn(LS_SATIPS, "%i/%s/%i: subscription fails because mux %s can't tune",
+ rs->frontend, rs->session, rs->stream, buf);
+ goto endclean;
+ }
if (rs->mux == mux && rs->subs)
goto pids;
rtsp_clean(rs);
5) Fix SETUP with STREAM parameter:
This is a definitive patch for resolve the problem described in #4474. Two questions are addressed in this patch:
a) The generation of a new stream identifier is restricted only to cases when it is necessary. For example, when a new SETUP command is received without a stream parameter. Or when a new subscription is created without a corresponding stream id. Additionally, the code now verifies that the stream value starts with “1” and not “0”. Therefore, each time the stream value at the end of the “rtsp_parse_cmd()” function is less than 1 a new identifier is created... and only in this case! So, in the case of a SETUP command with a stream parameter the received stream id is then reused.
b) The second problem is the behaviour of the RTP queue when a SETUP with a stream parameter is received. In this case, the stream must be in play when the signal is locked. Furthermore, and related to the previous fix (4), it’s required to not close the current session in case of receiving the same SETUP request. Only when the SETUP targets a new session, the current session assigned to the stream id must be closed.
This patch then replaces the burden solution described in #4474.
--- a/src/satip/rtsp.c
+++ b/src/satip/rtsp.c
@@ -964,7 +964,11 @@ rtsp_parse_cmd
goto ok;
}
*oldstate = rs->state;
- rtsp_close_session(rs);
+ /* At this point a SETUP cmd with a STREAM parameter is received,
+ only close the current session if the new request is different! */
+ if (freq != rs->dmc.dmc_fe_freq) {
+ rtsp_close_session(rs);
+ }
}
if (cmd == RTSP_CMD_SETUP) {
r = parse_transport(hc);
@@ -980,6 +984,11 @@ rtsp_parse_cmd
}
rs->frontend = fe > 0 ? fe : 1;
dmc = &rs->dmc;
+ if (stream > 0) {
+ /* At this point a SETUP cmd with a STREAM parameter is received,
+ so enable the play data flag! */
+ *oldstate = STATE_PLAY;
+ }
} else {
if (!rs || stream != rs->stream) {
if (rs)
@@ -1127,10 +1136,16 @@ rtsp_parse_cmd
rs->old_hc = hc;
if (cmd == RTSP_CMD_SETUP) {
+ if ((stream > 0) && (rs->stream < 1)) {
+ rs->stream = stream;
+ }
+ }
+ if (rs->stream < 1) {
stream_id++;
if (stream_id == 0)
stream_id++;
rs->stream = stream_id % 0x7fff;
+ tvhinfo(LS_SATIPS, "new stream-id generated (%d)",rs->stream);
}
rs->src = src;
We have tested all of these patches, and hope that they will be commited.
Regards,
D.
Updated by 
Updated by 
Updated by