MeGUI development

[Doom9]

For example (as said before), if i choose High profile, then enable all the macroblock options (such as adaptive DCT) and then i change to main profile and back to high profile, those boxes are no longer checked.Then who the heck broke that? It all worked nicely when I released my last build. So in the meantime somebody broke the functionality and all the confusion stems from that.

So let me say it once and for all: for the x264 configuration dialog, if an option is not to be accessible, it keeps its current value, but it's disabled. The rest is done in the x264TristateAdjustment method in the CommandlineGenerator class. It's rather frustrating for me to having spent hours to get this darned thing working only to have it broken again by somebody else :(

[Sharktooth]

i think the behaviour was modified when the whole tri-state was rewritten.
i've a local copy of the old method. i can restore it in few minutes (i'll do it tonight).

[godhead]

berrinam, go ahead and put me down for the following, since I already know the MP4 muxer and commandlinegenerator code from my work on track names.

Add support for the -sbr or -sbrx tags in mp4box commandline
Description: Add a checkbox in MeGUI MP4 Muxer which says something like "AAC is SBR", so that the audio information is correctly stored in MP4
Status : No-one is working on it

[Doom9]

Add support for the -sbr or -sbrx tags in mp4box commandlinePlease do not forget that this involves the automatic setting of that checkbox in auto-mode / one click mode when audio is being encoded to aac. That makes it a bit less than trivial ;)

[Richard Berg]

Isn't a disabled control essentially not playing any role?
No. A control that's disabled but checked means "I'm enabled and there's nothing you can do about it." The most common example of this state comes from installers. Typically you can check or uncheck optional components, but components required to make the program work will be checked-and-disabled.

[stax76]

No. A control that's disabled but checked means "I'm enabled and there's nothing you can do about it." The most common example of this state comes from installers. Typically you can check or uncheck optional components, but components required to make the program work will be checked-and-disabled.

You are both right, it depends on the case e.g. is it a child etc., a disabled control can also mean it has no relevance.

[berrinam]

Please do not forget that this involves the automatic setting of that checkbox in auto-mode / one click mode when audio is being encoded to aac. That makes it a bit less than trivial ;)
Actually, it turns out this isn't necessary. As far as I can see, all aac encoding by MeGUI is put straight into the mp4 container, and -sbr signalling isn't required with mp4 input, because the info is already there. This option is only really useful for people who are using externally encoded aac files.

[Doom9]

This option is only really useful for people who are using externally encoded aac files.Hmm, you're right. But it's something to be kept in mind if the winamp aac encoder is ever to be supported.

[godhead]

Please do not forget that this involves the automatic setting of that checkbox in auto-mode / one click mode when audio is being encoded to aac. That makes it a bit less than trivial ;)

Noted.

[Richard Berg]

What's the best way to generate a patch so it's easier for you to accept submissions?

[berrinam]

Noted.
godhead, it's not necessary to do that. See my post just below Doom9's.

[dimzon]

@all dev's
Hi!
I was @ new year vacation and I spend this time with my family mostly far from PC. But I found time to take closer look @ MeGUI sources. And I'm frustrating now!
I found some amount of non-optimal code architecture, including annoyng copy/paste programming paradigm... It's a really not good!
I really don't understand some decisions in it!
Take look @ audio-related staff. Why all configuration forms are created by copy/paste metodology? Hey, forms is objects and you can still use inheritance/poliphormysm with it! I really don't understand why does You use ugly ENUM<->Int casting, why not just use enums?
In other case I don't understand unnecesarry using of "Property Paradigm":

int someProperty;
public int SomeProperty
{
get{return someProperty;}
set{somePropert=value;}
}

why not use just fields to decrease code size:

public int SomeProperty;

(don't forget - You can switch from field to property @ first request in 1 minute!)

I believe we must stop implementing any additional features until full code review and refactoring!

[Doom9]

1) Forms: Please feel free to lay down a good foundation for GUI inheritance.. I only have limited experience with it, hence I implemented them all as separate forms but I do agree that it would make more sense and be easier to maintain to have basis forms and forms that derive from it. The same obviously also goes for the video config, muxer and settings forms. So when you start working at the audio, please do lay the groundwork for a major improvement in this area.

2) enums <-> ints... once again plain ugly and my fault.. I'm just not sure if you can bind enums to dropdowns.. I guess that's why I started with ints, then figured for code maintainability it would be better if the names of the options were spelled out, hence the enums. You'll see that in more recent codes, there are no such conversions anymore and I'm using the enums directly, as it should be. Once again, if you start working on the audio, please feel free to change that.

3) There however I have to disagree with you. Variables should not be declared public. In Java we have setters and getters to change/get variables, in .NET we have properties. In VS2k5 you can easily define your private variable, then using the refactoring functionality turn this into a property with about two clicks (I don't recall the exact procedure.. it's been a while since I watched that "what's new" video). And in both Java classes at university as well as coding practices for C# they tell you that accessing variables directly is bad style of programming. For most variables, you don't need any accessors because they are only used within the class where they are declared, but those that are to be accessed externally should use properties.

[berrinam]

I believe we must stop implementing any additional features until full code review and refactoring!
Although I agree with you, and it certainly would be nice to see the code more organised, this would mean a complete halt to development for a considerable time period. Could this code clean-up not be done gradually?

[dimzon]

3) There however I have to disagree with you. Variables should not be declared public. In Java we have setters and getters to change/get variables, in .NET we have properties. In VS2k5 you can easily define your private variable, then using the refactoring functionality turn this into a property with about two clicks (I don't recall the exact procedure.. it's been a while since I watched that "what's new" video). And in both Java classes at university as well as coding practices for C# they tell you that accessing variables directly is bad style of programming. For most variables, you don't need any accessors because they are only used within the class where they are declared, but those that are to be accessed externally should use properties.
C# is not Java. "Property Paradigm" is good for java @ early development stage - if you decide to add some logic @ properties later you does not need to replace direct field access by getter/setter call. But in C# it looks same:
obj.SomeFileld = value;
obj.SomeProperty = value;
So you can easy convert public field to property without massive codechange.
Until you does not expose your objects from MeGUI it's safe to use public fields instead of primitive public properties!

[dimzon]

this would mean a complete halt to development for a considerable time period
Definitly YES! Code structure will be significally changed during cleanup/refactoring. It's impossible to do such massive changes without halting development... I'm afraid everything must be altered and, maybe, backward compatiblity (saved jobs and profiles) will be broken...

Now let's discuss about MeGUI architecture.
I'm strongly prefer to keep main MeGUI architecture as simple as possible. As fact megui is job-based sofware so Job && JobList && JobExecutor is main GUI-less abstract classes. Another abstract classes would be JobProvider - some component wich provide some GUI for concrete job creation, tuning and tweaking and JoblessUtility - some additional visual staff (like bitrate calculator etc.)
AudioJobProvider/VideoJobProvider/MuxJobProvider implementations itself can contain multiple configureable and extensible components (pattern Facade)...
MeGUI must provide JobList management functionality and act as a host for multiple JobProviders and JoblessUtilities. It must provide additional bridge between JobProvider and JoblessUtility (for AviSynth script creation or bitrate calculator).

[Doom9]

Actually, you can use properties as full blown methods... the StatusUpdate contains a few examples, and there are some in other classes as well. For consistency purposes and design, every variable that needs to be available publicly should be wrapped in a property. Anything internal can stay a simple variable. This has a lot of advantages. Even inside the same project, with properties come nice descriptions that show up when using the commandline completion.. so even somebody not quite familiar with a class will be able to use those properties, whereas the use of the internal variables actually requires the study of the source.

JoblessUtility - some additional visual staff (like bitrate calculator etc.)You're getting a bit overboard here.. it's like you're trying to wrap in a job what isn't one.. I don't think that's a good idea, in fact that makes the whole thing appear stiff and artificial.

Another abstract classes would be JobProvider - some component wich provide some GUI for concrete job creation, tuning and tweaking andThat sounds nice an all and corresponds what I'm currently doing with the video encoding part (an incremental upgrade for the whole project though, first swap out just one part, then the next one, etc), but here's the problem with your approach: There's a great amount of interdependency between a VideoJobProvider and an AudioJobProvider or MuxJobProvider. For instance, if you're using x264 and mencoder and mp4 output, you don't get a single video job, or two or three, but another one that's a muxjob. Things get even more complex when you look at the auto-encoding section, which outputs video, audio and muxjobs at the same time. And then we have the one click mode.. which outputs all kinds of jobs at the same time (well.. almost). Then there's all the job postprocessing after a job.. where does that go? The Main GUI is much more than a VideoJobProvider and AudioJobProvider (and since there's no multiple inheritance in .NET, this already doesn't work and we have to move to interfaces).

I have already done a major refactoring once, redesigned a lot of the classes, and while in the end it's all nicer, it meant a really long time without even so much as a bugfix. Considering how MeGUI has grown, I don't think that's such a good idea. An incremental update, as I have begun to outline, where different parts of the entire project are marked off "under construction" and will be modified while the rest of the program continues to live, is a much more practicable approach.. it also lowers the pressure in having to get things done the right way right now as it will not be possible to make any big changes in the near future.

We can certainly implement many of your suggestions (e.g. JobExecutor, although the name is horrible, the concept is good, same for JobList), but I think it would be better to hammer out the details and do the whole thing in small steps so as to not break everything at once and then having to deal with hundreds of not thousands of compilation errors when trying to put it together, and then the millions of smallish bugs that will have crept in.

[Sharktooth]

i think the behaviour was modified when the whole tri-state was rewritten.
i've a local copy of the old method. i can restore it in few minutes (i'll do it tonight).
Done and CVS updated:

0.2.3.2003 10 Jan 2006
Restored x264 tri-state the way it was before the whole tri-state was rewritten to a single method (that implies grayed out controls means also they're disabled in the commandline)

However please do some testing and verify EVERY grayed out control is disabled in commandline :)

[Doom9]

Restored x264 tri-state the way it was before the whole tri-state was rewritten to a single method I really wonder what the rewrite contained then? It was meant to just mean the grouping of all this logic in one single method, but apparently somebody completely switched the way things were handled back to the two-state era.

[Sharktooth]

Dont remember who did it, but if you check the changelog it should be there (i think berrinam or charleski)
Also, x264 rev398 has a new switch "--nr <int>" (noise reduction, default value is 0).

[dimzon]

@Doom9
Ok, I understand You, lets continue architecture discussion.
1-st let's decide what kind of service must MeGUI provide for JobProvider. IMHO:

Centralized API for profiles/settings persistance
Centralized API for JobList navigation (to enum current jobs for dependant jobs )


let's decide what kind of information must Job provide. IMHO:

DependsOn - list of jobs to be complete before currenct job can execute
UniqueID - some unique id (maybe GUID?) for manajement puposes
Some virtual method or property set to fill JobList ListView item
Executor - method or property to obtain JobExecutor for current job
OutputFileNames - method or property to obtain list of output files

That sounds nice an all and corresponds what I'm currently doing with the video encoding part (an incremental upgrade for the whole project though, first swap out just one part, then the next one, etc), but here's the problem with your approach: There's a great amount of interdependency between a VideoJobProvider and an AudioJobProvider or MuxJobProvider. For instance, if you're using x264 and mencoder and mp4 output, you don't get a single video job, or two or three, but another one that's a muxjob. Things get even more complex when you look at the auto-encoding section, which outputs video, audio and muxjobs at the same time. And then we have the one click mode.. which outputs all kinds of jobs at the same time (well.. almost). Then there's all the job postprocessing after a job.. where does that go? The Main GUI is much more than a VideoJobProvider and AudioJobProvider (and since there's no multiple inheritance in .NET, this already doesn't work and we have to move to interfaces).

Ok, seems like abstract JobProvider is a really wrong idea... In other side we does not need polyphormism for Audio/Video job providers - all what we really need is ployphormism for JobExecutors. There will be such JobExecutors (i really don't know nothing about index/mux job yet - just only audio/video jobs)

generic command line audio encoder (fit 99.9% of audio encoding)
Nero AAC audio encoder (to eluminate standalone executable)
x264 encoder
xvidEncoder
generic Mencoder video encoder
generic ffmpeg video encoder


You're getting a bit overboard here.. it's like you're trying to wrap in a job what isn't one.. I don't think that's a good idea, in fact that makes the whole thing appear stiff and artificial.
No. I'm just thinking about some set of usefull utilities to be hosted @ MeGUI - there are a lot of small usefull tools we can integrate into MeGUI - like FourCC changer, AviSynth script creator/editor, tagger etc...

[dimzon]

Yet another question:
Does we really need Load/Update functionality @ JobList? How often does You use it?

[dimzon]

Some ideas about "one click mode"
I think a really reason for "one click mode" is because you can't create mux job for non-existing file in current implementation. Job.OutputFileNames main pupose is to provide list of "expected files" and JobProvider can enumerate pending jobs in JobList and refer to job output and add this job to DependsOn list :)

[Doom9]

No. I'm just thinking about some set of usefull utilities to be hosted @ MeGUI - there are a lot of small usefull tools we can integrate into MeGUI - like FourCC changer, AviSynth script creator/editor, tagger etc...But those are completely separate. The now partially integrated parts like the dgindex creator, or the avisynth script creator.. those were once completely separate utilities, and as it is today, most of their logic has been moved to common classes JobUtil and VideoUtil.. so the processing has been largely abstracted from the presentation classes. But there are other utils that are completely independent.. like the bitrate calculator, the quantizer matrix editor. And there are those that are multi-purpose, like the muxer.. it's used in different parts (manual, or auto-encode mode) and has different functionality (in part) in both modes.

let's decide what kind of information must Job provide. IMHO:The Job is one of the things that I think is rather well designed in fact.. it went through a complete redesign from the first version. The properties a job currently has are rather necessary I think. Clearly a job should have a name, should it not? And it needs a status (I see no sense storing that info someplace else), and I could go on about the other properties it has. The only debateable properties imho are position (I found this to be the most convenient way to restore the queue as it was prior to exiting.. you have to store that info someplace) and the commandline (it could be inferred.. and not every kind of job type imaginable in the future might have one. Inferring also has the advantage that all settings changes will certainly be picked up, some regeneration code in other classes can be eliminated, and finally you no longer have the problem where you move an application and the job still points to the old location).

DependsOn - list of jobs to be complete before currenct job can executeFirst off, the list is not necessary imho... it can be inferred.. a link to the one job that has to complete previous to the current one is enough, considering that the previous job will also have a link to the previous job, etc. Furthermore, I'm not so sure it's a good idea to be that strict. As it is now, you have the possibility to remove parts of a series of jobs.. processing is still possible.. those that know what they're doing can safely remove certain jobs without doing harm, so I don't consider it a "job X must have completed before this one" as a criteria that always has to be enforced no matter what.

UniqueID - some unique id (maybe GUID?) for manajement puposesWe already have that with the name.. it's always unique.

Some virtual method or property set to fill JobList ListView itemWhy would a non GUI class fill something in a GUI class? Wouldn't it be more reasonable to have the GUI class request certain information from the non GUI class for display purposes? After all we're trying to separate GUI and non GUI, so the Job should not be aware that there's such a thing as a GUI.

Executor - method or property to obtain JobExecutor for current jobOnce again, why should the job be aware of that? Shouldn't the JobExecutor be able to determine how and where a job should be executed? I see the Job as a dumb class.. it contains certain properties, other classes know what to do with it. For now, you can consider the class MeGUI to be the JobExecutor.. it gets the jobs from the JobList (so to speak.. it's a hashtable), looks at what kind of a job it is, then from that finds the proper JobExecutor (class derived from Encoder). That means that there is someone with the knowledge what to do with a certain kind of job, but I don't see that as a problem. You're perhaps thinking of a fully plug and play architecture but I'm scared of such a thing.. the rigorous constraints such an architecture requires, and the fact that it's highly unlikely MeGUI is going to be used for a lot of things we cannot think of right now (it has a clearly defined purpose.. we'll never use it to handle Email and the likes) is still somewhat slim. It's not like BeHappy where everyone can plug in a new encoder.. I actually want to be in charge of what features will be available from MeGUI and I never want to end up in a situation where somebody reports a bug or a problem that is caused by an external component. Think of the implications if somebody were to add an Ogg encoder for the whole process.. you gotta think encoder output, container and muxing... Perhaps it would be an interesting experiment to create something like that, but definitely only for the far future. Right now I rather have something that works just fine and does what I want it to, not what somebody else wants it to.

OutputFileNames - method or property to obtain list of output filesTwo comments here: Jobs currently have one output filename, and I don't see that changing. Second, it is possible that there can be multiple output files (in case of a dgindex job), but those are only known after processing... so then you'd have the paradox situation that when the whole thing goes into a processing class.. it has one output file, and when it comes back it suddenly has 5?

here will be such JobExecutors (i really don't know nothing about index/mux job yet - just only audio/video jobs)I already started with that.. I think you missed my notes on it. Currently, I have an interface IVideoEncoder, having 6 methods and an event:
setup, start, stop, pause, resume, changePriority and an even that returns a StatusUpdate object. There's the VideoEncoder that implements this interface, and which will pick the proper encoder once setup with the VideoJob has been called, a derived class CommandlineVideoEncoder which is a template for a generic commandline video encoder and offers methods to read from stdout and stderr and send the read lines somewhere, and finally the XviDEncrawVideoEncoder, which handles xvid_encraw.exe. Once I'm through with that, there's going to be another derived class from CommandlineVideoEncoder called x264Encoder or something like it, which implements the particularities of x264.exe, and another one for mencoder.exe.
Then if at some point somebody wants to add an encoder that makes direct use of an API, he can write a class derived from VideoEncoder, and thus the class will have to provide all the generic methods that encoding really needs.

A similar approach can be adopted for audio encoding and muxing. Perhaps the IVideoEncoder could be more generalized to a IJobProcessor or whatnot.. at the moment I'll be just happy if the whole video part runs as expected.. and if I have to redo parts for the most generic approach, so be it.. it's a learning experience as well after all.

[Doom9]

Does we really need Load/Update functionality @ JobList? How often does You use it?Yes, we need that.. it's a feature I make use of quite often.. it's a feature that is sorely missing from many other tools. I've often wished I could make changes to queues in other softwares, so I designed MeGUI to offer the missing functionality.

I think a really reason for "one click mode" is because you can't create mux job for non-existing file in current implementation. Job.OutputFileNames main pupose is to provide list of "expected files" and JobProvider can enumerate pending jobs in JobList and refer to job output and add this job to DependsOn listDid you really look through everything that goes on there? You cannot predict what output you may get from an indexing job.. You could for instance demux every audio track, demux no audio track, then there can be different audio types. There's a reason why an IndexJob has so many variables.. I wouldn't even dare think of changing anything in that department until you have stepped through the whole process, from the GUI coming up, to the end of postprocessing of an Index job created from one click mode in the debugger to see the whole scope of that process.. it's extremely extensive. The reason why only one job is created is twofold: 1) you don't know what audio files you're going to get, and 2) the whole "after index job" job creation requires that the input file be known.. since we do not know what audio we're going to get, it's not possible to create an audio job that then can be encoded (and I think it would be a very bad idea to create jobs that you cannot actually process because of missing input files), and the video job creation depends on the availability and possibility to open the script.. there's no video job that has not been opened.. that ensures that no matter what, our script can be processed.. and it's as far as we can go.. the rest is up to the actual encoder.

[dimzon]

Clearly a job should have a name, should it not?
...
We already have that with the name.. it's always unique.

job1, job2 - very informative, is'nt it? In other case we can provide user ability to set some additional label/name for a Job - it will be much more usable and informative...
And it needs a status (I see no sense storing that info someplace else)
Oh sorry, I just forget it!
The only debateable properties imho are position (I found this to be the most convenient way to restore the queue as it was prior to exiting..
Why not save joblist @ single file (and persist all jobs in right order)
and finally you no longer have the problem where you move an application and the job still points to the old location
why does you store full path to encoder? why not just save encoder name and get full path from configuration every time when job starts?

First off, the list is not necessary imho... it can be inferred.. a link to the one job that has to complete previous to the current one is enough, considering that the previous job will also have a link to the previous job, etc.
I like such mode much more... It's much more flexible... You can create 2 audio encoding job (to mp3 and to aac) and 2 videojob(to xvid and to x264) and create multiple mux job (to mp4 to avi to mkv)

Why would a non GUI class fill something in a GUI class? Wouldn't it be more reasonable to have the GUI class request certain information from the non GUI class for display purposes? After all we're trying to separate GUI and non GUI, so the Job should not be aware that there's such a thing as a GUI.
No, but it must provide data to fill. Something like ToString() but a little more complex...

Once again, why should the job be aware of that? Shouldn't the JobExecutor be able to determine how and where a job should be executed?
I believe it must and it alredy do! Take look @ you own VideoEncoder. It contains mencoder commandline, isn't it? Proposed Executor is flexible polymorphic replacement for it!

For now, you can consider the class MeGUI to be the JobExecutor...
Yes, and I don't like it! It contains a tons of ugly code like
if(job is AudioJob)
{
// bla-bla-bla
}
this is a realy fine place to use polyphormism instead of such code!

Two comments here: Jobs currently have one output filename, and I don't see that changing. Second, it is possible that there can be multiple output files (in case of a dgindex job), but those are only known after processing... so then you'd have the paradox situation that when the whole thing goes into a processing class.. it has one output file, and when it comes back it suddenly has 5?
1) Split 5.1 audio to six mono waves - just a sample!
2) It's possible to convert DGIndex into DLL with a rich API and wrap it into MeGUI :)

I already started with that.. I think you missed my notes on it. Currently, I have an interface IVideoEncoder, having 6 methods and an event:
setup, start, stop, pause, resume, changePriority and an even that returns a StatusUpdate object.
Yes, I know it, its fine... All what i want to do - use same interface for every encoder (audio/video/etc) - to avoid if(xx is TypeName) constructions in MeGUI...
Did you really look through everything that goes on there? You cannot predict what output you may get from an indexing job.. You could for instance demux every audio track, demux no audio track, then there can be different audio types.
Ok, I understand You! I really don't read OneClickMode code...
Yes, we need that.. it's a feature I make use of quite often...
Ok, let's keep this functionality...
You're perhaps thinking of a fully plug and play architecture
I'm thinking about partially plug and play architecture! Really, why not to be able to add another AAC encoder implementation? Or another AVC/ASP encoder? Or new source audio format via aviSynth plugin? It's easy!

[Sharktooth]

CVS Update:

0.2.3.2004 10 Jan 2006
Added --nr (noise reduction) x264 option support.

(even in tooltips)

[Doom9]

Really, why not to be able to add another AAC encoder implementation? Or another AVC/ASP encoder? Or new source audio format via aviSynth plugin? It's easy!Because it breaks things. Every encoder I add, I have to take care of all possible consequences. Let me give you an example: if I could just plug in any video encoder, I could add my xvidencrawvideoencoder. However, that encoder can only output raw files... MeGUI is painfully obvlivious of that and will offer mkv/mp4/avi output.. while the output files may just be named like that, they are indeed renamed raw files. S by making sure I get to see every encoder and write the full workflow for it, I can ensure that people can only try to do what makes sense. Asking for avi from xvid_encraw makes no sense.
Similarly, adding support for Winamp aac encoder actually means you need to sit down and think about the consequences first. That encoder gives raw aac files rather than MP4. That has an effect on something completely unrelated: bitrate calculations. Add Ogg Vorbis.. affects container selection and bitrate calculation. The list is long. It would be like supporting arbitrary input types in BeHappy.. you need to know the properties of the input stream so that you can create the proper AviSynth script. If I give you a format that you cannot handle, you have to refuse further processing, don't you? Same applies to MeGUI, except here the encoder output is also limited because there can be further processing.

1) Split 5.1 audio to six mono waves - just a sample!That's never going to be a functionality for MeGUI.
2) It's possible to convert DGIndex into DLL with a rich API and wrap it into MeGUIThat still doesn't solve the issue about getting multiple, and at the point of the start of the process yet unknown output files. And I don't see the point of doing that.. the way it's done now works just fine.

Yes, and I don't like it! It contains a tons of ugly code likePlease do show a better way (and by that I mean a set of classes ready to go.. they may only have dummy functionality but I want to see everything.. from the job to whomever processes the job, and there must be different jobtypes and different processory). If you have a job, at some point you have to pick if you're going to use the AudioEncoder, VideoEncoder or Muxer to do the processing, even though they all implement the same commands.

I believe it must and it alredy do! Take look @ you own VideoEncoder. You are mistaken. In fact, you could create an x264 job with a mencoder commandline, then if you change the x264 encoder to x264.exe in the settings, then try to start it.. it will try to send that job to the x264.exe commandline encoder ;) A Job doesn't know how it's going to be processed.. that's the job of other classes to find out and do the right thing with it. If I'll go back to creating the commandline just before job generation, it will in fact then be possible to create a mencoder x264 job, then change the encoder to x264, then start encoding, and encoding will be done by x264.exe. This is currently not possible (right now encoding would fail with a message that the commandline doesn't start with x264.exe) but that's the direction in which I'm heading. This also means a video job has to contain the output type, so that in case somebody does that change, changes an avi output mencoder job to an x264.exe job.. I can tell the user that x264 doesn't write AVIs.

No, but it must provide data to fill. Something like ToString() but a little more complex...And it does.. they're called properties. The GUI has to decide which date it wants to show.

job1, job2 - very informative, is'nt it? If you look at the queue, you have the codec type, and a lot more information about input/output/state/etc.. so yet, it's very informative. There's no way I'll ever allow people to edit jobnames on their own .

[Sharktooth]

CVS update:

0.2.3.2005 10 Jan 2006
Misc. x264 config dialog reworking

[berrinam]

From my testing with my interlace detection algorithm, I think I am ready to put it into MeGUI. However, I'm not sure how to treat it (ie as a Job, or something that just runs without a job). Here's a description of what happens when doing the analysis:

you press go. It analyses about 1% of the video by generating an AviSynth script and passing through that using the AVIFile wrapper, so that no external applications are used. If the source is interlaced/film, it runs through the file again to check field order.

As it is only analysing 1% of the file, it should be quite quick. Does this warrant a queued job, especially considering that if you use it in the AviSynth Creation Window, you won't want to close the window for the job to run? (I clearly think it doesn't but the same happened originally with DGIndex, so I want to try to get it right the first time).

Also, what sort of integration into MeGUI is wanted with this? I was thinking OneClick mode (obviously) would do this automatically, for better script generation, and AviSynth Window. In the OneClick mode it would be transparent to the user except for a checkbox: 'Automatic deinterlacing'. For the AviSynth window, it would have a button which says 'Analyse file...'. It would then analyse the video, tell the user the source type, and adjust the deinterlace filter combobox according to which filters are appropriate, and select the recommended one.

What do you think?

Also, about the 'Minimize to Tray' feature request: You say 'not a button'. I implemented this a little while ago with a menu item in the view menu, because I couldn't think of anywhere else to put it. Is this good enough?

[Sharktooth]

Use the [X] to minimize to tray... and File -> Exit to exit MeGUI.

[Doom9]

Also, what sort of integration into MeGUI is wanted with this? I was thinking OneClick mode (obviously) would do this automatically, for better script generationDefinitely. And the code has to be placed in the index job postprocessing routine.
AviSynth Window. In the OneClick mode it would be transparent to the user except for a checkbox: 'Automatic deinterlacing'. For the AviSynth window, it would have a button which says 'Analyse file...'. It would then analyse the video, tell the user the source type, and adjust the deinterlace filter combobox according to which filters are appropriate, and select the recommended one.sounds good as well.

Is this good enough?I think so. Some people prefer using the minimize box, but it's not the normal behavior of an app so I think using that menu will do just fine.

Use the [X] to minimize to tray... and File -> Exit to exit MeGUI.I don't agree. standard behavior of X is to exit.. I personally don't want MeGUI to go to the tray when I press X (or _ for that matter).. but being able to specifically tell it to go there, that might be useful. Of course, we need a small logo that shows up in the tray then.

[berrinam]

I don't agree. standard behavior of X is to exit.. I personally don't want MeGUI to go to the tray when I press X (or _ for that matter).. but being able to specifically tell it to go there, that might be useful. Of course, we need a small logo that shows up in the tray then.
The App.ico?

[Doom9]

The App.ico?No, app.ico is the big icon, the one you get when you put the app in the desktop. I'm talking about one of the size of that small icon you get in the upper left corner of an application. The icon in the tray has the same size (14x14 or 16x16 or something like that)

[berrinam]

Ah well, I'm using that icon for the moment, and it works fine. If someone wants to come along and design a better icon, feel free to, but it's not me.

[berrinam]

CVS Update:
0.2.3.2006 10 Jan 2006
Fix a crash in OneClick mode caused by audio stream's name not being set.
Fix a crash in OneClick mode when it detects too many files as DGIndex's output (made it's detection of these files more selective)
Load Defaults Button
Relocate the profile box in the individual codec configuration windows so it is accessible from all tabs

[berrinam]

CVS Update:
0.2.3.2007 10 Jan 2006
Add Minimize To Tray menu item
Fix the Load DLL bug
Fix the IVTC checkbox bug

[Richard Berg]

AutoEncode is broken in the most recent build. Select an AVS file, a WAV file, AutoEncode, accept the default option: null ref.

System.NullReferenceException: Object reference not set to an instance of an object.
at MeGUI.CommandLineGenerator.generateMP4BoxCommandline(String mp4BoxPath, MuxSettings settings, String input, String output)
at MeGUI.JobUtil.generateMuxJob(VideoJob vjob, SubStream[] audioStreams, SubStream[] subtitleStreams, String chapterFile, MUXTYPE type, String output)
at MeGUI.VideoUtil.generateJobSeries(String videoInput, String videoOutput, String muxedOutput, VideoCodecSettings videoSettings, AudioStream[] aStreams, SubStream[] audio, SubStream[] subtitles, String chapters, Int64 desiredSize, Int32 splitSize, Double containerOverhead, MUXTYPE muxtype)
at MeGUI.AutoEncodeWindow.queueButton_Click(Object sender, EventArgs e)
at System.Windows.Forms.Control.OnClick(EventArgs e)
at System.Windows.Forms.Button.OnClick(EventArgs e)
at System.Windows.Forms.Button.OnMouseUp(MouseEventArgs mevent)
at System.Windows.Forms.Control.WmMouseUp(Message& m, MouseButtons button, Int32 clicks)
at System.Windows.Forms.Control.WndProc(Message& m)
at System.Windows.Forms.ButtonBase.WndProc(Message& m)
at System.Windows.Forms.Button.WndProc(Message& m)
at System.Windows.Forms.Control.ControlNativeWindow.OnMessage(Message& m)
at System.Windows.Forms.Control.ControlNativeWindow.WndProc(Message& m)
at System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, Int32 msg, IntPtr wparam, IntPtr lparam)

I can't look at GPL code while at work, obviously, so can't investigate yet...

[berrinam]

CVS Update:
0.2.3.2008 10 Jan 2006
Update Minimize to Tray to work with conditional compiling
update compile.bat to work with .NET 2.0.50727. This only works if you have that exact version, so, hopefully, someone can find a better way to do this.

[Mutant_Fruit]

Just while on discussion of restructuring MeGUI. What do people think about moving all class definitions and related setters/getters to either seperate c# files or all in one c# file.

Its just if i wanted to make a change to the class, or several classes i can go to that class's c# file and make the change there without having to try and find where the getters and setters are in each dialog.

Thats just personal preferance really, but i think it makes code a bit easier to work with.

[Doom9]

@Mutant_Fruit: I don't understand what you want to do. Could you elaborate, perhaps with examples?

@everyone: I've run into a wall with the encoder reconstruction. the CommandlineVideoEncoder class used to implement the IVideoEncoder interface and I connected to it directly to the main form.. that worked as it should. Now that I changed all encoders to use that pattern, my VideoEncoder implements the interface instead.

Anyway, in the main class, I set the callback for status updates as follows.

vEnc.StatusUpdate += new VideoEncodingStatusUpdateCallback(enc_StatusUpdate);
Then when it's time to fire the event from the VideoEncoder class,
StatusUpdate(su); (line 61 in VideoEncoder.cs), StatusUpdate is null.

I might just be too tired, but right now I just don't see what I'm doing wrong. Compilable source code can be found here: http://forum.doom9.org/MeGUI-src.CVS.rar
Keep in mind, it's untested.. I just need this event routing to work to start testing properly.. and then there's all the fun in preventing all the use cases that don't work because encraw only supports raw output.

[Richard Berg]

If I were to suggest a "big" architectural change, it would be to eliminate all of the conditional compilation. Even if a user is only interested in the x264 or Snow features, it's not like the full MeGUI is more difficult to use than the special versions. Meanwhile the extra bugs, developer time, and (necessary but lacking) testing that come from separate versions affects everybody.

[berrinam]

CVS Update:
0.2.3.2009 10 Jan 2006
Fixed the AutoEncode crash caused by AudioStream.name being null



@Richard Berg: I agree with you. Doom9 also said something similar a while ago.

[berrinam]

@Doom9: Found the problem. Basically, you are setting the StatusUpdate value for the VideoEncoder vEnc with this line:vEnc.StatusUpdate += new VideoEncodingStatusUpdateCallback(enc_StatusUpdate);
However, when you call vEnc.start: (Form1.cs, line 3938)
if (vEnc.start(out error)) this goes on to the VideoEncoder.start(string) function (VideoEncoder.cs, line 100), which passes the start request onto its member called encoder with the line:(VideoEncoder.cs, line 103)
return encoder.start(out error); The point is this: the VideoEncoder called encoder sets itself up to call its own StatusUpdate function, which has not yet been set, because the StatusUpdate was set for vEnc. As it is, I'm not quite sure how your system is set up (ie, why does VideoEncoder have a VideoEncoder member variable?), so I haven't posted a fix, but I hope what I said helps you solve it.

@edit: Actually, a solution which avoids the bug is to turn StatusUpdate into a property, by replacing its declaration with the following code:
private event VideoEncodingStatusUpdateCallback statusUpdate;
public event VideoEncodingStatusUpdateCallback StatusUpdate
{
add
{
encoder.statusUpdate += value;
}
remove
{
encoder.statusUpdate -= value;
}
} and changing protected void sendStatusUpdateToGUI(StatusUpdate su)
{
StatusUpdate(su);
} to protected void sendStatusUpdateToGUI(StatusUpdate su)
{
statusUpdate(su);
} (the difference is in the capitalisation, making it access the variable directly as opposed to through the property) That seems to fix it.

[kopavel]

that's is normal if in 2-nd and 3-s pases in MeGUI we have parameters
--pass 3 in both comand strings?
But only in 3-pass section.
In 2-pass section there is --pass 2 (in 2-nd pass)

[godhead]

3) There however I have to disagree with you. Variables should not be declared public. In Java we have setters and getters to change/get variables, in .NET we have properties. In VS2k5 you can easily define your private variable, then using the refactoring functionality turn this into a property with about two clicks (I don't recall the exact procedure.. it's been a while since I watched that "what's new" video). And in both Java classes at university as well as coding practices for C# they tell you that accessing variables directly is bad style of programming. For most variables, you don't need any accessors because they are only used within the class where they are declared, but those that are to be accessed externally should use properties.

Another way to quickly create your private variable and property in VS2K5 is to use the keyboard shortcut, type "prop" and press tab and it will stub out a private variable and property for you.

I've only breifly looked over the code needed to make the changes that I've taken on, so my exposure has been the MuxSettings and CommandLineGenerator. I do feel there is some clean up that can be done but I didn't want to "rock the boat" during my first patch :)

I would like to change the CommandLineGenerator to use a StringBuilder, as it will probably be a bit easier to read. There's just so much string concatenation going on during the creation of the command line that it can get a bit confusing. I've always used the rule that if you're concating a string more than 4 times, it's probably best to move it to a StringBuilder.

If no one disagrees, I'll have this change completed tomorrow.

[godhead]

CVS Update:
0.2.3.2006 10 Jan 2006
Fix a crash in OneClick mode caused by audio stream's name not being set.
Fix a crash in OneClick mode when it detects too many files as DGIndex's output (made it's detection of these files more selective)
Load Defaults Button
Relocate the profile box in the individual codec configuration windows so it is accessible from all tabs

Did I miss something on the One-Click mode? Sorry about that, I'll take a look at the changes you made with a DIFF and make sure the same problem doesn't occur again when I add the subtitle names.

[godhead]

Sorry that I just caught up, but I'd like to suggest that if a redesign is going to occur that it be done in a branch of the code.

I think maybe Doom9 and dimzon (whoever else that wants to help here) should work out their redesign and stub out the architecture. Once the foundation has been designed, the rest of the developers can work on the actual implimentation.

This would allow the rest fo the developer's to continue with bug fixes and feature requests of the current code branch. The new requests that are implimented in this old branch become new requirements for the new branch.

I just wanted to make sure if a redesign does occur, we can continue supporting the existing users.

[Doom9]

I'm definitely going to remove the snow conditional compilation.. not sure about x264 yet. Since I'm implementing encraw support (no AVI output), I also have to rethink certain muxing mechanisms, so perhaps the dual video encoder thing for the same codec will also disappear, thus further uncluttering the code.
(ie, why does VideoEncoder have a VideoEncoder member variable?)It's a derived class actually... encoder will always be a subclass of VideoEncoder, but in order to use inheritance in the easiest way (no casting, no switches or ifs to call the variable of the proper type) I use the base type for the operations. Thanks for your analysis.. it definitely opened my eyes.. the one VideoEncoder that should return the status update is actually an inheritet member of CommandlineVideoEncoder and thus a different instance of a VideoEncoder class.

I would like to change the CommandLineGenerator to use a StringBuilder, as it will probably be a bit easier to read. There's just so much string concatenation going on during the creation of the command line that it can get a bit confusing. I've always used the rule that if you're concating a string more than 4 times, it's probably best to move it to a StringBuilder.Hmm.. did I never change that? At least the video commandlines are all generated by a StringBuilder.. feel free to change the muxing commandlines as well, it definitely makes sense.

VS2K5 is to use the keyboard shortcut,Which keyboard shortcut?

[Sharktooth]

CVS Update:
0.2.3.2010 11 Jan 2006
Reverted Levels -> MB changes.

Bins are up on SF.
Sources (until anonymous CVS gets in synch): http://files.x264.nl/?dir=./Sharktooth/megui/Sources