TThread questions

[j0x]

As I said above, you cannot do things like:

frmMain.lstOut.Items.add

from the non-main-Thread.
It all has to go through Synchronize.
Or even better, let the thread to its job and work only with the returned data in the Form/MainThread

oh ok now i got you lets see how can i code that tomorrow thanks for clarifying it to me

[j0x]

@theo

thanks a lot man and for the rest of the lazarus community on patiently guiding me i managed to finished it now and its working good

[theo]

thanks a lot man and for the rest of the lazarus community on patiently guiding me i managed to finished it now and its working good

Sorry man, but this code is a horror. ;-)
You should really try to understand threads first, before writing such code.
It seems like pure luck if this works. You still have things like...

Code: (pascal) [Select]

procedure TMyThread.Execute;
repeat
  if (scanned = false) and (i < frmMain.lstAdd.Items.Count) then
     begin
          sPro.CommandLine := 'HandbrakeCLI -i ' + '"' + frmMain.lstAdd.Items[i] + '"' + ' -t 0';

...in your code. You're still referencing frmMain unsynchronized in several places.

But you should completely rethink your design. It would be better to buffer all results in a Variable/Field of the Thread-Object and write the entire thread code first without ANY reference to frmMain.
Only in OnTerminate you should copy the result to the visual controls.
Once this works, you can add ONE Synchronize(@UpdateGui) or something to show that the thread is working.
But accessing the frmMain inside the thread should be the absolute exception, not the rule!

The way you do it now, you don't need a thread.  ;-)
If everything is synchronized, it's almost the same as running the code in the main thread.

Please read more and try to understand. Threads are not just another class.

[j0x]

ah ok ill try improving it if i could understand more TThreads but for now im good even if its luck that the program is now responsive when scanning a video file that is somewhat corrupt but still readable

[theo]

But don't say I didn't warn you! ;-)
Such hazardous thread code may work ten or a thousand times, but it will fail sooner or later giving really nasty and "unexplainable" results.

[j0x]

But don't say I didn't warn you! ;-)
Such hazardous thread code may work ten or a thousand times, but it will fail sooner or later giving really nasty and "unexplainable" results.

ok that caught my attention more lol

so do you mean to imply that there should be no frmMain (main thread) elements on the TMyThread.Execute area? i mean all frmMain elements should be on other procedure/s of the TMyThread class and just use synchronize between the other procedure/s and the execute area if i want to access/modify the frmMain elements?

EDIT:

i created a variable on TMyThread called strScan as TListBox the code on execute method looks like this now (no more frmMain references on execute method area)

Code: [Select]

procedure TMyThread.Execute;
const
     READ_BYTES = 2048;
begin

repeat

  if (scanned = false) and (i < strScan.Items.Count) then
     begin
          Synchronize(@ScanStats);
          sPro.CommandLine := 'HandbrakeCLI -i ' + '"' + strScan.Items[i] + '"' + ' -t 0';
          sPro.Options := [poUsePipes, poStderrToOutPut];
          sPro.ShowWindow := swoHide;
          sPro.Execute;
          while sPro.running do
          begin
               M.SetSize(BytesRead + READ_BYTES);
               n := sPro.Output.Read((M.Memory + BytesRead)^, READ_BYTES);
               if n > 0 then
               begin
                    Inc(BytesRead,n);
                    Synchronize(@prgUpdate);
               end;
          end;

          repeat
                Synchronize(@prgUpdate);
                M.SetSize(BytesRead + READ_BYTES);
                n := sPro.Output.Read((M.Memory + BytesRead)^, READ_BYTES);
                if n > 0 then
                begin
                     Inc(BytesRead,n);
                end;
          until n <= 0;

          M.SetSize(BytesRead);

          Synchronize(@ScanOut);
     end;

     if (scanned = true) and (i < strScan.Items.Count) then
     begin
           Synchronize(@ScanInc);
     end;

     if (scanned = false) and (i = strScan.Items.Count)then
     begin
           Synchronize(@ScanDone);
     end;
until i = strScan.Items.Count;

end;                      


Code: [Select]

TMyThread = class(TThread)
    private
      audioCount: integer;
      venString: string;
      x264String: string;
      sPro: TProcess;
      M: TmemoryStream;
      n: longInt;
      BytesRead: longInt;
      strScan: TListBox;
      procedure ScanOut;
      procedure ScanInc;
      procedure ScanDone;
      procedure ScanStats;
      procedure prgUpdate;
    protected
      procedure Execute; override;
    public
      Constructor Create(CreateSuspended : boolean);
    end;                    

i just put the frmMain.lstAdd listbox on the strScan variable like this

Code: [Select]

scanThread.strScan := lstAdd;

[theo]

i created a variable on TMyThread called strScan as TListBox the code on execute method looks like this now (no more frmMain references on execute method area)

Sorry, not GUI Code in threads, so no TListBox. But you can use TStringList.
It's better to look at the Thread like a console program.
Give some input -> Execute -> read the output.
I already told you this:
"Or even better, let the thread do its job and work only with the returned data in the Form/MainThread"

Once you have no more GUI/Mainform/Mainthread Stuff in your code, and read the output in the OnTerminate event handler. you can start using a single "Synchronize" if absolutely necessary.

Don't try to be smart, write just very simple and clean code.

[j0x]

i created a variable on TMyThread called strScan as TListBox the code on execute method looks like this now (no more frmMain references on execute method area)

Sorry, not GUI Code in threads, so no TListBox. But you can use TStringList.
It's better to look at the Thread like a console program.
Give some input -> Execute -> read the output.
I already told you this:
"Or even better, let the thread do its job and work only with the returned data in the Form/MainThread"

Once you have no more GUI/Mainform/Mainthread Stuff in your code, and read the output in the OnTerminate event handler. you can start using a single "Synchronize" if absolutely necessary.

Don't try to be smart, write just very simple and clean code.

lol i wish im even trying to be smart as im lost really maybe im lacking understanding in english as its my second language ill try understanding your points to me as im hard to understand them maybe because it also my first time doing classes, so ill be back in a few days if i happen to understand them

[j0x]

i remove all the frmMain references on the execute method of TMyThread and this is how it looks now

Code: [Select]

procedure TMyThread.Execute;
const
     READ_BYTES = 2048;
begin

repeat

          Synchronize(@ScanStats);
          sPro.CommandLine := 'HandbrakeCLI -i ' + '"' + strScan[ii] + '"' + ' -t 0';
          sPro.Options := [poUsePipes, poStderrToOutPut];
          sPro.ShowWindow := swoHide;
          sPro.Execute;
          while sPro.running do
          begin
               M.SetSize(BytesRead + READ_BYTES);
               n := sPro.Output.Read((M.Memory + BytesRead)^, READ_BYTES);
               if n > 0 then
               begin
                    Inc(BytesRead,n);
               end;
          end;

          repeat
                M.SetSize(BytesRead + READ_BYTES);
                n := sPro.Output.Read((M.Memory + BytesRead)^, READ_BYTES);
                if n > 0 then
                begin
                     Inc(BytesRead,n);
                end;
          until n <= 0;

          M.SetSize(BytesRead);

          Synchronize(@ScanOut);

     ii := ii + 1;

until ii = strScan.Count;

end;

it only has only 2 synchronize call now and strScan is the TStringList
thats the best i can make for now on my limited knowledge on programming as i cant seem to fully remove the @ScanOut method because TStringList will accumulate all input on the listbox that im looping on all at once and its a pain for me to make a coding for TStringlist.IndexOf() as it doesnt go beyond the last entry on TStringlist or rather i cant seem to extract each listing so i have to intervene with @ScanOut

but i accomplished my intention of making the sPro (TProcess) to be in a another thread so that makes the program more responsive now when it reads HandbrakeCLI parameters

so thanks for all the guides