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

RTCRtpSender not checking media track state or identity constraints in constructor #468

Closed
murillo128 opened this Issue Apr 13, 2016 · 10 comments

Comments

Projects
None yet
4 participants
@murillo128

murillo128 commented Apr 13, 2016

When an RTCRtpSender is created, the following error conditions should be checked (as in the setTrack and setTransport methods)

  • if track has peerIdentity constraints not matched by transport, then raise IncompatibleMediaStreamTrackError exception.
  • If track.readyState is "ended" then rasie an IncompatibleMediaStreamTrackError exception
  • if is called when transport.state or rtcpTransport.state is "closed", throw an InvalidStateError exception.

@murillo128 murillo128 changed the title from RTCRtpSender not checking media track state or identity constraints to RTCRtpSender not checking media track state or identity constraints in constructor Apr 13, 2016

@robin-raymond robin-raymond added the 1.1 label Apr 13, 2016

@robin-raymond

This comment has been minimized.

Show comment
Hide comment
@robin-raymond

robin-raymond Apr 14, 2016

Contributor

Correct, these same checks in the constructor need to happen as if it was done via setTrack.

Contributor

robin-raymond commented Apr 14, 2016

Correct, these same checks in the constructor need to happen as if it was done via setTrack.

@aboba

This comment has been minimized.

Show comment
Hide comment
@aboba

aboba Apr 21, 2016

Contributor

There is no mention of peerIdentity constraints in Media Capture and Streams:
http://w3c.github.io/mediacapture-main/#media-track-supported-constraints

Also, IncompatibleMediaStreamTrackError appears to have been removed from WebRTC 1.0:
http://w3c.github.io/webrtc-pc/

Contributor

aboba commented Apr 21, 2016

There is no mention of peerIdentity constraints in Media Capture and Streams:
http://w3c.github.io/mediacapture-main/#media-track-supported-constraints

Also, IncompatibleMediaStreamTrackError appears to have been removed from WebRTC 1.0:
http://w3c.github.io/webrtc-pc/

@murillo128

This comment has been minimized.

Show comment
Hide comment
@murillo128

murillo128 Apr 21, 2016

It is still there, check 10.4 http://w3c.github.io/webrtc-pc/#isolated-media-streams

WebRTC supports calling scenarios where media is sent to a specifically identified peer, without the contents of media streams being accessible to applications. This is enabled by use of the peerIdentity parameter to getUserMedia().

An application willingly relinquishes access to media by including a peerIdentity parameter in the MediaStreamConstraints. This attribute is set to a DOMString containing the identity of a specific peer.

The MediaStreamConstraints dictionary is expanded to include the peerIdentity parameter.

murillo128 commented Apr 21, 2016

It is still there, check 10.4 http://w3c.github.io/webrtc-pc/#isolated-media-streams

WebRTC supports calling scenarios where media is sent to a specifically identified peer, without the contents of media streams being accessible to applications. This is enabled by use of the peerIdentity parameter to getUserMedia().

An application willingly relinquishes access to media by including a peerIdentity parameter in the MediaStreamConstraints. This attribute is set to a DOMString containing the identity of a specific peer.

The MediaStreamConstraints dictionary is expanded to include the peerIdentity parameter.

@robin-raymond

This comment has been minimized.

Show comment
Hide comment
@robin-raymond

robin-raymond Apr 21, 2016

Contributor

From webrtc 1.0:

A track could have contents that are inaccessible to the application. This can be due to being marked with a peerIdentity option or anything that would make a track CORS cross-origin. These tracks can be supplied to the addTrack method, and have an RTCRtpSender created for them, but content must not be transmitted, unless they are also marked with peerIdentity and they meet the requirements for sending (see isolated streams and RTCPeerConnection).

So no, it would not throw an exception. It just would not transmit.

Contributor

robin-raymond commented Apr 21, 2016

From webrtc 1.0:

A track could have contents that are inaccessible to the application. This can be due to being marked with a peerIdentity option or anything that would make a track CORS cross-origin. These tracks can be supplied to the addTrack method, and have an RTCRtpSender created for them, but content must not be transmitted, unless they are also marked with peerIdentity and they meet the requirements for sending (see isolated streams and RTCPeerConnection).

So no, it would not throw an exception. It just would not transmit.

@aboba

This comment has been minimized.

Show comment
Hide comment
@aboba

aboba Apr 21, 2016

Contributor

WebRTC 1.0 Section 5.1.2 says:

"All other tracks that are not accessible to the application must not be sent to the peer, with silence (audio), black frames (video) or equivalently absent content being sent in place of track content."

So yes, I think that is correct.

Contributor

aboba commented Apr 21, 2016

WebRTC 1.0 Section 5.1.2 says:

"All other tracks that are not accessible to the application must not be sent to the peer, with silence (audio), black frames (video) or equivalently absent content being sent in place of track content."

So yes, I think that is correct.

@robin-raymond

This comment has been minimized.

Show comment
Hide comment
@robin-raymond

robin-raymond Apr 21, 2016

Contributor

My recommendation:

  • strip this peer identity stuff from the sender (receiver?)
  • add this to the peer identity section:

If the MediaStreamTrack has a peerIdentity constraint applied and this track is attached to an RTCRtpSender, then do not send media over the sender unless the attached RTCDtlsTransport has validated the same peerIdentity as the MediaStreamTrack's constraint.

Contributor

robin-raymond commented Apr 21, 2016

My recommendation:

  • strip this peer identity stuff from the sender (receiver?)
  • add this to the peer identity section:

If the MediaStreamTrack has a peerIdentity constraint applied and this track is attached to an RTCRtpSender, then do not send media over the sender unless the attached RTCDtlsTransport has validated the same peerIdentity as the MediaStreamTrack's constraint.

@murillo128

This comment has been minimized.

Show comment
Hide comment
@murillo128

murillo128 Apr 21, 2016

I think it is always better to yield an error than fail silently.

Anyway, the same behavior shall be consistent across the three methods (constructor,setMediaTrack,setTransport).

murillo128 commented Apr 21, 2016

I think it is always better to yield an error than fail silently.

Anyway, the same behavior shall be consistent across the three methods (constructor,setMediaTrack,setTransport).

@robin-raymond

This comment has been minimized.

Show comment
Hide comment
@robin-raymond

robin-raymond Apr 21, 2016

Contributor

@murillo128 convince 1.0 guys of this and we will follow their lead.

Contributor

robin-raymond commented Apr 21, 2016

@murillo128 convince 1.0 guys of this and we will follow their lead.

@murillo128

This comment has been minimized.

Show comment
Hide comment
@murillo128

murillo128 Apr 21, 2016

@robin-raymond I have enough trying to convince you ;)

I am fine to whatever 1.0 says

murillo128 commented Apr 21, 2016

@robin-raymond I have enough trying to convince you ;)

I am fine to whatever 1.0 says

@elagerway

This comment has been minimized.

Show comment
Hide comment
@elagerway

elagerway Apr 26, 2016

Contributor

Closing, this needs to be taken up in the WebRTC Working Group as per my recent message to the mail list:
https://lists.w3.org/Archives/Public/public-ortc/2016Apr/0165.html

Contributor

elagerway commented Apr 26, 2016

Closing, this needs to be taken up in the WebRTC Working Group as per my recent message to the mail list:
https://lists.w3.org/Archives/Public/public-ortc/2016Apr/0165.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment