Add custom video encoder factory support via SDK-owned protocols - #1109
Add custom video encoder factory support via SDK-owned protocols#1109hiroshihorie wants to merge 23 commits into
Conversation
Adds LiveKitSDK.set(videoEncoderFactory:) which accepts an implementation of the new public VideoEncoderFactory and VideoEncoder protocols instead of exposing LKRTCVideoEncoderFactory, keeping LiveKitWebRTC out of the public API surface. New public types mirror the WebRTC encoder interface: VideoCodecInfo, VideoEncoderSettings, VideoEncoderQpThresholds, EncodedVideoFrame and VideoEncoderStatus. Internal adapters bridge them to the RTC types, and the custom factory is wrapped in the simulcast adapter the same way the default factory is. Known gaps: the optional factory surface (encoderSelector, queryCodecSupport, implementations) is not bridged, and H265 codec-specific info falls back to generic on macOS because the prebuilt macOS slice does not ship RTCCodecSpecificInfoH265.h.
WebRTC 150.7871.01 ships the H265 codec specific info header in the macOS slice, so the generic fallback is no longer needed.
WebRTC assigns each frame an RTP timestamp in the 90kHz clock before it hands the frame to an encoder, and an encoded frame has to carry that same value back. VideoFrame only exposed the capture time in nanoseconds, so a custom encoder had no way to read it. Add rtpTimestamp to VideoFrame, populate it when converting from the WebRTC type, and write it back when converting to the WebRTC type so a frame that passes through a VideoProcessor keeps it. The existing initializer stays as is and defaults the value to 0.
Frames arrive as NV12 CVPixelBuffers and I420VideoBuffer has no public initializer, so an encoder that needs planar data had no way to get it. Add toI420() on VideoBuffer and VideoFrame, which copies the pixel data when the buffer is not already I420. I420VideoBuffer now holds the WebRTC I420 buffer protocol rather than its concrete class, since that is what the conversion returns, and gains width and height so the planes can be read without consulting the frame. Document the video buffer types and their accessors, which were public but undocumented.
The adapter handed the encoder a new closure on every setCallback call and captured the WebRTC block directly, so a cleared callback could still be invoked from an encoder thread. The block now lives in a small locked box, the encoder gets one stable forwarding closure, and releasing the encoder clears the box first. Frame types were built with compactMap, which silently shortened the array and misaligned the per stream requests. They are mapped one to one now, with anything unknown treated as a delta. A nil qp landed on the native side as 0, which reads as a perfect quantizer to the quality scaler, so it is now sent as -1 for unknown. The generic codec info object is allocated once instead of per frame, and supported codecs are computed once at factory construction. A custom factory is now paired with the built in encoders as the simulcast fallback, so an encoder reporting fallbackSoftware keeps the stream alive. A frame with a buffer the SDK cannot map reports that same status rather than an invalid parameter, which would drop every frame.
Fill in the pieces an encoder implementation needs and document the public types that were missing docstrings. VideoEncoderSettings gains a memberwise initializer so an encoder can be driven from a test. EncodedVideoFrame can now report the encode start and finish times and an NTP timestamp, which feed the encode time stats. VideoEncoderStatus gains the two remaining WebRTC codes and prints a readable name. VideoCodecInfo can resolve its SDP name to a VideoCodec. Setting a factory that advertises no codecs is now rejected up front rather than leaving video unpublishable, and the warning on the setter explains that any use of the SDK initializes the peer connection factory.
WebRTC only consults supports_native_handle when deciding whether to crop or scale a native frame for a simulcast layer. It never converts frames to I420 before handing them to the encoder, so the docstring now points encoders at toI420() instead.
WebRTC's frame encode metadata writer overwrites encode start and finish times, NTP time and the timing flags for every encoded image it matches to a source frame, and marks timing invalid otherwise. Values set by a custom encoder never reach stats or the wire, so the fields are removed rather than shipped as inert API.
The RTP packetizer for H264, VP8 and VP9 reads a typed video header that only exists when the encoded image carries matching codec specific info. A generic info object leaves the header empty and the packetizer aborts on the first frame. The adapter now synthesizes non interleaved H264 or H265 info from the codec the encoder was created for when a frame has none, and set(videoEncoderFactory:) rejects factories that advertise VP8 or VP9 since their layer info cannot be bridged yet. Also documents that a custom factory only takes over the codecs it lists, since the simulcast factory keeps advertising the built in ones.
WEBRTC_VIDEO_CODEC_TARGET_BITRATE_OVERSHOOT is 5, not -14. Adds the positive NO_OUTPUT and OK_REQUEST_KEYFRAME codes an encode call may return. Documents that the bridge always reports an alignment of 1 and hardware acceleration, and that startEncode may run before setCallback.
The underlying conversion only handles NV12, 32BGRA and 32ARGB. Other formats hit a debug assertion and return undefined pixel data in release builds, so toI420() now returns nil for them as its docstring promises.
The adapter forwarded whatever codec specific info an encoder returned. An H264 encoder returning H265 info left the RTP video header empty and the H264 packetizer aborted, the same path as the generic info case. The info is now always built for the codec the encoder was created for, taking only the packetization mode from the frame. The nil default also hardcoded non interleaved packetization. It now follows the packetization-mode parameter of the negotiated codec, so a factory that advertises mode 0 gets single NAL unit packetization.
The simulcast factory asks the primary factory to create an encoder for whatever codec was negotiated, including codecs only the built in fallback advertised. A custom factory that returned an encoder for VP8 in that situation would send generic codec info to the VP8 packetizer, which aborts. The adapter now declines any codec outside the validated list so the built in encoder takes it. The supported codec list is also read once in set(videoEncoderFactory:) and stored with the factory, so the list that was validated is the one advertised and enforced rather than a second read of a computed property.
set(videoEncoderFactory:) guarded on a flag that only the peer connection factory initializer set, but the encoder factory is a separate lazy static that captures the custom factory the moment it is resolved. Anything forcing it first, as CodecTests does, left a window where set() passed the guard, stored the factory and returned success while the built in encoders stayed in use. The encoder factory now records its own resolution in a dedicated flag and set() rejects on either. The existing flag keeps its meaning for the audio configuration guards, which read it as the peer connection factory and its audio module existing.
The block WebRTC hands the encoder captures a raw pointer to the native callback, so holding a lock while invoking it cannot extend that target's lifetime. It only narrowed the window while putting a non recursive lock around the whole packetize and send path, so release() and setCallback waited behind every frame. The box now uses StateSync, copies the block out under the lock and invokes it outside, keeping the stale callback drop and the single stable closure across consecutive registrations. The docstring also now says why the box exists: the simulcast adapter re-registers callbacks without an intervening nil when its stream contexts move, and that a re-attach after release() does hand the encoder a new closure.
The example advertised VP8, which set(videoEncoderFactory:) rejects, so copying it would throw invalidParameter.
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| // including ones only the built in fallback advertised. Declining here | ||
| // routes those to the fallback and keeps unbridgeable codecs such as VP8 | ||
| // away from the packetizer. | ||
| guard supportedCodecNames.contains(codec.name.uppercased()) else { return nil } |
There was a problem hiding this comment.
🟡 Unadvertised codec profiles reach custom encoders
When fallback and custom factories advertise different H264 profiles, supportedCodecNames accepts every negotiated H264 format. A name-only custom factory can encode an unsupported profile, producing an invalid stream.
Prompt for agents
Make VideoEncoderFactoryAdapter enforce the complete supported codec formats captured by LiveKitSDK.set(videoEncoderFactory:), rather than only uppercased names. H264 and H265 formats can share a name while differing in profile-level-id, packetization-mode, or other SDP parameters. The simulcast wrapper also advertises built-in fallback formats, so createEncoder can receive a fallback-only format with a custom-supported name. Match negotiated codec information against the custom snapshot using the same format compatibility semantics expected by WebRTC, while preserving case-insensitive codec-name handling and any valid negotiation adjustments.
Was this helpful? React with 👍 or 👎 to provide feedback.
pblazej
left a comment
There was a problem hiding this comment.
LGTM, I think the last one: #1109 (comment)
WebRTC fills capture time, rotation, content type, NTP time and encode timing from its own record of the source frame once the encoded image is matched by RTP timestamp, so values an encoder set were discarded. Those fields are removed from EncodedVideoFrame, which now carries only the data, dimensions, RTP timestamp, frame type, QP and an optional H264 packetization mode. The codec specific info enum is replaced by that single field since the adapter already knows the codec, and the H265 packetizer takes no mode. resolutionAlignment and applyAlignmentToAllSimulcastLayers are dropped from VideoEncoder because the ObjC bridge always reports an alignment of 1. VideoCodecInfo loses scalabilityModes because without a codec support query every mode is reported unsupported. AV1 leaves the bridgeable set since generic codec info cannot produce a dependency descriptor, so a custom factory may supply H264 and H265 only. All of this can return as additive changes once the fork carries the missing information, while removing it after release could not.
An H264 format advertised without packetization-mode negotiates as mode 0 on the wire, while frames without an explicit mode are packetized non interleaved. A factory advertising VideoCodecInfo(name: "H264") with no parameters, as the docstring example does, would therefore send STAP-A and FU-A on a payload the remote expects to be single NAL units. The codec list is normalized when the factory is set so the parameter is present, matching the built in factory which only advertises mode 1. Mode 0 remains available by advertising it explicitly.
The default codec support query ignores the scalability mode and reports supported, so the earlier comment gave the wrong reason for leaving the modes out. The changelog now states the H264 and H265 restriction and that the factory must be set before any other SDK API.
NO_OUTPUT, OK_REQUEST_KEYFRAME and TARGET_BITRATE_OVERSHOOT are used by decoders and libvpx internally, not by ObjC encoders. The stream encoder treats any non negative encode result as success, but the simulcast adapter returns early on anything other than OK, so a layer returning one of them would skip the remaining layers of that frame. Naming them invited that misuse.
Covers the pieces reviewers have been checking by hand: set() rejects empty and unbridgeable codec lists, H264 without packetization-mode is advertised as mode 1 while an explicit mode is kept, the factory adapter declines codecs outside the validated snapshot, frame type arrays keep their arity with unknown types mapped to delta, the callback closure is attached once per registration run and drops deliveries after release, and codec specific info follows the encoder's codec and the negotiated packetization mode.
A bare blank line inside the doc comment ended the block early, so only the tail of the example attached to the protocol.
|
@hiroshihorie Thanks for turning this around so fast. I ported our software openH264 encoder onto this branch and it fits the new protocols cleanly. Confirmed it's working end-to-end 👍 Both of the blockers from #1082 are gone:
A few things I ran into doing the port. 1. No way to make the custom factory exclusive This is the one that matters for us. Our receiver's hardware decoder can't handle VideoToolbox output, so publishing has to go through our encoder or not at all. Right now there are two ways back to VideoToolbox we can't turn off: it stays the simulcast fallback (so its H264 profiles are advertised alongside ours), and 842479f goes the other direction (declining codecs we didn't advertise), which is right for the VP8 abort but doesn't cover this. On our old fork we passed the custom factory as both primary and fallback. With the codec snapshot in place that's nearly a one-liner: 2. It says 3.
let i420 = frame.toI420()!
core.encode(y: i420.dataY, u: i420.dataU, v: i420.dataV, ...) // pointers may already be deadWe wrapped it in 4. Two heads-ups for anyone consuming this early We cherry-picked the PR onto the v2.16.0 tag rather than taking the branch, because two things on
Your commits cherry-pick onto the tag cleanly, so nothing here is a problem with the PR itself. 5. Small A line in the Happy to run any further change through the same device setup before it ships! |
Adds
LiveKitSDK.set(videoEncoderFactory:)with SDK-ownedVideoEncoderFactory/VideoEncoderprotocols, so apps can supply their own H264 or H265 encoder (e.g. a software openH264 encoder) withoutLiveKitWebRTCtypes in the public API. Supersedes #1082, thanks @sergeyphi for the PR and the detailed testing notes.Addresses the requests from that thread:
VideoFrame.rtpTimestampcarries the 90kHz RTP timestamp WebRTC assigns beforeencode, andEncodedVideoFrame.rtpTimestampmust be copied from it.VideoBuffer.toI420()/VideoFrame.toI420()give access to I420 planes for NV12 camera frames.supportsNativeHandleonly gates crop/scale in WebRTC, so this is the route to planar data.VideoEncoderSettingshas a public init for testing.FrameEncodeMetadataWriteroverwrites them for every frame it matches by RTP timestamp, so encoder-provided values never reach stats or the wire.Scope for v1, kept to what demonstrably reaches WebRTC (verified against the fork source):
RTCCodecSpecificInfobridge can carry, sosetrejects them. AV1 can follow once the bridge can emit a descriptor.EncodedVideoFramecarriesdata,dimensions,rtpTimestamp,frameType,qpand an optional H264packetizationMode. Capture time, rotation and content type are filled by WebRTC from its own record of the source frame, so they are not part of the type.packetization-mode; an H264 format advertised without that parameter is normalized to mode 1, matching the built in factory and how frames are packetized.VideoEncoderStatusnames only codes WebRTC acts on. Positive codes are omitted since the simulcast adapter stops encoding the remaining layers on anything other than OK.resolutionAlignmentandscalabilityModesare left out: the ObjC bridge always reports alignment 1 (fix in Honor resolutionAlignment and allow ObjC encoders to report software encoding webrtc-sdk/webrtc#289) and the bridge reports every encoder as hardware accelerated. Both return as additive changes once a WebRTC release carries the fixes..fallbackSoftware.setthrows if either the encoder factory or the peer connection factory has already been initialized.Testing: 12 unit tests exercise the bridge with fake factories and encoders (validation and normalization, codec gate, frame type arity, callback lifecycle, codec info selection). Compile verified on macOS and iOS Simulator. No real encoder has run through the path yet, and H265 is compile verified only. Swift only for now; ObjC encoders need a thin Swift shim.