Log in

View Full Version : Trying to write a simple frame caching filter - need help with smart pointers, again


wonkey_monkey
23rd May 2019, 21:13
Here is a (simplified) snippet of code from a filter I'm writing to force frames to be cached by Avisynth:

FrameCache::FrameCache(PClip _child, IScriptEnvironment* env) : GenericVideoFilter(_child) {
int i;

cache = new PVideoFrame[end - start]; // cache is a private PVideoFrame*

for (i = 0; i < vi.num_frames; i++) {
cache[i] = child->GetFrame(i, env);
}
}

FrameCache::~FrameCache() {
int i;

for (i = 0; i < vi.num_frames; i++) {
cache[i - start] = NULL;
}

delete cache; // crashes here
}

The constructor creates an array of PVideoFrame and then populates it with pointers to every frame of the clip. This seems to work, as it keeps those frames live in Avisynth's cache and makes working with them much faster (for my particular purposes).

But the filter crashes when the destructor is called. My understanding was that by setting the PVideoFrame elements to NULL, I would release the hold on those frames, but calling delete on cache causes a crash.

Is this expected behaviour? I thought calling a delete on an array of NULL pointers should generally have no effect other than freeing up the memory for the array.

If I use malloc/free instead of new/delete there's no crash, but I'd like to understand what's going on instead of just trusting that to work.

No comments on what a terrible idea this might be, please ;) It's for a very specific purpose and does its job perfectly (apart from the crash).

StainlessS
23rd May 2019, 21:51
Have you just tried just calling delete on cache, without setting elements to NULL.
Delete on array of UDT's should call the desructor of each element prior to deleting the array itself.

Dont know why its crashing though.

wonkey_monkey
23rd May 2019, 22:05
Yup, tried that, still crashed.

Wilbert
23rd May 2019, 22:28
If you set it to NULL there is nothing to delete. I guess 'delete cache[];' should work since it's an array (but it will not do anything in this case).

wonkey_monkey
23rd May 2019, 23:38
If you set it to NULL there is nothing to delete. I guess 'delete cache[];' should work since it's an array (but it will not do anything in this case).

"delete cache[];" is apparently not valid syntax. As I understand it, "delete cache;" should call delete on all the PVideoFrames in the array, but since they have been set to NULL, I would have expected it to do nothing. Something is trying to something, though. Maybe that kind of NULL safety isn't a feature of smart pointers, because simply setting them to NULL is equivalent to deletion (or so I understand).

StainlessS
23rd May 2019, 23:41
Oops, yes, I did not notice that in your source, I should have.


cache = new PVideoFrame[end - start];

You have to use below syntax to delete, when allocating array as above, eg [blue part]


delete [] cache;


The [] bit causes it to call destructor on each element of the array.

pinterf
24th May 2019, 07:45
Anyway, you can insert forced caching of an intermediate clip into the filter creation process

See "InternalCache" here:
http://avisynth.nl/index.php/Filter_SDK/Env_Invoke
(I think cache hints are not relevant in avs+, dunno)

Or actual usage
https://github.com/pinterf/AviSynthPlus/blob/MT/avs_core/convert/convert_planar.cpp#L3205

wonkey_monkey
24th May 2019, 21:52
delete [] cache;


The [] bit causes it to call destructor on each element of the array.

That was the answer! Thanks. Not that it needs to cause destruction of each element, but it seems that syntax is required if new was used to create an array with [].

StainlessS
25th May 2019, 00:56
Not that it needs to cause destruction

Not sure bout this but, perhaps when PVideoFrame set NULL, the secret pointer to data buffer is nullified [and reference count to the buffer are decremented],
the 'Safe Pointer' object still exists and needs destruction, but would return (as if) null if interogated (weird secret C++ stuff).