-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix: FIT-31: Ensure media buffers are syncable events that keep audio and video in sync #7633
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
Open
bmartel
wants to merge
28
commits into
develop
Choose a base branch
from
fb-fit-31
base: develop
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.
+353
−59
Conversation
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
… and video in sync
…ntrol spacing on video
✅ Deploy Preview for heartex-docs canceled.
|
✅ Deploy Preview for label-studio-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for label-studio-playground ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for label-studio-docs-new-theme canceled.
|
/fm sync |
…ly report to the sync group and cause over buffering
bmartel
commented
May 30, 2025
bmartel
commented
May 30, 2025
hlomzik
reviewed
May 30, 2025
yyassi-heartex
approved these changes
May 30, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Previously we were not syncing when a media source would buffer, which led to out of sync audio and video in many cases, especially in spotty network conditions or with very large file sizes. This addresses that issue by syncing buffering state events into SyncManager, and handling the state in the particular cases Audio, Video, Paragraphs. Also updated and aligned the buffering visuals by moving that into the TimelineControls.
This video showcases a ~300MB video throttled to 3G then seeked and played, and later unthrottled to simulate what was previously causing media sources to go out of sync with one another:
Kapture.2025-05-28.at.08.47.55.webm
Feature Flag:
FF_SYNCED_BUFFERING
to enable synchronized media buffering across components. (web/libs/core/src/lib/utils/feature-flags/flags.ts
)UI Updates:
Timeline
component's controls, including new SCSS styles and animations. (web/libs/editor/src/components/Timeline/Controls.scss
) [1] [2]VideoCanvas
component to conditionally display a buffering indicator based on the buffering state. (web/libs/editor/src/components/VideoCanvas/VideoCanvas.scss
,web/libs/editor/src/components/VideoCanvas/VideoCanvas.tsx
) [1] [2]Synchronized Buffering Logic:
useSyncedBuffering
hook to manage synchronized buffering states across components using Jotai atoms. (web/libs/editor/src/hooks/useSyncedBuffering.ts
)VideoCanvas
andHtml5Player
components to utilize the synchronized buffering logic, including debounced state updates and conditional behavior based on the feature flag. (web/libs/editor/src/components/VideoCanvas/VideoCanvas.tsx
,web/libs/editor/src/lib/AudioUltra/Controls/Html5Player.ts
) [1] [2]Playback Logic Enhancements:
Player
class to include abuffering
property and handle buffering events (canplay
andwaiting
) with debounced updates. (web/libs/editor/src/lib/AudioUltra/Controls/Player.ts
) [1] [2]Timeline
andVideoCanvas
components to propagate buffering states through their props and interfaces. (web/libs/editor/src/components/Timeline/Types.ts
,web/libs/editor/src/components/Timeline/Timeline.tsx
) [1] [2]These changes collectively improve the user experience by providing better visual feedback during buffering and ensuring consistent playback behavior across components.