Newb over head: Why is my home made screen recorder crashing?(c# and windows forms)

Question

[SOLVED] Ofir's solution worked right out of the box. Currently running for five minutes at 24 f/s.

I'm trying to save images from the screen.

Basically my issue, is that as I decrease Thread.sleep (to create a decent frame rate) the program crashes. The program crashes faster the closer thread.sleep is to 0, but I can't even find the problem because I've never dealt with this kind of stuff(If it was unity3d I'd be all over this).

void ScreenCapture()//this is from a youtube tutorial
    {
        while (true)
        {
            Bitmap bm = new Bitmap((int)Math.Round(Screen.PrimaryScreen.Bounds.Width * 1.5), (int)Math.Round(Screen.PrimaryScreen.Bounds.Height * 1.5));
    //I don't know why I had to multipyly these by 1.5, but it made the screen fit properly.
            Graphics g = Graphics.FromImage(bm);
            g.CopyFromScreen(0, 0, 0, 0, bm.Size);
            pictureBox.Image = bm;
            Thread.Sleep(250);
        }
    }

    private void button2_Click(object sender, EventArgs e)
    {
        Thread t = new Thread(ScreenCapture);
        t.Start();
    }

I've got some sweet sweet errors coming your way too.

An unhandled exception of type 'System.ComponentModel.Win32Exception' occurred in System.Drawing.dll

error code: -2147467259

and I built a try-catch(which I know little about) but it breaks too after a few tests, lemme see if I can get that log.

    System.ComponentModel.Win32Exception(0x80004005): The operation completed successfully as System.Drawing.Graphics.CopyFromScreen(Int32 sourceX, Int32 sourceY, Int32 destinationX,Int32 destinationY, Size blockRegionSize)
 at Screen_Recorder.Form1.ScreenCapture() in C:Users\Jupiter\Desktop\visual studio experiments\Screen Recorder\ScreenRecorder\Form1.cs:line 35

if I then click ok it says:

System.ArgumentException: Parameter is not valid.
 at System.Drawing.Bitmap..ctor(Int32 width, Int32 height, PixelFormat format)
 at System.Drawing.Bitmap..ctor(int32 width, Int32 height)
 at Screen_Recorder.Form1.ScreenCapture() C:Users\Jupiter\Desktop\visual studio experiments\Screen Recorder\ScreenRecorder\Form1.cs:line 32

and then it will repeat that error ad infinitum.

Anyway, I'm going to bed and then I'll get into it again tommorrow, but any advice before then would be much appreciated!

Answer 1

You have a memory leak.

Graphics and Bitmap are disposable object and should be disposed when you've done with them.
You can only put the Graphics in your case in a using block and the last bitmap should be disposed every iteration of the loop.
Therefore, your code should look like this:

private void ScreenCapture()
{
    while (true)
    {
        var bm = new Bitmap((int)Math.Round(Screen.PrimaryScreen.Bounds.Width * 1.5), 
                           (int)Math.Round(Screen.PrimaryScreen.Bounds.Height * 1.5));
        using (Graphics g = Graphics.FromImage(bm)) g.CopyFromScreen(0, 0, 0, 0, bm.Size);

        // As per the comment by HansPassant - the following would cause
        // a thread race with the UI thread.
        //this.pictureBox1.Image?.Dispose();
        //this.pictureBox1.Image = bm;

        // Instead we use beginInvoke to run this on the UI thread
        Action action = () =>
            {
                this.pictureBox1.Image?.Dispose();
                this.pictureBox1.Image = bm;
            };

        this.BeginInvoke(action);

        Thread.Sleep(250);
    }
}

This loop is never going to end (because of the while (true)), Maybe you should consider adding a CancelationToken and use Task instead of a Thread.

And one last thing, you should dispose the picturebox itself as well after finishing with it.