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

MediaStream.removeTrack() does no longer work #95

Closed
ibc opened this issue Nov 26, 2015 · 17 comments
Closed

MediaStream.removeTrack() does no longer work #95

ibc opened this issue Nov 26, 2015 · 17 comments
Labels

Comments

@ibc
Copy link
Collaborator

ibc commented Nov 26, 2015

This happens since ab771b0 (latest version of libwebrtc is included in the plugin).

Calling stream.removeTrack(track) does not work at all (track remain active in the new SDP) nor track.stop() (track media is still being transmitted to the remote peer).

Details in the issue reported in the libWebRTC tracker:

@ibc ibc added the bug label Nov 26, 2015
@ibc
Copy link
Collaborator Author

ibc commented Nov 27, 2015

What I see is that track.stop() does nothing (it fires a "ended" event, but video data is still transmitted to the remote).
In the other side, track.enabled = false does work as expected.

A dirty partial workaround is to also call track.enabled = false within the track.stop() method (I hate myself).

@ibc
Copy link
Collaborator Author

ibc commented Nov 27, 2015

To be fu**ing clear:

Having a connected PeerConnection with bidirectional audio/video:

  • pc.removeTrack(localVideoTrack); => ok
  • pc.createOffer() => It produces a SDP with video and a=sendrecv!!!

@ibc
Copy link
Collaborator Author

ibc commented Nov 27, 2015

To summarize, there are two bugs:

  • Have a RTCPeerConnection (pc) and provide it with a local audio+video RTCMediaStream (m).
  • Have v as the RTCVideoTrack of m.
  1. RTCMediaStream.removeAudio/VideoTrack() does not properly removes the track:
  • Call m.removeVideoTrack(v).
  • Later call pc.createOfferWithDelegate(...).

It produces a RTCSessionDescription which has m "video" section with "a=sendrecv". This is wrong. The m "video" section should have "a=inactive" or "a=recvonly" (in case offerToReceiveVideo is passed as constraint to the createOfferWithDelegate() method).

  1. RTCMediaStreamTrack.setState(RTCTrackStateEnded) does nothing:
  • Call v.setState(RTCTrackStateEnded)

The delegate gets called with state=ended and enabled=true, but the track media is still being transmitted to the remote peer.

@ibc
Copy link
Collaborator Author

ibc commented Dec 1, 2015

The issue placed at Google's libwebrtc tracker has been updated by devs:

I just hate the given rationale.

@ibc
Copy link
Collaborator Author

ibc commented Dec 2, 2015

It seems that the old behavior (mangling tracks in MediaStream also affects the PeerConnection) will be re-added to libwebrtc:

@ibc
Copy link
Collaborator Author

ibc commented Dec 15, 2015

Let's wait until libwebrtc devs revert the changes producing this issue.

@ibc
Copy link
Collaborator Author

ibc commented Dec 17, 2015

Good news! libwebrtc changes producing this issue were reverted yesterday:

https://bugs.chromium.org/p/webrtc/issues/detail?id=5265#c18

TASK: Update libwebrtc.

@ibc
Copy link
Collaborator Author

ibc commented Dec 17, 2015

Bad news: in a first attempt it does not work.

@ibc
Copy link
Collaborator Author

ibc commented Dec 17, 2015

It works after a clean-up.

@ibc
Copy link
Collaborator Author

ibc commented Dec 17, 2015

Fixed in 39db0e4 (after upgrading libwebrtc to 11063).

Version 2.2.2 coming soon including this fix.

@ibc ibc closed this as completed Dec 17, 2015
@saghul
Copy link
Collaborator

saghul commented Dec 17, 2015

@ibc can you please mention somewhere (a file in lib, a note in the release notes, etc) what libwebtc version is used? Thanks!

@ibc
Copy link
Collaborator Author

ibc commented Dec 17, 2015

It is noted in the doc: 11063

@saghul
Copy link
Collaborator

saghul commented Dec 17, 2015

I mean inside the source tree. If I download a given release I don't know what it bas built with. A "revision" file next to the binary would be nice. Just for documentation purposes.

@ibc
Copy link
Collaborator Author

ibc commented Dec 17, 2015

But the doc is also inside the project tree... And having to keep the revision in sync in two places (doc and a separate file) would carry problems, don't you agree?

@saghul
Copy link
Collaborator

saghul commented Dec 17, 2015

Oh, wait, I'm dumb, the doc is indeed inside the project! Sorry for the noise :-)

@ibc
Copy link
Collaborator Author

ibc commented Dec 17, 2015

No longer doc wikis!

@saghul
Copy link
Collaborator

saghul commented Dec 17, 2015

👍 🍰

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants