ryanheise / audio_service

Flutter plugin to play audio in the background while the screen is off.
806 stars 481 forks source link

Support artUri as absolute path to an image. #245

Closed Tizoner closed 4 years ago

Tizoner commented 4 years ago

Is your feature request related to a problem? Please describe. I can't assign an absolute image path to the artUri within MediaItem constructor. I have url of an image but I need to resize it so I have to download it at first then modify it's ratio and then use it for notification bar. I know that there already been similar issue #69 on 10 Jun 2019 but it doesn't seem that it's solved even though it's closed.

Describe the solution you'd like I would like to have local audio file image in notification bar

ryanheise commented 4 years ago

Hi @Tizoner

I've just fixed this in the latest commit, in that you can now use a "file://" URI as your artUri. If you have a file path, you could pass a value like "file://$path".

Can you try the latest commit on master and let me know if it solves your issue?

Tizoner commented 4 years ago

My image is located in external storage. So for me path = "/storage/emulated/0/Android/data/com.example.app_name/files/img.jpg" When I ran the following code: print(File(path).existsSync()); String artUri = File(path).uri.toString(); print(artUri);

I get the following output in debug console: true file:///storage/emulated/0/Android/data/com.example.app_name/files/img.jpg

And when I try to call AudioServiceBackground.setMediaItem using the MediaItem with such artUri image does not appear in the notification. But if I use url to an online image as artUri then everything works.

ryanheise commented 4 years ago

It would help if you included the logs. Are you sure it's not just a permission problem?

Tizoner commented 4 years ago

I'm grateful for all of your work on this package - it has more features, simpler and works better than any alternative that I've tested. Right now I'm trying to build an amazing app that might inspire you but for now I'm stuck with this problem. I can't attach the whole code of the app because it consists of multiple files with hundreds lines of code. But I'm sure it's not just a permission problem. Besides the image, there is a whole database in the external storage that works without any problems. My app has the following permissions:

<uses-permission android:name="android.permission.INTERNET"/>
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.WAKE_LOCK"/>
<uses-permission android:name="android.permission.FOREGROUND_SERVICE"/>

I'm using audio_service package with audioplayers package so keep this in mind while reading the logs: Launching lib\main.dart on SM M307FN in debug mode... √ Built build\app\outputs\apk\debug\app-debug.apk. D/FlutterActivity( 838): Using the launch theme as normal theme. D/FlutterActivityAndFragmentDelegate( 838): Setting up FlutterEngine. D/FlutterActivityAndFragmentDelegate( 838): No preferred FlutterEngine was provided. Creating a new FlutterEngine for this FlutterFragment. D/FlutterActivityAndFragmentDelegate( 838): Attaching FlutterEngine to the Activity that owns this Fragment. D/FlutterView( 838): Attaching to a FlutterEngine: io.flutter.embedding.engine.FlutterEngine@a3a2296 D/FlutterActivityAndFragmentDelegate( 838): Executing Dart entrypoint: main, and sending initial route: / D/MediaBrowserCompat( 838): Connecting to a MediaBrowserService. I/flutter ( 838): true I/flutter ( 838): file:///storage/emulated/0/Android/data/com.example.music_taming/files/img.jpg V/MediaPlayer-JNI( 838): native_setup V/MediaPlayerNative( 838): constructor V/MediaPlayerNative( 838): setListener V/MediaPlayer-JNI( 838): setVolume: left 1.000000 right 1.000000 V/MediaPlayerNative( 838): MediaPlayer::setVolume(1.000000, 1.000000) V/MediaPlayer-JNI( 838): setAuxEffectSendLevel: level 0.000000 V/MediaPlayerNative( 838): MediaPlayer::setAuxEffectSendLevel(0.000000) V/MediaPlayer-JNI( 838): setParameter: key 1400 V/MediaPlayerNative( 838): MediaPlayer::setParameter(1400) V/MediaPlayer-JNI( 838): setVolume: left 1.000000 right 1.000000 V/MediaPlayerNative( 838): MediaPlayer::setVolume(1.000000, 1.000000) V/MediaPlayer-JNI( 838): setLooping: 0 V/MediaPlayerNative( 838): MediaPlayer::setLooping V/MediaHTTPService( 838): MediaHTTPService(android.media.MediaHTTPService@f883ff): Cookies: null V/MediaPlayer-JNI( 838): setVolume: left 1.000000 right 1.000000 V/MediaPlayerNative( 838): MediaPlayer::setVolume(1.000000, 1.000000) V/MediaPlayer-JNI( 838): setLooping: 0 V/MediaPlayerNative( 838): MediaPlayer::setLooping V/MediaPlayerNative( 838): setVideoSurfaceTexture V/MediaPlayerNative( 838): prepareAsync I/flutter ( 838): true V/MediaHTTPService( 838): makeHTTPConnection: CookieManager created: java.net.CookieManager@84215cc I/flutter ( 838): file:///storage/emulated/0/Android/data/com.example.music_taming/files/img.jpg V/MediaHTTPService( 838): makeHTTPConnection(android.media.MediaHTTPService@f883ff): cookieHandler: java.net.CookieManager@84215cc Cookies: null D/MediaHTTPConnection( 838): setReadTimeOut = 15000ms D/NetworkSecurityConfig( 838): No Network Security Config specified, using platform default D/MediaHTTPConnection( 838): setReadTimeout with 15000ms I/System.out( 838): (HTTPLog)-Static: isSBSettingEnabled false I/System.out( 838): (HTTPLog)-Static: isSBSettingEnabled false D/NetworkManagementSocketTagger( 838): tagSocket(115) with statsTag=0xffffffff, statsUid=-1 I/MediaHTTPConnection( 838): response code = 200 D/MediaHTTPConnection( 838): setReadTimeout with 15000ms I/System.out( 838): (HTTPLog)-Static: isSBSettingEnabled false I/System.out( 838): (HTTPLog)-Static: isSBSettingEnabled false D/NetworkManagementSocketTagger( 838): tagSocket(115) with statsTag=0xffffffff, statsUid=-1 I/MediaHTTPConnection( 838): response code = 206 D/MediaHTTPConnection( 838): setReadTimeout with 15000ms I/System.out( 838): (HTTPLog)-Static: isSBSettingEnabled false I/System.out( 838): (HTTPLog)-Static: isSBSettingEnabled false I/MediaHTTPConnection( 838): response code = 200 D/MediaHTTPConnection( 838): setReadTimeout with 15000ms I/System.out( 838): (HTTPLog)-Static: isSBSettingEnabled false I/System.out( 838): (HTTPLog)-Static: isSBSettingEnabled false D/NetworkManagementSocketTagger( 838): tagSocket(115) with statsTag=0xffffffff, statsUid=-1 I/MediaHTTPConnection( 838): response code = 206 V/MediaPlayerNative( 838): message received msg=200, ext1=10973, ext2=0 W/MediaPlayerNative( 838): info/warning (10973, 0) V/MediaPlayerNative( 838): callback application V/MediaPlayerNative( 838): back from callback D/MediaHTTPConnection( 838): setReadTimeout with 15000ms I/System.out( 838): (HTTPLog)-Static: isSBSettingEnabled false I/System.out( 838): (HTTPLog)-Static: isSBSettingEnabled false I/MediaHTTPConnection( 838): response code = 200 V/MediaPlayerNative( 838): message received msg=3, ext1=4, ext2=0 V/MediaPlayerNative( 838): buffering 4 V/MediaPlayerNative( 838): callback application V/MediaPlayerNative( 838): back from callback V/MediaPlayerNative( 838): message received msg=1, ext1=0, ext2=0 V/MediaPlayerNative( 838): MediaPlayer::notify() prepared V/MediaPlayerNative( 838): callback application V/MediaPlayerNative( 838): back from callback V/MediaPlayerNative( 838): invoke 68 V/MediaPlayerNative( 838): getDuration_l V/MediaPlayer-JNI( 838): getDuration: 190093 (msec) V/MediaPlayer-JNI( 838): start V/MediaPlayerNative( 838): start V/MediaPlayerNative( 838): message received msg=300, ext1=0, ext2=0 V/MediaPlayerNative( 838): Received SEC_MM_PLAYER_CONTEXT_AWARE V/MediaPlayerNative( 838): callback application V/MediaPlayerNative( 838): back from callback V/MediaPlayerNative( 838): getDuration_l V/MediaPlayer-JNI( 838): getDuration: 190093 (msec) V/MediaPlayer-JNI( 838): getCurrentPosition: 0 (msec) V/MediaPlayerNative( 838): message received msg=3, ext1=12, ext2=0 V/MediaPlayerNative( 838): buffering 12 V/MediaPlayerNative( 838): callback application V/MediaPlayerNative( 838): back from callback V/MediaPlayerNative( 838): message received msg=6, ext1=0, ext2=0 V/MediaPlayerNative( 838): unrecognized message: (6, 0, 0) V/MediaPlayerNative( 838): callback application V/MediaPlayerNative( 838): back from callback V/MediaPlayer-JNI( 838): getCurrentPosition: 0 (msec) V/MediaPlayerNative( 838): getDuration_l V/MediaPlayer-JNI( 838): getDuration: 190093 (msec) V/MediaPlayer-JNI( 838): getCurrentPosition: 0 (msec) V/MediaPlayerNative( 838): message received msg=211, ext1=0, ext2=0 V/MediaPlayerNative( 838): unrecognized message: (211, 0, 0) V/MediaPlayerNative( 838): callback application V/MediaPlayerNative( 838): back from callback V/MediaPlayerNative( 838): message received msg=211, ext1=0, ext2=0 V/MediaPlayerNative( 838): unrecognized message: (211, 0, 0) V/MediaPlayerNative( 838): callback application V/MediaPlayerNative( 838): back from callback V/MediaPlayerNative( 838): getDuration_l V/MediaPlayer-JNI( 838): getDuration: 190093 (msec) V/MediaPlayer-JNI( 838): getCurrentPosition: 148 (msec) V/MediaPlayerNative( 838): getDuration_l V/MediaPlayer-JNI( 838): getDuration: 190093 (msec) V/MediaPlayer-JNI( 838): getCurrentPosition: 352 (msec) V/MediaPlayerNative( 838): getDuration_l V/MediaPlayer-JNI( 838): getDuration: 190093 (msec) V/MediaPlayer-JNI( 838): getCurrentPosition: 555 (msec) V/MediaPlayerNative( 838): message received msg=3, ext1=75, ext2=0 V/MediaPlayerNative( 838): buffering 75 V/MediaPlayerNative( 838): callback application V/MediaPlayerNative( 838): back from callback V/MediaPlayerNative( 838): getDuration_l V/MediaPlayer-JNI( 838): getDuration: 190093 (msec) V/MediaPlayer-JNI( 838): getCurrentPosition: 759 (msec) ... And so on.

In fact, then I try to use version 0.6.2 my code works great and I can see the local image in the notification. I've read source code and documentation of the audio_service package hundred times before this new version (0.7.0). So it didn't take a lot of time to find a problematic parts of the code: https://pub.dev/documentation/audio_service/latest/audio_service/AudioServiceBackground/setMediaItem.html Here, the code tries to find the image in the cache and if it fails it will try to use getSingeFile method of the BaseCache manager. But according to the source code of this method (https://pub.dev/documentation/flutter_cache_manager/latest/flutter_cache_manager/BaseCacheManager/getSingleFile.html), it will try to get the file from the cache or from the online. But my image is in the external storage, that is not cache nor online source. I may be wrong but I think this is the reason why with this new version I can't no longer use a local images with absolute path for MediaItem. So for now I'll stick to the previous version of the package (0.6.2).

ryanheise commented 4 years ago

Can you confirm that you did actually try the latest commit on master? In your reply, you referred to 0.7.0 which does not contain the fix, only the latest commit on master does.

ryanheise commented 4 years ago

Apologies! I forgot to commit it to master. Can you try master again?

Tizoner commented 4 years ago

I've tried it again and it does not work. After reading your commit it became obvious for me why it does not work - the problem is in the setMediaItem (https://pub.dev/documentation/audio_service/latest/audio_service/AudioServiceBackground/setMediaItem.html):

static Future<void> setMediaItem(MediaItem mediaItem) async {
  _mediaItem = mediaItem;
  if (mediaItem.artUri != null) {
    // We potentially need to fetch the art.
    final fileInfo = await _cacheManager.getFileFromMemory(mediaItem.artUri);
    File file = fileInfo?.file;
    if (file == null) {
      // We haven't fetched the art yet, so show the metadata now, and again
      // after we load the art.
      await _backgroundChannel.invokeMethod(
          'setMediaItem', _mediaItem2raw(mediaItem));
      // Load the art
      file = await _cacheManager.getSingleFile(mediaItem.artUri);
      // If we failed to download the art, abort.
      if (file == null) return;
      // If we've already set a new media item, cancel this request.
      if (mediaItem != _mediaItem) return;
    }
    final extras = Map.of(mediaItem.extras ?? <String, dynamic>{});
    extras['artCacheFile'] = file.path;
    final platformMediaItem = mediaItem.copyWith(extras: extras);
    // Show the media item after the art is loaded.
    await _backgroundChannel.invokeMethod(
        'setMediaItem', _mediaItem2raw(platformMediaItem));
  } else {
    await _backgroundChannel.invokeMethod(
        'setMediaItem', _mediaItem2raw(mediaItem));
  }
}

In your commit, you add resulting path to the extras, instead of modifying artUri. This leads to the problems:

file = await _cacheManager.getSingleFile(mediaItem.artUri);
// If we failed to download the art, abort.
if (file == null) return;

As you can see, it will try to use getSingleFile method of the BaseCacheManager class that will search for that file either in cache or online but not in external storage. Actually, I prefer the source code of the version 0.6.2 because in my opinion it's way cleaner and more efficient:

static Future<void> setMediaItem(MediaItem mediaItem) async {
    await _backgroundChannel.invokeMethod(
        'setMediaItem', _mediaItem2raw(mediaItem));
}

Remember (#132) when other users asked you to add the feature such that they can pass extras to the MediaItem? Why do you think they asked you to do this? Because dart throws compilation error if they'll try to add properties at runtime for the instance of the class that does not define those properties. You thought that this was a good idea to add this feature but just look at how many more problems it raises to your source code.
Those users were just too lazy to override the MediaItem class to describe a behaviour that they desire. Do you know what they do when they need some new methods for the MediaItem class? No, they don't override the MediaItem class with any methods they need. They just create a bunch of functions (sometimes even in the global scope) and they think that this is not a stupid idea. But when they need a new property for the MediaItem class they understand that this might be not the best idea to define a global variable for a such purpose. That's why they just complained to you. You were should just refused to add that useless (in my opinion) feature. If they need to store a path to the cache file they really just can easilly override MediaItem class and add property to store that and any other path.

Also, I wonder why you removed shouldPreloadArtwork argument from the AudioService.start method. It was nice to have the option to be able to store or not to store art images in the cache. If you think that AudioService.start method should preload the art image by default then just set it's default value to true instead of completely removing this argument. For example, I don't wan't to store the art images in the cache. But with this new version you leave me no choice.

ryanheise commented 4 years ago

The commit you're describing sounds like the commit that was released in 0.7.0, and that certainly introduced the bug. But the commit I'm referring you to is the one on git master, which corrects this bug. i.e.:

https://github.com/ryanheise/audio_service/commit/7f32fd041f8b4251652449e7242dd5cdc2b04bc0

Can you confirm that you have tried this commit?

Tizoner commented 4 years ago

Yes, I'm talking exactly about this commit.

ryanheise commented 4 years ago

I don't think so, since the fix is in the _loadArtwork method. The code you quoted:

file = await _cacheManager.getSingleFile(mediaItem.artUri);
// If we failed to download the art, abort.
if (file == null) return;

is the old code. The new code is:

filePath = await _loadArtwork(mediaItem);
// If we failed to download the art, abort.
if (filePath == null) return;
Tizoner commented 4 years ago

Yes, but in pubspec.yaml I use the following version and this new commit didn't change resulting code behaviour: audio_service: ^0.7.0

Also, this new commit didn't update the source code from the documentation which is strange to me(https://pub.dev/documentation/audio_service/latest/audio_service/AudioServiceBackground/setMediaItem.html)

How can I possibly test this new commit without downloading the whole source code? If this commit didn't update the package version then it should affect it's version (e.g. 0.7.0).

Or you want me to test it by downloading the whole source code before you'll update the package version to make sure that everything works correct?

ryanheise commented 4 years ago

Hi @Tizoner ,

The fix is on git master. There has not been a new release since the 0.7.0 version that you reported as not working. When someone requests a bug fix, I generally do not make a new public-wide release until after the original reporter confirms that the fix works for them.

If you downloaded git master manually or with git clone, you can try this dependency in your pubspec:

  audio_service:
    path: /path/to/where/you/downloaded/it/do

Or, you can get it to download automatically in your pubspec like this:

  audio_service:
    git:
      url: git@github.com:ryanheise/audio_service.git

Or depending on how you use git, you could also use an https URL:

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

I did test the fix myself and it worked for me, but I would appreciate if you could also let me know how it goes for you. Hopefully well, in which case I can include it in the next release.

Tizoner commented 4 years ago

Thank you, I've already looked at the pubspec.yaml in the example folder and understood how to add a package to the pubspec.yaml using relative path. I've tried to download and use source code and it works. But I still have some questions on that method:

/// Sets the currently playing media item and notifies all clients.
static Future<void> setMediaItem(MediaItem mediaItem) async {
  _mediaItem = mediaItem;
  if (mediaItem.artUri != null) {
    // We potentially need to fetch the art.
    final fileInfo = await _cacheManager.getFileFromMemory(mediaItem.artUri);
    String filePath = fileInfo?.file?.path;
    if (filePath == null) {
      // We haven't fetched the art yet, so show the metadata now, and again
      // after we load the art.
      await _backgroundChannel.invokeMethod(
          'setMediaItem', _mediaItem2raw(mediaItem));
      // Load the art
      filePath = await _loadArtwork(mediaItem);
      // If we failed to download the art, abort.
      if (filePath == null) return;
      // If we've already set a new media item, cancel this request.
      if (mediaItem != _mediaItem) return;
    }
    final extras = Map.of(mediaItem.extras ?? <String, dynamic>{});
    extras['artCacheFile'] = filePath;
    final platformMediaItem = mediaItem.copyWith(extras: extras);
    // Show the media item after the art is loaded.
    await _backgroundChannel.invokeMethod(
        'setMediaItem', _mediaItem2raw(platformMediaItem));
  } else {
    await _backgroundChannel.invokeMethod(
        'setMediaItem', _mediaItem2raw(mediaItem));
  }
}
// We haven't fetched the art yet, so show the metadata now, and again
// after we load the art.

Why it tries to set mediaItem, preload art, and then set mediaItem again? I ask because even with the previous version (I've tested it with 0.6.2) this code will display metadata anyway before the image loads:

static Future<void> setMediaItem(MediaItem mediaItem) async {
  await _backgroundChannel.invokeMethod(
      'setMediaItem', _mediaItem2raw(mediaItem));
}
ryanheise commented 4 years ago

The code logic is actually the same as 0.6.2 except before this logic was implemented in the Android platform side. It takes time to load the artwork, so we first display the title and album, and any media controls that need to go in the notification first, and then after the artwork has loaded, we update the notification a second time, this time with the artwork. Part of the reason for moving that code into the Flutter side was so that it could be shared by both platforms (Android and iOS). This explains why the code does not look as simple as it did before, but actually it's just a matter of moving that code from one layer to another layer where it would be more reusable.

Note that there are several other reasons for this move which are explained in the original issues, including the ability to move the preloading code into the Flutter side where it can also be used on iOS, not just Android. Another reason is that the Android implementation was always implementing its own caching, but in a very memory intensive way. Bitmaps is a notorious cause of OutOfMemoryErrors in Android, and the correct caching solution is to use some combination of memory and disk caches. The original commit that went into the 0.7.0 release implemented the disk cache part of it. The memory cache still is not an LRU cache, although its efficiency has been improved through the use of image scaling.

Preloading and caching are separate features. In the old version, even if you turned preloading off, caching was always turned on, and there was no way to turn it off. Now, caching has become more advanced through the combination of both memory and disk caches. You still can't turn the memory cache off (although it is more efficient now anyway), but you can control the disk cache through the flutter cache manager API which gives you complete freedom to be able to implement any use case that I can think of. But the most common use case is handled by default, which is the most important goal. For most apps that use cached_network_image, this happens to use the very same cache manager and this has the benefit that artwork doesn't need to be downloaded twice: once by your Flutter UI that displays the artwork itself, and then once again by the Android notification. Because the cache manager is now exposed, it is possible to share the cache between those different parts of your Flutter application, and in fact cached_network_images uses the same cache by default.

I am sorry if this feature hasn't been implemented in the way you like, but I do my best to weigh a lot of different factors and other requests, and it is also an open source effort so I hope you can appreciate that I am donating my time to help implement everyone's feature requests. Therefore, if someone complains, I can only plea that they consider helping in the collaborative open source spirit.

github-actions[bot] commented 3 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 audio_service.