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

sdp-utils: add AV1 fmtp parsing #3485

Open
wants to merge 1 commit into
base: 0.x
Choose a base branch
from

Conversation

tmatth
Copy link
Contributor

@tmatth tmatth commented Dec 7, 2024

This matches what is already done for VP9 and H264.

@tmatth tmatth changed the base branch from master to 0.x December 7, 2024 01:14
@lminiero
Copy link
Member

lminiero commented Dec 9, 2024

Are browsers negotiating profiles for AV1 too, now? What are the options and what impact does it have?

@lminiero
Copy link
Member

lminiero commented Dec 9, 2024

Found the new parameters documented here: https://aomediacodec.github.io/av1-rtp-spec/#72-sdp-parameters
Is keeping track of the profile enough? In H.264 the profile contains info on more things, and in VP9 it's just a different flavour of the codec, while in AV1 apparently the H.264 profile equivalent is split across multiple properties (e.g., profile + level-idx).

@tmatth
Copy link
Contributor Author

tmatth commented Dec 9, 2024

Is keeping track of the profile enough? In H.264 the profile contains info on more things, and in VP9 it's just a different flavour of the codec, while in AV1 apparently the H.264 profile equivalent is split across multiple properties (e.g., profile + level-idx).

Yeah right now I'm seeing level-idx, profile and tier, both e.g.,

a=fmtp:96 profile=0;level-idx=8;tier=0
a=fmtp:96 level-idx=8; profile=0; tier=0

I can add parsing for those as well.

@tmatth tmatth force-pushed the av1-profile-parsing branch 3 times, most recently from 0fd40e6 to 123fc36 Compare December 10, 2024 19:44
@tmatth
Copy link
Contributor Author

tmatth commented Dec 10, 2024

Also the code is simpler than I expected since the spec mentions that if the fields aren't present they must default to profile=0, level_idx=5 and tier=0 respectively.

@tmatth tmatth force-pushed the av1-profile-parsing branch from 123fc36 to 0111eed Compare December 10, 2024 22:06
@lminiero
Copy link
Member

That helps with generating a valid fmtp, but I suspect would not be enough, especially since when generating answers, providing a profile (and maybe more) acts as a way to filter/find the specific payload type we want to access. We currently use the VP9/H.264 profile as a parameter to add to this request, for instance

int pt = janus_sdp_get_codec_pt_full(offer, codec, video_profile);

where we look for the payload type associated with that codec and that specific profile. Your patch adds AV1 as a potential profile for that filtering, but since the profile on its own may not be enough, this could result in the wrong payload type being selected if there's multiple AV1 fmtp lines with the same profile but different tiers/layers.

Not sure how this should be addressed: we could extend the signature of janus_sdp_get_codec_pt_full to have other optional properties that only make sense for AV1, but 1. that would be a breaking change (sdp-utiils are available to plugins too), 2. would cause a precedent (what if a new codec comes up with properties of its own? do we extend this forever?), and 3. would probably overcomplicate the filtering process (especially where some of the fmtp properties may be omitted in the offer).

I think that for now we can live with just using the profile for filtering. If it turns out it causes the problems that I mentioned (I haven't checked how many options browsers give for AV1) we can revisit this. I guess we'll also need to ensure this doesn't break AV1 for browsers that don't use profiles: e.g., what if Chrome adds one, but Firefox doesn't?

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.

2 participants