-
Notifications
You must be signed in to change notification settings - Fork 18
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
RTCRtpSender.getCapabilities() may not return correct information in sync #49
Comments
IMO this API should have been made asynchronous in the first place; codec capabilities means querying HW which means "not synchronous". While you could still implement this with blocking-invokes I am not in favor of that. Besides, since this reveals information about the system, having it be promise-based would make sense for privacy reasons as well in case a browser wants to implement a prompt (different discussion but worth bringing up if we address this at the same time). Suggestion:
|
@aboba you might remember the original intention to make it sync in ORTC... |
@fippo. We made it a synchronous static method because:
|
@aboba thank you! The problem probably boils down to chrome on android not supporting h264 software encoding as a fallback. https://bugs.chromium.org/p/chromium/issues/detail?id=719023 -- Hotlist-Recharge-Cold is another way of saying "this is gonna bite", right? |
The reason the Chrome bug was an issue is that the process on Android to figure out whether or not a H.264 encoder is available takes time. It will return the same answer every time on the same hardware, but until WebRTC asks about it, it is not going to be known to the underlying system. So we have a short interval in which the reply to "do you support H.264 encode" is "I don't know yet". Depending on the answer resolving one way or the other is not a Good Thing. |
Since the same information (support for encode/decode) is needed when createOffer() is called, why not cache the answer then? That would allow subsequent getCapabilities() calls to return immediately, without querying the hardware again. |
Seems like a rabbit hole to me to leave it synchronous. If there's one bit where the information is not available immediately, there's usually another bit with the same problem just around the corner. @henbos' suggestions sound like a good compromise to me. |
@aboba Caching after this has already been obtained asynchronously by other APIs seems like a sensible optimization (and nothing would prevent you from returning a cached result in a promise immediately), but considering there is no requirement that the app invoke and await createOffer() prior to calling getCapabilities(), having this be synchronous doesn't make sense. I am not in favor of having one API throw an exception unless another API on a different interface is called first. |
@henbos, when you say "deprecate" If the latter, I'm still unclear what happens when the method is called with no synchronous answer available. If one of the others, is it Web compatible? |
Once we have an asynchronous alternative we can simply remove getCapabilities() as far as the spec is concerned unless we want to document its behavior in legacy interface extensions section. For backwards compatibility, until it is safe to remove it from implementations without breaking the web platform, its probably best to let implementations support getCapabilities() the way it is implemented today. I.e. add a deprecation warning but continue to return whatever the implementation is able to know synchronously. |
Alternatively we fix this in an extension spec and let getCapabilities() be whatever it is today in webrtc-pc |
I think @dontcallmedom was more asking for what you're going to return right now in the existing
|
OperationError with an error message indicating "call the async interface if you really want info now" would be my preference. |
Removing privacy-tracker. The privacy aspecs are covered under another issue. |
The privacy issue is tracked in w3c/webrtc-pc#2460, and this one can be moved to webrtc-extensions. |
The workaround for the timing issue is to call createOffer() (which is async), which can wait for the correct information to be gathered without blocking the main thread. Changing the API is an NV matter. |
Any solution to this specific problem would be an API change which is not for final CR at this point. Moving to webrtc-extensions. |
We seem to have another case where the sync nature of the interface causes some interesting issues: Slow initialization (?) of USB cameras. We know that the camera is there long before we know what the capabilities of the device is. |
See also w3c/webrtc-pc#2597 |
The result of the previous interim was to add this note: It says that the capabilities in getCapabilities() and in the SDP from createOffer()/createAnswer() need to be consistent. I think to really pinpoint when capabilities are discovered and exposed would require normative steps and is thus webrtc-extensions territory, but I don't think this needs to be tackled in the January interim. This may become a non-issue with media capabilities or if we add an async API. |
@alvestrand Is there interest in implementing an async version of |
@alvestrand It seems like the current direction is to add functionality to Media Capabilities rather than updating |
Makes sense to close it as we want to progressively remove this support (see #95) |
I agree. Let's focus on #95 for any new functionality, and keep getCapabilities as-is in the spec for backwards compatibility. |
Chrome has found a case where getCapabilities() is going to have issues with delivering correct capability information about codec availability synchronously.
The two immediately obvious workarounds are:
The latter would be preferable from an architectural viewpoint, but has some compatibility issues.
The text was updated successfully, but these errors were encountered: