Skip to content
Permalink
Browse files

Added NAKREPORT SRTHS flag as per m_bRcvNakreport field (#618)

* Added NAKREPORT SRTHS flag as per m_bRcvNakreport field
  • Loading branch information...
ethouris authored and rndi committed Mar 28, 2019
1 parent c732d8d commit cac926fa9d95d740f4d8474c479829a2525e2b51
Showing with 5 additions and 2 deletions.
  1. +5 −2 srtcore/core.cpp
@@ -1372,6 +1372,9 @@ size_t CUDT::fillSrtHandshake_HSREQ(uint32_t* srtdata, size_t /* srtlen - unused
}
}

if (m_bRcvNakReport)
srtdata[SRT_HS_FLAGS] |= SRT_OPT_NAKREPORT;

// I support SRT_OPT_REXMITFLG. Do you?
srtdata[SRT_HS_FLAGS] |= SRT_OPT_REXMITFLG;

@@ -1381,8 +1384,8 @@ size_t CUDT::fillSrtHandshake_HSREQ(uint32_t* srtdata, size_t /* srtlen - unused
srtdata[SRT_HS_FLAGS] |= SRT_OPT_STREAM;

HLOGC(mglog.Debug, log << "HSREQ/snd: LATENCY[SND:" << SRT_HS_LATENCY_SND::unwrap(srtdata[SRT_HS_LATENCY])
<< " RCV:" << SRT_HS_LATENCY_RCV::unwrap(srtdata[SRT_HS_LATENCY]) << "] FLAGS["
<< SrtFlagString(srtdata[SRT_HS_FLAGS]) << "]");
<< " RCV:" << SRT_HS_LATENCY_RCV::unwrap(srtdata[SRT_HS_LATENCY]) << "] FLAGS["
<< SrtFlagString(srtdata[SRT_HS_FLAGS]) << "]");

return 3;
}

4 comments on commit cac926f

@alexpokotilo

This comment has been minimized.

Copy link
Contributor

replied May 29, 2019

Hi @ethouris ,
testing srt-live-transmit from 1.3.2 release with srt sender in listener mode we noticed some packet losses. In case of using sender in caller mode there were no losses.

Sender In listener mode setup
ffmpeg -re -i /sources/video/sample.mp4 -c copy -f mpegts - | ./srt-live-transmit -s:50 -f file://con srt://:9005
Receiver in caller mode
./srt-live-transmit srt://10.254.254.142:9005 file://con | ffplay -

10.254.254.142 is sender's ip here.
Here is statistics from sender:
======= SRT STATS: sid=146681703 1034kB time=00:00:12.16 bitrate= 696.6kbits/s speed= 1x
PACKETS SENT: 832 RECEIVED: 0
LOST PKT SENT: 46 RECEIVED: 0
REXMIT SENT: 92 RECEIVED: 0
DROP PKT SENT: 0 RECEIVED: 0
RATE SENDING: 0.858812 RECEIVING: 0
BELATED RECEIVED: 0 AVG TIME: 0
REORDER DISTANCE: 0
WINDOW FLOW: 7970 CONGESTION: 8192 FLIGHT: 29
LINK RTT: 0.04ms BANDWIDTH: 1700.08Mb/s
BUFFERLEFT: SND: 12243000 RCV: 12286500

======= SRT STATS: sid=146681703 1087kB time=00:00:12.66 bitrate= 703.0kbits/s speed= 1x
PACKETS SENT: 1208 RECEIVED: 0
LOST PKT SENT: 52 RECEIVED: 0
REXMIT SENT: 418 RECEIVED: 0
DROP PKT SENT: 0 RECEIVED: 0
RATE SENDING: 1.17683 RECEIVING: 0
BELATED RECEIVED: 0 AVG TIME: 0
REORDER DISTANCE: 0
WINDOW FLOW: 7970 CONGESTION: 8192 FLIGHT: 79
LINK RTT: 0.04ms BANDWIDTH: 1700.08Mb/s
BUFFERLEFT: SND: 12168000 RCV: 12286500

Then our engineer checked master and problem is gone. 'git bisect' points us to this commit where the problem is fixed.
Am I right that we can just cherry pick this commit on top of 1.3.2 and this should fix losses in our setup?

@maxlovic

This comment has been minimized.

Copy link
Collaborator

replied May 29, 2019

@alexpokotilo Yes, you can.

@alexpokotilo

This comment has been minimized.

Copy link
Contributor

replied May 29, 2019

I'm sorry @maxlovic for bothering you. is it right commit for fixing losses for senders in listening mode ?
Just to double check.

@maxlovic

This comment has been minimized.

Copy link
Collaborator

replied May 29, 2019

@alexpokotilo Based on your description, yes.
This commit basically fixes the sender to detect that the receiver has set the SRTO_NAKREPORT socket option (send periodic NAK reports). When sender knows that the receiver will be sending periodic LOSS_REPORT packets, telling him about the losses it has, the sender will not try to resend potentially lost packets by itself.

Please sign in to comment.
You can’t perform that action at this time.