PDA

View Full Version : MeGUI development


Pages : 1 2 3 4 5 [6] 7 8 9 10 11 12 13 14 15 16 17 18 19

Doom9
15th January 2006, 01:39
hmm.. now this is weird. I did a new cvs checkout, then put all my patched files in.. then I commited my stuff. I had no conflicts whatsoever, but a couple merges. But the weirdest thing, I can't compile anymore now, I have 3 errors:

MeGUI.openVideoFile is missing
and MeGUISettings don't contain a SourceDetectorSettings

Something went wrong.. I ran CVS update on the files I felt should've been updated (the settings, settingsform, form1), and there was a merge, but where the heck are those changes?

Doom9
15th January 2006, 03:47
New CVS update:

0.2.3.2024 15 Jan 2006
New generalized muxer architecture
enum refactoring
fix for encoder FPS display on PCs using locals with non dot decimal separator

Doom9
15th January 2006, 13:47
New CVS update:

0.2.3.2025 15 Jan 2006
Bugfix: incorrect time elapsed / projected end time
Bugfix: Job FPS of xvid_encraw jobs was incorrect
New: Start and End time of job processing is added to the log

berrinam
15th January 2006, 14:00
0.2.3.2026 16 Jan 2006
Bugfix: another attempt at fixing missing Decomb.dll crash
Bugfix: reapplied the fix for multiple audio names. It was somehow lost in v2024

Doom9
15th January 2006, 20:38
0.2.3.2027 15 Jan 2006
Bugfix: Attempting to start a job that fails no longer returns a "no job found" error but the proper error message
Changed: Jobs are always encoded in their queue order regardless of their interdependence
Changed: Refactored the audio encoding part
Changed: Muxing / audio encoding progress is reported with two digits after the decimal separator

Mutant_Fruit
15th January 2006, 23:31
Jobs are always encoded in their queue order regardless of their interdependence
I assume its still impossible for pass3 to run without pass2 running? Or pass2 before pass1? I can't seem to get the code from the CVS to check.

berrinam
16th January 2006, 04:32
0.2.3.2028 16 Jan 2006
Added support for prerendering avs scripts.
Added a place-holder (but no support) for analysis passes of avs scripts.
Bugfix: Pressing Start in the queue tab will always change its name to stop, even if nothing does start.


Just as a side note: adding support for prerendering was really easy. I'm sure this was due to the recreated encoder architecture, because all I needed to do was add a commandline generator, a settings class, and adjust the job generation to include the prerendering script in the chain. Thank you.

Richard Berg
16th January 2006, 05:00
Good to hear. I was waiting for Doom9's refactoring before starting, but then anonymous CVS was b0rked for a day and you beat me to it ;)

Based on your experience, it should be equally easy to add my next feature: dvr-ms support. Just need to wrap dvrmstompeg (http://www.thegreenbutton.com/community/shwmessage.aspx?ForumID=26&MessageID=113435) the way you did avs2yuv, then feed it to dgindex.

berrinam
16th January 2006, 05:14
Just need to wrap dvrmstompeg (http://www.thegreenbutton.com/community/shwmessage.aspx?ForumID=26&MessageID=113435) the way you did avs2yuv, then feed it to dgindex.
Actually, I completely avoided any new programs. I just ran it through mencoder (so it wasn't exactly as zajc described -- I was intending to do that, but the commandline embedded in avs2yuv needed changing, and it crashed when I ran it with my updates, so I gave up on that)

Doom9
16th January 2006, 08:18
Just as a side note: adding support for prerendering was really easy. I'm sure this was due to the recreated encoder architecture, because all I needed to do was add a commandline generator, a settings class, and adjust the job generation to include the prerendering script in the chain. Thank you.I'm glad somebody found it useful. I'm considering many more changes (move the whole job handling away from Form1 for good.. that class is way too large and half the job creation is already in JobUtil).
I assume its still impossible for pass3 to run without pass2 running? Or pass2 before pass1?No, it's now up to the user. I'll use the "You are with stupid" smilie when people post "bug reports" because they moved the order of their jobs ;)

berrinam
16th January 2006, 11:31
I want to add support for running through an avisynth script (for two-pass avisynth scripts). My idea was to create a new Job type called AviSynthJob, and a new AviSynthProcessor. However, I am a bit confused: which class should AviSynthProcessor extend? IJobProcessor, or Encoder?

dimzon
16th January 2006, 12:23
Just proposal (don't kill me)
How about to drop LMP4 support (keep only XviD)
seems like LMP4 does'nt have ANY advantage over XviD:
XviD is Free like LMP4
XviD is faster
XviD provide better quality?

dimzon
16th January 2006, 12:29
Just another question:
does anybody perform x264 encoding via mencoder? I really prefer x254.exe (it's fresher)

Doom9
16th January 2006, 13:44
However, I am a bit confused: which class should AviSynthProcessor extend? IJobProcessor, or Encoder?Hmm.. why didn't I remove Encoder? I guess DGIndexer still derives from it.. so time for some additional refactoring. Encoder will definitely disappear, and it's proper to extend IJobProcessor.. it defines the event and functions each job processor must offer. In the end I'd like to just have a JobHandler class (it's already there, but not doing anything), which then dispatches the jobs to the proper processor. So in the GUI, pressing pause will result in a call to handler.pause(), rather than having to switch through all the different jobtypes.. so the guy no longer has to know what kind of types there are.

How about to drop LMP4 support (keep only XviD)Well.. it's always hard to give up something you've worked on.. but I have honestly not used it for anything than quick tests either (in its default config it's quite fast). If it causes me any grief during refactoring I might just get rid of it.

does anybody perform x264 encoding via mencoder? I really prefer x254.exe (it's fresher)the thought of removing it also crossed my mind.. the thing is.. we have avi output and x264.exe doesn't support that (neither does encraw). While mencoder can mux raw ASP and AVC streams into AVI, those AVIs cannot be played via VfW or DShow and are thus improper.. I've mentioned that in the mencoder list but so far nobody bothered to fix it. Time to ask for a good AVI muxer in the container forum.

Naturally, when the AVI muxing issue is resolved, x264 in mencoder is a goner. The same goes for the time when xvid_encraw offers all XviD features.

soresu
16th January 2006, 14:02
Just a quick (albeit probably dumb) question here. I was wondering what was meant by "Added support for prerendering avs scripts"?

berrinam
16th January 2006, 14:07
Check the feature request thread. It explains the feature in the first post.

dimzon
16th January 2006, 14:28
Naturally, when the AVI muxing issue is resolved, x264 in mencoder is a goner. The same goes for the time when xvid_encraw offers all XviD features.
Like to read this! Fine! Which additional features (except container) does You need in xvid_encraw? Maybe I can help to implement them?

Richard Berg
16th January 2006, 15:02
Keep in mind, our new Avisynth prerender feature relies on mencoder.

Richard Berg
16th January 2006, 15:32
I'm considering many more changes (move the whole job handling away from Form1 for good.. that class is way too large and half the job creation is already in JobUtil).
Definitely! The changes in my patch help abstract things a little bit -- e.g. functions like changeVideoOutputExtention and verifyVideoSettings now pass strings in & out instead of manipulating the forms directly -- but the knowledge contained in these functions shouldn't be "owned" by the MeGUI class in the first place. I've cleaned up the logic in many places, but it's still too complex, probably the source of many bugs, and very very difficult to extend.

Look at all the places where the MeGUI class has to know "insider information" about the various classes. That should never happen; we should be asking the classes (or their base interface) for info. For example, I split out part of the verification logic into MeGUI.isFirstPass(). Why should MeGUI have to figure this out? It should be a public property of the currently selected Encoder. The good news, I suppose, is that we can use the existing mess of code to figure out exactly what info needs to be available from each interface -- figuring this out when you originally designed the architecture would've been more "correct" but obviously harder.

Doom9
16th January 2006, 15:38
It should be a public property of the currently selected Encoder. Of the encoder? That implies you instantiate one (only jobhandler knows which), and send the settings to it.. shouldn't there be an easier way, like asking a VideoCodecSettings object?

figuring this out when you originally designed the architecture would've been more "correct" but obviously harder.Keep in mind that the redesign was done at a time when there was but one videoencoder, one audio encoder and nothing more.. and for starters there wasn't even a concept of a job.. it was configure.. then encode, then configure again, and encode again, etc.

Which additional features (except container) does You need in xvid_encraw?I've given syskin a list.. hopefully he'll come through with them. If the darned thing were written in C# I'd lay hands on it myself, but I like to avoid non managed languages unless I'm being paid not to.

dimzon
16th January 2006, 16:06
I've given syskin a list.. hopefully he'll come through with them. If the darned thing were written in C# I'd lay hands on it myself, but I like to avoid non managed languages unless I'm being paid not to.
I have take look @ xvidcore API. It's possible to write C# code to invoke it. Maybe it will be best solution for utilizing xvid in MeGUI?

Doom9
16th January 2006, 16:26
I have take look @ xvidcore API. It's possible to write C# code to invoke it. Maybe it will be best solution for utilizing xvid in MeGUI?Well.. if you are willing to do that, by all means :) Doing that might even make it possible to get AVI output..

Richard Berg
16th January 2006, 17:24
shouldn't there be an easier way, like asking a VideoCodecSettings object?

Maybe so. I haven't looked at the new interfaces enough to see exactly which object should encapsulate which knowledge. I'm just certain none of it should go in Form1.

It's possible to write C# code to invoke it.
If we don't mind adding dependencies on other code (including DirectShow), the same is true of dvrms -> mpeg2 conversion...the project I linked is already pure C#. At present, though, I think MeGUI is better set up to use external command-line tools. Running jobs "internally" means we need to spin off & synchronize with another thread, then refactor the Progress window, and probably more work I haven't thought of...

Doom9
16th January 2006, 17:37
well.. the dependency isn't any different from an encoder executable. The VfW dependency is already there and used quite heavily. I don't see how handling a dll would be much different from handling the commandline encoder.. we still keep track of number of frames encoded, just have to decode the frame first, send it to the encoder, receive a buffer back, and dump it to a file. The Progress window would still remain the same (well... it could use some improvement.. perhaps the statusupdate could have a message property and use more generic filesize1 and filesize2 names or an array of filesizes). Still.. those aren't very significant things imho. And the whole encoding thing is threaded anyway (now not so visible anymore.. it was much more apparent with the old code where I started threads myself)

Anyway, looking at the new encoders, any idea how a fully generalized JobHandler would look like? I'm still having somewhat of a beef at how much I can generalize.. I think a JobHandler -> VideoEncoder/AudioEncoder/Muxer -> Commandline(venc/aenc/mux) -> xyzencoder/muxer creates overhead and that basically there should be a basic CommandlineToolHandler.. but it's not possible that a xyzencoder inherits from both JobHandler and ComandlineToolHandler. Right now I'm unsure about the most efficient architecture that causes the smallest amount of code redundancy.

dimzon
16th January 2006, 17:41
Anyway, looking at the new encoders, any idea how a fully generalized JobHandler would look like? I'm still having somewhat of a beef at how much I can generalize.. I think a JobHandler -> VideoEncoder/AudioEncoder/Muxer -> Commandline(venc/aenc/mux) -> xyzencoder/muxer creates overhead and that basically there should be a basic CommandlineToolHandler.. but it's not possible that a xyzencoder inherits from both JobHandler and ComandlineToolHandler. Right now I'm unsure about the most efficient architecture that causes the smallest amount of code redundancy.
What is difference between JobHandler and ComandlineToolHandler? (I don't have fresh source code)

Doom9
16th January 2006, 18:04
JobHandler would be the generic implementation of IJobHandler.. it's the base class to handle any job. CommandlineToolHandler currently doesn't exist.. but it would be a base class to handle any commandline tool.. the actual commandline encoders and muxers would derive from it.

Richard Berg
16th January 2006, 18:11
Would this inheritance structure work?

x264Encoder
/ \
/ \
IVideoEncoder ICommandLineHandler
\ /
\ /
IJobHandler

dimzon
16th January 2006, 18:17
but it's not possible that a xyzencoder inherits from both JobHandler and ComandlineToolHandler
Richard Berg was first ;)

If CommandLineHandler is just a utility to wrap commandline tool execution You can use another pattern (pattern-adapter && pattern-strategy):

interface IJobHandler
{
...
}

interface IVideoEncoder:IJobHandler
{
...
}

class CommandLineHandler
{
...
}

class x264Encoder: IVideoEncoder
{
CommandLineHandler helper = new CommandLineHandler(...);

private void IVideoEncoder.Foo()
{
helper.Foo2(...);
}

}

dimzon
16th January 2006, 18:50
Seems like it's possible to write C# wrapper around libmatroska for muxing puposes :) So xvid_ecncraw with mkv output in C# can be a reality!

Doom9
16th January 2006, 20:08
@Richard: work of course, but I actually need some processing.. for instance the CommandlineHandler should at least offer methods to read and dispatch lines read from stdout / stderr.
The videoencoder right now has only little intelligence... it can figure out which VideoEncoder it needs based on the settings given in the job.. that logic still needs to go someplace and it's common logic. It can also calculate the completion percentage and fill in the filesize. Granted, the filesize could also be done by a class higher up the hierarchy.. if you go lower as in the two proposals, you end up having to copy&paste that code again.

Also, for both proposals: who handles the encoder instantiation so that in the main GUI, all that it finally does is call JobHandler.process(Job job)?

So xvid_ecncraw with mkv output in C# can be a reality!I'm glad you're volunteering. Though, I think the audio functionality should have priority as it allows the integration of yet another unique feature I have in mind for MeGUI: frame accurate cutting based on AviSynth.. once audio encoding uses AviSynth that'll be a realitiy and that is a strong USP for MeGUI.

And at some time we might have to rethink the GUI.. right now it starts with what is good for us advanced users who know how to get things done.. but a lot of people are overwhelmed by that and would be better of with something simpler aking the one click mode.

Richard Berg
16th January 2006, 21:31
Do you mean being able to AutoEncode directly from an AVS file (without having to split the audio out into a separate WAV)? That would be really great.

Doom9
16th January 2006, 23:24
Do you mean being able to AutoEncode directly from an AVS fileYup.. that's the goal.

Richard Berg
17th January 2006, 00:02
Awesome. I haven't looked at enough MeGUI architecture to answer the previous questions, but I'll keep refactoring here & there when it makes sense.

berrinam
17th January 2006, 01:07
0.2.3.2030 17 Jan 2006
Added support for analysis passes of avs scripts (the button was added in 2028)

Richard Berg
17th January 2006, 03:24
What does that do -- render out to null? For use with plugins like Call() and SSIM()?

Do I need to update my patch again?

berrinam
17th January 2006, 03:38
Yes, it just requests the frame and ignores it. Just as you said, for functions that write to files. This also includes 2pass scripts (eg DeDup and hybrid TIVTC). Perhaps 2pass AviSynth support should be added (it would allow for a better mode of IVTC'ing).

You might need to update your patch, but I'm not sure... actually, from your changelog, that doesn't seem to be the case.

I would integrate your patch into CVS, but I don't have access to it (pending approval of a mod). If you use the SourceForge patch tracker (http://sourceforge.net/tracker/?group_id=156112&atid=798478), then it's easier for me to access and integrate, because I don't have to wait for a mod's approval. OTOH, you need a SF account to post patches there.

Richard Berg
17th January 2006, 04:12
Forgot about mod approval -- I shouldn't use forum attachments, that's me being lazy. It's on SF now (http://sourceforge.net/tracker/index.php?func=detail&aid=1407878&group_id=156112&atid=798478). Here's a link (http://forum.doom9.org/showthread.php?p=768866#post768866) to the post with the changelist.

Richard Berg
17th January 2006, 04:22
Is it just me or is the menu bar still not working?

berrinam
17th January 2006, 05:01
Is it just me or is the menu bar still not working?
This is fixed in my local version. After I fix the CC issues, I will commit it (including your patch).

berrinam
17th January 2006, 05:19
New CVS Update (it's a big one)
0.2.3.2031 17 Jan 2006
Added Richard Berg's contextmenu changes patch:
Feature changes:
- queueContextMenu
- added Delete button
- fixed logic so buttons are disabled when no items selected
- changed order so most common items are toward the top
- added hotkeys
- audioOutput & videoOutput textboxes are now editable
- drag-n-drop is now disabled unless the Input tab is selected
- error messages related to audio & video job setup are now more helpful/detailed

Refactoring changes:
- new enum CodecType (works just like FileType)
- rewrote verifyVideoSettings & assorted helper methods, called it whenever queueing a video job
- ditto verifyAudioSettings
Other changes:
-added a contextmenustrip for the tray icon
-minimize to tray hides the progress window
-closing MeGUI while a job is running now calls abort instead of aborting manually
-fixed menus for megui-x264
-fixed loading of avs and hfyu jobs
-moved start/stop pause/play changes to a separate function

Doom9
17th January 2006, 09:25
I think we should set up someting like this: automatic daily CVS exports that are zipped along with a compile-x264.bat and compile-full.bat which compile the x264 and full release respectively, then publish this somewhere. That way we don't have to worry about making releases, and even the most clueless people can compile their own builds (that's what the compile files are for.. if you have the runtime, you can compile). That way, there are no more cumbersome releases for us to do and people can always use the latest code.

berrinam
17th January 2006, 10:21
Automatic CVS exports would be good. I don't know if they are possible. However, people will still want binaries, just because that is what they always get. We can't completely stop releasing binaries.

dimzon
17th January 2006, 12:05
I think we should set up someting like this: automatic daily CVS exports that are zipped along with a compile-x264.bat and compile-full.bat which compile the x264 and full release respectively, then publish this somewhere. That way we don't have to worry about making releases, and even the most clueless people can compile their own builds (that's what the compile files are for.. if you have the runtime, you can compile). That way, there are no more cumbersome releases for us to do and people can always use the latest code.
Take look @ Build Facility @ sf.net

dimzon
17th January 2006, 12:14
Also, for both proposals: who handles the encoder instantiation so that in the main GUI, all that it finally does is call JobHandler.process(Job job)?
I have an ansewer but You don't like it :) Job itself :) You can use pattern-facility:
Job provide some facility to create encoder

interface IJobHandlerFacility
{
IJobHandler CreateJobHandlerFor(IJob job)
}

interface IJob
{
IJobHandlerFacility HandlerFacility{get;}
}


...
job.HandlerFacility.CreateJobHandlerFor(job)
...

or You can wrap this call into job too:

abstract class Job:IJob
{
public IJobHandler CreateJobHandler()
{
(this as IJob).HandlerFacility.CreateJobHandlerFor(this)
}
}





Though, I think the audio functionality should have priority as it allows the integration of yet another unique feature I have in mind for MeGUI: frame accurate cutting based on AviSynth.. once audio encoding uses AviSynth that'll be a realitiy and that is a strong USP for MeGUI.
Yes, but I can't work on it without version control


And at some time we might have to rethink the GUI.. right now it starts with what is good for us advanced users who know how to get things done.. but a lot of people are overwhelmed by that and would be better of with something simpler aking the one click mode.
Agreed by 1000000% All my friends blame MeGUI for user unfriendly GUI

Doom9
17th January 2006, 12:19
Yes, but I can't work on it without version controlWhy? Mutant_Fruit and Richard Berg don't have CVS access and they are still contributing.

@I have an ansewer but You don't like it:You are right, I don't like it at all. To make matters worse, your approach requires that a job be aware of the MeGUI settings.. it has to return a jobhandler in function of the x264 / xvid encoder, and in the future which audio encoding mode is chosen. So you may have eliminated knowledge at one place only to put a lot of info inside a job that has no business being there.

dimzon
17th January 2006, 12:25
Why? Mutant_Fruit and Richard Berg don't have CVS access and they are still contributing.
I'm worry about conflicts (I need transactional atomic CheckIn/CheckOut).

berrinam
17th January 2006, 12:35
@Doom9: Is there a reason you want to restrict CVS access? I certainly think it would be easier to be able to avoid the patching system.

max-holz
17th January 2006, 12:45
As I can see in other project on sourceforge there is a restricted group of developers and other people that contribute submitting patches via bugzilla.

Doom9
17th January 2006, 13:15
@Doom9: Is there a reason you want to restrict CVS access?Considering that I was up till 4 am on Sunday morning because I fucked up a commit (or... CVS let me) and had to fix it (well, you were a big part of it teaching me how to handle the merge tool), I think that makes for a pretty strong argument to limited access.

I'm worry about conflictsExcept for the settings, I see no conflicts whatsoever. You have a look at all the audio encoding classes, add a little code that selects your own audio encoder, and after that you have full control. For starters you can also only add additional paths you need to MeGUISettings without exposing them in the GUI.. that will not cause any conflicts whatsoever. I don't think you should create a new class of Job, or AudioSettings but instead use what is already available.. the only changes really are to be made in the classes derived from AudioEncoder plus additional paths in MeGUISettings.

And in the end when you create a patch it's up to somebody with CVS access to integrate it..

dimzon
17th January 2006, 13:20
Considering that I was up till 4 am on Sunday morning because I fucked up a commit (or... CVS let me) and had to fix it (well, you were a big part of it teaching me how to handle the merge tool), I think that makes for a pretty strong argument to limited access.
In other hand I have 5-year expiriense working with source version control (not CVS but VSS)...

Except for the settings, I see no conflicts whatsoever. You have a look at all the audio encoding classes, add a little code that selects your own audio encoder, and after that you have full control. For starters you can also only add additional paths you need to MeGUISettings without exposing them in the GUI.. that will not cause any conflicts whatsoever. I don't think you should create a new class of Job, or AudioSettings but instead use what is already available.. the only changes really are to be made in the classes derived from AudioEncoder plus additional paths in MeGUISettings.
I want to perform some reactoring @ audio settings form (inheritance etc).
Ok, You can provide me fresh sources. Then I will add my functionality and sent it back to You. And merging with CVS woud be You problem. Does You agree?

Doom9
17th January 2006, 13:26
@dimzon: I've added you as a developer. However, when it comes to refactoring, the same rules still apply: ask me before you do anything. As far as the audio configuration dialogs go, you are cleared to modify those.. everybody else hands off please. Keep in mind though that I'd not yet like to lose the besweet encoding (it should be able to switch.. even if only by a non GUI exposed bool in the MeGUIsettings), and make your new audio encoder integrate with the current architecture.

dimzon
17th January 2006, 13:30
However, when it comes to refactoring, the same rules still apply: ask me before you do anything.
Ok. I will perform Audio Dialogs Refactoring & a little MeGuiSettings changes & cleanups
Keep in mind though that I'd not yet like to lose the besweet encoding (it should be able to switch.. even if only by a non GUI exposed bool in the MeGUIsettings), and make your new audio encoder integrate with the current architecture.
Ok.

dimzon
17th January 2006, 13:46
2 All
How does You commit changes to CVS?
I'm trying to use "Commit..."

In C:\Documents and Settings\DAlexandrov\My Documents\Visual Studio 2005\Projects\MeGUI\MeGUI-src.CVS: "C:\Program Files\TortoiseCVS\cvs.exe" "-q" "commit" "-m" "3 additional properties for faac, neroaac and lame" "MeGUISettings.cs"
CVSROOT=:ext:dimzon@cvs.sourceforge.net:/cvsroot/megui

cvs: rcs.c:4188: RCS_checkout: Assertion `rev == ((void *)0) || ((*__ctype_b_loc ())[(int) (((unsigned char) *rev))] & (unsigned short int) _ISdigit)' failed.
cvs [commit aborted]: received abort signal

Error, CVS operation failed

Doom9
17th January 2006, 13:55
never happend to you in 5 years? It happened for my second commit. Here's what you should not do: do a fresh CVS checkout, overwrite those files with your changed ones, then commit... that's what I did and it's no fun. I'm afraid it looks like you're in for a manual treat (I suggest a fresh checkout, compare files and merge your changes). But perhaps somebody knows more.. either way it looks like a bugreport for tortoise would be in order.

dimzon
17th January 2006, 14:02
never happend to you in 5 years?
Microsoft Visual Source Safe works fine!

dimzon
17th January 2006, 14:16
2 All
How does You commit changes to CVS?
I'm trying to use "Commit..."

In C:\Documents and Settings\DAlexandrov\My Documents\Visual Studio 2005\Projects\MeGUI\MeGUI-src.CVS: "C:\Program Files\TortoiseCVS\cvs.exe" "-q" "commit" "-m" "3 additional properties for faac, neroaac and lame" "MeGUISettings.cs"
CVSROOT=:ext:dimzon@cvs.sourceforge.net:/cvsroot/megui

cvs: rcs.c:4188: RCS_checkout: Assertion `rev == ((void *)0) || ((*__ctype_b_loc ())[(int) (((unsigned char) *rev))] & (unsigned short int) _ISdigit)' failed.
cvs [commit aborted]: received abort signal

Error, CVS operation failed

Ok. Just do not perform "unedit" command before

dimzon
17th January 2006, 15:56
Doom9, I have some questions/proposals:

1) Show command line @ AudioSettingsDialog
What I must output there (all processing except encoding is done by AviSynth script)

2) Multichannel code. Current solution consist of 2 radio buttons (stereo && 5.1) & 1 combo box (stereo, dpl, dpl2). I propose 1 combobox instead:
Name: Multichannel Processing
Values:

Keep Original Channel Count
Convert To Mono
Downmix to Stereo
DPL Downmix
DPL II Downmix

Doom9
17th January 2006, 16:02
2) Multichannel code. Current solution consist of 2 radio buttons (stereo && 5.1) & 1 combo box (stereo, dpl, dpl2). I propose 1 combobox instead:
Name: Multichannel ProcessingThat works for your dialog, but not the BeSweet one.. it needs to be told if the source is 2.0 is 5.1 (well, not exactly told.. but you need the construct a commandline that works for either 5.1 sources and keeps 'em that way, or works for 5.1 with downmix/2.0 sources). Changing these options in the existing dialog would make besweet encoding impossible as besweet neets to be told if it has to operate in 2.0 or 5.1 mode.. even though it would be perfectly capable of figuring that out on its own.

1) Show command line @ AudioSettingsDialog
What I must output there (all processing except encoding is done by AviSynth script) Well, you're still running a commandline encoder somewhere, are you not? So the commandline for it would be appropriate.

dimzon
17th January 2006, 16:46
That works for your dialog, but not the BeSweet one.. it needs to be told if the source is 2.0 is 5.1 (well, not exactly told.. but you need the construct a commandline that works for either 5.1 sources and keeps 'em that way, or works for 5.1 with downmix/2.0 sources). Changing these options in the existing dialog would make besweet encoding impossible as besweet neets to be told if it has to operate in 2.0 or 5.1 mode.. even though it would be perfectly capable of figuring that out on its own.
I'm planning to write managed wrapper around mediaInfo.dll to obtain original channel count in beSweet case..

Well, you're still running a commandline encoder somewhere, are you not? So the commandline for it would be appropriate.
Ok.

Doom9
17th January 2006, 16:54
I'm planning to write managed wrapper around mediaInfo.dll to obtain original channel count in beSweet case..Don't bother.. once your avisynth based encoding works and we are sure we're not going to lose functionality by removing the besweet part, the besweet part will be removed. It makes little sense to invest additional effort into something that is going to be gone sooner or later.

In addition, it wouldn't really always help.. consider the case where somebody is configuring audio first (e.g. creating a profile), and then loading the actual file. So in the end you'd have to override the commandline at the beginning of encoding to adapt in function of the source properties..

dimzon
17th January 2006, 17:03
2 all Developers

New file @ CVS - EmumProxy.cs

this is helper class to bind Enum's to UI with friendly names
use sample is commented out @ the end of file (take a look)
ps. May be not very optimal (private implementation is subject of change)


public enum DownmixMode
{
[EnumTitle("Keep Original Channels")]
DoNothong,
[EnumTitle("Convert to mono")]
ToMono,
...
[EnumTitle("Dolby Pro Logic II")]
ToDPL2
}


MyCombo.Options.AddRange(EnumProxy.CreateArray(new object[]{DownmixMode.DoNothong,...,DownmixMode.ToDPL2}))

Doom9
17th January 2006, 17:06
Why not just use ints? That works fine for the ProcessPriority enum (you can find it in Job.cs I think).. it is mapped to a dropdown.

Hmm.. on second thought, is the point to have the enum names in text?

dimzon
17th January 2006, 17:10
Don't bother.. once your avisynth based encoding works and we are sure we're not going to lose functionality by removing the besweet part, the besweet part will be removed.
Isn't existing technology implementation (BeHappy) enought to be sure? Maybe I will wait a little until You inspect BeHappy more careful?

So in the end you'd have to override the commandline at the beginning of encoding to adapt in function of the source properties..
I'm a really sorry, I do not understand You at all! Command line is _private_ implementation detail. You does not like about Job can create it's own encoder but You hold specific information for concrete encoder implementation!

dimzon
17th January 2006, 17:15
Why not just use ints? That works fine for the ProcessPriority enum (you can find it in Job.cs I think).. it is mapped to a dropdown.
If You change your enum order You must syncronize your changes with ComboBox.


Hmm.. on second thought, is the point to have the enum names in text?
Yes! You can specify friendly names for enum. This is MAIN point.

stax76
17th January 2006, 17:22
public enum DownmixMode
{
[EnumTitle("Keep Original Channels")]
DoNothong,
[EnumTitle("Convert to mono")]
ToMono,
...
[EnumTitle("Dolby Pro Logic II")]
ToDPL2

MyCombo.Options.AddRange(EnumProxy.CreateArray(new object[]{DownmixMode.DoNothong,...,DownmixMode.ToDPL2}))
}

That's nifty, exactly my way of doing things though I would automate much further. Is EnumTitle a .NET class? If so then I've missed it.

dimzon
17th January 2006, 17:28
Is EnumTitle a .NET class? If so then I've missed it.
:D
Yes, it's .NET class (but it's not a part of .NET Framework). Definitly - every MeGUI class is .NET class :)

EnumTitle is my custom attribute defined @ EnumProxy.cs

stax76
17th January 2006, 18:09
You could populate the list control by passing the enum type to a method, other than that my code is very similar:


Public Class ListBag(Of T)
Public Sub New(ByVal text As String, ByVal value As T)
Me.Text = text
Me.Value = value
End Sub

Private TextValue As String

Public Property Text() As String
Get
Return TextValue
End Get
Set(ByVal Value As String)
TextValue = Value
End Set
End Property

Private ValueValue As T

Public Property Value() As T
Get
Return ValueValue
End Get
Set(ByVal Value As T)
ValueValue = Value
End Set
End Property

Public Shared Sub SelectItem(ByVal cb As ComboBox, ByVal value As T)
Dim selectItem As Object = Nothing

For Each i As ListBag(Of T) In cb.Items
If i.Value.Equals(value) Then
selectItem = i
End If
Next

If Not selectItem Is Nothing Then
cb.SelectedItem = selectItem
End If
End Sub

Public Shared Function GetValue(ByVal o As Object) As T
Return DirectCast(DirectCast(o, ListBag(Of T)).Value, T)
End Function

Public Shared Sub PopulateList(ByVal l As IList)
For Each i As T In System.Enum.GetValues(GetType(T))
l.Add(New ListBag(Of T)(NameAttribute.GetEnumName(Of T)(i), i))
Next
End Sub

Public Overrides Function ToString() As String
Return Text
End Function
End Class

<AttributeUsage(AttributeTargets.All)> _
Public Class NameAttribute
Inherits Attribute

Public Sub New(ByVal name As String)
ValueValue = name
End Sub

Private ValueValue As String

Public ReadOnly Property Value() As String
Get
Return ValueValue
End Get
End Property

Public Shared Function GetEnumName(Of T)(ByVal value As T) As String
For Each i As FieldInfo In value.GetType.GetFields
If i.GetValue(value).Equals(value) Then
Dim attributes As Object() = i.GetCustomAttributes(False)

For Each i2 As Object In attributes
If i2.GetType Is GetType(NameAttribute) Then
Return DirectCast(i2, NameAttribute).Value
End If
Next

Return i.Name
End If
Next

Throw New Exception
End Function
End Class

dimzon
17th January 2006, 20:09
New base AudioConfiguration prototype - proposal
http://img388.imageshack.us/img388/5016/untitled1lt.png

FFWD
17th January 2006, 20:26
Dimzon, I see downmix to PLII in your screenshot. So is there a way to get a Dolby Pro Logic II AAC from a 5.1 AC3? Does it also work with HE-AAC files? And more important; does the decoding actually work? (my Logitech Z-5500 supports Pro Logic II)

Doom9
17th January 2006, 21:00
@dimzon: I think the load defaults button also couldn't hurt.. it was requested for video but it makes sense for audio as well. Other than that, it looks nice. Obivously this is only the prototype for AviSynth based processing, but since that's the direction we're heading in.. it looks how it should be.

berrinam
18th January 2006, 02:24
0.2.3.2032 18 Jan 2006
Bugfix: Scripts served by scriptserver now work (that's a good thing :D)
Bugfix: Fixed problems with comma locales in string->double conversions in source detection.

Is there a way to specify the locale across the entire GUI or do some other solution, as this problem keeps coming up?

berrinam
18th January 2006, 04:03
0.2.3.2033 18 Jan 2006
Bugfix: AviSynth errors are now displayed in the preview window (changes made by looking at AVIFile code).

EDIT: This code handles YV12 input differently from other input (because YV12 needs to be converted, whereas DIB is fine). As a result, it is possible to tell whether the AviSynth output is YV12, and if it isn't, warn the user that it won't work. What I'm saying is that it is possible for MeGUI to catch the 'unsupported input colorspace' error before it occurs. It could even suggest adding converttoyv12 if it isn't actually an error.

Richard Berg
18th January 2006, 04:23
Is there a way to specify the locale across the entire GUI or do some other solution, as this problem keeps coming up?
Not really. Programmers should know that anytime you do string manipulation, you need to decide whether current locale vs. invariant locale is appropriate. This is very easy compared with the problems I encounter with localized builds & non-unicode encodings :)

Doom9
18th January 2006, 09:30
you need to decide whether current locale vs. invariant locale is appropriate.Invariant doesn't seem to cut it.. I first tried that in the getFrameNumber methods in the video encoders.. it still didn't work on those comma locales, so I forced it to en-us.

dimzon
18th January 2006, 11:42
Dear Doom9
Can you answer on this post http://forum.doom9.org/showthread.php?p=769907#post769907

Doom9
18th January 2006, 11:50
Isn't existing technology implementation (BeHappy) enought to be sure?Looking at your configuration template I'd say yes.. as long as you can cover everything in the BESweet configuration groupbox we won't have any problems.

I'm a really sorry, I do not understand You at all! Command line is _private_ implementation detail. You does not like about Job can create it's own encoder but You hold specific information for concrete encoder implementation!Make that a "currently a job holds the commandline". However, that is going to change.. in fact as it is now it doesn't make a whole lot of sense and the commandline should only be generated from the settings when encoding is to take place. That'll even permit an encoder change at the last moment (for instance mencoder <-> x264). I already had it like that in the past.. there was a reason why I changed it but I can't quite remember right now why it was.. just hoping I won't break anything if I go back to the "encoder generates commandline" approach.
As it is now, it's already possible that you change the path of the encoder after creating the job and you can still encode (except for dgindex.. it still includes the binary name in the commandline).. so we're already halfway there.. but the other half mile is to be walked...

dimzon
18th January 2006, 11:55
Looking at your configuration template I'd say yes.. as long as you can cover everything in the BESweet configuration groupbox we won't have any problems.
Does it mean I can remove BeSweet-related code?

in fact as it is now it doesn't make a whole lot of sense and the commandline should only be generated from the settings when encoding is to take place
Does it mean we does'nt need "Show commandline" option @ configuration dialog?

Doom9
18th January 2006, 12:00
Does it mean I can remove BeSweet-related code?No, as I said.. keep all the code there.. I'll remove it in time. Just don't bother with anything BeSweet related.. write your own dialogs and your own encoder derived from AudioEncoder.

Does it mean we does'nt need "Show commandline" option @ configuration dialog?No, I think that's still good.. it also helps in development (you immediately see what the commandline will be.. the only changes that can come are bitrate related when the bitrate is being automatically calculated).

dimzon
18th January 2006, 12:00
Audio via AviSynth disadvantage
Current implementation (reading audio data via VfW) can't provide detailed information about script error.
Workaround: to work with avisynth.dll directly (without VfW). Unfortunally it's impossible bcz AviSynth API can be accessed only via C++. I'm planning to write wrapper in Managed C++ to provide full power of AviSynth functionality to .NET world...

dimzon
18th January 2006, 12:09
No, as I said.. keep all the code there.. I'll remove it in time.
As you can see I'm altering AudioSettings class (adding new items in emunerations etc). So I cant "Just do not touch BeSweet code" bcz it's break compilation. There are 2 way - perform appropriate adjustments in BeSeet-related code or just remove it @ forget it...

No, I think that's still good.. it also helps in development (you immediately see what the commandline will be.. the only changes that can come are bitrate related when the bitrate is being automatically calculated).
Does it mean I must invoke command-line creation logic twice (ones in audioSettingsDialog, ones before encoding startup)?
So does it mean I can do "getCommandLine()" - a virtual method for AudioSettings?

Doom9
18th January 2006, 12:50
Current implementation (reading audio data via VfW) can't provide detailed information about script error.Neither can any of the video tools.. that's why if I can't open the script via VfW, I do not allow encoding to proceed. I think your audio encoder should do something similar.. open it, see if the properties make sense, and if not, abort and ask the user to play the script in a media player.

As you can see I'm altering AudioSettings class (adding new items in emunerations etc). So I cant "Just do not touch BeSweet code" bcz it's break compilation.So use the variables you're going to add for the avisynth part, and leave the other ones in.. that means a bloated AudioSettings class, but that can be streamlined when we dump the besweet part.

Does it mean I must invoke command-line creation logic twice (ones in audioSettingsDialog, ones before encoding startup)? Yes.

So does it mean I can do "getCommandLine()" - a virtual method for AudioSettings?Since the encoder commandline is taken from the MeGUISettings class, that would mean your derived AudioSettings classes would need to be aware of the settings.. I don't think that's such a good idea. I like the current approch where the encoder and the GUI class have a reference to the settings better.. the job and its settings shouldn't be aware of settings which mostly don't affect them, should they?

dimzon
18th January 2006, 13:53
Since the encoder commandline is taken from the MeGUISettings class
Encoder executable path, not commandline. It's possible to generate commandline arguments without full path (and add real path later).

Doom9
18th January 2006, 14:01
Encoder executable path, not commandline.Yeah, sorry about that. I think we can do away with the necessity of also showing the executable path in the codec configuration, but having the rest (generated of course) is really nice.

Sharktooth
18th January 2006, 15:00
Sorry but i have an infection of the inner hear (dont know the english medical term) that causes me vertigos and other problems.
So i cant stay too much in front of the PC and cant read too much (i cant focus on text).
Bear with me... i'll be back.

dimzon
18th January 2006, 18:01
@Doom9

public interface IJobProcessor
{
/// <summary>
/// sets up encoding
/// </summary
/// <param name="job">the job to be processed</param>
/// <param name="error">output for any errors that might ocurr during this method</param>
/// <returns>true if the setup has succeeded, false if it has not</returns>
bool setup(Job job, out string error);

.NET Has PERFECT error reporting mechanism: Exceptions && Exceptions Hadling. Why does You try propose such ugly method?

Doom9
18th January 2006, 18:17
.NET Has PERFECT error reporting mechanism: Exceptions && Exceptions Hadling. Why does You try propose such ugly method?Because I don't like exceptions? It makes you have to wrap everything into a try/catch block.. and you shouldn't use exceptions for normal program flow.. For instance, if setup returns false because the encoder is not found, it would be a resource waste to resort to an exception for that. There are quite a few instances when an error is reported back that corresponds to regular program flow and thus shouldn't use an exception.

stax76
18th January 2006, 18:27
and you shouldn't use exceptions for normal program flow.

Are you sure? Never tried it but recently I thought it could be a cheap way to abort.

Doom9
18th January 2006, 18:29
Are you sure?It says so in all the literature I should get around to reading for my certification. And Exceptions are resource intensive (I don't recall the exact specifics... but throwing an exception slows you down quite a bit as opposed to using a return variable).

Richard Berg
18th January 2006, 19:51
I don't agree with dimzon in this specific example, but he has a good point. Having to pass error messages around as parameters -- making every caller store or otherwise handle them -- is usually unnecessary work for the programmer. The most helpful feature of exceptions isn't handling them, it's the option not to. If a method isn't equipped to handle some conditions, flow simply passes to the previous caller.

Throwing an exception is kinda expensive: it causes a hardware interrupt that has to be bubbled all the way up from the kernel. Figure a few thousand CPU cycles. But performance is totally irrelevant for asynchronous GUI apps like MeGUI...so long as you're not throwing exceptions in a tight loop you'd never notice.

A decent rule of thumb: exceptions should roughly correspond with problems that are important enough to show the user an error message. If a program throws & catches often without bothering the user, it's probably overusing exceptions. (It's crazy what you sometimes see if you run badly written software it under a debugger that's set to catch all native exceptions...)

With IJobProcessor.Setup, I think using an error parameter is appropriate. Any method that calls Setup should know what to do if it receives an error string. But it should be simpler: instead of having a bool and a string, just return a string; if it's not null, it's an error.

Mutant_Fruit
19th January 2006, 00:01
From what i've been told and what i've read:

Exceptions shouldn't be used to handle normal workflow. If a value you're using has a good chance of NOT being what you expect it to be, use an if statement to see if it's valid. Don't use try/catch to find out. Exceptions are what they say.... Exceptions. They should rarely (if ever) happen.

I'd agree with Richard Berg about not needing a bool AND a string. Thats about it from me... *looks for the 'i am a n00b' poster*

Richard Berg
19th January 2006, 01:12
I wouldn't take any of these rules too far. Exceptions and return values are just tools; good programmers simply choose the best one for the job. Worrying that you'll trigger an exception often enough that someone might call it "normal" is premature optimization, which we know is the root of all evil.

In this case, forcing the immediate caller of IJobProcessor.Setup to always test the return value works ok with our design, so no exception necessary. If IJobProcessor.Setup had dozens of possible entry points, using an exception would be better because it guarantees that someone in the call stack will do something about it.

dimzon
19th January 2006, 12:57
2 Doom9, Richard Berg, Mutant_Fruit
and you shouldn't use exceptions for normal program flow..
<offtopic>
Does and you know how does woking FileMapping/VirtualMemory/PageFile mechanism? Whe you trying to addreess to non-existing page in RAM processor will generate hardware interrupt (exception). Windows catch this interrupt, look for this page in PageFile, load it to the memory. So exceptions (sometimes) is a really fine to be used in normal program flow...
Another sample - for ASP.NET programmers. When You invoke Response.End exception is trown, really, try this code

try
{
Response.End();
}
catch(Exception e)
{
Response.Write(e.ToString());
}

And yet another sample for threadind: When you invoke thread.Abort() ThreadAbordet exeption is throwing
</offtopic>

But my sample IS NOT A NORMAL PROGRAM FLOW.
If JobProcessor can't setup it mean:

You trying to setup JobProcessor for unsupported job
Job configuration is invalid
JobProcessor configuration is invalid
Another shit happens

All options is exceptions, isn't it...

With IJobProcessor.Setup, I think using an error parameter is appropriate. Any method that calls Setup should know what to do if it receives an error string.
I really don't think You will have multiple places to call IJobProcessor.Setup, but if You really want to do it end you does'nt want to wrap your code using try/catch @ every place you can use PATTERN-ADAPTER for it:

public interface IJobProcessor
{
void Setup(Job job);
}

public class JobProcessorAdapter
{
private IJobProcessor _proc;
public JobProcessorAdapter(IJobProcessor p)
{
_proc = p;
}

public bool setup(Job job, out string error)
{
try
{
_proc.Setup(job);
return true;
}
catch(Exception e)
{
error = e.Message;
return false;
}

}
}



2 All developers

I have found another ugly code:

MyForm f = new MyForm();
if(f.ShowDialog()==DialogResult.OK)
{
// Do something
}


Proper way:

using(MyForm f = new MyForm())
{
if(f.ShowDialog(this)==DialogResult.OK)
{
// Do something
}
}

Doom9
19th January 2006, 13:30
I have found another ugly code:What is the problem with it? The local method variable will go out of scope as soon as you exit the method and will be garbage collected..

dimzon
19th January 2006, 14:14
What is the problem with it? The local method variable will go out of scope as soon as you exit the method and will be garbage collected..
Form implemets IDisposabe interface. It means it hold unmanaged resources. Unmanaged resources can't be collected via garbage collector.
Really, garbage collecter will be called only @ deficit memory. But you object can hold window handler or other handles, SqlConnections e.t.c.

Read this:
http://msdn.microsoft.com/library/default.asp?url=/msdnmag/issues/1100/GCI/TOC.ASP?frame=true
http://msdn.microsoft.com/library/default.asp?url=/msdnmag/issues/1200/GCI2/TOC.ASP?frame=true
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dndotnet/html/dotnetgcbasics.asp?frame=true

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/cpref/html/frlrfsystemidisposableclasstopic.asp
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dncscol/html/deepc10192000.asp

dimzon
19th January 2006, 14:33
Dear Doom9.
MeGUI invokes DGDecode.dll direcly. It causes such problem:
DgDecode.dll must be im MeGUI folder or be avaluable @ PATH enviroment string.
In other hand DGDecode.dll MUST be @ avisynth\plugins folder in order to be able open scripts.
So I want to propose solution - let's open d2v files via AviSynth temporaly script file.

Mutant_Fruit
19th January 2006, 15:00
Whe you trying to addreess to non-existing page in RAM processor will generate hardware interrupt (exception)
Aye. This is what i would classify as an "exception". You've tried doing something that is 'impossible'. You can't access a non-existing page, so rightfully an exception should be thrown. This would be normal program flow.

But in this example assume the string "str_value" usually contains an integer, but could occasionally be null....

try
{
int result = Convert.ToInt32(str_value);
}
catch
{
int result = -1; //the string was null or contained invalid data
}

Thats the wrong way of doing it. The right way is this:

try
{
if(str_value != null)
int result = Convert.ToInt32(str_value);
}
catch
{
int result = -1; //string contained invalid data
}

You check to see if it's null BEFORE trying to convert it. That way exceptions will only happen when an "unexpected" event happens. Being null is an expected event that should be checked for.

dimzon
19th January 2006, 15:06
@Mutant_Fruit
But in this example assume the string "str_value" usually contains an integer, but could occasionally be null..

agreed 100%. But IF You cange this stament to
But in this example assume the string "str_value" MUST contains an integer then
try
{
int result = Convert.ToInt32(str_value);
}
catch
{
int result = -1; //the string was null or contained invalid data
}
is valid.

* You trying to setup JobProcessor for unsupported job
* Job configuration is invalid
* JobProcessor configuration is invalid
* Another shit happens

You MUST call JobProcessor for supported job type only
You MUST provide valid job
You MUST provide valid configuaration

So using exceptions in that case is good, is'nt it?

Doom9
19th January 2006, 15:15
* You trying to setup JobProcessor for unsupported jobCan't happen
* Job configuration is invalidCan't happen
* JobProcessor configuration is invalidCan't happen
* Another shit happensLike?

The only thing that can go wrong in the setup is that the encoder cannot be found.. other than that, nothing can go wrong. It is the job of a jobdispatcher to send jobs to where they belong (yeah I know you hate the "is" type checks.. but they ensure this never ever happens). Similarly, it must not be possible to create invalid jobs.. it's the job of the jobcreator to make sure it returns a valid job, or nothing at all.. incidentally it's done that way.. jobs that would be invalid are not returned, instead you get a null value and it's not added to the queue.

dimzon
19th January 2006, 15:20
JobProcessor configuration is invalid
Can't happen

No, it can. Executable path is JobProcessor configuration.

JobProcessor configuration is invalid
Like?

Don't know. Some shit (maybe) @ some implementation...

Mutant_Fruit
19th January 2006, 19:36
@Mutant_Fruit

agreed 100%. But IF You cange this stament to

"But in this example assume the string "str_value" MUST contains an integer"

then
try
{
int result = Convert.ToInt32(str_value);
}
catch
{
int result = -1; //the string was null or contained invalid data
}
is valid.
Aye. In that case it would be valid. But in my example we knew that at times the input could be null and at times it could contain an int. In that case we should check if it is null BEFORE we try to parse out an int to avoid throwing an exception when the str_value is null. In my example, a null value can be expected and so shouldn't throw an exception.

In your example, you are relying on an int being contained in the string (str_value MUST contain an int). Since the int MUST be there, an exception should be thrown if an int is not there (i.e. the string is null or contains text).

In my example, if the string was null i could return a value indicating a fail result, but i wouldn't use an exception to let the program know that the string was null.

bool ParseOutMyInt(string str_value)
try
{
if(str_Value != null)
result = Convert.ToInt32(str_value);
else
return false; // str_value is null, therefore not parsable

//do rest of code and return true for success
}
catch
{
return false; // it failed somewhere in the code.
}

Richard Berg
19th January 2006, 20:11
Guys, at the level you're worrying about, it's just a style preference.

Doom9
19th January 2006, 20:21
Form implemets IDisposabe interface. It means it hold unmanaged resources. Unmanaged resources can't be collected via garbage collector.
I think you're misreading the interface definition. Implementing IDisposable means you give the GC a method to call to make sure everything is being cleaned up. There's no need to call finalize for any of these classes listed if you are properly cleaning up after yourself. E.g. if you close the StreamWriter there's no need for using.. it just makes the close implicit. As long as your form doesn't hold a handle, or keeping streams/connections open, there's no problem with the garbage collection (at least I didn't find anything in the articles you linked to).

Where native resources are used, they should be properly cleaned up (but obviously nobody's perfect and I might have missed something.. but for instance the avi reader implements additional cleanup stuff because it goes native).

No, it can. Executable path is JobProcessor configuration.That's not the JobProcessor configuration.. it's the MeGUI configuration. I think it's pointless to argue about these interfaces.. they may not be final, but it's what it is now.. I'd really like you to commit some code instead of always trying to run headfirst against the wall that is me opposing changes all over the place. The whole job launching mechanism is bound to be changed in the future anyway.. and I don't like changes over and over. I only changed the encoding stuff because I needed to accomodate more encoders.. and bringing everything to the same level reacquainted myself with the code that I might have to modify in the future.

Since the risk is rather high that I'll do considerable parts of the refactoring in the future as well.. it really shouldn't be a cause to step on the brakes (which is what I feel you're doing.. everybody else here has commited some code for the current "broken" architecture). A major architectural overhaul is bound to take weeks if not months and I'm not the one to put development on hold just because things aren't very pretty right now.. they still work after all.

So I want to propose solution - let's open d2v files via AviSynth temporaly script file.No.. I love my dgdecode code...not only was it a major achievement for me (I've never written interop code without any help before), and wrapping it into AviSynth just adds another dependency. Your solution is what I consider the cheap way.. other softwares do that.. I prefer to do it the hard way. And why bother at all.. it works, it has never caused any problems. It seems just another instance where you want to run headfirst in a wall instead of letting things that work be. I've made the decision to wrap dgdecode a long time ago fully aware (and with already existing code to get frames via avifile) and I'm not about to reverse that now unless you come up with an awsome reason that blows me away.. the path thing is in my "I couldn't care less" category.

Doom9
19th January 2006, 20:24
Guys, at the level you're worrying about, it's just a style preference.That's what I'm saying.. let's keep thing that work the way they are. In the end, it's still my baby in a way so as long as I keep my fingers in MeGUI, you'll always have certain things that reflect how I approach a problem.. I have never used custom exceptions (well.. except when going through the mcad courses), so I'm unlikely to start now because I just don't like try/catch code and like to handle exceptions at low levels.

It's similar for the interfaces.. if I'm going to change them.. I'll have to change all classes implementing them because a CVS commit must not break compilation.. so it's my problem in the end and I can live with that.

Mutant_Fruit
19th January 2006, 20:35
Just put the new version of the ContextHelp.xml file (written by LiFe) up on SF. Also made a few changes to the contexthelp code to account for the fact that linebreaks are picked up from the XML correctly.

And i moved some checkboxes slightly as there was a bit of overlap.

Doom9
19th January 2006, 20:46
by the way, how would you like that I feed the cutlist to the avisynth based audio encoder? In the video world it's probably an array with two ints per line, a start and end frame. I figure just before encoding I open the AviSynth script, and put a trim line at the end of the script.

stax76
19th January 2006, 22:25
I think you're misreading the interface definition. Implementing IDisposable means you give the GC a method to call to make sure everything is being cleaned up. There's no need to call finalize for any of these classes listed if you are properly cleaning up after yourself.

I'm pretty clueless about the topic but do you confuse Dispose with Finalize? I think the GC calls Finalize but not Dispose. Using ensures Dispose is called under every circumstance, even when a exception happens similar like (try/catch) finally which is also executed in every circumstance (exception, break, goto...).

dimzon
20th January 2006, 09:41
there's no problem with the garbage collection (at least I didn't find anything in the articles you linked to).
FYI: Windows form holds window handles & GDI handles even if it was unvisible!
Really IDisposable.Dispose performs cleanup(line finalizer) + remove futher finalizer invokation via GC.SuppressFinalize().

Finalization and Performance

With this basic understanding of finalization we can already deduce some very important things:

First, objects that need finalization live longer than objects that do not. In fact, they can live a lot longer. For instance, suppose an object that is in gen2 needs to be finalized. Finalization will be scheduled but the object is still in gen2, so it will not be re-collected until the next gen2 collection happens. That could be a very long time indeed, and, in fact, if things are going well it will be a long time, because gen2 collections are costly and thus we want them to happen very infrequently. Older objects needing finalization might have to wait for dozens if not hundreds of gen0 collections before their space is reclaimed.

Second, objects that need finalization cause collateral damage. Since the internal object pointers must remain valid, not only will the objects directly needing finalization linger in memory but everything the object refers to, directly and indirectly, will also remain in memory. If a huge tree of objects was anchored by a single object that required finalization, then the entire tree would linger, potentially for a long time as we just discussed. It is therefore important to use finalizers sparingly and place them on objects that have as few internal object pointers as possible. In the tree example I just gave, you can easily avoid the problem by moving the resources in need of finalization to a separate object and keeping a reference to that object in the root of the tree. With that modest change only the one object (hopefully a nice small object) would linger and the finalization cost is minimized.

Finally, objects needing finalization create work for the finalizer thread. If your finalization process is a complex one, the one and only finalizer thread will be spending a lot of time performing those steps, which can cause a backlog of work and therefore cause more objects to linger waiting for finalization. Therefore, it is vitally important that finalizers do as little work as possible. Remember also that although all object pointers remain valid during finalization, it might be the case that those pointers lead to objects that have already been finalized and might therefore be less than useful. It is generally safest to avoid following object pointers in finalization code even though the pointers are valid. A safe, short finalization code path is the best.

FYI: Invoking IDisposable.Dispose() is good programming style anycase.



That's not the JobProcessor configuration... it's the MeGUI configuration.
Yes, Okay, I can call it
JobProcessor related MeGUI configuration is invalid
instead of
JobProcessor configuration is invalid

always trying to run headfirst against the wall that is me opposing changes all over the place
I'm afraid You get me wrong (my poor english can be a reason). I'm not forcing You to make any changes (and I'm not trying to do them myself) - it's just a discussion ;)

No.. I love my dgdecode code...not only was it a major achievement for me (I've never written interop code without any help before)

I understand your motivation (but I'm complete disagree with You).
and wrapping it into AviSynth just adds another dependency. Your solution is what I consider the cheap way...
Cheap != Wrong. And don't forget - You are using DGIndex.dll for AviSynth script generation - isn't it? Using DGIndex.dll is useless without AviSynth, so opening it via AviSynth is valid way.
the path thing is in my "I couldn't care less" category.
Just take look @ SettingsForm.cs. There are ugly code to work with PATH.
And how about DllHell (one version @ AviSynth plugin folder, another @ DGIndex folder, third @ MeGUI folder). Using AVS-way you are ensure - AviSynth plugin folder version are used!

dimzon
20th January 2006, 09:44
by the way, how would you like that I feed the cutlist to the avisynth based audio encoder? In the video world it's probably an array with two ints per line, a start and end frame. I figure just before encoding I open the AviSynth script, and put a trim line at the end of the script.
Just a question - What does start and end frame means for audio?

Doom9
20th January 2006, 09:59
Using ensures Dispose is called under every circumstance, even when a exception happens similar like (try/catch) finally which is also executed in every circumstance (exception, break, goto...).my code that makes access to resources where this could be a problem is structured such that any failures are handled. I came from C# from Java where there's no using, so I'm using a pattern that works on both platforms and cleans up after itself in all cases... exceptions included. So instead of arguing semantics.. either prove with current code where I'm not cleaning up properly, or drop it.. I'm about to lose my motivation to consider making any changes at all. you're back to the let's do everything different ways I told you in the beginning I was afraid of. Why don't you do something that actualy benefits the user like adding the avs based audio encoder and answering my question on how you want the cutlist..
It is a bad idea to take an existing project and change everything to your coding style.. what if the person whose style it corresponds leaves in midstream? What if you don't see it through? Don't answer that.. I've run a site for almost 6 years now and there's been only one other person I've been able to completely rely on.. all others have come and gone.. so that's what I expect will happen with MeGUI.. it's the open source approach.. people come and go.. but I know I'll stick around. Making changes just because they fit somebody's coding style better are useless changes for a project.

dimzon
20th January 2006, 10:09
either prove with current code where I'm not cleaning up properly, or drop it...
private void mnuToolsSettings_Click(object sender, System.EventArgs e)
{
#if FULL
SettingsForm sform = new SettingsForm(this.videoProfiles, this.audioProfiles,
this.videoProfile.SelectedIndex, this.audioProfile.SelectedIndex);
#else
SettingsForm sform = new SettingsForm();
#endif
sform.Settings = this.settings;
if (sform.ShowDialog() == DialogResult.OK)
{
this.settings = sform.Settings;
this.shutdownCheckBox.Checked = this.settings.Shutdown;
this.saveSettings();
// this is here to prevent output extension mismatches when the x264 encoder is changed
videoOutput.Text = changeVideoOutputExtention(videoOutput.Text);
}
}
sform still hold unmanaged GUI resources (windows handles, etc) until it be garbage collected. But it will no garbage collector invocation until deficit of memory. Deficit of memory != deficit of windows handles (.NET Runtime can't say - "ok, there are not enought GDI resources, let's collect garbage").

dimzon
20th January 2006, 10:11
Why don't you do something that actualy benefits the user like adding the avs based audio encoder and answering my question on how you want the cutlist..

I'm working @ audio right now && I was answered to Your question about cutlist.

Doom9
20th January 2006, 10:28
sform still hold unmanaged GUI resources (windows handles, etc)None of the articles you linked to ever said that.. it would be rather stupid for the .NET designer to by default have the most used GUI class to not be garbage collected by default. And code is no proof.. proff is tracking memory usage.. it's quite hard to prove memory waste in a garbage collected environment.. but if you want to change my mind, that's what you need to do.
I've never seen a form sample using using, and I've seen many samples using streamreader using my approach at cleaning up .

ust a question - What does start and end frame means for audio?That's what I wanted to know from how.. what do you want in order to write the proper avisynth script? I expect that the encoder takes care of that given a cutlist in a format it understands. If you have an FPS along with framenumbers, you could translate it into timecodes.. would that work.. or do you prefer to have timecodes to begin with (and if so in which format). Here's your chance to have things your way for once.. take it ;)

dimzon
20th January 2006, 11:03
None of the articles you linked to ever said that.. it would be rather stupid for the .NET designer to by default have the most used GUI class to not be garbage collected by default.
Seem's like You don't understand me at all. No, it garbage collected by default(at this point finalizer are used) BUT garbage collector invocation time is undeterminied. Just try to understand - Garbage Collector will be invoked only when you try to allocate memory (create new object) && there are not enought memoty @ managed pool. But it does'nt affect on other resources like GDI objects. It means if You still have enought memory @ managed pool && you still can get out of resources! If You don't trust me just take look @ System.Windows.Forms.Control (base for all winform controls including Form itself) decompiled code:

public void Dispose()
{
this.Dispose(true);
GC.SuppressFinalize(this);
}

~Component()
{
this.Dispose(false);
}

protected override void Dispose(bool disposing)
{
if (this.GetState(0x200000))
{
object obj1 = this.Properties.GetObject(Control.PropBackBrush);
if (obj1 != null)
{
IntPtr ptr1 = (IntPtr) obj1;
if (ptr1 != IntPtr.Zero)
{
SafeNativeMethods.DeleteObject(new HandleRef(this, ptr1));
}
this.Properties.SetObject(Control.PropBackBrush, null);
}
}
if (disposing)
{
if (this.GetState(0x1000))
{
return;
}
if (this.GetState(0x40000))
{
object[] objArray1 = new object[] { "Dispose" } ;
throw new InvalidOperationException(SR.GetString("ClosingWhileCreatingHandle", objArray1));
}
this.SetState(0x1000, true);
try
{
this.DisposeAxControls();
ContextMenu menu1 = (ContextMenu) this.Properties.GetObject(Control.PropContextMenu);
if (menu1 != null)
{
menu1.Disposed -= new EventHandler(this.DetachContextMenu);
}
if (this.window.Handle != IntPtr.Zero)
{
this.DestroyHandle();
}
if (this.parent != null)
{
this.parent.Controls.Remove(this);
}
Control.ControlCollection collection1 = (Control.ControlCollection) this.Properties.GetObject(Control.PropControlsCollection);
if (collection1 != null)
{
for (int num1 = 0; num1 < collection1.Count; num1++)
{
Control control1 = collection1[num1];
control1.parent = null;
control1.Dispose();
}
this.Properties.SetObject(Control.PropControlsCollection, null);
}
base.Dispose(disposing);
return;
}
finally
{
this.SetState(0x1000, false);
this.SetState(0x800, true);
}
}
if ((this.window != null) && (this.window.Handle != IntPtr.Zero))
{
UnsafeNativeMethods.PostMessage(new HandleRef(this.window, this.window.Handle), 0x10, 0, 0);
this.window.ReleaseHandle();
}
base.Dispose(disposing);
}


Conclusion:
Settings form consist of more than 30 controls. It means every time you use it you will got unclosed GDI resources && at lest 30 unrequed object to be promoted to gen2 (finalizer effect).

And code is no proof.. proff is tracking memory usage...
Ok, if You still need it even afer decompiled code I can do it!

I've never seen a form sample using using, and I've seen many samples using streamreader using my approach at cleaning up .
Actually it means You got bad samples ;)


That's what I wanted to know from how.. what do you want in order to write the proper avisynth script? I expect that the encoder takes care of that given a cutlist in a format it understands. If you have an FPS along with framenumbers, you could translate it into timecodes.. would that work.. or do you prefer to have timecodes to begin with (and if so in which format). Here's your chance to have things your way for once.. take it ;)
Just one stupid question before - can you provide me usecase of it?

Doom9
20th January 2006, 11:19
can you provide me usecase of it?You capture from TV, and you want to cut out ads. Editing the video frame accurately is no problem, but the problem is make the audio match.. there's no way to cut at the exact points where the video was cut. So the way to have audio properly match video would be to decode the audio to wav, then cut using a wav editor, then encode to the final format. Using avisynth based audio encoding, we can skip the decode to wav and separate editing step by directly trimming the avisynth script. I have no problem doing that for the video avisynth script.. but I have no idea how it applies to your audio only avisynth scripts.

Actually it means You got bad samples What is bad in something like this:

StreamReader sr = null;
try
{
sr = new StreamReader("somefilepath");
string line = "";
while ((line = sr.ReadLine()) != null)
Console.WriteLine(line);
}
catch (Exception e)
{
Console.WriteLine("An exception has ocurred: " + e.Message);
}
finally
{
if (sr != null)
{
try
{
sr.Close();
}
catch (Exception x)
{
Console.WriteLine("an exception ocurred when closing the streamreader: " + x.Message)
}
}
}

This does just the same as
using(StreamReader sr = new StreamReader("somefile"))
{
string line = "";
while ((line = sr.ReadLine()) != null)
{
ConsoleWriteLine(line);
}
}

The latter is just a bit shorter (and I suspect there must be some error handling as well.. or does using catch those as well and what does it do with them?
Functionality wise, the first code bit ensures that the streamreader is in a state that it can be garbage collected because the ensured call to close ensures that any non managed references are properly disposed of.

dimzon
20th January 2006, 11:31
You capture from TV, and you want to cut out ads. Editing the video frame accurately is no problem, but the problem is make the audio match.. there's no way to cut at the exact points where the video was cut. So the way to have audio properly match video would be to decode the audio to wav, then cut using a wav editor, then encode to the final format. Using avisynth based audio encoding, we can skip the decode to wav and separate editing step by directly trimming the avisynth script. I have no problem doing that for the video avisynth script.. but I have no idea how it applies to your audio only avisynth scripts.
Hmmm. I can propose a little another algorytm. Create such script:

# Get audio && video
video = ...
audio = ...
# Dub them
audioDub(video,audio)
# Perform trimming
trim(....)

atfer this you can use this script as video source && as audio source both ;)


try
{
using(StreamReader sr = new StreamReader("somefile"))
{
string line = "";
while ((line = sr.ReadLine()) != null)
{
ConsoleWriteLine(line);
}
}
}
catch(Exception e)
{
Console.WriteLine(e.Message);
}

Both code are equal bcz:

public override void Close()
{
this.Dispose(true);
}

void IDisposable.Dispose()
{
this.Dispose(true);
}

Doom9
20th January 2006, 11:42
atfer this you can use this script as video source && as audio source bothI thought of that but the problem's are glaring: what if the user only wants to encode audio? I'm afraid manual mode means separated encoding. Your approach only works in case both audio and video are being encoded.. there's also the option on automated or one click mode where audio is only being muxed.. creating yet another problem because audiodub needs two parameters.

And doesn't Form.Close call Dispose?

dimzon
20th January 2006, 11:51
I thought of that but the problem's are glaring: what if the user only wants to encode audio? I'm afraid manual mode means separated encoding. Your approach only works in case both audio and video are being encoded.. there's also the option on automated or one click mode where audio is only being muxed.. creating yet another problem because audiodub needs two parameters.
Let's think together step by step... If user want oly to encode audio nobody force him to encode video but using both (audio & video) @ script allows him to be sync. Does you need trimming @ one click mode - no. Does you need trimming @ automated mode - I don't think so... Correct me if I'm wrong.

And doesn't Form.Close call Dispose?
Really no, bcz you can ReOpen form after closing ;)
public void Close()
{
if (base.GetState(0x40000))
{
object[] objArray1 = new object[] { "Close" } ;
throw new InvalidOperationException(SR.GetString("ClosingWhileCreatingHandle", objArray1));
}
if (base.IsHandleCreated)
{
base.SendMessage(0x10, 0, 0);
}
}

Doom9
20th January 2006, 12:05
Let's think together step by step... If user want oly to encode audio nobody force him to encode video but using both (audio & video) @ script allows him to be sync. Does you need trimming @ one click mode - no. Does you need trimming @ automated mode - I don't think so... Correct me if I'm wrong.Correction coming your way:
If user want oly to encode audio nobody force him to encode videoYes.. with audio only there's no trimming.

Does you need trimming @ one click mode - noYes you do.. it's definitely not possible in the current workflow as it requires manual interruption, but we have to come up with an intelligent way to handle that.. TV capture mostly means TS/PVA/MPG these days.. all that will go through DGIndex.. so indexing should run right away, then the user is prompted for editing after that (depending on filters we could theoretically load certain files via AviSynth's DirectShowSource.. but that has proven to be both instable, and it doesn't allow frame accuracy.. most DS parsers I'm afraid to say don't properly implement seeking by frame properly).

Does you need trimming @ automated mode - I don't think so... Yes you do.. but not necessarily.. both must be possible. For instance you could decide to keep an eventual AC3 (or even AAC) stream from the digital broadcast.. then there's no editing. But if you re-encode the audio, then there can be editing (not necessarily.. you may have captured an ad-free show / movie).

Really no, bcz you can ReOpen form after closingUmm.. uh, then why did I have to rewrite the progress window, including override close to make sure close it only called at the end of a job. Initially, pressing X would trigger the close. I still had the reference to the form, but when I tried to reopen it. I was getting an exception.. that's why I use show/hide now.

dimzon
20th January 2006, 13:58
@Doom9
Please, take look @ http://www.mytempdir.com/396614 - modified sources with Audio via AVS encoder (DimzonEncoder @ AudioEncoder.cs)
It's a little crap (reques refactoring/cleanups) but working...
For Nero7 encoding you need BeHappy.Extension.Encoder.Nero7AAC.exe from BeHappy

PS. I will not commit it to CVS until Your approval

Doom9
20th January 2006, 14:22
A question on the "using" statement. Right now I'm working with sql connections a lot.. using the pattern I outlined for the streamwriter. I'm also using transactions since I'm often running multiple statements that need to be executed all, or none. So using rids me of try/finally and manually closing.. however, what do you about the transaction? Basically I want to commit when all statements have been successfully executed, plus sometimes other conditions (like the statement returning something.. ).. when there's an exception.. it will be handled automatically and I'll break out of the using part. However, who makes sure if this happens my transaction is rolled back? And if an exception happens.. it's automatically caught.. but I need to send it to a tracewriter as well.. so how does the using approach if I can't put the commit/rollback in the finally and the tracewriter statement into the catch block?

dimzon
20th January 2006, 14:35
A question on the "using" statement. Right now I'm working with sql connections a lot.. using the pattern I outlined for the streamwriter. I'm also using transactions since I'm often running multiple statements that need to be executed all, or none. So using rids me of try/finally and manually closing.. however, what do you about the transaction? Basically I want to commit when all statements have been successfully executed, plus sometimes other conditions (like the statement returning something.. ).. when there's an exception.. it will be handled automatically and I'll break out of the using part. However, who makes sure if this happens my transaction is rolled back? And if an exception happens.. it's automatically caught.. but I need to send it to a tracewriter as well.. so how does the using approach if I can't put the commit/rollback in the finally and the tracewriter statement into the catch block?

System.Data.SqlCient.SqlTransaction:

private void Dispose(bool disposing)
{
if (disposing && (this._sqlConnection != null))
{
this._disposing = true;
this.Rollback();
}
}
public void Rollback()
{
if (this._sqlConnection == null)
{
throw ADP.TransactionZombied(this);
}
try
{
this._sqlConnection.ExecuteTransaction("IF @@TRANCOUNT > 0 ROLLBACK TRANSACTION", "RollbackTransaction");
this.Zombie();
}
catch
{
if ((this._sqlConnection != null) && (this.GetServerTransactionLevel() == 0))
{
this.Zombie();
}
if (!this._disposing)
{
throw;
}
}
}


You can use as many try/catch insude/outside using() as you want.
And yes, if you need atomic transactions it's safe to use using().
You still need try/catch block around every statement if you want to send detailed info in traicewriter. Unfortunally I does'nt know why to do it (i believe - exception callstack is enought to understand where error occured)

Doom9
20th January 2006, 14:42
You still need try/catch block around every statement if you want to send detailed info in traicewriter. Unfortunally I does'nt know why to do it (i believe - exception callstack is enought to understand where error occured)I ended up doing that.. so while using gets rid of try/catch you have to reintroduce it.. thus severely reducing the code amount savings, and in the end it comes down to syntax. It may be good to teach a n00b to use using because he might forget about try/catch/finally and cleaning up after himself.. but it's what has been drilled into us at university. and in java there's no using so in java you need to write it the way I'm usually writing such code.
What happens with the exception that can be thrown in Dispose? Do you need to wrap a try/catch around the using to get it?

dimzon
20th January 2006, 14:43
about Form.Close() - it's invoke Dispose asyncronically via PostMessage to it's own handle, sorry...

But anycase using() for every IDisposable is a good programming style :)

dimzon
20th January 2006, 14:46
What happens with the exception that can be thrown in Dispose? Do you need to wrap a try/catch around the using to get it?
Yes it can be thrown and you catch it by outer try/catch block. But throwng exception @ Dispose is a very bad style. IDisposable.Dispose means: cleanup all and die :)

The Link
20th January 2006, 14:56
@Doom9
Please, take look @ http://www.mytempdir.com/396614 - modified sources with Audio via AVS encoder (DimzonEncoder @ AudioEncoder.cs)
It's a little crap (reques refactoring/cleanups) but working...
For Nero7 encoding you need BeHappy.Extension.Encoder.Nero7AAC.exe from BeHappy

PS. I will not commit it to CVS until Your approval
Sorry for posting in this thread but I have a stupid question to dimzon: Will you add support for avs (serving an audio stream) input in the audio section?

dimzon
20th January 2006, 15:01
Sorry for posting in this thread but I have a stupid question to dimzon: Will you add support for avs (serving an audio stream) input in the audio section?
Yes :) There are *.* filemask now :)
This streams will be included via Import() until you check "Force Decoding via Directshow" @ this case DirectShowSource will be used

PS. This source is for Doom9 only :) Nobody will not support unapproved by Doom9 code!

Doom9
20th January 2006, 15:01
Yes it can be thrown and you catch it by outer try/catch block. But throwng exception @ Dispose is a very bad style.But since there's no catch in using, if one of the regular commands inside the using block throws an exception, you still have to wrap each using block with a try / catch (or you put it elsewhere.. me I like to catch exceptions at the lowest level). So essentially it's a syntactical difference that comes down to convenience.. I do it the java way because that's what I learned before C#, and since I still use Java a lot, it's probably not a bad thing to keep your coding style so that you can seamlessly switch between languages.. if you get used to using too much, you might end up forgetting the cleanup parts in Java and leave connections open and the likes.

dimzon
20th January 2006, 15:05
I like to catch exceptions at the lowest level
As for me i like to catch them @ highest possible level - that's the difference. It's enought callstack to find where exception occured. So I prefer to keep my code as much easy as it possible ;)

PS. I'm waiting for You reaction @ my modified code

Doom9
20th January 2006, 15:14
PS. I'm waiting for You reaction @ my modified codeI'm still at work.. that using discussion can be seen as part of reflection on what I do on the workplace.. but megui code is something else ;)

Richard Berg
20th January 2006, 15:14
You don't ever want to catch an exception unless you're ready to handle it -- the whole point of exceptions is that they let you bubble up to the correct stack frame automatically.


// bad bad bad
try
{
SomeThing();
}
catch (Exception)
{
return "something didn't work";
}


(Yes, that's what I did in verifyOutputFilename(), but that's because the rest of MeGUI isn't set up to handle exceptions properly and I didn't want to make a major change.)

dimzon
20th January 2006, 15:16
You don't ever want to catch an exception unless you're ready to handle it -- the whole point of exceptions is that they let you bubble up to the correct stack frame automatically.


// bad bad bad
try
{
SomeThing();
}
catch (Exception)
{
return "something didn't work";
}


Agreed by 100%

Inc
20th January 2006, 15:17
Dear Doom9.
MeGUI invokes DGDecode.dll direcly. It causes such problem:
DgDecode.dll must be im MeGUI folder or be avaluable @ PATH enviroment string.
In other hand DGDecode.dll MUST be @ avisynth\plugins folder in order to be able open scripts.
So I want to propose solution - let's open d2v files via AviSynth temporaly script file.

Calling DgDecode via a temp. avs script is not needed the mpeg2Source() API call serves all needed informations to a common Avisynth VideoInfo structure.
Accessing/recognising the dgdecode.dll is also possible if the dll itself isn't present in the actual main appl. folder. You can parse the Avisynth entry in the systems registry where the full path to the installed avisynth plugins dir is given.
It could be used to obtain the full path to dgdecode.dll.
And if it isn't found (however) then an else operation could look into the curr. appl. folder. Thats how PARanoia does it.

Richard Berg
20th January 2006, 15:18
Here's a way to use Trim() with audio-only clips. You just need the framerate:

# MeGUI needs to write these lines
fps = 30000.0 # .0 at the end ensures the script parser does floating-point math
fps_denom = 1001.0
srcfile = "ncstate.wav"
trimstart = 500
trimend = 600

wav = WavSource(srcfile)
AudioDub(BlankClip(height=4, width=4, length=int((wav.audiolengthf / wav.audiorate) * (fps / fps_denom))+1,
\ fps=int(fps), fps_denominator=int(fps_denom)), wav)
Trim(trimstart,trimend)



BTW, worrying about deterministic finalization is silly in MeGUI. 30 GDI handles is not important the way a database or socket is.

dimzon
20th January 2006, 15:30
Accessing/recognising the dgdecode.dll is also possible if the dll itself isn't present in the actual main appl. folder. You can parse the Avisynth entry in the systems registry where the full path to the installed avisynth plugins dir is given.
It could be used to obtain the full path to dgdecode.dll.
And if it isn't found (however) then an else operation could look into the curr. appl. folder. Thats how PARanoia does it.
Unfortunally it's impossible to do in anything except Managed C++. You caninvoke LoadLibrary("full dll path") in C#, You can ever call GetProcAddress but You can't call exported procedures at all (you can't assign obtained via GetProcAddress to procedure). There are 2 possible worarounds: write proxy in Managed C++ or use something other (read: AviSynth) that can act as a proxy

Here's a way to use Trim() with audio-only clips. You just need the framerate
Yes, I know this method, i'm using same @ BeHappy (check it).
Unfortunally i'm afraid overflow @ very long audio streams length=int((wav.audiolengthf / wav.audiorate) * (fps / fps_denom))+1

dimzon
20th January 2006, 15:40
@Doom9
I'm sorry, don't get me wrong, I dont push You, but I want to know what does You think about DllHell using DGDecode.dll
By the way, if we decide to switch to AviSynth there are 2 ways to solve it
1) Do nothig, DGDecode from aviSynth\Plugins folder will be used
2) Add LoadPlugin("<DgIndexFolder>\DgDecode.dll") as first line @ scripts

Richard Berg
20th January 2006, 15:53
@dimzon -- does it overflow with >2 billion samples even if you use audiolengthf? I forget how it's stored internally.

dimzon
20th January 2006, 16:03
@dimzon -- does it overflow with >2 billion samples even if you use audiolengthf? I forget how it's stored internally.

for audiolengthf it can drop accuracy...

Doom9
20th January 2006, 16:33
(Yes, that's what I did in verifyOutputFilename(), but that's because the rest of MeGUI isn't set up to handle exceptions properly and I didn't want to make a major change.)And what would you propose instead? Isn't is so in Java that if you perform an operation that can cause an exception, you need to make sure it's handled (and rethrown if you don't want to bother with it)?
I do have all uncatched exceptions caught on the application level since a few builds. If you do something bad and you know it can go wrong, wouldn't it make sense to place the logic to handle what can go wrong right there at the point where you know exactly which operation you're dealing with? If you're not doing that, in order to identify where the exception belongs, you need to add your own custom exceptions, so catch and rethrow.. and that sounds like a lot of wasted effort to me.

I'm sorry, don't get me wrong, I dont push You, but I want to know what does You think about DllHell using DGDecode.dllDidn't I write "I don't care" already? I just don't care, period.

Richard Berg
20th January 2006, 16:50
I'm not sure which part of the code you're talking about right now. It depends on the situation.

In verifyOutputFilename it's not ideal, but ok because:
(1) the complete exception info is returned to callers
(2) it's not hard to make sure all of the methods that call it always stop execution at the right place & error out

Part 2 is already causing a little trouble (see bug report thread) - it's not as easy as I thought to figure out when the callers need to error - apparently blank input audio filenames are sometimes ok.

It's bad design if it causes your callers to look like this:
[code]
bool err = Method1();
if (err)
handleProblem();

err = Method2();
if (err)
handleProblem();

err = Method3();
if (err)
handleProblem();
[/quote]
That's what old C code looks like, and it's exactly what people wanted to avoid when they added exceptions to C++ and later Java/C#.

C# learned from Java -- checked exceptions was a nice idea, but in practice it's worse to handle an exception you shouldn't than to forget to handle one you should. It made people stick catch(everything) in places they shouldn't just so they could get the darn thing compiling during quick development, then forget to remove them in final code.

It's even worse if you violate Part 1. If your application "hides" exceptions it becomes much much harder to debug later on. When consultants are hired to help fix old buggy codebases nobody can figure out, the first thing they do is attach a debugger set to break on all thrown exceptions (default setting in windbg is for uncaught only, the problem Java designers were wrongly worried about) because missing the former loses so much helpful info.

Inc
20th January 2006, 17:00
Sorry for again getting to this C#-coding related (less MeGUI related) issue:
@Dimzon
I Hope that I didn't catch something totally wrong:
Unfortunally it's impossible to do in anything except Managed C++. You caninvoke LoadLibrary("full dll path") in C#, You can ever call GetProcAddress but You can't call exported procedures at all (you can't assign obtained via GetProcAddress to procedure).
?
1. Loading a Library
2. Calling (using cdecl) the given dll Function via its API function name string. There`s also a stdcall wrapper inside dgdecode.dll if C# doesnt support the cdecl calling convention.
(2b. ... or via a function pointer out of the obtained proc.adress)
3. Call mpeg2Source(...) incl. the d2v file name string and the pointer to an VideoInfo Structure.
4. Voilá

Im shure that this will work in C# ;)

OffTopic /off

dimzon
20th January 2006, 17:09
Sorry for again getting to this C#-coding related (less MeGUI related) issue:
1. Loading a Library
2. Calling (cdecl) the given dll Function via its API function name string. There`s also a stdcall wrapper inside dgdecode.dll if C# doesnt support the cdecl calling convention.
(2b. ... or via a function pointer out of the obtained proc.adress)
3. Call mpeg2Source(...) incl. the dsv file name string and the pointer to an VideoInfo Structure.
4. Voilá

OffTopic /off
Dear incredible, unfortunally this way doesn't work @ C#.

lib = LoadLibrary("lib.dll"); // this will work in C#
someFunctionAddress = GetProcAddress("SomeProcName"); //this will work in c# if someFunctionAddress type is int or IntPtr
myProc = someFunctionAddress ; myProc(); // this is impossible in c#

Doom9
20th January 2006, 17:12
Part 2 is already causing a little trouble (see bug report thread) - it's not as easy as I thought to figure out when the callers need to errorI thought you said the check would be done before entering the autoencode window? I guess it's a debateable point if an incorrect stream is silently discarded (I believe that's what I did but it's been half a year), or if a warning is shown and the dialog doesn't open.. but I don't think it would be a good idea to just error out once the user has configured everything in the dialog.. that would mean all the configuration was for nothing because the user has to go back to the main screen.

And as far as your bad sample goes.. I would think you'd find a way to abort after the call to Method 1. E.g. I break out immediately from loops when something isn't quite right.
I see exceptions mostly in the "I'm going to use this library of which I know nothing but the API" case.. when I know what's going to happen, why shouldn't I handle it right there and return a meaningful warning? E.g. the stdout parsing in the encoders.. they used to be a source of great pain.. now as it is.. if something breaks beause the output of encoder X has changed again, the application will still work.. perhaps some onscreen indicator will break, but in such a case, the log will tell there was an exception in method X. Come to think of it, dumping the line variable would make sense as well.. so if the syntax changes, something breaks, the user posts a logfile, I see the offending line and can reproduce the problem without even having to start an encoding session. You can't do that if you handle the exception someplace higher up the foodchain.. plus, if you do that, say just in the MeGUI class, wouldn't the encoder stop working at the point when an exception is thrown?

dimzon
20th January 2006, 17:50
I have take look @ d2vReader.cs.
WOW! I found a great woraround here:
this.lib = LoadLibrary(this.dgDecodePath); - just load library first. Very nice!

to Doom9, sorry, I was mistaken about path problem, everything work fine

dimzon
20th January 2006, 18:01
http://www.mytempdir.com/397199 - fresh sources for AVS audio input

godhead
20th January 2006, 18:13
It is a bad idea to take an existing project and change everything to your coding style.. what if the person whose style it corresponds leaves in midstream? What if you don't see it through? Don't answer that.. I've run a site for almost 6 years now and there's been only one other person I've been able to completely rely on.. all others have come and gone.. so that's what I expect will happen with MeGUI.. it's the open source approach.. people come and go.. but I know I'll stick around. Making changes just because they fit somebody's coding style better are useless changes for a project.

These are the exact reasons that I'm sticking to smaller bug fixes and feature requests. I'd rather leave the main development and features up to the main developer (Doom9 in this case). I look to that developer as being the architect and I just follow along with their plans and pre-defined style. If I see something majorly wrong, then I might suggest something for consideration. I avoid trying to step on anyone's toes.

This is the reason I bought up the code fork approach if you're really set on doing a refactoring of any significance.

Just my opinion, back to doing actual real life work so I can get some free time to do the MeGUI changes I've got assigned.

godhead
20th January 2006, 18:29
Dear incredible, unfortunally this way doesn't work @ C#.

lib = LoadLibrary("lib.dll"); // this will work in C#
someFunctionAddress = GetProcAddress("SomeProcName"); //this will work in c# if someFunctionAddress type is int or IntPtr
myProc = someFunctionAddress ; myProc(); // this is impossible in c#

FYI -> http://www.codeproject.com/csharp/dyninvok.asp

Richard Berg
20th January 2006, 18:58
I thought you said the check would be done before entering the autoencode window?
Forget half a year, I don't even remember what my change from last week did...will investigate after work :)

I would think you'd find a way to abort after the call to Method 1
If you don't handle the exception, your method will surely abort.

I see exceptions mostly in the "I'm going to use this library of which I know nothing but the API" case
Again, depends on the situation. I think your stdout handlers are fine. The interface between the parser and the main MeGUI window consists of a log string, which is a fine place to bubble up error messages.

OTOH, let's say you had a call stack looked like:
SomeDialog()
- ValidateInput()
--- ValidateFirstTextbox()
----- CalculateBitrate()
------- UnknownMathLibrary.Multiply()

Putting a try/catch block in CalculateBitrate() just because you're calling into an unknown library that might throw something -- or worse, lots of try/catches like in my earlier sample in one of the intermediate methods -- would be bad design. It's easier to write and much easier to debug if you do:

void SomeDialog_callback()
{
...
try {
ValidateInpute();
}
catch(ArgumentNullException e)
{
...
}
...other exceptions you know are common to hit somewhere in validation...
catch(Exception e)
{
// if you catch general Exception, make sure you do something with e.Message()
// and e.StackTrace too in debug builds...
}
}

stax76
20th January 2006, 19:36
frame accurate cutting based on AviSynth.. once audio encoding uses AviSynth that'll be a realitiy and that is a strong USP for MeGUI.


Turned out to require only trivial changes in StaxRip which supported cutting only using VirtualDubMod so far so next version will allow also cutting regardles of what encoder and muxer is used.

@Dimzon

Do you mind me using your BePipe with StaxRip? There is no license included.

I want to use the normal AVS script which also includes video. Is this gonna be much slower?

dimzon
20th January 2006, 19:46
@Dimzon
Do you mind me using your BePipe with StaxRip? There is no license included.
I want to use the normal AVS script which also includes video. Is this gonna be much slower?
Feel free to do it. Use encoder.cs from BeHappy - BePipe has some bugs.

stax76
20th January 2006, 20:49
:thanks:

Sharktooth
20th January 2006, 21:02
CVS Update:
0.2.3.2035
Fixed some x264 CC warnings

0.2.3.2034
Encoders paths have now their own tab in Settings (changes by dimzon)

@dimzon: please update the assemblyinfo version and the changelog the next time you commit some changesl, thanks:)

Doom9
20th January 2006, 21:40
Putting a try/catch block in CalculateBitrate() just because you're calling into an unknown library that might throw something I wouldn't use try/catch unless I knew the library would thrown an exception.. if it's not part of the API description then the normal thing to do (imho) is assume there are no exceptions or that whatever might be caused will be handled internally.. so the api description would mention what exception can be thrown, or return a value of -1 (as an example) to indicate something has gone wrong internally.

And by the way, about the dgdecode path.. it stands to reason that those opening d2v's in MeGUI have created their d2v's in MeGUI.. and unpacked the entire zip to a directory and configured the dgindex path there.. plus then copied the dll. So having it two places would be nothing special but rather the standard scenario.

Doom9
20th January 2006, 22:03
@dimzon: when I compile I get two warnings in AudioEncoder telling me the Thread.Resume/Suspend is back? Didn't I eliminate that with my latest CVS update? I cannot open any but the baseAudioConfigurationDialog in the GUI designer.. once again, choice or error? I thought visual inheritance wouldn't deny the ability to use the GUI designer. Here's the full error message I'm getting:

The designer could not be shown for this file because none of the classes within it can be designed. The designer inspected the following classes in the file: lameConfigurationDialog --- The base class 'MeGUI.baseAudioConfigurationDialog' could not be loaded. Ensure the assembly has been referenced and that all projects have been built.

Where is the distinction made if avisynth or besweet is to be used for audio encoding and where can this be configured?

And perhaps most importantly.. where do I get neroraw.exe?

Another thing: the FAAC config dialog seems to default to CBR but doesn't have a bitrate set.. it should have 128 kbit/s preset. And should it say kbit/s at least for CBR and something for VBR as well? Okay, that's my fault actually.. seems the original didn't have those indications either.

last but not least.. I can't seem to encode anything.. here's the log

The current job contains errors. Skipping chained jobs
Starting job job1 at 22:02:54
Job is an audio job. Commandline:
-core( -input "D:\DVDs\DVDVolume\VIDEO_TS\residentevil AC3 T01 3_2ch 448Kbps DELAY 0ms.ac3" -output "D:\DVDs\DVDVolume\VIDEO_TS\residentevil AC3 T01 3_2ch 448Kbps DELAY 0ms.mp4" -logfile "D:\DVDs\DVDVolume\VIDEO_TS\residentevil AC3 T01 3_2ch 448Kbps DELAY 0ms.besweet.log" ) -azid( -cbr 128 ) -ota( -g max )
successfully started encoding
Processing ended at 22:02:54
----------------------------------------------------------------------------------------------------------

Log for job job1

Error:
System.ApplicationException: Can't find audio stream!
at MeGUI.DimzonEncoder.encode()And that's bs.. the audio stream is properly configured.

Oh, the new jobs are also not being loaded upon startup.. and the commandline (not shown...), seems to be the besweet commandline

@update: debugging the code I found that the settings object is not being properly propagated.. the DimzonEncoder seems to think my faac.exe has no path associated with it even though I've properly configured it.

Richard Berg
20th January 2006, 22:08
so the api description would mention what exception can be thrown, or return a value of -1 (as an example) to indicate something has gone wrong internally.

Doesn't matter. You still wouldn't want to put a try/catch there. In fact, you might consider throwing an exception if you received -1 from the API.

Richard Berg
20th January 2006, 22:09
@anyone -- what does setting the Avisynth plugin dir in Settings -> Path do? Any reason we shouldn't pull it from the registry?

Doom9
20th January 2006, 22:23
I also believe you got lame's cbr and abr mixed up:
case BitrateManagementMode.ABR:
m_commandLine = "-b " + m.Bitrate + " --cbr -h --silent - \"{0}\"";
break;
case BitrateManagementMode.CBR:
m_commandLine = "--abr " + m.Bitrate + " -h --silent - \"{0}\"";
break;
--abr smells like abr, not cbr..

And what's up with if (priority == ProcessPriority.IDLE)
m_encoderThread.Priority = ThreadPriority.Lowest;
There's a ThreadPriority.Idle, isn't there?

Doom9
20th January 2006, 22:31
and I have no clue how to debug this thing.. I set a breakpoint in your encoder start, it starts a thread with threadstart parameter encode so I put a breakpoint in the encode method, only to find it never gets there..
Are you cleaning up after the encoder? Because if you create a file in the temp folder.. you better.. the automated cleanup after encoding isn't capable of finding that file. I actually meant to suggest that the file have the same name as the audio, just with avs appended.. so you clearly know whom it belongs to.

Also, as far as audio input goes.. enabling everything isn't such a good idea.. most filetypes aren't audio so it should only support those that we can handle.

Also, there's already a class wrapping AVIFile.. it would make sense that you use this one and make changes where needed (without breaking current functionality of course.. make sure you test encoding and avisynth script preview)

And some more things: I think your encoder should derive from audioencoder, or at least be called from it so there's a common path it goes through.. just like the 3 video encoders and 3 muxers use a common class. That also allows easy switching of encoders.

How about using the commandlinegenerator to generate encoder commandlines?

Last but not least I have to ask the dreaded question "how well did you test this?" I mean.. encoding doesn't even get to the point where it would fail because the encoder paths are not being propagated..

dimzon
21st January 2006, 10:46
@dimzon: please update the assemblyinfo version and the changelog the next time you commit some changesl, thanks:)
Hi, happy to see your health is better.
Isn't possible to get changelog automatically via CVS?

dimzon
21st January 2006, 11:49
when I compile I get two warnings in AudioEncoder telling me the Thread.Resume/Suspend is back?
Don't worry I will eluminate it later. This is my code from BeHappy.
By the way
Are you shure Your current encoder.pause implementation really pauses external process? I do not think so, maybe I'm wrong? You only pauses StdOut/StdErr reading from it, isn't it. It means encoding still work...

I cannot open any but the baseAudioConfigurationDialog in the GUI designer.. once again, choice or error?
I have yet checked it - everything is fine? Maybe this error is VS Express (I'm using full version now)

The designer could not be shown for this file because none of the classes within it can be designed. The designer inspected the following classes in the file: lameConfigurationDialog --- The base class 'MeGUI.baseAudioConfigurationDialog' could not be loaded. Ensure the assembly has been referenced and that all projects have been built.
Does you compile project in VS before???

Where is the distinction made if avisynth or besweet is to be used for audio encoding and where can this be configured?
This is not exists yet. If you want to switch to BeSweet just replace aEnc= new DimzonEncoder() by aEnc= new AudioEncoder() @ Form1.cs


And perhaps most importantly.. where do I get neroraw.exe?

As I said before you need BeHappy.Extension.Encoder.Nero7AAC.exe from BeHappy - use it instead of neroraw. I plan to remove BeHappy-related code from it && rename to neroraw a bit later.

Another thing: the FAAC config dialog seems to default to CBR but doesn't have a bitrate set.. it should have 128 kbit/s preset. And should it say kbit/s at least for CBR and something for VBR as well? Okay, that's my fault actually.. seems the original didn't have those indications either.
Will be fixed later

last but not least.. I can't seem to encode anything.. here's the log
It means there are error @ avisynth script. You need NicAudio plugin for ac3/dts, MPASource for mp3

Oh, the new jobs are also not being loaded upon startup.. and the commandline (not shown...), seems to be the besweet commandline
I do not touch job-related commadline generation, your old code invoked there

debugging the code I found that the settings object is not being properly propagated.. the DimzonEncoder seems to think my faac.exe has no path associated with it even though I've properly configured it.
No, it does...

I also believe you got lame's cbr and abr mixed up
Thanx, Fixed

And what's up with if (priority == ProcessPriority.IDLE)
m_encoderThread.Priority = ThreadPriority.Lowest;
There's a ThreadPriority.Idle, isn't there?
No, look @ enum ThreadPriority@ MSDN - there are no ThreadPriority.Idle

and I have no clue how to debug this thing...
Place breakpoint @ DimzonEncoder::encode - everything is fine (I'm using VS2005 Proffessional Edition).

Are you cleaning up after the encoder? Because if you create a file in the temp folder..
Yes! Definitly everything is cleaned up (even uncomplete output if exception is occured)

I actually meant to suggest that the file have the same name as the audio, just with avs appended...
No. Name is obtained like:
System.IO.Path.GetTempPath() + "encode-" + Guid.NewGuid().ToString("N") + ".avs";

I'm strongly prefer using Guid's to avoid ANY conflicts.

Also, as far as audio input goes.. enabling everything isn't such a good idea.. most filetypes aren't audio so it should only support those that we can handle.

Really we need (I think) such filter:
All Supported Files (*.mp3,....)| *.mp3... | All files (*.*) | *.*
Bcz theoretically you can try to use any file via DirectShowSource. (RealAudio/WMA/AVI etc)

Also, there's already a class wrapping AVIFile.. it would make sense that you use this one and make changes where needed (without breaking current functionality of course.. make sure you test encoding and avisynth script preview)
Yes, i know, as i say before - this still reque refactoring. And I want to write direct avisynth.dll wrapper to got better error diagnostics (instead audio stream not found) in nearest future so i decide do not spent time to refactoring AVIFile usage now - it will be removed anycase when my wrapper will be done.

And some more things: I think your encoder should derive from audioencoder, or at least be called from it so there's a common path it goes through.. just like the 3 video encoders and 3 muxers use a common class. That also allows easy switching of encoders.
Current audio processing consist of 2 step's:

Decoding && DSP
Encoding

So AudioEncoder is not a valid name. I propose AudioJobHandler. I believe using AviSynth for Decoding && DSP is enought so we does not need polyphormism @ this point. Encoding in current impementation perform via command-line encoders via StdIn. In future is possible to define AudioEncoder class with such methods:

public interface IAudioEncoder
{
void Init(string outputFileName, int samplerate, int channels, int bitsPerSample);
void Done();
void Abort();
}


How about using the commandlinegenerator to generate encoder commandlines?
It will be used later (i do not want to change it until first initial commit. and I don't like it bcz this is one class wich knows about every commandline tool)

Last but not least I have to ask the dreaded question "how well did you test this?" I mean.. encoding doesn't even get to the point where it would fail because the encoder paths are not being propagated..
No, they was. I really don't understand why this code doesn't for for you. Try to extrac && compile fresh sources http://www.mytempdir.com/398686 @ clear folder

I'm waiting for your response!

max-holz
21st January 2006, 11:54
Where could I find neroraw.exe? I see this exe in the new program files tab of rev 2035.

The Link
21st January 2006, 12:08
Dimzon wrote in his posting above yours:
As I said before you need BeHappy.Extension.Encoder.Nero7AAC.exe from BeHappy - use it instead of neroraw. I plan to remove BeHappy-related code from it && rename to neroraw a bit later.

Doom9
21st January 2006, 13:08
I have yet checked it - everything is fine? Maybe this error is VS Express (I'm using full version now)I have VS 2k5 standard installed on this machine. However, today it seems to work.

Are you shure Your current encoder.pause implementation really pauses external process? I do not think so, maybe I'm wrong? You only pauses StdOut/StdErr reading from it, isn't it. It means encoding still work...Well.. stopping the reading of these outputs seems to do the trick.. it's not like you can tell an external process to just go to sleep (it starts with the problem that there's no processpriority.sleep) x264 takes a bit longer to react, but all the others pause instantaneously.

As I said before you need BeHappy.Extension.Encoder.Nero7AAC.exe from BeHappyI downloaded your latest Behappy release (December 29th).. there's no nero7aac.exe in it.

No, look @ enum ThreadPriority@ MSDN - there are no ThreadPriority.IdleOops, I mistook that for processpriority.idle (which does exist obviously since I'm using it for every encoder)

Place breakpoint @ DimzonEncoder::encode In the third encoding attempt it finally breaked there.. that was after I posted here.

Try to extrac && compile fresh sources I did that.. I used what you posted, opened that whole project and compiled.. there's no code from anyplace else.

Another few thoughts: every one of my encoders and muxers performs a check if the required files are there (executable).. before putting a line into the script that requires a plugin, in sticking with the current scheme of encoders, it should check if that plugin does exist (you do have to configure your avisynth plugin path in the settings.. so you know where to look - and in addition I think we could read that path from the registry so it would be pre-configured if the MeGUISettings object holds an empty string for that property)

void Init(string outputFileName, int samplerate, int channels, int bitsPerSample);
void Done();
void Abort();
Do you know all those properties? I certainly don't know them for a besweet encoder. And what does Done() do? What about pause/resume?

Really we need (I think) such filter:Well.. you can add the all files for completeness sake, but it should never be the default. The default filter should open just what kind of input types are supported (naturally you could name an mp3 audio.doc but that's not something we need to handle).

so i decide do not spent time to refactoring AVIFile usage nowWhy would you need to refactor it? I think it'll probably be enough to just add a few properties that work for audio since currently there's only video related stuff. And with all your critizizing of my code, I'm a bit surprised to see that the new audio encoder is at least as bad architecture wise.. I hope you'll understand where I might feel a bit miffed by that.. you don't go around critizizing and trying to make everybody change if you then don't live up to the standard yourself.

dimzon
21st January 2006, 13:36
IWell.. stopping the reading of these outputs seems to do the trick.. it's not like you can tell an external process to just go to sleep (it starts with the problem that there's no processpriority.sleep) x264 takes a bit longer to react, but all the others pause instantaneously.
WOW. Very interesting! This means we can't run encoder @ silen mode (without progress output) bcz we can't pause it in this case.

I downloaded your latest Behappy release (December 29th).. there's no nero7aac.exe in it.
;) not just nero7aac.exe BUT BeHappy.Extension.Encoder.Nero7AAC.exe


Another few thoughts: every one of my encoders and muxers performs a check if the required files are there (executable).. before putting a line into the script that requires a plugin, in sticking with the current scheme of encoders, it should check if that plugin does exist (you do have to configure your avisynth plugin path in the settings.. so you know where to look - and in addition I think we could read that path from the registry so it would be pre-configured if the MeGUISettings object holds an empty string for that property)
Yes, It can be done and I will implement it later. In addition i have an idea to implement "check all settings" feature - add one more item to menu. This feature must check every executable/plugin/etc and produce a report about possible troubles.

void Init(string outputFileName, int samplerate, int channels, int bitsPerSample);
void Done();
void Abort();
Do you know all those properties? I certainly don't know them for a besweet encoder.
You forgot again. BeSweet is not an encoder, its whole AudioJobProcessor (in my proposd terminology). AudioEncoder works after decoder/DSP so you know this parameters
And what does Done() do? What about pause/resume?
Sorry, my proposed interface was not complete yet
public interface IAudioEncoder
{
void Init(string outputFileName, int samplerate, int channels, int bitsPerSample);
void EncodeBlock(byte[] rawPcmData);
void Done();
void Abort();
// stop/pause/etc - TODO
}

Done means no nore aduo avaluable let's finalize.

Well.. you can add the all files for completeness sake, but it should never be the default.
Agreed by 100%. Yes, off couse! Current *.* filter is just for testing :)

Why would you need to refactor it? I think it'll probably be enough to just add a few properties that work for audio since currently there's only video related stuff.
I does not want to touch this code at all bcz I plan to replace it ASAP.


And with all your critizizing of my code, I'm a bit surprised to see that the new audio encoder is at least as bad architecture wise.. I hope you'll understand where I might feel a bit miffed by that.. you don't go around critizizing and trying to make everybody change if you then don't live up to the standard yourself.
Hey, I say "this code is crap && reques refactoring" isn't it? I just perform initially copy/paste of BeHappy.Encoder into MeGUI and perform a little tweaks to fit it to IEncoder interface - no more. I do not like this code myself and I plan to refactor it right after my managed Avisynth wrapper will be done. So do not bla me for it ;) And another reason which slowdown code refactoring still conditional compilation - it breaks VS2005 build-in code analyst features as well as it breaks my favorite ReSharper. Of couse this is not MeGUI problem but VS2005/ReSharper but i can't work productively without it. Just imaginate You switch form VS2005 to Notepad - for me this is the same when i switch from ReSharper to pure VS2005.

Doom9
21st January 2006, 14:11
WOW. Very interesting! This means we can't run encoder @ silen mode (without progress output) bcz we can't pause it in this case.No.. you can still read it.. but not process anything and thus no status updates.
not just nero7aac.exe BUT BeHappy.Extension.Encoder.Nero7AAC.exeThere's only one exe in the zip ;)
for me this is the same when i switch from ReSharper to pure VS2005.Visual Studio 2005 has a bunch of refactoring features built-in, and then you have those code templates (that I've never used). And they have no problem with conditional compilation. I can't imagine how ReSharper would help integrating all your duplicate VfW code and put it into the AviReader class.

dimzon
21st January 2006, 15:33
There's only one exe in the zip ;)
are you joking ???
sources
behappy.exe
behappy.exe.config
behappy.extensibility.dll
behappy.extension.encoder.nero7aac.exe
behappy.oggvorbis.encoder.extension
behappy.oggvorbis.encoder.extension.dll
downmix.extension
enc_aacplus.extension
ffmpeg-ac3.extension
flac.extension
lame.extension
nero7.txt
nero7aac.extension
nicaudio.extension
upmix.extension
wavpack.extension
http://img41.imageshack.us/img41/782/untitled5xh1.png
I can't imagine how ReSharper would help integrating all your duplicate VfW code and put it into the AviReader class.
Uhhh. I just tell You in prevoius post:
I does not want to touch this code at all bcz I plan to replace it ASAP.
ASAP == during 2-5 days (mostly AvisynthWrapper is alredy done, keep testing)

Richard Berg
21st January 2006, 16:51
@dimzon
I'm strongly prefer using Guid's to avoid ANY conflicts.
I think it's better to use DateTime.Now.Ticks for unique tempfiles/logs/etc
- much shorter/cleaner-looking that GUIDs
- filenames will always be in date order, makes finding the right one easy

Doom9
21st January 2006, 16:55
well.. in order to be consistent with the naming, the one click encoder uses the input file for avs scripts.. I think the most coherent thing would be that the name of a temporary avs file is the source name plus .avs appended. And there's one problem with moving things to the temp directory.. if something goes wrong, or the user doesn't have the cleanup option enabled.. you'll never find your temporary files as only those who've seen the code know where things go. I'm not happy at all to have applications that fill up my temp directory and force me to clean it on my own.. even if files are not being deleted, at least if I have them all together I'll realize quickly enough what I can get rid of. But perhaps I know too much so that I'm able to identify which files I no longer need?

Richard Berg
21st January 2006, 17:22
Just make it TempDir\MeGUI\<videofilename> + DateTime.Now.Ticks + ".avs"

That gives you everything you want: which input file it affects, proper sorting, and easy for a human to group by job or to blow away the whole directory.

dimzon
21st January 2006, 20:24
2 all developers
AviSynthWrapper is done. take look http://www.mytempdir.com/399806

berrinam
21st January 2006, 22:59
Small CVS update:
(In compile.bat) If there are no commandline arguments, do the same as compile all.

I haven't updated the version number, etc, because it doesn't affect anything else. I made this change so that compile.bat can be run without the commandline.

berrinam
21st January 2006, 23:03
@dimzon: This isn't compatible with current avs opening (ie for previewing), is it? As it is, if you want to preview a file, you need to open it with your wrapper, to check if there is an error, and then you open it with the current code if there isn't. Seems a bit wasteful to me.

dimzon
22nd January 2006, 10:51
@dimzon: This isn't compatible with current avs opening (ie for previewing), is it? As it is, if you want to preview a file, you need to open it with your wrapper, to check if there is an error, and then you open it with the current code if there isn't. Seems a bit wasteful to me.
I'm planning to add video support and create my own VideoReader later. Current implementation is for Audio...

dimzon
22nd January 2006, 12:02
@Doom9

Hi! How does You think, can I add this methods/properties to AudioCodecSettings:

/// <summary>
/// Must return command line arguments string for command-line audio encoder
/// {0} means output file name
/// {1} means samplerate in Hz
/// {2} means bits per sample
/// {3} means channel count
/// {4} means samplecount
/// {5} means size in bytes
/// </summary>
/// <returns>command line arguments</returns>
public abstract string GetCommandLineArguments();

/// <summary>
/// Must return logical flag
/// </summary>
public abstract bool IsRawPcmSupportedByEncoder
{
get;
}

/// <summary>
/// Must read executable path from settings
/// </summary>
/// <param name="from">settings to read path from</param>
/// <returns>path to encoder executable</returns>
public abstract string GetExecutablePath(MeGUISettings from);


/// <summary>
/// Return some additional code to be applied to the end of DSP
/// Example - 6==Audiochannels(last)?GetChannel(last,2,3,1,6,4,5):last{0}
/// </summary>
public abstract string AdjasmentAvisynthScript
{
get;
}

Doom9
22nd January 2006, 13:25
Hi! How does You think, can I add this methods/properties to AudioCodecSettings:If you need them, of course. Althought I'd name the last one AdjustAvisynthScript so that it's proper English ;)

umm wait.. why should the codec settings extract something from the settings? that should be the job of the encoder itself, shouldn't it? The settings should contains a little information about the actual encoding process as possible.. just enough so that the encoder can begin its work.. the encoder should have to intelligence to do whatever's needed to get the encoding done. It's also a question whether source properties really belong to the settings.. I'm afraid I didn't quick stick to that with the videosettings as well. If we have the means to figure out the source properties just before encoding, so that the encoder keeps whatever it needs for status updates, I think that would be the optimal approach.

dimzon
22nd January 2006, 13:40
@Doom9
OK, if you don't like it i will do not do it ;)

New audiosettings look (beSweet - related code is back)
http://img17.imageshack.us/img17/226/untitled9tg.png

Doom9
22nd January 2006, 13:46
umm.. the 24bit thing.. I haven't read the whole thread in the audio forum but I think I read something about avisynth automatically using the proper sample length so there won't be any overflows/cutoffs?

dimzon
22nd January 2006, 14:06
umm.. the 24bit thing.. I haven't read the whole thread in the audio forum but I think I read something about avisynth automatically using the proper sample length so there won't be any overflows/cutoffs?
If You will use 24bit or floating computations you will reduce ROUNDING ERRORS in CHAIN (where multiplication/division is used)

ConvertAudioTo32Bit
SomeOp
SomeOp
SomeOp
ConvertAudioTo16Bit

dimzon
22nd January 2006, 15:14
Doom9
please take look http://www.mytempdir.com/401361

PS. DimzonEncoder is better but still need refactoring
PS2. Maybe it's time to commit this code to CVS?

Doom9
22nd January 2006, 15:21
well.. did you take care of all the issues I've mentioned? checking for plugin existence, checking for encoder existence? move the encoder out of the audioencoder class? I'm also not quite happy about having to select which encoder is going to be used at configuration time.. I think it should be done at encoding time (I know, videoencoder needs an adjustment there as well).

dimzon
22nd January 2006, 15:36
well.. did you take care of all the issues I've mentioned? checking for plugin existence, checking for encoder existence?
Just partially... I have some idea how to do such check at better way in future:

Does you know how jUnit/csUnit/NUnit works? It's possible to create same infrastructure to provide API for checking installation/settings integrity...


move the encoder out of the audioencoder class?
It still in same file Yet. Now it is AudioEncoder descendant.

I'm also not quite happy about having to select which encoder is going to be used at configuration time.. I think it should be done at encoding time (I know, videoencoder needs an adjustment there as well).
I really don't understand what does you men exatly...

Doom9
22nd January 2006, 15:40
I really don't understand what does you men exatly...In the audio options, you have a radiobutton "encode via avisynth" and one "encode via besweet" .. if you go to the x264 or xvid codec setup.. there's no "use x264.exe" and "use mencoder.exe" radiobutton.. that is handled in the settings.

Also, classnames should reflect what a class does.. dimzonencoder does what now? ;)

dimzon
22nd January 2006, 15:44
In the audio options, you have a radiobutton "encode via avisynth" and one "encode via besweet" .. if you go to the x264 or xvid codec setup.. there's no "use x264.exe" and "use mencoder.exe" radiobutton.. that is handled in the settings.
Unfortunally AviSynth has a little different option set (and this option set is easy to increase using additional filters etc). So it's impossible (i think) to switch before them.

Also, classnames should reflect what a class does.. dimzonencoder does what now? ;)
Feel free to rename it as you wish ;)

dimzon
22nd January 2006, 15:57
And yet another programming trick. Does you know WinForms list controls (including ComboBox as well) can host any objects. object.ToString() is used to obtain string to display and you can access selected object via myCombo.SelectedItem property...

So i propose more modular audio settings editing solution. This is my proposal

1) define AudioSettingsProvider interface (or, maybe, class)

public interface IAudioSettingsProvider
{
void LoadDefaults();
bool IsSupportedSettings( AudioCodecSettings settings);
AudioCodecSettings GetCurrentSettings();
void LoadCodecSettings( AudioCodecSettings settings);
bool EditCurrentSettings( IWin32Window parentWindow, profiles, etc )

}


2) implement this interface in
FaacSettingsProvider (override object.ToString to return FAAC)
NaacSettingsProvider (override object.ToString to return NAAC)
LameSettingsProvider (override object.ToString to return Lame MP3)

3) fill AudioCodecComboBox using:
AudioCodecComboBox.Items.AddRange( new object[]{
new FaacSettingsProvider (),
new NaacSettingsProvider(),
new LameSettingsProvider() })


Volia.
To get current settings just call
(AudioCodecComboBox.SelectedItem as IAudioSettingsProvider).GetCurrentSettings()

to load settings from job:

foreach(IAudioSettingsProvider p in AudioCodecComboBox.Items)
{
if(p.IsSupportedSettings(settingsToLoad))
{
p.LoadCodecSettings(settingsToLoad);
AudioCodecComboBox.SelectedItem = p;
break;
}
}

stax76
22nd January 2006, 16:40
And yet another programming trick. Does you know WinForms list controls (including ComboBox as well) can host any objects. object.ToString() is used to obtain string to display and you can access selected object via myCombo.SelectedItem property...

It's important to know this and in most cases more robust, flexible and easier then DataBinding and other things. The only thing I'm disappointed is Microsoft don't have versions of this controls supporting generics, the GUI design time environment is already amazing so supporting this should probably be doable. VB's clunky cast syntax reminds me all the time that it would be a nice feature.

dimzon
22nd January 2006, 17:07
It's important to know this and in most cases more robust, flexible and easier then DataBinding
You can use then both @ same time:


private object[] _besweetDownMix = new object[]{....};
private object[] _avisynthDownMix = new object[]{....};


{
if(isBeSweetMode)
{
dowmixCombo.DataSource = _besweetDownMix;
}
else
{
dowmixCombo.DataSource = _avisynthDownMix;
}
}


Don't forget - you can use DataBinding for any IList (object.ToString() will be used to get representation)

stax76
22nd January 2006, 17:23
If overriding ToString gets you around using DisplayMember then it should be fine as well.

dimzon
22nd January 2006, 17:25
If overriding ToString gets you around using DisplayMember then it should be fine as well.
I prefer to owerrite toString bcz I know how it will be looked @ screen even if I do not touch default ComboBox settings ;)

stax76
22nd January 2006, 17:47
I prefer to owerrite toString bcz I know how it will be looked @ screen even if I do not touch default ComboBox settings

Me too, I don't recall using any DataBinding in StaxRip. Some people will try to solve everything with DataBinding while in many cases it's a poor solution.

berrinam
23rd January 2006, 08:42
0.2.3.2036 23 Jan 2006
Applied Mutant_Fruit's Context Sensitive Help update:
-Just attaching a new version of the contexthelp.xml as written by LiFe.
-I also made a few changes to the contexthelp method as i didn't realise that newlines would be correctly picked up from the xml file.
-Tooltips should no longer stretch across the entire screen. But could possibly do with nicer paragraphing.

Doom9
23rd January 2006, 09:25
@berrinam: did you see richard's patch?

Inc
23rd January 2006, 09:43
2 all developers
AviSynthWrapper is done. take look http://www.mytempdir.com/399806

Could you also offer the sources? :)

My comments:

I had a fast look over your header cs file where the functions of the avsredirect.dll are called.

1. You did declare the samplecount/offset etc. of the arguments in Getaframe() as Long. Did you let pass these valuesas proper Int64 by doing a Cast of the Long values to Int64? as the the orig avs function expects int64 values in here.

2. Seems you still handle the 3 -Y U V- planes separately by calling getvframe() as it was the orig state of avsredirect.dll for supporting FFmpegs native YV12 support. I modded the avsredirect so only the readpointer to the whole bitmap is returned, NO return to a VideoPlanestructure. Keep that in mind :) Thats why I also added the RGB24/32, YUY2 and YV12 output support.

dimzon
23rd January 2006, 09:49
Could you also offer the sources? :)
No problem
http://www.mytempdir.com/402921 - it uses AviSynth so it's LGPL anycase. Just keep in mind it's not finished yet


1. You did declare the samplecount/offset etc. of the arguments in Getaframe() as Long. Did you let pass these valuesas proper Int64 by doing a Cast of the Long values to Int64? as the the orig avs function expects int64 values in here.
long in c# means int64 :cool:


2. Seems you still handle the 3 -Y U V- planes separately by calling getvframe() as it was the orig state of avsredirect.dll for supporting FFmpegs native YV12 support. I modded the avsredirect so only the readpointer to the whole bitmap is returned, NO return to a VideoPlanestructure. Keep that in mind :) Thats why I also added the RGB24/32, YUY2 and YV12 output
support.
I doesn't touch video part yet - i'm planing to get it later

Doom9
23rd January 2006, 09:51
@dimzon: can you handle the cutlist format I proposed? And I really think the encoder should be separate.. so that if you do further work on it, it touches the smallest amount of files. The current AudioEncoder should just serve as an audio job dispatcher.

Inc
23rd January 2006, 10:00
- it uses AviSynth so it's LGPL anycase. Just keep in mind it's not finished yet
btw: Even if AVS is LGPL licensed the root wrapper code by Mobilehackers was released under the GPL thats why I kept it automatically as GPL when I modded the sources. So even the resulted dll is GPL'ed *imho* (not 100% sure)

If you want a preview of the frame or the audio, you can simply use the approaches I discribed in here:
http://forum.doom9.org/showthread.php?p=772595#post772595
There you will obtain a pointer where you can access the Bitmap-startingadress by using for example SetDIBits() of the Win API (or something similair in C#)

dimzon
23rd January 2006, 10:07
btw: Even if AVS is LGPL licensed the root wrapper code by Mobilehackers was released under the GPL thats why I kept it automatically as GPL when I modded the sources. So even the resulted dll is GPL'ed *imho* (not 100% sure)

Sorry, my fault:
Linking Avisynth statically or dynamically with other modules is making a combined work based on Avisynth. Thus, the terms and conditions of the GNU General Public License cover the whole combination.

Doom9
23rd January 2006, 10:27
well.. license wise MeGUI is GPL anyway so whatever parts are integrated fall under this license as well.

dimzon
23rd January 2006, 10:34
@dimzon: can you handle the cutlist format I proposed?
Lets define something like with (for better code maintenance)


public class FrameInterval
{
public long FirstFrame;
public long LastFrame;
}

public class Cutlist
{
// actual fps = FramerateNoninator/FramerateDeNoninator
public long FramerateNoninator;
public long FramerateDeNoninator;
public FrameInterval[] Intervals;
}



And I really think the encoder should be separate.. so that if you do further work on it, it touches the smallest amount of files.
I do not understand exatly what does you mean (my poor english can be a reason). Does you want i move my encoder in separate file? No problem in this case.

The current AudioEncoder should just serve as an audio job dispatcher.
AFAIK it is ;)
public virtual bool setup(Job job, out string error)
{
error = null;

if (((AudioJob)job).Settings.EncodeViaBeSweet)
encoder = new BeSweetEncoder(settings.BesweetPath);
else
encoder = new DimzonEncoder(settings);

return encoder.setup(job, out error);
}


And I want to perform CVS commit before continue. Can I?


By the way - what does you think about my proposal (http://forum.doom9.org/showthread.php?p=773033#post773033)

And I have another proposal about new "prerender" feature. I really have a beteter idea how it must be done!
let's define:
t1 = average time to read 1 avs frame
t2 = average time to encode 1 frame to losless
t3 = average time to decode 1 frame from lossles
t4 = average time to 1-st pass encode for 1 frame
t5 = average time to 2-nd pass encode for 1 frame

Assuming 2-pass encoding we need
t1 + t2 + t3 + t4 + t3 + t5 time to encode 1 frame

We can save marked t3 interval if we write some more intelligent code:
first pass:
1) Read frame from AviSynth
2) Encode it to lossless (it's possible to wrap VfW driver direcly via PInvoke) && save encoded frame @ temporaly stream
3) Encode it via x264 (send frame to it via StdIn like audio encoders do)

second pass:
1) Decode frame from loseless (it's possible to wrap VfW driver direcly via PInvoke)
2) Encode it via x264 (send frame to it via StdIn like audio encoders do)

So were are new unique features:

Since we are 100% controlling process we can use multiple hard drives to store losless temporaly files. Really if we are running out of space @ HDD1 we can start writing rest data to HDD2 etc...
It posiible to perform partially prerender. If we are running out of space @ HDD we can stop caching frames (for first N frames for second pass cached data will be used, for rest fromes - source AviSynth)

Invoking lossless encoder via MeGUI:
there are 2 way to do it:

It's possible to call VfW driver via PInvoke
Since there are open source losless encoders we can ceate our own .NET-friendly DLL ;)

Doom9
23rd January 2006, 10:51
Lets define something like with (for better code maintenance)Agreed.

Does you want i move my encoder in separate file? Yup..

And I want to perform CVS commit before continue. Can I?I still need to do a few testruns myself. Have you tested auto-mode and one click mode?

Now about your prerender suggestion.. wouldn't we incurr a loss of speed if we go from native -> managed -> native again as opposed to having the encoder read frames directly from the avisynth script? PInvoke isn't the most efficient thing after all, is it?

berrinam
23rd January 2006, 11:00
@dimzon: About the prerender scripting: What you suggested about skipping t3 has indeed been suggested before (I linked to zajc's post about it in the first post of the feature request thread). I did plan to implement it that way, using avs2yuv as the way of managing this, as this gives you the option of doing this. I think I even said when I added the support for this that I decided not to in the end, because:
1. avs2yuv wasn't flexible enough for MeGUI.
2. When I tried to make it flexible, my limited C++ skills resulting in the program crashing.
I scrapped that then. However, the t3 time interval is probably the least computer-intensive one. Especially with ffvhuff, which was designed only for speed and lack of colorspace conversions, the decoding speed is very high (I haven't measured it).

As to running it all through MeGUI.... yeesh, I thought the whole point of MeGUI was to get rid of VfW, and now you are suggesting going back to it? Also, using avi's and not splitting them across directories makes it more easy to use this outside of MeGUI (we don't want to create a monopoly on the video we just processed, do we?).

My real objection, of course, is that what you are saying is all too complex for me. I don't think directly controlling VfW is what we want with MeGUI, but a .NET-friendly DLL might not be too bad. Let's see what Doom9 thinks.

@dimzon: I'm planning some refactoring and quite a few changes to the currentNAACSettings, etc, as well as other audio in Form1.cs. Does this clash with the changes you have made?

@Doom9: No, I didn't see Richard's patch. I've found it now.
I objected a while ago to all the currentSomethingSettings in MeGUI, and you asked if I have a better idea. Here we go: update the VidCodecSettings property to have a get and set, and use this throughout MeGUI:
public VideoCodecSettings VidCodecSettings
{
get
{
VideoCodecSettings vSettings = null;
#if FULL
switch (videoCodec.SelectedIndex)
{
case 0:
vSettings = this.currentLavcSettings;
break;
case 1:
vSettings = this.currentX264Settings;
break;
case 2:
vSettings = this.currentSnowSettings;
break;
case 3:
vSettings = this.currentXvidSettings;
break;
}
#elif X264_ONLY
vSettings = this.currentX264Settings;
#elif SNOW_ONLY
vSettings = this.currentSnowSettings;
#endif
return vSettings;
}
set
{
#if FULL
if (value is x264Settings)
{
this.currentX264Settings = (x264Settings)value;
videoCodec.SelectedIndex = (int)VideoJob.CodecType.AVC;
}
if (value is lavcSettings)
{
this.currentLavcSettings = (lavcSettings)value;
videoCodec.SelectedIndex = (int)VideoJob.CodecType.ASP;
}
if (value is snowSettings)
{
this.currentSnowSettings = (snowSettings)value;
videoCodec.SelectedIndex = (int)VideoJob.CodecType.SNOW;
}
if (value is xvidSettings)
{
this.currentXvidSettings = (xvidSettings)value;
videoCodec.SelectedIndex = (int)VideoJob.CodecType.XVID;
}
#elif X264_ONLY
if (value is x264Settings)
this.currentX264Settings = (x264Settings)value;
#elif SNOW_ONLY
if (value is snowSettings)
this.currentSnowSettings = (snowSettings)value;
#endif
}
}Of course, I would do likewise for audio (audio seems even worse.... it has HEAPS of code all over the place, which I don't think has been changed for ages). What do you think?

dimzon
23rd January 2006, 11:07
Agreed.
Yup..
Ok. I will do it after CVS commit. By the way - what name does you propose for DimzonEncoder? Is AvisynthAudioEncoder good for it?

I still need to do a few testruns myself. Have you tested auto-mode and one click mode?
there are some bug, replace audioEncoder.cs http://www.mytempdir.com/403001

wouldn't we incurr a loss of speed if we go from native -> managed -> native again as opposed to having the encoder read frames directly from the avisynth script?
there are some trick to improve PInvoke perfomance. The first off all - avoid Marshaller work for strings, structures and bytearrays. It can be done using unsafe C# code or playing with CGHandle created with Pinned option.

Doom9
23rd January 2006, 11:13
update the VidCodecSettings property to have a get and set, and use this throughout MeGUI:sounds good (though in the end I think we'll go much further than this.. dimzon mentioned some stuff to think about).

dimzon
23rd January 2006, 11:13
@dimzon: I'm planning some refactoring and quite a few changes to the currentNAACSettings, etc, as well as other audio in Form1.cs. Does this clash with the changes you have made?
Please, do not touch audio-related stuff until my commit (I'm waiting for Doom9 approval).

Here we go: update the VidCodecSettings property to have a get and set, and use this throughout MeGUI:
<skiped/>
Of course, I would do likewise for audio (audio seems even worse.... it has HEAPS of code all over the place, which I don't think has been changed for ages). What do you think?

Does You read my proposal:
http://forum.doom9.org/showthread.php?p=773033#post773033

It's sutable for vide too ;)

Doom9
23rd January 2006, 11:28
I'm waiting for Doom9 approvalAnd I asked a couple critical questions.. have you checked how both besweet and avisynth audio encoding behave in manual, auto and one click mode? there must not be any commit unless the correct behavior has been checked everywhere. This change is too big to not check every mode at least once.
Is AvisynthAudioEncoder good for it?Yes, that would've been my suggestion

By the way, would you mind cutting down your signature? It kinda bugs me a lot.

dimzon
23rd January 2006, 11:35
have you checked how both besweet and avisynth audio encoding behave in manual, auto and one click mode?
Oh, sorry, missed them. No, i do not perform this check. I have not material to check one click mode (i'm @ ofice and my home PC is broked). Can you perform this check for me?[/QUOTE]


By the way, would you mind cutting down your signature? It kinda bugs me a lot.
What signature? Where?

dimzon
23rd January 2006, 11:38
@Doom9
addition about audio cutlist
We are propesed long(int64) for frames and nominators. Unfortunally AviSynth does support only int (int32).
I prefer to stand @ longs (for futher) but avoid values > int.MaxValues

Doom9
23rd January 2006, 11:57
Can you perform this check for me?I'm also at work.
What signature?Yours.. colored, bold, 7 lines and 8 links. It makes me think about a rule change.
I actually keep nbFrames as an int throughout the application.
BTW, when can you get started on the video part of the avisynth wrapper? And the whole nominator/denominator that fps was recently changed to already gives me a headache now.. it was so nice having a double and that's that.. it's what people commonly understand as a framerate. All those changes were imho extremely unnecessary and now have the sideeffects I expected.. they break existing applications.

dimzon
23rd January 2006, 12:22
I'm also at work.
I can't perform this check even @ home. Can You perform this check when yoiu will be @ home?
Or can somebody else, well known to Doom9, perform this check?

Yours.. colored, bold, 7 lines and 8 links. It makes me think about a rule change.
Oh, I understand. Is it better now?

BTW, when can you get started on the video part of the avisynth wrapper?

At first I must perform CVS commit
I must split AudioEncoder.cs and rename DimzonEncoder
Maybe some cleanups in AvisynthAudioEncoder
CVS commit

After this I can try start ideo part of the avisynth wrapper (Seems like you like it :cool: )


And the whole nominator/denominator that fps was recently changed to already gives me a headache now.. it was so nice having a double and that's that..

at least let's switch from double to System.Decimal (I believe it's easy to replace).


All those changes were imho extremely unnecessary and now have the sideeffects I expected.. they break existing applications.

In other hand you can make something like this


public class FramesPerSecond
{
public long Nominator
public long Denominator

public FramesPerSecond(System.Decimal fps)
{
Denominator = 1000000;
Nominator = (long)(fps * Denominator);
}

public System.Decimal ToDecimal()
{
System.Decimal fps = Nominator ;
fps/=Denominator;
return fps;

}

public static implicit operator FramesPerSecond(System.Decimal fps)
{
return new FramesPerSecond(fps);
}

public static implicit operator System.Decimal(FramesPerSecond fps)
{
return fps.ToDecimal()
}


}

this code allows you step-by-step conversion from Double/Decimal to Nominator/Denominator

berrinam
23rd January 2006, 12:25
Please, do not touch audio-related stuff until my commit (I'm waiting for Doom9 approval). Ok, sure thing. I'll wait. I'll apply Richard Berg's patch, but looking at it, there are no changes to the audio code -- only the AutoEncode setup.
Does You read my proposal:
http://forum.doom9.org/showthread.php?p=773033#post773033

It's sutable for vide too ;)Good proposal. I assume that this code:
foreach(IAudioSettingsProvider p in AudioCodecComboBox.Items)
{
if(p.IsSupportedSettings(settingsToLoad))
{
p.LoadCodecSettings(settingsToLoad);
AudioCodecComboBox.SelectedItem = p;
break;
}
}
would be in a property?

Doom9
23rd January 2006, 12:38
In other hand you can make something like thisThe thing is though, if you look at the discussion in the avisynth forum.. it's not such a nice denominator (http://forum.doom9.org/showthread.php?t=104681)

And it's really up to you to test your modifications.. not somebody else. We'll get nowhere if every dev just checks if something compiles.. that would imply that after every commit, there's a new build that will cause a long list of bugreports and somebody has to fix things again. You don't release stuff at work without having tested it, do you? Even if not using jUnit, the least amount of testing that should be done is playing through the most likely scenarios.

berrinam
23rd January 2006, 12:42
CVS Update:
0.2.3.2037 23 Jan 2006
Applied Richard Berg's filechecking bugfix
Fixed OneClickWindow to register the automatic deinterlacing settings

It has some changes to Form1.cs, but I don't think they would clash with dimzon's, and the changes are very few.

Also, dimzon, the correct word is not Nominator, but Numerator.

Doom9
23rd January 2006, 13:25
another question regarding the audio encoder: the exe currently has nero7 on it.. does it work with nero6 as well?

dimzon
23rd January 2006, 13:39
The thing is though, if you look at the discussion in the avisynth forum.. it's not such a nice denominator (http://forum.doom9.org/showthread.php?t=104681)
Ok, lets stay @ double (or, maybe Decimal, it's better)


And it's really up to you to test your modifications.. not somebody else.
As I said before I have not ability to check this functionality at all. In other side I does'nt think I boreke something (i does not perform any changes in this part of code & does not perform any structural changes and project still compile).
AutoEncode works fine (just tested on Version() avs :) )

You don't release stuff at work without having tested it, do you? Even if not using jUnit, the least amount of testing that should be done is playing through the most likely scenarios.
We have special QA division ;)

the correct word is not Nominator, but Numerator.
thanx

Doom9
23rd January 2006, 13:44
As I said before I have not ability to check this functionality at all.Why not? Don't tell me you don't have any DVDs and digital streams whatsoever.. and there are freely downloadable VOB trailers just in case.

dimzon
23rd January 2006, 13:46
another question regarding the audio encoder: the exe currently has nero7 on it.. does it work with nero6 as well?
Yes an No ;)

really current encoder work with both dll versions but it will not perform any channel workaround (it just read data from stdin and sent it to nero dll)
dimzonEncoder assumes Nero7 is used and adds such workaround script @ the end of AVS:

6==Audiochannels(last)?GetChannel(last,2,3,1,6,4,5):last
really it's possible to add checkbox @ settings form or AudioSettingsDialog to switch before versions.

By the way - you can use Nero7 aac encoder without Nero7 installed.
place my executable + bsn.dll from besweet + aacenc32.dll + aac.dll + neroipp.dll + mfc71.dll at same folder and enjoy ;)

dimzon
23rd January 2006, 13:50
Why not? Don't tell me you don't have any DVDs and digital streams whatsoever.. and there are freely downloadable VOB trailers just in case.
If You can provide me a VOB for testing wich size is <=10-20 MB i will be happy ;)
And I can't download multimedia content directly (i'm behinde evil corporate firewall) so you need to archive it with passwo twice (this firewall can read archive toc)

Doom9
23rd January 2006, 14:02
You have no DVDs and hang out in this place, no DV, no digital TV? I'm wondering what you're doing here then.. why be a member of a digital video forum if you do not have any digital video to process? I wouldn't download VOBs at work either but there's always the option of downloading from home. I'm getting the excuses feeling again. I don't find it plausible that any member here would not have any digital video he or she could process..

Doom9
23rd January 2006, 14:19
If supporting Nero6 means changes, then I guess we need another dropdown in the settings.. depending on that value, you'd then add or not add the nero6 workaround.

stax76
23rd January 2006, 14:37
You have no DVDs and hang out in this place, no DV, no digital TV?

I got some stuff in case somebody is looking for something, has anybody some DV samples?

I've processed some of my samples probably more than 100 times, funny thing is with every watch of a sample I either hate or like it a little bit more :)

dimzon
23rd January 2006, 14:47
You have no DVDs and hang out in this place, no DV, no digital TV?
As i said before I'm working on non-usial HW configuration now. My primary HDD still broken and I'm working @ 20GB HDD now. This HDD is bloated by instrumental tools and I running approx 500MB free only so I can't test on full-size DVD. If testing on small AVI is enought to test OneClick mode I will do it.
And I have not internet connection avaluable @ home :scared:

dimzon
23rd January 2006, 15:03
@Doom9
I have found some mpg file but seems like MeGUI for OneClickMode (never used before) searching for some additional file with txt extension in same folder. I really doen't know where to get it...

gets information about a video source based on its DVD Decrypter generated info file

Doom9
23rd January 2006, 15:17
doesn't matter if you don't have it and I suppose that's in berrinam's guide. and come to think of it, testing auto-mode should be enough because one click mode is based on the same code.. the jobs are just created after indexing so that comes down to the same as auto-mode.

dimzon
23rd January 2006, 15:38
doesn't matter if you don't have it and I suppose that's in berrinam's guide. and come to think of it, testing auto-mode should be enough because one click mode is based on the same code.. the jobs are just created after indexing so that comes down to the same as auto-mode.
Auto mode works fine, can I perform commit?

Doom9
23rd January 2006, 15:50
if you've tested both besweet and avisynth mode, then yes. good luck

dimzon
23rd January 2006, 16:09
if you've tested both besweet and avisynth mode, then yes. good luck
0.2.3.2038 23 Jan 2006
Commit by dimzon
-New audio encoding mode via AviSynth
-Refectored && redesigned AudioSettings dialogs
-AviSynth wrapper (only audio)
-Don't forget to put avisynthwrapper.dll into executable folder
-avisynthwrapper.zip contains avisynthwrapper.dll sources
-use BeHappy.Extension.Encoder.Nero7AAC.exe from BeHappy instead of neroraw.exe for nero encoding

dimzon
23rd January 2006, 16:21
http://vzlaird.narod.ru/x264/ContextHelp.xml --> russian translation ;) Maybe we can add support for multiple languages for this file via configuration switch?
this is not full MeGUI localisation but at least meaningfull localazed information about codecs...

dimzon
23rd January 2006, 16:26
@Doom9:
How I have multiple posiible ways what to do. I can:

perform futher AviSynthAudioEncoder refactoring
create video-part for the AviSynthWrapper
implement proposed changes (IAudioSettingsProvider in Combo)
add support for Nero 6 AAC encoder
create neroraw.exe


What does You want I do first ?

Doom9
23rd January 2006, 16:30
well.. if I can chose: 5, 4, 1, 2

Wait with number 3 because the whole shebang should be redone. I'm planning on first compiling a list with all the functions and their classes that we have now, then decide what should go where, any possible naming changes, and then finally the implementation.

dimzon
23rd January 2006, 16:37
well.. if I can chose: 5, 4, 1, 2

Wait with number 3 because the whole shebang should be redone. I'm planning on first compiling a list with all the functions and their classes that we have now, then decide what should go where, any possible naming changes, and then finally the implementation.
create neroraw.exe
how i must commit it to CVS? if i commit compiled executable + sources in zip file - is it OK?

Doom9
23rd January 2006, 16:42
hmm.. you have to ask somebody more familiar with the CVS system.. I tend to think this shouldn't be part of the megui source tree but something else. I don't really know what we can do to achieve that.. ideally I think it (and the avisynth wrapper) should be a separate subtree, so you can check them out and compile independently of megui.

dimzon
23rd January 2006, 17:00
hmm.. you have to ask somebody more familiar with the CVS system.. I tend to think this shouldn't be part of the megui source tree but something else. I don't really know what we can do to achieve that.. ideally I think it (and the avisynth wrapper) should be a separate subtree, so you can check them out and compile independently of megui.
this is extremely easy console application, it does not produce any compilation troubles so i decide to add it to MeGUI solution. You can still compile MeGUI only OR you can compile them both or you can build neroraw only. If You don't like my way we can remove it from solution/CVS anytime

PS. It's alredy @ CVS :cool:

dimzon
23rd January 2006, 17:26
add support for Nero 6 AAC encoder
0.2.3.2039 23 Jan 2006
Commit by dimzon
-Support for Nero6 AAC
http://img227.imageshack.us/img227/4347/untitled9mq1.png

dimzon
23rd January 2006, 18:32
btw: Even if AVS is LGPL licensed the root wrapper code by Mobilehackers was released under the GPL thats why I kept it automatically as GPL when I modded the sources. So even the resulted dll is GPL'ed *imho* (not 100% sure)

If you want a preview of the frame or the audio, you can simply use the approaches I discribed in here:
http://forum.doom9.org/showthread.php?p=772595#post772595
There you will obtain a pointer where you can access the Bitmap-startingadress by using for example SetDIBits() of the Win API (or something similair in C#)


PVideoFrame f = clip[clip_num]->GetFrame(frm, env);
g_lasterr[0] = 0;
return (int)f->GetReadPtr();

I really doesn't like this implementation. PVideoFrame is smartpointer.
It means it will be destroyed right after function exits. So you have no warranty actual frame data still @ obtained adress
(depends on memory manager implementation && current heap fullness)
There are 2 ways howto fix it
First way means providing buffer into rotune and filling it actual data. This is mostly clear way. Unfortunally it cost additional CPU cycles for copyng memory and reques additional memory.
Second way is storing PVideoFrame f in dynamicaly-allocated memory and returning it outside your dll. It means you need 3 functions instead one:

frame = avs_getvframe(clip_num, frm, env) - returns &f
avs_getvframepointer(frame) - returns (*frame)->GetReadPtr();
avs_disposevframe(frame) - must deallocate memory: (*frame=NULL; free(frame))

berrinam
23rd January 2006, 21:22
doesn't matter if you don't have it [stream information.txt] and I suppose that's in berrinam's guide.Hmmm.... not at the moment. As it is, I am not convinced whether that will work. It seems to rely on the audio being in the first two track IDs (presuming the user chooses track 1 and track 2). Can this be assumed to always be the case? If so, I will put it into the guide.

Doom9
23rd January 2006, 22:02
Can this be assumed to always be the case? I'm not really sure.. there are 8 tracks you can chose from and I always thought they'd map to the 0x8* used for AC3 tracks, but I have no clue how it works for PCM, MPEG or DTS (DTS uses 0x88 and 0x89).

berrinam
23rd January 2006, 22:25
Perhaps it is necessary, then, to rethink about integrating MediaInfo.dll?

Inc
23rd January 2006, 22:32
AC3 ... 0x80 to 0x8?
DTS ... 0x88 or 0x89
PCM ... 0xA0 to 0xA?
MP2 ... 0xC? to ????

klicker4546
24th January 2006, 01:13
0.2.3.2039 23 Jan 2006
Commit by dimzon
-Support for Nero6 AAC


I'm just wondering, where can I find the latest source code to compile it? Don't tell me on sourceforget in the MeGUI cvs. Could anyone please upload it somewhere?

I'd really appreciate.

berrinam
24th January 2006, 01:26
I'm just wondering, where can I find the latest source code to compile it? Don't tell me on sourceforget in the MeGUI cvs. Could anyone please upload it somewhere?

I'd really appreciate.
The people who can upload the sources are the same people who can upload the compield versions, so I don't see why you are asking for this. You either have to wait until someone who can upload does, (in which case you will get a binary -- why would you want to compile?) or get it sooner, which does mean the Sourceforge CVS.

klicker4546
24th January 2006, 01:30
The people who can upload the sources are the same people who can upload the compield versions, so I don't see why you are asking for this. You either have to wait until someone who can upload does, (in which case you will get a binary -- why would you want to compile?) or get it sooner, which does mean the Sourceforge CVS.

Thanks for your quick reply. I've tried to deal with the Sourceforge CVS, but experienced problems with the anonymous login. Since I don't know if that's fixed, I was just wondering if it's possible to get the sources here. Or can you tell me how to download the latest changes from the CVS?

I guess I have to be patient then. ;)

Inc
24th January 2006, 01:44
Don't tell me on sourceforget in the MeGUI cvs.It gots its sense why such appl. sources are at Sourceforge.net

cvs -d:pserver:anonymous@cvs.sourceforge.net:/cvsroot/megui login

cvs -z3 -d:pserver:anonymous@cvs.sourceforge.net:/cvsroot/megui co -P modulenamehttp://sourceforge.net/cvs/?group_id=156112

Try this line using CygWin's cvs.exe for example, thats how I do download mencoder and ffmpeg sources when I do need them.
Im not shure if it works since I saw some lines above something about an anonymous login issue?

Also DevC++ or any other CVS Win32 browser could do such D/L Jobs.

berrinam
24th January 2006, 03:11
0.2.3.2040 24 Jan 2006
Commit by berrinam:
-Fixed d2v/audio extension bug
-Stopped progress window opening on a new job when MeGUI is minimized.
-Fixed AviSynthJobs to open the progress window when required
-Fixed AutoEncode to work with prerender jobs

berrinam
24th January 2006, 04:03
0.2.3.2041 24 Jan 2006
Commit by berrinam:
-Fix the incorrect fps displayed in the bitrate calculator due to new DGIndex fps signalling

Doom9
24th January 2006, 04:48
Perhaps it is necessary, then, to rethink about integrating MediaInfo.dll?Well.. I think some tests would be in order to determine how it works with the non standard streams. As you know, there are but 8 tracks you can select from within DGIndex and in a way they need to be able to extract everything (8 tracks is the maximum DVD offers).. so I think the mapping is dynamic and it's likely that if you have three tracks, they'd be track 1 - 3. Let's ask neuron2 to make sure (well you ask.. I gotta take the 5:05am train to get to the airport).

dimzon
24th January 2006, 11:56
CVS Update:
fixed compilation bug involved by my previous update (changed compile.bat)

klicker4546
24th January 2006, 13:43
Thanks for your help. I've tried it several times now to login to the megui cvsroot. Without any succes. There's everytime a timeout. Think I've read something about it before. Does anyone know how to get it working?

:confused:

It gots its sense why such appl. sources are at Sourceforge.net

cvs -d:pserver:anonymous@cvs.sourceforge.net:/cvsroot/megui login

cvs -z3 -d:pserver:anonymous@cvs.sourceforge.net:/cvsroot/megui co -P modulenamehttp://sourceforge.net/cvs/?group_id=156112

Try this line using CygWin's cvs.exe for example, thats how I do download mencoder and ffmpeg sources when I do need them.
Im not shure if it works since I saw some lines above something about an anonymous login issue?

Also DevC++ or any other CVS Win32 browser could do such D/L Jobs.

dimzon
24th January 2006, 13:44
0.2.3.2042 24 Jan 2006
Commit by dimzon:
-fixed some compilation warnings

dimzon
24th January 2006, 14:01
0.2.3.2043 24 Jan 2006
Commit by dimzon:
-changed Thread sync to fix some compilation warnings

dimzon
24th January 2006, 15:15
0.2.3.2044 24 Jan 2006
Commit by dimzon:
-AvsReader (not checked yet, not used anymore)

dimzon
24th January 2006, 15:53
0.2.3.2045 24 Jan 2006
Commit by dimzon:
-Modified AvsReader are now used for Avs preview ;) Everyting works fine...

http://img296.imageshack.us/img296/8974/untitled4pi.png

dimzon
24th January 2006, 15:59
@Doom9
I need you assistance for AvsAudioEncoder refactoring. Take look @ it. Sometimes it's looks like CommandLineAudioEncoders, sometimes not. Maybe we can create CommandLineAudioEncoder ancestor to collect all common code in it? (I can't do it without Your approval bcz it is architectural change)

PS. Does we need mediaInfo.dll wrapper? I can write it.