feat: BROS-461: Add playback speed params to video tag #8548
Conversation
Co-authored-by: andrew <[email protected]>
|
Cursor Agent can help with this pull request. Just |
✅ Deploy Preview for label-studio-docs-new-theme canceled.
|
✅ Deploy Preview for label-studio-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for heartex-docs canceled.
|
✅ Deploy Preview for label-studio-playground ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8548 +/- ##
===========================================
- Coverage 67.42% 60.60% -6.83%
===========================================
Files 780 543 -237
Lines 59888 38425 -21463
Branches 10147 10153 +6
===========================================
- Hits 40382 23289 -17093
+ Misses 19503 15133 -4370
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hlomzik
left a comment
hlomzik
left a comment
There was a problem hiding this comment.
We need to improve and simplify params handling in afterCreate()
docs/source/tags/video.md
Outdated
| ### Parameters | ||
|
|
||
| | Param | Type | Default | Description | | ||
| | --- | --- | --- | --- | | ||
| | name | `string` | | Name of the element | | ||
| | value | `string` | | URL of the video | | ||
| | frameRate | `number` | `24` | Video frame rate per second; can use task data like `$fps` | | ||
| | sync | `string` | | Object name to sync with | | ||
| | muted | `boolean` | `false` | Muted video | | ||
| | height | `number` | `600` | Height of the video player | | ||
| | timelineHeight | `number` | `64` | Height of the timeline with regions | | ||
| | defaultPlaybackSpeed | `number` | `1` | Default playback speed the player should start with when loaded | | ||
| | minPlaybackSpeed | `number` | `1` | Minimum allowed playback speed; defaultPlaybackSpeed cannot be set below this value | | ||
|
|
There was a problem hiding this comment.
Revert this change. Params table is auto-generated and stored in includes/tags/video.md
| const minPlaybackSpeed = Number(parseValue(self.minplaybackspeed, self.store.task?.dataObj)); | ||
|
|
||
| if (!defaultPlaybackSpeed || Number.isNaN(defaultPlaybackSpeed) || defaultPlaybackSpeed < 0.25) { | ||
| self.defaultplaybackspeed = "1"; |
There was a problem hiding this comment.
We should store internal lowcased params in their final state, so use only numbers here and further.
|
@cursor address comments |
|
I refactored the video playback speed attributes to use numeric types directly, enhancing validation logic and ensuring consistent data handling. This simplifies the codebase and improves the robustness of playback speed controls.
3 tasks completed
Learn more about Cursor Agents |
Co-authored-by: andrew <[email protected]>
|
Hi @cursor[bot]! Unfortunately you don't have membership in HumanSignal organization, Follow Merge dispatch is skipped. |
hlomzik
changed the title
|
/fmt |
|
/git merge
|
hlomzik
changed the title
Reason for change
This PR implements two new parameters for the
Videotag:defaultPlaybackSpeedandminPlaybackSpeed. This addresses an urgent request to allow users to define the initial playback speed and restrict the minimum adjustable speed for video labeling tasks.defaultPlaybackSpeed: Sets the initial playback speed when the video loads (default:1).minPlaybackSpeed: Defines the lowest speed a user can set (default:1).defaultPlaybackSpeedcannot be set below this value.Screenshots
No new visual UI components are introduced. The changes affect the functional behavior of the existing video player controls.
Rollout strategy
This is an additive feature with new parameters. It can be rolled out directly without feature flags.
Testing
The following scenarios were verified:
defaultPlaybackSpeed:defaultPlaybackSpeed="2". Verified the video starts at 2x speed.defaultPlaybackSpeed="0.5". Verified the video starts at 0.5x speed.minPlaybackSpeed:minPlaybackSpeed="1.5". Verified the playback speed slider and controls prevent going below 1.5x.minPlaybackSpeedvia console (if applicable) and confirmed it's constrained.defaultPlaybackSpeed="1"andminPlaybackSpeed="2". Verified the video starts at 2x speed (i.e.,defaultPlaybackSpeedis adjusted up tominPlaybackSpeed).defaultPlaybackSpeedandminPlaybackSpeedare present indocs/source/tags/video.mdwith correct descriptions and an example.tags.jsonincludes the new parameters with correct types and defaults.Risks
Low risk. This is an additive feature. Performance impact is negligible as it only affects initial state and playback speed calculations.
Reviewer notes
Video.js: Contains the core logic for parsing, validating, and applyingdefaultPlaybackSpeedandminPlaybackSpeedinafterCreate()andhandleSpeed().VideoConfigControl.tsx: The playback speed slider now respects theminSpeedprop passed from theVideotag.Video.jsthat ensuresdefaultPlaybackSpeedis not set belowminPlaybackSpeed.General notes
Example usage:
This will start the video at 2x speed, and the user can only slow it down to 1.5x.