"invalid TMMB bitrate value"

[ibc]

In master branch:

RTC::RTCP::FeedbackRtpTmmb::FeedbackRtpTmmbItem() | invalid TMMB bitrate value : 43461 *2^63

[jmillan]

The value may indeed be invalid :-). I'll check it.

[ibc]

NOTE: I also see this:

RTC::Peer::onTransportRtcpPacket() | ignoring unsupported Feedback packet 'TMMBN' [sender_ssrc:0, media_ssrc:0]

media_ssrc:0?

[jmillan]

It's Okey, the useful info is in the packet items, which contains the SSRCs they refer to, each. This packet type to be properly handled (forwarded) as commented this morning.

[ibc]

opps, ok ok, sorry.

[ibc]

I don't think this is ok:

	template <typename T>
	FeedbackRtpTmmbItem<T>::FeedbackRtpTmmbItem(const uint8_t* data)
	{

		this->ssrc = Utils::Byte::Get4Bytes(data, 0);

		// Read the 4 bytes block.
		uint32_t compact = Utils::Byte::Get4Bytes(data, 4);

		// Read each component.
		uint8_t exponent = compact >> 26;              // 6 bits.
		uint64_t mantissa = (compact >> 9) & 0x1ffff;  // 17 bits.
		this-> overhead = compact & 0x1ff;             // 9 bits.

Here we are reading bytes from 5 to 8 as a uint32_t number, and then split it into components. Is that right?

[ibc]

What mostly worries me (but I'd may be 100% wrong) is this:

template <typename T>
FeedbackRtpTmmbItem<T>::FeedbackRtpTmmbItem(const uint8_t* data)
{
	this->ssrc = Utils::Byte::Get4Bytes(data, 0);

	// Read the 4 bytes block.
	uint32_t compact = Utils::Byte::Get4Bytes(data, 4);

	// Read each component.
	uint8_t exponent = compact >> 26;              // 6 bits.
	uint64_t mantissa = (compact >> 9) & 0x1ffff;  // 17 bits.
	this-> overhead = compact & 0x1ff;             // 9 bits.

	// Get the bitrate out of exponent and mantissa.
	this->bitrate = (mantissa << exponent);

	if ((this->bitrate >> exponent) != mantissa)
	{
		MS_WARN_TAG(rtcp, "invalid TMMB bitrate value : %" PRIu64" *2^%" PRIu8, mantissa, exponent);

		this->isCorrect = false;
	}
}

I mean: that seems to be a pure mathematical assertion, how can it be wrong?

[jmillan]

That check it to prevent shift overflows. Ie:

RTC::RTCP::FeedbackRtpTmmb::FeedbackRtpTmmbItem() | invalid TMMB bitrate value : 43461 *2^63

mantissa = 43461
exponent = 63

bitrate = mantissa << exponent = 9223372036854775807 (in a uint64_t)
bitrate >> exponent != bitrate

[ibc]

Closing due to same reasons as those in #93