Call back function problem

Mikl

Hello

I have problem with life time of variable. It is destroyed BEFORE i have chance to use it in the slot of observer.
For the moment, observer and observable working on one thread. At least, i doing nothing to run it in parallel. Later, they will run in multi thread.
The only way to make it work, is to specify Qt::DirectConnection in connect function. But a am not sure, what it will not make problems later.
Should i always make pointers and destroy on observer after usage? I found it as the worst solution. QT and C++ have to be much smarter.

Help me, please. What i am doing wrong?

Here is my code without all other functions.

Frame producer is a camera object

class IPCAMERABLACKFLY_EXPORT IpCameraBlackFly : public DeviceInterface
{
	Q_OBJECT
public:
	IpCameraBlackFly();
	~IpCameraBlackFly();

	//ip camera SDK
	GigECamera _camera;

	void StartCapturing()
	{
		//make OnImageGrabbed as callback function with pointer to this instance
		_camera.StartCapture(OnImageGrabbed, this);
	}
	
	void OnNewFrameStaticBridge(DeviceFrame* frame)
	{
		emit OnNewFrame(frame);
	}

public slots:
	void OnNewFrame(DeviceFrame* frame)
	{
		//in step by step debug it is not going inside of observer function, but jumping over
		//this is creating problem for me, because object will be destroyed at the end of this function
		emit NewFrame(frame);
	}
signals:
	void NewFrame(DeviceFrame* frame);
private:
	//only static function can be call back
	static void OnImageGrabbed(Image* Image, const void* Device)
	{
		//extract RGB data from Image
		DeviceFrame frame;	//exactly this object i NEED

		//store data
		frame.Count = 1;
		frame.Type = DeviceFrameType::Video;
		frame.Size = Image->GetDataSize();
		frame.Frames = new unsigned char[frame.Size];
		memcpy(frame.Frames, Image->GetData(), frame.Size);

		//call slot in instance
		//((IpCameraBlackFly*)Device)->OnNewFrameStaticBridge(&frame); //nice, but bad try to solve my problem
		((IpCameraBlackFly*)Device)->OnNewFrame(&frame);


		//frame destroyed here
	}	
};

Frame consumer

class STREAMBUFFER_EXPORT StreamBuffer
{
	Q_OBJECT
public:
	StreamBuffer();
	~StreamBuffer();

public slots:
	void OnNewFrame(DeviceFrame* frame)
	{

		//make internal object
		DeviceFrame _buffer;

		//copy object, BUT access exception, because frame is already dead
		_buffer.CopyFrom(frame);
	}

};

Frame class

class DeviceFrame
{
public:
	DeviceFrame()
	{
		Frames = nullptr;
	}
	~DeviceFrame()
	{
		if (Frames != nullptr)
			delete Frames;
	}
		
	//copy function	
	DeviceFrame& operator = (DeviceFrame* original)
	{
		if (Size != original->Size)
		{
			if(Frames != nullptr)
				delete Frames;

			Frames = new unsigned char[original->Size];

		}

		Type = original->Type;
		Size = original->Size;
		Count = original->Count;
		Time = original->Time;
		//THE moment of exception
		memcpy(Frames, original->Frames, original->Size);

		return *this;
	}

	DeviceFrameType Type;
	int Size = 0;
	int Count = 1;
	qint64 Time = QDateTime::currentMSecsSinceEpoch();

	//QByteArray Frames;
	unsigned char* Frames;
};

Connection code

	//OK
	connect(_device, SIGNAL(NewFrame(DeviceFrame*)), this, SLOT(OnNewFrame(DeviceFrame*)),Qt::DirectConnection);
	//NOK
	connect(_device, SIGNAL(NewFrame(DeviceFrame*)), this, SLOT(OnNewFrame(DeviceFrame*)));

JohanSolo

You're giving the address of a temporary object. So either allocate the object on the heap (but then you should properly destroy it), or you can have a DeviceFrame member instance in your IpCameraBlackFly class, in which case the lifetime of the DeviceFrame is equal to the one of your IpCameraBlackFly object.

Edit: I replied a bit too soon, therefore let me develop a bit. You've already noticed your problem comes from object lifetime. IIRC, the DirectConnection ensure the slot is called right away, whilst the AutoConnection (the default one) can wait until the next event loop, in which case the stack object is deleted before the slot has a chance to process the camera frame.

Mikl

Thank you for the answer.
Do not think, what make it global will work. Camera is producing 20fps. In this case, i will not have exception, but it is a chance, what it will be overwritten by next frame, before i can use it.
I have already working solution, where i am allocating memory in call back function and destroying in StreamBuffer. I found it bad. Is`t it?
Just let me know, if you need more information.

JohanSolo

@Mikl said in Call back function problem:

I have already working solution, where i am allocating memory in call back function and destroying in StreamBuffer. I found it bad. Is`t it?
Just let me know, if you need more information.

I actually worked with cameras from Point Grey as well, and we did allocate the frame each time data was retrieved. I know this looks ugly, but we did not find any smarter / faster solution (I'm not telling there are none). This even becomes trickier with multithreading...

Mikl

@JohanSolo
It is a BIG chance, what my app will run during days. Recording, playback, network communication, ...
Want to make it as "perfect" as possible

Is Smart pointers can be a solution?
May be i need to make wrapper around IpCamera class?
.....

Sometimes i missing garbage collector with dcnew keyword from .NET.

JohanSolo

@Mikl said in Call back function problem:

Is Smart pointers can be a solution?

Yes, this can be very handy. We basically did this.

Mikl

@JohanSolo
Smart pointers can be a solution, but it will overload my functions. I was trained to write code so, what next developer after me will be maniacal serial killer, who know my address. And i believe in karma. Next reader will think about me "what a sh@t?". And i don`t like this.

void IpCameraBlackFly::OnNewFrame(DeviceFrame* frame)
goes to
void IpCameraBlackFly::OnNewFrame(QWeakPointer<DeviceFrame> frame)

Another solution, is to pass object. Not pointer. Yes it will be one extra copy done by system (time + memory), but i can be sure, what will not have memory access error. who care in our time about 2Mb and 200 cpu tics? Not really elegant and optimal, but safe.

JohanSolo

@Mikl said in Call back function problem:

Another solution, is to pass object. Not pointer. Yes it will be one extra copy done by system (time + memory), but i can be sure, what will not have memory access error. who care in our time about 2Mb and 200 cpu tics? Not really elegant and optimal, but safe.

That sounds reasonable to me. As long as the extra time for the copy is not a problem this is the easiest I'd say.

Mikl

@JohanSolo
Heh! You never know...
Do you know how it is happening?
You developing safe app on nice new laptop. Demonstrating to your boss. He is happy.
Next month you see him using in on 40fps with 1024x900 on 11'' 6 year old netbook. Blaming you, what it is too slow.

Mikl

@JohanSolo
I did modifications in a code and now passing object. It looks more simple and brings less problems. No need to delete pointer after usage, no worries about memory access,...

But, speed test, using QueryPerformanceCounter, schowing, what is it 1500 times slower, comparing passing pointer. 1500 ticks instate of 1.

Question is "what is 1500 ticks"? Should i worry about it? Can i speed it up?

My copy constructor is

DeviceFrame(const DeviceFrame& original)
{
	Copy(&original);
}

void Copy(const DeviceFrame* original)
{
	if(Frames != nullptr)
		delete Frames;

	Type = original->Type;
	Size = original->Size;
	Count = original->Count;
	Time = original->Time;
	Frames = new unsigned char[original->Size];
	memcpy(Frames, original->Frames, original->Size);
}

JohanSolo

@Mikl said in Call back function problem:

I did modifications in a code and now passing object. It looks more simple and brings less problems. No need to delete pointer after usage, no worries about memory access,...

But, speed test, using QueryPerformanceCounter, schowing, what is it 1500 times slower, comparing passing pointer. 1500 ticks instate of 1.

Question is "what is 1500 ticks"? Should i worry about it? Can i speed it up?

IIRC 1 tick is one millisecond, therefore you're running at 0.6 Hz. There should be a macro which performs the conversion number of ticks to seconds.

My copy constructor is

DeviceFrame(const DeviceFrame& original)
{
	Copy(&original);
}

void Copy(const DeviceFrame* original)
{
	if(Frames != nullptr)
		delete Frames;

	Type = original->Type;
	Size = original->Size;
	Count = original->Count;
	Time = original->Time;
	Frames = new unsigned char[original->Size];
	memcpy(Frames, original->Frames, original->Size);
}

I don't see how there a way to speed up this, unless if you precallocate a bunch of frames in a circular buffer. What is the typical size of a frame?

Mikl

@JohanSolo said in Call back function problem:

IIRC 1 tick is one millisecond, therefore you're running at 0.6 Hz. There should be a macro which performs the conversion number of ticks to seconds.

Are you sure about "millisecond"? It means, what copy is taking 1.5 sec.
http://superuser.com/questions/101183/what-is-a-cpu-tick

I don't see how there a way to speed up this, unless if you precallocate a bunch of frames in a circular buffer. What is the typical size of a frame?

Bad news. Frame size is set from UI. No "typical" size. It is a "maximum" size. Depending of camera spec. But i will "eat" huge amount of memory for wining couple of ticks.

JohanSolo

@Mikl said in Call back function problem:

Bad news. Frame size is set from UI. No "typical" size. It is a "maximum" size. Depending of camera spec. But i will "eat" huge amount of memory for wining couple of ticks.

Well, you could only reallocate if more memory is needed (and "waste" extra memory if the frame is smaller than the allocated area).

About the tick duration: I might have mixed up with the CLOCKS_PER_SEC (I'm not an intensive user of the windows API). The 1 ms was just a (apparently wrong) guess.