carlonluca / pot

Raspberry Pi accelerated video/image rendering with Qt: custom QML components and QtMultimedia backend.
https://bugfreeblog.duckdns.org
GNU General Public License v2.0
177 stars 42 forks source link

Memory leaks #13

Closed arielmol closed 9 years ago

arielmol commented 9 years ago

Leaving the player switching between several videos, like a playlist it seems to leak about 3~5 MB on each loop, and depending on the video. Im using the Big Buck Bunny and others to test.

carlonluca commented 9 years ago

Have a look at what valgrind has to say.

penguinest commented 9 years ago

Not just playlist has an increasing memory cost but also create and destroy videoOutput object. For example, if you create a Loader in qml for multimedia features, when you access and destroy the source of this loader 2 or 3 times, then the screen goes black (and videos are never displayed again). I suppose the there should be some non freed pointer.

For the playlist issue...It's just a bad patch, but you can try freeing memory by calling "echo 3 > /proc/sys/vm/drop_caches" after each video in order to free some ram.

carlonluca commented 9 years ago

PiOmxTextures is just a POC of what can be achieved and how, so I expect many leaks and issues. I never bothered to check. If you need to use in production, I suggest you run valgrind and have a closer look at what happens.

Nightmare204 commented 9 years ago

Hi, has anybody manage this issue already? I'm trying to make a video banner like application, so I need to switch videos and had found the same memory leak problem...

carlonluca commented 9 years ago

Can you upload the logs from a few loops? My Pi is not usable to work on this but let's see if there is something I can see.

What branch are we talking about? Master or no_tearing?

arielmol commented 9 years ago

When leaving loop-playing on a small video it will eventually lock the app. When using a playlist with different videos the issue seems to be worse as it locks about 30~50% faster. I think it depends on the videos themselves.

I will leave my pi a few hours and report the log.

penguinest commented 9 years ago

Hi Luca, I have realized that the "donate" option is not anymore avaliable from your blog. You may could reactivate it. That way we could provide you a new rpi2 for testing.

carlonluca commented 9 years ago

I think such a button never existed... as I said in #14, I can't guarantee much time on this.

Nightmare204 commented 9 years ago

I'm using the master branch of the git.

At least in my case the loop property of the MedyaPlayer component doesn't works. Imade the loop by using the signal onPlaybackStateChanged to play the video again. If I loop just one video (no mather the size, but same path) there's no problem. If use a playlist and change the path (even if all the videos are copies of the same one) the mediaplayer will eventually stop playing videos, and if I try to force it (with a button), the app crashes. I have split 128Mbts for the GPU and that allows me 93 changes of videos.

This is a log of the 93 loops... they are almost all the same, the last ones are the more important as they show a "Audio Decode Timeout" (Sometimes is a "Video Decode Timeout", sometimes it doesn't say anything but stops) that is related to the memory being filled... http://pastebin.com/C0jVasSz.

I don't know if this posts are related to our ploblem: https://github.com/popcornmix/omxplayer/issues/12 https://github.com/raspberrypi/firmware/issues/172 https://github.com/huceke/omxplayer/issues/178

They establish that there where some errors in the handling of OMX before that lead to memory leaks similar to ours, but know they are fixed. @carlonluca from when are the libs that you are using for the openmaxil backend?

Thanks...

carlonluca commented 9 years ago

The libs are in your Pi. I'm inclined to think this is a PiOmxTextures issue: as I said, I paid no attention to these issues nor I did any test. I had no practical interest in this project, it is just fun.

Ok, the output is interesting, thank you. Can you also post a minimal code that "shows" the leak issue?

Nightmare204 commented 9 years ago

This is the minimal code you asked for... https://dl.dropboxusercontent.com/u/46248879/QT_VideoLoop_Tests.tar I think maybe there's no correct dispose of the buffers or tunnels that are being instantiated. Just guessing (as I'm just starting to understand the code) but maybe when there's and event of change in the source of MediaPlayer there's no correct flush of the memory and new textures are being instantiated.

I'm trying to compare your code with that in the hello_video example of the Pi, and I never find for example calls to the functions ilclient_disable_tunnel or ilclient_cleanup_components...

penguinest commented 9 years ago

Maybe you could take a look at "freeTexture" function (I think know is call ·"free"). I think the problem is some pointer is not correctly freed once mediaplayer is stopped.

Nightmare204 commented 9 years ago

Checking the code in openmaxilplayercontrol.cpp, I found out that the next section of code is never called:

void OpenMAXILPlayerControl::processCommands() { ....... if (command->m_playerCommandType == PLAYER_COMMAND_TYPE_FREE_TEXTURE_DATA) { LOG_VERBOSE(LOG_TAG, "Processing post freeTexture()..."); PlayerCommandFreeTextureData* freeTextureCommand = dynamic_cast<PlayerCommandFreeTextureData*>(command); m_texProvider->freeTexture(freeTextureCommand->m_textureData); } ......... }

The command PLAYER_COMMAND_TYPE_FREE_TEXTURE_DATA is never assigned and it allows the call to the "freeTexture" function @atr0n referenced.

The other thing I notice is that the OpenMAXILPlayerControl::stop() function doesn't resembles other functions like OpenMAXILPlayerControl::play(), where it is instantiated a new PlayerCommandXXX and it is send to the deferred list with appendCommand

@carlonluca: Could it be enough to adjust that stop() function adding also a deferred command PLAYER_COMMAND_TYPE_FREE_TEXTURE_DATA ???

Nightmare204 commented 9 years ago

In the function OpenMAXILPlayerControl::setMediaInt I notice the following line:

m_textureData = NULL;

If I understand correctly when I change the source property of the MediaPlayer component, it will eventually call the function OpenMAXILPlayerControl::setMediaInt, which will set NULL the m_textureData....

The functions calls is something as follows:

m_mediaProcessor->setFilename(... , m_textureData) //omx_mediaprocessor.cpp setFilenameInt (... textureData) //omx_mediaprocessor.cpp m_player_video->Open(..., textureData, ...) //OMXPlayerVideo.cpp OpenDecoder( textureData ) //OMXPlayerVideo.cpp m_decoder->Open(... , textureData) //OMXVideo.cpp

In this last function: .... m_textureData = textureData; // Used in case not NULL. .... if (!m_textureData) { m_textureData = m_provider->instantiateTexture(videoSize); LOG_VERBOSE(LOG_TAG, "Texture generated!"); }

Then... the instruction "m_textureData = NULL" doesn't destroys the textures but force the generation of new ones...

Am I digging correctly the code?

carlonluca commented 9 years ago

According to the log file reported it seems that texture data is not freed. Probably both the buffer and the texture. Free the texture data and the leak should, at lest, get smaller.

Nightmare204 commented 9 years ago

I had compiled a version of PiOmxTextures in Verbose log level, so then the logs of debug level where not there...

I tried removing the line "m_textureData = NULL" and compiled all over again with DEBUG level.

Now I can get logs like:

08:05:32.170 INFORMATION: Cleaning up... 08:05:32.171 VERBOSE: Closing players... 08:05:32.312 VERBOSE: Closing players... 08:05:32.314 VERBOSE: Freeing texture... 08:05:32.315 DEBUG: Freeing KHR image... 08:05:32.317 DEBUG: Freeing texture... 08:05:32.318 DEBUG: Freeing texture data... 08:05:32.320 INFORMATION: Cleanup done.

However the memory leak is still the same, so maybe the problem is not at the call of the function "freeTexture" but in the function itseft... but I can't see anything wrong there...

carlonluca commented 9 years ago

I can't guarantee, but if possible I'll have a look at this in a few days. I'll work on the no_tearing branch as it is far better.

Nightmare204 commented 9 years ago

Thanks a lot @carlonluca !!! Just for the record, I had just tested the no_tearing branch and it only lasts 1 loop. It seems to get stucked decoding, giving the following message:

15:40:50.707 VERBOSE: No buffer available.

But you have already warned that ("This commit breaks reopen and external controls.")...

carlonluca commented 9 years ago

I worked a bit on the defects of the no_tearing branch and commit 4c8e999 should fix many of them. Unfortunately code still is bad and very inefficient, I had to temporarily disable the position() implementation and still there are leaks in the GPU.

However commit 4c8e999 skips the GPU cleanup and reuses the same structures for the following video. I'm currently over 250 loops of the code posted by @Nightmare204. No issue so far.

Unfortunately my Pi seems to have severe issues, so I don't know how much I can go on implementing/testing this. The hardware seems to crash. If you're interested, have a look at the commit (always use 1080p videos) and let me know. It may be a start.

Nightmare204 commented 9 years ago

Thanks a lot @carlonluca for your hard work!

I had some issues with git yesterday (checkout -f didn't download all the changes), but after your https://github.com/carlonluca/pi/commit/59523ef4b59f89590b126701f4fd1b2e98d591ba commit I was able to get the repo.

Right now I have the sample minimal code in execution and it's in the loop number 520, using 1080p videos as you asked. I overcame the lack of position() functionality with a timer started together with the video. I need it to generate the fade effect between videos, starting some seconds before the last one stops.

I'm going to let the code 24h and I will let you know!

Thanks again and don't hesitate in letting me know if I can help you getting a new Raspberry Pi...

penguinest commented 9 years ago

Hi, Your code helped a lot to me. I would like to donate some money if you need a RPi. Regards,

Nightmare204 commented 9 years ago

24h completed! 4000 loops!... The modifications @carlonluca made are great... Video Size can't be changed, but that's not a problem to me as I can modify the videos prior to put them in the banner...

However I guess memory leak still exists in the generation of new buffers, and that for the quick solution is reusing them...

arielmol commented 9 years ago

@carlonluca I would also support you via Paypal if you need to get another pi or and cleaner power source for it.

@Nightmare204 Is there a possibility to have the last round of binaries to further test?. My OS is currently OSX and can't build it right now. However I can fully test it on a B and B+.

carlonluca commented 9 years ago

Ok, if someone is willing to provide a Pi 2 for development and testing please contact me in private.

Nightmare204 commented 9 years ago

@desert I'm going to upload the binaries here https://www.dropbox.com/sh/ex8gxp9myy5weof/AADPAvKulF-LeLKhycJ2p5qwa?dl=0 These binaries are for Qt5.4.2 and follow the same paths that @carlonluca.

penguinest commented 9 years ago

@carlonluca I sent you a private message from your blog talking about the donations for your pi2 but I haven't received any reply. Did you receive?

carlonluca commented 9 years ago

Mmh... yes yes I got it, I think I answered you... I just checked... However no problem. A Pi is on its way thanks to another Pi enthusiast who had one spare. Thanks for those who offered!

I have a few updates waiting in my branch but... my Pi keeps crashing so difficult to confirm. This isn't happening to anyone else, is it? The usb driver seems to crash somehow and the ssh connection is interrupted... Only when using PiOmxTextures...

Nightmare204 commented 9 years ago

@carlonluca I can do the tests for you, just tell me what to check... I have one Raspberry Pi B+ in the loop test over one week and it has got to 33000 loops using 15 sec videos of 1080p resolution. I was going to change to larger videos, but I can use it to further testing...

I have a Raspberry Pi 2 which I'm working on, so It will not be a trouble to let the B+ running...

Nightmare204 commented 9 years ago

@carlonluca: I have noticed that two mediaplayers can't reproduce at the same time with the actual approach in the no_tearing branch... however they can coexist, but one must be played just after the other stops, making it useless to have two mediaplayers.

I was going to use two players because of the lag time between video changes, as I told you in your blog... Recently you said at the blog: "There was a wrong sequence in the setup of the omx graph when opening a video. The time to start is now a little lower. I'll push this change shortly".

Now I'm even more interested in making test for you... :+1:

arielmol commented 9 years ago

@carlonluca I strongly believe your freezes are because a faulty power adaptor, those chinese 2.1Amp are really very unstable. I can donate you via Paypal for a new power adaptor and a pi, just need your paypal email, please.

arielmol commented 9 years ago

@carlonluca @Nightmare204 OTOH, Have you seen? http://qt-project.org/wiki/New-Features-in-Qt-5.5

GStreamer 1.x is supported, but it does not make clear if it has hardware acceleration for the Pi, also is not enabled by default. In theory it should work, but have anyone tested yet? Last time it was rather slow, and @carlonluca 's implementation was less resource intensive.

dimovnike commented 9 years ago

Hi guys. Last time i checked (in november i think), gstreamer compiled and worked on PI. I mean gstreamer-1.0 the gst-omx plugin and the qtmultimedia gstreamer-1.0 backend (still experimental atm). It even worked, but the image had strange ghosting, looked like one of the colors was misplaced. I am going to test it in a week or two and let you know if it works. Anyway this library here is very promissing since it should create less overhead than gstreamer but it needs some effort to be polished.

carlonluca commented 9 years ago

@desert Yes, https://codereview.qt-project.org/#/c/100421/ seems promising, but I'm not going to use it on Pi any time soon. Therefore sorry, I can't provide info on that.

@desert Possible but I never changed power adapter, and my Pi is up 24h without issues until I use the GPU with PiOmxTexutres... The new Pi 2 has just arrived anyway, so I'm having a look at this. Let's see how it works. If interested please contact me in private. Thanks!

Have a look at d2f7817. I added a --loop param to POCPlayer and I used that to test. Switching media was taking more than 4 seconds. Now it is around 2 seconds. Tested with a 720p and a 1080p video. It seems resources are properly freed now. Happy testing.

I also did a couple of other changes (unrelated to this issue) that should improve the rendering. I'll probably upload in a few days.

carlonluca commented 9 years ago

@Nightmare204 mmh... I'd avoid using two players at the same time. I never tested that as I supposed just one 1080p for the GPU was sufficient. Test the latest commit and see if the interval from one video to the next is better now.

Nightmare204 commented 9 years ago

@carlonluca Testing the last commit I noticed there is more garbage at the beginning of the videos... With each change of video I got some frames like the following

2:23:12.621 VERBOSE: Creating buffers for 4 images. 12:23:12.623 INFORMATION: Buffer created 0x1b1f930 12:23:12.625 INFORMATION: Buffer created 0x19dd5a0 12:23:12.627 INFORMATION: Buffer created 0x19e0c80 12:23:12.633 INFORMATION: Buffer created 0x1aee950 12:23:12.635 VERBOSE: Component renderer: 1a23440. 12:23:12.667 WARNING: One frame couldn't be shown. 12:23:12.674 WARNING: One frame couldn't be shown. 12:23:12.681 WARNING: One frame couldn't be shown. 12:23:12.698 WARNING: One frame couldn't be shown. 12:23:12.706 WARNING: One frame couldn't be shown. 12:23:12.714 WARNING: One frame couldn't be shown. 12:23:12.735 WARNING: One frame couldn't be shown. 12:23:12.743 WARNING: One frame couldn't be shown. 12:23:12.766 WARNING: One frame couldn't be shown. 12:23:12.784 WARNING: One frame couldn't be shown.

After that the video plays normal...

carlonluca commented 9 years ago

Probably related to #12. I actually don't remember all those skipped frames... I have another update however that influences the frame rate, I'll see if the same happens in there or not. Thank you for testing!

carlonluca commented 9 years ago

If you guys confirm no leaks even when changing video/resolution the no_tearing branch may be merged.

arielmol commented 9 years ago

I had mixed results with the last version, @Nightmare204 can you, please, upload the binaries in order for me to test and confirm the fixed leaks?

arielmol commented 9 years ago

@carlonluca About the freezes when using GPU, I still get some lockups when using GPU and high CPU usage if i have faulty power adapters, so I am still inclined to think that's the reason.

carlonluca commented 9 years ago

@desert Possible... If possible contact me in private please. There is a form in the blog. Let me also know the result of your tests. Thanks.

Nightmare204 commented 9 years ago

@desert I have uploaded the binaries at mi public dropbox folder (https://www.dropbox.com/s/n6r1tyuswlc9zwu/QT5.4.2%2BPiOmxTextures_4.4.tar?dl=0)

@carlonluca The time between videos has improved a lot and memory leak seem to be solved, I'm changing videos and resolution... I just started the program again so I'm at the 100 loop. but I'll let you know tomorrow...

For the frame lost I think it could be related to the sync that h264 demands, so I added some black frames at the start and end of the videos and that seems to solved it. I did it with ffmpeg and I left the .sh at dropbox as well (https://www.dropbox.com/s/7i6zqeeek70a8zb/fixVideo.sh?dl=0)... give it a try @desert.

arielmol commented 9 years ago

I tested with @Nightmare204 binaries, my output is here. However I still see big gray slices and some noise at the begining of every video. I will keept it running until crash (if any).

My output when playing 3 videos: http://pastebin.com/dMwQ2PjB

arielmol commented 9 years ago

I ran them using the qmlscene with :

ApplicationWindow {
    id: app
    title: qsTr("Hello World")
    width: 640
    height: 1000
    visible: true
    property var videos: ["/home/pi/test.mp4", "/home/pi/test.mp4", "/home/pi/test1.mp4", "/home/pi/test2.mp4", "/home/pi/test3.mp4", "/home/pi/este-no-existe.mp4"]
    property int index:0

    Component.onCompleted: vid.play()
    Video{
        id: vid
        width: 640
        height: 320

        anchors.fill: parent
        source: videos[index]
        autoPlay: false
        onStopped: {
            var i = app.index
            i = (i+1)%videos.length
            index=i
            play()
        }
    }
}

I still see several: 12:16:47.131 VERBOSE: EGL image 14 created... 12:19:59.681 VERBOSE: Creating OMX_TextureData... 12:19:59.682 VERBOSE: Texture generated!

with the number going up and up. In my fourth video Im at 20.

Memory is freed, as my 'top' says, but not all memory is freed and keeps going up. Interestingly, I do not see the WARNINGs about that frame as @Nightmare204

arielmol commented 9 years ago

I also notice CPU usage is about 10~12% with some spikes at 20% which I didn't notice before, that's a nice improvement! 'cause I remember it being about 40% with spikes at 50%.

carlonluca commented 9 years ago

@desert Can you run valgrind on it? The index of the EGL image always increase it seems. Even if you kill the process.

@Nightmare204 What video are you using to test? Can you provide it? Are you using POCPlayer for testing or some different code?

arielmol commented 9 years ago

@carlonluca Im using the version @Nightmare204 provided so I cannot Valgrind it yet. These are my test files: http://io.edis.mx/pitests/ I use qmlscene to run the qml test.

Yes, the message: 10:47:21.845 VERBOSE: EGL image 11 created...

Increases even then restarting the software, after three runs it goes thru 11.

This is a video where I constantly hit CONTROL-C and relaunch http://youtu.be/cIOYJf9Yr7Q.

Nightmare204 commented 9 years ago

I checked the memory usage through top command as @desert suggested and didn't noticed relevant memory increase due to a possible memory leak... In the Raspberry Pi B+ the memory usage was 30% and in the Raspberry Pi 2 the memory usage was 9%... However I'm using a program where I create two video surface (video output + mediaplayer) but I was using just one, so the memory usage of my program is supposed to be higher that when just one mediaplayer is instantiated... The program is a slight variation of the one I shared before and I'm using all kind of videos: "Big Buck Bunny" from the official site, a short version of the same video cut with ffmpeg, some videos downloaded from youtube using www.clipconverter.cc, some videos from a professional camera edited with Encoder (on a Mac) that a partner provided me, etc... Some of these videos have different video format and resolution... Today the loop count was in about 6000 when I stopped it... and the memory usage was the same.

The index of the EGL image always increase... but that didn't make any difference to me.

@desert the messages you get with the VERBOSE label are just part of the code of the PiOmxTextures because I compiled with that level of debug messages... A "Final" version should be compiled in WARNING or ERROR level...

@carlonluca I think No_Tearing branch can be merged! ;) :+1:

Nightmare204 commented 9 years ago

@carlonluca I forgot to reconfirm you that it is impossible to play two videos at the same time, even when, as far as I understand, each mediaplayer crates it's own buffers.

The program ends with the error: 7736 Segmentation fault

With the code of master branch two videos can be played at the same time, but obviously the memory leaks duplicates and the program crash sooner.

arielmol commented 9 years ago

Interesting, I still get bad frames. And then I got: http://pastebin.com/p4uxKmP1

Could this be a problem of my binary, I got it from: (https://www.dropbox.com/s/n6r1tyuswlc9zwu/QT5.4.2%2BPiOmxTextures_4.4.tar?dl=0

Just in case these are my MD5, are those the same as you @Nightmare204 @carlonluca? Should be at least identical to @Nightmare204

$ md5sum /usr/local/qt5.4/plugins/mediaservice/libopenmaxilmediaplayer.so 
c7e50b48ebd5c83af522ab343c16443b  /usr/local/qt5.4/plugins/mediaservice/libopenmaxilmediaplayer.so

$ ldd /usr/local/qt5.4/plugins/mediaservice/libopenmaxilmediaplayer.so
        /usr/lib/arm-linux-gnueabihf/libcofi_rpi.so (0xb6f03000)
        libEGL.so => /opt/vc/lib/libEGL.so (0xb6ec8000)
        libPiOmxTextures.so.4 => /usr/local/lib/libPiOmxTextures.so.4 (0xb650c000)
        libQt5MultimediaWidgets.so.5 => /usr/local/qt5.4/lib/libQt5MultimediaWidgets.so.5 (0xb64f6000)
        libQt5Multimedia.so.5 => /usr/local/qt5.4/lib/libQt5Multimedia.so.5 (0xb6441000)
       <...>

$ md5sum /usr/local/lib/libPiOmxTextures.so.4
280ed9521f1d9a73bcbc67f8befc5b00  /usr/local/lib/libPiOmxTextures.so.4
arielmol commented 9 years ago

Also, I uploaded a video of my artifacts and grayboxes here (taken just 5 minutes ago): https://www.youtube.com/watch?v=e1_N6W3ZiKs

Also you can see a gray box at the beginning here: https://www.youtube.com/watch?v=8PYOIbh0X1I