[PATCH] Memory leak with ASS subtitles, possible patch
#1
I've noticed a couple of memory leaks when watching videos with ASS subtitles in them, particularly when USE_EXTERNAL_LIBASS=1.

Build: SVN r35621
Platform: Ubuntu 10.04 (Lucid), current as of about yesterday

To reproduce the leak, watch a bunch of videos with ASS subtitles, such as modern anime videos with soft subtitles. This seems to be more severe with more ASS effects like fades, etc. As you watch more videos, xbmc.bin's resident memory keeps climbing, as visible in top. Valgrind also reports leaks like:

Code:
==27696== 560,142 bytes in 450 blocks are possibly lost in loss record 1,499 of 1,503
==27696==    at 0x4C274A8: malloc (vg_replace_malloc.c:236)
==27696==    by 0x8EAAB44: ??? (in /usr/lib/libass.so.4.0.0)
==27696==    by 0x8EAB28C: ??? (in /usr/lib/libass.so.4.0.0)
==27696==    by 0x8EABB0D: ??? (in /usr/lib/libass.so.4.0.0)
==27696==    by 0x8EAE4CD: ??? (in /usr/lib/libass.so.4.0.0)
==27696==    by 0x8EAF4B5: ass_render_frame (in /usr/lib/libass.so.4.0.0)
==27696==    by 0xB1F8A0: CDVDSubtitlesLibass::RenderImage(int, int, double) (DllLibass.h:96)
==27696==    by 0x9E182E: CDVDOverlayRenderer::Render(stDVDPictureRenderer*, CDVDOverlaySSA*, double)
(DVDOverlayRenderer.cpp:74)
==27696==    by 0x9CECCE: CDVDPlayerVideo::ProcessOverlays(DVDVideoPicture*, YV12Image*, double) (DVDO
verlayRenderer.h:69)
==27696==    by 0x9CF570: CDVDPlayerVideo::OutputPicture(DVDVideoPicture*, double) (DVDPlayerVideo.cpp
:1184)
==27696==    by 0x9D1212: CDVDPlayerVideo::Process() (DVDPlayerVideo.cpp:605)
==27696==    by 0x6A4917: CThread::staticThread(void*) (Thread.cpp:203)

The main problem appears to be in the destructor for CDVDSubtitlesLibass, in xbmc/cores/dvdplayer/DVDSubtitles/DVDSubtitlesLibass.cpp. It's calling m_dll.IsLoaded() to see whether it should free some libass stuff, but when USE_EXTERNAL_LIBASS=1, IsLoaded() always returns false. This is because DllLibass never sets its m_dll pointer to anything non-null, and that's what IsLoaded() looks for.

It seems like the reference counting on CDVDSubtitlesLibass is sufficient to make this dtor do the right thing, but I may not be seeing the big picture.

After I took out the check for IsLoaded(), valgrind still found leaks like:

Code:
==27950==    at 0x4C267CC: calloc (vg_replace_malloc.c:467)
==27950==    by 0x8EA34CF: ass_new_track (in /usr/lib/libass.so.4.0.0)
==27950==    by 0xB1FABE: CDVDSubtitlesLibass::DecodeHeader(char*, int) (DllLibass.h:98)
==27950==    by 0xAE8434: CDVDFactoryCodec::OpenCodec(CDVDOverlayCodec*, CDVDStreamInfo&, std::vector<CDVDCodecOption, std::allocator<CDVDCodecOption> >&) (DVDFactoryCodec.cpp:111)
==27950==    by 0xAE879C: CDVDFactoryCodec::CreateOverlayCodec(CDVDStreamInfo&) (DVDFactoryCodec.cpp:344)
==27950==    by 0x9CCFA1: CDVDPlayerSubtitle::OpenStream(CDVDStreamInfo&, std::string&) (DVDPlayerSubtitle.cpp:195)
==27950==    by 0x9C4307: CDVDPlayer::OpenSubtitleStream(int, int) (DVDPlayer.cpp:2649)
==27950==    by 0x9C4D76: CDVDPlayer::OpenDefaultStreams() (DVDPlayer.cpp:611)
==27950==    by 0x9CABBC: CDVDPlayer::Process() (DVDPlayer.cpp:827)
==27950==    by 0x6A4917: CThread::staticThread(void*) (Thread.cpp:203)
==27950==    by 0x88549C9: start_thread (pthread_create.c:300)
==27950==    by 0xD96870C: clone (clone.S:112)

This appears to be because CDVDSubtitlesLibass can get an allocated structure in its m_track member, but it never frees it.

I came up with this patch to solve both. (It's ignoring whitespace so indenting will look wrong):

Code:
--- xbmc/cores/dvdplayer/DVDSubtitles/DVDSubtitlesLibass.cpp    (revision 35621)
+++ xbmc/cores/dvdplayer/DVDSubtitles/DVDSubtitlesLibass.cpp    (working copy)
@@ -89,13 +89,12 @@

CDVDSubtitlesLibass::~CDVDSubtitlesLibass()
{
-  if(m_dll.IsLoaded())
-  {
+  if(m_track != NULL)
+    m_dll.ass_free_track(m_track);
     m_dll.ass_renderer_done(m_renderer);
     m_dll.ass_library_done(m_library);
     m_dll.Unload();
   }
-}

This fixes the memory leaks for me. valgrind no longer complains about leaks (even possible ones) going through libass.

Comments are welcome, and logs from xbmc and valgrind can be provided upon request.
Reply
#2
Mind posting this on trac and cc'ing elupus ?

Cheers,
Jonathan
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#3
correct fix is to implement IsLoaded() and returning true.
Reply
#4
Done: http://trac.xbmc.org/ticket/10896

Apologies for posting this in the wrong spot!
Reply
#5
Thanks for the patch Smile
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply

Logout Mark Read Team Forum Stats Members Help
[PATCH] Memory leak with ASS subtitles, possible patch0