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

Memory leak while using StreamAudioSource (Even after calling dispose) #1201

Open
sibias opened this issue Mar 5, 2024 · 8 comments
Open
Assignees
Labels
1 backlog bug Something isn't working

Comments

@sibias
Copy link

sibias commented Mar 5, 2024

Which API doesn't behave as documented, and how does it misbehave?
GC failed to remove instance of StreamAudioSource following the execution of audio play. This is confirmed using flutter dev tools (memory). So if the user executes audio play 50 times there will be 50 instances of AudioStreamSource in below example.

    ByteData res = await rootBundle.load('assets/test.mp3');
    player = AudioPlayer();  
    await player.setAudioSource(AudioStreamSource(res.buffer.asUint8List()));            
    await player.play();
    player.dispose();

// Implementation of AudioStreamSource given below

    class AudioStreamSource extends StreamAudioSource {
      final Uint8List _buffer;
    
      AudioStreamSource(this._buffer) : super(tag: 'MyAudioSource');
    
      @override
      Future<StreamAudioResponse> request([int? start, int? end]) async {
        // Returning the stream audio response with the parameters
        return StreamAudioResponse(
          sourceLength: _buffer.length,
          contentLength: (end ?? _buffer.length) - (start ?? 0),
          offset: start ?? 0,
          stream: Stream.fromIterable([_buffer.sublist(start ?? 0, end)]),
          contentType: 'audio/mpeg',
        );
      }

Minimal reproduction project
Provide a link here using one of two options:
Link to example

To Reproduce (i.e. user steps, not code)
Steps to reproduce the behavior:

  1. Run the example in above link (main.dart) and press "Play Byte Stream" button.
  2. Open Dev tools in browser and watch memory tab
  3. Watch the instances count of AudioStreamSource or TestStreamAudioSource or LockCachingAudioSource after each audio play

Error messages

N/A

Expected behavior
All instances of classes derived from StreamAudioSource should be removed from memory by clearing all retention paths following dispose or based on scope

Screenshots
image

Retention paths
image

Desktop (please complete the following information):
Not running on Desktop / Web

Smartphone (please complete the following information):

  • Device: Both iPhone and Android
  • OS: Both iOS and Android

Flutter SDK version

Flutter SDK - 3.19

Additional context
N/A

@sibias sibias added 1 backlog bug Something isn't working labels Mar 5, 2024
@ryanheise
Copy link
Owner

Minimal reproduction project
Provide a link here using one of two options: "The example".

You can't say "The example" when the example doesn't use StreamAudioSource. This is your second strike. Please get the issue submission right on the 3rd time.

sibias added a commit to sibias/just_audio that referenced this issue Mar 6, 2024
@sibias
Copy link
Author

sibias commented Mar 6, 2024

Hi @ryanheise, updated the "Minimal reproduction project" with link to forked project with modifications in example.

@gwhizofss
Copy link

Minimal reproduction project
Provide a link here using one of two options: "The example".

You can't say "The example" when the example doesn't use StreamAudioSource. This is your second strike. Please get the issue submission right on the 3rd time.

It looks like the sample code has been updated to use the StreamAudioSource starting at line 247. We are wanting to use this player, but need to use HLS streaming. Is this bug a showstopper for us in that case? @ryanheise @sibias

class AudioStreamSource extends StreamAudioSource {
  final Uint8List _buffer;

  AudioStreamSource(this._buffer) : super(tag: 'MyAudioSource');

  @override
  Future<StreamAudioResponse> request([int? start, int? end]) async {
    // Returning the stream audio response with the parameters
    return StreamAudioResponse(
      sourceLength: _buffer.length,
      contentLength: (end ?? _buffer.length) - (start ?? 0),
      offset: start ?? 0,
      stream: Stream.fromIterable([_buffer.sublist(start ?? 0, end)]),
      contentType: 'audio/mpeg',
    );
  }
}

class TestStreamAudioSource extends StreamAudioSource {
  TestStreamAudioSource({dynamic tag}) : super(tag: tag);

  @override
  Future<StreamAudioResponse> request([int? start, int? end]) async {
    return StreamAudioResponse(
      contentType: 'audio/mock',
      stream: Stream.value(byteRangeData.sublist(start ?? 0, end)),
      contentLength: (end ?? byteRangeData.length) - (start ?? 0),
      offset: start ?? 0,
      sourceLength: byteRangeData.length,
    );
  }
}

@Colton127
Copy link

Colton127 commented Sep 11, 2024

This issue isn't exclusive to StreamAudioSource, but it is the most troublesome because it opens streams that consume a lot more resources/RAM than other audio sources.

Once set, AudioSources are stored in this map:

final Map<String, AudioSource> _audioSources = {};

This map is only cleared when dispose() is called:

_audioSources.clear();

Thus, when you set multiple audio sources, each AudioSource is kept alive and Dart GC isn't able to dispose the object correctly.

My proposed solution is to remove the Map<String, AudioSource> altogether, and simply dispose the existing AudioSource when a new AudioSource is set. The previous AudioSource can probably be disposed here without issue:

_audioSource = null;

Additionally, it would be helpful to expose dispose and even setup methods for custom audio sources. This way we can manually clean up any open streams, if necessary.

Edit: The memory overhead of storing all AudioSource's in this map is very minimal. As ryanheise pointed out, this doesn't keep any audio buffers alive. Just the AudioSource object itself. The only way this would have an impact of significance is if hundreds of different AudioSource's were set overtime.

@ryanheise
Copy link
Owner

The Dart side doesn't affect memory management, since the native side is what ultimately holds the audio buffers in memory via its own caching. It may be a consideration in the future to have a dispose method on audio sources to instruct the native side to free memory for just one audio source, but at the moment, there is only a dispose method on the player itself. There are also the various platform specific buffering parameters you can pass into the AudioPlayer constructor to tell just_audio how much to hold in memory.

@eivihnd
Copy link

eivihnd commented Sep 12, 2024

In other words, the current recommended practice/workaround is to dispose the player after each (or x) playback(s) in order to avoid this memory leak?

@ryanheise
Copy link
Owner

It would be a case-by-case basis, because there appear to be different scenarios being described, and I don't think they are all the same type of thing. For example, there is a memory issue in the implementation of the lock caching audio source specifically, which is on the Dart side. Outside of that, it has been reported that StreamAudioSource implementations besides that one have their own issues. And then outside of that, it has been reported that audio sources besides StreamAudioSource have an issue. These are 3 different issues with not necessarily the same workaround.

It would help we had a separate issue submission for each of these. In your case, which of the 3 cases is it?

@eivihnd
Copy link

eivihnd commented Sep 12, 2024

In my case it is the StreamAudioSource.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 backlog bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants