|
|
| Xzyx987X |
| Posted: Apr 23 2003, 03:59 AM |
 |
|
Unregistered
|
Here is the code I have so far for a horizontal stabilizer filter I'm working on to fix wavyness in old vhs tapes:
int tutorialRunProc(const FilterActivation *fa, const FilterFunctions *ff) {
PixDim w, h;
Pixel32 *src, *dst;
int i;
src = (Pixel32 *)fa->src.data;
dst = (Pixel32 *)fa->dst.data;
h = fa->src.h;
do {
w = fa->src.w;
Pixel32 RowSourceArray [MAX_PATH];
Pixel32 RowDestArray [MAX_PATH];
int PixelOffset;
for (i = 0; i <= w; i++) {
RowSourceArray [i] = *src++;
}
for (i = 0; i <= w; i++) {
if ((RowSourceArray [i] | 0x00FF00) < 0x002000) //Temp constant until GUI is implemented
{
PixelOffset++;
}
else {break;}
}
for (i = 0; (i + PixelOffset) <= w; i++) {
RowDestArray [i] = RowSourceArray [i + PixelOffset];
}
for (PixelOffset; PixelOffset > 1; PixelOffset--) {
RowDestArray [i] = 0x000000; //fill left over pixels in with black
}
for (i = 0; i <= w; i++) {
*dst++ = RowDestArray [i];
}
src = (Pixel32 *)((char *)src + fa->src.modulo);
dst = (Pixel32 *)((char *)dst + fa->dst.modulo);
} while(--h);
return 0;
}
It compiles with no problems but crashed virtual dub when executed. Admittedly I'm new to C++ and the tutorial for the filter was kinda confusing so I just muddled my way through writing some simplistic code. I'm guessing the error is from incompatible data types (i <= w) but I'm not sure how I could write this so it would work. If you know how to do this or can suggest some speed optimizations for the code I'd apreciate it. Here is the Vdub crash report if it helps:
An out-of-bounds memory access (access violation) occurred in module 'stabalizer' while running filter "Horizontal Stabalizer" (FilterSystem.cpp:516).
This is the simple version, tell me if you need the full report. |
|
| phaeron |
| Posted: Apr 23 2003, 06:29 AM |
|
|
Virtualdub Developer
Group: Administrator
Posts: 7773
Member No.: 61
Joined: 30-July 02
|
You're overrunning your local arrays. MAX_PATH should only be used for buffers that hold filenames, as it's defined to be 260. You need to allocate your arrays on the fly, according to the size of the bitmap you're processing. One way to do this is with the STL vector class:
| CODE |
#include <vector>
...
std::vector<Pixel32> RowSourceArray(fa->src.w);
|
I'll freely admit the tutorial is confusing, and VirtualDub filters can be cleaned up with some C++ sugar. I just haven't written that part yet....
|
|
| Xzyx987X |
| Posted: Apr 23 2003, 04:40 PM |
|
|
Unregistered
|
I implemented your changes but it still crashes with the same error as before (though it's in a different section of the code then before if you look at the dissasembly.) I even found a part of the code that just plain didn't make sense, fixed it and it still didn't work. Anyway here is the code now:
int stabilizerRunProc(const FilterActivation *fa, const FilterFunctions *ff) {
PixDim w, h;
Pixel32 *src, *dst;
src = (Pixel32 *)fa->src.data;
dst = (Pixel32 *)fa->dst.data;
int i;
h = fa->src.h;
do {
w = fa->src.w;
std::vector<Pixel32> RowSourceArray(fa->src.w);
std::vector<Pixel32> RowDestArray(fa->src.w);
int PixelOffset;
for (i = 0; i <= w; i++) {
RowSourceArray [i] = *src++;
}
for (i = 0; i <= w; i++) {
if ((RowSourceArray [i] | 0x00FF00) < 0x002000)
{
PixelOffset++;
}
else {break;}
}
for (i = 0; (i + PixelOffset) <= w; i++) {
RowDestArray [i] = RowSourceArray [i + PixelOffset];
}
for (i; i <= w; i++) {
RowDestArray [i] = 0x000000;
}
for (i = 0; i <= w; i++) {
*dst++ = RowDestArray [i];
}
src = (Pixel32 *)((char *)src + fa->src.modulo);
dst = (Pixel32 *)((char *)dst + fa->dst.modulo);
} while(--h);
return 0;
}
Incedentally, should i be initialized to 1 in this case? I tried it that way but even though it didn't fix the current bug that doesn't mean it won't be a problem later. |
|
| Kippesoep |
| Posted: Apr 23 2003, 04:58 PM |
|
|
Moderator of the Virtualdub support forum
Group: Moderators
Posts: 447
Member No.: 441
Joined: 6-October 02
|
| QUOTE (Xzyx987X @ Apr 23 2003, 05:40 PM) | | CODE | | for (i = 0; i <= w; i++) |
Incedentally, should i be initialized to 1 in this case? I tried it that way but even though it didn't fix the current bug that doesn't mean it won't be a problem later. |
You do have an off-by-one error. Use this instead:
| CODE | | for (i = 0; i < w; ++i) |
Note the use of < instead of <= !!! (The ++i instead of i++ is a semantic thing that doesn't change the functionality of the code). |
|
| Xzyx987X |
| Posted: Apr 23 2003, 06:28 PM |
|
|
Unregistered
|
I changed it but it still didn't fix anything. I tried running the debugger on the prog (probobly should've done that in the first place) and here is where it found the error:
for (i = 0; i < (w - PixelOffset); i++) { //Changed this line cause it made no sense before (I gotta quit writing code at 1 in the morning...)
RowDestArray [i] = RowSourceArray [i + PixelOffset]; //<-- Here is bug
}
Tried everything I could think of to fix it but nothing worked so does anyone here know what I could do? One thing I should add, the value of PixelOffset was eing returned as -858993460 by the debugger for some reason. |
|
| fccHandler |
| Posted: Apr 23 2003, 06:32 PM |
|
|
Administrator n00b
Group: Moderators
Posts: 3961
Member No.: 280
Joined: 13-September 02
|
You've forgotten to initialize the PixelOffset value.
Replace "int PixelOffset;" with "int PixelOffset = 0;"
EDIT: (hehe, you found the bug while I was typing)
--------------------
May the FOURCC be with you... |
|
| Xzyx987X |
| Posted: Apr 23 2003, 07:01 PM |
|
|
Unregistered
|
*Yay* it didn't crash! thanx for the help. |
|
| Xzyx987X |
| Posted: Apr 23 2003, 07:19 PM |
|
|
Unregistered
|
Cool, after putting a & were there was an | the filer is having the desired result. I'll have to implement some sort of detection for it to work on frames with a black backgroung but after that this should prove to be a very useful filter. |
|
| fccHandler |
| Posted: Apr 23 2003, 07:47 PM |
|
|
Administrator n00b
Group: Moderators
Posts: 3961
Member No.: 280
Joined: 13-September 02
|
A little optimizing wouldn't hurt either. For example, it strikes me that RowSourceArray[] and RowDestArray[] are kinda redundant in the original code; why not access src[] and dst[] directly?
(hint hint)
 
--------------------
May the FOURCC be with you... |
|
| Xzyx987X |
| Posted: Apr 23 2003, 07:55 PM |
|
|
Unregistered
|
Yea, the only reason I did it the way I did is I was kinda expecting vdub to supply the source in array format to start out with, so when it didn't I loaded the information into arrays myself to try and ditch any variable handling bugs my lack of experience would create (not that I still didn't make some  ) |
|
