Skip to content

Conversation

@staskus
Copy link
Contributor

@staskus staskus commented Jun 17, 2025

Description

Continuation of #15746

As @joshheald noticed, my "optimizations" to reuse AVAudioPlayer have resulted in sound playing blocking the main thread.

To avoid that, we can turn PointOfSaleSoundPlayer into actor. It ensures that playerCache is accessed synchronously and at the same time the player creation can happen in a background thread.

Steps to reproduce

  1. Open POS
  2. Add invalid barcode items
  3. Observe that the cart row loading shimmering animation doesn't get stalled before transitioning to the error state
  4. Confirm the failure sound plays

Testing information

Tested on iPad Air 18.3 Device

Screenshots


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@staskus staskus added this to the 22.7 milestone Jun 17, 2025
@staskus staskus requested a review from joshheald June 17, 2025 20:24
@wpmobilebot
Copy link
Collaborator

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Number30524
VersionPR #15770
Bundle IDcom.automattic.alpha.woocommerce
Commitdaff034
Installation URL7743p32d5nv6g
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

Copy link
Contributor

@joshheald joshheald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works much better... nice use of actor.

There's still some blocking on the first error... but it seems like AVAudioPlayer does always block the main thread when you make one, even when you create it from the background.

I tried adding a preloadSound function that warmed the cache, which works but just moves the problem; it blocks the main thread when that's called instead. I did it on the first scan. I guess we could do it some time when we are more confident that nothing's happening... but nothing's guaranteed.

Other advice seems to be to use AudioServicesCreateSystemSoundID, but that requires wav or other simpler (larger) audio files.

I think we can keep it as it is for release.

@staskus
Copy link
Contributor Author

staskus commented Jun 18, 2025

@joshheald, thanks for the context. Frustrating that the main thread gets blocked in this way. I'm not too familiar with AVAudioPlayer but it feels like there has to be a way to avoid the lag. 🤔 I'm not noticing it too much myself, maybe it's more apparent when using an actual scanner.

@joshheald
Copy link
Contributor

I'm not noticing it too much myself, maybe it's more apparent when using an actual scanner.

Or maybe it's an iOS 17 thing...

@staskus staskus merged commit 83d067a into trunk Jun 18, 2025
20 of 22 checks passed
@staskus staskus deleted the fix/pos-sound-blocking-main-thread branch June 18, 2025 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants