4
\$\begingroup\$

I basically have 3 processes which I loop through.

The first process is encoding Video. It's actually 2 processes but they are connected, so I don't need to worry about how it handles the other one.

The second and third are connected by me, however. One reads and the other writes, so I have to manually feed the stdin/out.

And here I am a bit skeptical. If I do it the "normal" way, which basically is having one callback thread which gets the data, and then writes it immediately. The performance will be locked to one thread.

With that I mean that both processes will share Thread, so both will pretty much run at 50%.

To solve this I am letting the Callback run and just feed the Data to a BlockingCollection, which the other process then reads from (that's being done on another Thread). I am not, however, sure this is truly an efficient way to do things, but it works.

The last is simply a process which takes 2 files and puts them into a container.

3 of these processes are run on the same variable cmdProcesses. The exception is the one that is a separate cause of the piping.

Here is how the main part of this code runs:

private void Encoding(string[] GetFile)
{
    using(AutoResetEvent OpusCheck = new AutoResetEvent(false))
    using (QU = new BlockingCollection<byte[]>())
    using (OpusEncoder = new Process())
    for (int i = 0; i < GetFile.Length; i++)
    {
        try
        {
            ReadBuffer = new byte[4096];
            string filename = Path.GetDirectoryName(GetFile[i]) + "\\" + Path.GetFileNameWithoutExtension(GetFile[i]);

            using (cmdCommands = new Process())
            {

                cmdCommands.StartInfo.UseShellExecute = false;
                cmdCommands.StartInfo.CreateNoWindow = false;
                cmdCommands.StartInfo.FileName = SC.GetAVS4x264Path();
                cmdCommands.StartInfo.Arguments = string.Format("\"{0}\" --x264-binary \"{1}\" --colormatrix=bt709 {2} --output=\"{3}\" -", filename + ".avs", SC.Getx264Path(), SC.Getx264Settings(true), filename + ".mkv");
                cmdCommands.Start();
                cmdCommands.WaitForExit();

                if (EncodingStopped)
                    break;

                cmdCommands.StartInfo.UseShellExecute = false;
                cmdCommands.StartInfo.CreateNoWindow = true;
                cmdCommands.StartInfo.RedirectStandardOutput = true;
                cmdCommands.StartInfo.FileName = SC.GetAVS2PipeModPath();
                cmdCommands.StartInfo.Arguments = string.Format("{0} \"{1}\" -wav", SC.GetAVS2PipeModPath(), filename + ".avs");
                cmdCommands.Start();

                Thread OpusThread = new Thread(() => OpusEncode(filename, OpusCheck));
                OpusThread.Start();

                cmdCommands.StandardOutput.BaseStream.BeginRead(ReadBuffer, 0, ReadBuffer.Length, PipeWrite, null);
                cmdCommands.WaitForExit();
                OpusCheck.WaitOne();
                if (EncodingStopped)
                    break;

                cmdCommands = new Process();
                cmdCommands.StartInfo.UseShellExecute = false;
                cmdCommands.StartInfo.RedirectStandardError = true;
                cmdCommands.StartInfo.FileName = SC.GetMKVMergePath();
                cmdCommands.StartInfo.Arguments = (" -o \"" + filename + "-muxed.mkv\" \"" + filename + ".mkv\" " + "\"" + filename + ".opus\"");
                cmdCommands.Start();
                cmdCommands.WaitForExit();



                File.Delete(filename + ".mkv");
                File.Delete(filename + ".opus");
            }
        }
        catch (Exception e)
        {
            MessageBox.Show(e.Message);

        }
    }
     StopEncoding.Dispatcher.BeginInvoke(DispatcherPriority.Normal, new System.Action(() => { StopEncoding.IsEnabled = false; Encode.IsEnabled = true; }));
}

The Pipe (CallBack)

    private void PipeWrite(IAsyncResult ar)
    {
        int read = cmdCommands.StandardOutput.BaseStream.EndRead(ar);
        if (read != 0)
        {
            QU.Add((byte[])ReadBuffer.Clone());
            cmdCommands.StandardOutput.BaseStream.BeginRead(ReadBuffer, 0, ReadBuffer.Length, PipeWrite, null);
            cmdCommands.StandardOutput.BaseStream.Flush();
        }
        else
        {
            ReadBuffer = new byte[0];
            QU.Add(ReadBuffer);
            cmdCommands.StandardOutput.BaseStream.Close();
        }
    }

The separate Process Thread

private void OpusEncode(string filename,AutoResetEvent OpusCheck)
{
        OpusEncoder.StartInfo.UseShellExecute = false;
        OpusEncoder.StartInfo.RedirectStandardInput = true;
        OpusEncoder.StartInfo.FileName = SC.GetOpusEncPath();
        OpusEncoder.StartInfo.Arguments = string.Format("{0} - \"{1}\"", SC.GetOpusSettings(true), filename + ".opus");
        OpusEncoder.Start();
        byte[] temp;
        while (QU.TryTake(out temp, Timeout.Infinite))
        {
            if (temp.Length == 0)
                break;
            OpusEncoder.StandardInput.BaseStream.Write(temp, 0, temp.Length);

        }
            OpusEncoder.StandardInput.Close();
            OpusEncoder.WaitForExit();
        OpusCheck.Set();               
}

I want to know if this approach is the way to go, or if I am handling this wrong. And also if it can be more efficient, which probably goes in hand with it being right/wrong.

\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

Naming. It's hard.

I'll focus on that so a better programmer than I may focus more important things when they review this.

  • QU is completely meaningless to Mr. Maintainer.
  • cmdCommands is a bit repetitive. commands would be sufficient. commandProcess might be even better.
  • Methods should have verb-noun names. PipeWrite would be better as WritePipe or maybe WriteToPipe.
  • ar should be asyncResult.

So I lied, there are a couple of other things I can point out.

  • In this line, ReadBuffer = new byte[4096];, 4096 is a magic number. It's only been used once, so defining a constant for it might be overkill, but a quick comment about why 4096 as opposed to any other size would be nice.
  • I don't think you're getting a whole lot out of this catch block. You should really take the time to think about what kind of errors are likely to happen and what should be done with them if they do.

    catch (Exception e)
    {
        MessageBox.Show(e.Message);
    
    }
    
\$\endgroup\$
1
  • 1
    \$\begingroup\$ taken into account, didn't really think much of the names, but it's indeed important that the clearly point out what they are made for. \$\endgroup\$
    – Zerowalker
    Commented Jul 24, 2014 at 9:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.