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

Exact seek for mp3 on Android #650

Closed
addie9000 opened this issue Feb 5, 2022 · 25 comments
Closed

Exact seek for mp3 on Android #650

addie9000 opened this issue Feb 5, 2022 · 25 comments
Assignees
Labels
1 backlog enhancement New feature or request

Comments

@addie9000
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Lack of ability to choose slow but exact seek for mp3 on Android.

When seek on Android (ExoPlayer), the seek is inaccurate.
This is because ExoPlayer prioritize seek speed more than accuracy.
For someone needs accurate seek, ExoPlayer has an option (FLAG_ENABLE_INDEX_SEEKING), but just audio doesn't.

Describe the solution you'd like

Create an option to enable exact seek for mp3 on Android.

Additional context

I'll make a PR for this.

@ryanheise
Copy link
Owner

This is related to #333 . I am definitely in support of this feature, but I'm not sure yet on the API.

There are a number of different seeking strategies and so an API decision will need to be made based on how all of these fit together.

@ryanheise
Copy link
Owner

On the iOS side, we have seek options toleranceBefore and toleranceAfter:

https://developer.apple.com/documentation/avfoundation/avplayer/1388493-seektotime?language=objc

That's a fundamentally different API since it is configured at seek time rather than at load time (something ExoPlayer can't do right now.).

Looking at this, I guess for ExoPlayer we want potentially a bundle of options packaged with each audio source, so that more options can be added to that bundle in the future. I normally would include a darwin and android bundle although at the moment I'm not sure if there are any darwin load time options for this.

@addie9000
Copy link
Contributor Author

I completely agree to the idea of a bundle of options for future flexibility.

Could be like below?

# if we know it's ProgressiveAudioSource
final progressiveAudioSource = ProgressiveAudioSource(audioUri, options: ProgressiveAudioSourceOptions(
        androidOptions: AndroidProgressiveAudioSourceOptions(indexSeeking: true),
        darwinOptions: DarwinProgressiveAudioSourceOptions(toleranceBefore: 1000, toleranceAfter: 1000)));

# if we do not know it's ProgressiveAudioSource
final source = AudioSource.uri(audioUri);
if (source is ProgressiveAudioSource) {
    source.setOptions(ProgressiveAudioSourceOptions(
        androidOptions: AndroidProgressiveAudioSourceOptions(indexSeeking: true),
        darwinOptions: DarwinProgressiveAudioSourceOptions(toleranceBefore: 1000, toleranceAfter: 1000)));
}

As you mentioned, iOS has different API, so put toleranceBefore and toleranceAfter to DarwinProgressiveAudioSourceOptions is questionable.
In my opinion, it's OK to have these options for only each audio source on iOS. And if we can override it on seek method, it's the best, but I think this is optional.

@ryanheise
Copy link
Owner

I'd probably put these specific iOS options only on seek. Of course that doesn't need to go in this PR, but I just want to plan ahead so that future features have a place to go.

I can't remember but is it possible to enable index based seeking and constant bitrate seeking at the same time? That would affect whether these are individual booleans or an enum.

@addie9000
Copy link
Contributor Author

I understand that we are talking about general design, not about this PR. I just hope we could have better feature in the future.

About ExoPlayer, it's possible to set both index based and constant bitrate seeking. There are two more options available which are, constant bitrate seeking always, and disable ID3 metadata. The last one seems very standalone.
About seeking options, doc said that if index based seeking is set, other two options are ignored. And if constant bitrate seeking always is set, constant bitrate seeking is also set implicitly.
Thus, these options are looks exclusive, thus it probably should be an enum.
https://exoplayer.dev/doc/reference/com/google/android/exoplayer2/extractor/mp3/Mp3Extractor.html

@ryanheise
Copy link
Owner

Although see this comment:

google/ExoPlayer#9408 (comment)

I'm still not sure exactly how it works to combine these flags.

@addie9000
Copy link
Contributor Author

Hmm, the comment means that currently these options are exclusive (as a result), but in the future it could be changed to choose the best way from set options automatically, right?

I re-thought about this, it may be safer to stick the same way with platform design.
So how about just leave them as an integer? If so user can merge flags by bitwise or operation.
...Ah, first I thought if it's integer, user can add options that just audio not implement, but because ExoPlayer version is locked by just audio itself, it's not happening. Therefore, looks like individual booleans are the best way at this moment.

@ryanheise
Copy link
Owner

I think the comment means they could be mutually exclusive for MP3 files but not for some other types of files. I'd be fine with a collection of bools or a bitfield. I've done typesafe version of the latter in audio session.

@addie9000
Copy link
Contributor Author

Ah, yes, some of other file types options looks not exclusive. And even for MP3, disable MP3 metadata option is not exclusive.

@snipd-min
Copy link

Hey guys, any plans to include this feature soon? Also I assume applying this flag means that the duration of the mp3 won't be available unless the end of file has been indexed?

@ryanheise
Copy link
Owner

ryanheise commented Jul 12, 2022

I did some recent work on the ExoPlayer seek flags, and in particular implemented support for constant bitrate seeking. Implementing that also finally gave some answers on how ExoPlayer deals with these flags when you combine them.

As for when I'll have time to implement exact seeking (using the method that scans the whole file on the fly to build an index), it's not that this is difficult to implement, you could just edit the line of code in your own copy of just_audio to enable it. But rather, to officially incorporate this into just_audio, the hard work is deciding on an API that is not only sensible for Android, but also makes sense for any future work on the iOS side.

If anyone wants to help, please consider proposing API designs below, keeping in mind that this is an option that must be supplied not at seek time but before the audio is loaded.

One other thing about this index-based seeking is that it is very inefficient for large files to do this on the fly. Ideally you could cache the index for future use, but implementing that is much harder.

Also I assume applying this flag means that the duration of the mp3 won't be available unless the end of file has been indexed?

The duration of the media will be available immediately in any case whenever the actual audio file provides that metadata, and whenever either that metadata is at the front of the file, or, if it's at the back, your HTTP server supports range requests (which are required on iOS anyway, so that's good practice). The exact seek feature would be implemented independently of that.

@sundayz
Copy link

sundayz commented Oct 18, 2022

Is there a workaround while the API is still being worked on? I've got a file that's encoded with a constant bitrate, but on Android the seeking is really inaccurate. It's a rather long file (1 hour) so from what I understand the indexed seeking isn't a good option anyway.

Funnily enough even though the seeking is inaccurate, the position stream is still correct, but I suppose that's expected, right?

@ryanheise
Copy link
Owner

The only workaround that does not involve hacking the code is to transcode your audio file into a format that has a seek table. But if the audio file is not under your control and you don't have a way to programmatically transcode files on the fly, I could only suggest you help this project to get closer to a solution.

The main thing I need before implementing this is a decision on what the API should look like, which takes into account potentially other ways we might want to do seeking in the future so that this API will be future proof.

@sundayz
Copy link

sundayz commented Oct 19, 2022

I'm under a bit of time pressure at the moment so I just need a quick solution, so I will probably encode a 2nd mp4/aac version of my files for the app.

Once my schedule clears up a little bit I would be happy to help design the API. When you say hacking the code, is there a PR or ExoPlayer API specifically I should take a look at to better familiarize myself with this issue?

@ryanheise
Copy link
Owner

You can look at the git log for where I implemented constant bitrate seeking. That would be the same region to apply the indexed seeking flag.

@DamonChen117
Copy link

DamonChen117 commented Nov 30, 2022

I fixed it by a quick and dirty method.

in AudioPlayer.java add

extractorsFactory.setConstantBitrateSeekingEnabled(true);
extractorsFactory.setMp3ExtractorFlags(com.google.android.exoplayer2.extractor.mp3.Mp3Extractor.FLAG_ENABLE_INDEX_SEEKING); // <--- new added

@rovshan-b
Copy link

I fixed it by a quick and dirty method.

in AudioPlayer.java add

extractorsFactory.setConstantBitrateSeekingEnabled(true);
extractorsFactory.setMp3ExtractorFlags(com.google.android.exoplayer2.extractor.mp3.Mp3Extractor.FLAG_ENABLE_INDEX_SEEKING); // <--- new added

Thank you very much!!!

@amugofjava
Copy link

Hi @ryanheise,

Just adding to this thread as I am also having issues with this, particularly when trying to sync transcripts to audio. Adding the FLAG_ENABLE_INDEX_SEEKING flag seems to resolve the issue for me.

In terms of API, we already have Android specific options within the AudioServiceConfig. Could this flag be added to that, perhaps using an enum to represent each of the four option ExoPlayer supports? Thanks.

@ryanheise
Copy link
Owner

AudioServiceConfig is an audio_service API, and just_audio doesn't have a similar all-purpose config object, although it does have various other config objects corresponding to specific APIs.

I would be leaning towards adding another config object for the extractor.

@amugofjava
Copy link

Ah yes! Sorry, should have realised - it's even in the name!

@ryanheise
Copy link
Owner

ryanheise commented Oct 15, 2023

I am currently working on this, and would like some feedback on the proposed API below.

(Edit: I've added a corresponding option for iOS precise seeking)

class ProgressiveAudioSourceOptions {
  final AndroidExtractorOptions? androidExtractorOptions;
  final DarwinAssetOptions? darwinAssetOptions;

  ProgressiveAudioSourceOptions({
    this.androidExtractorOptions,
    this.darwinAssetOptions,
  });
}

class DarwinAssetOptions {
  final bool preferPreciseDurationAndTiming;

  DarwinAssetOptions({this.preferPreciseDurationAndTiming = false});
}

class AndroidExtractorOptions {
  static const flagMp3EnableIndexSeeking = 1 << 2;
  static const flagMp3DisableId3Metadata = 1 << 3;

  final bool constantBitrateSeekingEnabled;
  final bool constantBitrateSeekingAlwaysEnabled;
  final int mp3Flags;

  const AndroidExtractorOptions({
    this.constantBitrateSeekingEnabled = true,
    this.constantBitrateSeekingAlwaysEnabled = false,
    this.mp3Flags = 0,
  });
}

class ProgressiveAudioSource extends UriAudioSource {
  final ProgressiveAudioSourceOptions? options;

  ProgressiveAudioSource(
    super.uri, {
    super.headers,
    super.tag,
    super.duration,
    this.options,
  });
}

This leaves room for mirroring ExoPlayer's other flags for other extractors like FLAC and so on (if someone later requests it), or for mirroring additional iOS options. The options parameters default to the current behaviour, and I have omitted constants for the mp3 flags associated with constant bitrate seeking since we have these dedicated bools which should activate the relevant flag in any extractor that supports it.

@ryanheise

This comment was marked as outdated.

CapmartinLeo added a commit to linto-ai/just_audio that referenced this issue Oct 30, 2023
@ryanheise
Copy link
Owner

ryanheise commented Nov 12, 2023

I've just published the platform interface with the proposed new API, and merged to the minor branch an implementation of the API for Android, iOS and macOS.

Since the platform interface is published, it is now easier to test this branch without having to patch the pubspec files.

Note that index based seeking on Android behaves exactly the way ExoPlayer implemented it natively which might be a bit surprising. In particular, enabling this option may cause it to return a null duration, and if you depend on the duration to render the seek bar, then you'll have no input to be able to actually seek. The reason this happens is that it only indexes as much as it loads, and enabling this option does not affect your preferences for how the file should be loaded. The way to get it to return the duration is effectively to cause it to buffer the whole file, either through the load params, or by seeking to the end of the file. Once the end of the file is reached, the duration is known. Of course without first knowing the duration, you won't know exactly how far to seek, or with the load params how much to buffer to get the whole file and hence the duration. But if you pick some really large duration, that will do the trick.

I know that's not the ideal behaviour, but that's unfortunately the way this parameter works in ExoPlayer.

ExoPlayer only recommends using index based seeking on variable bitrate files, since then it can actually return the duration instantly, whereas it tends to have the problem more with constant bitrate files. So they recommend not using it for that. The problem is that there is no way we can detect in advance whether a file is variable or constant bitrate, so that recommendation doesn't really help. In other words, you'll probably have to enable index based seeking on all mp3 files, and then forcefully trigger the whole file to load using one of the hacks mentioned above.

I suggest at least detecting in advance whether the media is something like m4a which already has an accurate seek table, and then not using index based seeking in that case.

@ryanheise
Copy link
Owner

The above changes have now been published on pub.dev so I'll close this, although see the comment above for usage hints.

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1 backlog enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants