10

I know this is a common question, however this stack trace shows something else is wrong. You can see that even though setDisplay(holder) is called inside of surfaceCreated it still throws IllegalArgumentException. This isn't a rare exception either, yesterday happening ~125,000 times in ~3,000,000 clip views. I can assure you that mCurrentPlayer is initialized correctly as well.

surfaceCreated:

@Override
public void surfaceCreated(SurfaceHolder holder) {
    mIsSurfaceCreated = true;
    mCurrentPlayer.setDisplay(holder);
}

surfaceDestroy:

@Override
public void surfaceDestroyed(SurfaceHolder holder) {
    mIsSurfaceCreated = false;

    // Could be called after player was released in onDestroy.
    if (mCurrentPlayer != null) {
        mCurrentPlayer.setDisplay(null);
    }
}

Stacktrace:

java.lang.IllegalArgumentException: The surface has been released
    at android.media.MediaPlayer._setVideoSurface(Native Method)
    at android.media.MediaPlayer.setDisplay(MediaPlayer.java:660)
    at com.xxx.xxx.view.VideoPlayerView.surfaceCreated(VideoPlayerView.java:464)
    at android.view.SurfaceView.updateWindow(SurfaceView.java:543)
    at android.view.SurfaceView.access$000(SurfaceView.java:81)
    at android.view.SurfaceView$3.onPreDraw(SurfaceView.java:169)
    at android.view.ViewTreeObserver.dispatchOnPreDraw(ViewTreeObserver.java:590)
    at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1644)
    at android.view.ViewRootImpl.handleMessage(ViewRootImpl.java:2505)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loop(Looper.java:154)
    at android.app.ActivityThread.main(ActivityThread.java:4945)
    at java.lang.reflect.Method.invokeNative(Native Method)
    at java.lang.reflect.Method.invoke(Method.java:511)
    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:784)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:551)
    at dalvik.system.NativeStart.main(Native Method)

Any ideas on what else could be going wrong? Is SurfaceHolder potentially destroying the surface on a background thread and then waiting for the main thread (currently occupied by surfaceCreated) to finish it's block before it can call surfaceDestroyed on the main thread (which I don't even think locks can fix)? Something else?

Update -- After drilling down a little farther I found out what causes "The surface has been released" to be thrown:

Which references android_view_Surface_getSurface which can be found here:

This is where my lack of C++ knowledge hurts, it's looking like it's trying to lock onto the surface, and if it can't the surface returned will be null. Once it gets returned as null, IllegalArgumentException will be thrown.

Girish Nair
  • 5,148
  • 5
  • 40
  • 61
bclymer
  • 6,679
  • 2
  • 27
  • 36
  • Do you do any rendering on a background thread? What is your implementation of surfaceDestroyed? (Note that the docs for surfaceDestroyed say: " If you have a rendering thread that directly accesses the surface, you must ensure that thread is no longer touching the Surface before returning from this function.".) – Dan Breslau Aug 28 '13 at 18:18
  • There is no background rendering, the `Surface` is only used by the `MediaPlayer` to display video. Once `mp.setDisplay(surface)` is called I do not touch the surface. I updated my post with surfaceDestroyed also. Note that according to android_media_MediaPlayer.cpp, if `jsurface` is `null` this exception will not be thrown. – bclymer Aug 28 '13 at 18:30
  • 1
    Can you share the project? It's highly probable that the issue is not in the code you show but in some other part of the app. – allprog Aug 29 '13 at 14:12
  • @allprog I can't share it sadly, it's owned by my employer. I can say the `SurfaceHolder` isn't touched except for `getHolder().setFixedSize(0, 0);` in `onMeasure` after the one `MediaPlayer` has an error, before the second `MediaPlayer` buffers itself enough to call `requestLayout` again to reset the `SurfaceHolder`'s size and display it's content. – bclymer Aug 29 '13 at 14:24
  • Are you calling `setDisplay()` before or after `mp.prepare()`? The docs say it can be called in any state, but I remember having issues if called after prepare. Can't say if it was this same trace, it was some time ago. – Geobits Aug 29 '13 at 14:27
  • Can you put together a small project that reproduces the error? – allprog Aug 29 '13 at 14:31
  • @Geobits both, upon loading the fragment hosting the `Surface`, I call `setDisplay()` as soon as `surfaceCreated()` is fired. I also call `setDisplay()` on a 2nd instance of `MediaPlayer` after the first `MediaPlayer` finishes playing it's content. Then those swap back and forth as primary vs. buffering. That seems unrelated to the message about the surface being released though. But maybe I'm wrong. – bclymer Aug 29 '13 at 14:32
  • @allprog I will do what I can. My team has never repro'd this in house, despite having it happen a lot in production. I will try to simplify the code to be as accurate as possible though. – bclymer Aug 29 '13 at 14:34
  • 1
    Is it possible that the issue arises when the phone comes back from locked state? – allprog Aug 29 '13 at 14:55
  • 1
    I had issues like this in the past using Android VideoViews/MediaPlayers. It turned out that the underlying SurfaceView was getting garbage collected. I solved it by adding an onPreparedLister to the MediaPlayer, and then holding an explicit reference to it in my class while I was using it. Maybe this helps. – mszaro Sep 18 '13 at 00:56
  • @Sparky, I got pulled off Android for a little bit to do some iOS 7 updating, but I will test this ASAP. If it was just a GC issue I will be both furious at Android and super thankful to you! – bclymer Sep 18 '13 at 17:39
  • @bclymer, good luck - my experiences with surface views and videos were neither straightforward nor pleasant. I ended up having to subclass & roll my own media player and video view, just to be able to make sure the video's aspect ratio was preserved. :( – mszaro Sep 18 '13 at 18:20
  • @Sparky I finally got back to Android, released a build with explicit refs to the surface and surface holder, dropped the error count from 41,000 per day to 70. Can't thank you enough for that. If you want to add your comment as an answer I'll gladly accept it! – bclymer Oct 29 '13 at 18:02
  • Sure, thanks. Adding as an answer. – mszaro Nov 07 '13 at 00:19

2 Answers2

10

I've just fighted a similar problem.

And my investigation shows, that there is a bug in the SurfaceView, which causes passing invalid surface to the surfaceCreated callback method.

This is the commit in the android repo that fixes it: link.

It seems that fix in android sources was introduced in 4.2 version. And, from crashes of application I see that crash caused by invalid surface occured only on 4.0 and 4.1.

So, I can assume that before 4.0 it was valid to pass invalid surface to the MediaPlayer. And there was change in logic of SurfaceView/MediaPlayer in 4.0 which caused this being valid no more. But code in SurfaceView was not updated before 4.2 (in which this problem in SurfaceView is fixed).

I have checked in git repo of android and really, version tagged android-4.0.1_r1 does not include fix and version tagged android-4.2.1_r1 includes it.

So, to fix it for platforms which doesn't contain fix for it, manual check if surface valid before setting it to the MediaPlayer is needed only for platforms 4.0 and after:

@Override public void surfaceCreated(final SurfaceHolder holder) {
    final Surface surface = holder.getSurface();

    if ( surface == null ) return;

    // on pre Ice Scream Sandwich (4.0) versions invalid surfaces seems to be accepted (or at least do not cause crash)
    final boolean invalidSurfaceAccepted = Build.VERSION.SDK_INT < Build.ICE_CREAM_SANDWICH;
    final boolean invalidSurface = ! surface.isValid();

    if ( invalidSurface && ( ! invalidSurfaceAccepted ) ) return;

    _mediaPlayer.setDisplay(holder);
}

In this way, on older platforms invalid surface will be successfully set to the media player and video will playback, on platforms 4.0-4.1 it will throw invalid surfaces away (and i think that surfaceCreated will be called again with valid surface) and on 4.2 and later surfaceCreated will just not be called with invalid surface.

E.L.K.
  • 116
  • 1
  • 5
  • What action do you take if the surface is invalid? `surfaceCreated` won't be called again, so do you just remove the `SurfaceView` and re-add it, hoping that the created `Surface` is valid the 2nd time? – bclymer Nov 29 '13 at 19:16
  • Unfortunately I have no way of reproducing this issue, only crashlogs, so the only thing I can do is prevent crash - I have no possibility to test how it will behave in different "fix" cases. Maybe this information about reason of problem will be useful for someone. – E.L.K. Dec 02 '13 at 07:42
  • I have updated my answer with more details and more useful solution. – E.L.K. Dec 30 '13 at 08:54
  • My crash logs seem to reflect that the fix was made in Android version 4.1.2. I've caught ~2.5 million exceptions thrown by this on versions 4.0.3, 4.0.4, and 4.1.1, however none on 4.1.2 and 4.1.2 is the most used OS version with our app. However, great investigating, that information is very valuable! – bclymer Dec 30 '13 at 13:58
  • what's the value of invalidSurfaceAccepted? – MetaSnarf Feb 01 '17 at 09:30
4

I had issues like this in the past using Android VideoViews/MediaPlayers. It turned out that the underlying SurfaceView was getting garbage collected. I solved it by adding an onPreparedLister to the MediaPlayer, and then holding an explicit reference to it in my class while I was using it. Maybe this helps.

mszaro
  • 1,362
  • 1
  • 13
  • 25