Re: Attributes! (Re: Proposal: Add RtpSenderInit parameter to addTrack.)

I'll repeat some text I wrote previously about replaceTrack:


In the work in the ORTC CG, we chose to follow the .setFoo pattern rather
than the .foo= for all things, not just setTrack for the following reasons:

1.  It allows return values, async and sync.  You might think you don't
want one now, but you might want to add one later.
2.  It allows async errors.
3.  It allows for adding additional parameters.  You might think you don't
want one now, but you might want to add one later.


Specifically, in your example:

    var fooParameters = foo.getParameters();
    fooParameters.bar = "big";
    fooParameters.fum = " deal";
    foo.setParameters(fooParameters)

That's exactly the situation we want to encourage, because it allows and
encourages the setting of parameters to be atomic.  Doing something like
this is madness:

foo.parameters.bar = "big";
foo.parameters.fum = " deal";

Because then you have all this half-set stuff and you have to reason about
what those things mean when half-set.



Finally, I don't think it makes sense to have RtpParameters and
RtpSenderInit be the same class because there are certain arguments which
can change (stuff in RtpParameters) and certain arguments which can't
change (label).  If we put label, for example, into RtpParameters, that
would imply that it can change via setParameters.  On the other hand, if we
either want other things in RtpParameters to be unchangable or if we have
no things we want to have be unchangable (we remove the ability to set the
label, for example), then RtpParameters could be the same class as
RtpSenderInit.






On Fri, May 1, 2015 at 9:11 AM, Jan-Ivar Bruaroey <jib@mozilla.com> wrote:

>  Agree we need this stuff, and I love dictionaries, but IMHO this web API
> surface is turning into a mess.
>
> The ideal surface for a parameter is: foo.bar
>
> foo.setBar() / foo.getBar() is some kind of c++ sickness.
>
> foo.setParameters({..., bar: }) / foo.getParameters().bar is a kick in the
> mouth to JavaScript.
>
> Synchronization? JS is entirely event-driven, so:
>
>     var foo = {};
>     Promise.resolve(() => console.log(foo.bar + foo.fum); // "big deal"
>     foo.bar = "big";
>     foo.fum = " deal";
>
> WebIDL attributes are powerful, and existing idioms - like Events - go to
> great lengths to use them (as should we): e.g. var bar = new FooEvent({
> bar: 42 }).bar;
>
> I love promises too. Love them. But the following is a kick in the mouth
> to ... I don't know, mouths:
>
>     var fooParameters = foo.getParameters();
>     fooParameters.bar = "big";
>     fooParameters.fum = " deal";
>     foo.setParameters(fooParameters).then(() => console.log("all set!"));
>
> We should not let implementation dictate API, and  We should need
> well-understood and explicit reasons before resorting to something this
> deplorable (see constraints).
>
> Comments inline.
>
> On 4/30/15 8:53 PM, Cullen Jennings (fluffy) wrote:
>
> Looks reasonable to me.
>
> I think it needs to get just a bit more complicated so there is a way to
> add new members to the RtpParameters dictionary in the future but still
> have a way for applications to know which members are understood by the
> browser they are running it.
>
>
> if (foo.bar !== undefined) // attribute supported by browser
>
>  On 4/21/15 7:06 PM, Peter Thatcher wrote:
>
>  While talking to Stefan about the possibility of per-RtpSender control
> of VAD, and thinking about all the recent discussion around MSID, I
> realized that there was a common need to add parameters to addTrack.
>
>  I propose we add a dictionary parameter "RtpSenderInit", like we have
> DataChannelInit that contains parameters for
> PeerConnection.createDataChannel without making the parameter list a mess.
>
>
> +1
>
>   IDL that would retain current semantics could look like this:
>
>
>  partial interface PeerConnection {
>     RtpSender addTrack(MediaStreamTrack track, RTCRtpSenderInit init);
> }
>
>  dictionary RTCRtpSenderInit {
>    sequence<MediaStream> streams;
>  }
>
>
> AFAIK a key motivation behind the InitDict pattern is to support cloning
> like this:
>
>     var foo = new Foo({ bar: 42 });
>     var foo2 = new Foo(foo);
>     console.log(foo2.bar); // 42
>
> Working backwards from that suggests RtpSender attributes and InitDict
> members should line up.
>
>   A more advanced form, if we choose to add more parameters later, could
> look like this:
>
> dictionary RTCRtpSenderInit {
>   // These are based on our discussion of MSID.
>   DOMString label;
>    sequence<DOMString> synchronizationGroups;
>
>    // The initial RtpParameters to use, as if you call .setParameters
> immediately after
>   // calling addTrack.
>   RtpParameters parameters;
> }
>
>
> Why do we need two classes of properties (RtpParameters vs. other
> RtpSender arguments)? What would dictate where a new feature goes?
>
>   ​dictionary RtpParameters {
>   // This is based my discussion with Stefan.
>   boolean voiceActivityDetection;
>  }​
>
>
>
>  Let's continue the discussion around voiceActivityDetection,
> synchronizationGroups, and label/ID on separate threads, but for this
> thread, I'd like to ask the group:  Does it look like a good idea to follow
> the createDataChannel(..., DataChannelInit) pattern by having addTrack(...,
> RtpSenderInit)?
>
>
>  I'm in favor of it because:
> 1.  We'd avoid with a mess of parameters in addTrack over time.
> 2.  It gives the app to specify initial RtpParameters (such as VAD,
> potentially).
> 3.  It follows the same pattern we already have for createDataChannel,
> which has been successful in adding many options without making the
> parameter list a mess.
> 4.  It gives us more flexibility in our work around IDs and labels.
>
>  Thanks,
> Peter
>
>
> We should look at imageCapture [1] which seems to have iterated on this
> subject. I was initially going to call it out here as a bad example, except
> it appears to have improved since last I looked.
>
> .: Jan-Ivar :.
>
> [1] gmandyam.github.io/image-capture
>
>