Skip to content

feat(libwebrtc): return from five calls as firmware 13.42 does - #15609

Open
BunnyFl wants to merge 7 commits into
boykopovar:mainfrom
BunnyFl:feat/libwebrtc-free-trivial-returns
Open

BunnyFl wants to merge 7 commits into
boykopovar:mainfrom
BunnyFl:feat/libwebrtc-free-trivial-returns

Conversation

@BunnyFl

@BunnyFl BunnyFl commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

What

Five libwebrtc exports threw. In firmware 13.42 each one returns at once, and this change does the same:

Export NID Signature Returns
webrtc::PacketOptions::~PacketOptions() (D1) 3yjhuJS+93s void (void* self) nothing
webrtc::rtcp::Pli::~Pli() (D1) OWYOJU0yBRY void (void* self) nothing
webrtc::AudioEncoder::OnReceivedUplinkRecoverablePacketLossFraction(float) JBEfUOB-Xa8 void (void* self, float) nothing
webrtc::VideoFrameBuffer::GetI420() const ILLU2NiyjU8 const void* (const void* self) nullptr
unnamed libwebrtcUnknown37 Vf6lJz4-bEU int (void), unverified 0

The parameters come from the mangled names. The return types come from WebRTC 8d78f5de6c, the revision docs/dev/debt/libwebrtc.md already cites:

  • api/video/video_frame_buffer.h:91 declares virtual const I420BufferInterface* GetI420() const, and video_frame_buffer.cc:39 returns nullptr.
  • api/audio_codecs/audio_encoder.cc:94 is void AudioEncoder::OnReceivedUplinkRecoverablePacketLossFraction(float). Its body is only RTC_DCHECK_NOTREACHED(), which a release build leaves out; the firmware body at 0x3da0e0 (below) is a lone ret.

The declarations in core/libs/prx/libwebrtc/include/Api.hpp change with them.

In docs/dev/debt/libwebrtc.md, the inventory entry goes to 325 throwing exports (59 without a name), counted from the merged Export.cpp. main 56374036ad states 334 (60) but has 330 throwing exports: #14246 moved four FileWrapper exports to src/FileWrapper.cpp without lowering the count, so the 325 also corrects that. A new entry says that Vf6lJz4-bEU has no known name or signature, and that the values are read from disassembly, not measured on a console.

Nine more libwebrtc exports with the same kind of body are left out of this PR. Open PRs already implement them: #14224 (RmsLevel), #15193 (AudioNetworkAdaptorImpl::Config), and #15265 to #15269 and #15272 (GetMdnsResponder, SetFecControllerOverride, and the PacketInfo, RateControlParameters, AudioEncoderRuntimeConfig and SmoothingFilterImpl destructors).

Source

Firmware 13.42 system/common/lib/libwebrtc.sprx (sha256 d59d0caf6967329b39201a2a0bd6df7b994516f21701b9801908db6735ae5314), read from disassembly, not run on a console. Export addresses come from the module's dynamic symbol table. Each body, to its return:

3ece00: ret                       ; webrtc::PacketOptions::~PacketOptions (D1)
3c6480: ret                       ; webrtc::rtcp::Pli::~Pli (D1)
3da0e0: ret                       ; AudioEncoder::OnReceivedUplinkRecoverablePacketLossFraction
150e10: xor %eax,%eax ; ret       ; VideoFrameBuffer::GetI420
3da0b0: xor %eax,%eax ; ret       ; Vf6lJz4-bEU

None of the five bodies above reads an argument or stores anything. A null or invalid this is never dereferenced, here or in firmware.

Tested

  • Merged main 56374036ad into the branch; the head is now 4886fe077f. The only conflict was the inventory count in docs/dev/debt/libwebrtc.md, which feat(libwebrtc): implement the rtc::SocketAddress port, ipaddr, IsAnyIP, IsComplete and IsNil accessors #15261 had lowered to 334 when it implemented five rtc::SocketAddress accessors. The merged number is counted from the merged Export.cpp: 325 exports still throw through NotImplemented_nid_no_patch, 59 of them bound by NID only. The same count gives 330 and 60 on main 56374036ad.
    • On 4886fe077f, Windows 11 Pro 10.0.26300, WinLibs GCC, Release: 85 of 85 pass, none Not Run, with the 20 guest_webrtc_* test executables rebuilt (now including socket_address, added on main by feat(libwebrtc): implement the rtc::SocketAddress port, ipaddr, IsAnyIP, IsComplete and IsNil accessors #15261), plus public_headers and the 64 public_headers_compile_* batches.
    • The branch's added and removed lines against main are the same as at e0c8c01d20 apart from the count line, so the negative control below was not repeated.
    • tools/check_conventions.py --base upstream/main on the committed head 4886fe077f: all checks pass.
  • Windows 11 Pro 10.0.26300, WinLibs GCC, Release, head e0c8c01d20 (main c9f43dfa1d merged in): 84 of 84 pass, none Not Run, with the 19 test executables built at this head. The suite is the 19 guest_webrtc_* tests registered at this head - file_wrapper (added on main by feat(libwebrtc): implement FileWrapper output and ownership #14246) and audio_codec_info, audio_decoder, audio_device_generic, audio_encoder, audio_frame, cpu_features, exp_filter, fast_int_to_buffer, ip_address, opus, recursive_critical_section, rms_level, rtcp_common_header, rtcp_pli, task_queue_base, time, trivial_returns and vad - plus public_headers and the 64 public_headers_compile_* batches.
  • New core/libs/prx/libwebrtc/tests/TrivialReturns.cpp:
    • It calls the five and checks the two return values.
    • It checks that the object buffers passed as this are unchanged.
  • Negative control (run at the earlier head 2364923fa6; the branch's diff against main differs between that head and e0c8c01d20 only in the count at docs/dev/debt/libwebrtc.md:5 and one blank line main left after the FileWrapper entry): I put NotImplemented_nid_no_patch back at the top of the five bodies, which is main's behaviour, and kept this branch's signatures and test. The new test then aborts with _ZN6webrtc13PacketOptionsD1Ev_nid_postfix not implemented (1 of 1 failed).
  • tools/check_conventions.py --base upstream/main on the committed head e0c8c01d20: all checks pass.
  • tools/pr_overlap.py --branch feat/libwebrtc-free-trivial-returns --jobs 8 at head e0c8c01d20, against 1,901 open pull requests: 33 share files with the branch. It lists 28 with conflicting lines, all in docs/dev/debt/libwebrtc.md: 27 on the inventory count entry (line 5) alone and feat(libwebrtc): implement VideoFrame width and height #15766 on lines 5 and 13. No row reports a shared export or a shared new file. The run failed closed (exit 2) and lists 3 more (docs(debt): separate entries with blank lines and drop derived export totals #14212, feat(libwebrtc): implement CurrentThreadId #15154, feat(libwebrtc): implement the webrtc::RtpExtension destructor #15264) in its "Some comparisons could not be completed" table, which it does not compare line by line. I grepped the added and removed lines of all 31 listed pull requests' diffs for the five exports' names and NIDs, and none of them names any of the five. Whichever lands second recounts the inventory entry.
  • The merge of main c9f43dfa1d conflicted only next to the new debt entry. The merged number is counted from the merged Export.cpp rather than subtracted: 330 of its 338 APS5_VABI entries still throw through NotImplemented_nid_no_patch, 59 of them bound by NID only. The same count gives 335 of 338 and 60 on main c9f43dfa1d.
  • Linux and macOS were not built or run. No title was run.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

🤖 Generated with Claude Code

The PacketOptions and rtcp::Pli destructors and
OnReceivedUplinkRecoverablePacketLossFraction return at once,
VideoFrameBuffer::GetI420 returns null and Vf6lJz4-bEU returns 0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Progress report (b2e43d5 → 4886fe0)

📈 System libraries: 30.38% (+0.01%, +5 implemented functions)

✅ 5 implemented
Library Name
libwebrtc _ZN6webrtc12AudioEncoder45OnReceivedUplinkRecoverablePacketLossFractionEf_nid_postfix
libwebrtc _ZN6webrtc13PacketOptionsD1Ev_nid_postfix
libwebrtc _ZN6webrtc4rtcp3PliD1Ev_nid_postfix
libwebrtc _ZNK6webrtc16VideoFrameBuffer7GetI420Ev_nid_postfix
libwebrtc libwebrtcUnknown37
⚪ 5 behaviourally empty (constant return or throw)
Library Name
libwebrtc _ZN6webrtc12AudioEncoder45OnReceivedUplinkRecoverablePacketLossFractionEf_nid_postfix
libwebrtc _ZN6webrtc13PacketOptionsD1Ev_nid_postfix
libwebrtc _ZN6webrtc4rtcp3PliD1Ev_nid_postfix
libwebrtc _ZNK6webrtc16VideoFrameBuffer7GetI420Ev_nid_postfix
libwebrtc libwebrtcUnknown37

Changed libraries (implemented exports)

Library Before After Change
libwebrtc 81/412 (19.66%) 86/412 (20.87%) +1.21%

…-trivial-returns

# Conflicts:
#	docs/dev/debt/libwebrtc.md

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Nov 5, 2026
Resolve the docs/dev/debt/libwebrtc.md conflict. Both sides changed the
export count in the inventory entry title and nothing else on that line:
main reads 346 exports with 60 unnamed, this branch read 342 with 59.
Counted from the merged Export.cpp instead of subtracting, 341 of its
349 APS5_VABI entries still throw through NotImplemented_nid_no_patch,
59 of those bound by NID only, so the entry now reads 341 and 59.

The merge also drops this branch's throwing stub of
webrtc::rtcp::Pli::Parse, which main removed when boykopovar#15092 implemented it:
the merged tree declares it once in include/Api.hpp and defines it in
src/RtcpPli.cpp. This branch's five returning entries are unaffected,
among them the Pli destructor, which is a different export.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Nov 6, 2026
The debt inventory count conflicted: main 340 (60 NID-only) after six
RecursiveCriticalSection exports moved out of Export.cpp, ours 341 (59)
after five immediate returns. Counted from the merged tree with a
brace-matching parser: 335 throwing exports, 59 of them NID-only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Nov 6, 2026
Recount the throwing exports from the merged tree: 334, 59 of them bound by NID.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Nov 6, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Nov 6, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Nov 6, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Nov 6, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Nov 6, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build docs needs rebase Conflicts with main, rebase needed prx

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant