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

FrameworkSampleSource: Attached equaliser is not kept after calling seekTo? #252

Closed
ened opened this issue Jan 21, 2015 · 5 comments
Closed
Assignees

Comments

@ened
Copy link
Contributor

ened commented Jan 21, 2015

Given a player is initialised like this:

mediaPlayer = ExoPlayer.Factory.newInstance(1);

final FrameworkSampleSource sampleSource = new FrameworkSampleSource(this, uri, null, 1);

final MediaCodecAudioTrackRenderer audioTrackRenderer = new MediaCodecAudioTrackRenderer(sampleSource) {
    @Override
    protected void onAudioSessionId(int audioSessionId) {
        equalizer = new Equalizer(1000, audioSessionId);
    }
};

mediaPlayer.addListener(new ExoPlayer.Listener() {
    @Override
    public void onPlayerStateChanged(boolean playWhenReady, int playbackState) {
        log.log(Level.FINEST, "playWhenReady: {0}, state: {1}", new Object[]{playWhenReady, playbackState});

        playerState = playbackState;

        switch (playbackState) {
            case ExoPlayer.STATE_IDLE:
                publishState(State.IDLE);
                break;
            case ExoPlayer.STATE_PREPARING:
                publishState(State.PREPARING);
                break;
            case ExoPlayer.STATE_BUFFERING:
                break;
            case ExoPlayer.STATE_READY:
                setupEqualizer();
                generateMediaStatus();

                mediaPlayer.setPlayWhenReady(true);
                musicQueue.setCurrentPositionPlaying(currentPosition, true);
                publishState(State.STARTED);
                break;
            case ExoPlayer.STATE_ENDED:
                playbackComplete();
                break;
        }
    }

    @Override
    public void onPlayWhenReadyCommitted() {
    }

    @Override
    public void onPlayerError(ExoPlaybackException e) {
    }
});

setupEqualizer() will set some bands etc.

Then the behaviour I noticed is:

  • The equaliser is not attached to the Audio stream right away, but maybe a split second later. This effect is noticeable if you lower all EQ bands and start playing a song that starts with a super high pitch ("Lana Del Rey - National Anthem", for example).
  • As soon as I navigate through the song using seekTo on the mediaPlayer object, the equaliser is completely disabled. Internally, seekTo is calling "stop" - is it possible that the AudioSession is then discarded as well? If so, would it be sensible to call onAudioSessionId again to share the new session?

Test was conducted using revision a879819.

@ojw28
Copy link
Contributor

ojw28 commented Jan 21, 2015

  • I think the Equalizer is attached right away. Your problem is that you're calling setupEqualizer too late, so the bands aren't configured early enough. I think the right place to do your equalizer setup would be in onAudioSessionId, immediately after you instantiate it.
  • We do throw away the AudioTrack and make a new one when seeking (it's more reliable), but the new one is supposed to use the same session id, so this should work. I'll try and get around to looking at this, but if you could debug also, that would be great. In particular, check whether the same (correct) session id is being used each time this code is executed:

https://github.com/google/ExoPlayer/blob/master/library/src/main/java/com/google/android/exoplayer/audio/AudioTrack.java#L257

@ened
Copy link
Contributor Author

ened commented Jan 22, 2015

@ojw28, the code has changed per your suggestion and I moved the EQ setup into the onAudioSession method.

The EQ setup looks like this:

short[] range = equalizer.getBandLevelRange();

equalizer.setBandLevel((short) 0, range[0]);
equalizer.setBandLevel((short) 1, range[0]);
equalizer.setBandLevel((short) 2, range[0]);
equalizer.setBandLevel((short) 3, range[0]);
equalizer.setBandLevel((short) 4, range[0]);

equalizer.setEnabled(true);

When playing the same song again and again, I did notice the EQ is still not activated right away, songs will still play high pitched in their first split second or so. Although, the behaviour is indeed a bit better than before.

Regarding the session ID, I can confirm it is the same.

All tests are done on Android 4.4 (API19), by the way.

@ojw28
Copy link
Contributor

ojw28 commented Jan 23, 2015

  • Were you able to resolve the issue with seekTo? The issue does not reproduce on the device I'm testing with. If this is still an issue, please provide details of the device + exact Android build used. Thanks!
  • Note that in a full solution, you should ensure that you release the equalizer when done. The best place to do this is in onDisabled, as below.
audioRenderer = new MediaCodecAudioTrackRenderer(......) {

  private Equalizer equalizer;

  @Override
  public void onAudioSessionId(int audioSessionId) {
    releaseEqualizer();
    equalizer = new Equalizer(..., audioSessionId);
    // Configure equalizer here.
    equalizer.setEnabled(true);
  }

  @Override
  public void onDisabled() {
    releaseEqualizer();
  }

  private void releaseEqualizer() {
    if (equalizer != null) {
      equalizer.release();
      equalizer = null;
    }
  }

};

@ened
Copy link
Contributor Author

ened commented Jan 23, 2015

Thanks for the input. I've adapted my implementation to properly release the EQ in onDisabled. Both problems remain active.

I'm testing on a custom MTK tablet running Android 4.4.2. A previous solution, even with out the re-released Equalizerdid work well based on AndroidsMediaPlayer`.

@ojw28
Copy link
Contributor

ojw28 commented Mar 25, 2015

This was diagnosed as a platform issue where the effect wasn't being properly included in a reference count associated with the audio session. The issue was fixed in Lollypop, but affected earlier releases of Android.

We've added a workaround that you can use to get correct behavior prior to Lollypop, but we've left it disabled by default because attaching effects isn't a common use case, and we'd rather not have the workaround enabled in all cases (it's pretty hacky). To make use of the workaround, you can simply flip this flag to true:

https://github.com/google/ExoPlayer/blob/dev/library/src/main/java/com/google/android/exoplayer/audio/AudioTrack.java#L100

@ojw28 ojw28 closed this as completed Mar 25, 2015
@google google locked and limited conversation to collaborators Jun 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants