ryanheise / just_audio

Audio Player
1.01k stars 620 forks source link

ConcatenatingAudioSource with lots of children takes a very long time to load #294

Open smkhalsa opened 3 years ago

smkhalsa commented 3 years ago

Which API doesn't behave as documented, and how does it misbehave?

Creating a ConcatenatingAudioSource with lots (~1000) of children takes a very long time (>20 seconds).

In my application, users can have playlists with an arbitrary number of items.

Minimal reproduction project

To Reproduce (i.e. user steps, not code)

final player = AudioPlayer();
final songs = /// 1000+ sources
await player.setAudioSource(
  ConcatenatingAudioSource(children: songs),
);

Error messages

Expected behavior

I'd expect to be able to set the audio source and start playing the first source within a couple seconds, even with a large number of sources (since only the initial few sources are actually being fetched).

Screenshots

Desktop (please complete the following information):

Smartphone (please complete the following information):

Flutter SDK version

[✓] Flutter (Channel beta, 1.25.0-8.3.pre, on Mac OS X 10.15.7 19H114 darwin-x64, locale en-US)
[✓] Android toolchain - develop for Android devices (Android SDK version 29.0.2)
[✓] Xcode - develop for iOS and macOS
[✓] Chrome - develop for the web
[✓] Android Studio (version 4.1)
[✓] VS Code (version 1.52.1)
[✓] Connected device (2 available)

• No issues found!

Additional context

NOTE added by @ryanheise :

Adding a large number of children is problematic at multiple levels due to the single-threaded model. For now, there are three approaches to workaround this issue:

  1. Initialise a ConcatenatingAudioSource with zero children, and then add the children in a loop that yields every now and then to let other coroutines execute on the same thread. This can be done for example by periodically calling await Future.delayed(Duration.zero);.
  2. Initialise a ConcatenatingAudioSource with an initial sublist of children small enough to load without issue, and then write your own application logic to lazily add more children as needed (just-in-time). If you want to create the illusion that the list of children is complete from the beginning, you will need to write your UI around some other more complete list of metadata, separate from ConcatenatingAudioSource's own internal list of items.
  3. There is an experimental branch feature/treadmill (now merged) that implements the logic of (2) above but on the iOS native side (Android already has this implemented on the native side when useLazyPreparation is true). This will stop iOS from trying to load every item in the playlist, and instead only load items that are nearer to the front of the queue. If you test this branch, please share below whether you found it stable enough to release.
ryanheise commented 3 years ago

Hi @smkhalsa unfortunately I will need you to fill in the sections as instructed. In the section called "User steps, not code", you actually copied and pasted code. This should be in the previous section "Minimal reproduction project" which you have left empty, but which should contain a link to a git repository that I can clone and test.

ryanheise commented 3 years ago

This issue affects both iOS and macOS. I have a reproduction case on #206 which has helped me to reproduce this issue.

The solution is to rewrite the enqueueFrom method so that it enqueues only the next few items, and whenever the player advances to the next item, we should add another item onto the end. This behaviour should be the default, but it should also be tied to the ConcatenatingAudioSource.useLazyPreparation option.

This is a tricky area of the code to work on since it can easily break things, but it is an important feature to add, and as long as it undergoes sufficient testing, it should be worth doing.

I've marked this issue as "fixing" which means it is on my priority list. That said, there are some other things that were already on my priority list, such as the visualizer, and the null-safe release of audio_service (which will be based on the one-isolate model).

suragch commented 3 years ago

@ryanheise The enqueFrom you are talking about is here? So that's not something we could handle on plugin user side, right?

I was noticing similar behavior with 43 items in the playlist. My UI freezes while they are loading on macOS and iOS. On the iOS simulator the freeze lasted 30 seconds. Web and Android are fine.

Can you think of any temporary workarounds I could pursue on my side?

ryanheise commented 3 years ago

That's correct. The solution involves improving the enqueueFrom so that it can enqueue just a few items ahead instead of the whole list, when the useLazyPreparation option is set to true. That involves modifying the Objective C code in the plugin.

As for a workaround, I can think of 2 things to try:

  1. The easiest thing to try (although I don't know whether it will have any effect) is to play with the new buffering options available in the dev branch, which you can pass into the constructor of AudioPlayer. With these options (in particular preferredForwardBufferDuration), you may be able to limit the size of the forward buffer and thus reduce the time spent loading. The problem with this is that iOS isn't guaranteed to follow your preferred buffer parameters, and even if it did, it still may need to load a little bit of each track.
  2. Forego just_audio's playlist management and implement the lazy loading within your own app. So let's say you have a list of 43 items, you could create a ConcatenatingAudioSource with just the first 3 items in it. Then when playback approaches the 3rd item, you can dynamically insert more items onto the end of it. The drawback is that you can't simply use just_audio's state to tell you what's in your entire playlist, because it'll only know about the sublist of items you've added, so you'll need to maintain your own full playlist state outside of just_audio.
suragch commented 3 years ago

I forgot about the possibility of adding items to an existing playlist. That's a good idea. Thank you!

YaarPatandarAA commented 3 years ago

Just to add onto this, I have only noticed this issue on MacOS release/debug and only on iOS Simulator. This issue is not present, at least for me, on Physical iOS device whether release or debug. Android is Fine. I am adding 100 tracks at once.

FallenChromium commented 2 years ago

As far as I understand, the question that I have is heavily related to this issue (though not exactly relevant, I use Android and it isn't a "bug" per say, it's just that retrieving 40+ URLs is time-costly with the API I use)

I am not sure if I can inject a function to lazy load the track URL in AudioSource objects, so that the URL retrieval (and also caching) will kick in only when the track is about to begin playing. Did I get it correctly that it's now not an option, unless I make an "external" playlist state and control just_audio according to that playlist?

ryanheise commented 2 years ago

ConcatenatingAudioSource.useLazyPreparation is already implemented on Android, it just needs to be implemented on iOS/macOS.

FallenChromium commented 2 years ago

As far as I understood, useLazyPreparation is used to delay caching, but not the retrieval of children in ConcatenatingAudioSource, so, if I use API which, for example, returns download links which are valid only for 1 minute, this is not an option for me. I was initially supposing that after setting this bool, the children themselves will be resolved lazily. So, it's not related to the issue? How can I achieve this kind of effect using just_audio then?

ryanheise commented 2 years ago

This issue is about iOS/macOS. If you have found an Android bug or have an Android feature request, it will be appreciated to open a separate issue for that.

ertgrulll commented 2 years ago

Any progress about the issue? I'm developing an app for windows/mac and player is freezing for 4-5 seconds when i add 60 song.

ddfreiling commented 2 years ago

This bug should have high priority in my opinion. Would solving it require switching to an AVAudioQueuePlayer and letting the platform handle prefetching?

ryanheise commented 2 years ago

The current AVQueuePlayer will work fine, but the enqueue logic should be rewritten to enqueue a configurable number of items, and at the same time, the gapless looping approach should be adapted to this. It's going to take a while to develop and test.

However, you can work around this by doing the same sort of queue management in dart. That is, only add to the concatenating audio source as many children as you want to actually load, then lazily add more children just before they're needed.

ryanheise commented 2 years ago

Also I'll quote my earlier comment from above which I think may do a better job of explaining how you could achieve your use case:

  1. Forego just_audio's playlist management and implement the lazy loading within your own app. So let's say you have a list of 43 items, you could create a ConcatenatingAudioSource with just the first 3 items in it. Then when playback approaches the 3rd item, you can dynamically insert more items onto the end of it. The drawback is that you can't simply use just_audio's state to tell you what's in your entire playlist, because it'll only know about the sublist of items you've added, so you'll need to maintain your own full playlist state outside of just_audio.

Incidentally, @ddfreiling , I would also recommend clicking the :+1: on this issue to vote for it. I do try to fix critical bugs quickly regardless of votes, but since this issue is not a critical bug (you can take resource management into your own hands to some extent with the above workaround), and since I'm dealing with a great number of requests and have to prioritise what I work on next based on helping the greatest number of people, the votes do count.

The alternative is that if you can't wait for me personally to work on this, you can consider becoming a contributor, since those who need it will often have the strongest motivation to help work on it. I can't say it's an easy one to work on, though. First, it requires Objective C knowledge, and then second it also requires studying and comprehending what is probably the most complex part of the iOS implementation, and how it fits together with the rest of the implementation.

ryanheise commented 2 years ago

For those subscribed to this issue, there is a related issue that can cause the same symptom of blocking the UI thread, but has a simpler solution.

Basically, if you are constructing a very large list of children to pass into ConcatenatingAudioSource in a loop like this:

for (var i = 0; i < 100000; i++) {
  children.add(...);
}

Then that code itself will block the UI thread because it does not yield to co-routines. You can fix that for example by inserting await Future.delayed(Duration.zero); inside the loop.

I mention this just in case anyone experiencing the UI blocking issue actually has this other easier-to-fix issue.

pro100svitlo commented 2 years ago

Any update on this issue? For me UI locking even with 100 items (around 7-10 sec). Quite sad experience...

ryanheise commented 2 years ago

This will always be problematic to some degree because of Dart's single threaded model. Fortunately there are ways to deal with this which I have already mentioned (in my edit at the bottom of the very first post).

mhutshow commented 2 years ago

Hi. I have tried a lot to play with your workaround. I was programatically adding the track in the list. But unfortunately it didn't work for me.

It's a humble request to you. Please work for a fix. We love your work, You made it amazing. <3

Hope this will be fixed soon.

ryanheise commented 2 years ago

I know that people are using that workaround successfully (lazily adding items to the playlist just-in-time so that you don't try to load them all at once). I suggest you keep trying to implement that workaround because it is going to be much harder to build multi-threaded behaviour into the plugin and you'll be waiting much longer.

mhutshow commented 2 years ago

@ ryanheise : Can you provide us a code example that how can I lazily load items? I tried this one.

for (var i = 0; i < 100000; i++) {
  children.add(...);
 await Future.delayed(Duration.zero);
}
pro100svitlo commented 2 years ago

For me lazy loading didn't work. But i implemented another suggestion: instead adding whole list at once I added 3 items: previous, current and next. Then if user click next, or previous, i add one new playlist item at the start of at the end of the list.

To be honest - it looks ugly. But when it's covered with the tests - somehow you can trust it

mhutshow commented 2 years ago

I think lazy loading will work. Because if it listen to the list that I am adding new item then it will work.

But in my case the problem is when it loads, it loads with initial items. Then I keep adding the file by a for loop. But it don't see the changes (new files). After playing the initial files it stops.

I think if we get an example code from @ryanheise then it will be easy to implement the feature.

Let's wait with a hope from @ryanheise .

ryanheise commented 2 years ago

@pro100svitlo 's approach should work, it's a treadmill with always 3 items.

I'm not going to implement it here @mhutshow but @pro100svitlo has given the pseudocode in his previous comment. If the treadmill contains 3 items [0,1,2], then each time the current index changes to 0 or 2, you can do the treadmill thing that @pro100svitlo describes well.

mhutshow commented 2 years ago

@ryanheise @pro100svitlo : My case is little bit different . There is no next or previous button. the tracks are 5-6 seconds long. A track is divided into 30-260 pieces and I have to play it one by one. If I add item to the children after it starts playing the initial 3, the newly added item can not be played. Sadly, It stops after playing the initial 3.

pro100svitlo commented 2 years ago

The point is that you need to add one more each time next clicked or current ended. In this way you should always have one item ahead

ryanheise commented 2 years ago

@mirkancal What are you referring to by the word "it"? Do you mean one of the workarounds or pseudocode mentioned above, and if so, which one?

MrVipinVijayan commented 2 years ago

use preload: false It will fix the ui freezing issue.

mhutshow commented 2 years ago

Believe me. This only happens in the iOS simulator. I have a 100K users production app. And no issue comes regarding freezing. So just ignore it during development in the simulator. In production/real device no issue will come.

MrVipinVijayan commented 2 years ago

Good to hear that, but unfortunately for me, it happens in debug and release mode, both in simulator and device.

mirkancal commented 2 years ago

Just removing the await keyword helped in my case.

// ignore: unawaited_futures
 _player.setAudioSource(_concatenatingAudioSource);

As soon as I get my sounds from the backend, I send a custom action to the handler.

 @override
  Future<dynamic> customAction(String name,
      [Map<String, dynamic>? extras]) async {
    switch (name) {
      case 'load':
        final urls = extras!['urls'] as List<String>;
        if (!kIsWeb) {
          cachedAudios =
              urls.map((e) => LockCachingAudioSource(Uri.parse(e))).toList();
        } else {
          cachedAudios =
              urls.map((e) => ProgressiveAudioSource(Uri.parse(e))).toList();
        }

        _concatenatingAudioSource = ConcatenatingAudioSource(
          children: cachedAudios,
        );
        // ignore: unawaited_futures
        _player.setAudioSource(_concatenatingAudioSource);
        break;
    }
  }
ryanheise commented 2 years ago

I am recently considering implementing the treadmill approach within the Dart side of the plugin, and that would involve removing the existing Android implementation which is currently implemented entirely on the native side. If this is implemented as a treadmill in Dart, then it should work on all platforms consistently, but there is also a chance that the Android behaviour will not be exactly the same as before (although I can't predict yet whether that would cause problems).

suragch commented 2 years ago

That sounds like a great solution to me. It would remove complexity on the native side and solve the only major issue i have with the plugin (slow ios load times).

ryanheise commented 2 years ago

Hmm, while this idea is attractive at first glance, I just spent a little time thinking about how I would implement this approach and I don't think it would be compatible with the way just_audio_background works because just_audio_background needs to be aware of the whole queue at all times in order to broadcast the queue, not merely the part of the queue that is on the top surface of the treadmill. So the platform will need to know about the complete list of children. Another issue is that such an implementation may introduce a lot of complexity and hence bugs.

I guess I will be implementing the iOS treadmill in native code after all.

ryanheise commented 2 years ago

I have just implemented an iOS/macOS treadmill. It is available for testing on the feature/treadmill branch.

Currently it is hard coded to be always on because it is actually difficult with the current design to emulate the Android behaviour (that may influence a future API change sot hat lazy loading is a property of the player rather than of the playlist).

Also, the treadmill is hardcoded to hold two items at a time. Ideally the length of the treadmill would dynamically adapt based on the shortness of the items. For example, for short items, you'd want to preload more items onto the treadmill to ensure gapless playback. For longer items, the current and next item would be enough.

suragch commented 2 years ago

Wow, that was fast! Thank you!

ryanheise commented 2 years ago

It took me 1.5 years to find a quick solution :-) But even still I dreaded changing the iOS queuing logic since it was complex to write the first version and it's important not to introduce bugs. Hopefully we can see through testing whether it is solid or has any issues.

austinried commented 2 years ago

I found this thread because I have a similar issue with slow load times for large playlists, but in my case it's more related to things I need to do while constructing the playlist (async check a cache & potentially perform a network request per song). I'm wondering, and I haven't put enough thought into this yet to put together a full feature request, but @ryanheise would you consider a solution to this that involves something like a "childBuilder" method for ConcatenatingAudioSource that implements this treadmill on the dart side, and also allows us to do those arbitrary just-in-time preparations on the library user end?

ryanheise commented 2 years ago

As stated above, that approach will not work due to the way the platform interface works.

However, with the current API it is completely possible for an app to implement a simple treadmill of its own with all of its app-specific requirements. You can listen to player.currentIndexStream and whenever it changes, you can trigger your own logic to add the next item to the ConcatenatingAudioSource according to your own business logic.

austinried commented 2 years ago

Ah understood, I will go that route then. It's a bit of a shame because it means I'll also need to re-implement shuffle/loop and the events that go with those, but that's not a huge deal I suppose.

ryanheise commented 1 year ago

Is anyone able to report on their experience with the solution implemented in feature/treadmill? As mentioned, I am reluctant to merge changes to this core code unless people find that it is actually working well for them. Once it's proven to be an effective solution, I can merge it into an official release.

Yesterday17 commented 1 year ago

The currentPlaying index stream seems wrong when I test it last week.

ryanheise commented 1 year ago

Does this happen for you on example_playlist.dart?

burhanaksendir commented 1 year ago

I have just implemented an iOS/macOS treadmill. It is available for testing on the feature/treadmill branch.

Currently it is hard coded to be always on because it is actually difficult with the current design to emulate the Android behaviour (that may influence a future API change sot hat lazy loading is a property of the player rather than of the playlist).

Also, the treadmill is hardcoded to hold two items at a time. Ideally the length of the treadmill would dynamically adapt based on the shortness of the items. For example, for short items, you'd want to preload more items onto the treadmill to ensure gapless playback. For longer items, the current and next item would be enough.

Wow, that was fast! Thank you! With that, problem solved.

burhanaksendir commented 1 year ago

Is anyone able to report on their experience with the solution implemented in feature/treadmill? As mentioned, I am reluctant to merge changes to this core code unless people find that it is actually working well for them. Once it's proven to be an effective solution, I can merge it into an official release.

Can you merge this into an official version? Thanks.

ryanheise commented 1 year ago

Thank you for testing it, but I need to hear from people who have been using it for a while to know whether it is actually stable before I merge it.

burhanaksendir commented 1 year ago

Thank you for testing it, but I need to hear from people who have been using it for a while to know whether it is actually stable before I merge it.

Any chance to update feature/treadmill? In feature/treadmill, I cannot access some features in the official release version (eg positionDiscontinuityStream etc..).

ryanheise commented 1 year ago

Updated.

xweng1016 commented 1 year ago

treadmill probably should be merged, been on this branch for like half a year now

ryanheise commented 1 year ago

@xweng1016 and yet you never provided any testing feedback on it. So no, it will not be merged yet.

(Earlier comment:)

Thank you for testing it, but I need to hear from people who have been using it for a while to know whether it is actually stable before I merge it.

mz5210 commented 1 year ago

我测试了502个音乐,批量加入播放列表中,它运行良好,没有卡顿,也许您应该把这个改动合并到master

image