feat(playback): restart from beginning when play pressed at end of timeline (Fixes #536)#542
Conversation
|
@saiteja-in is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
👷 Deploy request for appcut pending review.Visit the deploys page to approve it
|
WalkthroughUpdated play() in playback-store to compute effective timeline duration using timeline store and project FPS, and if current time is at or beyond end minus one frame, it seeks to 0 before starting playback, then sets isPlaying and starts the timer. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PlaybackStore
participant TimelineStore
participant Project as Project/FPS
User->>PlaybackStore: play()
PlaybackStore->>TimelineStore: getTotalDuration()
PlaybackStore->>Project: getActiveFPS() (default 30)
PlaybackStore->>PlaybackStore: compute endThreshold = duration - 1/FPS
alt currentTime ≥ endThreshold
PlaybackStore->>PlaybackStore: seek(0)
end
PlaybackStore->>PlaybackStore: set isPlaying = true
PlaybackStore->>PlaybackStore: start timer
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@mazeincoding can you review this? |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/src/stores/playback-store.ts (2)
87-92: DRY: extract effectiveDuration/frameOffset helpers to avoid divergence with updateTimeThe same concepts (effectiveDuration via timeline store and 1-frame offset by FPS) live here and in updateTime. Extract small helpers to keep behavior consistent and easier to evolve.
Example (outside the store definition):
const getEffectiveDuration = (state: PlaybackStore) => { const actual = useTimelineStore.getState().getTotalDuration(); return actual > 0 ? actual : state.duration; }; const getFrameOffset = () => { const rawFps = useProjectStore.getState().activeProject?.fps; return 1 / (typeof rawFps === "number" && Number.isFinite(rawFps) && rawFps > 0 ? rawFps : 30); };Then reuse in play() and updateTime to compute thresholds.
98-101: Tests: cover restart-at-end pathRecommend adding a test for pressing play/space at endThreshold that verifies a seek(0) occurs before playback starts (e.g., assert playback-seek event fired with 0, isPlaying true afterwards). I can draft the test quickly if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/stores/playback-store.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use TypeScript namespaces.
Don't use non-null assertions with the!postfix operator.
Don't use parameter properties in class constructors.
Don't use user-defined types.
Useas constinstead of literal types and type annotations.
Use eitherT[]orArray<T>consistently.
Initialize each enum member value explicitly.
Useexport typefor types.
Useimport typefor types.
Make sure all enum members are literal values.
Don't use TypeScript const enum.
Don't declare empty interfaces.
Don't let variables evolve into any type through reassignments.
Don't use the any type.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use implicit any type on variable declarations.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use the TypeScript directive @ts-ignore.
Use consistent accessibility modifiers on class properties and methods.
Use function types instead of object types with call signatures.
Don't use void type outside of generic or return types.
Files:
apps/web/src/stores/playback-store.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{js,jsx,ts,tsx}: Don't use the return value of React.render.
Don't use consecutive spaces in regular expression literals.
Don't use theargumentsobject.
Don't use primitive type aliases or misleading types.
Don't use the comma operator.
Don't write functions that exceed a given Cognitive Complexity score.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use uselessthisaliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names that aren't base 10 or use underscore separators.
Remove redundant terms from logical expressions.
Use while loops instead of...
Files:
apps/web/src/stores/playback-store.ts
🔇 Additional comments (1)
apps/web/src/stores/playback-store.ts (1)
85-101: End-of-timeline restart behavior in play() is correct and aligns with #536The effectiveDuration + frame-offset threshold check and seek(0) before starting playback achieve the desired UX. Good centralization in play().
merge into staging branch not main |
|
lfg! |
Description
When Play (or Space) is pressed while the playhead is at the end of the timeline, playback now restarts from the beginning (time 0) and continues playing.
Why here
Fixes #536
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Repro steps:
Test Configuration:
Screenshots (if applicable)
opencut-q.mp4
Checklist:
Additional context
Centralizing the logic in the playback store’s play() ensures consistent behavior regardless of how playback is toggled
Summary by CodeRabbit