Adding trimmer functionality to media recording #11908
Conversation
barklund
added
Type: Enhancement
|
Size Change: +615 B (0%) Total Size: 2.66 MB
ℹ️ View Unchanged
|
|
Plugin builds for 9c79704 are ready 🛎️!
|
|
Might be connected to that stale
If I wait it seems ok? Play(/pause) in preview (without touching trimming) doesn't work (works on main). |
barklund
changed the title
|
The remaining issues are both very straight-forward to understand and test around, but also pretty annoying to fix. Well, the first one with the preview in the media recorder is fairly easy, I can copy the code from the trimmer to start and stop the video at the right frame offsets, but manipulating the video in the preview on canvas seems a lot harder (and even more code to duplicate), and the frame snapshot too. I'm considering doing only the first issue in this version and filing the other two tickets as followup. The latter at least seem better suited for WP pod, as this thing is in their domain. But please, @merapi and @miina, check it out as-is both the result and the code and give me a review. I think this is solid for bug bash at least. Finally, we could also file a follow-up bug to deal with the duration only coming after the video has played through once making trimming not work until then. We could use the duration (in seconds only though) stored inside the recorder provider until a more precise time is known. We could probably also extract the exact time from the blob somewhere and use that value. Or if we can't do either of those, we could at least disable the trim button until it's available after the first play-through. |
| </DisplayPageArea> | ||
| <Footer captureImage={debouncedCaptureImage} videoRef={videoRef} /> | ||
| <StyledFooter showOverflow> | ||
| {isTrimming ? <VideoTrimmer /> : <Footer />} |
There was a problem hiding this comment.
Maybe we should display a loading indicator here until we have the duration when the trim icon is clicked?
There was a problem hiding this comment.
Yeah, we should definitely do something here. I have some ideas for how to improve, but it's a based on technical limitations so hard to solve well in UX.
Wondering if it would make sense to upload the video in full length and do the trimming the same way as in case of "normal" video elements, and just remove the video if dismissed / not inserted? Or keeping it as the parent video.
Mentioned it in the comment as well but maybe we could just display a loading indicator after clicking on the Trim icon? Otherwise the user might get the impression that trimming is not possible at all, if it's disabled. Thoughts?
SGTM if @merapi is ok with it. |
| args.resource.lengthFormatted = length | ||
| ? lengthFormatted | ||
| : getVideoLengthDisplay(duration); | ||
| args.resource.length = duration; |
There was a problem hiding this comment.
We should round this to a whole number.
There was a problem hiding this comment.
This is rounded, AFAIK. The original value here comes from the recording counter (which just counts seconds), and when trimming, the value is updated to the ceil() of the trim interval.
packages/story-editor/src/components/videoTrim/recordingProvider.js
Outdated
Show resolved
Hide resolved
# Conflicts: # packages/story-editor/src/app/highlights/quickActions/useQuickActions.js # packages/story-editor/src/components/canvas/mediaRecordingLayer.js # packages/story-editor/src/components/mediaRecording/footer.js # packages/story-editor/src/components/mediaRecording/index.js
|
Sharing comment for visibility. #11887 (comment) |
This PR only covers trimming for videos, not for audio files. In the future we may consider adding an audio trimmer though, but that requires UX and prioritization. |
packages/story-editor/src/components/mediaRecording/mediaRecording.js
Outdated
Show resolved
Hide resolved
|
Trimming works for gifs and video; |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
When I insert the video into canvas, the time is wrong.
|
Screen.Recording.2022-07-21.at.14.00.29.movOn a 40 second video, it takes nearly 40 seconds for the trim option to show up. |
packages/story-editor/src/components/mediaRecording/playPauseButton.js
Outdated
Show resolved
Hide resolved
…rs/web-stories-wp into add/11685-trim-recording
Context
This refactors both the existing trimmer and the media recording system to be able to merge the two concepts.
Due to the necessary extra provider, the media recording has been split out from the media recording layer. This will probably cause conflicts with the audio work.
To-do
videoNodeis stale and things fail.User-facing changes
TBD
Testing Instructions
This PR can be tested by following these steps:
Please note known bugs above.
Checklist
Type: XYZlabel to the PRFixes #11685
Fixes #11922