MeGUI development

[Richard Berg]

Guys, at the level you're worrying about, it's just a style preference.

[Doom9]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

@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]

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]

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]

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]

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]

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]

@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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

@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]

@dimzon -- does it overflow with >2 billion samples even if you use audiolengthf? I forget how it's stored internally.

[dimzon]

@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]

(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]

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]

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]

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]

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]

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]

http://www.mytempdir.com/397199 - fresh sources for AVS audio input

[godhead]

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]

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]

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]

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]

@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]

:thanks: