Welcome to Doom9's Forum, THE in-place to be for everyone interested in DVD conversion.

Before you start posting please read the forum rules. By posting to this forum you agree to abide by the rules.

 

Go Back   Doom9's Forum > Capturing and Editing Video > VapourSynth

Reply
 
Thread Tools Search this Thread Display Modes
Old 16th December 2016, 12:19   #21  |  Link
Myrsloik
Professional Code Monkey
 
Myrsloik's Avatar
 
Join Date: Jun 2003
Location: Kinnarps Chair
Posts: 2,548
Quote:
Originally Posted by xekon View Post
So there are no compiler errors, but VSedit crashes without any error messages when I try to use the filter.

how can I get VSedit to report the reason it crashed? to get more details, it has something to do with this filter, but i have no idea what without some kind of feedback.

...
Run it in a debugger then.
__________________
VapourSynth - proving that scripting languages and video processing isn't dead yet
Myrsloik is offline   Reply With Quote
Old 16th December 2016, 13:41   #22  |  Link
shekh
Registered User
 
Join Date: Mar 2015
Posts: 775
Some hints assuming you use Visual Studio
build just the plugin with debug info and with no optimizations
set breakpoints to all major functions
specify launch options for debugging (application and command line)
__________________
VirtualDub2
shekh is offline   Reply With Quote
Old 16th December 2016, 14:21   #23  |  Link
feisty2
I'm Siri
 
feisty2's Avatar
 
Join Date: Oct 2012
Location: void
Posts: 2,633
Code:
	rgblab = new long[16777216];
	labrgb = new long[16777216];
and no delete[] for both arrays, basic memory leak..
use smart pointers if you want some kind of automatic GC for C++ like in Java or C#

and both arrays do not feature a runtime determined length, simply make them static if worrying about blowing the stack up
Code:
	static int32_t rgblab[16777216];
	static int32_t labrgb[16777216];
feisty2 is offline   Reply With Quote
Old 16th December 2016, 14:58   #24  |  Link
feisty2
I'm Siri
 
feisty2's Avatar
 
Join Date: Oct 2012
Location: void
Posts: 2,633
or a bit more organized with C++ 14..
Code:
#include <cstdint>

auto operator""ElementsLUTTable(size_t length) {
	struct LUTTable final {
		int32_t *RGBToLab = nullptr;
		int32_t *LabToRGB = nullptr;
		LUTTable(size_t length = 0) {
			RGBToLab = new int32_t[length];
			LabToRGB = new int32_t[length];
		}
		LUTTable(const LUTTable &) = delete;
		LUTTable(LUTTable &&obj) {
			RGBToLab = obj.RGBToLab;
			LabToRGB = obj.LabToRGB;
			obj.RGBToLab = nullptr;
			obj.LabToRGB = nullptr;
		}
		auto &operator=(const LUTTable &) = delete;
		auto &operator=(LUTTable &&obj) {
			if (this != &obj) {
				RGBToLab = obj.RGBToLab;
				LabToRGB = obj.LabToRGB;
				obj.RGBToLab = nullptr;
				obj.LabToRGB = nullptr;
			}
			return *this;
		}
		~LUTTable() {
			delete[] RGBToLab;
			delete[] LabToRGB;
		}
	};
	return LUTTable(length);
}

auto table = 16777216ElementsLUTTable;

Last edited by feisty2; 16th December 2016 at 15:00.
feisty2 is offline   Reply With Quote
Old 16th December 2016, 17:03   #25  |  Link
Mystery Keeper
Beyond Kawaii
 
Mystery Keeper's Avatar
 
Join Date: Feb 2008
Location: Russia
Posts: 724
Quote:
Originally Posted by feisty2 View Post
or a bit more organized with C++ 14..
Code:
#include <cstdint>

auto operator""ElementsLUTTable(size_t length) {
	struct LUTTable final {
		int32_t *RGBToLab = nullptr;
		int32_t *LabToRGB = nullptr;
		LUTTable(size_t length = 0) {
			RGBToLab = new int32_t[length];
			LabToRGB = new int32_t[length];
		}
		LUTTable(const LUTTable &) = delete;
		LUTTable(LUTTable &&obj) {
			RGBToLab = obj.RGBToLab;
			LabToRGB = obj.LabToRGB;
			obj.RGBToLab = nullptr;
			obj.LabToRGB = nullptr;
		}
		auto &operator=(const LUTTable &) = delete;
		auto &operator=(LUTTable &&obj) {
			if (this != &obj) {
				RGBToLab = obj.RGBToLab;
				LabToRGB = obj.LabToRGB;
				obj.RGBToLab = nullptr;
				obj.LabToRGB = nullptr;
			}
			return *this;
		}
		~LUTTable() {
			delete[] RGBToLab;
			delete[] LabToRGB;
		}
	};
	return LUTTable(length);
}

auto table = 16777216ElementsLUTTable;
One component of good coding is explicit intent. Your example is obscure. Does it even work? Isn't the structure declared in the scope of operator? Or does "auto" pull the declaration outside?
__________________
...desu!
Mystery Keeper is offline   Reply With Quote
Old 16th December 2016, 17:18   #26  |  Link
shekh
Registered User
 
Join Date: Mar 2015
Posts: 775
There was nothing wrong about labrgb tables except missing delete/wrong singleton. God idea to fix just that issue. Tables must not be static (they take space).
__________________
VirtualDub2
shekh is offline   Reply With Quote
Old 16th December 2016, 17:39   #27  |  Link
feisty2
I'm Siri
 
feisty2's Avatar
 
Join Date: Oct 2012
Location: void
Posts: 2,633
Quote:
Originally Posted by Mystery Keeper View Post
One component of good coding is explicit intent. Your example is obscure. Does it even work? Isn't the structure declared in the scope of operator? Or does "auto" pull the declaration outside?
it works, test code, compiled and executed smoothly under vs2015
Code:
#include <cstdint>
#include <cstdlib>
#include <iostream>

auto operator""ElementsLUTTable(size_t length) {
	struct LUTTable final {
		int32_t *RGBToLab = nullptr;
		int32_t *LabToRGB = nullptr;
		LUTTable(size_t length = 0) {
			RGBToLab = new int32_t[length];
			LabToRGB = new int32_t[length];
		}
		LUTTable(const LUTTable &) = delete;
		LUTTable(LUTTable &&obj) {
			RGBToLab = obj.RGBToLab;
			LabToRGB = obj.LabToRGB;
			obj.RGBToLab = nullptr;
			obj.LabToRGB = nullptr;
		}
		auto &operator=(const LUTTable &) = delete;
		auto &operator=(LUTTable &&obj) {
			if (this != &obj) {
				RGBToLab = obj.RGBToLab;
				LabToRGB = obj.LabToRGB;
				obj.RGBToLab = nullptr;
				obj.LabToRGB = nullptr;
			}
			return *this;
		}
		~LUTTable() {
			delete[] RGBToLab;
			delete[] LabToRGB;
		}
	};
	return LUTTable(length);
}

auto table = 16777216ElementsLUTTable;

auto main()->int {
	table.LabToRGB[123] = 123456;
	std::cout << table.LabToRGB[123] << std::endl;
	system("pause");
}
Quote:
One component of good coding is explicit intent.
that's exactly what I'm trying to do.
Code:
auto value = 1.234;
auto val = 123;
such statements are legal for all basic (hardware supported) data types, but wouldn't work on user defined data types before C++11.

user defined literals (operator""xxx) have been introduced to C++ since C++11, which makes such stuff
Code:
auto cval = 3 + 1.123i; //operator""i
auto table = 16777216ElementsLUTTable; //operator""ElementsLUTTable
possible, and now you could make all your custom data types just as native as the basic data types long as you want to.
Quote:
Does it even work? Isn't the structure declared in the scope of operator? Or does "auto" pull the declaration outside?
it works because the move constructor (since C++11) has been defined.

Last edited by feisty2; 16th December 2016 at 17:41.
feisty2 is offline   Reply With Quote
Old 16th December 2016, 17:45   #28  |  Link
captaiŋadamo
Guest
 
Posts: n/a
Code:
LUTTable(LUTTable &&obj) {
	RGBToLab = obj.RGBToLab;
	LabToRGB = obj.LabToRGB;
	obj.RGBToLab = nullptr;
	obj.LabToRGB = nullptr;
}
Code:
auto &operator=(LUTTable &&obj) {
	if (this != &obj) {
		RGBToLab = obj.RGBToLab;
		LabToRGB = obj.LabToRGB;
		obj.RGBToLab = nullptr;
		obj.LabToRGB = nullptr;
	}
	return *this;
}
So a C idiom is bad coding style, but setting data within an object to null within a copy constructor is good code? lolwut? A copy constructor is only meant to clone an object, it's not meant to modify the original object. Adding side-effects to a copy constructor is horrendous.
  Reply With Quote
Old 16th December 2016, 17:51   #29  |  Link
feisty2
I'm Siri
 
feisty2's Avatar
 
Join Date: Oct 2012
Location: void
Posts: 2,633
Quote:
Originally Posted by captaiŋadamo View Post
Code:
LUTTable(LUTTable &&obj) {
	RGBToLab = obj.RGBToLab;
	LabToRGB = obj.LabToRGB;
	obj.RGBToLab = nullptr;
	obj.LabToRGB = nullptr;
}
Code:
auto &operator=(LUTTable &&obj) {
	if (this != &obj) {
		RGBToLab = obj.RGBToLab;
		LabToRGB = obj.LabToRGB;
		obj.RGBToLab = nullptr;
		obj.LabToRGB = nullptr;
	}
	return *this;
}
So a C idiom is bad coding style, but setting data within an object to null within a copy constructor is good code? lolwut? A copy constructor is only meant to clone an object, it's not meant to modify the original object. Adding side-effects to a copy constructor is horrendous.
Code:
LUTTable(const LUTTable &) // copy constructor
LUTTable(LUTTable &&) // MOVE constructor (since C++11)
https://msdn.microsoft.com/en-us/library/dd293665.aspx
feisty2 is offline   Reply With Quote
Old 16th December 2016, 18:12   #30  |  Link
feisty2
I'm Siri
 
feisty2's Avatar
 
Join Date: Oct 2012
Location: void
Posts: 2,633
and @captaiŋadamo
it's almost 2017 now, not 1997, time to shake the obsolete ideas off, C++ has always been EVOLVING! (C++17 is just around the corner )
feisty2 is offline   Reply With Quote
Old 17th December 2016, 02:29   #31  |  Link
xekon
Registered User
 
Join Date: Jul 2011
Posts: 224
Quote:
Originally Posted by shekh View Post
Some hints assuming you use Visual Studio
build just the plugin with debug info and with no optimizations
set breakpoints to all major functions
specify launch options for debugging (application and command line)
I am using Visual Studio 2015, when I try to run it in debug mode it says that it is not a valid Win32 application, which is because there is no MAIN function, since its library.
I could add a main function, but I would assume that I would also have to pass data to the functions like it would when the library is in use, otherwise how could I find the problem.

when I read about debugging dll libraries on microsoft website they suggest debugging the host application.
So I am currently installing Qt5, and trying to get everything in place to debug VSedit while its using the GradCurve plugin.
I just hope I am barking up the right tree here, and not off in left field.

EDIT:
I updated the github https://github.com/jieiku/GradCurve/
Removed variables left behind from vdub specific stuff as well as variables from starting off with HolyWu's AddGrain filter.
There is more cleanup to do once I have this working.

Last edited by xekon; 3rd December 2021 at 07:32.
xekon is offline   Reply With Quote
Old 17th December 2016, 02:35   #32  |  Link
feisty2
I'm Siri
 
feisty2's Avatar
 
Join Date: Oct 2012
Location: void
Posts: 2,633
Fix that memory leak and ur done.
feisty2 is offline   Reply With Quote
Old 17th December 2016, 02:36   #33  |  Link
shekh
Registered User
 
Join Date: Mar 2015
Posts: 775
You need to start the host application but you dont have to compile and debug it, the debugger kicks in once your dll is loaded.
__________________
VirtualDub2
shekh is offline   Reply With Quote
Old 17th December 2016, 02:36   #34  |  Link
xekon
Registered User
 
Join Date: Jul 2011
Posts: 224
OH! I didn't realize that was the only problem! I will try your implementation feisty2! thank you!
xekon is offline   Reply With Quote
Old 17th December 2016, 02:38   #35  |  Link
shekh
Registered User
 
Join Date: Mar 2015
Posts: 775
Btw, concerning lab mode
AFAIK is was both slow and low quality, maybe just worth removing anyway.
__________________
VirtualDub2
shekh is offline   Reply With Quote
Old 17th December 2016, 02:39   #36  |  Link
feisty2
I'm Siri
 
feisty2's Avatar
 
Join Date: Oct 2012
Location: void
Posts: 2,633
Plus it's "delete[]" for arrays...
not sure if "delete array" will still lead to memory leak (only the first element got deleted)

Edit: just googled about that and "delete" works in your case also, cuz both arrays are an array of the basic data type (no destructor). But delete[] makes more sense, so use that instead

Edit2: I'm talking about line 673 and line 674 in ur source code

Last edited by feisty2; 17th December 2016 at 03:02.
feisty2 is offline   Reply With Quote
Old 17th December 2016, 03:09   #37  |  Link
xekon
Registered User
 
Join Date: Jul 2011
Posts: 224
Quote:
Originally Posted by feisty2 View Post
Plus it's "delete[]" for arrays...
not sure if "delete array" will still lead to memory leak (only the first element got deleted)

Edit: just googled about that and "delete" works in your case also, cuz both arrays are an array of the basic data type (no destructor). But delete[] makes more sense, so use that instead

Edit2: I'm talking about line 673 and line 674 in ur source code
Thank you everyone!

and thank you feisty2, i switched it to using brackets.

I assumed there were problems other than the memory leak, because when I build under linux vsedit still crashes after correcting the memory leaks, im building a dll under windows and going to try vsedit under windows to see if I still crash.

it sounds like the plugin is functioning for you guys, so something must be up with my build environment.
xekon is offline   Reply With Quote
Old 17th December 2016, 06:40   #38  |  Link
feisty2
I'm Siri
 
feisty2's Avatar
 
Join Date: Oct 2012
Location: void
Posts: 2,633
since you PMed me, some debug info from vs2015




no offense but, ur code is way too cryptic and pretty much machine code to me..
feisty2 is offline   Reply With Quote
Old 17th December 2016, 06:52   #39  |  Link
xekon
Registered User
 
Join Date: Jul 2011
Posts: 224
ah, thanks for taking a look for me. the second picture you posted is where my vsedit crashed I believe. I will keep trying to figure out the reason its crashing. very little of the code was actually written by me, I migrated the code from the vdub c++ filter. I will probably need to get a better idea of how gradation curves is supposed to work before i can fix the problem.

Last edited by xekon; 17th December 2016 at 06:59.
xekon is offline   Reply With Quote
Old 17th December 2016, 16:07   #40  |  Link
jackoneill
unsigned int
 
jackoneill's Avatar
 
Join Date: Oct 2012
Location: 🇪🇺
Posts: 760
Is d.filename initialised with zeroes? If not, strcat may find a zero past its end. Use the "n" versions of string functions if possible, i.e. strncat. Although I don't see why this isn't str(n)cpy. And since you're using C++ you might as well use std::string.

Are you sure you want those global variables in your library?

As far as I can tell, you could call ImportCurve and PreCalcLut from GradCurveCreate. They don't seem to use any data from the filter's input frames. It also looks like oldr, oldg, oldb, medr, medg, and medb can be local variables. Then you can switch to fmParallel.

Your filter accepts integer formats, but it assumes the frames always contain float pixels. If you test your filter with integer clips, the processing loop will inevitably read and write past the end of the frame.
__________________
Buy me a "coffee" and/or hire me to write code!

Last edited by jackoneill; 17th December 2016 at 16:14.
jackoneill is offline   Reply With Quote
Reply

Tags
color correct, curves, gradation, vapoursynth, virtualdub

Thread Tools Search this Thread
Search this Thread:

Advanced Search
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump


All times are GMT +1. The time now is 23:24.


Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2024, vBulletin Solutions Inc.