Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass the reference of the proxy if needed #2522

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

danielebogo
Copy link
Contributor

@danielebogo danielebogo commented Dec 9, 2024

Ref. #2521

This PR should fix the crash in the Audio callback. Compared to the previous fix, we missed few spot where the unretained pointer was injected directly into the callback. This was causing a crash as the expected opaque type should always be the proxy and not the player.

To test

  • CI must be 🟢
  • Run this branch and play an episode with the volume boost on
  • Confirm the app won't crash
  • Try to play and change episodes while playing
  • When the system decides to finalize the tap you will see 2 logs in this order:
  1. The log from the finalize closure: [AudioProcessingTapProxy] Finalize tap: <MTAudioProcessingTap 0x6000030513b0> Retain count 3 Created with i/f/p/u/t callbacks = {0x10aa14448/0x10aa14600/0x10aa149b4/0x10aa14d08/0x10aa15330} flags = 0x1
  2. The log from the proxy deinit: [AudioProcessingTapProxy] Deinit proxy

Checklist

  • I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
  • I have considered adding unit tests for my changes.
  • I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

@dangermattic
Copy link
Collaborator

1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

@danielebogo danielebogo added this to the 7.79 ❄️ milestone Dec 10, 2024
@danielebogo danielebogo changed the base branch from trunk to release/7.79 December 10, 2024 10:31
@danielebogo danielebogo added [Type] Bug Used for issues where something is not functioning as intended. crash Crash related issues labels Dec 10, 2024
@danielebogo danielebogo marked this pull request as ready for review December 10, 2024 14:23
@danielebogo danielebogo requested a review from a team as a code owner December 10, 2024 14:23
@danielebogo danielebogo requested review from bjtitus and removed request for a team December 10, 2024 14:23
Copy link
Contributor

@bjtitus bjtitus left a comment

Choose a reason for hiding this comment

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

✅ Checked playback effects
✅ Checked downloaded & streamed episodes

@danielebogo danielebogo merged commit 921d871 into release/7.79 Dec 12, 2024
7 of 9 checks passed
@danielebogo danielebogo deleted the danieleb/issues/fix-default-player-crash branch December 12, 2024 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crash Crash related issues [Type] Bug Used for issues where something is not functioning as intended.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants