skopus
November 30, 2012, 11:09am
1
Hello!
It seems that ~DShowCameraDeviceInteral() doesn’t free its callback declared as
ComSmartPtr callback;
defined as callback = new GrabberCallback (*this);
and released as callback = nullptr;
The memory stays allocated according to visual studio 2010 express report
Detected memory leaks!
Dumping objects ->
{2125} normal block at 0x03482DA0, 12 bytes long.
Data: <P8 ?H > 50 38 DA 01 02 00 00 00 B0 3F 48 03
Juce version 2.0.28
jules
November 30, 2012, 7:03pm
2
Hmm. My code seems fine, but maybe the win32 stuff needs to be forced to release the grabber. Does it help if you add this?
[code] ~DShowCameraDeviceInteral()
{
if (mediaControl != nullptr)
mediaControl->Stop();
removeGraphFromRot();
for (int i = viewerComps.size(); --i >= 0;)
viewerComps.getUnchecked(i)->ownerDeleted();
if (sampleGrabber != nullptr)
sampleGrabber->SetCallback (nullptr, 0);
[/code]
skopus
December 3, 2012, 9:13am
3
It doesn’t change anything. Is it ok to use the camera device API like this?
here is my test scenario
class MyClass: public CameraDevice::Listener
{
public:
MyClass(int width, int height)
{
...
camera = CameraDevice::openDevice( 0, 176, 144, width, height);
if(camera) camera->addListener( this );
...
}
~MyClass()
{
...
if(camera)
{
ScopedPointer<CameraDevice> tmp = camera;
tmp->removeListener( this );
}
}
...
private:
ScopedPointer <CameraDevice> camera;
void imageReceived (const Image& image)
{
//some processing
}
...
}
jules
December 3, 2012, 3:07pm
4
Well, in that case it looks like something in DirectShow is hanging onto that reference and not releasing our object. Can’t think of anything else to suggest…
Your code looks fine, but I don’t think there’s any need for all that stuff in your destructor, as the camera object is about to get deleted anyway.
skopus
December 6, 2012, 2:31pm
5
DShow is not the point. Adding some temporary pointer storage in the following manner resolves the issue. I believe you have some nicer solution
class DShowCameraDeviceInteral
{
GrabberCallback * c;
DShowCameraDeviceInteral(): c(nullptr)
{
c = new GrabberCallback (*this);
callback = c;
//callback = new GrabberCallback (*this);
}
~DShowCameraDeviceInteral()
{
callback = nullptr;
delete c ;
}
}
jules
December 6, 2012, 2:46pm
6
er… but something still has a reference to that object (we know that because its ref-count is non-zero), so to forcibly delete it will leave a dangling pointer somewhere. That’s not a fix, it’s just a bug waiting to happen!
And yes, DShow is the point, because something is keeping a ref-count to that object, and it’s not my code doing it, so I assume it must be DShow.
skopus
December 18, 2012, 1:57pm
7
ComBaseClassHelperBase() initializes RefCount with 1
assignment callback = new GrabberCallback (*this); makes Refcount==2
looks like double reference count for single pointer, no?
jules
December 18, 2012, 3:19pm
8
Goddammit… Why does it use 1 as the initial ref-count!??
Thanks! I’ll do some sanity-checking there!
skopus
December 24, 2012, 7:33am
9
It still leaks in juce 2.0.30
Is the code below reasonable ?
DShowCameraDeviceInteral(...)
{
...
callback = new GrabberCallback (*this); //RefCount==1
hr = sampleGrabber->SetCallback (callback, 1); //RefCount==2
/* decrease ref count to avoid memory leak */
callback->Release(); //RefCount==1
...
}
jules
December 24, 2012, 11:47am
10
No, that code is definitely not reasonable! I changed it so that the initial ref count is now 0, not 1.
Perhaps it needs both the new fix and the one I suggested earlier in this thread, i.e. adding the SetCallback (nullptr, 0) call?
skopus
December 24, 2012, 1:41pm
11
I confirm that both fixes are required
jules
December 24, 2012, 5:53pm
12
Ok, thanks, I’ll add the other one too.