Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Complete support for multi-stream calls and renegotiation #63

Closed
ekzobrain opened this issue Apr 8, 2015 · 11 comments
Closed

Complete support for multi-stream calls and renegotiation #63

ekzobrain opened this issue Apr 8, 2015 · 11 comments

Comments

@ekzobrain
Copy link

Hello.

Currently plugin supports renegotiation in general, but I cannot make use of it because of these problems:

  1. lack of support for PC.removeStream(), see Safari/Explorer plugin bugs/features #51
  2. When I add a new stream to PC on the other side - onaddstream event is fired and I succesfilly obtain a new stream object. But when I attach it to rendering (video) element (instead of the old one) - video track from the stream is not displayed (i.e. the first stream had only audio, and the new one - audio + video). Or if the first stream had both audio and video, and the new only video - rendeding element doesn't become black, it just displays whe last frame from previous stream. So general problem is when a rendering element already has an attached stream, and I try to attach another one instead - it does not honor new or removed stream tracks.
  3. When I add a new stream to the connection (also on the other side), renegotiate peers - closing peer connection causes Safari to freeze.
  4. When I remove stream from the connection on the other side - onremovestream event doesn't fire.
@ekzobrain ekzobrain changed the title Complete support for Renegotiation Complete support for multi-stream calls and renegotiation Apr 8, 2015
@johache
Copy link
Contributor

johache commented Apr 8, 2015

  1. This is on the roadmap
  2. This should work fine. See this example for instance:
    https://plugin.temasys.com.sg/demo/samples/web/content/peerconnection/munge-sdp/index.html
    You attach a new mediaStream whether or not the previous one is still running.
    Would you have a code semple for us? (or is it the same application as we tried together last time?)
  3. I think this is a known issue.
  4. How do you remove a stream without PC::removeStream implemented? If you are just stopping the MS, it should fire an "onended" event

@ekzobrain
Copy link
Author

Hi. This ticket is not abandoned, just the situation changed a little. As you may already know - Firefox implemented support for renegotiation in v.38, byt they did it according the latest spec editors draft, so they did not implement method pc.removeStream(), till it is not in the spec any more. Now pc.addTrack(), pc.removeTrack() and 'track' events are used to send media data via peerconnection, no more streams at all. So i think there migth be no need to implement pc.removeStream() method at all, because no one could use it to implement a cross-browser solution (Firefor will not support it: https://bugzilla.mozilla.org/show_bug.cgi?id=842455). I am currently implementing a new renegotiation solution:

  1. Try pc.addTrack()/removeTrack() methods
  2. If no support - try pc.addStream() and then stream.addTrack()/removeTrack()
    I will test this approach and then publish testing results here.

@ekzobrain
Copy link
Author

Hi. So, I am using the following aproach with the new version of the plugin:

  1. When call is Webkit -> Webkit - use stream.addTrack()/stream.removeTrack() with Plan B SDP.
  2. When call is FF -> FF - use new API pc.addTrack()/pc.removeTrack() with Unified Plan SDP.
  3. When call is Webkit -> Plugin/Plugin -> Plugin - use pc.addStream()/pc.removeStream() with Plan B SDP.
    So I now have three questions:
  4. There is a bug in the new version of the Plugin (tested under Safari 7.1, OSX Maverics) - camera is not released when calling pc.removeStream() and stream.stop(). Steps to reproduce:
    a. Start new call with video enabled.
    b. Get new_stream without video.
    c. pc.removeStream(old_stream_with_video), pc.addStream(new_stream)
    d. old_stream_with_video.stop() - here camera should be released, because we removed stream from connection and stoped it, but camera signaling on Mac shows that it is still active. Even pc.close() doesn't release camera, only page refresh.
  5. Do you plan to implement Unified Plan instead of Plan B for multisteam calls?
  6. When do you plan to implement stream.removeTrack()? You now have stream.addTrack() now, but it is useless without removeTrack().

@johache
Copy link
Contributor

johache commented Jul 20, 2015

Hi,

1 - @magmaerupts can you try reproducing this on Safari, IE and Chrome, and fill a JIRA if necessary ?
2 - Yes, eventually. We are based on libWebRTC and therefore are dependent on the implementation from Google. I think the ticket on their side is there: https://code.google.com/p/chromium/issues/detail?id=465349
Note that I think you should still be able to send two streams in one PC using plan B. (addStream on the PC and re-negociation)
3 - Yes, It is planned for the next iteration. (although I would not say addTrack is useless without removeTrack

@ekzobrain
Copy link
Author

Hi, what about camera lock? Is it confirmed and would it be fixed in the next release?

@johache
Copy link
Contributor

johache commented Aug 24, 2015

Hi,
I just tested the scenario
a. Start new call with video enabled.
b. Get new_stream without video.
c. pc.removeStream(old_stream_with_video), pc.addStream(new_stream)
d. old_stream_with_video.stop()

The camera was properly released.

I am not sure of what is the purpose of getting a second stream if it have no track though. The scenario could have been simplfied to :
a. Start new call with video enabled.
c. pc.removeStream(old_stream_with_video),
d. old_stream_with_video.stop()

Do you agree on this ?

@ekzobrain
Copy link
Author

Streams with or without video of course contain audio, otherwise it really makes no sense. The goal is to disable/enable video in an audio call.
Tested under Safari 7.1 and OSX 10.9?

@agouaillard
Copy link

actually Dmitry, streams from a screen/window (chrome) or tab (firefox)
capturers only have one video track and no audio track, so you cannot
assume that if there is a video track there also is an audio track.

On Tue, Aug 25, 2015 at 1:22 AM, Dmitry notifications@github.com wrote:

Streams with or without video of course contain audio, otherwise it really
makes no sense. The goal is to disable/enable video in an audio call.
Tested under Safari 7.1 on OSX?


Reply to this email directly or view it on GitHub
#63 (comment).

Alex. Gouaillard, PhD, PhD, MBA

CTO - Temasys Communications, S'pore / Mountain View

President - CoSMo Software, Cambridge, MA

sg.linkedin.com/agouaillard

@ekzobrain
Copy link
Author

Yes, I generally agree that I did not provide complete information about that. But you missed that we are talking about camera, so screensharing is not aplicable here as a video stream....

@agouaillard
Copy link

fair enough ;)

On Tue, Aug 25, 2015 at 1:59 AM, Dmitry notifications@github.com wrote:

Yes, I generally agree that I did not provide complete information about
that. But you missed that we are talking about camera, so screensharing is
not aplicable here as a video stream....


Reply to this email directly or view it on GitHub
#63 (comment).

Alex. Gouaillard, PhD, PhD, MBA

CTO - Temasys Communications, S'pore / Mountain View

President - CoSMo Software, Cambridge, MA

sg.linkedin.com/agouaillard

@johache
Copy link
Contributor

johache commented Aug 25, 2015

Streams without audio make a lot of sense.
Think surveillance cameras for example.
I think we also have people sending the audio and video different PCs so that in case of connection breaks or renegociations, the audio stream is kept alive.

Anyways, tested on Safari 8, OSX 10.10, but it will be the same result on Safari 7.1, OSX 10.9.
I tested with AJS 0.12.0 and the plugin version 0.8.854.

@ekzobrain ekzobrain closed this as not planned Won't fix, can't repro, duplicate, stale Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants