HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Sending & Receive Image Over TCP/IP

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
imagesendingtcpreceiveover

Problem

I've created a program that can send & receive a screenshot. However, I feel like my method is a little bit...inefficient. I have to write to the stream twice, because from what I've observed, in order for the image to form properly, I must know the exact file size aka. the length. So, what I do is write the length to the the stream, than write the image to the stream. Is there a way to combine this into one step, or optimize it more?

Here's my code for the client

else if (plainText.Contains("screenshot")) //check if received command is asking for screenshot
        {
            MemoryStream ms = new MemoryStream();
            Bitmap bitmap = new Bitmap(Screen.PrimaryScreen.Bounds.Width, Screen.PrimaryScreen.Bounds.Height);
            Graphics graphics = Graphics.FromImage(bitmap);
            graphics.CopyFromScreen(0, 0, 0, 0, bitmap.Size); 
            bitmap.Save(ms, System.Drawing.Imaging.ImageFormat.Jpeg);//save bitmap to MemoryStream so I can read length
            writeToStream(ms.ToArray().Length.ToString()); //Write the length to the NetWorkStream
            writeToStream(ms.ToArray()); //Write the actual image to the NetworkStream
            ms.Close(); //close the MemoryStream when done
        }


And for my server:

```
private void screenShotBTN_Click(object sender, EventArgs e)
{
screenShotBTN.Enabled = false;

NetworkStream stream = getSelectedClient().GetStream();

writeBuffer = Encoding.ASCII.GetBytes("screenshot");

stream.Write(writeBuffer, 0, writeBuffer.Length); // send screenshot request

int data = 0;

byte[] readBuffer = new byte[getSelectedClient().ReceiveBufferSize];

data = stream.Read(readBuffer, 0, readBuffer.Length);

string pictureSize = Encoding.ASCII.GetString(readBuffer, 0, data);

Console.WriteLine(pictureSize); //for debugging purposes so I can see the length

string x = new Random().Next().ToString();

FileStream f = new

Solution

ms.Close(); //close the MemoryStream when done


It's a good thing that you close the ms (MemoryStream) but there are two more to dispose.

The bitmap and graphics instances also need to be disposed/closed. The same applies to the NetworkStream but I got the impression that you don't know the using statement that allows you to automatically handle resources so:

NetworkStream stream = getSelectedClient().GetStream();
..
stream.Close();


would become:

using(var networkStream = getSelectedClient().GetStream())
{
    ..
} // at this point it will be closed/disposed by the runtime


The benefit of this is that it will be correctly disposed even if an exception occurs. Without it you would need to write the try/finally yourself (what a using actually does for you (or rahter the compiler))

NetworkStream networkStream = null;

try
{
    networkStream = getSelectedClient().GetStream();
    ..
}
finally
{
    networkStream?.Close();
}


You can use the same pattern for bitmaps, graphics and anything else that is IDisposable.

Code Snippets

ms.Close(); //close the MemoryStream when done
NetworkStream stream = getSelectedClient().GetStream();
..
stream.Close();
using(var networkStream = getSelectedClient().GetStream())
{
    ..
} // at this point it will be closed/disposed by the runtime
NetworkStream networkStream = null;

try
{
    networkStream = getSelectedClient().GetStream();
    ..
}
finally
{
    networkStream?.Close();
}

Context

StackExchange Code Review Q#154648, answer score: 5

Revisions (0)

No revisions yet.