33

After a lot of debugging I finally found what is causing this error! Garbage Collection!

I have a video playing in media view and in the background I am looking for new videos from a Rest API.

Every now and then I see the Garbage collection running:

02-22 13:14:57.969: D/dalvikvm(16888): GC_EXPLICIT freed 152K, 4% free 6746K/6979K, paused 2ms+2ms

And the straight after that:

02-22 13:14:57.969: W/MediaPlayer-JNI(16888): MediaPlayer finalized without being released

So I tested it by calling System.gc() every 5 seconds.

As soon as the first GC is called it happens!

02-22 13:19:47.813: D/dalvikvm(17060): GC_EXPLICIT freed 167K, 5% free 6745K/7047K, paused 2ms+2ms ---- I call GC
02-22 13:19:47.813: W/MediaPlayer-JNI(17060): MediaPlayer finalized without being released ---- VIDEO PLAY INTERRUPTED

Why does this happen? Can I prevent it?

Playing the video:

private void playMedia(int playListIndex) throws IOException {
        File mediadir = getDir("tvr", Context.MODE_PRIVATE);
        filelist = mediadir.listFiles();
        Log.i("media player", "play media!");
        String path = filelist[playListIndex].getAbsolutePath();
        FileInputStream fileInputStream = new FileInputStream(path);
        final Uri uri = Uri.parse(path);
        String filename = filelist[playListIndex].getName();
        if (filename.contains("image")) {
            imageView = (ImageView)findViewById(R.id.imageView);
            imageView.setVisibility(View.VISIBLE);
            imageView.setImageURI(uri);
            mHandler.postDelayed(new Runnable() {
                public void run() {
                    imageView.setVisibility(View.GONE);
                    imageView.setImageURI(uri);
                    onCompletion(null);
                }
            }, 4000);
        } else if (filename.contains("video")) {

            MediaPlayer pl = new MediaPlayer();
            pl.setOnCompletionListener(this);
            pl.setDisplay(holder);
            pl.setDataSource(fileInputStream.getFD());
            pl.prepare();
            pl.start();
        }
    }

And when it is done:

@Override
    public void onCompletion(MediaPlayer mp) {
        Log.i("media player", "play next please!");
        if (mp != null) {
            mp.release();
        }
//      play next video
        currentMedia++;
        if (currentMedia > playList.size() - 1) {
            currentMedia = 0;
        }
        try {
            playMedia(currentMedia);
        } catch (IOException e) {
            e.printStackTrace();
        }


    }
Jonathan Hall
  • 75,165
  • 16
  • 143
  • 189
Harry
  • 13,091
  • 29
  • 107
  • 167

2 Answers2

72

I think this is because you create the media player within the scope of the method, therefore, when the method completes, it goes out of scope. This means there are no references, so is ok for garbage collection.

This means, it can be free'd by the GC before it has even called onCompletion, hence won't release before cleared. Instead, you need to store a reference to the media player as a member variable in your class.

T. Kiley
  • 2,752
  • 21
  • 30
  • 1
    Ag damn! thanks so much! this works now, I knew it was something dumb! – Harry Feb 22 '13 at 12:12
  • I also had the same problem, although it's caused by inattention by the developer it's often problematic to find these kind of errors when your into something;-) Thanks! – Robert Jun 16 '13 at 17:39
  • 3
    one of the best answer I've ever read on stack, to the point and extremely insightful on java, scoping and Gc. Thank You man! – sirvon Jan 03 '14 at 16:11
  • Thanks a lot :). Yes it is silly mistake on my part. But the point here is should't the GC wait since MediaPlayer is bound to the application's context? – Himanshu Virmani Apr 19 '14 at 09:02
  • I'm not really sure what you mean. I've never used the media player, but I can't see how , without some explicit call, the media player could have any scope outside the if statement. – T. Kiley Apr 19 '14 at 10:25
  • I think is a bug of Android. It should keep a reference in somewhere and wait for release() invoked. – Jiang YD Jun 10 '15 at 06:54
  • I don't think so - more a property of using a GC'd language. The media player can't keep itself alive (since if that was possible it would be trivial to introduce memory leaks) and nothing outside the scope of the method knows about the object. – T. Kiley Jun 10 '15 at 09:12
  • So then we need the Media Player to be static right? – Ruchir Baronia Nov 15 '15 at 20:45
  • @rich it just needs to be a member variable (fields) as opposed to a local variable. – T. Kiley Nov 15 '15 at 20:48
  • So when I tried moving the declaration out of all methods, and just kept the instantiation in one of my methods, I got: `Media player cannot be referenced from a nonstatic context` – Ruchir Baronia Nov 15 '15 at 20:50
  • But the error still occurs: `MediaPlayer finalized without being released` – Ruchir Baronia Nov 15 '15 at 20:52
  • And the media player isn't even playing anymore – Ruchir Baronia Nov 15 '15 at 20:58
  • You are my hero @T.Kiley – Zapateus Jun 01 '16 at 09:15
  • 1
    Well if we use OnCompletionListener, the MediaPlayer object is passed back in callback so why is it garbage collected? – cubesoft Oct 10 '17 at 14:53
11

Java GC only manages memory. So when other resources are used (e.g. files, sockets, etc), you need to manually manage them. In the case of MediaPlayer, the documentation mentions that:

It is also recommended that once a MediaPlayer object is no longer being used, call release() immediately so that resources used by the internal player engine associated with the MediaPlayer object can be released immediately. Resource may include singleton resources such as hardware acceleration components and failure to call release() may cause subsequent instances of MediaPlayer objects to fallback to software implementations or fail altogether.

So, when you are done with a MediaPlayer instance, you need to make sure you explicitly call release() on that instance. A good place to do this could be in a lifecycle method of the containing Activity, e.g. onDestroy(). Otherwise, when that MediaPlayer instance is eventually garbage collected (at some arbitrary time after you no longer reference it), the finalizer will notice that you never called release() and will output the warning you are seeing.

Jason Sankey
  • 2,328
  • 1
  • 15
  • 12
  • Hi thanks! I am calling release() after the video is done playing – Harry Feb 22 '13 at 11:31
  • Its not, I was calling release(). This happens even before the first video is done playing, it happens in the first 5 seconds when I call gc().. – Harry Feb 22 '13 at 11:39
  • Now there is code, T. Kiley has your answer. You're not keeping any reference to the MediaPlayer after you create it, so it is being garbage collected early. – Jason Sankey Feb 22 '13 at 11:46
  • Where do I call release? If I call it right after MediaPlayer.start(), it doesn't play – Ruchir Baronia Nov 15 '15 at 20:54
  • Ruchir: as mentioned, a good place to do this is in a lifecycle method of the activity, e.g. onStop() or onDestroy(). – Jason Sankey Nov 16 '15 at 02:14