Re: PRs for either PC.createRtpSender or PC.addTrack(..., RtpSenderInit, ...)

Having reviewed both PRs, I agree with Adam and Peter. createRtpSender is
simple and intuitive.

On Tue, Aug 25, 2015 at 10:20 AM, Peter Thatcher <pthatcher@google.com>
wrote:

> On Tue, Aug 25, 2015 at 12:28 AM, Adam Bergkvist <
> adam.bergkvist@ericsson.com> wrote:
>
>> On 2015-08-22 08:36, Peter Thatcher wrote:
>> > On the list we have had discussion around adding an RtpSenderInit
>> > parameter to PC.addTrack, and around adding PC.createRtpSender.  Today I
>> > realized that RtpSenderInit could also be used the same way
>> > PC.createRtpSender is.
>> >
>> > So, I have two PRs to choose from:
>> >
>> > PR 271 (https://github.com/w3c/webrtc-pc/pull/271):
>> >
>> > +          <dt>RTCRtpSender createRtpSender (DOMString kind)</dt>
>> > +
>> > +          <dd>
>> > +            <p>Equivalent to calling <code>addTrack(null)<code>, but
>> > +            the returned RtpSender is configured to send a track of
>> > +            the given <code>kind</code>.  If the returned RtpSender is
>> > +            later set to have a different track, that track must have
>> > +            the same kind.
>> > +          </dd>
>> >
>> > Example:
>> >
>> > var sender = pc.createRtpSender("audio");
>> > // ... later ...
>> > sender.setTrack(track);
>> >
>> > Pros:
>> > - Simple
>> >
>> > Cons:
>> > - Adds a new method (two ways to do things)
>> >
>> > PR 272 (https://github.com/w3c/webrtc-pc/pull/272):
>> >
>> > +          <dt>RTCRtpSender addTrack (
>> > +            MediaStreamTrack track,
>> > +            optional RTCRtpSenderInit senderDict,
>> > +            MediaStream... streams)</dt>
>> >
>> > +        <dl class="idl" title="dictionary RTCRtpSenderInit">
>> > +          <dt>DOMString kind</dt>
>> > +          <dd>
>> > +            <p>The kind of the track that the <code>RtpSender</code>
>> > +              will send.  If <code>addTrack</code> is passed a null
>> track,
>> > +              this must be set to indicate the type of track that will
>> be
>> > +              set later.  If <code>addTrack</code> is passed a non-null
>> > +              track, this kind must either be unset or agree with the
>> kind
>> > +              of the given track.</p>
>> > +          </dd>
>> > +
>> > +          <dt>RTCRtpParameters parameters<dt>
>> > +            <dd>
>> > +              <p>The initial parameters to use for
>> > +                the <code>RtpSender</code>.  This is equivalent to
>> > +                calling <code>RTCRtpSender.setParameters</code> on the
>> > +                RTCRtpSender returned by <code>addTrack</code>.
>> > +              </p>
>> > +            </dd>
>> > +        </dl>
>> >
>> > Example:
>> >
>> > var sender = pc.addTrack(null, {kind: "audio"});
>> > // ... later ...
>> > sender.setTrack(track);
>> >
>> > Pros:
>> > - RtpSenderInit is useful for other things (Could be used for VAD or
>> > MSID replacement)
>> >
>> > Cons:
>> > - A little more complex
>> >
>> >
>> > So, which do we like?  I'd be happy with either.
>>
>> Hi
>>
>> I think #271 is more straight forward. It's a simple addition to the
>> API. It's not possible to provide any stream here, but perhaps that's ok.
>>
>> The approach in #272, with an "injected" argument, affects the most
>> common usage of addTrack(), no options - just a track and a stream, and
>> breaks a lot of existing code.
>>
>> pc.addTrack(track, null, stream);
>> or
>> pc.addTrack(track, {}, stream);
>>
>> And in the majority of cases, the "kind" member is redundant since the
>> track has a kind. Also, what does it mean if its called with a
>> null-track and actual streams?
>>
>
> ​Having thought about this so more after having written the PRs, I agree.
> I also prefer #271 (createRtpSender).​
>
>
>
>>
>> I agree that the options dictionary (in #272) could be useful for other
>> things, but I don't think it's a big loss since we have the RTCRtpSender
>> object as an API surface.
>
> ​
>>
>
>
>> If people don't want the extra API surface and the
>> two-ways-of-doing-thinsg of createSender(), we could go with something
>> like:
>>
>> RTCRtpSender addTrack ((MediaStreamTrack or DOMString) trackOrKind,
>>                         MediaStream... streams);
>>
>> That doesn't affect the common usage and creating an "empty" sender
>> would be:
>>
>> pc.addTrack("audio");
>>
>> The most common case would still look the same:
>>
>> pc.adDTrack(track, stream);
>>
>
> ​addTrack("audio") was one of the first ideas suggested on the list
> before, and if I recall correctly, it was unpopular.​
>
>
>
>>
>>
>> /Adam
>>
>
>