[$500] Changing playback speed is not taking effect when changing playback speed before playing

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.43-0 Reproducible in staging?: y Reproducible in production?: new feature If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: applause internal team Slack conversation:

Action Performed:

1. Open a chat
2. Click on '+' icon > Add attachment > select a video
3. Click on three dot icon > Playback speed > select 2 or other option
4. Click on play icon

Expected Result:

Change in playback speed setting should take effect

Actual Result:

Change in playback seep doesn't take effect

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [x] Android: Native
• [ ] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/43996225/c4743e25-6f29-4dfd-a2ee-0e50bb0ae3c9

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017f68d54c34c3a131
• Upwork Job ID: 1759791176103968768
• Last Price Increase: 2024-02-20
• Automatic offers:
• dukenv0307 | Contributor | 0
• shubham1206agra | Contributor | 0

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~017f68d54c34c3a131

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (External)

[melvin-bot[bot]]

Triggered auto assignment to @miljakljajic (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[github-actions[bot]]

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot[bot]]

Triggered auto assignment to @MariaHCD (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[kavimuru]

We think this bug might be related to #vip-vsb cc @quinthar

[dukenv0307]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Change in playback seep doesn't take effect

What is the root cause of that problem?

We don't pass rate prop into Video component to control the speed of the video so the speed is always the default

Ref: https://docs.expo.dev/versions/latest/sdk/video/#rate

https://github.com/Expensify/App/blob/1806422ea2208540a89271da7026799a1ade9d69/src/components/VideoPlayer/BaseVideoPlayer.js#L203

What changes do you think we should make in order to solve the problem?

Return currentPlaybackSpeed in contextValue of VideoPlayerContexts

const contextValue = useMemo(() => ({menuItems, updatePlaybackSpeed, currentPlaybackSpeed}), [menuItems, updatePlaybackSpeed]);

https://github.com/Expensify/App/blob/1806422ea2208540a89271da7026799a1ade9d69/src/components/VideoPlayerContexts/VideoPopoverMenuContext.js#L60

const {currentPlaybackSpeed} = useVideoPopoverMenuContext();

rate={currentPlaybackSpeed}

Then in BaseVideoPlayer get this speed and pass it as rate prop into Video component.

What alternative solutions did you explore? (Optional)

Each video should have a default speed is 1.0, we can create a speed state in BaseVideoPlayer and then when we update the speed we will update speed state instead of updating it in VideoPlayerContexts

Result

https://github.com/Expensify/App/assets/129500732/37b55b80-3977-4091-b2b1-629d98b11550

[MariaHCD]

Looks like this is also related to https://github.com/Expensify/App/pull/30829

cc: @Skalakid @francoisl @akinwale

I think @dukenv0307's proposal makes sense. Thoughts, @Santhosh-Sellavel?

[miljakljajic]

Waiting for feedback from @Santhosh-Sellavel

[shubham1206agra]

@Santhosh-Sellavel is OOO. Since this is a deploy blocker, I will help out here.

@dukenv0307's proposal looks good to me. 🎀 👀 🎀 C+ reviewed

[melvin-bot[bot]]

Current assignee @MariaHCD is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[shubham1206agra]

Context for OOO https://expensify.slack.com/archives/C02NK2DQWUX/p1708107813288869

[MariaHCD]

Thanks, @shubham1206agra!

[melvin-bot[bot]]

📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[dukenv0307]

@MariaHCD Can you please re-assign to the correct C+ so when I open the PR, bot can assign reviewer correctly.

[melvin-bot[bot]]

📣 @shubham1206agra 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[MariaHCD]

Although perhaps we shouldn't proceed with a fix for this just yet: https://expensify.slack.com/archives/C01GTK53T8Q/p1708443654362909?thread_ts=1708443084.946829&cid=C01GTK53T8Q

[shubham1206agra]

Yes.

[dukenv0307]

@shubham1206agra The PR is ready for review.

[francoisl]

I don't think we're going to end up reverting the PR, we already merged other fixes and there would be too many conflicts to resolve at this point. @shubham1206agra please proceed with reviewing and testing the PR. Thanks!

[puneetlath]

Removing the deploy blocker label. Seems like something we can fix at the normal cadence.

[shubham1206agra]

PR was already merged and deployed.

[miljakljajic]

is payment due here?

[shubham1206agra]

Yes

[shubham1206agra]

@miljakljajic Can you start payment process here?

[miljakljajic]

@shubham1206agra and @dukenv0307 sent contracts to you both, previous job expired.

[shubham1206agra]

@miljakljajic I have discussed this internally here. You may close this issue whenever you pay the other contributor as I am keeping track of payment internally and will ask to pay once the issue is resolved. Just write in the payment summary that I still need to be paid.

[miljakljajic]

Thank you @shubham1206agra

@shubham1206agra - owed 500 USD for their work on this issue.

@dukenv0307 please accept the offer when you can.

[dukenv0307]

@dukenv0307 please accept the offer when you can.

@miljakljajic I accepted, thank you!

[shubham1206agra]

@miljakljajic You can process payment here now.

[shubham1206agra]

@miljakljajic Bump here.

[shubham1206agra]

@miljakljajic Bump for the payment.

[shubham1206agra]

@miljakljajic Bump here.

[miljakljajic]

Paid!