Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Troubles with statistics #245

Closed
lars18th opened this issue Jan 26, 2018 · 17 comments
Closed

Troubles with statistics #245

lars18th opened this issue Jan 26, 2018 · 17 comments

Comments

@lars18th
Copy link
Contributor

Hi,

I'm running both versions of "stransmit", master 1.2.2 and dev 1.3.0. And the protocol seems to work right with a test streaming over the Internet.

However, the statistics do not seem to be working. Here the environment:

RECEIVER:
stransmit -r:500 -s:500 "srt://sender.ddns.org:10000?mode=server&latency=2400" udp://192.168.1.20@239.1.1.1:12345

SENDER:
stransmit -r:500 -s:500 udp://239.1.1.1:12345 "srt://receiver.ddns.org:10000?mode=client&latency=2400"

And all works right. The multicast in the udp://@239.1.1.1:12345 address it's captured in the "sender" network and streamed without errors to the "receiver" network. Great! 😄

However, I see a very strange STATISTICS in both ends...

RECEIVER:

======= SRT STATS: sid=812086801
PACKETS SENT: 0 RECEIVED: 1
LOST PKT SENT: 0 RECEIVED: 0
REXMIT SENT: 0 RECEIVED: 32724
RATE SENDING: 0 RECEIVING: 2.889006904
BELATED RECEIVED: 190 AVG TIME: 1.837705424e+16
REORDER DISTANCE: 0
WINDOW: FLOW: 8192 CONGESTION: 8192 FLIGHT: 0
RTT: 466.563ms  BANDWIDTH: 3.768Mb/s
BUFFERLEFT: SND: 12288000 RCV: 11083500
+++/+++SRT TRANSFER: 78958684B DURATION: 222501ms SPEED: 346.563kB/s
+++/+++SRT BANDWIDTH: 3.468

SENDER:

+++/+++SRT TRANSFER: 80932684B DURATION: 224463ms SPEED: 352.112kB/s
+++/+++SRT TRANSFER: 81590684B DURATION: 226272ms SPEED: 352.137kB/s
+++/+++SRT TRANSFER: 82248684B DURATION: 227981ms SPEED: 352.320kB/s
+++/+++SRT TRANSFER: 82906684B DURATION: 229697ms SPEED: 352.490kB/s
+++/+++SRT TRANSFER: 83564684B DURATION: 231401ms SPEED: 352.675kB/s
+++/+++SRT TRANSFER: 84222684B DURATION: 233126ms SPEED: 352.826kB/s
+++/+++SRT TRANSFER: 84880684B DURATION: 234837ms SPEED: 352.996kB/s
+++/+++SRT TRANSFER: 85538684B DURATION: 236564ms SPEED: 353.115kB/s
+++/+++SRT TRANSFER: 86196684B DURATION: 238333ms SPEED: 353.192kB/s
+++/+++SRT TRANSFER: 86854684B DURATION: 240002ms SPEED: 353.418kB/s

It's this correct? Here some questions:

  • Why the stats of both ends are different? Why the sender doesn't prints the SRT STATS, but only the SRT TRANSFER?
  • Why the "SENT" values in the receiver are allways zero?
  • Why the number of REXMIT (retransmitions?) is a fixed value even from the start?
  • Why the RATE RECEIVING has a format of "digit.digits"? It's this a Megabit/s rate?
  • Why the LOST PKT it's allways zero, even when some packets are lost?
  • Why the RECEIVED PACKET count changes from 0-4 in each print? This is for dev branch, with master branch it seems to be a incremental counter.
  • What it's the "sid" value?
  • And finally, why the SRT STATS aren't tabulated for an easier reading? 😄

Please, help me to understand this.
Thank you!

@lars18th
Copy link
Contributor Author

lars18th commented Jan 26, 2018

Hi @ethouris ,

Regarding the difficulty to read the statistics, I suggest this patch of the file common/transmitmedia.cpp that makes it more human readable:

 #include <iostream>
+#include <iomanip>
 #include <fstream>

...

template <class PerfMonType>
void PrintSrtStats(int sid, const PerfMonType& mon)
{
    cout << "======= SRT STATS: sid=" << sid << endl;
    cout << "PACKETS SENT:     " << setw(11) << mon.pktSent            << "  RECEIVED:   " << setw(11) << mon.pktRecv              << endl;
    cout << "LOST PKT SENT:    " << setw(11) << mon.pktSndLoss         << "  RECEIVED:   " << setw(11) << mon.pktRcvLoss           << endl;
    cout << "REXMIT SENT:      " << setw(11) << mon.pktRetrans         << "  RECEIVED:   " << setw(11) << mon.pktRcvRetrans        << endl;
    cout << "RATE SENDING:     " << setw(11) << mon.mbpsSendRate       << "  RECEIVING:  " << setw(11) << mon.mbpsRecvRate         << endl;
    cout << "BELATED RECEIVED: " << setw(11) << mon.pktRcvBelated      << "  AVG TIME:   " << setw(11) << mon.pktRcvAvgBelatedTime << endl;
    cout << "REORDER DISTANCE: " << setw(11) << mon.pktReorderDistance << endl;
    cout << "WINDOW: FLOW:     " << setw(11) << mon.pktFlowWindow      << "  CONGESTION: " << setw(11) << mon.pktCongestionWindow  << "  FLIGHT: " << setw(11) << mon.pktFlightSize << endl;
    cout << "RTT:              " << setw(9)  << mon.msRTT            << "ms  BANDWIDTH:  " << setw(7)  << mon.mbpsBandwidth    << "Mb/s " << endl;
    cout << "BUFFERLEFT: SND:  " << setw(11) << mon.byteAvailSndBuf    << "  RCV:        " << setw(11) << mon.byteAvailRcvBuf      << endl;
}

It makes both (source and logs) more readable. You agree?

@lars18th
Copy link
Contributor Author

lars18th commented Jan 26, 2018

Hi @ethouris ,

Regarding the error in the printed statistics, I found that perhaps the problem is related to the declaration of the SRT_ENABLE_BSTATS. In all the code, exists an #ifdef SRT_ENABLE_BSTATS except in the source of the "stransmit.cpp". In this file it's allways used CBytePerfMon perf instead of the standard CPerfMon perf.

Perhaps the problem is related to this. I'll check it more.
Any idea?

UPDATE: Don't worry about this! I see that in the "dev" branch this is deprecated. I search in another place.

@lars18th
Copy link
Contributor Author

Hi @ethouris ,

I have found why the SENDER doesn't writes the stats!
It's because it needs to print them in the SrtTarget::Write() function, like the RECEIVER does in the SrtSource::Read().
I'll prepare a PR, or you do it! 😄

However, I'm still looking why the data printed it's wrong. I feel it's something related to signed/unsigned comparison and wrong types, and perhaps also the declaration of TRACEBSTATS and SRT_TRACEBSTATS as they are different:

typedef CBytePerfMon TRACEBSTATS;

typedef struct CBytePerfMon SRT_TRACEBSTATS;

I continue working.

@ethouris
Copy link
Collaborator

Whoa, slow down. :) There are several problems here.

Well, these TRACEBSTATS and SRT_TRACEBSTATS is that there are two "competitive" API files, which are:

  • srt.h - official, C-compatible API
  • udt.h - legacy, UDT-derived and C++-only compatible API (plus some C++-only API extras added by SRT)

So, this TRACEBSTATS is defined this way because this is C++, so " you don't need no stinkun' " struct, as well as this declaration is under UDT namespace. This is kinda legacy, actually not coming from UDT, just backward compatibility with applications using older versions of SRT. So, this doesn't matter at all.

The SRT_ENABLE_BSTATS is some legacy thing, likely could be removed, just this kind of refactoring goes slowly. This is always set to 1, otherwise stransmit won't even compile.

If you think you can fix anything around printing stats, you are welcome to do it. Just one small problem here about printing sender stats: Currently this is turned on with an option, and options are thought to be general for the whole applications, while stats obviously touch upon one of two media, so if you want to provide printing SRT stats also for sending, it would be better done with not following the option parameter, but rather by adding an extra URI parameter, so that it can be specified directly when defining SRT medium.

How to do it - look for SrtCommon::InitParameters in transmitmedia.cpp file. The URI parameters after being parsed are stored in the par parameter here. Normally these parameters should map directly to SRT socket options, but there can be some "special parameters", and they are managed here, like for example mode or blocking. Just remember that if this is a special parameter, remove it from this map after you find and process it, otherwise an option with that name will be searched later and the error reported if not found. The transmit_stats_report variable can be then completely removed, and the field controlling it can be added to SrtCommon class instead, this way you can set it in this function that interprets the URI parameters, and then use in both SrtSource::Read and SrtTarget::Write. The same can be done also with Bandwidth Report. You can use it the same way as the option is used now - a parameter with integer value, which defines report frequency.

@lars18th
Copy link
Contributor Author

Hi @ethouris ,

I found the errors! 😄

My first patch it's in this PR #255.
Futhermore, I see where it's the error with the retrasmission received counter. I'll publish also a patch (this error it's in the core).

However, I still have a question: Where it's the counter for the UNCORRECTED PACKETS? That it's the finally lost (stolen) packets. This counter exists?

@lars18th
Copy link
Contributor Author

Hi @ethouris ,

All my patches related to this are in the PR #255 .

However, I still have the question: The countet UNCORRECTED PACKETS (in the receiver) exists?
If not, what you think about inclue it? If so, where is it?

Regards.

@ethouris
Copy link
Collaborator

There's no such thing as "uncorrected packets" in SRT. The only packets that are not delivered directly at normal sending process are retransmitted and dropped. Might be that the latter is what you need.

Note that packets may be dropped in both sender and receiver due to:

  • receiver: the next-to-lost packet is NOW ready to play and longer waiting for the lost packet recovery would cause the ready packet to be delayed and we don't even know how long, possibly indefinitely, so the packet is agreed to be lost
  • sender: the sender is aware of that the receiver does TLPKTDROP and its latency, and that the packet being still lost will never make it on time, regarding the current RTT value and latency, so the packet is removed from retransmission list

Both sender and receiver have their own stats of dropped packets. Obviously, the packets that are notified as dropped by the sender are also dropped at the receiver side, but the sender might be sometimes unaware of that the packet was dropped by the receiver stating that the receiver acknowledges the packet that has been dropped. The sender notifies only those packets that have been resigned from further retransmitting.

@lars18th
Copy link
Contributor Author

lars18th commented Feb 1, 2018

Hi @ethouris ,

And you know where it's the counter for the dopped packets in the receiver?
I like to know the number of stolen packets without monitoring the streaming (only reading the stats in the client side).

Thank you for your comments!

@ethouris
Copy link
Collaborator

ethouris commented Feb 1, 2018

Fields named pktSndDrop and byteSndDrop and the corresponding ones with Rcv inside. I think they are not added to the stats table in transmitmedia.cpp - you can add them.

@lars18th
Copy link
Contributor Author

lars18th commented Feb 1, 2018

Hi @ethouris ,

Thank you!
Done in #264
I hope you merge it! 😄

@ethouris
Copy link
Collaborator

ethouris commented Feb 1, 2018

You need to fix it first

@lars18th
Copy link
Contributor Author

lars18th commented Feb 1, 2018

You need to fix it first

I thought I already did! 😕

@ethouris
Copy link
Collaborator

ethouris commented Feb 1, 2018

Ok, now it looks much better, but I don't understand what happened to checkers and AppVeyor...

@lars18th
Copy link
Contributor Author

lars18th commented Feb 1, 2018

Ok, now it looks much better, but I don't understand what happened to checkers and AppVeyor...

This is because I changed the "target" from "master" to "dev". Now the checkers don't restart.
In any case the patch is quite simple. ☺️

@ethouris
Copy link
Collaborator

ethouris commented Feb 1, 2018

Yes I know, we must kick this thing somehow. Or, if not possible, I'll make a check by myself.

@lars18th
Copy link
Contributor Author

lars18th commented Feb 1, 2018

If you prefer, I can delete the PR and create a new one.
Or you can try to force the check.

@lars18th
Copy link
Contributor Author

lars18th commented Feb 1, 2018

I close as all it's merged! 👍

@lars18th lars18th closed this as completed Feb 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants