Skip to content
This repository has been archived by the owner on Sep 19, 2024. It is now read-only.

Add ability for JS to switch audio/video track #13

Merged
merged 4 commits into from
Aug 17, 2021
Merged

Add ability for JS to switch audio/video track #13

merged 4 commits into from
Aug 17, 2021

Conversation

sax
Copy link
Contributor

@sax sax commented Jul 28, 2021

This, like the rest of the JS, assumes only one audio or video track is being streamed up to the SFU. This should close #11.

@mickel8 mickel8 added api enhancement New feature or request labels Jul 28, 2021
@mickel8 mickel8 requested review from mickel8 and mat-hek July 28, 2021 15:20
* })
* ```
*/
public async replaceTrack(track: MediaStreamTrack): Promise<any> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd consider accepting the old track id as the first argument and using it to find the track to replace. That way the API should remain unchanged when we support multiple tracks.

@sax
Copy link
Contributor Author

sax commented Aug 9, 2021

One thing I need to track down is to ensure that when the track is swapped out, it is recognized as a new SSRC rather than re-using the old one. This is specified as a SHOULD in RFC 3550, and that RFC does specify that that a single SSRC can change encodings on the fly. Adding a new track and removing the old track seems closer to the intention of the RFC, though.

@mickel8
Copy link
Contributor

mickel8 commented Aug 13, 2021

@sax I think that both secnarios (adding/removing and replacing SSRC) require from us an ICE restart.

I think that we could also add some kind of notification that some track has been replaced. It might be useful in some cases.

Regarding changing codecs. Yeah, we were analyzing this some time ago and it looks like each side can use any codec accepted by the peer so if e.g. in the offer there is audio track with 4 codecs

m=audio 120 121 122 123

and response accepts, let's say 3 off them

m=audio 120 121 122

then each of this codec can be used and sender can switch between them without renegotiation.

However this feature is quite tricky and we don't support it.

Could you link relevant sections from RFC 3550. I cannot find them.

@sax
Copy link
Contributor Author

sax commented Aug 13, 2021

@mickel8 Some links to RFC3550 that jump out to me:

2.1 - Simple Multicast Audio Conference: In this section is the following reference:

The RTP header indicates what type of audio encoding (such as PCM, ADPCM or LPC) is contained in each packet so that senders can change the encoding during a conference, for example, to accommodate a new participant that is connected through a low-bandwidth link or react to indications of network congestion.

3 - Definitions: The definition for SSRC has the following references:

Examples of synchronization sources include the sender of a stream of packets derived from a signal source such as a microphone or a camera

A synchronization source may change its data format, e.g., audio encoding, over time.

It also specifies this:

If a participant generates multiple streams in one RTP session, for example from separate video cameras, each MUST be identified as a different SSRC.

So by one reading of this, switching cameras does not create "multiple streams," so it might be fine if it retains the same SSRC on the backend. I think my concern comes in with multiple streams, in this flow:

  • User joins a multimedia session with camera A
  • User switches to camera B
  • User adds a second video track, with camera A

If the camera switch retains the same SSRC, and the second video track winds up being in the same RTP session, but somehow gets the same SSRC, it might be detected as a loop. I'm not sure if this is a problem, but just want to make sure that the device switching doesn't leave loose ends.

@mickel8
Copy link
Contributor

mickel8 commented Aug 16, 2021

I think I see your point.

but somehow gets the same SSRC

This shouldn't be possible. Each time we create a new track we choose some random SSRC and we check wheather it is already used by any other track. We repeat the whole process until we generate the proper SSRC.

Does it answer your question?

The only thing I think we should add is ability to update track metadata while replacing it. Let's assume the following scenario

addTrack(some_track, metadata: {source: cameraA})
replaceTrack(some_other_track) // here we want to update metadata as source is no longer cameraA

In such a case we should also add some notification that track has been updated passing to the user new metadata.

cc @mat-hek

@mat-hek
Copy link
Contributor

mat-hek commented Aug 16, 2021

Agreed, I think that replaceTrack could just accept optional metadata argument replaceTrack(old_track_id, new_track, optional_metadata)

@sax
Copy link
Contributor Author

sax commented Aug 16, 2021

Just pushed the change to require the old track id when replacing a track.

It seems to me that updating the metadata will require a new MediaEvent type, no? Right now metadata and tracksMetadata only come in on the join media event.

Could the feature to update metadata be split out into a separate issue? If there was a more generic ability to change metadata and tracks metadata, publicly exposed on the client, then passing optional track metadata could just call that function.

@mickel8
Copy link
Contributor

mickel8 commented Aug 17, 2021

Could the feature to update metadata be split out into a separate issue? If there was a more generic ability to change metadata and tracks metadata, publicly exposed on the client, then passing optional track metadata could just call that function.

Sounds good to me!

Could the feature to update metadata be split out into a separate issue?

Sure, I will make an issue

IMO, we can merge this PR

@mat-hek mat-hek merged commit c46458a into fishjam-dev:master Aug 17, 2021
@jasonwc jasonwc deleted the add-replace-track-js branch October 5, 2021 20:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability for client to switch audio/video track
4 participants