libmediasoupclient icon indicating copy to clipboard operation
libmediasoupclient copied to clipboard

vp8 and h264 only support L1Tx, but c++ code

Open runner365 opened this issue 1 year ago • 3 comments

Hi, I find the C++ code:

	SendHandler::SendResult SendHandler::Send(
           ...
	  std::vector<webrtc::RtpEncodingParameters>* encodings,
	 ....)
	{
              ....
		if (
			sendingRtpParameters["encodings"].size() > 1 &&
			(mimeType == "video/vp8" || mimeType == "video/h264")
		)
		{
			for (auto& encoding : sendingRtpParameters["encodings"])
			{
				encoding["scalabilityMode"] = "S1T3";
			}
		}
         }

vp8 and h264 have been forced to set "S1T3" in scalabilityMode.

but js code:

			for (const encoding of encodings)
			{
				encoding.rid = `r${nextRid++}`;
				encoding.scalabilityMode = `L1T${maxTemporalLayers}`;
			}

And I check the webrtc js site: https://w3c.github.io/webrtc-svc/#scalabilitymodes*

For example, VP8 [[RFC6386](https://w3c.github.io/webrtc-svc/#bib-rfc6386)] only supports temporal scalability (e.g. ["L1T2"](https://w3c.github.io/webrtc-svc/#dfn-l1t2), ["L1T3"](https://w3c.github.io/webrtc-svc/#dfn-l1t3)); H.264/SVC [[RFC6190](https://w3c.github.io/webrtc-svc/#bib-rfc6190)], which supports both temporal and spatial scalability, only permits transport of simulcast on distinct SSRCs, so that it does not support "S" modes, where multiple encodings are transported on a single RTP stream

It says the H264 doesn't support "S" modes. Is it a bug in libmediasoupclient c++ code?

runner365 avatar Apr 01 '24 03:04 runner365

Not really a bug. This was done for a previous version of libwebrtc where scalabilityMode wasn't available. Now we are using M110 so the code in libmediasoupclient's Handler should match what we do in Chrome111 handler in mediasoup-client.

CC @jmillan to confirm.

ibc avatar Apr 03 '24 09:04 ibc

Yes, we need to finish the update to M110, which is still not done due to the issues commented here https://github.com/versatica/libmediasoupclient/pull/173

jmillan avatar Apr 03 '24 09:04 jmillan

Oh, and it's not M110 but M120.

ibc avatar Apr 03 '24 09:04 ibc