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

On Fri, May 1, 2015 at 2:29 PM, Jan-Ivar Bruaroey <jib@mozilla.com> wrote:

>  On 5/1/15 4:00 PM, Peter Thatcher wrote:
>
>  I think making all the parameters of RtpParameters into attributes of
> RtpSender is a terrible idea.  Every time a change to parameters is made,
> it's basically telling the RtpSender "stop what you're currently sending,
> and send something different",
>
>
> This wont happen in my model, if you follow what I wrote.
>
>    For example, look at the RtpParameters in ORTC:
>
>  dictionary RTCRtpParameters {
>              DOMString                                 muxId = "";
>              sequence<
> ​ ​
> RTCRtpCodecParameters>           codecs;
>              sequence<RTCRtpHeaderExtensionParameters> headerExtensions;
>              sequence<RTCRtpEncodingParameters>        encodings;
>              RTCRtcpParameters                         rtcp;
> };
>
>  ​ If the JS wants to change both the codecs and headerExtensions, it
> doesn't want to do:
>
>   sender.parameters.codecs = newCodecs;
>  sender.parameters.headerExtensions = newHeaderExtensions;
>
>
> First off, I'm proposing:
>
>  sender.codecs = newCodecs;
>  sender.headerExtensions = newHeaderExtensions;
>
> neater.
>
>    ​ Because in between those two lines, you'll end up in a weird state
> where you are sending the new codecs but the old header extensions.
>
>
> No, not in the model I'm proposing. Internally:
>
> RtpSender.codecsSetter would set this.codec = arg, and call
> this._updateNeeded().
> RtpSender.headxSetter would set this.headx = arg, and call
> this._updateNeeded().
>
> where this._updateNeeded = function() {
>     this._uNRequested || Promise.resolve(() => (this._uNRequested = false,
> this._update()));
>     this._uNRequested = true;
> };
>
> where this._update() = function() {
>   this._actualCodec = this._codec;
>   this._actualHeadx = this._headx;
>  // etc.
>   this._makeItSo(); // where this._actualCodec and this._actualHeadx
> matter.
> };
>

​That all seems so much more complex than just one method that means
"change what I'm sending".



>
>
>     And, worse, what if you setup something invalid on the header
> extensions, and it throws an exception?  Now you're doing half old and half
> new.  And as you expand it from 2 different settings like this to 5
> different things, it gets even more problematic.
>
>
> Provided the JS catches the exception then there is no problem, if it is
> uncaught then what you get need only be deterministic imho.
>
>     But the problems don't end ther​​
> e, because the ​objects are more than one level deep.  Imagine the JS does
> this:
>
>   var first = sender.parameters.codecs.splice(0, 1);
>  sender.parameters.codecs.push(first);
>
>
>  ​ Now there's an intermediate state where the codec is missing.  ​
> ​
> ​ And what if there were only one codec in the list to begin with?  Does
> the sender stop sending in the intermediate state?
>
>
> Not a problem in the model above.
>
>    And I could come up with a lot more examples.  The point is that there
> will be intermediate sender states for every little operation the JS does,
> and reasoning about those states is too complex.  It's much easier to have
> one atomic operation done via setParameters.​
>
>
> Not a problem in the model above.
>
> This is deterministic, and attributes reflect what's been set in the
> current task for values set in the current task, and actual values for
> values not set in the current task.
>
> A little more work on the implementation, but simpler for the JS writer.
>
> .: Jan-Ivar :.
>
>