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 send should return a promise #469

Closed
murillo128 opened this Issue Apr 15, 2016 · 5 comments

Comments

Projects
None yet
4 participants
@murillo128

murillo128 commented Apr 15, 2016

Not sure if it is duplicated, but just in cae.

Currently, RTCRtpSender.send() returns void and error is launched if

This event must be fired if an issue is found with the RTCRtpParameters object passed to send(), that is not immediately detected.

According to the promise best practices (https://www.w3.org/2001/tag/doc/promises-guide), about when to use promises this should be changed to a promise instead of an error handler

Because promises can be subscribed to even after they’ve already been fulfilled or rejected, they can be very useful for a certain class of "event." When something only happens once, and authors often want to observe the status of it after it’s .already occurred, providing a promise that becomes fulfilled when that eventuality comes to pass gives a very convenient API.

The prototypical example of such an "event" is a loaded indicator: a resource such as an image, font, or even document, could provide a loaded property that is a promise that becomes fulfilled only when the resource has fully loaded (or becomes rejected if there’s an error loading the resource). Then, authors can always queue up actions to be executed once the resource is ready by doing resource.loaded.then(onLoaded, onFailure). This will work even if the resource was loaded already, queueing a microtask to execute onLoaded. This is in contrast to an event model, where if the author is not subscribed at the time the event fires, that information is lost.

@robin-raymond

This comment has been minimized.

Show comment
Hide comment
@robin-raymond

robin-raymond Apr 15, 2016

Contributor

$related #399

Contributor

robin-raymond commented Apr 15, 2016

$related #399

@murillo128

This comment has been minimized.

Show comment
Hide comment
@murillo128

murillo128 Apr 15, 2016

Could we return the browser filled RTCRtpParameters on success?? ;)

Promise<RTCRtpParameters> send(RTCRtpParameters)

murillo128 commented Apr 15, 2016

Could we return the browser filled RTCRtpParameters on success?? ;)

Promise<RTCRtpParameters> send(RTCRtpParameters)
@robin-raymond

This comment has been minimized.

Show comment
Hide comment
@robin-raymond

robin-raymond Apr 15, 2016

Contributor

@murillo128 ;) not in this bug -- but nice try! I know that was a semi-serious-joking but let's keep the issues separate.

FYI - I am personally in favour of it, but I would prefer a RTCRtpSender.getParameters() because I'm also in favour of returning the filled parameters for a receiver with a RTCRtpReceiver.getParameters(); the receiver can't be done as a promise since it's filled in an "ongoing" basis so I'd like them somewhat consistent for the future if we did that later. Alas, I've not been able to convince others in the CG that is has enough value to merit is mandatory inclusion in the API and I've made the same arguments as you and @ibc (I believe).

UPDATE: To be clear, a method like RTCRtpSender.getParameters() would require a promise result from send() to know when the developer can fetch the parameters and expect to get the resulting values.

Contributor

robin-raymond commented Apr 15, 2016

@murillo128 ;) not in this bug -- but nice try! I know that was a semi-serious-joking but let's keep the issues separate.

FYI - I am personally in favour of it, but I would prefer a RTCRtpSender.getParameters() because I'm also in favour of returning the filled parameters for a receiver with a RTCRtpReceiver.getParameters(); the receiver can't be done as a promise since it's filled in an "ongoing" basis so I'd like them somewhat consistent for the future if we did that later. Alas, I've not been able to convince others in the CG that is has enough value to merit is mandatory inclusion in the API and I've made the same arguments as you and @ibc (I believe).

UPDATE: To be clear, a method like RTCRtpSender.getParameters() would require a promise result from send() to know when the developer can fetch the parameters and expect to get the resulting values.

@ibc

This comment has been minimized.

Show comment
Hide comment
@ibc

ibc Apr 16, 2016

Contributor

Could we return the browser filled RTCRtpParameters on success?? ;)

I don't want to restart the trending topic :) but getting the full parameters once send() is called is of little use. That's why I suggested:

rtpSender.setParameters(params);

var fullParams = rtpSender.getParameters();

// Signal them if needed

rtpSender.send();

Don't reply please! :)

Contributor

ibc commented Apr 16, 2016

Could we return the browser filled RTCRtpParameters on success?? ;)

I don't want to restart the trending topic :) but getting the full parameters once send() is called is of little use. That's why I suggested:

rtpSender.setParameters(params);

var fullParams = rtpSender.getParameters();

// Signal them if needed

rtpSender.send();

Don't reply please! :)

@aboba aboba added PR exists and removed publication-blocker labels Apr 25, 2016

aboba added a commit that referenced this issue Apr 25, 2016

aboba added a commit that referenced this issue Apr 26, 2016

Examples: async usage of send(), receive() and setTransport()
Fix for Issues: 
#399
#469

More work to do (please do not merge yet).
@robin-raymond

This comment has been minimized.

Show comment
Hide comment
@robin-raymond

robin-raymond Apr 28, 2016

Contributor

Looks better.

Contributor

robin-raymond commented Apr 28, 2016

Looks better.

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