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 a new webrtcCodec parameter to MediaCapabilitiesInfo #186

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

Conversation

youennf
Copy link

@youennf youennf commented Nov 19, 2021

Fixes #185


Preview | Diff

@chcunningham
Copy link
Contributor

Generally looks good. See questions in #185 (posting there to increase visibility).

<li>
If the user agent is able to encode the media represented by
<var>configuration</var> and <var>configuration</var> has a type
of {{MediaEncodingType/webrtc}}, run the following substeps:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
of {{MediaEncodingType/webrtc}}, run the following substeps:
of {{MediaEncodingType/webrtc}}:

@@ -867,6 +880,25 @@ spec: secure-contexts; urlPrefix: https://www.w3.org/TR/secure-contexts/
source has side effects such as enabling different encoding
modules.
</li>
<li>
If the user agent is able to encode the media represented by
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If the user agent is able to encode the media represented by
If the user agent can encode the media represented by

@@ -867,6 +880,25 @@ spec: secure-contexts; urlPrefix: https://www.w3.org/TR/secure-contexts/
source has side effects such as enabling different encoding
modules.
</li>
<li>
If the user agent is able to encode the media represented by
<var>configuration</var> and <var>configuration</var> has a type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we link "type" explicitly to a config member?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the "valid audio MIME type" and "valid video MIME type" from section 2.1.4 are probably good targets.

<ol>
<li>Let <var>webrtc</var> be a
{{WebRTCMediaCapabilitiesInfo}} dictionary.</li>
<li>If audio is represented by <var>configuration</var>, set
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, can we link to something that explicitly determines "audio" and "video" types are being represented?

<li>
If the user agent is able to decode the media represented by
<var>configuration</var> and <var>configuration</var> has a type
of {{MediaDecodingType/webrtc}}, run the following substeps:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
of {{MediaDecodingType/webrtc}}, run the following substeps:
of {{MediaDecodingType/webrtc}}:

<ol>
<li>Let <var>webrtc</var> be a
{{WebRTCMediaCapabilitiesInfo}} dictionary.</li>
<li>If audio is represented by <var>configuration</var>, set
Copy link
Member

@marcoscaceres marcoscaceres Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments as above about determining audio and video is being represented the configuration.

<var>configuration</var> and <var>configuration</var> has a type
of {{MediaEncodingType/webrtc}}, run the following substeps:
<ol>
<li>Let <var>webrtc</var> be a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

Suggested change
<li>Let <var>webrtc</var> be a
<li>Let |webrtc| be a

Copy link
Member

@marcoscaceres marcoscaceres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of suggestions...

@alvestrand
Copy link

alvestrand commented Jan 25, 2024

I think we need this, now that we're using codec descriptions more in APIs.
Ref #webrtc-encoded-transform issue 172 for one example of such usage.

@youennf
Copy link
Author

youennf commented Feb 20, 2024

Discussed at today's WebRTC interim and this would probably help moving WebRTC piggy backing on Media Capabilities to expose codecs (instead of exposing all/most codecs).

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.

Retrieving RTCRtpCodecCapability from MediaCapabilities when queried for webrtc
5 participants