Implementing cache hints

[dividee]

@Bidoche:
The Cache is currently created after the filter is evaluated (see ExpFunctionCall::Evaluate in expression.cpp).

@Shodan:We added another filter - let's call it cachemanager. This filter will be added before all filters. One filter in the filter chain before all filters.
This filter will not cache any images. It will however recieve cache hints from the filter it is cachemanager for. It (of course) also intercepts all GetFrame requests.

We will still add the output filter, as we used to. So basicly the cachemanager is sending commands to the output filter of the previous filter.That's what I was calling "cache proxy" in my previous post, but "cachemanager" is a better term, and you explained it a lot better. :)The cachemanager generates a random number on startup, so it can identify itself uniquely to the output filter.Why generate a random number ?? Just passing the 'this' pointer should work as well.The cachemanager then sends commands to the cache about which files it needs to have stored on each frame. Or it could simply send which frames it would like to have removed from the cache.Not sure I understand here. Do you suggest having only one cachemanager per filter, even if the filter has multiple clip arguments? I'd say you need one cachemanager per upstream filter; I can't see how you would implement it otherwise.

[sh0dan]

@dividee:
The random number thingie is to be able to use the current cache-hinting feature to pass arguments. Can we extend the PClip class (with a new method) without forcing recompiles of the plugins?

I see your point on one cachemanager per upstream clip. Attaching that on script generation shouldn't be that difficult.

My point by making the cache manager communicate to the cache is to allow one output cache (from one filter) to have several cachemanagers talking to it. The number (or the this argument) is only to let the output cache of the previous filter know which cache-manager it is talking to.

To clarify: We maintain an output cache on all filters - just as it is today. Inbetween the output cache and the input of the filter we stick a cache manager filter. This filter identifies itself to the cache, and sends hints to the cache about which input filter it is. The cache is then able to determine which unique input filter is requesting frames now. Thereby it is able to cache only the frames needed by this input filter. There can be several cache managers talking to the cache, but since they are all uniquely identified it is now able to distinguish.

If we can extend IClip to include a PVideoFrame GetCachedFrame(int frame, IClip* cache, IScriptEnvironment* env) then that could be the way the cache-manager requested frames from the output cache of the previous filter.

That could work - couldn't it? We get all the advantages, no random numbers and no recompiles.

[dividee]

I attach a figure of what I think you suggest. Looks good to me, provided you can add the method to IClip without a recompile of the plugins. I think it's possible, if you add the new method at the end (textually; in declaration order) of IClip in avisynth.h.

I've been experimenting a bit with loading V1/V2.0 plugins in V2.5 an saw that when an old plugin was calling IClip::GetVideoInfo, in fact SetCacheHints was called because it 'has taken it's place' in the vtable (compare the old definition of IClip and the new one to see what I mean). So, it seems virtual methods are called by their index in the vtable and the order of the methods in the vtable follow the order of declaration...

PS: for this experiment of loading old plugins, I tried to create two "Adapter" filters (one upstream and one downstream) which would translate between the old and the new interfaces. It worked for some filters (those who used 'MakeWritable') but not others (those who used NewVideoFrame), because I didn't "wrap" the ScriptEnvironment and the plugin passed the wrong VideoInfo struct. I gave up because the code was already a real mess (with void* casts and the like) and I couldn't see how to deallocate things properly. If anyone is interested I can open a new thread about it.

[scmccarthy]

@dividee

Dividing all filters between those that use MakeWritable and those that use NewVideoFrame is helpful to me because I never used MakeWritable before. I was looking at PeachSmoother script and didn't understand when the frame was input or output plus how to pass/read? video info and the ScriptEnvironment.

The question that occurs to me now is is there a difference between the way exceptions are thrown between the two types of filters?

Stephen

[sh0dan]

@dividee: Your illustration hits the nail right on the head! That's what I was thinking.

I'll try do to an experimental implementation, if Bidoche isn't interested (it's really only minor changes to his code, and wrapping it into cache.cpp)

Thanks for investigating the issue regarding plugins. I hope that most people use NewVideoFrame, except if they only do partial image processing - in my head it would theoretically be faster (avoiding one image blit per frame).


@scmccarthy: When you use env->MakeWritable() you take an input image, and makes it possible for your filter to write on it. That way it becomes both source and destination. This is a good thing, if you only write on a part of the image (so you don't have to copy the entire image).
Otherwise you might as well create a new image to write your new image into.

[Guest]

Originally posted by sh0dan
I hope that most people use NewVideoFrameI use MakeWritable() a lot. Is that going to be a problem?

[Bidoche]

@sh0dan

You can use my code, no problem.

I don't understand why doing everything to avoid recompile here, since 2.5beta is supposed to require one...
I think we can afford limited structure change in the api rather than adding complexity.

About MakeWritable and such, I think there is a lack in the api :
PVideoFrame works as a VideoFrame * const
if we add a PCVideoFrame working as a Const VideoFrame * const, it would solve some issues.
Then filters will return PCVideoFrame (not modifiable)
if you want to modify, you cast it to PVideFrame (same as MakeWritable) or make a new PVideoFrame.
This will allow to remove the sequence number in the vfb class and make sure than VideoFrames in cache are not modified.

I will come with relative code when ready.

[dividee]

@sh0dan

As I said, I mostly gave up this plugin adaptation, but since you insist, I'll have another look ;) It somehow worked for some filters, but crashed at the end and was more a proof of concept than anything. I doubt I can make clean code for it, and my goal was just to make a temporary solution to help the adoption of the new version; when enough filters are available for 2.5, it would have been removed.

@neuron2

Using MakeWritable is good, IMHO. Really, I don't understand why Sh0dan hopes that most people use NewVideoFrame.

@Bidoche

Won't this (CPVideoFrame) affect performance ? After all, when a framebuffer is modified in the cache, that's for performance reason, isn't it ? And won't casting it to a PVideoFrame negate the benefit anyway ?

@all

This leads me to a question. I don't understand perfectly how all this works. When is a frame in the cache modified ? When the filter use MakeWritable ? But then, MakeWritable makes a copy of the frame if the refcount is >1. And how can the refcount be only 1 if it's owned both by the cache and the filter ?
Also sh0dan stated above that MakeWritable is mostly useful when you only write part of the image. But in my experience it's nearly always benefical (if you can use it). For instance, Tweak is noticeably faster with MakeWritable, and it filters the whole image. That leads me to think that often enough MakeWritable doesn't have to do a memory copy. So what's this thing with the refcounter ? Doesn't the cache "owns" the frame too ?

[Bidoche]

@dividee

The actual caching code invalidates VideoFrame modified while cached.
So making them impossible to modify will simplify.

To my undestanding, these modifications are processing fault anyway. A filter is supposed to always give same output, that's why caching is possible, if a cached frame is modified, it will contradict that and may give strange (and unpredictable) results.

MakeWritable can be faster when a VideoFrame is not shared.
Because then it does not copy , but return the source itself.
Of course, for that to happen, it should not be cached.

And no, I don't think it can affect performance.
The cast will 'steal' the pointer when possible (ie 1 ref) or copy otherwise.

[Bidoche]

In attachement, a modified avisynth.h (not complete, nor final) with some (lots) structure change.

It breaks lots of things, I don't know if it breaks something really vital. (like the exporting env stuff...)

[Bidoche]

@sh0dan

About this optional parameter, it will force everyone to update their GetFrame definition to include it, but not change the internal code.

Can you precise which linker error occurred ?
Are you sure it's not because you did not commented/uncommented the good sections ?

a default to cache last 5 (or 10) should be okay I think

[sh0dan]

The linker error:

cache.obj : error LNK2001: unresolved external symbol "public: void __thiscall FrameCache::CachedVideoFrame::set(int,class PVideoFrame const &)" (?set@CachedVideoFrame@FrameCache@@QAEXHABVPVideoFrame@@@Z)
Release/avisynth.dll : fatal error LNK1120: 1 unresolved externals
Error executing link.exe.

Seems like it is missing a set function.

About the GetFrame stuff, I assume that only those interested in changing the default caching mechanism will need to change it.

[Bidoche]

set is missing...
Arghhhh, stupid me !!!
I added this one to increase efficiency and I forgot to implement it...

[Guest]

Umm, isn't it risky to put new caching in right before the beta release?

[sh0dan]

It is - we should test it as good as possible. It'll probably be the main reason for beta 1.
Otherwise I actually feel we might just as well go ahead and make a 2.5.0, without the beta label.

It shouldn't mean total reaarangement of anything. There could be some memory issues, but that's the worst I think could happend - functional bugs in the cache are rather easy to discover.