Skip to content

Removed payload size dependency and option #1130

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.

Sign up for GitHub

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

Open
ethouris wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Removed payload size dependency and option #1130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fabd52f
Improved log
Feb 13, 2020
164cccf
Fixed FEC-corrected payload size. Restored payload size as single sen…
Feb 13, 2020
b4e906b
Fixed unit test
Feb 13, 2020
4772dba
Merge branch 'master' into dev-fix-payloadsize-internals
Feb 28, 2020
b31cc0d
Updated to latest upstream
ethouris May 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions srtcore/congctl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ class LiveCC: public SrtCongestionControlBase
: SrtCongestionControlBase(parent)
{
m_llSndMaxBW = BW_INFINITE; // 1 Gbbps in Bytes/sec BW_INFINITE
m_zMaxPayloadSize = parent->OPT_PayloadSize();
m_zMaxPayloadSize = parent->userPayloadSize();
if ( m_zMaxPayloadSize == 0 )
m_zMaxPayloadSize = parent->maxPayloadSize();
m_zSndAvgPayloadSize = m_zMaxPayloadSize;
m_zSndAvgPayloadSize = 0;

m_iMinNakInterval_us = 20000; //Minimum NAK Report Period (usec)
m_iNakReportAccel = 2; //Default NAK Report Period (RTT) accelerator
Expand Down Expand Up @@ -148,8 +148,13 @@ class LiveCC: public SrtCongestionControlBase
// Worst case scenario, the procedure running in CRcvQueue::worker
// thread will pick up a "slightly outdated" average value from this
// field - this is insignificant.
m_zSndAvgPayloadSize = avg_iir<128, size_t>(m_zSndAvgPayloadSize, packet.getLength());
HLOGC(cclog.Debug, log << "LiveCC: avg payload size updated: " << m_zSndAvgPayloadSize);
size_t plen = packet.getLength();
if (m_zSndAvgPayloadSize == 0)
m_zSndAvgPayloadSize = plen;
else
m_zSndAvgPayloadSize = avg_iir<128>(m_zSndAvgPayloadSize, plen);

HLOGC(cclog.Debug, log << "LiveCC: avg payload size updated: " << plen << " -> " << m_zSndAvgPayloadSize);
}

void updatePktSndPeriod_onTimer(ETransmissionEvent , EventVariant var)
Expand All @@ -165,6 +170,9 @@ class LiveCC: public SrtCongestionControlBase

void updatePktSndPeriod()
{
if (m_zSndAvgPayloadSize == 0)
return; // do not update - nothing was sent so far

// packet = payload + header
const double pktsize = (double) m_zSndAvgPayloadSize + CPacket::SRT_DATA_HDR_SIZE;
m_dPktSndPeriod = 1000 * 1000.0 * (pktsize / m_llSndMaxBW);
Expand Down
82 changes: 30 additions & 52 deletions srtcore/core.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ CUDT::CUDT(CUDTSocket* parent): m_parent(parent)
m_HSGroupType = SRT_GTYPE_UNDEFINED;
m_bTLPktDrop = true; // Too-late Packet Drop
m_bMessageAPI = true;
m_zOPT_ExpPayloadSize = SRT_LIVE_DEF_PLSIZE;
m_zUserPayloadSize = calcMaxPayloadSize();
m_iIpV6Only = -1;
// Runtime
m_bRcvNakReport = true; // Receiver's Periodic NAK Reports
Expand Down Expand Up @@ -324,7 +324,7 @@ CUDT::CUDT(CUDTSocket* parent, const CUDT& ancestor): m_parent(parent)
m_iOPT_PeerIdleTimeout = ancestor.m_iOPT_PeerIdleTimeout;
m_uOPT_StabilityTimeout = ancestor.m_uOPT_StabilityTimeout;
m_OPT_GroupConnect = ancestor.m_OPT_GroupConnect; // NOTE: on single accept set back to 0
m_zOPT_ExpPayloadSize = ancestor.m_zOPT_ExpPayloadSize;
m_zUserPayloadSize = ancestor.m_zUserPayloadSize;
m_bTLPktDrop = ancestor.m_bTLPktDrop;
m_bMessageAPI = ancestor.m_bMessageAPI;
m_iIpV6Only = ancestor.m_iIpV6Only;
Expand Down Expand Up @@ -789,30 +789,7 @@ void CUDT::setOpt(SRT_SOCKOPT optName, const void* optval, int optlen)
throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
}

if (m_OPT_PktFilterConfigString != "")
{
// This means that the filter might have been installed before,
// and the fix to the maximum payload size was already applied.
// This needs to be checked now.
SrtFilterConfig fc;
if (!ParseFilterConfig(m_OPT_PktFilterConfigString, fc))
{
// Break silently. This should not happen
LOGC(mglog.Error, log << "SRTO_PAYLOADSIZE: IPE: failing filter configuration installed");
throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
}

size_t efc_max_payload_size = SRT_LIVE_MAX_PLSIZE - fc.extra_size;
if (m_zOPT_ExpPayloadSize > efc_max_payload_size)
{
LOGC(mglog.Error,
log << "SRTO_PAYLOADSIZE: value exceeds SRT_LIVE_MAX_PLSIZE decreased by " << fc.extra_size
<< " required for packet filter header");
throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
}
}

m_zOPT_ExpPayloadSize = *(int *)optval;
LOGC(mglog.Warn, log << "SRTO_PAYLOADSIZE: this option is deprecated and ignored; you can safely remove it.");
break;

case SRTO_TRANSTYPE:
Expand All @@ -838,7 +815,7 @@ void CUDT::setOpt(SRT_SOCKOPT optName, const void* optval, int optlen)
m_iOPT_SndDropDelay = 0;
m_bMessageAPI = true;
m_bRcvNakReport = true;
m_zOPT_ExpPayloadSize = SRT_LIVE_DEF_PLSIZE;
m_zUserPayloadSize = calcMaxPayloadSize();
m_Linger.l_onoff = 0;
m_Linger.l_linger = 0;
m_CongCtl.select("live");
Expand All @@ -858,7 +835,7 @@ void CUDT::setOpt(SRT_SOCKOPT optName, const void* optval, int optlen)
m_iOPT_SndDropDelay = -1;
m_bMessageAPI = false;
m_bRcvNakReport = false;
m_zOPT_ExpPayloadSize = 0; // use maximum
m_zUserPayloadSize = 0; // no limit check
m_Linger.l_onoff = 1;
m_Linger.l_linger = DEF_LINGER_S;
m_CongCtl.select("file");
Expand Down Expand Up @@ -948,13 +925,11 @@ void CUDT::setOpt(SRT_SOCKOPT optName, const void* optval, int optlen)
throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
}

size_t efc_max_payload_size = SRT_LIVE_MAX_PLSIZE - fc.extra_size;
if (m_zOPT_ExpPayloadSize > efc_max_payload_size)
size_t efc_max_payload_size = calcMaxPayloadSize() - fc.extra_size;
if (m_zUserPayloadSize > efc_max_payload_size)
{
LOGC(mglog.Warn,
log << "Due to filter-required extra " << fc.extra_size << " bytes, SRTO_PAYLOADSIZE fixed to "
<< efc_max_payload_size << " bytes");
m_zOPT_ExpPayloadSize = efc_max_payload_size;
m_zUserPayloadSize = efc_max_payload_size;
HLOGC(mglog.Debug, log << "SRTO_PACKETFILTER: fixed user max payload size: " << m_zUserPayloadSize);
}

m_OPT_PktFilterConfigString = arg;
Expand Down Expand Up @@ -1260,7 +1235,8 @@ void CUDT::getOpt(SRT_SOCKOPT optName, void *optval, int &optlen)

case SRTO_PAYLOADSIZE:
optlen = sizeof(int);
*(int *)optval = m_zOPT_ExpPayloadSize;
LOGC(mglog.Warn, log << "SRTO_PAYLOADSIZE: option deprecated and ignored.");
*(int *)optval = 0;
break;

case SRTO_GROUPCONNECT:
Expand Down Expand Up @@ -1331,9 +1307,9 @@ std::string CUDT::getstreamid(SRTSOCKET u)
void CUDT::clearData()
{
// Initial sequence number, loss, acknowledgement, etc.
int udpsize = m_iMSS - CPacket::UDP_HDR_SIZE;

m_iMaxSRTPayloadSize = udpsize - CPacket::HDR_SIZE;
m_iMaxSRTPayloadSize = calcMaxPayloadSize();
if (m_zUserPayloadSize != 0 && int(m_zUserPayloadSize) > m_iMaxSRTPayloadSize)
m_zUserPayloadSize = m_iMaxSRTPayloadSize;

HLOGC(mglog.Debug, log << "clearData: PAYLOAD SIZE: " << m_iMaxSRTPayloadSize);

Expand Down Expand Up @@ -3326,13 +3302,11 @@ bool CUDT::checkApplyFilterConfig(const std::string &confstr)
m_OPT_PktFilterConfigString = confstr;
}

size_t efc_max_payload_size = SRT_LIVE_MAX_PLSIZE - cfg.extra_size;
if (m_zOPT_ExpPayloadSize > efc_max_payload_size)
size_t efc_max_payload_size = calcMaxPayloadSize() - cfg.extra_size;
if (m_zUserPayloadSize > efc_max_payload_size)
{
LOGC(mglog.Warn,
log << "Due to filter-required extra " << cfg.extra_size << " bytes, SRTO_PAYLOADSIZE fixed to "
<< efc_max_payload_size << " bytes");
m_zOPT_ExpPayloadSize = efc_max_payload_size;
m_zUserPayloadSize = efc_max_payload_size;
HLOGC(mglog.Debug, log << "checkApplyFilterConfig: fixed user max payload size: " << m_zUserPayloadSize);
}

return true;
Expand Down Expand Up @@ -4926,13 +4900,18 @@ EConnectStatus CUDT::processConnectResponse(const CPacket& response, CUDTExcepti
return postConnect(response, false, eout, synchro);
}

size_t CUDT::calcMaxPayloadSize() const
{
const int udpsize = m_iMSS - CPacket::UDP_HDR_SIZE;
return udpsize - CPacket::HDR_SIZE;
}

void CUDT::applyResponseSettings()
{
// Re-configure according to the negotiated values.
m_iMSS = m_ConnRes.m_iMSS;
m_iFlowWindowSize = m_ConnRes.m_iFlightFlagSize;
int udpsize = m_iMSS - CPacket::UDP_HDR_SIZE;
m_iMaxSRTPayloadSize = udpsize - CPacket::HDR_SIZE;
m_iMaxSRTPayloadSize = calcMaxPayloadSize();
m_iPeerISN = m_ConnRes.m_iISN;

setInitialRcvSeq(m_iPeerISN);
Expand Down Expand Up @@ -5808,9 +5787,9 @@ void CUDT::acceptAndRespond(const sockaddr_any& peer, const CPacket& hspkt, CHan
memcpy((m_piSelfIP), w_hs.m_piPeerIP, sizeof m_piSelfIP);
CIPAddress::ntop(peer, (w_hs.m_piPeerIP));

int udpsize = m_iMSS - CPacket::UDP_HDR_SIZE;
m_iMaxSRTPayloadSize = udpsize - CPacket::HDR_SIZE;
HLOGC(mglog.Debug, log << "acceptAndRespond: PAYLOAD SIZE: " << m_iMaxSRTPayloadSize);
m_iMaxSRTPayloadSize = calcMaxPayloadSize();
HLOGC(mglog.Debug, log << "acceptAndRespond: PAYLOAD SIZE user: " << m_zUserPayloadSize
<< " max:" << m_iMaxSRTPayloadSize);

// Prepare all structures
if (!prepareConnectionObjects(w_hs, HSD_DRAW, 0))
Expand Down Expand Up @@ -6000,7 +5979,7 @@ SRT_REJECT_REASON CUDT::setupCC()

// At this point we state everything is checked and the appropriate
// corrector type is already selected, so now create it.
HLOGC(mglog.Debug, log << "filter: Configuring Corrector: " << m_OPT_PktFilterConfigString);
HLOGC(mglog.Debug, log << "filter: Configuring: " << m_OPT_PktFilterConfigString);
if (!m_PacketFilter.configure(this, m_pRcvBuffer->getUnitQueue(), m_OPT_PktFilterConfigString))
{
return SRT_REJ_FILTER;
Expand Down Expand Up @@ -11213,7 +11192,7 @@ CUDTGroup::gli_t CUDTGroup::add(SocketData data)
gli_t end = m_Group.end();
if (m_iMaxPayloadSize == -1)
{
int plsize = data.ps->m_pUDT->OPT_PayloadSize();
int plsize = data.ps->m_pUDT->m_zUserPayloadSize;
HLOGC(mglog.Debug, log << "CUDTGroup::add: taking MAX payload size from socket @" << data.ps->m_SocketID << ": " << plsize
<< " " << (plsize ? "(explicit)" : "(unspecified = fallback to 1456)"));
if (plsize == 0)
Expand Down Expand Up @@ -11540,7 +11519,6 @@ void CUDTGroup::deriveSettings(CUDT* u)
IM(SRTO_RCVLATENCY, m_iOPT_TsbPdDelay);
IM(SRTO_PEERLATENCY, m_iOPT_PeerTsbPdDelay);
IM(SRTO_SNDDROPDELAY, m_iOPT_SndDropDelay);
IM(SRTO_PAYLOADSIZE, m_zOPT_ExpPayloadSize);
IM(SRTO_TLPKTDROP, m_bTLPktDrop);
IM(SRTO_STREAMID, m_sStreamName);
IM(SRTO_MESSAGEAPI, m_bMessageAPI);
Expand Down
8 changes: 4 additions & 4 deletions srtcore/core.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -895,7 +895,8 @@ class CUDT

uint32_t latency_us() const {return m_iTsbPdDelay_ms*1000; }
size_t maxPayloadSize() const { return m_iMaxSRTPayloadSize; }
size_t OPT_PayloadSize() const { return m_zOPT_ExpPayloadSize; }
size_t calcMaxPayloadSize() const;
size_t userPayloadSize() const { return m_zUserPayloadSize; }
int sndLossLength() { return m_pSndLossList->getLossLength(); }
int32_t ISN() const { return m_iISN; }
int32_t peerISN() const { return m_iPeerISN; }
Expand Down Expand Up @@ -1220,9 +1221,8 @@ class CUDT
CUDTSocket* const m_parent; // temporary, until the CUDTSocket class is merged with CUDT
SRTSOCKET m_SocketID; // UDT socket number
SRTSOCKET m_PeerID; // peer id, for multiplexer

int m_iMaxSRTPayloadSize; // Maximum/regular payload size, in bytes
size_t m_zOPT_ExpPayloadSize; // Expected average payload size (user option)
int m_iMaxSRTPayloadSize; // Maximum/regular payload size, in bytes (dependent on MSS)
size_t m_zUserPayloadSize; // Expected average payload size (user option)

// Options
int m_iMSS; // Maximum Segment Size, in bytes
Expand Down
3 changes: 1 addition & 2 deletions srtcore/packetfilter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,7 @@ bool PacketFilter::configure(CUDT* parent, CUnitQueue* uq, const std::string& co
init.socket_id = parent->socketID();
init.snd_isn = parent->sndSeqNo();
init.rcv_isn = parent->rcvSeqNo();
init.payload_size = parent->OPT_PayloadSize();

init.payload_size = parent->userPayloadSize();

// Found a filter, so call the creation function
m_filter = selector->second->Create(init, m_provided, confstr);
Expand Down
2 changes: 1 addition & 1 deletion test/test_epoll.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ class TestEPoll: public testing::Test
ASSERT_EQ(read[0], acpsock); // read event is for bind socket
}

char buffer[1316];
char buffer[SRT_LIVE_MAX_PLSIZE];
ASSERT_EQ(srt_recvmsg(acpsock, buffer, sizeof buffer), 1316);

char pattern[4] = {1, 2, 3, 4};
Expand Down