Feature #4881: SAT>IP: Printing the UDP/RTP ports in the connection status of the UI - Tvheadend

Actions

#1

Updated by Jaroslav Kysela about 7 years ago

It needs some cleanups: Export only 'raw' data through API. So create htsmsg with connection type (string) and port values (integer). The strings (visual representation) should be handled inside the javascript (renderer).

Actions

Copy link

#2

Updated by Mono Polimorph about 7 years ago

Jaroslav Kysela wrote:

It needs some cleanups: Export only 'raw' data through API. So create htsmsg with connection type (string) and port values (integer). The strings (visual representation) should be handled inside the javascript (renderer).

Hi Jaroslav,

So, you say that the "string" "peer_udp_ports" needs to be a list of used UDP ports? And then in the UI I need to parse the list and create the String? I feel this quite more complex to implement it.

If you say another thing, them please explain in a more detail.
(Sorry for taking your time!)

For sure I like to create good patches! And in this case the objective is quite clear: print the UDP ports (and AVP mode) in the connection status. I'm sure this info will be useful for users that need to distinguish between connections using TCP streaming and RTP streaming.

I wait for your comments! :)

Actions

Copy link

#3

Updated by Mono Polimorph about 7 years ago

File SAT-IP-Print-UDP-RTP-ports-in-connection-status-v2.diff SAT-IP-Print-UDP-RTP-ports-in-connection-status-v2.diff added

Jaroslav Kysela wrote:

It needs some cleanups: Export only 'raw' data through API. So create htsmsg with connection type (string) and port values (integer). The strings (visual representation) should be handled inside the javascript (renderer).

Hi Jaroslav,

I do that you suggested. Now, the message contains the list of ports used, without any visual representation (a simple digit list separated by ','). Now it's more simple and clear. For the format in the renderer I don't touched anything, as I think it's not required (0 is equivalent to AVP).

I hope you agree another time for a patch not in a PR. I'm sorry, I can't use the GitHub!
Regards.

diff --git a/src/satip/rtsp.c b/src/satip/rtsp.c
index 7641b85..25e5c63 100644
--- a/src/satip/rtsp.c
+++ b/src/satip/rtsp.c
@@ -1656,8 +1656,9 @@ rtsp_flush_requests(http_connection_t *hc)
 static void
 rtsp_stream_status ( void *opaque, htsmsg_t *m )
 {
-  http_connection_t *hc = opaque;
-  char buf[128];
+  http_connection_t *hc = opaque;  char buf[128];
+  int buf_size = sizeof(buf);
+  struct session *rs = NULL;

   htsmsg_add_str(m, "type", "SAT>IP");
   if (hc->hc_proxy_ip) {
@@ -1666,6 +1667,23 @@ rtsp_stream_status ( void *opaque, htsmsg_t *m )
   }
   if (hc->hc_username)
     htsmsg_add_str(m, "user", hc->hc_username);
+
+  buf[0]='\0';
+  TAILQ_FOREACH(rs, &rtsp_sessions, link) {
+    if (buf_size < 10) break;
+    if (hc->hc_session &&
+        strcmp(rs->session, hc->hc_session) == 0 &&
+        strcmp(rs->peer_ipstr, hc->hc_peer_ipstr) == 0 &&
+        rs->rtp_peer_port > 0) {
+      if (rs->rtp_peer_port == RTSP_TCP_DATA)
+        buf_size -= snprintf(buf + sizeof(buf) - buf_size, buf_size, "%s0", buf_size == sizeof(buf)? "" : ",");
+      else
+        buf_size -= snprintf(buf + sizeof(buf) - buf_size, buf_size, "%s%d,%d", buf_size == sizeof(buf)? "" : ",", rs->rtp_peer_port, rs
->rtp_peer_port + 1);
+    }
+  }
+  if (buf[0] != '\0') {
+    htsmsg_set_str(m, "peer_udp_ports", buf);
+  }
 }

 /*
diff --git a/src/webui/static/app/status.js b/src/webui/static/app/status.js
index 0ac4b78..3a7db18 100644
--- a/src/webui/static/app/status.js
+++ b/src/webui/static/app/status.js
@@ -607,6 +607,7 @@ tvheadend.status_conns = function(panel, index) {
                 { name: 'server_port' },
                 { name: 'peer', sortType: stype },
                 { name: 'peer_port' },
+                { name: 'peer_udp_ports' },
                 { name: 'proxy' },
                 { name: 'user', sortType: stype },
                 {
@@ -645,11 +646,17 @@ tvheadend.status_conns = function(panel, index) {
             }, {
                 width: 50,
                 id: 'peer_port',
-                header: _("Client Port"),
+                header: _("Client TCP Port"),
                 dataIndex: 'peer_port',
                 sortable: true
             }, {
                 width: 50,
+                id: 'peer_udp_ports',
+                header: _("Client UDP Ports"),
+                dataIndex: 'peer_udp_ports',
+                sortable: true
+            }, {
+                width: 50,
                 id: 'user',
                 header: _("Username"),
                 dataIndex: 'user',

Actions

Copy link

#4

Updated by Mono Polimorph about 7 years ago

Jaroslav Kysela wrote:

It needs some cleanups: Export only 'raw' data through API.

Hi Jaroslav,

This it's done in the version 2 of the patch. Any comment?

Actions

Copy link

#5

Updated by Jaroslav Kysela about 7 years ago

It's not what I'd like to implement. Just put the UDP ports to a list like:

l = htsmsg_create_list();
for_each_sessions() {
  htsmsg_add_s32(l, NULL, rs->rtp_peer_port == RTSP_TCP_DATA ? -1 : rs->rtp_peer_port);
}
htsmsg_add_msg(m, "udp_peer_ports", l);

And create proper renderer for udp_peer_ports field in webui javascript (use javascript for the visual representation of JSON integer array).

Actions

Copy link

#6

Updated by Jaroslav Kysela about 7 years ago

Also, we should probably skip RTSP_TCP_DATA case completely, because this port is in the different field.

Actions

Copy link

#7

Updated by Mono Polimorph about 7 years ago

Jaroslav Kysela wrote:

It's not what I'd like to implement. Just put the UDP ports to a list like:

[...]

And create proper renderer for udp_peer_ports field in webui javascript (use javascript for the visual representation of JSON integer array).

Ok. I'll try to do in this way. Thank you for the comment!

Jaroslav Kysela wrote:

Also, we should probably skip RTSP_TCP_DATA case completely, because this port is in the different field.

I explain why this: As we need to know when the user it's using Interlaved AVP Streaming, I prefer to map the value to the port '0'. If we not map it to any number, when streaming over the TCP RTSP connection then no info it's shown in the UI. It'll be more clear (and simple) to use the number '0' that it's an impossible port number.
You agree with that?

Actions

Copy link

#8

Updated by Mono Polimorph about 7 years ago

File SAT-IP-Print-UDP-RTP-ports-in-connection-status-v3.diff SAT-IP-Print-UDP-RTP-ports-in-connection-status-v3.diff added

Hi Jaroslav,

This new version (release 3) of the patch it's conformant with all your resquests.

Please, review it and commit it (when you have time)! ;)

diff --git a/src/satip/rtsp.c b/src/satip/rtsp.c
index 1ed0191..c93afd1 100644
--- a/src/satip/rtsp.c
+++ b/src/satip/rtsp.c
@@ -1658,6 +1658,9 @@ rtsp_stream_status ( void *opaque, htsmsg_t *m )
 {
   http_connection_t *hc = opaque;
   char buf[128];
+  struct session *rs = NULL;
+  htsmsg_t *l;
+  int udpport;

   htsmsg_add_str(m, "type", "SAT>IP");
   if (hc->hc_proxy_ip) {
@@ -1666,6 +1669,23 @@ rtsp_stream_status ( void *opaque, htsmsg_t *m )
   }
   if (hc->hc_username)
     htsmsg_add_str(m, "user", hc->hc_username);
+
+  l = htsmsg_create_list();
+  TAILQ_FOREACH(rs, &rtsp_sessions, link) {
+    if (hc->hc_session &&
+        strcmp(rs->session, hc->hc_session) == 0 &&
+        strcmp(rs->peer_ipstr, hc->hc_peer_ipstr) == 0 &&
+        rs->rtp_peer_port > 0) {
+      udpport = rs->rtp_peer_port;
+      if ( udpport == RTSP_TCP_DATA ) {
+        htsmsg_add_s32(l, NULL, -1);
+      } else {
+        htsmsg_add_s32(l, NULL, udpport);
+        htsmsg_add_s32(l, NULL, udpport+1);
+      }
+    }
+  }
+  htsmsg_add_msg(m, "peer_udp_ports", l);
 }

 /*
diff --git a/src/webui/static/app/status.js b/src/webui/static/app/status.js
index 0ac4b78..06206dd 100644
--- a/src/webui/static/app/status.js
+++ b/src/webui/static/app/status.js
@@ -607,6 +607,7 @@ tvheadend.status_conns = function(panel, index) {
                 { name: 'server_port' },
                 { name: 'peer', sortType: stype },
                 { name: 'peer_port' },
+                { name: 'peer_udp_ports' },
                 { name: 'proxy' },
                 { name: 'user', sortType: stype },
                 {
@@ -645,11 +646,27 @@ tvheadend.status_conns = function(panel, index) {
             }, {
                 width: 50,
                 id: 'peer_port',
-                header: _("Client Port"),
+                header: _("Client TCP Port"),
                 dataIndex: 'peer_port',
                 sortable: true
             }, {
                 width: 50,
+                id: 'peer_udp_ports',
+                header: _("Client UDP Ports"),
+                dataIndex: 'peer_udp_ports',
+                sortable: false,
+                renderer: function(v) {
+                    // return ("000");
+                    var r = [];
+                    Ext.each(v, function(port) {
+                      r.push(port);
+                    });
+                    if (r.length < 1) return "";
+                    r.sort();
+                    return r.join(',');
+                }
+            }, {
+                width: 50,
                 id: 'user',
                 header: _("Username"),
                 dataIndex: 'user',

Actions

Copy link

#9

Updated by Mono Polimorph about 7 years ago

Hi,

Please, remove the

// return ("000");

prior to commit it! This is for testing only.

And one comment: In the function rtsp_stream_status() I use the var "udpport" for clearing the code. If some other special ports will be used in the future, then will be more simple to use this var instead of the pointer "rs->rtp_peer_port".

I hope this time you agree with it! :)

Actions

Copy link

#10

Updated by Jaroslav Kysela about 7 years ago

1) don't put -1 to the list, because the udp ports are 'per RTSP connection', it is obvious that the data are transmitted through TCP or not - if you like, you may show _("TCP") string when the udp port array is missing or empty
2) I don't see the reason to put both UDP ports to the list - by RTP standard, it's always defined that there are two ports (port, port+1), handle this in the renderer, if you like to see both

It's really pitty that I cannot comment your changes directly, the github is really nice for this.

Actions

Copy link

#11

Updated by Mono Polimorph about 7 years ago

Jaroslav Kysela wrote:

1) don't put -1 to the list, because the udp ports are 'per RTSP connection', it is obvious that the data are transmitted through TCP or not - if you like, you may show _("TCP") string when the udp port array is missing or empty
2) I don't see the reason to put both UDP ports to the list - by RTP standard, it's always defined that there are two ports (port, port+1), handle this in the renderer, if you like to see both

It's really pitty that I cannot comment your changes directly, the github is really nice for this.

Hi Jaroslav,

Thank you for commenting it! :)
I'm affraid that I can not use at all GitHUB. Sorry! I'm searching for a solution. Belive me!

1) I put the "-1" as it's the only solution to show what users are using AVP. If you don't put it in the list, then you can't distinguish between a simple open RTSP connection and a RTSP doing a TCP streaming. Futhermore, note that one TCP socket can be used for multiple streamings (controlling more than one session), so using one RTSP socket you can stream for example one RTP and one AVP. And at all, my final objective it's show in the UI the AVP connections. So, please don't remove this.

2) Yes, but this it's for completition. Even, if the RTPC channel will be refused, then the UI can be updated showing only the RTP port. So, writing both ports seems to be ok.

At the end, you will finally accept this patch? ;)

Actions

Copy link

#12

Updated by Jaroslav Kysela about 7 years ago

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

Applied in changeset commit:tvheadend|f0f9aa605ce4293b28430af3344b65ba14ec4e6e.

Actions

Copy link

#13

Updated by Mono Polimorph about 7 years ago

File SAT-IP-Print-UDP-RTP-ports-in-connection-status-v4.diff SAT-IP-Print-UDP-RTP-ports-in-connection-status-v4.diff added

Hi Jaroslav,

Simple update...

diff --git a/src/satip/rtsp.c b/src/satip/rtsp.c
index 1ed0191..c93afd1 100644
--- a/src/satip/rtsp.c
+++ b/src/satip/rtsp.c
@@ -1658,6 +1658,9 @@ rtsp_stream_status ( void *opaque, htsmsg_t *m )
 {
   http_connection_t *hc = opaque;
   char buf[128];
+  struct session *rs = NULL;
+  htsmsg_t *l;
+  int udpport;

   htsmsg_add_str(m, "type", "SAT>IP");
   if (hc->hc_proxy_ip) {
@@ -1666,6 +1669,23 @@ rtsp_stream_status ( void *opaque, htsmsg_t *m )
   }
   if (hc->hc_username)
     htsmsg_add_str(m, "user", hc->hc_username);
+
+  l = htsmsg_create_list();
+  TAILQ_FOREACH(rs, &rtsp_sessions, link) {
+    if (hc->hc_session &&
+        strcmp(rs->session, hc->hc_session) == 0 &&
+        strcmp(rs->peer_ipstr, hc->hc_peer_ipstr) == 0 &&
+        rs->rtp_peer_port > 0) {
+      udpport = rs->rtp_peer_port;
+      if ( udpport == RTSP_TCP_DATA ) {
+        htsmsg_add_s32(l, NULL, -1);
+      } else {
+        htsmsg_add_s32(l, NULL, udpport);
+        htsmsg_add_s32(l, NULL, udpport+1);
+      }
+    }
+  }
+  htsmsg_add_msg(m, "peer_udp_ports", l);
 }

 /*
diff --git a/src/webui/static/app/status.js b/src/webui/static/app/status.js
index 988934e..57cc0ac 100644
--- a/src/webui/static/app/status.js
+++ b/src/webui/static/app/status.js
@@ -625,6 +625,7 @@ tvheadend.status_conns = function(panel, index) {
                 { name: 'server_port' },
                 { name: 'peer', sortType: stype },
                 { name: 'peer_port' },
+                { name: 'peer_udp_ports' },
                 { name: 'proxy' },
                 { name: 'user', sortType: stype },
                 {
@@ -663,11 +664,26 @@ tvheadend.status_conns = function(panel, index) {
             }, {
                 width: 50,
                 id: 'peer_port',
-                header: _("Client Port"),
+                header: _("Client TCP Port"),
                 dataIndex: 'peer_port',
                 sortable: true
             }, {
                 width: 50,
+                id: 'peer_udp_ports',
+                header: _("Client UDP Ports"),
+                dataIndex: 'peer_udp_ports',
+                sortable: false,
+                renderer: function(v) {
+                    var r = [];
+                    Ext.each(v, function(port) {
+                      r.push(port);
+                    });
+                    if (r.length < 1) return "";
+                    r.sort(function(a, b){return a-b});
+                    return r.join(',');
+                }
+            }, {
+                width: 50,
                 id: 'user',
                 header: _("Username"),
                 dataIndex: 'user',

You can download the diff file and apply it!

Actions

Copy link

#14

Updated by Mono Polimorph about 7 years ago

Jaroslav Kysela wrote:

Applied in changeset commit:tvheadend|f0f9aa605ce4293b28430af3344b65ba14ec4e6e.

Great! I completly agree with your final implementation. Thank you! (/)

SAT-IP-Print-UDP-RTP-ports-in-connection-status.diff (2.54 KB) SAT-IP-Print-UDP-RTP-ports-in-connection-status.diff		Mono Polimorph, 2018-01-22 20:24
SAT-IP-Print-UDP-RTP-ports-in-connection-status-v2.diff (2.56 KB) SAT-IP-Print-UDP-RTP-ports-in-connection-status-v2.diff	version 2	Mono Polimorph, 2018-02-01 16:07
SAT-IP-Print-UDP-RTP-ports-in-connection-status-v3.diff (2.67 KB) SAT-IP-Print-UDP-RTP-ports-in-connection-status-v3.diff	version 3	Mono Polimorph, 2018-02-12 16:45
SAT-IP-Print-UDP-RTP-ports-in-connection-status-v4.diff (2.65 KB) SAT-IP-Print-UDP-RTP-ports-in-connection-status-v4.diff	version 4	Mono Polimorph, 2018-02-12 21:08

Project

General

Profile

Tvheadend

Custom queries

Feature #4881

SAT>IP: Printing the UDP/RTP ports in the connection status of the UI

Updated by Jaroslav Kysela about 7 years ago

Updated by Mono Polimorph about 7 years ago

Updated by Mono Polimorph about 7 years ago

Updated by Mono Polimorph about 7 years ago

Updated by Jaroslav Kysela about 7 years ago

Updated by Jaroslav Kysela about 7 years ago

Updated by Mono Polimorph about 7 years ago

Updated by Mono Polimorph about 7 years ago

Updated by Mono Polimorph about 7 years ago

Updated by Jaroslav Kysela about 7 years ago

Updated by Mono Polimorph about 7 years ago

Updated by Jaroslav Kysela about 7 years ago

Updated by Mono Polimorph about 7 years ago

Updated by Mono Polimorph about 7 years ago