Click here to Skip to main content
15,886,036 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
Hi Coders,

This is my first socket code and i hoping for some feedback/improvements!

I myself think allot of frames go to waste because of incorrect length (i see percentages above 100%), i think the wrong 8 bytes are read.
But i don't know how else to mark the start/end without adding some kind of large delay which would be the same as losing several frames.

Thank you for your time!


Image send code
private void video_NewFrame(object sender, NewFrameEventArgs eventArgs)
{
    //Disable new frame event while sending a frame
    if (videoSource != null)
        videoSource.NewFrame -= new NewFrameEventHandler(video_NewFrame);

    Image img = (Image)eventArgs.Frame.Clone();

    NetworkStream nfs = new NetworkStream(camSocket);
    MemoryStream ms = new MemoryStream();

    //Check if already sending a frame
    if (busy == false)
    {
    	//Same image to memory stream
        img.Save(ms, System.Drawing.Imaging.ImageFormat.Jpeg);
        //Set boolean to busy
        busy = true;
        int len = 0;
        long rdby = 0;
        byte[] tmp = new byte[8];

        //Send length (8 bytes)
        //Get the length of the memory stream and convert it to 8 bytes and write it to the tmp array
        BitConverter.GetBytes(ms.Length).CopyTo(tmp, 0);
        nfs.Write(tmp, 0, tmp.Length);

        //Send File
        ms.Seek(0, SeekOrigin.Begin);
        tmp = new byte[1024];
        while (rdby < ms.Length && nfs.CanWrite)
        {
            len = ms.Read(tmp, 0, tmp.Length);
            nfs.Write(tmp, 0, len);
            rdby = rdby + len;
        }
        ms.Close();
        busy = false;
        
    }

    //reattach event for new frames
    if (videoSource != null)
        videoSource.NewFrame += new NewFrameEventHandler(video_NewFrame);
}


Image receive code
public void WaitForData(SocketPacket s)
{
        if (pfnWorkerCallBack == null) 
        {
            pfnWorkerCallBack = new AsyncCallback(OnDataReceived);
        }
//Read 8 bytes (image length)
        s.currentSocket.BeginReceive(s.Buffer, 0, 8, SocketFlags.None, 	pfnWorkerCallBack, s);
}

public void OnDataReceived(IAsyncResult asyn)
{
    SocketPacket s = (SocketPacket)asyn.AsyncState;

    NetworkStream nfs = new NetworkStream(s.currentSocket);

    byte[] tmp = new byte[1024];
    
    //Convert the 8 read bytes (from BeginReceive) to file length
    long length = BitConverter.ToInt64(s.Buffer, 0);
    long read = 0;

    MemoryStream ms = new MemoryStream();
    while (read < length)
    {
        byte[] buffer = new byte[1024];
        int i = nfs.Read(buffer, 0, buffer.Length);
        ms.Write(buffer, 0, i);
        read = read + i;

        //Calculate percentage done
        int pc = (int)(((double)read / (double)length) * 100.00);
		log(pc + "%");
    }

    ms.Seek(0, SeekOrigin.Begin);
    
    //Invoke new frame event
    if (nImage != null)
        nImage(Image.FromStream(ms));

    WaitForData(s);
}


Socket setup
public Webcam()
{
    mainSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);

    IPEndPoint ipLocal = new IPEndPoint(IPAddress.Any, 1402);
    mainSocket.Bind(ipLocal);
    mainSocket.Listen(-1);

    mainSocket.BeginAccept(new AsyncCallback(OnClientConnect), null);
}

public void OnClientConnect(IAsyncResult asyn)
{
        Socket workerSocket = mainSocket.EndAccept(asyn);

        SocketPacket SocPkt = new SocketPacket(workerSocket);

        WaitForData(SocPkt);
        mainSocket.BeginAccept(new AsyncCallback(OnClientConnect), null);
}
Posted

1 solution

Here is my feedback:

Hard-coded immediate constants like 8, 1024, 1402, 100 are quite bad. How would you maintain such code? And the solution is so simple: declare them as explicit constants. This way, you can easily reuse them, modify consistently and move all or some of them to data files, resources or whatever is needed.

The names like video_NewFrame violate (good) Microsoft naming conventions.

Finally, the whole idea of using asynchronous API looks very questionable to me. I think that those APIs flourished when multithreading was not a common place, but even then I highly preferred threading. These days, multithreading with synchronous API is much more practical. You use synchronous API, which means some blocking calls, but this is not a problem because you execute it in some separate thread. It makes the logic more linear and observable. Even when you need complex thread synchronization, you solve this problem in a general ways designed for thread synchronization, not in ways specific to the particular asynchronous API you use. This, too, lets you do to less work with more readable, maintainable and reliable results.

—SA
 
Share this answer
 
Comments
enhzflep 24-Jul-12 15:23pm    
The last paragraph deserves 6/5. I can't, so I'll give you a +5 and an answer to a question that was perplexing you recently.
Q: Where do all of the posts with önclick instead of onlick come from?
A: people that post javascript code with onXXXXX functions, that aren't html encoded. It's a feature of the board that makes the code non-executable when displayed.

I mentioned it here earlier with 4 samples of the same code-block:

Where does code with önclick come from?
Sergey Alexandrovich Kryukov 24-Jul-12 16:05pm    
Thank you for voting; I appreciate your comment on my last paragraph. In fact, I did not expect that many will really understand my arguments in favor of threading -- too many people do not understand threading and do all kind of wasteful stuff to avoid it.

At to onclick... Wow! This is confusing. I actually tried to post things like onclick many times, and it worked correctly in all cases. After your note, I tried again and immediately obtained "önclick". Then I edited the code again, and got "onclick", could not get "önclick" again. What exactly do you mean by "HTML encoded"? Not that it's a big problem, but seeing all those "önclick" is somewhat irritating. (It also shows that few people actually read what they posted :-)

--SA
enhzflep 24-Jul-12 16:20pm    
That's okay mate. Your answers are almost always really clearly explained and seem well targeted at the poster and likely audience. It's also the sort of knowledge that is much easier & more quickly learnt from another than by one's own bitter experience. With multi-threading getting funky at random places for indeterminate reasons. It's also much more akin to standard sequential programming - it's just that you can do a couple of things sequentially at once. Again, a sterling answer. ******/5

"(It also shows that few people actually read what they posted :-) )"
I'm always so anal about re-reading mine that it's just a foreign concept to me, but a good source of a giggle. (edit: see I had to edit a long paragraph)

By 'html encoded', I mean all &, <, > and " characters converted to their corresponding html codes. The editor will do that automatically when you paste something (I think you can configure it not to)

You can (a) paste and have it converted automatically (b) paste it then hit the 'encode' button (c) encode using a 3rd party tool before copy and pasting.

I use Notepad++'s built in one [ TextFX->TextFX Convert->Encode HTML (&<>") ]
Then paste as plain text - it gives me control and prevents the editor from guessing the wrong language to use for syntax-highlighting.
lewax00 24-Jul-12 16:30pm    
Not quite built in, you have to install an extension for it...but thanks, I didn't know about that one, faster than text replacements!
enhzflep 24-Jul-12 16:34pm    
It's a pleasure - and a good swap..

I hadn't realized it was an extension - I guess I must've always ticked all the checkboxes every time I install/update it.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900