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

File allocating tons of memory #3670

Closed
PaulWoitaschek opened this issue Jan 5, 2018 · 18 comments
Closed

File allocating tons of memory #3670

PaulWoitaschek opened this issue Jan 5, 2018 · 18 comments
Assignees
Labels

Comments

@PaulWoitaschek
Copy link

PaulWoitaschek commented Jan 5, 2018

I have a m4a audio file that when trying to play it just allocates tons of memory in constantly triggers the garbage collection and finally lets the device crash.

I reproduced this on a Nexus 5x with Android 8.1 and ExoPlayer 2.6.0 and 2.6.1

Audio file incoming via mail. Please let me know when I can take down the file.

@tonihei
Copy link
Collaborator

tonihei commented Jan 5, 2018

The problem is that your m4a file contains a video and an audio track and both are very poorly interleaved.

Actually, the video track contains one frame only to display the artwork of the audio file. Because the player needs to present video and audio synchronously, it needs to load the file up to the point where it has encountered all the audio and video data needed to play to a certain timestamp.
In your file all the audio samples are in byte range [323, 292578089] and the single video frame with timestamp 0 is in the byte range [292578090, 292597402]. So to start playing at timestamp 0, the player tries to load the entire 292 MB until it finds the video data for timestamp 0. (see #3481 for a similar issue).

There are multiple possible solutions:

  • If you control the creation of the file:
    -- Move the artwork into the metadata of the file (APIC Id3 frame) instead of the video track.
    -- You can also make sure to interleave the tracks properly. In this case it probably means to put the video data at the beginning.
  • If you don't control the file creation, try disabling the video track by using DefaulTrackSelector.setRendererDisabled or by overriding DefaultRenderersFactory leaving out the video renderer.

@tonihei
Copy link
Collaborator

tonihei commented Jan 5, 2018

Marking as a bug because ExoPlayer shouldn't just crash but rather detect the problem and handle it gracefully.

@tonihei tonihei added the bug label Jan 5, 2018
@PaulWoitaschek
Copy link
Author

Thanks. So when I never use video I can always safely call setRendererDisabled for the video track?

My use case is the linked audiobook player above so it's really all kind of files I don't control.

@PaulWoitaschek
Copy link
Author

So i disabled the video track:

  private fun disableVideoTracks(player : ExoPlayer, trackSelector : DefaultTrackSelector) {
    for (i in 0 until player.rendererCount) {
      val isVideo = player.getRendererType(i) == C.TRACK_TYPE_VIDEO
      trackSelector.setRendererDisabled(i, isVideo)
    }
  }

Should this be done only once when creating the player?
This still does not play the file.

The other think I tried was like suggested and leave out the buildVideoRenderers, but that did not have an effect either:

    val factory = object : DefaultRenderersFactory(context) {
      override fun buildVideoRenderers(
          context: Context?,
          drmSessionManager: DrmSessionManager<FrameworkMediaCrypto>?,
          allowedVideoJoiningTimeMs: Long,
          eventHandler: Handler?,
          eventListener: VideoRendererEventListener?,
          extensionRendererMode: Int,
          out: ArrayList<Renderer>?
      ) {

      }
    }
    player = ExoPlayerFactory.newSimpleInstance(factory, DefaultTrackSelector())

@tonihei
Copy link
Collaborator

tonihei commented Jan 5, 2018

Indeed. Seems like we ignore the selected/enabled tracks when checking whether we are ready to play.
This needs fixing in our code, I'm afraid.

@tonihei
Copy link
Collaborator

tonihei commented Jan 17, 2018

FYI: We are working on a proper fix to allow playback of your file and other files with similar problems.

In the meantime, if this problem is blocking your work and you really would like to have a workaround, you could replace the method getBuffereredDurationUs in ExtractorMediaPeriod with the following code. This allows you to at least disable the video track (with one of the methods mentioned before) and get the audio to play without OOM:

  @Override
  public long getBufferedPositionUs() {
    if (loadingFinished) {
      return C.TIME_END_OF_SOURCE;
    } else if (isPendingReset()) {
      return pendingResetPositionUs;
    }
    long largestQueuedTimestampUs = Long.MAX_VALUE;
    if (haveAudioVideoTracks) {
      // Ignore non-AV tracks, which may be sparse or poorly interleaved.
      int trackCount = sampleQueues.length;
      for (int i = 0; i < trackCount; i++) {
        if (trackIsAudioVideoFlags[i] && trackEnabledStates[i]) {
          largestQueuedTimestampUs = Math.min(largestQueuedTimestampUs,
              sampleQueues[i].getLargestQueuedTimestampUs());
        }
      }
    }
    if (largestQueuedTimestampUs == Long.MAX_VALUE) {
      largestQueuedTimestampUs = getLargestQueuedTimestampUs();
    }
    return largestQueuedTimestampUs == Long.MIN_VALUE ? lastSeekPositionUs
        : largestQueuedTimestampUs;
  }

ojw28 pushed a commit that referenced this issue Jan 23, 2018
When determining the next sample to load, the Mp4Extractor now takes
into account how far one stream is reading ahead of the others.
If one stream is reading ahead more than a threshold (default: 10 seconds),
the extractor continues reading the other stream even though it needs
to reload the source at a new position.

GitHub:#3481
GitHub:#3214
GitHub:#3670

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=182504396
@PaulWoitaschek
Copy link
Author

Any news on this? Still present in 2.7.2.

@tonihei
Copy link
Collaborator

tonihei commented Apr 3, 2018

Unfortunately not. We pushed a fix for the general problem of badly aligned streams. However, your problem is special because it has this strange one-sample video stream with a sample time of zero.

This problem is not really high-priority as it is somewhat unusual. Have you tried the workaround I posted above which at least lets you disable the video stream to play the files?

@PaulWoitaschek
Copy link
Author

If I understand correctly that would mean I had to pull the whole library in? Also I have extensions building /https://github.com/PaulWoitaschek/ExoPlayer-Extensions) for an open source project so I rather wait.

@tonihei
Copy link
Collaborator

tonihei commented Apr 3, 2018

You could also fork just ExtractorMediaSource and ExtractorMediaPeriod and then use your own version to prepare the player instead of the library one.

@PaulWoitaschek
Copy link
Author

I'm getting more and more bug reports with files like this. Could this be pushed in priority?

@ojw28
Copy link
Contributor

ojw28 commented Jan 12, 2019

Maybe we should put a workaround in where we just drop video tracks that consist of a single sample, where that sample is also the last sample in the stream. @tonihei - WDYT?

@tonihei
Copy link
Collaborator

tonihei commented Jan 14, 2019

@PaulWoitaschek Have you tried the workaround I proposed above? That is, fork ExtractorMediaSource and ExtractorMediaPeriod, replace the code I posted above, and then disable the video renderer? That should work for your case until we provide a proper fix.

@ojw28 I'd rather implement the proposed solution [internal ref: b/71624068] if we have the time instead of putting a workaround in code which just works for this one case.

@ojw28
Copy link
Contributor

ojw28 commented Jan 14, 2019

Agreed the proposed solution is what we should aim for long term, but we haven't found time to do that in the past 8 months. So the question is whether we can do something reasonable in a couple of hours to mitigate the issue until we get round to it.

@PaulWoitaschek
Copy link
Author

I received a mail where you asked me to send you the file again.
You automatically replied

Thank you for your email.
Your subject doesn't seem to refer to a GitHub issue using the format "Issue #1234".
We only use this address to receive bugreports for existing GitHub issues and won't be able to answer other question. Please open a new GitHub issue at https://github.com/google/ExoPlayer/issues and fill in the issue template.

Did you receive the link I sent you?

@tonihei
Copy link
Collaborator

tonihei commented Jan 15, 2019

Yes, we received the email. The automatic reply is trigger when the email subject isn't in the "Issue #1234" format. It's just "#3670" in your case. Sorry for the confusion!

ojw28 pushed a commit that referenced this issue Jan 15, 2019
…ion.

The buffered position is currently based on the mimimum queued timestamp of
all AV tracks. If the tracks have unequal lengths, one track continues loading
without bounds as the "buffered position" will always stay at the shorter
track's duration.

This change adds an optional buffer flag to mark the last sample of the
stream. This is set in the Mp4Extractor only so far. ExtractorMediaSource
uses this flag to ignore AV streams in the buffered duration calculation if
they already finished loading.

Issue:#3670
PiperOrigin-RevId: 229359899
ojw28 pushed a commit that referenced this issue Jan 15, 2019
…ion.

The buffered position is currently based on the mimimum queued timestamp of
all AV tracks. If the tracks have unequal lengths, one track continues loading
without bounds as the "buffered position" will always stay at the shorter
track's duration.

This change adds an optional buffer flag to mark the last sample of the
stream. This is set in the Mp4Extractor only so far. ExtractorMediaSource
uses this flag to ignore AV streams in the buffered duration calculation if
they already finished loading.

Issue:#3670
PiperOrigin-RevId: 229359899
@tonihei
Copy link
Collaborator

tonihei commented Jan 18, 2019

Closing as fixed.

@tonihei tonihei closed this as completed Jan 18, 2019
@PaulWoitaschek
Copy link
Author

Thanks a lot!

@google google locked and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants