-
Notifications
You must be signed in to change notification settings - Fork 35
feat(sdk): add maintainResolution publish option to hold a constant encoder resolution #524
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@millicast/sdk": minor | ||
| --- | ||
|
|
||
| Added a `maintainResolution` publish option that keeps the encoder at a constant resolution, giving up frame rate instead when bandwidth or CPU is constrained. `contentHint` is now also carried over to the replacement track by `replaceTrack`. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,12 +22,62 @@ const connectOptions = { | |
| codec: VideoCodec.H264, | ||
| simulcast: false, | ||
| scalabilityMode: null, | ||
| maintainResolution: false, | ||
| peerConfig: { | ||
| autoInitStats: true, | ||
| statsIntervalMs: 1000 | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Ask the pipeline to keep resolution constant and sacrifice frame rate instead. | ||
| * | ||
| * A mid-stream resolution change forces the encoder to re-emit its parameter sets, and | ||
| * max_num_ref_frames is derived from the level's DPB capacity divided by the frame size, so it | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its not clear why max_num_ref_frames is important. I think we can avoid mentioning it except in the PR description. In the PR description, mention it but also describe why it is important this stays the same. I.e. Some decoders have glitches in the output if this changes, we cant set it directly but by holding the resolution constant we avoid it changing for the VideoToolbox encoder. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed —
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please update There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in |
||
| * changes with the resolution. Some downstream transcoders mishandle that combination. Holding | ||
| * the resolution steady avoids the situation entirely. | ||
| * | ||
| * Applied in two places because neither alone is reliable across browsers: contentHint is | ||
| * broadly supported and steers libwebrtc's degradation preference implicitly, while | ||
| * degradationPreference states it explicitly but is not implemented everywhere. | ||
| * @param {MediaStream|Array<MediaStreamTrack>} mediaStream - Stream or tracks being published. | ||
| */ | ||
| const applyResolutionContentHint = (mediaStream) => { | ||
| const tracks = Array.isArray(mediaStream) | ||
| ? mediaStream.filter(track => track.kind === 'video') | ||
| : mediaStream?.getVideoTracks?.() ?? [] | ||
| for (const track of tracks) { | ||
| track.contentHint = 'detail' | ||
| if (track.contentHint !== 'detail') { | ||
| logger.warn('maintainResolution: contentHint was not accepted for track ', track.id) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Set degradationPreference on every video sender, and verify it stuck. | ||
| * | ||
| * Browsers may accept setParameters() and silently drop the field, so the value is read back | ||
| * rather than assumed. A warning here means the option is resting on contentHint alone. | ||
| * @param {RTCPeerConnection} peer - Peer connection whose senders should be configured. | ||
| */ | ||
| const applyDegradationPreference = async (peer) => { | ||
| const senders = peer.getSenders().filter(sender => sender.track?.kind === 'video') | ||
| for (const sender of senders) { | ||
| try { | ||
| const parameters = sender.getParameters() | ||
| parameters.degradationPreference = 'maintain-resolution' | ||
| await sender.setParameters(parameters) | ||
| const applied = sender.getParameters().degradationPreference | ||
| if (applied !== 'maintain-resolution') { | ||
| logger.warn('maintainResolution: degradationPreference not honoured, read back as ', applied) | ||
| } | ||
| } catch (error) { | ||
| logger.warn('maintainResolution: could not set degradationPreference ', error) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @class Publish | ||
| * @extends BaseWebRTC | ||
|
|
@@ -72,6 +122,10 @@ export default class Publish extends BaseWebRTC { | |
| * @param {Boolean} [options.simulcast = false] - Enable simulcast. **Only available in Chromium based browsers and with H.264 or VP8 video codecs.** | ||
| * @param {String} [options.scalabilityMode = null] - Selected scalability mode. You can get the available capabilities using <a href="PeerConnection#.getCapabilities">PeerConnection.getCapabilities</a> method. | ||
| * **Only available in Google Chrome.** | ||
| * @param {Boolean} [options.maintainResolution = false] - Hold the encoder at a constant resolution, | ||
| * giving up frame rate instead when bandwidth or CPU is constrained. Sets `contentHint = 'detail'` on | ||
| * the video track and `degradationPreference = 'maintain-resolution'` on the sender. Both are | ||
| * preferences, not guarantees; the browser may still adapt under sustained pressure. | ||
| * @param {PeerConnectionConfig} [options.peerConfig = null] - Options to configure the new RTCPeerConnection. | ||
| * @param {Boolean} [options.record = false ] - Enable stream recording. If record is not provided, use default Token configuration. **Only available in Tokens with recording enabled.** | ||
| * @param {Array<String>} [options.events = null] - Specify which events will be delivered by the server (any of "active" | "inactive" | "viewercount").* | ||
|
|
@@ -167,6 +221,9 @@ export default class Publish extends BaseWebRTC { | |
| logger.error('Error while broadcasting. MediaStream required') | ||
| throw new Error('MediaStream required') | ||
| } | ||
| if (this.options.maintainResolution) { | ||
| applyResolutionContentHint(this.options.mediaStream) | ||
| } | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
Outdated
|
||
| if (!data.migrate && this.isActive()) { | ||
| logger.warn('Broadcast currently working') | ||
| throw new Error('Broadcast currently working') | ||
|
|
@@ -221,6 +278,11 @@ export default class Publish extends BaseWebRTC { | |
| promises = await Promise.all([getLocalSDPPromise, signalingConnectPromise]) | ||
| const localSdp = promises[0] | ||
|
|
||
| // Senders only exist once getRTCLocalSDP has added the tracks above. | ||
| if (this.options.maintainResolution) { | ||
| await applyDegradationPreference(webRTCPeerInstance.getRTCPeer()) | ||
| } | ||
|
Comment on lines
+279
to
+282
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 degradationPreference set before setLocalDescription
Was this helpful? React with 👍 or 👎 to provide feedback. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intentional. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The timing is fine and intentional: |
||
|
|
||
| if (this.options.metadata) { | ||
| if (!this.worker) { | ||
| this.worker = new TransformWorker() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1297,6 +1297,13 @@ declare module "@millicast/sdk" { | |
| * **Only available in Google Chrome.** | ||
| */ | ||
| scalabilityMode?: string | ||
| /** | ||
| * - Hold the encoder at a constant resolution, giving up frame rate instead when bandwidth | ||
| * or CPU is constrained. Sets `contentHint = 'detail'` on the video track and | ||
| * `degradationPreference = 'maintain-resolution'` on the sender. Both are preferences, not | ||
| * guarantees; the browser may still adapt under sustained pressure. | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make clear this options is primarily to work-around a bug found in some decoders (such as youtube after the webrtc is restreamed). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noted — will lead with the intent rather than the mechanism, i.e. that this is primarily a workaround for decoders that glitch on a mid-stream resolution change (seen with YouTube when the WebRTC feed is restreamed), not a general quality knob, and keep the caveats that it trades frame rate for resolution and is a preference the browser may still override.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make sure this update is applied There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Applied in |
||
| */ | ||
| maintainResolution?: boolean | ||
| /** | ||
| * - Options to configure the new RTCPeerConnection. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| Feature: As a broadcaster I want to keep a constant resolution so downstream consumers never see a mid-stream resolution change | ||
|
|
||
| Scenario: Broadcast with maintainResolution enabled | ||
| Given an instance of Publish with connection path | ||
| When I broadcast a stream with maintainResolution enabled | ||
| Then the video track is hinted for detail | ||
| And the video sender prefers to maintain resolution | ||
|
|
||
| Scenario: Broadcast without maintainResolution | ||
| Given an instance of Publish with connection path | ||
| When I broadcast a stream with media stream | ||
| Then the video track has no content hint | ||
| And the video sender has no degradation preference | ||
|
|
||
| Scenario: Broadcast with maintainResolution enabled and an unsupported browser | ||
| Given an instance of Publish with connection path | ||
| And a browser that ignores the degradation preference | ||
| When I broadcast a stream with maintainResolution enabled | ||
| Then the broadcast is still connected | ||
| And the video track is hinted for detail |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| import { loadFeature, defineFeature } from 'jest-cucumber' | ||
| import Publish from '../../src/Publish' | ||
| import './__mocks__/MockRTCPeerConnection' | ||
| import './__mocks__/MockMediaStream' | ||
| import './__mocks__/MockBrowser' | ||
|
|
||
| const feature = loadFeature('../features/MaintainResolution.feature', { loadRelativePath: true, errors: true }) | ||
|
|
||
| jest.mock('../../src/Signaling') | ||
|
|
||
| jest.mock('../../src/workers/TransformWorker.worker.js', () => | ||
| jest.fn(() => ({ | ||
| postMessage: jest.fn(), | ||
| terminate: jest.fn() | ||
| })) | ||
| ) | ||
|
|
||
| const mockTokenGenerator = jest.fn(() => { | ||
| return { | ||
| urls: [ | ||
| 'ws://localhost:8080' | ||
| ], | ||
| jwt: process.env.JWT_TEST_TOKEN ?? 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJtaWxsaWNhc3QiOnt9fQ.IqT-PLLz-X7Wn7BNo-x4pFApAbMT9mmnlupR8eD9q4U' | ||
| } | ||
| }) | ||
|
|
||
| const videoSenderOf = (publisher) => | ||
| publisher.getRTCPeerConnection().getSenders().find(sender => sender.track?.kind === 'video') | ||
|
|
||
| defineFeature(feature, test => { | ||
| afterEach(async () => { | ||
| jest.restoreAllMocks() | ||
| }) | ||
|
|
||
| test('Broadcast with maintainResolution enabled', ({ given, when, then, and }) => { | ||
| let publisher | ||
| let mediaStream | ||
|
|
||
| given('an instance of Publish with connection path', async () => { | ||
| publisher = new Publish('streamName', mockTokenGenerator) | ||
| mediaStream = new MediaStream([{ kind: 'video' }, { kind: 'audio' }]) | ||
| }) | ||
|
|
||
| when('I broadcast a stream with maintainResolution enabled', async () => { | ||
| await publisher.connect({ mediaStream, maintainResolution: true }) | ||
| }) | ||
|
|
||
| then('the video track is hinted for detail', async () => { | ||
| expect(mediaStream.getVideoTracks()[0].contentHint).toEqual('detail') | ||
| }) | ||
|
|
||
| and('the video sender prefers to maintain resolution', async () => { | ||
| expect(videoSenderOf(publisher).getParameters().degradationPreference) | ||
| .toEqual('maintain-resolution') | ||
| }) | ||
| }) | ||
|
|
||
| test('Broadcast without maintainResolution', ({ given, when, then, and }) => { | ||
| let publisher | ||
| let mediaStream | ||
|
|
||
| given('an instance of Publish with connection path', async () => { | ||
| publisher = new Publish('streamName', mockTokenGenerator) | ||
| mediaStream = new MediaStream([{ kind: 'video' }, { kind: 'audio' }]) | ||
| }) | ||
|
|
||
| when('I broadcast a stream with media stream', async () => { | ||
| await publisher.connect({ mediaStream }) | ||
| }) | ||
|
|
||
| then('the video track has no content hint', async () => { | ||
| expect(mediaStream.getVideoTracks()[0].contentHint).toBeUndefined() | ||
| }) | ||
|
|
||
| and('the video sender has no degradation preference', async () => { | ||
| expect(videoSenderOf(publisher).getParameters().degradationPreference).toBeUndefined() | ||
| }) | ||
| }) | ||
|
|
||
| test('Broadcast with maintainResolution enabled and an unsupported browser', ({ given, when, then, and }) => { | ||
| let publisher | ||
| let mediaStream | ||
|
|
||
| given('an instance of Publish with connection path', async () => { | ||
| publisher = new Publish('streamName', mockTokenGenerator) | ||
| mediaStream = new MediaStream([{ kind: 'video' }, { kind: 'audio' }]) | ||
| }) | ||
|
|
||
| // Browsers may accept setParameters() and drop degradationPreference, or reject the call | ||
| // outright. Neither may take the broadcast down: the option degrades to contentHint alone. | ||
| and('a browser that ignores the degradation preference', async () => { | ||
| jest.spyOn(global.RTCPeerConnection.prototype, 'addTransceiver') | ||
| .mockImplementation(function (track) { | ||
| this.senders.push({ | ||
| track, | ||
| getParameters: () => ({}), | ||
| setParameters: () => Promise.reject(new Error('Not supported')), | ||
| replaceTrack: () => {} | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| when('I broadcast a stream with maintainResolution enabled', async () => { | ||
| await publisher.connect({ mediaStream, maintainResolution: true }) | ||
| }) | ||
|
|
||
| then('the broadcast is still connected', async () => { | ||
| expect(publisher.isActive()).toBeTruthy() | ||
| }) | ||
|
|
||
| and('the video track is hinted for detail', async () => { | ||
| expect(mediaStream.getVideoTracks()[0].contentHint).toEqual('detail') | ||
| }) | ||
| }) | ||
| }) |
Uh oh!
There was an error while loading. Please reload this page.