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

Add Firefox support #1

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

Pehrsons
Copy link

See individual commits for more detailed comments.

Firefox includes ssrcs in its sdp, but will misbehave if three
m-sections include the same six ssrcs (3 media, 3 rtx) and 3
ssrc-groups.
This bypasses an issue where Firefox would block RTX packets because they
don't carry the mid header extension, while media packets do (translated
from rid).
@Pehrsons
Copy link
Author

This comes from Orphis/webrtc-sandbox#19.
@alvestrand could you take a look?

@fippo
Copy link
Contributor

fippo commented Jan 25, 2025

Replacing all this stringsoup by https://github.com/fippo/simulcast-playground/blob/gh-pages/split-merge.js (which has worked with Firefox for years) would be better

@Pehrsons
Copy link
Author

Ping @alvestrand

@jan-ivar
Copy link
Member

This is phrased as getting Firefox working, but all looks like improvements that are to spec to me (e.g. avoiding unnecessary ssrc conflicts from mid->rid cut'n'paste swizzling). cc @henbos do you agree?

@fippo
Copy link
Contributor

fippo commented Feb 21, 2025

@jan-ivar according to some theories the spec forbids a=ssrc lines with simulcast. I am not a fan of that theory and could not even find spec language but Firefox including both mid/rid and ssrc is overspecifying things.

Your tone is not helping.

@fippo
Copy link
Contributor

fippo commented Feb 22, 2025

This is moving from mid/rid towards ssrc which IIUC is not what the spec (for better or worse) desires?

It turns out this shows what I believed to be a very interesting stats bug:
image
I couldn't repro on my playground (which uses the same GUM constraints) but chatting with @Orphis I figured out that the resolution of 1600x896 can be written as (1280+320)x(... wait this did not add up)
The GUM constraints are:

video: Object { width: 1280, height: 720, frameRate: "30" }

but

document.querySelectorAll('video').forEach(v => console.log(v.id, v.videoWidth, v.videoHeight))

shows the same numbers so it can not be a stats bug. track.getSettings returns height: 896, width: 1600

@Pehrsons
Copy link
Author

This is moving from mid/rid towards ssrc which IIUC is not what the spec (for better or worse) desires?

We could also remove the ssrc lines but would then lose the rtx mapping, so should remove rtx as well.

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

Successfully merging this pull request may close these issues.

None yet

3 participants