Fix old buffer items cleanup in RTC::RateCalculator

[dsdolzhenko]

In the PR, besides fixing the bug described in #818, I also replaced the if condition with an assert as the code under it is not supposed to be executed anyway. Moreover, the assertion allows to catch the bug by running the unit test for RTC::RateCalculator

[ibc]

@jmillan ?

[gnauhtt]

Something wrong in this PR, will stuck in an infinite loop when 'this->oldestItemStartTime == newOldestTime'.

while (this->oldestItemStartTime <= newOldestTime)
{
	BufferItem& oldestItem = this->buffer[this->oldestItemIndex];
	this->totalCount -= oldestItem.count;
	oldestItem.count = 0u;
	oldestItem.time  = 0u;

	if (++this->oldestItemIndex >= this->windowItems)
		this->oldestItemIndex = 0;

	const BufferItem& newOldestItem = this->buffer[this->oldestItemIndex];
	this->oldestItemStartTime       = newOldestItem.time;
}

[ibc]

Can you detail where/why the infinite loop would happen?

[jmillan]

PR reverted until the issue is clarified.

[gnauhtt]

Create a PlainTransport to receive data from ffmpeg. At this point the plainTransport sends an rtcp packet every second. And 'sendTransmission' 's windowSizeMs are also one second. When the program runs to ‘RateCalculator::RemoveOldData()’, When 'this->oldestItemStartTime == newOldestTime' happens at the same time as 'just one BufferItem in the RateCalculator::buffer'. It will remove this last BufferItem, so 'this->oldestItemStartTime' will set to 0. So that (this->oldestItemStartTime <= newOldestTime) ==> (0 <= newOldestTime) always true.

[ibc]

Thanks. The commit has been reverted. In fact I found our demo server with the worker process in an infinite loop. Thanks.

[dsdolzhenko]

Oh. @rtctt thank you for reporting that. I'll prepare a new PR with proper fix and unit test later this week