ryanheise / just_audio

Audio Player
1.03k stars 656 forks source link

iOS Slow PlaybackRate Echo/Warble #83

Closed gitmoto closed 4 years ago

gitmoto commented 4 years ago

Is your feature request related to a problem? Please describe. When slowing the playback rate on iOS with a remote spoken word mp3 (like a podcast), it sounds funny. It sounds great in the Android emulator and devices, at the same slower rate. But on iOS device (or in the emulator ), there is an echo/warble. Almost like there are two streams playing at once and one is just a few milliseconds off. It happens any time I call setSpeed with anything less than 1.0

Describe the solution you'd like It appears the issue may be related to this: https://stackoverflow.com/questions/52462881/the-sound-quality-of-slow-playback-using-avplayer-is-not-good-enough-even-when-u

Can I request a feature to toggle the value of AVPlayerItem.audioTimePitchAlgorithm for iOS?

Describe alternatives you've considered

Additional context

ryanheise commented 4 years ago

Interesting. I originally planned to implement the same algorithm I was using on Android (which is very good) for iOS because I didn't know there were any good built-in options for iOS.

However, the built-in options you linked to sound better than I originally thought.

What default iOS algorithm would you suggest to most closely match the Android quality? I also don't think we need to add options for all 4 algorithms at this stage, probably one optimised for voice and one for music.

gitmoto commented 4 years ago

Thanks for looking into this, Ryan! Really appreciate it.

I've played around a little with AVPlayerItem.audioTimePitchAlgorithm, but I'm not sure what the best implementation would be for just_audio. I don't have the best ear in the world, but I can't tell much of a difference between different pitch algorithms at standard speeds.

Best I can tell, I think the best mode to match Android quality at varying playback speeds depends on the type of content. Like you said, maybe just music (spectral) and voice (timeDomain) settings?

And perhaps rely on the underlying iOS default value if just_audio caller doesn't specify which type of content is being played? So I guess there would be 3 possible algorithms, with the default being best for short clips of low-fidelity sounds?

ryanheise commented 4 years ago

The Android algorithm is also optimised for speech, which makes sense because it is a more common use case to slow down speech than it is to slow down music. So for iOS I think the algorithm suitable for speech would be a reasonable default.

Does changing the algorithm to timeDomain make it sound better for you at speeds less than 1.0? You can test this by editing the iOS code with the code you referenced in the StackOverflow post.

gitmoto commented 4 years ago

I ran a quick test in your setUrl method and it works beautifully! I wonder if there are other use-cases for modifying the pitch algorithm, beyond just the type of audio (simple app sounds vs. music vs. spoken word) being played? Your idea of a setting for voice and a setting for music seems reasonable for a simple, initial implementation though.

Audio Url: https://raw.githubusercontent.com/gitmoto/public/master/borrarfronteras.mp3

//works ok for simple, low quality audio playback at 1.0 speed playerItem.audioTimePitchAlgorithm = AVAudioTimePitchLowQualityZeroLatency;

//best for high quality music, but also good for spoken word audio (although additional computation intensity tradeoff seems unnecessary for spoken word) playerItem.audioTimePitchAlgorithm = AVAudioTimePitchAlgorithmSpectral;

//best for spoken word audio (especially when modifying playback rate) playerItem.audioTimePitchAlgorithm = AVAudioTimePitchAlgorithmTimeDomain;

//only use if you don't want any pitch adjustments playerItem.audioTimePitchAlgorithm = AVAudioTimePitchAlgorithmVarispeed;

gitmoto commented 4 years ago

Also, fyi, this setting appears to only be available in iOS 11.0+

https://developer.apple.com/documentation/avfoundation/avaudiotimepitchalgorithm

ryanheise commented 4 years ago

So pre iOS11.0 we just leave it as is, and on post iOS 11.0 we set the default to timeDomain, at the very least.

Then additionally we can consider adding an option to change the algorithm. Either we provide a general option to switch between music vs voice optimised, or we add an iOS specific option to select specific iOS algorithms.

gitmoto commented 4 years ago

I dig it. Really interesting the implementation quirks that have to be taken into consideration when developing an abstraction layer like this flutter package. Thanks for being so collaborative on improving this and for putting it out there for the community!

ryanheise commented 4 years ago

No worries. I've just committed the fix, which basically just chooses timeDomain as the default if available. I may consider adding more options at a later stage.

Can you try it out and let me know if this works for you?

gitmoto commented 4 years ago

did you commit the source files? or should it work with just the example subdirectory update?

ryanheise commented 4 years ago

I've committed the fix to git master, so you should be able to try the example subdirectory, or alternatively with your own project if you temporarily update your pubspec to:

  audio_service:
    git:
      url: https://github.com/ryanheise/audio_service.git
gitmoto commented 4 years ago

which pubspec file? I don't have an audio_service entry to temporarily replace with your git url. I only have just_audio

ryanheise commented 4 years ago

Oops, I had copied that from a post I made in my other plugin. It should be:

  just_audio:
    git:
      url: https://github.com/ryanheise/just_audio.git
gitmoto commented 4 years ago

Thanks! Just wanted to make sure I wasn't missing something

gitmoto commented 4 years ago

I must be doing something wrong. When I update the example/pubspec.yaml file (and then do a pub get), the audio doesn't seem to be using the timeDomain setting. Audio still warbles. Should I be pointing to a different branch or tag in the github url?

This is what I'm doing in the pubspec:

just_audio:

path: ../

just_audio: git: url: https://github.com/ryanheise/just_audio.git

stonega commented 4 years ago

try flutter pub upgrade

On Wed, Apr 29, 2020, 12:24 Matthew Douglass notifications@github.com wrote:

I must be doing something wrong. When I update the example/pubspec.yaml file (and then do a pub get), the audio doesn't seem to be using the timeDomain setting. Audio still warbles. Should I be pointing to a different branch or tag in the github url?

This is what I'm doing in the pubspec:

just_audio:

path: ../

just_audio: git: url: https://github.com/ryanheise/just_audio.git

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ryanheise/just_audio/issues/83#issuecomment-620981458, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARIH5ZYGYW3RQVNBSQIHW3RO6TWRANCNFSM4MRAJBSA .

gitmoto commented 4 years ago

I meant to mention I tried that too. No luck.

ryanheise commented 4 years ago

@gitmoto Just checked my commit, and it does seem I missed a file! Oops :-) Let me try that again...

ryanheise commented 4 years ago

Fixed! Sorry about that. Would you like to test it again?

gitmoto commented 4 years ago

No prob at all. I'm just ecstatic I'm not going crazy...

Just tested with the git link in my pubspec and it works great! Thank you for doing this.

ryanheise commented 4 years ago

Awesome. Thanks for testing, and I've now published in the 0.1.8 release.

gitmoto commented 4 years ago

sweet. Really appreciate the quick turnaround on this!

github-actions[bot] commented 2 years ago

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 just_audio.