{"action":"created","issue":{"url":"https://api.github.com/repos/fippo/adapter/issues/2","labels_url":"https://api.github.com/repos/fippo/adapter/issues/2/labels{/name}","comments_url":"https://api.github.com/repos/fippo/adapter/issues/2/comments","events_url":"https://api.github.com/repos/fippo/adapter/issues/2/events","html_url":"https://github.com/fippo/adapter/issues/2","id":111595924,"number":2,"title":"A few improvements on Edge support","user":{"login":"lminiero","id":3684796,"avatar_url":"https://avatars.githubusercontent.com/u/3684796?v=3","gravatar_id":"","url":"https://api.github.com/users/lminiero","html_url":"https://github.com/lminiero","followers_url":"https://api.github.com/users/lminiero/followers","following_url":"https://api.github.com/users/lminiero/following{/other_user}","gists_url":"https://api.github.com/users/lminiero/gists{/gist_id}","starred_url":"https://api.github.com/users/lminiero/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lminiero/subscriptions","organizations_url":"https://api.github.com/users/lminiero/orgs","repos_url":"https://api.github.com/users/lminiero/repos","events_url":"https://api.github.com/users/lminiero/events{/privacy}","received_events_url":"https://api.github.com/users/lminiero/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-10-15T11:17:18Z","updated_at":"2015-10-21T03:37:34Z","closed_at":null,"body":"Hi,\r\n\r\nas anticipated on Twitter, I've been playing a bit with Edge support in order to get it working with Janus. On the client side we mostly use a slightly older version of adapter.js, so it made sense to try and incorporate the code you wrote some time ago (bundle-edge.js) in there in order to expose Edge support too. It wasn't all roses, mostly because of the obscure error reporting on the Edge JavaScript developer console (which I guess is reasonable, considering the experimental nature of the new features), but eventually I mostly got it working.\r\n\r\nI know you said a lot has changed since that version I based on (I can see that on the code here, great job!), but I thought I'd share some of the things I found out as they may be helpful in improving the current version of the shim too.\r\n\r\nBesides some additions that are not really relevant, I had to fix a few things in the original code to get it working with Edge in my scenario, which as was always using Janus as the peer:\r\n\r\n1. I had to comment out the ```'use strict';``` part, as Edge was complaining with a \"variable undefined in strict mode\" when setting ```webrtcMinimumVersion```.\r\n2. Not sure if it's an issue with Edge or not, but whatever ```iceServers``` I passed I got an exception; some more details can be found [here](https://connect.microsoft.com/IE/feedbackdetail/view/1894505/rtcicegatherer-rtcicetransport-issues), on the Connect issue I opened; anyway, I had to force an empty array as ```iceServers``` as the only way to go on, thus skipping what you do [here](https://github.com/fippo/adapter/blob/shim-ortc/adapter.js#L927).\r\n3. Your original code only added candidates received via trickling, but ignored any candidate that may be available in the SDP, thus preventing support for half-trickle. At the moment, Janus always advertizes its own candidates in the SDP (while browsers themselves can trickle), so I added a very simple filter to also parse candidates in there. The way I did this is pretty crude, right now, and you can find it [here](https://github.com/meetecho/janus-gateway/blob/janus-edge/html/adapter.js#L869); the idea is to parse candidates, add them to an array, and then pass them one by one to ```addIceCandidate``` when needed (SLD or SRD). It's quite crude as it doesn't work well when remote candidates are both in the SDP and trickles, as the way I did it now always adds an ```endOfCandidates``` after the remote candidates parsed in the SDP, which is not what it should do, but it does the job in my case.\r\n4. I modified ```_getCommonCapabilities``` so that the name matching would be done on the ```toLowerCase``` version of the codec name, rather than the name itself. In fact, in my tests this was failing when negotiating audio as the \"OPUS\" Edge used didn't match the \"opus\" Janus negotiated. I checked and it still is an issue in your [latest version](https://github.com/fippo/adapter/blob/shim-ortc/adapter.js#L977) too.\r\n5. I had to add a further check in ```_getRemoteCapabilities```, in particular when parsing codecs in the SDP in order to add them to the list of codecs the peer supports. Parsing works fine, but when passing them to Edge it would cause exceptions for codecs it didn't know about. For instance, in some of my tests I used the SIP gateway feature in Janus, which resulted in the peer advertizing supports for stuff like GSM, speex, telephone-event, etc. which confused Edge: I can't recall where this happened exactly, I think when doing ```rtpReceiver.receive(params);``` after getting the common capabilities, which should have already filtered them out. I tried selectively removing the non-WebRTC codecs one by one, but I always got the exception: the only way to get that working was to filter them all out in ```_getRemoteCapabilities``` as done [here](https://github.com/meetecho/janus-gateway/blob/janus-edge/html/adapter.js#L469). A very ugly hack, but until this is fixed in Edge there's no prettier way of doing that.\r\n\r\nI think that's all. I have some more notes but, again, it's mostly on some things I have to move around a bit to make it work with the adapter version I had, and so may be more confusing than helpful. Let me know if there's any other detail I can provide you with, or if there's any way I can help you improving the code you already have in place now.\r\n\r\nThanks!"},"comment":{"url":"https://api.github.com/repos/fippo/adapter/issues/comments/149771687","html_url":"https://github.com/fippo/adapter/issues/2#issuecomment-149771687","issue_url":"https://api.github.com/repos/fippo/adapter/issues/2","id":149771687,"user":{"login":"fippo","id":289731,"avatar_url":"https://avatars.githubusercontent.com/u/289731?v=3","gravatar_id":"","url":"https://api.github.com/users/fippo","html_url":"https://github.com/fippo","followers_url":"https://api.github.com/users/fippo/followers","following_url":"https://api.github.com/users/fippo/following{/other_user}","gists_url":"https://api.github.com/users/fippo/gists{/gist_id}","starred_url":"https://api.github.com/users/fippo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fippo/subscriptions","organizations_url":"https://api.github.com/users/fippo/orgs","repos_url":"https://api.github.com/users/fippo/repos","events_url":"https://api.github.com/users/fippo/events{/privacy}","received_events_url":"https://api.github.com/users/fippo/received_events","type":"User","site_admin":false},"created_at":"2015-10-21T03:37:33Z","updated_at":"2015-10-21T03:37:33Z","body":"hey Lorenzo,\r\n\r\nthanks for the feedback and sorry for the delay.\r\n(1) odd, not an issue on any sites i've tried. Do you have a url where I can try to repro?\r\n(2) known issue (but I forgot to comment that). This should have been fixed in early October but is AFAIK not yet in build 10565. I'll check and get your issue updated. What works is a turn: url with port and transport attributes, i.e. turn:host:3478?transport=udp\r\n(3) should be easier with the new SDPUtils module. Do you include a=end-of-candidates in the SDP? Then it would be possible to use setRemoteCandidates.\r\n(4) fixed in https://github.com/fippo/adapter/commit/5a111f8c60d862287d4e0a1088b558974732cabd, will report upstream as well.\r\n(5) sounds familiar, Edge doesn't like unknown codecs (and it's probably correct spec-wise). Do you have an example of the SDP and the resulting common caps? (and I need to check if I have a regression here in the lastest version... thanks!)."}}