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

[iOS] Now playing freeze ui when changing media #218

Closed
MohamedAbdallah-14 opened this issue Mar 25, 2020 · 8 comments · Fixed by #219
Closed

[iOS] Now playing freeze ui when changing media #218

MohamedAbdallah-14 opened this issue Mar 25, 2020 · 8 comments · Fixed by #219
Assignees
Labels
bug Something isn't working iOS

Comments

@MohamedAbdallah-14
Copy link
Contributor

MohamedAbdallah-14 commented Mar 25, 2020

Describe the bug
When changing media item ui temporary freezes, Also if the url contain Arabic chars or spaces the image doesn't show

Expected behavior
Changing media shouldn't affect the UI thread

Runtime Environment

  • Device: [iPhone X]

  • iOS version: [13.3]

  • Device: [iPhone 6s Plus]

  • iOS version: [13.4]

  • Device: [iPhone 6s]

  • iOS version: [12.3.1]

Flutter SDK version

[✓] Flutter (Channel stable, v1.12.13+hotfix.8, on Mac OS X 10.15.4 19E266, locale en)

[✓] Android toolchain - develop for Android devices (Android SDK version 29.0.2)
[✓] Xcode - develop for iOS and macOS (Xcode 11.4)
[✓] Android Studio (version 3.5)
[✓] VS Code (version 1.43.2)
[✓] Connected device (3 available)

• No issues found!

Additional context
This happen because fetching image happen in the main thread and it should be done in background thread
I will open pull request to fix this issue.

@ryanheise
Copy link
Owner

HI @MohamedAbdallah-14 , after merging, I thought on it further and think it might be more advantageous to move the image fetching logic to the Flutter side. The reason is that the Android side currently implements the same sort of logic, pre-fetching images in a background thread, but also prefetching the next track so it's ready to display the image by the time the next track starts playing, and caching, so it doesn't have to re-fetch images when skipping between different tracks. It makes sense to push this code to the Flutter side so it can be reused by both platforms.

The Flutter side would then instead of passing through the URL of some image from the Internet, would pass a file:// URI to the cache. Given that the iOS code would now always be loading the image from a local file, what approach would you recommend on the iOS side for loading the image?

@MohamedAbdallah-14
Copy link
Contributor Author

HI @ryanheise It make more sense to implement the logic in flutter side,
The thing is I'm iOS developer but I code in Swift and have very little experience in Objective C
So I was going to open an issue to suggest to convert the code to Swift I have the time to do it if you are willing to merge the swift code.

in Swift you can get image from path like this let image = UIImage(contentsOfFile: path)
What is your thoughts?

@ryanheise
Copy link
Owner

@MohamedAbdallah-14 , thanks for the generous offer to convert the code to Swift. I have been hesitant about using Swift up until now because there have been various issues with mixing Swift and Flutter in the past and I wanted to wait until they had resolve all of those issues before considering it. I don't know if all issues have been resolved yet.

However, clearly the Flutter team are nudging developers in the direction of Swift and you'd have to bet that they'd "eventually" sort out all the issues. And on Flutter MacOS, Swift is the only officially supported language (unofficially, Objective C still works, though).

I'd say go for it! If the converted code works, I'll merge. I know there are still a lot of iOS developers who prefer Objective C, but to me its not so much about which language people prefer (since we'll be writing most of our code in Dart anyway), but which language is going to have better support within Flutter. Right now, I think Objective C may still work a bit better, but the future is heading in the direction of Swift.

@ryanheise ryanheise self-assigned this Mar 28, 2020
@MohamedAbdallah-14
Copy link
Contributor Author

I agree, Let's Do It 💯

@ryanheise
Copy link
Owner

FYI I've just moved the image caching and fetching code from Android to Dart, and updated the iOS implementation to follow suit. Since the artwork is being loaded directly from file, I decided not to load it asynchronously like you did in your PR, but let me know if the UI still freezes for you.

@MohamedAbdallah-14
Copy link
Contributor Author

I Just Finished converting it to Swift,
I will see the modification and apply it in my code before pushing the PR

@ryanheise
Copy link
Owner

I'll close this issue now that the image loading has been moved to the Flutter side.

By the way, I've worked on a cleanroom implementation of the parts of the Swift code we discussed in your PR, and if you'd like to pursue this, pop me an email any time.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs, or use StackOverflow if you need help with audio_service.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working iOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants