Skip to content

feat(sdk): add maintainResolution publish option to hold a constant encoder resolution - #524

Open
bcostdolby wants to merge 2 commits into
mainfrom
devin/1787536301-maintain-resolution
Open

feat(sdk): add maintainResolution publish option to hold a constant encoder resolution#524
bcostdolby wants to merge 2 commits into
mainfrom
devin/1787536301-maintain-resolution

Conversation

@bcostdolby

@bcostdolby bcostdolby commented Aug 24, 2026

Copy link
Copy Markdown

Summary

Adds an opt-in maintainResolution publish option (default false) that asks the pipeline to hold the encoder at a constant resolution and give up frame rate instead.

await publisher.connect({ mediaStream, maintainResolution: true })

Why. This is a workaround for decoders that glitch on a mid-stream resolution change — we hit it relaying a browser publisher to a third-party RTMP destination (YouTube), where the publisher ramps 1280x720 → 960x540 → 1280x720 within the first ~7 seconds while the bandwidth estimate settles, and the downstream output is corrupted for minutes afterwards.

The mechanism: a resolution change forces the H.264 encoder to re-emit its parameter sets, and max_num_ref_frames changes along with the resolution because it is bounded by the level's DPB capacity divided by the frame size. Some decoders mishandle a stream where that value changes mid-flight. max_num_ref_frames is not settable from the browser — there is no API for it — so the only lever we have is to remove the trigger, i.e. keep the resolution constant so the parameter sets never need to be re-emitted in the first place. (Note this is not specific to any one encoder; VideoToolbox is simply where we observed it. Nor can the SDK select the encoder implementation — no browser exposes that to JS.)

Until now publishers had no way to ask for a stable resolution: neither contentHint nor degradationPreference was reachable through the SDK.

Why both properties, not one. Neither is reliable alone. contentHint = 'detail' is broadly supported and steers libwebrtc's degradation preference implicitly; degradationPreference = 'maintain-resolution' states it explicitly but is not implemented everywhere. Both are set, both are read back, and a logger.warn fires if a browser silently ignored one instead of leaving the caller to assume it worked. A rejected setParameters() is caught and does not fail the broadcast — the option degrades to contentHint alone.

Publish.initConnection():

// after the "MediaStream required" validation and the duplicate-connect isActive() guard,
// so a rejected connect() never mutates a live track's hint
if (this.options.maintainResolution) applyResolutionContentHint(this.options.mediaStream)
...
// after getRTCLocalSDP() resolves — that is what adds the tracks, so senders do not exist before it
if (this.options.maintainResolution) await applyDegradationPreference(webRTCPeerInstance.getRTCPeer())

The sender pass uses the local webRTCPeerInstance rather than this.getRTCPeerConnection() so it is also correct on the data.migrate path, where this.webRTCPeer still points at the previous peer. applyResolutionContentHint handles mediaStream being either a MediaStream or an array of tracks, matching what the rest of the option surface accepts.

Why replaceTrack had to change. contentHint is a property of the track, not of the sender, so replacing a track (e.g. a camera switch) drops it and the option would silently stop applying mid-broadcast. PeerConnection.replaceTrack() now carries the outgoing track's hint over to the incoming one unless the incoming track declares its own. This is unconditional rather than gated on the new option: preserving an explicitly set hint is correct regardless, and it is inert when no hint was set. degradationPreference needs no equivalent — it lives on the sender and survives replacement.

Caveat. Both settings are preferences, not guarantees; a browser may still adapt under sustained pressure. On a constrained uplink this trades frame rate for resolution, so a publisher that would previously have shrunk the picture will instead drop frames.

Also: maintainResolution?: boolean added to PublishConnectOptions in src/types/index.d.ts, plus a minor changeset. The user-facing docs describe the option by intent (a decoder-glitch workaround) and deliberately leave max_num_ref_frames out — that detail lives here rather than in the source.

Tests

New MaintainResolution feature (jest-cucumber): option enabled sets both properties; option absent leaves both untouched; a browser whose setParameters() rejects still connects and still gets the content hint. ChangeMediaTrack gains two scenarios covering hint carry-over and non-override of an explicit incoming hint. The shared MockRTCPeerConnection senders gained getParameters/setParameters (on both the addTrack and addTransceiver sender objects) so the sender path is testable.

Verified: npx jest --testMatch "**/unit/*.steps.js" → 30 suites / 178 tests pass; npx eslint src/Publish.js clean; npx tsc --build clean.

Note: the e2e-test CI job fails on this branch with Unauthorized: Account disabled, which also fails identically on main — a test-account issue, not a code issue.

Link to Devin session: https://dolby.devinenterprise.com/sessions/9709b96479024ecb90873c68dadf5535
Requested by: @bcostdolby


Open in Devin Review

…ncoder resolution

A mid-stream resolution change makes the encoder 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 changes along with the resolution. Some downstream transcoders mishandle that pairing and corrupt their output. Publishers had no way to ask for a stable resolution: neither contentHint nor degradationPreference was reachable through the SDK.

maintainResolution (opt-in, default false) sets contentHint = 'detail' on every video track and degradationPreference = 'maintain-resolution' on every video sender, so the pipeline gives up frame rate rather than resolution. Both are applied because neither is reliable alone: contentHint is widely supported and steers the preference implicitly, while degradationPreference states it explicitly but is not implemented everywhere. Values are read back and a warning is logged when a browser silently ignores one; a rejected setParameters() does not fail the broadcast.

replaceTrack now carries any contentHint across to the replacement track unless that track declares its own. contentHint belongs to the track rather than the sender, so without this the option would quietly stop applying the first time an application switched camera. degradationPreference needs no equivalent handling as it lives on the sender and survives the replacement.

Co-Authored-By: Brendon Costa <brendon.costa@dolby.com>
@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@changeset-bot

changeset-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 1dc653e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@millicast/sdk Minor

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

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

Open in Devin Review

Comment thread packages/millicast-sdk/src/Publish.js Outdated
Comment on lines +281 to +284
// Senders only exist once getRTCLocalSDP has added the tracks above.
if (this.options.maintainResolution) {
await applyDegradationPreference(webRTCPeerInstance.getRTCPeer())
}

@devin-ai-integration devin-ai-integration Bot Aug 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 degradationPreference set before setLocalDescription

applyDegradationPreference runs right after getRTCLocalSDP resolves, while setLocalDescription happens later. Senders already exist from addTrack, so setParameters is valid and failures are caught with a read-back warning. Confirm browsers honor the value set at this stage.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Intentional. degradationPreference is a send-parameter on the RTCRtpSender, not part of the negotiated description, so it is not tied to setLocalDescription() — the senders exist from addTrack() inside getRTCLocalSDP(), which is the earliest point setParameters() can be called at all. It takes effect once the encoder starts. Failures are caught and the value read back, so a browser that ignores it produces a warning rather than a failed broadcast.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The timing is fine and intentional: setParameters() only requires the sender to exist, which it does once getRTCLocalSDP() has run addTrack/addTransceiver, and degradationPreference is a send-side encoder preference that isn't negotiated in the SDP — so it doesn't need to be in place before setLocalDescription. Setting it here also means it applies from the first encoded frame rather than after the connection is already up. If a browser does reject or ignore it, the read-back warning surfaces that and the broadcast continues on contentHint alone.

@bcostdolby bcostdolby closed this Aug 24, 2026
@bcostdolby bcostdolby reopened this Aug 24, 2026
Comment thread packages/millicast-sdk/src/Publish.js Outdated
* 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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Agreed — max_num_ref_frames is implementation detail that doesn't help someone reading the SDK source. Noted for when I make changes: strip it from the JSDoc, leaving just "holds resolution constant, trades frame rate, workaround for decoders that glitch on mid-stream resolution changes", and move the mechanism into the PR description with the why spelled out (resolution change → parameter sets re-emitted → max_num_ref_frames changes with frame size since it's bounded by the level's DPB capacity → some decoders glitch on that pairing; we can't set it directly, so holding the resolution constant is what keeps it stable).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Please update

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Done in 1dc653e. The source JSDoc no longer mentions max_num_ref_frames; it now just says the option keeps resolution constant and trades frame rate, as a workaround for decoders that glitch on a mid-stream resolution change. The mechanism (resolution change -> parameter sets re-emitted -> max_num_ref_frames moves with the frame size since it is bounded by the level's DPB capacity -> not settable from the browser, so holding resolution constant is the only lever) is now spelled out in the PR description instead.

* - 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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The 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).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Please make sure this update is applied

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Applied in 1dc653e. The type doc now leads with the intent: "Work around decoders that glitch on a mid-stream resolution change (seen with YouTube when the stream is restreamed) by holding the encoder at a constant resolution, giving up frame rate instead when bandwidth or CPU is constrained." The preference-not-guarantee caveat is kept. The changeset was reworded the same way.

Comment thread packages/millicast-sdk/src/Publish.js Outdated
Comment thread .changeset/olive-donkeys-shave.md Outdated
… decoder workaround

Set contentHint only after the duplicate-connect guard, so a rejected connect() no longer mutates a live track's hint. Drop the max_num_ref_frames detail from the JSDoc and describe the option by intent instead.

Co-Authored-By: Brendon Costa <brendon.costa@dolby.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant