MeGUI development

[Inc]

Originally Posted by dimzon

0.2.3.2053 26 Jan 2006
Commit by dimzon
	-AviSynth wrapper now returns information about source colorspace && audio sampletype

do you mean the dll itself? That was already given ....

typedef struct AVSDLLVideoInfo {

	// Video

 	int width;
	int height;
	int raten;
	int rated;
	int aspectn;
	int aspectd;
	int interlaced_frame;
	int top_field_first;
	int num_frames;

	int pixel_type;        // <-----------------

	// Audio
	int audio_samples_per_second;
	int sample_type;     // <-----------------
	int nchannels;

	int num_audio_frames;

	int64_t num_audio_samples;
} AVSDLLVideoInfo;

[dimzon]

Originally Posted by incredible

do you mean the dll itself? That was already given ....

I mean my avisynthwrapper.dll now provide both before/after conversion. So you can open script forcing RGB24 but will obtain information about original colorspace

[Dayvon]

Not to spam your thread or anything, but I just wanted to thank you MeGUI dev's. I just got done backing-up/encoding the Band of Brothers series, and your utility worked perfectly. More no hassle rendering than I've done in a long time. Thanks for your hard work, and great GUI!!

Dave

[berrinam]

Originally Posted by Doom9

Your video source uses the {0} colorspace. Video encoding requires the YV12 colorspace. Add ConvertToYV12() at the end of your AviSynth script and try again.
{0} is the actual colorspace of the video.

What about asking 'Do you want me to fix this up by adding ConvertToYV12() to the end of the script'?

[Doom9]

well.. I had that part typed out, then I somehow moved away, and once again coming back my text was gone.. thus the shorter message.. yes, having a dialog which would offer the automatic adding would be good, but then we also need the re-opening of the source

[Inc]

Originally Posted by dimzon

I mean my avisynthwrapper.dll now provide both before/after conversion. So you can open script forcing RGB24 but will obtain information about original colorspace

? That was alreay done as inf.pixel_type was taken from the original frame before forcing the conversion to RGB/YUY2/YV12

vi->pixel_type = inf.pixel_type

And the outgoing Cspace is determined by the Cspace requested in the avs_getvframe(....,....,"RGB") command.


I do think we should merge the project AvsRedirect and AvsWrapper as its getting confusing. If you have questions about whats integrated and what could be done better, lets do it together.

[dimzon]

Originally Posted by berrinam

What about asking 'Do you want me to fix this up by adding ConvertToYV12() to the end of the script'?

How does you want to patch such script

blablabla
blablabla
blablabla
return last
blablabla
blablabla
blablabla
blablabla

I propose a little another method. Let's create additional avs @ same path:
filename = <originalFileName>.ConvertToYV12.avs

Import(<originalFileName>).ConvertToYV12()

we can do it automatically for every script (even without colorspace checking)

[dimzon]

Originally Posted by incredible

? That was alreay done as inf.pixel_type was taken from the original frame before forcing the conversion to RGB/YUY2/YV12

vi->pixel_type = inf.pixel_type

And the outgoing Cspace is determined by the Cspace requested in the avs_getvframe(....,....,"RGB") command.

avisynthWrapepr returns both - original && forced pixel type

Originally Posted by incredible

I do think we should merge the project AvsRedirect and AvsWrapper as its getting confusing. If you have questions about whats integrated and what could be done better, lets do it together.

No. I have branch it to avoid any backward-compatibility support. It must work only with complimentary AvisynthWrapper.cs. You can use it's source code to merge parts back to AVSRedirect throught...

[Inc]

No. ...
You can use it's source code to merge parts back to AVSRedirect throught...

The source code, and main add ons are from new_avsRedirect.dll So no need to merge things "back"

however ....

[berrinam]

Originally Posted by dimzon

How does you want to patch such script

blablabla
blablabla
blablabla
return last
blablabla
blablabla
blablabla
blablabla

I see that a script like that could be a problem, but who really does that? There's not any point in writing return last in a script. If everything is in variables, then what you would do is just write the line "video", or whatever your variable-name is. The only appropriate place for return is in AviSynth functions. Also, why would you have more text after you have returned the video?

I propose a little another method. Let's create additional avs @ same path:
filename = <originalFileName>.ConvertToYV12.avs

Import(<originalFileName>).ConvertToYV12()

Is this really necessary? In my local implementation, it just appends ConvertToYV12() to the end of the script. It then checks if the modified script is fine, and if not, it tells the user, who can choose whether to abort. I agree, your method is guaranteed to work and might be a more elegant solution because of this, but:
1. It creates an extra file to be deleted, and the unexpecting user may be surprised to find two avisynth files where there should only be one. Which one should he/she choose?
2. It requires more code restructuring to handle the fact that there is ANOTHER file.
3. As I said above, that situation should never really arise, and my implementation will abort gracefully in this situation: telling the user that appending converttoyv12 didn't work, and giving the user the option of aborting.

People who really know a lot about scripting will almost definitely not want the quick fix ConvertToYV12() at the end of the script, because, for most sources, it should have been YV12 in the first place, and so they will want to work out where the non-YV12 colorspace came from. I think it is only these people who will be writing the scripts with return last in them.

we can do it automatically for every script (even without colorspace checking)

I think that shouldn't be done, because:
1. It adds additional pointless filters. Why slow the encoding down (even though not by much).
2. It will hide problems that the experienced AviSynth user will want to know about. If there is a conversion to RGB, these people won't want it hidden by a ConvertToYV12 at the end, because colorspace conversions cause quality loss.

Never-the-less, none of these points actually give much reason against either of our implementations, so let's see what Doom9 says.

[dimzon]

0.2.3.2054 27 Jan 2006
Commit by dimzon
	-SourceDetector doesn't create temporaly AVS anymore

[dimzon]

Originally Posted by berrinam

I see that a script like that could be a problem, but who really does that?

I do
Actually I use

return last

instead of commenting/removing rest lines of script. Such trick (return last) is not a good programming style for regular code but avisynth scripts is mostly temporaly (just encode and forget) so i prerer this way bcz it is faster

[berrinam]

I still don't like this return last. As you said, it's not good programming style. Also, would you seriously just let MeGUI do your ConvertToYV12 wrapping? If you found out it isn't YV12, wouldn't you go: "Hang on a minute, what's up? My source should be YV12, so where's it been converted away from YV12? I better fix it up"

Ok, how about this: I'm just about to commit my version, which manages the "don't ask me again" dialogs as well as all the error messages. If Doom9 wants your way, then you just change the ConvertToYV12 section of the code. There are two places you should look at: JobUtil.prepareVideoJob, which calls the function VideoUtil.checkVideo(string)

[berrinam]

0.2.3.2055 27 Jan 2006
Commit by berrinam:
  -Added video input checking (video exists, has no errors, colorspace==yv12, mod16) and warns the user
  -Refactored dialogs, added MessageBoxEx reference, and removed MPEG2SourceChoice
  -Removed OneClickDefaults.cs, OneClickDefaultWindow.cs/.resx

[dimzon]

Originally Posted by berrinam

0.2.3.2055 27 Jan 2006
Commit by berrinam:
  -Added video input checking (video exists, has no errors, colorspace==yv12, mod16) and warns the user
  -Refactored dialogs, added MessageBoxEx reference, and removed MPEG2SourceChoice
  -Removed OneClickDefaults.cs, OneClickDefaultWindow.cs/.resx

                AvsReader avi = AvsReader.OpenScriptFile(avsFile);
                if (!avi.Clip.HasVideo)
                    return "AviSynth script has no video";

Take look @ AvsReader.OpenScriptFile first:

                _enviroment = new AviSynthScriptEnvironment();
                _clip = parse? _enviroment.ParseScript(script, AviSynthColorspace.RGB24) : _enviroment.OpenScriptFile(script, AviSynthColorspace.RGB24);
                if (!_clip.HasVideo)
                    throw new ArgumentException("Script doesn't contain video");

[Doom9]

If Doom9 wants your way, then you just change the ConvertToYV12 section of the code. There are two places you should look at: JobUtil.prepareVideoJob, which calls the function VideoUtil.checkVideo(string)

I don't think such breakout scripts should be of our concern.. those who write advanced scripts should be knowledgeable enough to figure out what can go wrong.. our main concern has to be making things work for the users that have very little or no AviSynth knowledge at all.

@dimzon: why do you use _ prefixes for variable names when the rest of the project uses no such thing? It's breaking with style. I generally like to use the java style notation (somevariablename becomes someVariableName.. first letter small, subsequent letters at positions where a word could be separated in capital letters). I know I'm not being consistent but I think considering that so many people are working on this now, we really should strive to be consistent throughout the code. The same goes for comments.. every method should have one (once again I've been too lazy in the past), and I'm also no fan of the whole if something ? then : else notation.. it makes a program harder to read, and forces you to read the entire line, then hammer out the logic in your head rather than just read over the if and know what it's all about.

[dimzon]

Originally Posted by Doom9

why do you use _ prefixes for variable names when the rest of the project uses no such thing?

first of all - unfortunally MeGUI project doesn't have consustent style across it' sources at all
second - AFAIK Java has not fixed (only one style applicable) notation
third - I'm using Microsoft-proposed style.
this means:
all nonpublic fields must start with _ (private int _someInt)
all nonpublic methods must have lowercase fiirst char (private int someInt() )
all public methods must have upper case first char (public int SomeInt() )

its really usefull notation: looking @ such code

_script = generateScript(val);
Start();

You can say:
_script is protected field, Start() is public and generateScript is protected method without looking @ definition.

But if You don't like this stye feel free to change it using Refactoring features of VS2005 (right click on name and choose Refactor->Rename)

Originally Posted by Doom9

The same goes for comments.. every method should have one

It's my fault. Unfortunally my english is poor so I believe reading source will give you better infotmation against reading comments.

Originally Posted by Doom9

and I'm also no fan of the whole if something ? then : else notation.. it makes a program harder to read, and forces you to read the entire line, then hammer out the logic in your head rather than just read over the if and know what it's all about.

Agreed, sometimes it's harder to read then if/else...

[Doom9]

all nonpublic fields must start with _ (private int _someInt)

But Microsoft also suggests the use of properties for publicly accessible variables.. thus you can use what you're using for methods and transfer it directly to variables. Any variablename starting with a lowercase letter is private, anything that's exposed is a property which has the same name but starts with a capital letter. I think I've been following that style pretty closely where variables are concerned.

However, I don't quite see the necessity to see the difference.. if you're working "inside" the class.. you don't really care whether a method is public or not. If you're working "outside" the class.. if it's private you won't even get to see it.. plus then there are the protected and internal intricacies that you can cover with changing just one letter. If working "outside" a class, if intellisense doesn't pick up a method name you know is there.. you have to go back and change the access modifier anyway.

Come to think of it.. iirc most samples I've seen using the _ notation were done by programmers coming from a language that doesn't have properties and doesn't commonly work with setters/getters. I'm not used to seeing _ in variable names in msdn samples (for C# of course)

[dimzon]

Originally Posted by Doom9

if you're working "inside" the class.. you don't really care whether a method is public or not.

When I'm looking @ ugly code insude class can say - ok, this method use only nonpublic features, I can refactor it

Originally Posted by Doom9

Come to think of it.. iirc most samples I've seen using the _ notation were done by programmers coming from a language that doesn't have properties and doesn't commonly work with setters/getters. I'm not used to seeing _ in variable names in msdn samples (for C# of course)

http://www.akadia.com/services/naming_conventions.html
http://dotnet.mvps.org/dotnet/faqs/?...ntions&lang=en
http://msdn.microsoft.com/library/de...guidelines.asp

[dimzon]

Originally Posted by berrinam

added MessageBoxEx reference

What is MessageBoxExLib.dll ?
Where I can find it's source code?