Skip to content
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.

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

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

Open
runner365 opened this issue Apr 1, 2024 · 3 comments
Open

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

runner365 opened this issue Apr 1, 2024 · 3 comments

Comments

@runner365
Copy link

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?

@ibc
Copy link
Member

ibc commented Apr 3, 2024

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.

@jmillan
Copy link
Member

jmillan commented Apr 3, 2024

Yes, we need to finish the update to M110, which is still not done due to the issues commented here #173

@ibc
Copy link
Member

ibc commented Apr 3, 2024

Oh, and it's not M110 but M120.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants