[asterisk-dev] [Code Review] Asterisk WebRTC Wiki Page

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2080/#review6949
-----------------------------------------------------------


Let's go through this section-by-section.

Background:
* The second sentence reads oddly. Specifically, I don't know what the antecedent of "This" is at the beginning of the sentence.
* It may be worthwhile to list browsers that provide support for WebRTC. If keeping up with this list is going to be problematic, then I have to imagine that WebRTC keeps a list somewhere, so linking to that would be a good idea.
* Make the words ICE, TURN, and STUN link to their respective RFCs.

SRTP:
I agree with lathama that there should be a link to the secure calling tutorial here. Even though that page focuses on TLS more than SRTP, it is valuable for learning how to set up certificates, etc. for use with WSS transport.

Configuring res_http_websocket:
The text in the yellow box is awkward. You can simplify it down to simply "Ensure that res_http_websocket.so is selected in menuconfig prior to building Asterisk. Also ensure that res_http_websocket.so is loaded prior to chan_sip.so if you are not using autoload in modules.conf" . Non-specific pronouns and run-on sentences are the bane of technical documents.

Configuring chan_sip:
* While it may seem obvious to us, "sip.conf" does not appear anywhere in this section. Make sure that it is clear that these options apply to this file.
* The yellow box has an incomplete sentence at the end of the first paragraph: "You can also set"
* Does the registration bug occur on inbound or outbound registrations?
* Provide a link to the JIRA issue corresponding to the registration bug. That way people can determine if the reported issue is the same as what they are seeing, and they can find out in what revision the issue was fixed.
* Everything after the first paragraph in the yellow box seems out of place by being in the yellow box. The yellow box appears to be intended to warn of the known defect regarding registration, but then it goes on to talk about encryption and nat settings. Everything after the warning about the registration defect should be outside the yellow box. Preferably, the items moved outside will appear before the box.

Using WebSocket:
* This doesn't really go into much detail about actually using WebSocket. It just tells about how some JavaScript libraries may need to be modified to use the proper directory.
* One thing that's not clear is whether the use of /ws as the only location for WebSocket is due to us following some specification that certain JavaScript libraries do not follow or whether the use of /ws is a temporary limitation of Asterisk's WebSocket support. Basically, is the burden on Asterisk developers to support more locations or is it the duty of the JavaScript developers to follow specifications properly?

JavaScript Libraries:
What modifications are required to make sipml5 work with Asterisk? Is it the modification you described above in the Using WebSocket section? If so, why not provide a patch or some guidelines on what in sipml5 needs to be changed.

Issues:
Are these messages that Asterisk will send or receive? Are these SIP or HTTP responses? If SIP, do they occur for all SIP methods or specific ones?

- Mark


On Aug. 22, 2012, 6:34 a.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2080/
> -----------------------------------------------------------
> 
> (Updated Aug. 22, 2012, 6:34 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> I've created a wiki page at https://wiki.asterisk.org/wiki/display/~jcolp/Asterisk+WebRTC+Support which describes the WebRTC a bit, along with the configuration items and potential issues that may occur. Any feedback is welcome!
> 
> 
> Diffs
> -----
> 
> 
> Diff: https://reviewboard.asterisk.org/r/2080/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joshua
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120822/8861992b/attachment.htm>