feat: LEAP-1367: Add TimelineLabels to mark parts of the video #6191
Conversation
- new control tag TimelineLabels with corresponding result type - new TimelineRegion to store values in `ranges` field - new data format, shorter than `sequence` to represent timespans - adjustments to Video tag to support new kind of results - UX draft to draw timeline regions at any point, will definitely be improved - fix deserialization to properly read new results
Now they are timespans with distinct borders
Video tag has `mergeLabelsAndResults: true` so it created extra `labels` property, messing with results. Now there are 3 cases: - usual tags and regions: all results - video with rectangles: labels added to rectangles - video with timeline labels: usual labels result with extra values
- fix types - remove excess hooks - react on video region changes
✅ Deploy Preview for heartex-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for label-studio-docs-new-theme ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Add it when the label is selected but no regions or selected, so user has intent to create a new region with label.
…nto fb-leap-1362/timeline-regions
|
/git merge develop
|
Split controls, detect correct video region type, etc.
New region was created every time user worked with video rectangle on timeline. That was because timeline behaviour kicked in even without proper control in place.
|
/git merge
|
Much better check for not being on labels area. Plus no excess check for classname, it's already checked by `!regionRow`
Create regions only when labels are selected.
Apply labels in a regular way via `resultValue` in `createResult()`. Respect “select region after” and “keep label selected” settings. Add comments about multi-labels.
Toggle Keypoint and Toggle Interpolation are disabled for them.
To not break the usual flow with video rectangles.
|
/git merge
|
- fix usage of wrong units: - positions are 1-based - offsets are 0-based, so in calculations positions should be deducted by 1 - positions are offsets * steps, they were mixed up together - calculate start and end of currently visible window correctly to hop back and forth without skipping frames - fix conditions for hopping, hop back was unreachable - fix scrubbing on random page when it's not aligned with `framesInView`; jump to next page only if we are moving outside of current view - fix value stored as previous scroll to be the correct unit - don't reset current position when video is not fully loaded - add more meaningful comments
|
/git merge
|
Fix region type detection in our crazy dispatcher in Area.
We don't focus on half-visible frames now, so that had to be fixed. Plus now test logic is more flexible and bulletproof.
hlomzik
changed the title
|
/git merge
|
|
/docker build
|
|
@ellipsis-dev review this |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to f1272b6 in 51 seconds
More details
- Looked at
1159lines of code in22files - Skipped
1files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. web/libs/editor/src/components/Timeline/Timeline.tsx:6
- Draft comment:
ThefixMobxObservefunction is imported but not used. Consider removing it to clean up the code. - Reason this comment was not posted:
Confidence changes required:50%
InTimeline.tsx, thefixMobxObservefunction is imported but not used. This is unnecessary and should be removed to clean up the code.
Workflow ID: wflow_IQNeJsSffboi4oVH
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 6 days left in your free trial, upgrade for $20/seat/month or contact us.
|
|
||
| if (!isDefined(start)) { | ||
| if (!isDefined(end)) return []; | ||
| return { frame: end, enabled: false }; |
There was a problem hiding this comment.
The rangeToSequence function returns an object when only end is defined, but it should return an array to maintain consistency with other return types. Consider returning an array with a single object instead.
| @@ -172,6 +174,8 @@ const TimelineComponent: FC<TimelineProps> = ({ | |||
| </Elem> | |||
| ); | |||
|
|
|||
| regions.map((reg) => fixMobxObserve(reg.sequence)); | |||
There was a problem hiding this comment.
The regions.map call on this line is not used and can be removed to improve performance.
rangesfieldsequenceto represent time spansVideo timelineHeightparam to adjust the timeline areaPR fulfills these requirements
Describe the reason for change
This is a helpful feature, requested for a long time. This first iteration is focused on instant and short time spans with not so many expected regions for annotation.
What alternative approaches were there?
per-frame choices, which are less flexible and have a worse fit into our architecture.
What feature flags were used to cover this change?
none, this feature considered an add-on, not changing much any core and existing functionality.
Does this PR introduce a breaking change?
What level of testing was included in the change?
tests will follow
Summary:
Introduced
TimelineLabelsfor video annotation, enabling frame classification and timeline region marking, with updates across models, components, and documentation.Key points:
TimelineLabelstag for video frame classification indocs/source/tags/timelinelabels.md.TimelineRegionmodel inweb/libs/editor/src/regions/TimelineRegion.jsto store time ranges.Videotag inweb/libs/editor/src/tags/object/Video/Video.jsto supporttimelineHeightparameter.Timelinecomponent inweb/libs/editor/src/components/Timeline/Timeline.tsxto handle new drawing events.Framesview inweb/libs/editor/src/components/Timeline/Views/Frames/Frames.tsxto support timeline region drawing.Resultmodel inweb/libs/editor/src/regions/Result.jsto includetimelinelabels.Labelcomponent inweb/libs/editor/src/tags/control/Label.jsxto supportTimelineLabels.web/libs/editor/tests/e2e/tests/regression-tests/video-timeline-seek-indicator.test.js.Generated with ❤️ by ellipsis.dev