Feature #4881
SAT>IP: Printing the UDP/RTP ports in the connection status of the UI
100%
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.
Files
History
Updated by Jaroslav Kysela almost 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).
Updated by Mono Polimorph almost 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!
Updated by Mono Polimorph almost 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',
Updated by Mono Polimorph over 6 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?
Updated by Jaroslav Kysela over 6 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).
Updated by Jaroslav Kysela over 6 years ago
Also, we should probably skip RTSP_TCP_DATA case completely, because this port is in the different field.
Updated by Mono Polimorph over 6 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?
Updated by Mono Polimorph over 6 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',
Updated by Mono Polimorph over 6 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!
Updated by Jaroslav Kysela over 6 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.
Updated by Mono Polimorph over 6 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 bothIt'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?
Updated by Jaroslav Kysela over 6 years ago
- Status changed from New to Fixed
- % Done changed from 0 to 100
Applied in changeset commit:tvheadend|f0f9aa605ce4293b28430af3344b65ba14ec4e6e.
Updated by Mono Polimorph over 6 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!
Updated by Mono Polimorph over 6 years ago
Jaroslav Kysela wrote:
Applied in changeset commit:tvheadend|f0f9aa605ce4293b28430af3344b65ba14ec4e6e.
Great! I completly agree with your final implementation. Thank you!