Skip to content

Repair two libwebrtc patches that had stopped applying - #1329

Merged
pblazej merged 3 commits into
mainfrom
blaze/fix-drifted-webrtc-patches
Aug 14, 2026
Merged

Repair two libwebrtc patches that had stopped applying#1329
pblazej merged 3 commits into
mainfrom
blaze/fix-drifted-webrtc-patches

Conversation

@pblazej

@pblazej pblazej commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Both patches stopped applying to m144_release and were being skipped silently, because the build scripts apply patches with git apply ... || true — so the build succeeds with the feature missing. Surfaced by the webrtc-b9233c3 build, where the linux and android jobs failed.

external_audio_source.patch

The api/media_stream_interface.h hunk assumed options() was the last member of AudioSourceInterface:

   virtual const AudioOptions options() const;
+  virtual void SetOptions(const AudioOptions & /* options */) {}   <- added by webrtc-sdk/webrtc#247
 };

That landed on 8 June, so is_external_source() has been absent from shipped builds since.

jni_prefix.patch

The generated_peerconnection_jni hunk anchored on TurnCustomizer.java being the last entry of that source list; six entries have been appended since. Android-only, which is why it did not show up in the linux job. Re-anchored on the current last entry.

Verification

All twelve patches now apply against webrtc-sdk/webrtc@b9233c36a2, the commit the webrtc-b9233c3 build is running from. The five that report missing files target third_party/, build/ and buildtools/, which only exist after gclient sync.

Worth considering separately: dropping the || true would have surfaced the June regression the day it landed.

external_audio_source.patch stopped applying when webrtc-sdk/webrtc#247 added
SetOptions() to AudioSourceInterface in June; the hunk's context assumed
options() was the last member. jni_prefix.patch anchored on TurnCustomizer.java
being the final entry of generated_peerconnection_jni, and six sources have been
appended since.

Both are skipped silently today because the build scripts apply patches with
'|| true', so builds succeed with the features missing. Verified that all twelve
patches now apply against webrtc-sdk/webrtc@b9233c36a2, the commit the current
webrtc-b9233c3 build is running from.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pblazej
pblazej requested a review from ladvoc as a code owner August 14, 2026 11:41
@pblazej
pblazej requested a review from cloudwebrtc August 14, 2026 11:43
devin-ai-integration[bot]

This comment was marked as resolved.

This PR changes only the patch files consumed by the libwebrtc build. The crates
consume a prebuilt libwebrtc pinned by WEBRTC_TAG, which is unchanged here, so
merging this alters no published artifact. The changelog entry belongs on the
WEBRTC_TAG bump, where the fix actually reaches consumers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pblazej pblazej added the internal to tag changes that don't require changelog documentation label Aug 14, 2026
Adding SetOptions() as context grew the hunk to 7 pre-image and 12 post-image
lines, but the header still declared 6/11. git apply stops consuming the
fragment once the declared counts are exhausted, so the trailing context line
was being ignored rather than matched; GNU patch only applied it with fuzz.

Verified that header and body now agree for every hunk in all twelve patches,
and that 'git apply --numstat' matches '--numstat --recount' for both edited
patches.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

--- a/api/media_stream_interface.h
+++ b/api/media_stream_interface.h
@@ -267,6 +267,11 @@ class RTC_EXPORT AudioSourceInterface : public MediaSourceInterface {
@@ -269,7 +269,12 @@ class RTC_EXPORT AudioSourceInterface : public MediaSourceInterface {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Pull request ships without a changeset entry

The change is merged without the required changeset file (the branch's .changeset/fix_drifted_webrtc_patches.md was deleted in commit 20ac411) even though the repository rules require one for every pull request, so the change lands undocumented.
Impact: The fix is not recorded in release notes and version bumps may be missed.

Repository rule requiring a changeset for every PR

AGENTS.md states under "Documenting changes": "Every PR needs a changeset" and "Changeset must list any crates which need to be bumped stemming from the change". The branch added .changeset/fix_drifted_webrtc_patches.md in commit 28ce6da and then removed it in commit 20ac411, leaving .changeset/ with only unrelated entries (add_caching_to_livekit_token_source_crate.md, automatically_retry_webrtc_build_downloads.md). Even if no published artifact changes, the rule as written has no exemption; if an exemption is intended it should be added to AGENTS.md.

Prompt for agents
AGENTS.md requires every PR to include a changeset (created via `knope document-change` or manually under `/.changeset`), listing crates that need bumping. This PR originally had `.changeset/fix_drifted_webrtc_patches.md` but it was removed in commit 20ac4114 with the rationale that only libwebrtc patch files changed and WEBRTC_TAG is unchanged. Either restore a changeset for this change, or update AGENTS.md to document an explicit exemption for build-only/patch-only changes so the rule and practice stay consistent.
Open in Devin Review

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

@pblazej
pblazej merged commit 9e3b4f8 into main Aug 14, 2026
24 checks passed
@pblazej
pblazej deleted the blaze/fix-drifted-webrtc-patches branch August 14, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal to tag changes that don't require changelog documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants