xbmc/xbmc — pull request #5389 by jimfcarroll (2014-09-19 02:06:23+00:00)
Title: fix memory problems pointed out by valgrind in AE. 

The top 3 memory issues in this log are related: http://pastebin.com/1yFp4Frz

Notice the 2nd parameter in the call to: av_probe_input_buffer(io_ctx, &io_fmt, file.c_str(), NULL, 0, 0);

&io_fmt. io_fmt is a AVInputFormat* on the stack so &io_fmt is an AVInputFormat**.

Inside av_probe_input_buffer2 (which it's passed directly to) it's used first CHECKING IF IT'S BEEN SET. The first time it's used, it's used at line 283 as the variable "fmt" here:

for (probe_size = PROBE_BUF_MIN; probe_size <= max_probe_size && !*fmt;
probe_size = FFMIN(probe_size << 1,
FFMAX(max_probe_size, probe_size + 1))) {

Notice, the loop will continue UNTIL IT'S BEEN SET. In our case, since it's on the stack and not initialized to NULL, this loop USUALLY never executes. I checked the docs and they don't seem to indicate that it needs to be initialized to NULL ... but it does.

The next place it's used is line 328:

if (!*fmt)
ret = AVERROR_INVALIDDATA;

This if condition will ALMOST always be false because we passed it in already set (to garbage from the stack) and it wasn't reset because it never went into the loop where it was actually supposed to be set.

It exits the method never being touched (it has the same garbage value we passed in) and so when WE use it at line 2514 (the 3rd problem pointed to by Valgrind):

if (!io_fmt)
{

That "if" statement will ALMOST always succeed because io_fmt has the same crap it had from the stack.

That's the long way around to say we should have NULLed it.

---------------------------------------------------------------

Secondly: See the valgrind trace here: http://pastebin.com/xrfcS08i

Notice that DVDAudio is calling log on uninitialized data here: CDVDPlayerAudio::UpdatePlayerInfo() (DVDPlayerAudio.cpp:476)

That line is this: s << ", att:" << fixed << setprecision(1) << log(GetCurrentAttenuation()) * 20.0f << " dB";

GetCurrentAttenuation() is this: m_dvdAudio.GetCurrentAttenuation();

m_dvdAudio is implemented by AE ... jumping to the stream's GetVolume call:

float CActiveAEStream::GetVolume()
{
return m_streamVolume;
}

Valgrind notes the source of the allocation is: ActiveAE::CActiveAE::CreateStream(ActiveAE::MsgStreamNew*) (ActiveAE.cpp:1234)

That new's the CActiveAEStream who's constructor doesn't set m_streamVolume (it sets "volume" but not "streamVolume" - streamVolume is only set with SetVolume which I'm guessing is never called prior to DVDAudio using it).