Feature #4449

SAT>IP: Fix tuner status in state 1 when client waits for RTCP messages

Added by DSA APF 4 months ago. Updated 4 months ago.

Status:FixedStart date:2017-06-20
Priority:NormalDue date:
Assignee:Jaroslav Kysela% Done:

100%

Category:SAT>IP
Target version:-

Description

Hi,

We like to publish another useful patch for SAT>IP that fixes some specific case.

First the description of the problem:

When a SAT>IP client likes to tune one frequency, at minimum two commands are required: SETUP and PLAY. The trouble is related about the server actions in between the two commands. As described in the SAT>IP documentation (diagram “Server Stream Output State Machine” at section 3.5.3 page 34 of version 1.2.2), the sever doesn’t need to stream anything in the State 1 (the middle state after SETUP (unicast) and previous to PLAY).

However, several SAT>IP clients are requesting in the SETUP command a list of pids. Then, they expect to start receiving some RTCP packets with the tuner status... just after receive the response to the SETUP command over the RTSP protocol. The idea behind this is to receive some confirmation of the tuner status to asserting that the frequency is locked. And only after receiving a good signal status the client will sends the PLAY command.

We have confirmed this behaviour with several clients (hardware and software). And if the client doesn’t receive any RTCP packet then it fails and doesn’t send the PLAY command.

The specification is ambiguous about this behaviour. It is not described in the document. However, a lot of implementations assume this behaviour. In fact, it has sense, as the SETUP command implies a “tuning action”. And when a tuner is active the RTCP messages must be received indicating the status. However, if the server is in state 1 (no streaming), it’s common then that it doesn’t send any RTCP command.

So, we have prepared one patch that implements this behaviour in TVHeadend. It fixes a lot of scanning failures with a large list of SAT>IP clients. For sure, it only has sense with clients that require receiving RTCP packets before sending the PLAY command. With other clients this patch is completely transparent. So don’t care about breaking functionality with other clients.

We hope you agree to commit this patch.
Regards,
D.

Here the path (Fix_tuner_status.diff):

--- tvheadend/src/satip/rtsp.c    2016-04-04 14:00:41.187765643 +0200
+++ tvheadend/src/satip/rtsp.c    2016-04-04 14:39:47.289866499 +0200
@@ -577,6 +578,11 @@ rtsp_start
     if (oldstate) {
       setup = 0;
       rs->state = STATE_SETUP;
+      tvhtrace("satips", "Change State to SETUP (1)");
+    }
+    if (setup && (rs->state == STATE_DESCRIBE)) {
+      rs->state = STATE_SETUP;
+      tvhtrace("satips", "Change State to SETUP (2)");
     }
   } else {
 pids:
@@ -590,7 +596,7 @@ pids:
     if (rs->used_weight != weight && weight > 0)
       subscription_set_weight(rs->subs, rs->used_weight = weight);
   }
-  if (!setup && rs->state != STATE_PLAY) {
+  if ((!setup && rs->state != STATE_PLAY) || (setup && (rs->state == STATE_SETUP))) {
     if (rs->mux == NULL)
       goto endclean;
     satip_rtp_queue((void *)(intptr_t)rs->stream,
@@ -1223,7 +1229,7 @@ rtsp_describe_header(session_t *rs, htsb
 static void
 rtsp_describe_session(session_t *rs, htsbuf_queue_t *q)
 {
-  char buf[4096];
+  char buf[8192];

   htsbuf_qprintf(q, "a=control:stream=%d\r\n", rs->stream);
   htsbuf_append_str(q, "a=tool:tvheadend\r\n");
@@ -1232,10 +1238,15 @@ rtsp_describe_session(session_t *rs, hts
     htsbuf_append_str(q, "c=IN IP6 ::0\r\n");
   else
     htsbuf_append_str(q, "c=IN IP4 0.0.0.0\r\n");
+  tvhtrace("satips", "Rtsp_describe_session: rs->state=%i,rs->stream=%i",rs->state,rs->stream);
   if (rs->state == STATE_PLAY) {
     satip_rtp_status((void *)(intptr_t)rs->stream, buf, sizeof(buf));
     htsbuf_qprintf(q, "a=fmtp:33 %s\r\n", buf);
     htsbuf_append_str(q, "a=sendonly\r\n");
+  } else if (rs->state == STATE_SETUP) {
+    //fixme: send real tuner status (at this point the status is empty!)
+    htsbuf_append_str(q, "a=fmtp:33 ver=1.0;src=0;tuner=0,240,1,15,,,,,,,,;pids=all\r\n");
+    htsbuf_append_str(q, "a=sendonly\r\n");
   } else {
     htsbuf_append_str(q, "a=fmtp:33\r\n");
     htsbuf_append_str(q, "a=inactive\r\n");

Associated revisions

Revision a7086200
Added by Jaroslav Kysela 4 months ago

satip server: start streaming directly after SETUP, but RTCP only, fixes #4449

Revision a50e1860
Added by Jaroslav Kysela 4 months ago

satip server: pass cmd to rtsp_start() to make core more readable, fixes #4449

Revision 1d7c337b
Added by Jaroslav Kysela 4 months ago

satip server: show the stream status for DESCRIBE in SETUP state, fixes #4449

Revision 550ed7e6
Added by Jaroslav Kysela 3 months ago

satip server: start streaming directly after SETUP, but RTCP only, fixes #4449

Revision 0d46c431
Added by Jaroslav Kysela 3 months ago

satip server: show the stream status for DESCRIBE in SETUP state, fixes #4449

Revision dd6dfe52
Added by Jaroslav Kysela 3 months ago

satip server: pass cmd to rtsp_start() to make core more readable, fixes #4449

History

#1 Updated by Jaroslav Kysela 4 months ago

  • Status changed from New to Fixed
  • % Done changed from 0 to 100

#2 Updated by Jaroslav Kysela 4 months ago

  • Status changed from Fixed to Accepted
  • Assignee set to Jaroslav Kysela

#3 Updated by Jaroslav Kysela 4 months ago

I pushed two changes (one is cosmetic, but it makes code more readable). I don't understand the DESCRIBE change, do you think that this is enough?

diff --git a/src/satip/rtsp.c b/src/satip/rtsp.c
index 9325996..4bf9e7b 100644
--- a/src/satip/rtsp.c
+++ b/src/satip/rtsp.c
@@ -1251,7 +1251,7 @@ rtsp_describe_session(session_t *rs, htsbuf_queue_t *q)
     htsbuf_append_str(q, "c=IN IP6 ::0\r\n");
   else
     htsbuf_append_str(q, "c=IN IP4 0.0.0.0\r\n");
-  if (rs->state == STATE_PLAY) {
+  if (rs->state == STATE_PLAY || rs->state == STATE_SETUP) {
     satip_rtp_status((void *)(intptr_t)rs->stream, buf, sizeof(buf));
     htsbuf_qprintf(q, "a=fmtp:33 %s\r\n", buf);
     htsbuf_append_str(q, "a=sendonly\r\n");

The status should be sent with the 'initial' values until the real signal/snr is obtained from the input source.

#4 Updated by Jaroslav Kysela 4 months ago

  • Status changed from Accepted to Fixed

#5 Updated by Jaroslav Kysela 4 months ago

OK. I pushed slightly modified code (last commit in this bug). Let me know, if this works.

#6 Updated by Thomas Göttgens 4 months ago

I'm not sure why, but the cosmetic changes push specifically (a50...) breaks DVBViewer SatIP Clients accessing a TVHeadend server. With a70... Applied, this still works.

Before this patch:

Jun 27 12:53:08 blackhole tvheadend14596: satips: 0/F9A25CCA/3: create mux DVB-S2 freq 11494000 H sym 22000000 fec AUTO mod PSK/8 roff 35 is_id -1 pls_mode ROOT pls_code 0
Jun 27 12:53:08 blackhole tvheadend14596: mpegts: 11494H in Astra 19,2°E - tuning on Silicon Labs Si2183 : DVB-S #1
Jun 27 12:53:08 blackhole tvheadend14596: epggrab: 11494H in Astra 19,2°E - registering mux for OTA EPG
Jun 27 12:53:08 blackhole tvheadend14596: subscription: 004C: "SAT>IP" subscribing to mux "11494H", weight: 100, adapter: "Silicon Labs Si2183 : DVB-S #1", network: "Astra 19,2°E", service: "Raw PID Subscription", hostname="192.168.100.17"

After the patch:

Jun 27 12:46:12 blackhole tvheadend5348: subscription: 0014: "SAT>IP" subscribing to mux "11494H", weight: 100, adapter: "Silicon Labs Si2183 : DVB-S #1", network: "Astra 19,2°E", service: "Raw PID Subscriptio
n", hostname="192.168.100.17"
Jun 27 12:46:12 blackhole tvheadend5348: http: 192.168.100.17: RTSP/1.0 PLAY rtsp://192.168.100.181:9983/stream=1?delpids=5101,5102,0,5104,17 -- 400
Jun 27 12:46:12 blackhole tvheadend5348: http: 192.168.100.17: RTSP/1.0 PLAY rtsp://192.168.100.181:9983/stream=1?delpids=5100 -- 400
Jun 27 12:46:12 blackhole tvheadend5348: http: 192.168.100.17: RTSP/1.0 PLAY rtsp://192.168.100.181:9983/stream=1?delpids=18 -- 400
Jun 27 12:46:12 blackhole tvheadend5348: subscription: 0014: "SAT>IP" unsubscribing, hostname="192.168.100.17"
Jun 27 12:46:12 blackhole tvheadend5348: mpegts: 11494H in Astra 19,2°E (0x7fc174013e40) - deleting

#7 Updated by Mono Polimorph 4 months ago

Thomas Göttgens wrote:

I'm not sure why, but the cosmetic changes push specifically (a50...) breaks DVBViewer SatIP Clients accessing a TVHeadend server. With a70... Applied, this still works.

Hi Thomas,

I not already tested this patch. However, I feel the problem is related to the RTSP_CMD_SETUP state. I'm not sure, but seems that TVH doesn't send RTCP packets until the tuner is on... and if the client not waits sufficient time, then the trouble appears.

Why not starting to send RTCP packets just before process the SETUP request? If the tuner has 0 signal, or no signal at all, it's reasonable to send signal=0.

I feel this is the best way to resolve any trouble with clients that expect to receive some RTCP data before sending the PLAY command.
I'm right or confused?

#8 Updated by Thomas Göttgens 4 months ago

What baffles me is, that the actual change in behaviour is processed fine by the client, but the later cosmetic change breaks stuff. I'm going to sniff some actual packets on the network tomorrow to see what might confuse either tvheadend or dvbviewer.

#9 Updated by Jaroslav Kysela 4 months ago

It should be fixed in v4.3-272-g1651209 . Sorry for that overlook.

#10 Updated by Jaroslav Kysela 4 months ago

The proper full fix is in v4.3-279-gdaa5a1e . Early morning for me.

#11 Updated by Thomas Göttgens 4 months ago

Thanks, that fixed it for me.

Also available in: Atom PDF