Project

General

Profile

Feature #4881

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

Added by Mono Polimorph 11 months ago. Updated 10 months ago.

Status:
Fixed
Priority:
Normal
Category:
SAT>IP
Target version:
-
Start date:
2018-01-22
Due date:
% Done:

100%

Estimated time:

Description

Hi Jarsolav,

Here another patch related to the UI, as promised in #4748#note-13:

diff --git a/src/satip/rtsp.c b/src/satip/rtsp.c
index 7641b85..3bb6d5a 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, "%sAVP", buf_size == sizeof(buf)? "" : ",");
+      else
+        buf_size -= snprintf(buf + sizeof(buf) - buf_size, buf_size, "%s%d+1", buf_size == sizeof(buf)? "" : ",", rs->rtp_peer_port);
+    }
+  }
+  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',

This patch is for printing the UDP used ports by SAT>IP clients. It has a very interesting advantage: when the user uses the TCP socket for the streaming it prints “AVP”. So, you can check in the server which users are using Interlaved RTSP.

Furthermore, the patch prints all ports used by each client, as the user can request more than one stream using the same RTSP session.

I hope you like to commit this patch too! :)
Regards.

Associated revisions

Revision f0f9aa60 (diff)
Added by Jaroslav Kysela 10 months ago

satip,webui: add client extra ports to the status page, fixes #4881

History

#1 Updated by Jaroslav Kysela 11 months 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).

#2 Updated by Mono Polimorph 11 months 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! :)

#3 Updated by Mono Polimorph 10 months 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,

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',

#4 Updated by Mono Polimorph 10 months 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?

#5 Updated by Jaroslav Kysela 10 months 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).

#6 Updated by Jaroslav Kysela 10 months ago

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

#7 Updated by Mono Polimorph 10 months 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?

#8 Updated by Mono Polimorph 10 months ago

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',

#9 Updated by Mono Polimorph 10 months 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! :)

#10 Updated by Jaroslav Kysela 10 months 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.

#11 Updated by Mono Polimorph 10 months 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? ;)

#12 Updated by Jaroslav Kysela 10 months ago

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

#13 Updated by Mono Polimorph 10 months ago

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!

#14 Updated by Mono Polimorph 10 months ago

Jaroslav Kysela wrote:

Applied in changeset tvheadend|f0f9aa605ce4293b28430af3344b65ba14ec4e6e.

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

Also available in: Atom PDF