-
Notifications
You must be signed in to change notification settings - Fork 253
User Timestamp Parsing #1812
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
Draft
chenosaurus
wants to merge
7
commits into
main
Choose a base branch
from
dc/feature/user_timestamp
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+483
−26
Draft
User Timestamp Parsing #1812
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
41e5ec1
wip adding user timestamp extraction
chenosaurus 4aab3e5
wip
chenosaurus e3c48d3
add css class
chenosaurus df6c1d8
eslint
chenosaurus 9db706b
Merge branch 'main' into dc/feature/user_timestamp
chenosaurus aec3263
fix worker import
chenosaurus 7db8729
Merge branch 'dc/feature/user_timestamp' of github.com:livekit/client…
chenosaurus File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,13 +36,13 @@ | |
| import { EncryptionEvent } from '../e2ee'; | ||
| import { type BaseE2EEManager, E2EEManager } from '../e2ee/E2eeManager'; | ||
| import log, { LoggerNames, getLogger } from '../logger'; | ||
| import type { | ||
| InternalRoomConnectOptions, | ||
| InternalRoomOptions, | ||
| RoomConnectOptions, | ||
| RoomOptions, | ||
| } from '../options'; | ||
| import { getBrowser } from '../utils/browserParser'; | ||
|
Check failure on line 45 in src/room/Room.ts
|
||
| import { BackOffStrategy } from './BackOffStrategy'; | ||
| import DeviceManager from './DeviceManager'; | ||
| import RTCEngine from './RTCEngine'; | ||
|
|
@@ -74,7 +74,12 @@ | |
| import LocalVideoTrack from './track/LocalVideoTrack'; | ||
| import type RemoteTrack from './track/RemoteTrack'; | ||
| import RemoteTrackPublication from './track/RemoteTrackPublication'; | ||
| import RemoteVideoTrack from './track/RemoteVideoTrack'; | ||
| import { Track } from './track/Track'; | ||
| import { stripUserTimestampFromEncodedFrame } from '../user_timestamp/UserTimestampTransformer'; | ||
| //@ts-ignore | ||
| import UserTimestampWorker from '../user_timestamp/userTimestamp.worker?worker'; | ||
| import { isScriptTransformSupported } from '../e2ee/utils'; | ||
| import type { TrackPublication } from './track/TrackPublication'; | ||
| import type { TrackProcessor } from './track/processor/types'; | ||
| import type { AdaptiveStreamSettings } from './track/types'; | ||
|
|
@@ -93,6 +98,7 @@ | |
| getDisconnectReasonFromConnectionError, | ||
| getEmptyAudioStreamTrack, | ||
| isBrowserSupported, | ||
| isChromiumBased, | ||
| isCloud, | ||
| isLocalAudioTrack, | ||
| isLocalParticipant, | ||
|
|
@@ -2177,6 +2183,13 @@ | |
| track.on(TrackEvent.VideoPlaybackFailed, this.handleVideoPlaybackFailed); | ||
| track.on(TrackEvent.VideoPlaybackStarted, this.handleVideoPlaybackStarted); | ||
| } | ||
| // When e2ee is NOT enabled, set up a standalone insertable-streams | ||
| // transform to extract LKTS user timestamp trailers from inbound | ||
| // video frames. When e2ee IS enabled, the FrameCryptor worker | ||
| // handles this before decryption. | ||
| if (!this.hasE2EESetup && track instanceof RemoteVideoTrack && track.receiver) { | ||
| this.setupUserTimestampTransform(track); | ||
| } | ||
| this.emit(RoomEvent.TrackSubscribed, track, publication, participant); | ||
| }, | ||
| ) | ||
|
|
@@ -2591,6 +2604,69 @@ | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Sets up an insertable-streams transform on a remote video track's receiver | ||
| * to strip LKTS user timestamp trailers. Used only when e2ee is NOT enabled | ||
| * (when e2ee is enabled, the FrameCryptor worker handles this). | ||
| * | ||
| * Uses RTCRtpScriptTransform on browsers that support it (non-Chromium), | ||
| * and falls back to createEncodedStreams (legacy insertable streams) on | ||
| * Chromium where encodedInsertableStreams is enabled on the PeerConnection. | ||
| */ | ||
| private setupUserTimestampTransform(track: RemoteVideoTrack) { | ||
| const receiver = track.receiver; | ||
| if (!receiver) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| if (isScriptTransformSupported() && !isChromiumBased()) { | ||
| const worker = new UserTimestampWorker(); | ||
| worker.onmessage = (ev: MessageEvent) => { | ||
| if (ev.data?.kind === 'userTimestamp' && typeof ev.data.timestampUs === 'number') { | ||
| track.setUserTimestamp(ev.data.timestampUs, ev.data.rtpTimestamp); | ||
| } | ||
| }; | ||
| // @ts-ignore | ||
| receiver.transform = new RTCRtpScriptTransform(worker, { kind: 'decode' }); | ||
| return; | ||
| } | ||
|
|
||
| // @ts-ignore | ||
| if (typeof receiver.createEncodedStreams === 'function') { | ||
| // @ts-ignore | ||
| const { readable, writable } = receiver.createEncodedStreams(); | ||
|
|
||
| const transformStream = new TransformStream<RTCEncodedVideoFrame, RTCEncodedVideoFrame>({ | ||
| transform: (encodedFrame, controller) => { | ||
| try { | ||
| const result = stripUserTimestampFromEncodedFrame(encodedFrame); | ||
| if (result !== undefined) { | ||
| track.setUserTimestamp(result.timestampUs, result.rtpTimestamp); | ||
| } | ||
| } catch { | ||
| // Best-effort: never break the media pipeline if parsing fails. | ||
| } | ||
| controller.enqueue(encodedFrame); | ||
| }, | ||
| }); | ||
|
|
||
| readable | ||
| .pipeThrough(transformStream) | ||
| .pipeTo(writable) | ||
| .catch(() => {}); | ||
| return; | ||
| } | ||
|
|
||
| this.log.debug( | ||
| 'user timestamp transform: no supported API available', | ||
| this.logContext, | ||
| ); | ||
| } catch { | ||
| // If anything goes wrong (e.g., unsupported environment), just skip. | ||
| } | ||
| } | ||
|
|
||
| // /** @internal */ | ||
| emit<E extends keyof RoomEventCallbacks>( | ||
| event: E, | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Check warning
Code scanning / CodeQL
Unsafe HTML constructed from library input Medium
Copilot Autofix
AI 14 days ago
General approach: Avoid constructing HTML with untrusted values via
innerHTMLand instead either (1) construct the DOM tree withdocument.createElement/appendChildwhile assigning IDs directly as properties, or (2) sanitize/escape untrusted data before inserting into HTML. Since the only dynamic piece here isidentityused for element IDs, the safest, least invasive fix is to build the elements programmatically and assignid/classNamevia DOM properties, which do not interpret the string as HTML.Best way to fix this case without changing functionality:
div.innerHTML = \...`` with code that:video,audio,div,span,progress/inputelements viadocument.createElement.id,className, inlinestyle, and other attributes using properties orsetAttribute.isLocalParticipant(participant)check to decide whether to append avolume-control<div>with an<input>range or a<progress>element.#video-${identity},#user-ts-${identity}, etc.) continues to work.Room.ts,Participant.ts,RemoteParticipant.ts, orLocalParticipant.ts; CodeQL’s taint traces into those files will be cut once theinnerHTMLsink is removed.Concretely:
examples/demo/demo.ts, insiderenderParticipant, replace lines 833–863 (the template literal assigned todiv.innerHTML) with imperative DOM creation code as described above.No new helper methods or imports are required; we can use the standard DOM API already available.