Add experimental fallback codec support #334
Conversation
🦋 Changeset detectedLatest commit: 1de74c7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
thinking a bit about the API, and just reading the options, it doesn't seem intuitive to me what I should be doing there. Let's discuss @lukasIO; maybe there's a simple version of it that could work for the majority of the cases? |
| @@ -492,32 +497,31 @@ export default class LocalParticipant extends Participant { | |||
| req.height = height ?? 0; | |||
| // for svc codecs, disable simulcast and use vp8 for backup codec | |||
| if (track instanceof LocalVideoTrack) { | |||
| if (opts?.videoCodec === 'vp9' || opts?.videoCodec === 'av1') { | |||
| if (opts.backupCodec && (opts?.videoCodec === 'vp9' || opts?.videoCodec === 'av1')) { | |||
| // set scalabilityMode to 'L3T3' by default | |||
| opts.scalabilityMode = opts.scalabilityMode ?? 'L3T3'; | |||
There was a problem hiding this comment.
I think we need to set scalability mode as long as it's AV1, even if a backupCodec isn't set.
We should also remove VP9 references, since we've decided to focus on AV1 and not worry about VP9
| }; | ||
| // clear scalabilityMode setting for backup codec | ||
| opts.scalabilityMode = undefined; | ||
| opts.videoCodec = videoCodec; |
There was a problem hiding this comment.
does this get set elsewhere? it gets used here
There was a problem hiding this comment.
the link doesn't work, but generally get's set in computeTrackBackupEncodings
Does that resolve the issue you saw?
There was a problem hiding this comment.
ah ha I see it. Just didn't expect that computeXXX would mutate one of its arguments.
There was a problem hiding this comment.
that's fair. I moved it there in order to keep the arguments untouched wherever possible and then only have a distinct function that needs the mutation to actually do it.
src/room/participant/publishUtils.ts
Outdated
| encoding.maxBitrate = encoding.maxBitrate * 0.7; | ||
| break; | ||
| case 'h264': | ||
| encoding.maxBitrate = encoding.maxBitrate * 1.1; |
There was a problem hiding this comment.
aren't H.264 and VP8 at similar efficiencies?
There was a problem hiding this comment.
yeah, we can ignore it by default. I think the efficiency ultimately comes down to which h.264 profile is used
| }; | ||
| // clear scalabilityMode setting for backup codec | ||
| opts.scalabilityMode = undefined; | ||
| opts.videoCodec = videoCodec; |
There was a problem hiding this comment.
ah ha I see it. Just didn't expect that computeXXX would mutate one of its arguments.
lukasIO
changed the title
| this.setPreferredCodec(transceiver, track.kind, videoCodec); | ||
| track.setSimulcastTrackSender(videoCodec, transceiver.sender); | ||
|
|
||
| // TODO do we want to support av1 as a backup codec? |
There was a problem hiding this comment.
I think we will not use av1 as backup, it should be primary codec if exist
This is a draft/suggestion of how to expose custom multi codec publishing encodings to users.
Naming (polycast) of course also up for debate, but as everything is marked as
@experimentalit should also be fine to rename afterwards