MeGUI development

[Sharktooth]

@berrinam: update_cache is indeed causing problems to some users. i thought to make it optional but as i previously said, i havent time to look at it for some time.

[Eamon21]

My profiles patch: http://megui.org/berrinam_patches/profiles_refactor/profiles_refactor_1.patch.
The change on lines 4839 & 4840 of your patch break the nero configuration panel (select the nero scratchpad and try to config and save a new one):

- protected override AudioCodecSettings CodecSettings
+ protected NeroAACSettings CodecSettings

because CodecSettings is accessed from the parent class AudioConfigurationPanel in the Settings property setter which will use it's own virtual method (which does nothing) because it is no longer overridden.

The CodecSettings property in AudioConfigurationPanel ideally would be marked as abstract but that is impossible because the VS IDE will not let you design abstract classes obviously, I suggest modifing the implementation so a problem like this is easier to spot in future like thisprotected virtual AudioCodecSettings CodecSettings
{
//going to throw a NotImplementedException because this should always be overridden as this
//base class contains no codec specific UI.
//Ideally this would be an abstract property but limitations of the Visual Studio IDE
//prevent you from designing abstract controls. Throwing this exception here should mean
//that the problem is picked up imediately.
//Changed to a Debug.Assert() for the moment, still deciding which is best.
get
{
//throw new NotImplementedException("This virtual method should have been overridden");
System.Diagnostics.Debug.Assert(false);
return new AudioCodecSettings(null);
}
set
{
//throw new NotImplementedException("This virtual method should have been overridden");
System.Diagnostics.Debug.Assert(false);
}
}
Haven't decide if it is better to throw the exception or to Assert, but think it should at least be one or the other.

Eamon

[berrinam]

Does it matter which you throw? They should both be caught pretty much instantly, and fixed before the user sees them. I would personally stick with the assert, though, because asserts always mean programming errors.

Beware, though, that throwing exceptions in properties can be dangerous when VS designer tries to modify them. It may be worth adding the [Browsable(false), DesignerSerializationVisibility(DesignerSerializationVisibility.Hidden)] attributes to CodecSettings to avoid this.

[Eamon21]

Does it matter which you throw? They should both be caught pretty much instantly, and fixed before the user sees them. I would personally stick with the assert, though, because asserts always mean programming errors.

Beware, though, that throwing exceptions in properties can be dangerous when VS designer tries to modify them. It may be worth adding the [Browsable(false), DesignerSerializationVisibility(DesignerSerializationVisibility.Hidden)] attributes to CodecSettings to avoid this.Generally , no it shouldn't matter whether an exception or assert is thrown because is should be caught during development time, but the exception gives a clearer idea what is wrong using the description while assert is probably more user friendly, allowing you to ignore and continue and see the resulting problem. In the end, I think that a comment here saying that this is expected to be overridden and the reason with an Assert is probably the best.

As it is, I am pretty much ready to submit my transcode tool patch, but it is now against your patch so I will wait until your patch is applied to the svn then build my new patch against the new revision rather than submitting a patch that contains what is in you patch plus changes.

too much wine and time for bed.....

Eamon

[berrinam]

If you also have a patch against svn, that would be good to see now, just for comments and not for actually committing.

[Maccara]

I'm reluctant for the moment simply because I have VS 2005 Pro installed and I have heard reports that VS2008 express does not play nice when installed side by side. Give it maybe a couple of weeks to see how things settle first?

Not sure of the Express editions, but gathering from the MSDN forums with Pro 2008 most of the problems seem to stem 1. having difficulty after having the CTP/Beta versions installed 2. Uninstalling 2005 (probably incorrectly - exactly right components in exactly right order have to be uninstalled) before installing 2008 (Full VS uninstalls never seem to go right :)).

I bit the bullet myself and installed VS 2008 Pro side-by-side with 2005 (still have some C++ code which needs to be updated to compile cleanly on 2008, so I still need 2005 for that - I actually have 2003 installed too as one customer project requires that) and everything seems ok so far (extensive testing is starting today - new windows SDK had changed some settings on 2005 but those were easily fixed, haven't checked 2003 yet). So for me, having 3 versions side-by-side seems to work just fine so far.

But I do agree with you - it might be better to wait a couple of weeks to sort out all the problem reports that are coming in. (although some of the C# 3.0 new features are tempting, and I would love there was a real project using it instead of my own testing only ;))

[berrinam]

More informative logtree names for jobs:
Index: core/gui/JobWorkerWindow.cs
===================================================================
--- core/gui/JobWorkerWindow.cs (revision 406)
+++ core/gui/JobWorkerWindow.cs (working copy)
@@ -497,8 +497,7 @@
{
try
{
- log = mainForm.Log.Info("Log for " + job.Name);
- log.LogValue("Job type", job.Job.EncodingMode);
+ log = mainForm.Log.Info(string.Format("Log for {0} ({1}, {2} -> {3})", job.Name, job.Job.EncodingMode, job.InputFileName, job.OutputFileName));
log.LogEvent("Started handling job");
log.Expand();

[berrinam]

Expand/collapse all for log:

Index: core/gui/LogTree.cs
===================================================================
--- core/gui/LogTree.cs (revision 406)
+++ core/gui/LogTree.cs (working copy)
@@ -132,5 +132,39 @@
}

}
+
+ private void expandOrCollapseAll(LogItem i, bool expand)
+ {
+ if (expand)
+ i.Expand();
+ else
+ i.Collapse();
+
+ foreach (LogItem i2 in i.SubEvents)
+ expandOrCollapseAll(i2, expand);
+ }
+
+ private void expandAll(LogItem i) { expandOrCollapseAll(i, true); }
+ private void collapseAll(LogItem i) { expandOrCollapseAll(i, false); }
+
+ private void expandLog_Click(object sender, EventArgs e)
+ {
+ expandAll(Log);
+ }
+
+ private void expandBranch_Click(object sender, EventArgs e)
+ {
+ expandAll(selectedLogItem);
+ }
+
+ private void collapseLog_Click(object sender, EventArgs e)
+ {
+ collapseAll(Log);
+ }
+
+ private void collapseBranch_Click(object sender, EventArgs e)
+ {
+ collapseAll(selectedLogItem);
+ }
}
}
Index: core/gui/LogTree.Designer.cs
===================================================================
--- core/gui/LogTree.Designer.cs (revision 406)
+++ core/gui/LogTree.Designer.cs (working copy)
@@ -38,6 +38,12 @@
this.saveToolStripMenuItem = new System.Windows.Forms.ToolStripMenuItem();
this.saveBranch = new System.Windows.Forms.ToolStripMenuItem();
this.saveLog = new System.Windows.Forms.ToolStripMenuItem();
+ this.expandAllSubitemsToolStripMenuItem = new System.Windows.Forms.ToolStripMenuItem();
+ this.expandLog = new System.Windows.Forms.ToolStripMenuItem();
+ this.expandBranch = new System.Windows.Forms.ToolStripMenuItem();
+ this.collapseAllSubitemsToolStripMenuItem = new System.Windows.Forms.ToolStripMenuItem();
+ this.collapseLog = new System.Windows.Forms.ToolStripMenuItem();
+ this.collapseBranch = new System.Windows.Forms.ToolStripMenuItem();
this.saveDialog = new System.Windows.Forms.SaveFileDialog();
this.contextMenu.SuspendLayout();
this.SuspendLayout();
@@ -55,9 +61,11 @@
//
this.contextMenu.Items.AddRange(new System.Windows.Forms.ToolStripItem[] {
this.editTextToolStripMenuItem,
- this.saveToolStripMenuItem});
+ this.saveToolStripMenuItem,
+ this.expandAllSubitemsToolStripMenuItem,
+ this.collapseAllSubitemsToolStripMenuItem});
this.contextMenu.Name = "contextMenuStrip1";
- this.contextMenu.Size = new System.Drawing.Size(153, 70);
+ this.contextMenu.Size = new System.Drawing.Size(173, 114);
//
// editTextToolStripMenuItem
//
@@ -66,27 +74,27 @@
this.editBranch,
this.editLog});
this.editTextToolStripMenuItem.Name = "editTextToolStripMenuItem";
- this.editTextToolStripMenuItem.Size = new System.Drawing.Size(152, 22);
+ this.editTextToolStripMenuItem.Size = new System.Drawing.Size(172, 22);
this.editTextToolStripMenuItem.Text = "Edit text";
//
// editIndividualNode
//
this.editIndividualNode.Name = "editIndividualNode";
- this.editIndividualNode.Size = new System.Drawing.Size(152, 22);
+ this.editIndividualNode.Size = new System.Drawing.Size(107, 22);
this.editIndividualNode.Text = "node";
this.editIndividualNode.Click += new System.EventHandler(this.ofIndividualNodeToolStripMenuItem_Click);
//
// editBranch
//
this.editBranch.Name = "editBranch";
- this.editBranch.Size = new System.Drawing.Size(152, 22);
+ this.editBranch.Size = new System.Drawing.Size(107, 22);
this.editBranch.Text = "branch";
this.editBranch.Click += new System.EventHandler(this.ofBranchToolStripMenuItem_Click);
//
// editLog
//
this.editLog.Name = "editLog";
- this.editLog.Size = new System.Drawing.Size(152, 22);
+ this.editLog.Size = new System.Drawing.Size(107, 22);
this.editLog.Text = "log";
this.editLog.Click += new System.EventHandler(this.editLog_Click);
//
@@ -96,7 +104,7 @@
this.saveBranch,
this.saveLog});
this.saveToolStripMenuItem.Name = "saveToolStripMenuItem";
- this.saveToolStripMenuItem.Size = new System.Drawing.Size(152, 22);
+ this.saveToolStripMenuItem.Size = new System.Drawing.Size(172, 22);
this.saveToolStripMenuItem.Text = "Save";
//
// saveBranch
@@ -113,6 +121,52 @@
this.saveLog.Text = "log";
this.saveLog.Click += new System.EventHandler(this.saveLog_Click);
//
+ // expandAllSubitemsToolStripMenuItem
+ //
+ this.expandAllSubitemsToolStripMenuItem.DropDownItems.AddRange(new System.Windows.Forms.ToolStripItem[] {
+ this.expandLog,
+ this.expandBranch});
+ this.expandAllSubitemsToolStripMenuItem.Name = "expandAllSubitemsToolStripMenuItem";
+ this.expandAllSubitemsToolStripMenuItem.Size = new System.Drawing.Size(172, 22);
+ this.expandAllSubitemsToolStripMenuItem.Text = "Expand all subitems";
+ //
+ // expandLog
+ //
+ this.expandLog.Name = "expandLog";
+ this.expandLog.Size = new System.Drawing.Size(120, 22);
+ this.expandLog.Text = "of log";
+ this.expandLog.Click += new System.EventHandler(this.expandLog_Click);
+ //
+ // expandBranch
+ //
+ this.expandBranch.Name = "expandBranch";
+ this.expandBranch.Size = new System.Drawing.Size(120, 22);
+ this.expandBranch.Text = "of branch";
+ this.expandBranch.Click += new System.EventHandler(this.expandBranch_Click);
+ //
+ // collapseAllSubitemsToolStripMenuItem
+ //
+ this.collapseAllSubitemsToolStripMenuItem.DropDownItems.AddRange(new System.Windows.Forms.ToolStripItem[] {
+ this.collapseLog,
+ this.collapseBranch});
+ this.collapseAllSubitemsToolStripMenuItem.Name = "collapseAllSubitemsToolStripMenuItem";
+ this.collapseAllSubitemsToolStripMenuItem.Size = new System.Drawing.Size(172, 22);
+ this.collapseAllSubitemsToolStripMenuItem.Text = "Collapse all subitems";
+ //
+ // collapseLog
+ //
+ this.collapseLog.Name = "collapseLog";
+ this.collapseLog.Size = new System.Drawing.Size(152, 22);
+ this.collapseLog.Text = "of log";
+ this.collapseLog.Click += new System.EventHandler(this.collapseLog_Click);
+ //
+ // collapseBranch
+ //
+ this.collapseBranch.Name = "collapseBranch";
+ this.collapseBranch.Size = new System.Drawing.Size(152, 22);
+ this.collapseBranch.Text = "of branch";
+ this.collapseBranch.Click += new System.EventHandler(this.collapseBranch_Click);
+ //
// saveDialog
//
this.saveDialog.Filter = "Log files (*.log)|*.log|All files (*.*)|*.*";
@@ -143,5 +197,11 @@
private System.Windows.Forms.ToolStripMenuItem saveBranch;
private System.Windows.Forms.ToolStripMenuItem saveLog;
private System.Windows.Forms.ToolStripMenuItem editLog;
+ private System.Windows.Forms.ToolStripMenuItem expandAllSubitemsToolStripMenuItem;
+ private System.Windows.Forms.ToolStripMenuItem expandLog;
+ private System.Windows.Forms.ToolStripMenuItem expandBranch;
+ private System.Windows.Forms.ToolStripMenuItem collapseAllSubitemsToolStripMenuItem;
+ private System.Windows.Forms.ToolStripMenuItem collapseLog;
+ private System.Windows.Forms.ToolStripMenuItem collapseBranch;
}
}

[berrinam]

Silently create worker if none exists (#1837578 (http://sourceforge.net/tracker/index.php?func=detail&aid=1837578&group_id=156112&atid=798479)):

Index: core/details/JobControl.cs
===================================================================
--- core/details/JobControl.cs (revision 406)
+++ core/details/JobControl.cs (working copy)
@@ -44,12 +44,8 @@
public void StartAll(bool restartStopping)
{
if (workers.Values.Count == 0)
- {
- DialogResult r = MessageBox.Show("Can't start queue because there are no workers. You can create one from the Workers menu. Do you want to create one now?",
- "Create new worker?", MessageBoxButtons.OKCancel, MessageBoxIcon.Question);
- if (r == DialogResult.OK)
- RequestNewWorker();
- }
+ NewWorker(freeWorkerName(), false);
+
foreach (JobWorker w in workers.Values)
if (!w.IsEncoding) w.StartEncoding(false);
else if (restartStopping && w.Status == JobWorkerStatus.Stopping) w.SetRunning();

[Eamon21]

If you also have a patch against svn, that would be good to see now, just for comments and not for actually committing.OK, will build one now and find somewhere to stick, I'll post the link once done.

One more breaking change from you earlier patch though is that AudioCodecSettings requires a parameterless constructor to be serialised, otherwise you can't add any audio jobs. I've just added a private blank oneprivate AudioCodecSettings(){}and all is good again.

Eamon

[berrinam]

Here's a new version of the patch with the two bugs Eamon pointed out fixed: http://megui.org/berrinam_patches/profiles_refactor/profiles_refactor_2.patch.

Haven't decide if it is better to throw the exception or to Assert, but think it should at least be one or the other.I decided on an exception because the C# compiler is aware of it and it doesn't raise the error, "not all code paths return a value".

One more breaking change from you earlier patch though is that AudioCodecSettings requires a parameterless constructor to be serialised, otherwise you can't add any audio jobs. I've just added a private blank oneprivate AudioCodecSettings(){}and all is good again.I fixed this by making AudioCodecSettings abstract, as it should be.

[Eamon21]

Ok, patch for transcode tool up at http://www.mytempdir.com/2067780

It's against SVN revision 405, but should be able to use it still. There are still a few things to be added and maybe a bit of work on the interface but basic functionality is there.

Fire in the comments/suggestions/problems.

The patch is huge at the moment because it was built against berrinam's earlier patch to refactor profiles, once that is committed, I'll build a new more compact one. The actual changes to the core are very minimal as I have kept everything as localised as possible with a few additional functions in a temporary helper class. Once it's accepted these will move to new more sensible homes. I think the only change to the core (appart from berrinam's patch) are a single line in the mainform to add the tool to the menu and the changes to the poject file to include the new files.

Eamon

[berrinam]

Interesting patch. I'll have to play with it some more, though.

I assume that the main focus of this tool is for recompressing already-encoded media files, which have their audio and video all-in-one and don't need preparation like VOB files? I'm afraid I don't have much experience in that field, so I don't know what kind of configuration is required. As a result, I'm sorry that my comments are going to appear negative, but it feels to me that some options are missing. (It is probably partly my excessive desire for control speaking.)

Some settings are missing, as I said. My first thoughts were:
Output filesize
Video resolution (although we've discussed this)
Keeping the original audio track
Track selection
Deinterlacing (automatic?)

I could believe it possible that these can be sacrificed if you are only shrinking already-encoded media files, but I'm interested in hearing your opinions, since this isn't a situation I am very familiar with.

I haven't looked much at the code yet, so I'm wondering what your approach to things like VOB files is. (Things which should really be run through DGIndex first). And how about media files with 0 audio tracks, or 2+ ?

The GUI isn't updating when you select Output location -> specify. The filebar remains disabled.

Also, as a matter of style, I mostly tend to avoid log.Info(). I like to use log.LogValue(,) a lot, and for the places where that isn't so appropriate, I use log.LogEvent(), which timestamps it (so that a post-mortem analysis of the log can reveal if anything took unusually long/short).

I think LogValue is especially useful and you should use it more often. For instance, you could replace
catch (Exception ex)
{
joblog.Add(new LogItem("Unable to create temporary AVS script file: " + inputAVSFile).Error(ex.ToString()));
return false;
}

with

catch (Exception ex)
{
joblog.LogValue("Error creating temporary AVS script file", ex, ImageType.Error);
return false;
}

which has the advantage that the logger will automagically display the exception, giving not only the message but also inner exceptions and stacktraces.

Just as a side note, I always smile when I see this (although I myself write it quite often):
Debug.Assert(mainForm != null);
mainForm.Log.Add(_log);

I know it acts as good documentation, but it somehow seems somehow pointless, as .NET will give us a perfectly good NullReferenceException if the Assert weren't there. :)

Also, the GUI hangs when jobs are being prepared; you should consider running the job preparation in another thread as it can take quite some time.

And it seems like you forgot to do SVN Add on the new files added by my patch to core/gui. They didn't appear in the patch, anyway.

[berrinam]

Oh, and I would really like to be able to right-click on that MultiFileSelector; it just invites it, and I'm disappointed when nothing appears. :P

[Eamon21]

Interesting patch. I'll have to play with it some more, though.

I assume that the main focus of this tool is for recompressing already-encoded media files, which have their audio and video all-in-one and don't need preparation like VOB files? I'm afraid I don't have much experience in that field, so I don't know what kind of configuration is required. As a result, I'm sorry that my comments are going to appear negative, but it feels to me that some options are missing. (It is probably partly my excessive desire for control speaking.)Yeah, the original reason I started this was to provide an easy way for me to transcode a large number of MJPEG *.Mov files from my digital camera to H264 as I have several gig worth after around 12 months travelling. Initial tests showed that I could use x264 and have files that looked pretty much identical but in some cases 10% of the original size using the const. qual option. Anyway I tried a program called SUPER and it was easy enough to use but not much control, not nearly as efficient as fully cranking the settings up in x264 and I don't care how long the encodes take, this is for storage.

In general, what is there vs what is missing comes down to two things, if it's not there (yet) then likely: I don't need it to transcode my digi cam movies and secondly, it was not straight forward to do with the current code/profiles/job types.


Some settings are missing, as I said. My first thoughts were:
Output filesize
Video resolution (although we've discussed this)
Keeping the original audio track
Track selection
Deinterlacing (automatic?)

I could believe it possible that these can be sacrificed if you are only shrinking already-encoded media files, but I'm interested in hearing your opinions, since this isn't a situation I am very familiar with.Well output size is something I've never really used but I guess I can see where some people might. I kind of feel that this belongs in a video profile though as it really applies to the encoder doesn't it? I'm more concerned with have a minimum quality than a specific file size, but then I never burn to CD/DVD media.

Video resolution and de-interlacing I think belong to the AVS profile and that is where it should be done. If you look at the CreateScript function in the TemporaryHelperClass, you can see where I have provisioned for them, getting the settings from the AVS profile. Again I make my arguments that these settings should be part of the AVS profile as it is the avs script that implements them.

Audio tracks: well I suspected this would be brought up and I had intended to add support to specify an alternate track based on a name template applied to the input file or not encode the audio at all, both easy enough to do. Direct stream copy of the audio is not as straight forward as it means I have to extract the stream myself somehow, probably a new type of job I guess similar to the DGIndex job. At the moment AVISynth makes it easy for me, it will grab the audio stream out of any file and pass it out as wav data. The additional effort and the fact that I don't need it for m initial purposes for the transcode meant that I have not pursued direct stream copy further. Similar reasons for track selection, I don't currently have an easy way of getting different tracks from a file, avisyth just grabs the default I think.


I haven't looked much at the code yet, so I'm wondering what your approach to things like VOB files is. (Things which should really be run through DGIndex first). And how about media files with 0 audio tracks, or 2+ ?I'm no DVD converting expert, not even close but I understand that there is a lot involved, I think it belongs in a more specialized tool which is really what the OneClickTool is supposed to do. Initially I considered trying to modify the OneClickTool to support more filetypes but ended putting that in the "Too Hard" basket. I deliberately set out NOT to develop a replacement for that though I did look at it a lot in trying to work out how to go about things.


The GUI isn't updating when you select Output location -> specify. The filebar remains disabled.On to it...sorted.


Also, as a matter of style, I mostly tend to avoid log.Info(). I like to use log.LogValue(,) a lot, and for the places where that isn't so appropriate, I use log.LogEvent(), which timestamps it (so that a post-mortem analysis of the log can reveal if anything took unusually long/short).

I think LogValue is especially useful and you should use it more often. For instance, you could replace
catch (Exception ex)
{
joblog.Add(new LogItem("Unable to create temporary AVS script file: " + inputAVSFile).Error(ex.ToString()));
return false;
}

with

catch (Exception ex)
{
joblog.LogValue("Error creating temporary AVS script file", ex, ImageType.Error);
return false;
}

which has the advantage that the logger will automagically display the exception, giving not only the message but also inner exceptions and stacktraces.I'm not fussed, there are no comments in the LogItem class to provide any guidance for usage, I just did the first thing that seemed logical. But I take your point for exception logging so will change that now too.


Also, the GUI hangs when jobs are being prepared; you should consider running the job preparation in another thread as it can take quite some time.Good idea with the new thread, had changed the cursor to an hourglass, but not quite sufficient :)

And it seems like you forgot to do SVN Add on the new files added by my patch to core/gui. They didn't appear in the patch, anyway.oops, just created the patch and only thought to add files I've added for the transcode. Wont be a problem when i make the next patch against the SVN once your patch makes it in there.

Finally, with the multifile selector, I intend to add things like right click and to add the icon associated with each file to the display, basically just got the minimum together first. Usability features like context menus come second :)

Thanks for the feedback.

Eamon

[Eamon21]

new patch that contains files added in berrinam's ealier patch that i left out last time :)
http://www.mytempdir.com/2067872

Eamon

[berrinam]

Here's another thought. When I used it at first, I was a bit scared because I didn't know what it was going to do with my files and I didn't know which ones would work. Some other tools which allow you to select multiple files read in the source information the moment you select the files, unlike your tool, which does it when you press add. I'm not sure, but that might be clearer for the user as they know from the start which ones can work.

Since it can take some time, you might also want to put a progress bar in a statusbar at the bottom of the window. (Yes, I know: usability features come second :))

[Eamon21]

Can anyone tell me what the deal with the Mediainfo.dll and the MediaInfoWrapper class? the dll in the svn is version 0.7.4.4 which is from 5th Feb 2007 (current version is 0.7.5.5)

Anyway, when I try to use it I get the wrong results, it doesn't seem to process the information properly.

I'm going to have a look through the MediaInfoWrapper solution and see if i can work out what is wrong, but is there a reason it is still using such an old version of the dll? Or is it just that no one else uses it so haven't bothered?

Eamon

[berrinam]

I have had no problems with Mediainofo 0.7.4.4 except a crash with Vista which I fixed in the MediaInfoWrapper class a while ago. However, other people have since complained of MediaInfo crashes under Vista. As I don't run Vista, I am somewhat separated from this situation. I believe we plan to update to 0.7.5.5 soon, but we may also have to accompany that with some changes to the OpenFileDialog calls (don't ask me why... you can read the last pages of this thread to get some details, though).

What are these wrong results it gives you?

[Kurtnoise]

the dll in the svn is version 0.7.4.4 which is from 5th Feb 2007
This is wrong. I updated to the latest build few days ago. It's still the old one on auto-update servers though (which overwrites the new one when we use the update)...

[Eamon21]

This is wrong. I updated to the latest build few days ago. It's still the old one on auto-update servers though (which overwrites the new one when we use the update)...
OK, I seem to have the latest version now. Still having the same problems though..

I have had no problems with Mediainofo 0.7.4.4 except a crash with Vista which I fixed in the MediaInfoWrapper class a while ago. However, other people have since complained of MediaInfo crashes under Vista. As I don't run Vista, I am somewhat separated from this situation. I believe we plan to update to 0.7.5.5 soon, but we may also have to accompany that with some changes to the OpenFileDialog calls (don't ask me why... you can read the last pages of this thread to get some details, though).

What are these wrong results it gives you?
When I try to create a MediaInfoFile, stepping into the constructor and viewing the second lineMediaInfo info = new MediaInfo(file);if I put a watch on the info object after it is created I see the following:

- info {MediaInfoWrapper.MediaInfo} MediaInfoWrapper.MediaInfo
+ Audio Count = 0 System.Collections.Generic.List<MediaInfoWrapper.AudioTrack>
AudioCount 1 int
+ Chapters Count = 0 System.Collections.Generic.List<MediaInfoWrapper.ChaptersTrack>
ChaptersCount 0 int
+ General Count = 0 System.Collections.Generic.List<MediaInfoWrapper.GeneralTrack>
GeneralCount 1 int
InfoComplete ...snip...
InfoCustom "General\r\n\r\nVideo\r\n\r\nAudio\r\n\r\nText\r\n\r\nChapters\r\n" string
InfoStandard ...snip...
+ Text Count = 0 System.Collections.Generic.List<MediaInfoWrapper.TextTrack>
TextCount 0 int
+ Video Count = 0 System.Collections.Generic.List<MediaInfoWrapper.VideoTrack>
VideoCount 1 int
+ Non-Public members now this file definately has both audio(aac) and video (H264) but the information doesn't seem to be beign parsed correctly. I have cut the "InfoComplete" filed here because it messed up the formatting, but it contains the following:General #0
Count : 176
Count of stream of t : 1
Kind of stream : General
StreamKindID : 0
Count of video strea : 1
Count of audio strea : 1
Count of text stream : 0
Count of chapter str : 0
Codecs Video : H.264
Audio codecs : AAC LC
Complete name : C:\b\a-muxed.mp4
Folder name : C:\b
File name : a-muxed
File extension : mp4
Format : isom
Format : MPEG-4
Format/Info : ISO 14496-1 Base Media
Format/Family : MPEG-4
Format/Url : http://www.apple.com/quicktime/download/standalone.html
Format/Extensions : mp4
Codec : isom
Codec : MPEG-4
Codec/Info : ISO 14496-1 Base Media
Codec/Family : MPEG-4
Codec/Url : http://www.apple.com/quicktime/download/standalone.html
Codec/Extensions : mp4
File size : 755388
File size : 738 KiB
File size : 738 KiB
File size : 738 KiB
File size : 738 KiB
File size : 737.7 KiB
PlayTime : 3326
PlayTime : 3s 326ms
PlayTime : 3s 326ms
PlayTime : 3s 326ms
PlayTime : 00:00:03.326
Bit rate : 1816928
Bit rate : 1817 Kbps
StreamSize : 2518
StreamSize : 2.46 KiB
StreamSize : 2 KiB
StreamSize : 2.5 KiB
StreamSize : 2.46 KiB
StreamSize : 2.459 KiB
Encoded date : UTC 2007-11-15 12:29:07
Tagged date : UTC 2007-11-15 12:29:07

Video #0
Count : 73
Count of stream of t : 1
Kind of stream : Video
StreamKindID : 0
ID : 1
Codec : avc1
Codec : H.264
Codec/Info : H.264 (3GPP)
Codec/Url : http://www.apple.com/quicktime/download/standalone.html
PlayTime : 3000
PlayTime : 3s
PlayTime : 3s
PlayTime : 3s
PlayTime : 00:00:03.000
Bit rate : 1970858
Bit rate : 1971 Kbps
Width : 640
Width : 640 pixels
Height : 480
Height : 480 pixels
Aspect ratio : 1.333
Aspect ratio : 4/3
Frame rate : 30.000
Frame rate : 30.000 fps
FrameCount : 90
Bits/(Pixel*Frame) : 0.189
StreamSize : 739072
StreamSize : 722 KiB
StreamSize : 722 KiB
StreamSize : 722 KiB
StreamSize : 722 KiB
StreamSize : 721.8 KiB
Encoded date : UTC 2007-11-15 12:29:07
Tagged date : UTC 2007-11-15 12:29:07

Audio #0
Count : 58
Count of stream of t : 1
Kind of stream : Audio
StreamKindID : 0
ID : 2
Codec : A_AAC/MPEG4/LC
Codec : AAC LC
Codec/Info : AAC Low Complexity
PlayTime : 3328
PlayTime : 3s 328ms
PlayTime : 3s 328ms
PlayTime : 3s 328ms
PlayTime : 00:00:03.328
Bit rate : 33168
Bit rate : 33 Kbps
Bit rate mode : VBR
Channel(s) : 1
Channel(s) : 1 channel
Sampling rate : 8000
Sampling rate : 8000 Hz
SamplingCount : 26624
Resolution : 16
Resolution : 16 bits
StreamSize : 13798
StreamSize : 13.5 KiB
StreamSize : 13 KiB
StreamSize : 13 KiB
StreamSize : 13.5 KiB
StreamSize : 13.47 KiB
Encoded date : UTC 2007-11-15 12:29:07
Tagged date : UTC 2007-11-15 12:29:07so you can see the information is there but it doesn't seem to be parsed properly, going to have a quick look at the MediaInfoWrapper project now and see what I can find out.

That brings me to another question though, is there a particular reason that the MediaInfoWrapper project is not part of the same solution?

Eamon

[Eamon21]

Appears to be something wrong with the MediaInfoWrapper.dll. If I use the one in the SVN i.e.
Size: 68.0 KB (69,632 bytes)
Version: 0.7.5.5

I have problems but if I run through the MediaInfoWrapper project it works, if I compile it myself from the MediaInfoWrapper project (Debug or release) and copy that dll to the MeGui directory it works, and finally if I include the MediaInfoWrapper project in the MeGui solution and change the reference in MeGui to the project instead of the dll, it works.

Can anyone else replicate this?
Simply create a MediaInfoFile and see if it has populated the fields correctly.


maybe easier to just apply this patch and try to create a MediaInfoFile
Index: MediaInfoFile.cs
===================================================================
--- MediaInfoFile.cs (revision 405)
+++ MediaInfoFile.cs (working copy)
@@ -228,6 +228,8 @@
{
this.file = file;
MediaInfo info = new MediaInfo(file);
+ System.Diagnostics.Debug.Assert(info.AudioCount == info.Audio.Count);
+ System.Diagnostics.Debug.Assert(info.VideoCount == info.Video.Count);

bool hasVideo = (info.Video.Count > 0);

Eamon

[Kurtnoise]

what's the size of your own MediaInfoWrapper.dll ?

[Eamon21]

well in debug mode, it is 76.0 KB (77,824 bytes) but when I compile as release, it is actually exactly the same size as the one that causes me problems.

Eamon

[Kurtnoise]

More informative logtree names for jobs:
...
looks ok.

Expand/collapse all for log:...
looks ok.

Silently create worker if none exists (#1837578):...
looks ok.

[Sharktooth]

dev:0.3.0.1001 (includes 0.2.x branch fixes)
- (berrinam) Add missing output streams back to log (fixes #1836281)
- (Kurtnoise) [AutoEncodeWindow] mainform was being referenced in default constructor before it had ever
been assigned (#1836041). patch by Eamonh.
- (Kurtnoise) Removed unused code
- (Kurtnoise) more Audio Input FileType.
- (Kurtnoise) [ProgressWindow] Remaining Time -> Time Remaining.
stable:0.2.6.1040
- (Kurtnoise) [MainForm] move AutoUpdate checking to the shown Event (to have the MessageBox in the foreground). bob0r should be happy now...;-)
- (Kurtnoise) Added several new languages.
- (Kurtnoise) [OneClickWindow] re-ordered the initOneClickHandler() (#1832675). patch by Eamonh.
- (Kurtnoise) Updated MediaInfo.dll and his wrapper.

[Kurtnoise]

ok...I would like to have your opinion guys about Vista stuff. To sum up : the main issue concerns the preview window which can't be opened properly. Basically, it's due to the openfiledialog bug. To resolve this without some extra tweakings or tunings, an upgrade to the last .Net Framework is sufficient (maybe the 2.0 SP1 is enough...dunno). I tested this during all the weekend and I got no problem so far.

So, what can we do now ? I suggest to raise a warning during the mainform loading to inform people who run on Vista and who don't have the appropriate framework to upgrade. I've already the patch for that. ;)

[Sharktooth]

with "latest" you mean 3.5? or latest versions of 2.0?

[Kurtnoise]

Some extra comments...

if I choose the .Net 2.0 as target framework, it's the 2.0 SP1 which is used.
if I choose the .Net 3.5 as target framework, it's the 3.5 RTM which is used.
both builds work flawlessly...:)


Note: when we install the .Net 3.5, there is also an update for the 2.0 (i.e an upgrade to the SP1) wich adds some methods and properties, required for the Framework 3.5 features such as LINQ, to the BCL classes in the Framework 2.0. These changes do not affect applications written for version 2.0 of the .NET Framework.

[Eamon21]

Some extra comments...

if I choose the .Net 2.0 as target framework, it's the 2.0 SP1 which is used.
if I choose the .Net 3.5 as target framework, it's the 3.5 RTM which is used.
both builds work flawlessly...:)


Note: when we install the .Net 3.5, there is also an update for the 2.0 (i.e an upgrade to the SP1) wich adds some methods and properties, required for the Framework 3.5 features such as LINQ, to the BCL classes in the Framework 2.0. These changes do not affect applications written for version 2.0 of the .NET Framework.I would vote for sticking with 2.0 for now, I suspect there would be a lot of unhappy campers running win2k who would flood into the forums if they could no longer run it.

Secondly, I just want to check the purpose of the MediaInfoWrapper.dll, I suspect that it is just there to wrap the MediaInfo.dll and ensure it is disposed of cleanly with all handles and non-managed memory cleaned up?

Eamon

[berrinam]

That brings me to another question though, is there a particular reason that the MediaInfoWrapper project is not part of the same solution?MediaInfoWrapper was provided to MeGUI by someone else, as a complete project that you see in the SVN. We simply never joined it into the main solution. However, better than adding the project to the solution would be just adding the files to the MeGUI project. Currently there are two stages of interpretation (DLL->strings, strings->MeGUI values) which could perhaps be simplified if it was all in the main project.

ok...I would like to have your opinion guys about Vista stuff. To sum up : the main issue concerns the preview window which can't be opened properly. Basically, it's due to the openfiledialog bug. To resolve this without some extra tweakings or tunings, an upgrade to the last .Net Framework is sufficient (maybe the 2.0 SP1 is enough...dunno).
...
So, what can we do now ? I suggest to raise a warning during the mainform loading to inform people who run on Vista and who don't have the appropriate framework to upgrade. I've already the patch for that. ;)
...
if I choose the .Net 2.0 as target framework, it's the 2.0 SP1 which is used.
...
both builds work flawlessly...:)

If I understand correctly, it all works if the users run 2.0 SP1? If that is available to Win2k users as well, then that seems the best solution, so let's do that.

Secondly, I just want to check the purpose of the MediaInfoWrapper.dll, I suspect that it is just there to wrap the MediaInfo.dll and ensure it is disposed of cleanly with all handles and non-managed memory cleaned up?Yes, although when I looked at it recently, it doesn't appear to delete the handle. I'm not sure if that's a bug...

[kumi]

I just wanted to note that the 0.2.6.1040 stable upgrade has resolved my MediaInfoWrapper.dll problem:

https://sourceforge.net/tracker/index.php?func=detail&aid=1832210&group_id=156112&atid=798477

[Eamon21]

Yes, although when I looked at it recently, it doesn't appear to delete the handle. I'm not sure if that's a bug...Yeah, was going to point this out. Though it does have a Dispose() method, it doesn't actually implement the IDisposable interface, also, the way Dispose() was being used was not complete and the unmanaged resources (handles etc) only got cleaned up if Dispose was explicitly called and therefore a potential memory leak if the object was left to be collected by the garbage collector.

I have since made it IDisposable and implemented a better dispose pattern for now and I will look at moving the code into MeGui too as that would resolve another thing I was going to bring up and that is the MediaInfoFile class wraps the MediaInfoWrapper.dll which wraps MediaInfo.dll, an uneccessary extra layer as you point out.

Eamon

[berrinam]

looks ok.
...
looks ok.
...
looks ok.

Can someone else commit these three patches please? I've got computer troubles at the moment.

[berrinam]

I understand that people would like to have some of the x264's experimental (non-SVN) features supported in MeGUI. I reckon we can support experimental and vanilla x264 at the same time in MeGUI, giving the user the choice.

What the user will see is one more codec. Instead of the 4 video codecs being (as we currently have) x264, XviD, LMP4, and Snow, there would be 5: x264, x264-experimental, XviD, LMP4, and Snow. As MeGUI treats it as an entirely separate codec, there will be no problems of the two interfering, and the user can choose to stick with vanilla x264 just by using the plain 'x264' codec. Naturally, we would also have to add another video encoder to MeGUI, corresponding to the different build of x264.

As far as code reuse goes, I think inheritance pretty much does the trick. We derive x264ExperimentalSettings from x264Settings, and x264ExperimentalConfigurationPanel from x264ConfigurationPanel, and x264ExperimentalEncoder from x264Encoder.

What do you think?

[berrinam]

There's some code in the Bitrate Calculator tool which allows the calculated bitrate to be saved back into the selected video profile. I find it a bit hacky, and I don't really expect that anyone uses it much: if you want to encode to a given filesize, then the AutoEncoder generally is easier to use. I think we should consider removing this as it seems pointless.

[Kurtnoise]

If I understand correctly, it all works if the users run 2.0 SP1? If that is available to Win2k users as well, then that seems the best solution, so let's do that.
According to the webpage (http://www.microsoft.com/downloads/details.aspx?familyid=79BC3B77-E02C-4AD3-AACF-A7633F706BA5&displaylang=en), XP or Server 2003 is required...

Anyway, my notice was for Vista users only...;)

Here is the pseudo-code:
if (OS_type == Windows Vista)
{
if (Runtime == 2.0 SP1 || Runtime == 3.0 || Runtime == 3.5)
do nothing;
else {
if (MessageBox.Show("You're running MeGUI on Windows Vista but to avoid several issues, may I ask you to upgrade your .Net Framework ?\n", "Warning", MessageBoxButtons.YesNo, MessageBoxIcon.Warning) == DialogResult.Yes)
run http://www.microsoft.com/downloads/details.aspx?familyid=79BC3B77-E02C-4AD3-AACF-A7633F706BA5&displaylang=en;
}
}

[Kurtnoise]

As far as code reuse goes, I think inheritance pretty much does the trick. We derive x264ExperimentalSettings from x264Settings, and x264ExperimentalConfigurationPanel from x264ConfigurationPanel, and x264ExperimentalEncoder from x264Encoder.

What do you think?
you read my mind...;) it's exactly what I thought this weekend.

[Kurtnoise]

Can someone else commit these three patches please? I've got computer troubles at the moment.
ok, done...

[Sharktooth]

There's some code in the Bitrate Calculator tool which allows the calculated bitrate to be saved back into the selected video profile. I find it a bit hacky, and I don't really expect that anyone uses it much: if you want to encode to a given filesize, then the AutoEncoder generally is easier to use. I think we should consider removing this as it seems pointless.
i use it :)
for what concerns x264 experimental builds, i dont think we should add them since it will require continous effort in updating support for that. plus there are a lot of different builds with different command line options... it will just be a PITA.

[rack04]

There's some code in the Bitrate Calculator tool which allows the calculated bitrate to be saved back into the selected video profile. I find it a bit hacky, and I don't really expect that anyone uses it much: if you want to encode to a given filesize, then the AutoEncoder generally is easier to use. I think we should consider removing this as it seems pointless.

I use it. Haven't tried AutoEncoder though.

[Sharktooth]

maybe we should "ask" the user if he wants to update the bitrate value of the currently selected profile or not.
that would be a much better idea.

[Kurtnoise]

We have definitively a big issue with the last MediaInfoWrapper release. :( Check out the bugtracker. When we use the muxers, FPS value is not detected anymore. Using the old one (0.7.4.4), that works...

Eamon: if you've fixed some things in it, that could be great to test it...at least a patch. :)

[Sharktooth]

dang... maybe the api changed? i didnt look at the changelog

[Eamon21]

We have definitively a big issue with the last MediaInfoWrapper release. :( Check out the bugtracker. When we use the muxers, FPS value is not detected anymore. Using the old one (0.7.4.4), that works...

Eamon: if you've fixed some things in it, that could be great to test it...at least a patch. :)Don't know what it was exactly but the last mediinfowrapper.dll (0.7.5.5) was just broken, a recompile with the same source code fixed everything for me, dll is still version 0.7.5.5 but works now. That simple patch I posted earlier would show the problem first time you tried to create MediaInfoFile where the number of streams was correct but it just was not populating them with info.

I have started trying to move the code in MediaInfoWrapper into MeGui, but I think it will be a while before I am done, in the meantime, my re-compiled mediainfowrapper.dll should fix the issues, well it did for me. I have uploaded it to http://www.mytempdir.com/2069458. Updated link to point to newly compile dll that actually is version 07.5.5

Or if someone wants to just recompile the dll them self, I have made slight changes to the dispose functionality, though nothing that would explain the previous problems. Here is the patchIndex: trunk/MediaInfoWrapper/MediaInfoWrapperDll/MediaInfoMain.cs
===================================================================
--- trunk/MediaInfoWrapper/MediaInfoWrapperDll/MediaInfoMain.cs (revision 405)
+++ trunk/MediaInfoWrapper/MediaInfoWrapperDll/MediaInfoMain.cs (working copy)
@@ -46,10 +46,8 @@
/// every information MediaInfo.dll can collect.
/// Tracks are accessibles as properties.
/// </summary>
- public class MediaInfo
+ public class MediaInfo : IDisposable
{
-
-

[DllImport("MediaInfo.dll")]
internal static extern int MediaInfo_Close(IntPtr Handle);
@@ -72,7 +70,7 @@
[DllImport("MediaInfo.dll")]
internal static extern int MediaInfo_State_Get(IntPtr Handle);

-
+
private List<VideoTrack> _Video;
private List<GeneralTrack> _General;
private List<AudioTrack> _Audio;
@@ -87,7 +85,7 @@
private string _InfoStandard;
private string _InfoCustom;
private string _FileName;
- private bool disposedValue;
+
private IntPtr Handle;
//public static const string MediaInfoPath="MediaInfo.dll";

@@ -103,35 +101,76 @@
//if (!CheckFileExistence("MediaInfo.dll")) return;
if (!CheckFileExistence(path)) return;
_FileName = path;
-
+
this.Handle = MediaInfo.MediaInfo_New();
MediaInfo.MediaInfo_Open(this.Handle, path);
+ try
+ {
+ getStreamCount();
+ getAllInfos();
+ }
+ finally //ensure MediaInfo_Close is called even if something goes wrong
+ {
+ MediaInfo.MediaInfo_Close(this.Handle);
+ }

- getStreamCount();
- getAllInfos();
+ }

- MediaInfo.MediaInfo_Close(this.Handle);
+ #region Disposable Pattern
+ private bool disposed;

+ ~MediaInfo()
+ {
+ Dispose(false);
}
+
+ protected bool IsDisposed
+ {
+ get { return disposed; }
+ }
+
+ protected void CheckDisposed()
+ {
+ if (disposed)
+ {
+ throw new ObjectDisposedException(this.ToString());
+ }
+ }
+
/// <summary>Call this one to kill the wrapper, and close his handle to the MediaInfo.dll, you should never need it anyway </summary>
public void Dispose()
{
this.Dispose(true);
GC.SuppressFinalize(this);
}
-
-
-
- /// <summary>Call this one to kill the wrapper, and close his handle to the MediaInfo.dll, you should never need it anyway </summary>
+
+ /// <summary>
+ ///
+ /// </summary>
+ /// <param name="disposing"></param>
protected virtual void Dispose(bool disposing)
{
- if (!this.disposedValue && disposing)
+ if (!this.disposed)
{
- MediaInfo.MediaInfo_Close(this.Handle);
- MediaInfo.MediaInfo_Delete(this.Handle);
+ if (disposing)
+ {
+ DisposeManagedResources();
+ }
+ DisposeUnmanagedResources();
}
- this.disposedValue = true;
+ this.disposed = true;
}
+
+ protected virtual void DisposeManagedResources()
+ {
+ }
+ protected virtual void DisposeUnmanagedResources()
+ {
+ MediaInfo.MediaInfo_Close(this.Handle);
+ MediaInfo.MediaInfo_Delete(this.Handle);
+ }
+ #endregion
+
/// <summary>
/// Simply checks file presence else throws a FileNotFoundException
/// </summary>
@@ -780,5 +819,6 @@
return this._ChaptersCount;
}
}
+
+ }
}
-}

But as I said, I think it will probably be better to incorporate the code from mediainfowrapper into MeGui and effectively merge mediainfowrapper and mediainfofile.

Eamon

[Sharktooth]

i agree. however in the meanwhile i will re-compile the wrapper and update the packages ASAP.

[Kurtnoise]

dohh...my bad. It was my fault. :scared: I disabled a function in the wrapper when I've made some tests earlier. :o:o:o


Sorry guys. I'll upload the new one tonight...

[Sharktooth]

ok... :)

EDIT: btw, i moved to VS2008

[Eamon21]

It would still be worth applying the patch I posted for MediaInfoWrapper with the new dispose functionality as there is currently a memory leak. Where the class MediaInfoWrapper.MediaInfo is used (e.g. in MediaInfoFile.cs, line 230) it is not currently disposed and the previous dispose pattern used did not clean up unmanaged resources if it was cleaned up by the garbage collector, only if someone explicitly called Dispose() was the handle actually deleted.

Eamon

[Kurtnoise]

yeah...I applied your patch.