Csdrhrt

I'm currently trying to make a screen recorder in C#, and so far it works but the problem is that something as simple as a 20second video will take about 1GB of space. I have it setup so a timer continuously takes screenshots with this method:

void takeScreenshot()

{

    Rectangle bounds = Screen.FromControl(this).Bounds;

    using (Bitmap bitmap = new Bitmap(bounds.Width, bounds.Height))

    {

        using (Graphics g = Graphics.FromImage(bitmap))

        {

            //Add screen to bitmap:

            g.CopyFromScreen(new Point(bounds.Left, bounds.Top), Point.Empty, bounds.Size);

        }

        //Create and save screenshot:

        string name = path + "//screenshot-" + fileCount + ".jpeg";

        bitmap.Save(name, ImageFormat.Jpeg);

        inputImageSequence.Add(name);

        fileCount++;



        //Dispose of bitmap:

        bitmap.Dispose();

    }

}

And then it stores those pictures in a temporary folder in the D:// drive, and then when it's done it takes all the pictures and creates an AVI video out of them like this:

//Set bounds of video to screen size:

Rectangle bounds = Screen.FromControl(this).Bounds;

int width = bounds.Width;

int height = bounds.Height;



var framRate = 5;



using (var vFWriter = new VideoFileWriter())

{

    //Create new video file:

    vFWriter.Open(outputPath+"//video.avi", width, height, framRate, VideoCodec.Raw);



    //Make each screenshot into a video frame:

    foreach (var imageLocation in inputImageSequence)

    {

        Bitmap imageFrame = System.Drawing.Image.FromFile(imageLocation) as Bitmap;

        vFWriter.WriteVideoFrame(imageFrame);

        imageFrame.Dispose();

    }

    vFWriter.Close();

}

//Delete the screenshots and temporary folder:

DeletePath(path);

Any help on reducing the inefficiency of this is appreciated, I'm fairly new to this kind of programming.

takeScreenshot()

• Based on the .NET Naming Guidelines methods should be named using PascalCase casing takeScreenshot() => TakeScreenshot()


• You are enclosing the usage of the Bitmap inside a using statement which is the way to go but calling Dispose() on that Bitmap is superfluous because that is what a using statement is doing.


• 
  

  If you have two using statements without any code between you can stack them which saves one level of indentation like so (already removed the Dispose())

  
  
  

  using (Bitmap bitmap = new Bitmap(bounds.Width, bounds.Height))
  
  using (Graphics g = Graphics.FromImage(bitmap))
  
  {
  
      //Add screen to bitmap:
  
      g.CopyFromScreen(new Point(bounds.Left, bounds.Top), Point.Empty, bounds.Size);
  
  
  
      //Create and save screenshot:
  
      string name = path + "//screenshot-" + fileCount + ".jpeg";
  
      bitmap.Save(name, ImageFormat.Jpeg);
  
      inputImageSequence.Add(name);
  
      fileCount++;
  
  }  
  
  

  
  


• Comments should explain why something is done in the way it is done. Let the code itself tell what is done by using meaningful named things. In this way your comments won't lie like e.g //Create and save screenshot: which isn't what that code does. Saving yes but no creating of a screenshot is taking place.

Creating the video

• Try to be consistent. Here you create a Bitmap and call Dispose() instead of using a using statement. In addition, sometime you use var and sometimes you use the concrete type although the type is seen at first glance.


• If you are using the as operator you should always add a null check for that object because an as operator won't throw an exception but the resulting object may be null which will trigger an exception somewhere else.

Implementing these points will look like this:

Rectangle bounds = Screen.FromControl(this).Bounds;

var width = bounds.Width;

var height = bounds.Height;



var framRate = 5;



using (var vFWriter = new VideoFileWriter())

{

    vFWriter.Open(outputPath+"//video.avi", width, height, framRate, VideoCodec.Raw);



    foreach (var imageLocation in inputImageSequence)

    {

        using(var imageFrame = (Bitmap)System.Drawing.Image.FromFile(imageLocation))

        {

            vFWriter.WriteVideoFrame(imageFrame);

        }

    }

    vFWriter.Close();

}

DeletePath(path);

My guess about the big file size is that you are using the VideoCodec.Raw. Try to change it to some other codec and see if this helps.

You could write the screenshots directly to the video stream. This could save you a lot of disk space depending on your capture time.

To make the video file smaller you need to use a codec that compresses the output like MPEG-4.

Finally you could encapsulate the recording in its own class:

public sealed class ScreenRecorder : IDisposable

{

    private readonly VideoFileWriter videoFileWriter = new VideoFileWriter();

    private readonly string videoFilePath;

    private readonly Rectangle bounds;



    public ScreenRecorder(string videoFilePath, Rectangle bounds, int frameRate = 5, VideoCodec videoCodec = VideoCodec.MPEG4)

    {

        if (string.IsNullOrWhiteSpace(videoFilePath))

        {

            throw new ArgumentException("Must be a valid filename", nameof(videoFilePath));

        }



        if(frameRate < 1)

        {

            throw new ArgumentOutOfRangeException(nameof(frameRate));

        }



        this.videoFilePath = videoFilePath;

        this.bounds = bounds;

        videoFileWriter.Open(videoFilePath, bounds.Width, bounds.Height, frameRate, videoCodec);

    }



    public void TakeScreenshot()

    {

        if (disposed)

        {

            throw new ObjectDisposedException(nameof(ScreenRecorder));

        }



        using (Bitmap bitmap = new Bitmap(bounds.Width, bounds.Height))

        {

            using (Graphics g = Graphics.FromImage(bitmap))

            {

                //Add screen to bitmap:

                g.CopyFromScreen(new Point(bounds.Left, bounds.Top), Point.Empty, bounds.Size);

            }



            videoFileWriter.WriteVideoFrame(bitmap);

        }

    }



    public void Stop() => Dispose();



    #region IDisposable Support

    private bool disposed = false; // To detect redundant calls



    void Dispose(bool disposing)

    {

        if (!disposed)

        {

            if (disposing)

            {

                videoFileWriter.Dispose();

            }



            disposed = true;

        }

    }



    public void Dispose()

    {

        Dispose(true);

    }

    #endregion

}

Using string concatenation for creating file paths opens you up to a whole lot of headache. Should you use /, //, or \? The answer depends on the environment you are running in. Luckily, the Path class can do all this logic for us:

Instead of

path + "//screenshot-" + fileCount + ".jpeg";

you could do

Path.Combine(path, $"screenshot-{fileCount}.jpeg");

and instead of

outputPath+"//video.avi"

Path.Combine(outputPath, "video.avi")

This way you don't have to worry about using the right fileseparator.

Thanks to all your help as well as the help of many others, I have actually fully compelted the screen recorder and have typed up an explanation here if anyone is interested: https://benbcompsci.wordpress.com/2018/12/04/c-screen-recorder/