Passing PVideoFrame to Functions

[MysteryX]

I remember in the past I tried passing PVideoFrame to functions and it caused problems, but I never understood why. Also other codes generally call GetWritePtr/GetRowSize/GetPitch/GetHeight and pass those to functions.

I'm trying it again and the code isn't working as expected. Could someone enlighten me on this issue?

I'm porting Deblock from VapourSynth to Avisynth with high-bit-depth support as an exercise to understand VapourSynth code.

Here's the function that needs the frame reference.
https://github.com/mysteryx93/Avisynth-Deblock/blob/master/Src/deblock.cpp#L197

The problem with instead taking 4 arguments is that I have 3 planes, and it would requires a LOT of parameters to pass the information of all 3 planes -- does it have to be done that way?

Either way I'd like to understand why passing PVideoFrame doesn't work. If I call MakeWriteable in GetFrame, the frame isn't writeable within the function so I have to call it there before calling GetWritePtr.

Current code doesn't crash, but the output is exactly the same as the input.

[pinterf]

I remember in the past I tried passing PVideoFrame to functions and it caused problems, but I never understood why. Also other codes generally call GetWritePtr/GetRowSize/GetPitch/GetHeight and pass those to functions.

I'm trying it again and the code isn't working as expected. Could someone enlighten me on this issue?

I'm porting Deblock from VapourSynth to Avisynth with high-bit-depth support as an exercise to understand VapourSynth code.

Here's the function that needs the frame reference.
https://github.com/mysteryx93/Deblock/blob/master/Deblock/deblock.cpp#L197

Either way I'd like to understand why passing PVideoFrame doesn't work. If I call MakeWriteable in GetFrame, the frame isn't writeable within the function so I have to call it there before calling GetWritePtr.

Current code doesn't crash, but the output is exactly the same as the input.
Have you tried PVideoFrame &dst instead of PVideoFrame dst?

[TheFluff]

I remember in the past I tried passing PVideoFrame to functions and it caused problems, but I never understood why. Also other codes generally call GetWritePtr/GetRowSize/GetPitch/GetHeight and pass those to functions.

I'm trying it again and the code isn't working as expected. Could someone enlighten me on this issue?

I'm porting Deblock from VapourSynth to Avisynth with high-bit-depth support as an exercise to understand VapourSynth code.

Here's the function that needs the frame reference.
https://github.com/mysteryx93/Deblock/blob/master/Deblock/deblock.cpp#L197

The problem with instead taking 4 arguments is that I have 3 planes, and it would requires a LOT of parameters to pass the information of all 3 planes -- does it have to be done that way?

Either way I'd like to understand why passing PVideoFrame doesn't work. If I call MakeWriteable in GetFrame, the frame isn't writeable within the function so I have to call it there before calling GetWritePtr.

Current code doesn't crash, but the output is exactly the same as the input.

Are you aware of the distinction between pass-by-value, pass-by-pointer and pass-by-reference? If not, you should probably read up on it, it's kinda sorta important. The very short version though is that directly passing an object instance as an argument to a function in C++, like you do in the code you linked, then you pass it by value. This will invoke the PVideoFrame copy constructor (or potentially move constructor in C++11 or later I think?) and you'll get a new copy of the object inside the function you called. Not only is this rather inefficient, it's probably not what you want. You want to pass a reference to the original object. pinterf told you how to do that.

[MysteryX]

Have you tried PVideoFrame &dst instead of PVideoFrame dst?
Ah, I thought PVideoFrame was already a pointer by itself.

I'll try that then.

[amichaelt]

Ah, I thought PVideoFrame was already a pointer by itself.

I'll try that then.

PVideoFrame is a class not a typedef to a pointer. Going to the declaration in avisynth.h would have shown you this.

[MysteryX]

Alright it works.

Deblock now works in 8-bit and in 32-bit.

16-bit however returns this.
https://s22.postimg.cc/s8jw3xe0t/Deblock.png (https://postimg.cc/image/s8jw3xe0t/)

Anyone sees what I'm missing?
https://github.com/mysteryx93/Deblock/blob/master/Deblock/deblock.cpp

Curiously 16-bit works on a Y8 clip, but not on a YV420 clip.

[wonkey_monkey]

I always thought P stood for pointer...

[MysteryX]

I always thought P stood for pointer...
Ya, right?

Now where did that idea come from?

[Wilbert]

PVideoFrame is a smart pointer to VideoFrame (just like PClip is a smart pointer to Clip): http://avisynth.nl/index.php/Filter_SDK/Cplusplus_API#PVideoFrame. The difference with a regular pointer is that you don't need to explicitly destroy the object VideoFrame it points to, among other things.

See http://ootips.org/yonat/4dev/smart-pointers.html or https://en.wikipedia.org/wiki/Smart_pointer

[amichaelt]

I always thought P stood for pointer...

And it does in this case as well. It's a smart pointer class that wraps a raw VideoFrame pointer but is not itself a raw pointer. That's why it doesn't act like a raw pointer.

// smart pointer to VideoFrame
class PVideoFrame {

VideoFrame* p;

void Init(VideoFrame* x);
void Set(VideoFrame* x);

public:
PVideoFrame() AVS_BakedCode( AVS_LinkCall_Void(PVideoFrame_CONSTRUCTOR0)() )
PVideoFrame(const PVideoFrame& x) AVS_BakedCode( AVS_LinkCall_Void(PVideoFrame_CONSTRUCTOR1)(x) )
PVideoFrame(VideoFrame* x) AVS_BakedCode( AVS_LinkCall_Void(PVideoFrame_CONSTRUCTOR2)(x) )
void operator=(VideoFrame* x) AVS_BakedCode( AVS_LinkCall_Void(PVideoFrame_OPERATOR_ASSIGN0)(x) )
void operator=(const PVideoFrame& x) AVS_BakedCode( AVS_LinkCall_Void(PVideoFrame_OPERATOR_ASSIGN1)(x) )

VideoFrame* operator->() const { return p; }

// for conditional expressions
operator void*() const { return p; }
bool operator!() const { return !p; }

~PVideoFrame() AVS_BakedCode( AVS_LinkCall_Void(PVideoFrame_DESTRUCTOR)() )
#ifdef BUILDING_AVSCORE
public:
void CONSTRUCTOR0(); /* Damn compiler won't allow taking the address of reserved constructs, make a dummy interlude */
void CONSTRUCTOR1(const PVideoFrame& x);
void CONSTRUCTOR2(VideoFrame* x);
void OPERATOR_ASSIGN0(VideoFrame* x);
void OPERATOR_ASSIGN1(const PVideoFrame& x);
void DESTRUCTOR();
#endif
}; // end class PVideoFrame

[foxyshadis]

Alright it works.

Deblock now works in 8-bit and in 32-bit.

16-bit however returns this.
https://s22.postimg.cc/s8jw3xe0t/Deblock.png (https://postimg.cc/image/s8jw3xe0t/)

Anyone sees what I'm missing?
https://github.com/mysteryx93/Deblock/blob/master/Deblock/deblock.cpp

Curiously 16-bit works on a Y8 clip, but not on a YV420 clip.

Your Deblock Repo is private; I only get a 404 from the link.

[DJATOM]

It's just renamed: https://github.com/mysteryx93/Avisynth-Deblock

[`Orum]

Not sure (yet) why you're getting the weird chroma issues, but at the very least you might want to address lines like these (https://github.com/mysteryx93/Avisynth-Deblock/blob/master/Src/deblock.cpp#L112-L114). The issue is it looks like you're using the preprocessor macros for min()/max() (from Windows.h), which are subject to the double-evaluation issue (though in your case you will get the right answer, it will be slower). There are several ways to work around this but the simplest for C++ is probably detailed here (https://stackoverflow.com/questions/20661836/implementing-min-and-max-in-clang-without-double-evaluation#20661901), or just use the STL equivalent (http://en.cppreference.com/w/cpp/algorithm/max).

Edit: If your compiler had C++17 support there's also std::clamp() (http://en.cppreference.com/w/cpp/algorithm/clamp), or you can always make your own template.

[MysteryX]

It's working now. I was simply checking BitsPerPixel instead of BitsPerComponent in the initializer which caused the initial data to remain at 0 in certain cases.

As for min/max, #define NOMINMAX is there for that. It's using min and max functions defined in Avisynth.

[`Orum]

Ah okay, I missed that. Looking breifly at the code, this doesn't appear to be one of the fancier deblockers that varies the strength of deblocking on the amount of blocking detected. Any chance we could see that in a later release? Though, to be honest, I haven't needed a deblocking filter for anything encoded in the last decade thanks to in-loop deblocking.

[SaurusX]

Ah okay, I missed that. Looking breifly at the code, this doesn't appear to be one of the fancier deblockers that varies the strength of deblocking on the amount of blocking detected. Any chance we could see that in a later release? Though, to be honest, I haven't needed a deblocking filter for anything encoded in the last decade thanks to in-loop deblocking.

There's a lot of old DVD video out there that definitely needs deblocking.