<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - WebRTC: Add RTCRtpTransceiver interface and RTCPeerConnection.addTransceiver()"
href="https://bugs.webkit.org/show_bug.cgi?id=158189#c7">Comment # 7</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - WebRTC: Add RTCRtpTransceiver interface and RTCPeerConnection.addTransceiver()"
href="https://bugs.webkit.org/show_bug.cgi?id=158189">bug 158189</a>
from <span class="vcard"><a class="email" href="mailto:darin@apple.com" title="Darin Adler <darin@apple.com>"> <span class="fn">Darin Adler</span></a>
</span></b>
<pre>Comment on <span class="bz_obsolete"><a href="attachment.cgi?id=280093&action=diff" name="attach_280093" title="Proposed patch">attachment 280093</a> <a href="attachment.cgi?id=280093&action=edit" title="Proposed patch">[details]</a></span>
Proposed patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=280093&action=review">https://bugs.webkit.org/attachment.cgi?id=280093&action=review</a>
Noticed a few more things while re-looking at this that we could refine later.
<span class="quote">>>> Source/WebCore/Modules/mediastream/RTCPeerConnection.idl:69
>>> + [StrictTypeChecking, RaisesException] RTCRtpTransceiver addTransceiver(DOMString kind, optional Dictionary init);
>>
>> New code should never use Dictionary. These dictionaries should defined.
>>
>> dictionary RTCRtpTransceiverInit {
>> boolean send = true;
>> boolean receive = true;
>> sequence<MediaStream> streams;
>> sequence<RTCRtpEncodingParameters> sendEncodings;
>> };
>>
>> If the bindings generator can’t handle this dictionary, we should fix it so it can handle it. Every single time we instead use the deprecated Dictionary, we add code we will need to later delete!
>
> Using dictionary from latest published editor's draft [1]. Not all members are supported yet. Should I add a FIXME+bug# for that?
>
> [1] <a href="https://w3c.github.io/webrtc-pc/archives/20160513/webrtc.html#idl-def-rtcrtptransceiverinit">https://w3c.github.io/webrtc-pc/archives/20160513/webrtc.html#idl-def-rtcrtptransceiverinit</a></span >
I don’t think we need a FIXME about each piece that is missing as we implement something new like this. Eventually it might have some value. but during development it’s probably OK to just do things a bit at a time.
<span class="quote">> Source/WebCore/Modules/mediastream/RTCRtpSender.cpp:58
> + if (track)
> + setTrack(WTFMove(track));</span >
Not sure the if here is valuable.
<span class="quote">> Source/WebCore/Modules/mediastream/RTCRtpSenderReceiverBase.h:47
> virtual ~RTCRtpSenderReceiverBase() { }</span >
It’s probably more elegant to write "= default" here instead of writing out an empty body.
<span class="quote">> Source/WebCore/Modules/mediastream/RTCRtpSenderReceiverBase.h:53
> + RTCRtpSenderReceiverBase()
> + { }</span >
It’s probably more elegant to write "= default" here instead of writing out an empty body.
<span class="quote">>>> Source/WebCore/Modules/mediastream/RTCRtpTransceiver.cpp:98
>>> +}
>>
>> Since bindings create functions like this for all enumerations, we should avoid writing this code ourselves if possible. What makes this necessary?
>
> Removed parseDirectionString() as part of moving to properly defined dictionary in idl. directionString() is used when the direction value is added to a PeerMediaDescription (which represents direction as a string). I guess there's no nice way to use the generated version from the binding?</span >
Yes, at the moment there is no way to convert an enumeration into the string from the bindings, but if this pattern keeps recurring, I suppose we can add a way.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>