Strange performance problem with ffdshow + avisynth [Archive]

e-t172

16th March 2009, 20:24

Hi,

I'm using avisynth with ffdshow for realtime use.

I have some performance problems with my filters, and when investigating I found the cause of the problem : not the avisynth filters, but... avisynth itself.

Testing with the "Elecard Chegepuga" Directshow filter for average frame rate measurement, with the following chain:

- Source: MPEG-2 TS HDTV Capture (1080i MPEG-2, NTSC)
- Haali Media Splitter
- ffdshow r2768 (2009-03-10, tried with 2009-03-02, same thing) video decoder, all filters disabled
- Avisynth 2.5.8

Now the results (average frame rate, tested over 4000 frames):

- With the avisynth filter disabled in ffdshow: 110.94 FPS
- With the avisynth filter enabled (with an empty script): 77.75 FPS

So, the simple fact that avisynth was enabled, even if it was doing nothing, reduced the performance by 40%!

Is this normal ?

Sagekilla

16th March 2009, 21:24

If it's not dropping frames, I don't see why this should be a problem if you're doing playback.

It could be just that the avisynth ffdshow_source() filter is slower than ffdshow natively decoding.

e-t172

16th March 2009, 21:39

If it's not dropping frames, I don't see why this should be a problem if you're doing playback.

Well, it's a problem because I want to use some resource-consuming avisynth filters but this problem is limiting my options.

It could be just that the avisynth ffdshow_source() filter is slower than ffdshow natively decoding.

40% slower? After all it's just passing buffers...

Sagekilla

16th March 2009, 22:44

I'm not sure why it's so slow, you could try asking one of the ffdshow tryouts devs if there's an issue or something.

Didée

16th March 2009, 22:52

BTW, your math is not correct. 77.75/110.94 is 0.700~. So it's 30% slower, not 40%. Your claim was off by 25%. (30/40).

(edit - deleted the bandwidth nonsense.) :o

Leak

16th March 2009, 23:03

One cause is probably that every input frame is copied once before being fed to AviSynth if buffering or "Apply pulldown" is turned on since the current frame is lost when the next frame is processed.

Of course turning off buffering will prevent you from using any script that needs access to more than one frame...

np: Telefon Tel Aviv - Nothing Is Worth Losing That (Map Of What Is Effortless)

e-t172

16th March 2009, 23:13

I'm not sure why it's so slow, you could try asking one of the ffdshow tryouts devs if there's an issue or something.

Yeah. I'm considering posting to the mailing list.

BTW, your math is not correct. 77.75/110.94 is 0.700~. So it's 30% slower, not 40%. Your claim was off by 25%. (30/40).

(edit - deleted the bandwidth nonsense.) :o

Sorry, my bad. Well, 30 or 40%, it's still a huge performance degradation.

One cause is probably that every input frame is copied once before being fed to AviSynth if buffering or "Apply pulldown" is turned on since the current frame is lost when the next frame is processed.

Buffering was disabled and "Ignore pulldown" was selected during the tests. Besides, there was no color space conversion (at least not that I know of).

e-t172

20th March 2009, 15:36

After all it's just passing buffers...

I'm reading the code right now, and it seems I was wrong. It's not just passing buffers:

AVS_VideoFrame* AVSC_CC TimgFilterAvisynth::Tffdshow_source::get_frame(AVS_FilterInfo *fi, int n)
{

PVideoFrame frame(input->env,input->env->avs_new_video_frame_a(*input->env,&filter->vi,16));

(...)

// Find the buffered frame that's closest to n and return it

(...)

TffPict::copy(frame->GetWritePtr(PLANAR_Y),frame->GetPitch(PLANAR_Y),buffer.data[0],buffer.pitch[0],buffer.width[0],buffer.height[0]);
TffPict::copy(frame->GetWritePtr(PLANAR_U),frame->GetPitch(PLANAR_U),buffer.data[1],buffer.pitch[1],buffer.width[1],buffer.height[1]);
TffPict::copy(frame->GetWritePtr(PLANAR_V),frame->GetPitch(PLANAR_V),buffer.data[2],buffer.pitch[2],buffer.width[2],buffer.height[2]);

(...)

}

(src\imgFilters\avisynth\TimgFilterAvisynth.cpp:141)

In fact, each frame is copied at least once no matter what, even if "buffer back/ahead" is disabled. It seems to be a limitation in Avisynth's public interface: in order to pass the frame to avisynth, one must use the avisynth provided buffer (thus the necessary copy). Apparently there is no way to pass ffdshow's buffers directly to avisynth using avisynth's public interface. If we use "buffer back/ahead", it's even worse: the frames are copied TWICE, first in process() to buffer them, then in get_frame() to serve them to avisynth.

One (very hackish) solution would be to violate avisynth's interface and create a AVS_VideoFrame manually (bypassing the avs_new_video_frame_a() call) pointing to ffdshow's buffer. I don't know if avisynth will accept it though: the "DO NOT USE THIS STRUCTURE DIRECTLY" warning is probably here for a reason...

However, one non-hackish solution that should improve performance of the "buffer back/ahead" feature would be to use AVS_VideoFrames (properly created using avs_new_video_frame_a()) for buffering instead of ffdshow TframeBuffers. Then in getframe() we pass the AVS_VideoFrames directly instead of copying them. It would seriously decrease the overhead of the "buffer back/ahead" option.

Jeremy Duncan

24th March 2009, 19:19

I'm reading the code right now, and it seems I was wrong. It's not just passing buffers:

AVS_VideoFrame* AVSC_CC TimgFilterAvisynth::Tffdshow_source::get_frame(AVS_FilterInfo *fi, int n)
{

PVideoFrame frame(input->env,input->env->avs_new_video_frame_a(*input->env,&filter->vi,16));

(...)

// Find the buffered frame that's closest to n and return it

(...)

TffPict::copy(frame->GetWritePtr(PLANAR_Y),frame->GetPitch(PLANAR_Y),buffer.data[0],buffer.pitch[0],buffer.width[0],buffer.height[0]);
TffPict::copy(frame->GetWritePtr(PLANAR_U),frame->GetPitch(PLANAR_U),buffer.data[1],buffer.pitch[1],buffer.width[1],buffer.height[1]);
TffPict::copy(frame->GetWritePtr(PLANAR_V),frame->GetPitch(PLANAR_V),buffer.data[2],buffer.pitch[2],buffer.width[2],buffer.height[2]);

(...)

}

(src\imgFilters\avisynth\TimgFilterAvisynth.cpp:141)

In fact, each frame is copied at least once no matter what, even if "buffer back/ahead" is disabled. It seems to be a limitation in Avisynth's public interface: in order to pass the frame to avisynth, one must use the avisynth provided buffer (thus the necessary copy). Apparently there is no way to pass ffdshow's buffers directly to avisynth using avisynth's public interface. If we use "buffer back/ahead", it's even worse: the frames are copied TWICE, first in process() to buffer them, then in get_frame() to serve them to avisynth.

One (very hackish) solution would be to violate avisynth's interface and create a AVS_VideoFrame manually (bypassing the avs_new_video_frame_a() call) pointing to ffdshow's buffer. I don't know if avisynth will accept it though: the "DO NOT USE THIS STRUCTURE DIRECTLY" warning is probably here for a reason...

However, one non-hackish solution that should improve performance of the "buffer back/ahead" feature would be to use AVS_VideoFrames (properly created using avs_new_video_frame_a()) for buffering instead of ffdshow TframeBuffers. Then in getframe() we pass the AVS_VideoFrames directly instead of copying them. It would seriously decrease the overhead of the "buffer back/ahead" option.

I wonder if leak will reply to your last post?

e-t172,

Why don't you write a patch with the code you wrote about and post it here for leak to look at? :)

e-t172

24th March 2009, 19:55

Why don't you write a patch with the code you wrote about and post it here for leak to look at? :)

For now I'm trying to use the avisynth structures manually to avoid the mandatory frame copy in get_frame(). I will probably need to modify Avisynth as well to make this work. I got some promising results (I can get few seconds of video before crashing). Don't hold your breath though: covering all corner cases in Avisynth is like walking on a minefield...

Leak

25th March 2009, 15:07

Don't hold your breath though: covering all corner cases in Avisynth is like walking on a minefield...
I seem to remember that this is why I wrote it like that in the first place... :p (That, and the extra copies don't hurt much with DVDs... :D)

(NB: Of course I didn't get any work done this weekend - so if you manage to whip up a working solution I'd be grateful...)

e-t172

25th March 2009, 16:40

I seem to remember that this is why I wrote it like that in the first place... :p (That, and the extra copies don't hurt much with DVDs... :D)

Yes. But with 1080p content it's another story, because copying 1080p frames is much more expensive.

(NB: Of course I didn't get any work done this weekend - so if you manage to whip up a working solution I'd be grateful...)

Well, perhaps you could work on the second assignment (use AVS_VideoFrames for buffers instead of Tframebuffers in process()). Judging from how the code looks, it should be fairly easy (not very exciting, though). Thing is, if I manage to pass frames to Avisynth by reference, then this second assignment is useless (because then it won't matter to choose between Tframebuffers or AVS_VideoFrames anymore).

Hold on - I just got a different idea. Right now I'm trying to create a avisynth frame and force it to reference the ffdshow buffer. What about the exact opposite: in the ffdshow initialization code, instead of just allocating a buffer, we allocate a Avisynth frame and THEN we use the write pointers as the ffdshow buffers. It would be very audacious (because it would mean integrating avisynth within the _core_ of ffdshow), but in theory it would work. Moreover, if we manage to force ffdshow to use different buffers for different frames (maybe it does that already?), then it would be very interesting: we could eliminate all copies even with the "buffer back/ahead" option enabled!

By the way, can you explain to me what the "Apply pulldown" and "Smooth timestamps" features do exactly? Even with the doc and the code, I don't understand the purpose. When I do IVTC, I use Telecide() and Decimate() and it just works, even with "Ignore pulldown" selected.

e-t172

25th March 2009, 17:55

Hold on - I just got a different idea. Right now I'm trying to create a avisynth frame and force it to reference the ffdshow buffer. What about the exact opposite: in the ffdshow initialization code, instead of just allocating a buffer, we allocate a Avisynth frame and THEN we use the write pointers as the ffdshow buffers. It would be very audacious (because it would mean integrating avisynth within the _core_ of ffdshow), but in theory it would work. Moreover, if we manage to force ffdshow to use different buffers for different frames (maybe it does that already?), then it would be very interesting: we could eliminate all copies even with the "buffer back/ahead" option enabled!

Hum, after some code reading it appears it would not be an easy task: there is no common place where the buffer gets allocated, it's codec dependant. Never mind.

Leak

25th March 2009, 18:28

Hum, after some code reading it appears it would not be an easy task: there is no common place where the buffer gets allocated, it's codec dependant. Never mind.
Exactly what is codec dependant? Definitely not TimgFilterAviSynth's internal buffer allocation...

EDIT: OMG, I think I just figured out what you meant - no, that definitely won't work, and it'd also make ffdshow depend on AviSynth being installed, which is another big no-no... sorry, but buffering is one copy you just can't avoid. You can just try replacing ffdshow's internal TframeBuffer with an AVS_VideoFrame.

Anyway, you need to keep allocating a new AVS_VideoFrame for every buffered frame since you can't reuse them as you can't know if they're still used by one of AviSynth's filters, but that's a rather fast operation since AviSynth's cache is tuned for that, especially if all frames are of the same size...

"Apply pulldown" checks the field repeat flags and applies soft pulldown to the frames, i.e. it turns the flagged 23.976 FPS video into 29.976 FPS "hard" telecined video.

Telecide() and Decimate() without applying pulldown will work with video that has hard telecine (i.e. the pulldown frames were fed to the encoder), but with soft pulldown you'd have a 23.976 FPS video as input and Telecide() and Decimate() would reduce it to a probably very jerky 19.18 FPS.

Of course doing this is pointless if the pulldown flags were set correctly to begin with (in which case you should simply use "Smooth timestamps"), but I have a few DVDs where just throwing away the pulldown flags gives you ugly combing some of the time, which "Apply pulldown" + IVTC fixes.

"Smooth timestamps" doesn't do anything to the frames themselves, but it averages out the duration of the timestamps of soft telecined material.

Their duration usually alternates between 33ms and 49ms, and "Smooth timestamps" averages the durations down to 41ms for each frame. Otherwise if you apply, say, a bob filter that doubles the amount of frames you'll get two bobbed frames of 16.5ms duration followed by two frames of 24.5ms, which again will be jerky. (This is mainly due to AviSynth having no concept of variable framerate while DirectShow does...)

EDIT: Also, like I said I don't know when I'll find time to work on ffdshow as I'm currently quite stressed with urgent stuff at work and I just don't want to work on code at home when I've done it under time-pressure all day at work...

np: Robert Babicz - Don't Look Back (Kompakt Total 9 (Disc 2))

e-t172

25th March 2009, 21:19

I managed to make the thing stable enough to do some performance tests with my modifications. Results (average FPS):

Avisynth filter disabled: 141.30
Avisynth filter enabled (empty script): 139.16

Seems I was right. It works!

Now I have a serious problem: the whole thing crashes after a few frames of video, but only under certain conditions:
- With the Avisynth DLL in debug mode, it works. No crashes.
- With the Avisynth DLL in release mode, it crashes after a few frames.
- With the Avisynth DLL in release mode AND the debugger running, it works. (??!)

I don't understand what's happening... might come from my compilation parameters, though. I'm investigating. Note that the tests above were made with the Release ffdshow and Avisynth DLLs, but with the debugger running. (without the debugger it's 150 fps instead of 140)

Jeremy Duncan

26th March 2009, 00:09

post your modified src. Tell me what files you changed. I will keep your code changes and use the default includes and property settings.
it should work since all you did was change some code, the building should not have changed from how I know to built the avisynth.
post your modified src and tell me the files you altered, the file names. :D !

e-t172

26th March 2009, 00:20

I modified ffdshow, not avisynth. Except for a very small change I had to make to Avisynth which is only necessary if you build Avisynth in debug mode (it skips a check which makes no sense if the VideoFrame is pointing to ffdshow's buffers).

Here's what I changed in ffdshow:

Index: src/imgFilters/avisynth/TimgFilterAvisynth.cpp
===================================================================
--- src/imgFilters/avisynth/TimgFilterAvisynth.cpp (revision 2791)
+++ src/imgFilters/avisynth/TimgFilterAvisynth.cpp (working copy)
@@ -109,7 +109,7 @@
{
Tffdshow_source *filter=(Tffdshow_source*)fi->user_data;
Tinput* input=filter->input;
- PVideoFrame frame(input->env,input->env->avs_new_video_frame_a(*input->env,&filter->vi,16));
+

// Calculate request statistics for currently produced frame

@@ -128,33 +128,35 @@

// Find the buffered frame that's closest to n and return it

- if (input->numBuffers > 0)
+ if (input->numBuffers > 0 && (filter->input->csp&FF_CSPS_MASK) == FF_CSP_420P)
{
TframeBuffer& buffer=input->buffers[findBuffer(input->buffers,input->numBuffers,n)];

if (debugPrint)
DPRINTF(_l("TimgFilterAvisynth: Looked up frame %i, using frame %i"),n,buffer.frameNo);

- switch (filter->input->csp&FF_CSPS_MASK)
- {
- case FF_CSP_420P:
- TffPict::copy(frame->GetWritePtr(PLANAR_Y),frame->GetPitch(PLANAR_Y),buffer.data[0],buffer.pitch[0],buffer.width[0],buffer.height[0]);
- TffPict::copy(frame->GetWritePtr(PLANAR_U),frame->GetPitch(PLANAR_U),buffer.data[1],buffer.pitch[1],buffer.width[1],buffer.height[1]);
- TffPict::copy(frame->GetWritePtr(PLANAR_V),frame->GetPitch(PLANAR_V),buffer.data[2],buffer.pitch[2],buffer.width[2],buffer.height[2]);
- break;
+ AVS_VideoFrameBuffer* avsFrameBuffer = new AVS_VideoFrameBuffer;
+ avsFrameBuffer->data = buffer.data[0];
+ avsFrameBuffer->data_size = buffer.pitch[0] * buffer.height[0] * buffer.bytesPerPixel;
+ avsFrameBuffer->sequence_number = 0;
+ avsFrameBuffer->refcount = 1;

- case FF_CSP_YUY2:
- TffPict::copy(frame->GetWritePtr(),frame->GetPitch(),buffer.data[0],buffer.pitch[0],buffer.width[0]*buffer.bytesPerPixel,buffer.height[0]);
- break;
+ AVS_VideoFrame* avsFrame = new AVS_VideoFrame;
+ avsFrame->refcount = 1;
+ avsFrame->vfb = avsFrameBuffer;
+ avsFrame->height = buffer.height[0];
+ avsFrame->offset = 0;
+ avsFrame->offsetU = buffer.data[1] - buffer.data[0];
+ avsFrame->offsetV = buffer.data[2] - buffer.data[0];
+ avsFrame->pitch = buffer.pitch[0];
+ avsFrame->pitchUV = buffer.pitch[1];
+ avsFrame->row_size = buffer.width[0];

- case FF_CSP_RGB24:
- case FF_CSP_RGB32:
- TffPict::copy(frame->GetWritePtr(),frame->GetPitch(),buffer.data[0]+buffer.pitch[0]*(buffer.height[0]-1),-buffer.pitch[0],buffer.width[0]*buffer.bytesPerPixel,buffer.height[0]);
- break;
- }
+ return avsFrame;
}
else
{
+ PVideoFrame frame(input->env,input->env->avs_new_video_frame_a(*input->env,&filter->vi,16));
int count;
unsigned long* dest;

@@ -186,9 +188,11 @@

break;
}
+
+ return frame;
}

- return frame;
+
}

//============================= TimgFilterAvisynth::Tffdshow_setAR =============================

(note that this is very experimental and incomplete code which is absolutely not suitable for normal use; it only accepts YV12 as input, and is a blatant memory leak, amongst other things)

If you want to build Avisynth in debug mode, you'll have to make a change in core/cache.cpp, line approx. 241, Cache::childGetFrame(): comment all the "#ifdef _DEBUG" block.

If you want to try, make sure you enable the Avisynth filter, disable buffer back/ahead, ignore pulldown, and use an empty script.

Leak

26th March 2009, 00:38

Sorry, but I don't think that's gonna fly - you can't know what frames the filters in your (or rather, any) AviSynth script will request, so if a filter (re-)requests one of those frames you cobbled together without going through the proper channels (GetWritePtr and friends) while ffdshow has already released or re-used the memory the frame's data was stored at you'll get garbage at best and an access violation at worst...

The internal cache AviSynth will automatically insert between ffdshow_source and the filter after it WILL cause you all kinds of pain here.

You might want to ask IanB if he's got an idea...

np: Klimek - True Enemies False Friends (Yesteryears Suite) (Pop Ambient 2009)

Jeremy Duncan

26th March 2009, 00:47

I think Leak is saying that the thing you codes will run one waY BUT avisynth IS BUILT TO GO THE OTHER WAY too.
And when you go the other way the path has been muddled, so the whole idea of a dll which is to together different paths quickly is ruined.
So you need to think of how to let a dll run properly when you build your code, rather than thinking of just avisynth "TimgFilterAvisynth".

Leak

26th March 2009, 00:52

So you need to think of how to let a dll run properly when you build your code, rather than thinking of just avisynth "TimgFilterAvisynth".
No, I'm simply saying that an AviSynth script can request any frame it likes, not just the current one, and that AviSynth keeps input frames around after processing a frame via it's internal caching mechanism.

And the way he force-feeds the image data from ffdshow directly into an AviSynth frame will make AviSynth think the image data is still there when the filterchain after ffdshow_source re-requests a frame that has been cached, but ffdshow has already thrown out the frame along with the memory that was allocated for the image data...

So in short: ffdshow doesn't keep images around longer than neccessary (from it's non-AviSynth point of view), while AviSynth does and expects it from the AVS_VideoFrameBuffers you give it. Nothing to do with DLLs.

np: Sylvain Chauveau - Fly Like A Horse (Pop Ambient 2009)

e-t172

26th March 2009, 10:08

No, I'm simply saying that an AviSynth script can request any frame it likes, not just the current one, and that AviSynth keeps input frames around after processing a frame via it's internal caching mechanism.

And the way he force-feeds the image data from ffdshow directly into an AviSynth frame will make AviSynth think the image data is still there when the filterchain after ffdshow_source re-requests a frame that has been cached, but ffdshow has already thrown out the frame along with the memory that was allocated for the image data...

So in short: ffdshow doesn't keep images around longer than neccessary (from it's non-AviSynth point of view), while AviSynth does and expects it from the AVS_VideoFrameBuffers you give it. Nothing to do with DLLs.

I already thought about this. I think I have a solution: when ffdshow moves on to the next frame, if there is still Avisynth references to the current frame, then the frame will be copied and the VideoFrameBuffer pointers will be modified to reference the copy. So the Avisynth frame contents remains unchanged.

Theorically, the Avisynth cache manager should not be a problem because unless I'm mistaken, the cache manager is not called if I create VideoFrames and VideoFrameBuffers manually. It makes sense: if you don't ask buffers from the cache manager, then it will be "unaware" that you created your own.

Jeremy Duncan

26th March 2009, 10:55

It makes sense: if you don't ask buffers from the cache manager, then it will be "unaware" that you created your own.

No that won't work.
The way ffdshow works is there is ffdshow_source().
ffdshow_source() goes to the avisynth in ffdshow.
Then to the output.

In avisynth ffdshow there may be a distributor() used like in setmtmode.

distributor takes ideas like yours and muxes them.

see icture for better description:
http://img441.imageshack.us/img441/7645/seedistributor.th.jpg (http://img441.imageshack.us/my.php?image=seedistributor.jpg)

In the picture your idea of a buffer fix would qualify as thread 1.
preserving the integrity leak mentioned would qualify as threead two.

Both thread 1 and 2 use ffdshow source and go out through distributor.

Your idea is to use ffdshow_source with no distributor muxing the two threads.
In order for your idea to work it needs to recieve the ffdshow source and mt it to two threads, using distributor. In order to blend the two principles, yours and the one Leak mentioned.

e-t172

26th March 2009, 13:09

Maybe it's me (sorry, english is not my native language), but I absolutely don't understand what you're saying. Can you be a little more explicit?

By the way, I'm starting to think that this whole "use ffdshow's buffers" thing might be more trouble than it's worth. I mean, it only improves performance in one case: if "buffer back/ahead" is disabled, and pulldown ignored. In this case there is absolutely no frame copying and performance is improved. However, it will probably mean using a modified Avisynth DLL which is bad (can't use the official DLL anymore) and making it bug-free will not be easy. On the other hand, if we simply use Avisynth frames instead of Tframebuffers in process(), it will cause no buffer problems (because we use avisynth's buffers), and if "buffer back/ahead" or pulldown is enabled, we're still down to one frame copy instead of two.

So:

- Case 1: If "buffer back/ahead" and pulldown are disabled AND we pass ffdshow's buffers to Avisynth, performance is improved (before: one frame copy, after: no frame copy)
- Case 2: If "buffer back/ahead" and pulldown are enabled AND we pass ffdshow's buffers to Avisynth, performance is improved (before: two frame copies, after: one frame copy)
- Case 3: If "buffer back/ahead" and pulldown are disabled AND we just use avisynth frames in process(), performance is NOT improved (before: one frame copy, after: one frame copy)
- Case 4: If "buffer back/ahead" and pulldown are enabled AND we just use avisynth frames in process(), performance is improved (before: two frame copies, after: one frame copy)

My point is: we will never be able to get rid of the remaining frame copy when buffering. So when buffering is enabled, we will never get better performance than case 4 and 2. Consequently, the only situation where using ffdshow's buffers is the only way to improve performance is when buffering is disabled.

Correct me if I'm wrong, but I am under the impression that most people use the Avisynth filter with the "buffer back/ahead" option enabled (in order to do IVTC, motion interpolation, etc.). Using the filter without buffering is not very useful unless you're only using pure still image filters in Avisynth. So passing ffdshow's buffers to avisynth is not that useful after all, because in the majority of cases, we will have to do at least one frame copy anyway (in process()). So we better just use AVS_VideoFrames in process() and return them in get_frame(). This way, performance will be optimal when buffering. With buffering disabled performance will not be optimal, but who cares?

Leak

26th March 2009, 16:03

So we better just use AVS_VideoFrames in process() and return them in get_frame(). This way, performance will be optimal when buffering. With buffering disabled performance will not be optimal, but who cares?
Yeah, that sounds just about right, though it did improve performance in case you used AviSynth for, say, resizing, tweaking the image and (spatial) sharpening and stuff. All that requires only the current image...

I wonder if the easiest way wouldn't be to modify TframeBuffer to store a PVideoFrame instead of the buffer pointers it holds now, and just move the code that handles storing image data in the TframeBuffer there. But you'd have to make sure to delete the existing PVideoFrame before you re-use a TframeBuffer so AviSynth's reference counting is still correct, and you'd have to delete all PVideoFrames before shutting down the AviSynth filter.

That way you could choose between the old and the new behaviour, at least until all the kinks are worked out.

e-t172

26th March 2009, 16:10

I wonder if the easiest way wouldn't be to modify TframeBuffer to store a PVideoFrame instead of the buffer pointers it holds now, and just move the code that handles storing image data in the TframeBuffer there.

I had exactly the same idea in mind. I'm already working on it.

But you'd have to make sure to delete the existing PVideoFrame before you re-use a TframeBuffer so AviSynth's reference counting is still correct, and you'd have to delete all PVideoFrames before shutting down the AviSynth filter.

I'm not experienced enough in C++ to know the precise details of destructor calling. I've modified TframeBuffer as follows:

struct TframeBuffer
{
int frameNo;
int bytesPerPixel;
REFERENCE_TIME start;
REFERENCE_TIME stop;
int fieldType;
Tavisynth_c::PVideoFrame frame;
};

And here is a part of my modified process():

TframeBuffer& curBuffer=buffers[curBufferNo];

curBuffer.frameNo=curInFrameNo;

VideoInfo vi;
vi.width=input->dx;
vi.height=input->dy;
vi.fps_numerator=input->fpsnum;
vi.fps_denominator=input->fpsden;
vi.num_frames=NUM_FRAMES;
// RGB values: avisynth refers to the write order, FF_CSP_ enum refers to the "memory byte order",
// which under x86 is reversed, see the comment above the FF_CSP_ enum definition.
if (input->csp & FF_CSP_420P) vi.pixel_type=AVS_CS_YV12;
else if (input->csp & FF_CSP_YUY2) vi.pixel_type=AVS_CS_YUY2;
else if (input->csp & FF_CSP_RGB32) vi.pixel_type=AVS_CS_BGR32;
else if (input->csp & FF_CSP_RGB24) vi.pixel_type=AVS_CS_BGR24;

curBuffer.frame = PVideoFrame(self->input->env, self->input->env->avs_new_video_frame_a(self->input->env, &vi, 16));

I'm assuming that when curBuffer.frame gets reassigned (last line), then the destructor of the old curBuffer.frame gets called (thus decrementing the reference counter of the underlying AVS_VideoFrame). Is this right?

The filter does "delete buffers;" when shutting down. It should be enough to call the destructor of the PVideoFrames, right?

Leak

26th March 2009, 16:18

I'm assuming that when curBuffer.frame gets reassigned (last line), then the destructor of the old curBuffer.frame gets called (thus decrementing the reference counter of the underlying AVS_VideoFrame). Is this right?

The filter does "delete buffers;" when shutting down. It should be enough to call the destructor of the PVideoFrames, right?
Sounds about right. Setting the PVideoFrame to a new value and deleting the TframeBuffer it's contained in should decrease the reference count.

I just hope keeping that PVideoFrame around is enough to prevent the frame from being evicted from AviSynth's cache in case it hits the cache size, but it should. That's one of the reasons I stored the frames in my own buffers in the first place...

e-t172

26th March 2009, 16:24

Setting the PVideoFrame to a new value and deleting the TframeBuffer it's contained in should decrease the reference count.

I just read on the C++ Faq Lite that for that to work I need to do "delete[] buffers" instead of the current "delete buffers".

Jeremy Duncan

26th March 2009, 17:14

I'll try to be more clear so you might understand me.

a.) When you use ffdshow avisynth the next function is aware of what the previous function did.

Then after the last function it goes to the next ffdshow filter.
There is no quote 'unawareness'.

b.) When you use mt, the frames are split to worker threads processed by the cpu.
mt("",2) splits the picture.
setmtmode() splits differently and keeps the frame whole.

When cpu thread one runs, thread two works semi unaware about it.
You can see this in mt motion interpolation with the tearing.

So there you see how ffdshow avisynth can be made unaware.
It is how you use threads.
And setmtmode() in my experience needs distribute() to enable mt in ffdshow avisynth.

Leak

26th March 2009, 17:22

I just read on the C++ Faq Lite that for that to work I need to do "delete[] buffers" instead of the current "delete buffers".
Yeah, I forgot about that (or rather, I blame too much C# coding) - and with the existing TframeBuffers there was no need for a destructor, so you could just throw out the whole buffers array...

e-t172

26th March 2009, 17:54

There is some problems with the code I wrote:

curBuffer.frame = PVideoFrame(self->input->env, self->input->env->avs_new_video_frame_a(*(self->input->env), &vi, 16));

Using a debugger I noticed that the references counters in the AVS_VideoFrame (refcount and vfb->refcount) are completely wrong (both 0). Then when I call GetWritePtr() it returns NULL because of the refcounts and of course it crashes miserably.

However if I do this:

PVideoFrame* test = new PVideoFrame(self->input->env, self->input->env->avs_new_video_frame_a(*(self->input->env), &vi, 16));

It works, the refcounts in "test" are both 1, as they should be, and GetWritePtr() works. Conclusion: if I don't use "new", the reference counters are wrong. Any idea why?

EDIT: I noticed using a debugger that the destructor of curBuffer gets called just after "curBuffer.frame = PVideoFrame(...)", decrementing the reference counters and thus causing the problem. Why is the destructor called? It doesn't make sense to me.

Leak

26th March 2009, 18:37

It works, the refcounts in "test" are both 1, as they should be, and GetWritePtr() works. Conclusion: if I don't use "new", the reference counters are wrong. Any idea why?
Because the PVideoFrame here sadly isn't the smart pointer one from AviSynth.h but the very basic one from Tavisynth.h, which basically is just a structure to contain a AVS_VideoFrame and a IScriptEnvironment pointer. I'm afraid I just left that as it was from Milan's original code.

It's probably better to skip the PVideoFrame entirely and just call env->avs_new_video_frame_a to create a new frame, store it's resulting AVS_VideoFrame* directly in the TframeBuffer and then call env->avs_release_video_frame on that pointer in the destructor of TframeBuffer. You're not using the AVS_VideoFrame anywhere else, so there's really no need for any smart pointering.

What happened in your case was your PVideoFrame(...) call creating a temporary variable that was assigned to curBuffer.frame and immediately destroyed, thus decrementing the reference count. If you use new that of course doesn't happen, but I really think the above is the better approach.

EDIT: Don't mess with the PVideoFrame in Tavisynth.h though, as it's also used in TvideoCodecAvisynth... take a look at Tavisynth.h line 184 onward, that's the wrapper functions used - just call the wrapped C functions directly as each is only used once so the wrapper isn't exactly neccessary...

np: Lawrence - Rabbit Tube (DJ Koze Remix) (DJ Koze - Reincarnations)

e-t172

26th March 2009, 18:44

What happened in your case was your PVideoFrame(...) call creating a temporary variable that was assigned to curBuffer.frame and immediately destroyed

Oh. Well, I learned something today :)

I will follow your advice and use AVS_VideoFrames directly.

e-t172

27th March 2009, 13:27

Okay, I have a working patch. Here it is: http://www.e-t172.net/files/ffdshow-tryout-avisynth-patch-01.patch

It should work flawlessly when buffering is disabled and "Ignore pulldown" selected. Pulldown has been disabled in the patch (it's a big piece of code, I will "convert" it later). Buffering does not work properly (frame ordering is completely wrong, try it yourself).

So here is the TODO list:
- Fix buffering
- Re-enable pulldown
- Clean the code

I won't have much time to work on it this week-end, so don't expect improvements before at least Sunday evening.

Leak

27th March 2009, 15:08

Okay, I have a working patch. Here it is: http://www.e-t172.net/files/ffdshow-tryout-avisynth-patch-01.patch
I'll take a look at it this weekend.

I hope you don't mind if I fix some of the missing things while I'm at it... :)

np: The Fireman - Dance 'til We're High (Electric Arguments)

Leak

29th March 2009, 20:30

e-t172

30th March 2009, 12:00

Okay, so I took a deeper look at your patch, but then decided to implement it myself mostly by improving on TframeBuffer step-by-step...

A work-in-progress patch can be found here (http://leak.no-ip.org/Stuff/AviSynth_Internal_Buffering_1.diff); it does buffering correctly but it's also missing pulldown reconstruction (I've just commented that part out, so trying to use it will probably result in spectacular failure), I'll do that tomorrow.

I applied your patch. Results:

ffdshow-tryout r2737
Both versions (original and patched) compiled using Visual C++ 2008 Express
Results are averaged FPS on 4000 frames
"Real world script" is: IVTC using Uncomb() and Decimate(3), buffering 0/5.
Patch used: http://leak.no-ip.org/Stuff/AviSynth_Internal_Buffering_1.diff
CPU: Core 2 Duo E6600 @ 3769 Mhz
Source: Gossip Girl S02E01, The CW HDTV Capture, 1080i30, MPEG-2

ORIGINAL PATCHED
WITHOUT AVISYNTH 152.07 149.70
WITH AVISYNTH, BUFFERING DISABLED 101.71 102.36
WITH AVISYNTH, BUFFERING 8/8 81.17 100.03
WITH AVISYNTH, REAL WORLD SCRIPT 40.37 45.77

It works :)