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

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
>