Media: Video Segmentation Prototype #12335
Merged
Conversation
This file contains 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
timarney
added
Elements: Video
Group: Media
Group: Video Optimization
timarney
changed the title
timarney
force-pushed
the
fix/12164-video-segmentation
branch
10 times, most recently
from
1835ae0
to
8308177
Compare
timarney
changed the title
timarney
force-pushed
the
fix/12164-video-segmentation
branch
5 times, most recently
from
f0ebc4c
to
73348fd
Compare
|
@spacedmonkey @swissspidy --- adding this as ready for review now to get some feedback --- still testing on my end.
|
|
Plugin builds for 5fd476f are ready 🛎️!
|
|
Size Change: +1.41 kB (0%) Total Size: 2.72 MB
ℹ️ View Unchanged
|
timarney
force-pushed
the
fix/12164-video-segmentation
branch
from
73348fd
to
addcf91
Compare
For a later PR we could account for this type of thing when we calc the segment times Check the length of the last segment time and if <x tag it onto the previous segment time --- something like that would prevent videos of x length when doing the split. |
Re-tested with a few different videos 4-5 times worked ... the other time I didn't see any errors and no pages created. Possibly hitting the early return on the add pages code.
For sure need better UX feedback to show what's happening. |
|
@swissspidy @timarney I believe we should start by moving everything into a hook, the click is a light length as is. We can use trimming component as a base to copy from. |
|
Ah, good to know. Yeah we can do such a refactoring as a follow-up item. |
|
Anything else needed on this one given it's behind the feature flag? Follow-ups: State issue
End video issue (x sec last segment) UI Progress
|
|
Let's discuss this in our next meeting. |
|
I am not sure I want to approve this PR. Change the sidebar to another tab breaks it. At the very least, I would expect todos in the code, a callout in the testing / QA notes to say not to do this and a ticket explain how this functionality will be fixed in the future. Prototype that breaks this easily is going to make problems for those testing it and evaluating the functionality. I already used a lot of time debugging. |
This was referenced Oct 13, 2022
|
Added a QA note with the following issues for follow-up Also added a todo for the state issue on in the code |
|
Thanks for creating the follow-up tickets 👍 I think we're good to proceed here for now. |
swissspidy
approved these changes
Oct 13, 2022
swissspidy
added a commit
that referenced
this pull request
Oct 14, 2022
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com> Co-authored-by: Pascal Birchler <pascalb@google.com>
swissspidy
added a commit
that referenced
this pull request
Oct 21, 2022
* Improve taxonomies e2e tests * Add `jest.retryTimes` to every spec file * Further improve taxonomy tests * Improve hotlinking tests * Wrap all `page.waitForNavigation()` calls in `Promise.all` * Deactivate importer again after install * Remove `:focus-visible` polyfill Browser support has much improved this year, with Safari adding support in version 15.4 in March 2022. Safari 15.4 ships with the iOS and iPadOS 15.4 and macOS 12.3. * Dynamically import `@mediapipe/selfie_segmentation` Fixes #12383 * Re-enable admin menu e2e tests * Re-enable shopify test See #11994 * Re-enable WooCommerce test See #11976 * Re-enable story bg audio test See #12025 * Re-enable font check test See #11361 * Re-enable story deletion test See #11362 * Re-enable story saving failing test See #11992 * Re-enable quick edit test * Improve status check and re-enable status check tests See #11991 * Re-enable CORS check test See #11981 * Re-enable widget tests See #11990 See #6879 * Re-enable font check metrics test See #11978 * Make publishPost util more robust * Re-enable archive page tests See #9636 See #11990 * Re-enable block widget test See #11931 * Re-enable pending stories test See #11993 * Re-enable Site Kit analytics test See #9985 * Re-enable Web Stories block tests See #11975 See #6237 * Remove redundant test See #12023 * Re-enable shopping product test See #11989 * Re-enable dashboard keyboard navigation test See #11930 * Re-enable publisher logo test See #9152 * Add types for `expect-puppeteer` to improve IDE assistance * Re-enable checklist test See #11977 * Re-enable some Media3P tests See #7481 * Remove ext from import * Remove outdated export * Re-enable media library test See #7107 * Avoid using `undefined` as background image * Re-enable webm video tests See #11959 * Re-enable media insertion test See #7107 * Re-enable mov insertion test See #8232 * Re-enable pre-publish modal poster test See #7107 * Re-enable pre-publish modal poster test w/ contributor See #7107 * Remove useless telemetry banner test See #9619 * Do not prefetch colorthief chunk * Remove some debug cruft * Do not use `toMatchElement` return value * Delete also cropped media in cleanup * Improve addTextElement util * Improve previewStory util * Use sync fs api * Remove useless check * Do not use `toMatchElement` return value * Fix draft-js-export-html dependency entry * Add types for `expect-puppeteer` to improve IDE assistance * Do not fail fast for now * Update browserslist db (#12446) * Add missing await * Update to Puppeteer v18 * Improve custom font util * Bump eslint-plugin-jest from 27.0.4 to 27.1.1 (#12458) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump @rollup/plugin-json from 4.1.0 to 5.0.0 (#12457) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump semver from 7.3.7 to 7.3.8 (#12456) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Disallow closing story takeover dialog (#12440) * Bump eslint from 8.24.0 to 8.25.0 (#12455) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump @rollup/plugin-node-resolve from 14.1.0 to 15.0.0 (#12454) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump @rollup/plugin-url from 7.0.0 to 8.0.0 (#12452) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump css-minimizer-webpack-plugin from 4.2.0 to 4.2.1 (#12453) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump @typescript-eslint/parser from 5.39.0 to 5.40.0 (#12451) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump sirbrillig/phpcs-variable-analysis from 2.11.8 to 2.11.9 (#12450) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump github/codeql-action from 2.1.26 to 2.1.27 (#12448) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump actions/checkout from 3.0.2 to 3.1.0 (#12449) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Improve story locking dialog wording and layout (#12447) Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com> * Run tests against WordPress 6.1 RC (#12412) * [WIP] CI: Use composite actions (#12438) * Composer update * Add checkout step * Revert "Add checkout step" This reverts commit 07ef47b. * Revert "[WIP] CI: Use composite actions (#12438)" This reverts commit 6347baf. * Revert "Composer update" This reverts commit faab6ba. * Lint fix * Code Quality: Use `preloadImage` in `wp-dashboard` (#12466) * Code Quality: Fix `preloadImage` function signature (#12468) * Media: Video Segmentation Prototype (#12335) Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com> Co-authored-by: Pascal Birchler <pascalb@google.com> * Improve widget tests * Improve publisher logo test * Remove duplicate entry * Improve taxonomy test * Fix workflow * Fix fail-fast * Update lock file * Update dashboard test * Improve widget tests * Fix password protected story test * Improve createNewTerm util * Try improving `addCategory` util * Update to Puppeteer v19 * Improve `addTextElement` * Adjust `previewPage` * Try patching jest-environment-puppeteer * Update `previewStory` util * Improve addTextElement * Fix patch * Update patch * Fix zero width chars * Remove patch * Fix publishing flow test See #11970 * Update `addTextElement` * Refactor `deleteWidgets` * Adjust publisher logo test * Improve web stories block test Don’t use request interception for block test, fix AMP validation * Add env var * Add fix for REST API change in 6.1 See https://make.wordpress.org/core/2022/10/11/miscellaneous-rest-api-improvements-in-wordpress-6-1/ * Wait for dialog to be dismissed * Adjust author test * Add hardening * Update lock file * Adjust widget test * Adjust font check test * Increase retry times * Retry unit tests with AMP validation * A few focus-visible fixes * Fix selfie segmentation lazy load * More rigorously trash posts between tests Co-authored-by: Google for Creators Bot <94923726+googleforcreators-bot@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com> Co-authored-by: Tim Arney <timarney@users.noreply.github.com>
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.
Context
Automatically split videos into smaller parts.
Example: you upload a 2 minute video and we allow you to split it into 15 seconds long segments (so 8 parts in total), putting every segment on its own page.
For this prototype we can create a new feature flag and some simple button (and maybe a range slider to set the segmentation length) somewhere that triggers this action.
Summary
Adds a segment video panel under styles when you select a video
Pressing segment will split the selected video into parts based on the number of seconds selected
Relevant Technical Choices
To-do
User-facing changes
Testing Instructions
QA Notes added some known issues for follow-up
#12463
#12464
#12465
👉 Note: the video is segmented based on the videos keyframes i.e. using a 30 sec segment length will not result in exactly 30 secs segments but rather the segments will be based on the closet keyframe.
This PR can be tested by following these steps:
Reviews
Does this PR have a security-related impact?
No
Does this PR change what data or activity we track or use?
No
Does this PR have a legal-related impact?
No
Checklist
Type: XYZlabel to the PRFixes #12164