[Okta Webinar] Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 2527
  • Last Modified:

Winsock UDP Client/Server problems

Hi,

i'm developing a small video conferencing app. currently it's using the TCP protocol to send the video data.  I wanted to change the protocol to UDP as I get bad lag from TCP (expected).  

I have changed the protcols to UDP and chop the data into 512b packets to avoid fragmentation before sending them.  I have also added a CRC check on each video frame so I know if its bad.  I do get an occational frame but in all unless the video frame size is under 2K before I split it into 512k chunks I don't seem to get anything.  Here is a summary of the code.  I'm sure one of you UDP experts can tell me what I'm doing wrong.  Thanks

[Server Code]
/* SETUP */
void CApplication::SetUpServer()
{
  WORD       wVersionRequested = MAKEWORD(1, 1);  
  WSADATA wsaData;
  int res;
      
  //Winsock startup

   res = WSAStartup(wVersionRequested, &wsaData);

   if (res != 0) return;
   if (LOBYTE(wsaData.wVersion)!= 1 || HIBYTE(wsaData.wVersion)!= 1)
  {
    WSACleanup();
    return;
   }


   listenport = 1135;
   if ((m_sckServerVideo = socket(AF_INET, SOCK_DGRAM, 0)) < 0) return;
   memset((char *) &m_addr_ServerVideo, 0, sizeof(m_addr_ServerVideo));
   m_addr_ServerVideo.sin_family      = PF_INET; //TCP
   m_addr_ServerVideo.sin_addr.s_addr = htonl(INADDR_ANY);
   m_addr_ServerVideo.sin_port        = htons(listenport);

   if (bind(m_sckServerVideo, (sockaddr*)&m_addr_ServerVideo, sizeof(m_addr_ServerVideo)) < 0) return;

   m_hUDPVideo = (HWND)::CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)(CApplication::UDPVideoServer),this,0,&m_dwUDPVideoID);

   m_nClientOrServer = 2; //server
}

/* Thread */
void* CApplication::UDPVideoServer(void* pUser)
{

  CApplication* pApp = (CApplication*)pUser;
  int cliLen = sizeof(pApp->m_addr_ServerVideo);
  char m_wsdata_video[WINSOCK_BUFFER_SIZE] = {0};

   while(pApp->m_bMainRunning)
   {
                  
    res = recvfrom(pApp->m_sckServerVideo,m_wsdata_video, WINSOCK_BUFFER_SIZE, 0,(struct sockaddr *) &pApp->m_addr_ServerVideo, &cliLen);      
    if (res > 0) pApp->m_pReadHeader->ProcessVideo(m_wsdata_video,res);
            
    memset(m_wsdata_video,0,WINSOCK_BUFFER_SIZE);
   }
   
 return pUser;
}

[CLIENT CODE]

void CApplication::ClientConnect(char* pIP)
{

  WORD       wVersionRequested = MAKEWORD(1, 1);  
  WSADATA wsaData;
  int res;
      
  //Winsock startup

   res = WSAStartup(wVersionRequested, &wsaData);

   if (res != 0) return;
   if (LOBYTE(wsaData.wVersion)!= 1 || HIBYTE(wsaData.wVersion)!= 1)
  {
    WSACleanup();
    return;
   }

      
      m_sckClientVideo = socket(AF_INET, SOCK_DGRAM, 0);//UDP
      m_addr_ClientVideo.sin_port = htons(1135);
      m_addr_ClientVideo.sin_family = AF_INET;
      m_addr_ClientVideo.sin_addr.s_addr = inet_addr(pIP);
      res = connect(m_sckClientVideo, (struct sockaddr *)&m_addr_ClientVideo, sizeof(m_addr_ClientVideo));
      
      if (res == -1) {
            closesocket(m_sckClientVideo);
            closesocket(m_sckClientText);
            return; //exit out here
      }      

      //all ok lets go !!!
      WSAAsyncSelect(m_sckClientVideo, g_hWnd, SOCKET_MSGS, FD_READ|FD_CLOSE);

    m_nClientOrServer = 1; //Client

}


[USED BY BOTH CLIENT AND SERVER]
void CApplication::SplitUDPVideo(char* pDataX,int nlen)
{
//local copy
char* pPacket = new char[nlen];
memset(pPacket,0,nlen);
memcpy(pPacket,pDataX,nlen);

char szMaxPacket[512] = {0}; //max of 512
int nPacketSize = 512;
int nCurrentMarker = 0;
int nBytesLeft = 0;
int nBytesToSend = 0;
      
while (nCurrentMarker < nlen)
{
      nBytesLeft = nlen - nCurrentMarker;
            
            
      if (nlen <= nPacketSize)
      {
            memcpy(szMaxPacket,pPacket + nCurrentMarker,nlen);
            nCurrentMarker += nlen;
            nBytesToSend  = nlen;
      }else {
                  
            if ((nCurrentMarker + nPacketSize) > nlen) {
                  memcpy(szMaxPacket,pPacket + nCurrentMarker, nBytesLeft);
                        nCurrentMarker += nBytesLeft;
                        nBytesToSend  = nBytesLeft;
                  }else{
                  memcpy(szMaxPacket,pPacket + nCurrentMarker, nPacketSize);
                        nCurrentMarker += nPacketSize;
                        nBytesLeft = nPacketSize;
                  }

            }
            
            if (m_nClientOrServer == 1 )
            {      //Client
                  res = send(m_sckClientVideo,(char*)szMaxPacket,nBytesLeft,0);
            }else{
                  res = sendto(m_sckServerVideo,(char*)szMaxPacket,nBytesLeft,0,(struct sockaddr *)&m_addr_ServerVideo,sizeof(m_addr_ServerVideo));
            }
            
            if (res == SOCKET_ERROR)
            {
                  m_nSocketErrors++;
                  if (m_nSocketErrors > 50) { //something is wrong
                        m_nSocketErrors = 0; //reset
                        //reset your connection
                        ::SendMessage(g_hWnd,INET_RESET_CONNECTION,0,0);
                  }
                  
                  if (pPacket)
                  {
                        delete [] pPacket;
                        pPacket = NULL;
                  }


                  return;
            }else{ //Reset Errors
                  m_nSocketErrors = 0;
            }

      }

      
      if (pPacket)
      {
            delete [] pPacket;
            pPacket = NULL;
      }



}
0
windows_programing
Asked:
windows_programing
  • 10
  • 5
  • 5
  • +1
2 Solutions
 
_corey_Commented:
Well, the code is a *little* akward when you switch variable uses around, but it looks to be accurate.  However, I don't see the ProcessVideo method which is the most important here.  Sending might be right, but let's see receiving.

Also, I'd probably use AF_INET or PF_INET consistantly :)

Well, actually one problem I see is the deletion of pPacket at the end of the while() statement.

Shouldn't this wait until after while is done?
0
 
windows_programingAuthor Commented:
corey,

pPacket gets deleted in the while loop if there is a socket error as I return after it.  I pretty sure that the ProcessVideo method is working fine, it works great when I use TCP.  If I test the code on the LAN without spliting it into 512bytes it works great but this is not useable over the internet due to errors. Here is the ProcessVideo method...basically I put a CRC on the data before sending and spliting it which I then check here.

DWORD CHeaderReader::ProcessVideo(char* pData, int dwLen)
{


            //Simple Check on what is passed in
            if (!pData)
            {
                  return -1;
            }
            
            if (dwLen <=0 || dwLen > VIDEO_READ_BUFFER_SIZE)
            {
                  return -1;
            }




            //copy to Intermediate buffer

            if (m_dwTotalBytes_Video + dwLen < VIDEO_READ_BUFFER_SIZE)
            {
                  //Copy To the buffer      
                  memcpy(m_PacketBuffer_Video + m_dwTotalBytes_Video, (char*)pData, dwLen);
                  m_dwTotalBytes_Video += dwLen;
            }else{
                  m_dwTotalBytes_Video = 0;
                  //Take the chance to keen up the buffer
                  memset(m_PacketBuffer_Video,0,VIDEO_READ_BUFFER_SIZE); //Clean it out
                  //Fill it up with the incomming data
                  memcpy(m_PacketBuffer_Video + m_dwTotalBytes_Video, (char*)pData, dwLen);
                  return 0;
            }




            int nCounter = 0;

            //Verify we have a video stream in the buffer
            char* pIsVideoStart = NULL;
            char* pIsVideoEnd = NULL;
            char* pFound = NULL;
            
            
            pIsVideoStart = (char*)CUtil::GetNextWord(m_PacketBuffer_Video, "<video",m_dwTotalBytes_Video);
            if (!pIsVideoStart)
            {
            
                  return 0;
            }

            pIsVideoEnd = (char*)CUtil::GetNextWord(m_PacketBuffer_Video, "</video/>",m_dwTotalBytes_Video);
            if (!pIsVideoEnd)  
            {

                  return 0;//We've not got all the data
            }

             //Make sure the End > start
            if (pIsVideoStart >= pIsVideoEnd)
            {

                  return 0;//End before start
            }



            //Get the total length of the packet
            m_dwPacketLen = (pIsVideoEnd + strlen("</video/>")) - m_PacketBuffer_Video;
            if (m_dwPacketLen <= 0)
                        return -1; //Error


     
             pFound = (char*)CUtil::GetNextWord(m_PacketBuffer_Video, "<video datalen=",m_dwTotalBytes_Video);
             if (!pFound)
             {
                  
                   return 0; //not a valid video tag ... return       
             }



             //All ok lets bypass "<video datalen="
             pFound += strlen("<video datalen=");
     
             nCounter = 0;
             nCounter = CUtil::GetNextWhiteSpace(pFound);

             if (nCounter <= 0)
             {
                         return 0; // bad len
             }


             //Make a buffer to hold the video length value.
             char szLength[10] = { 0 };
             //copy to the buffer
             memcpy(szLength, pFound, nCounter);
             


             //Get the CRC !!!!!

             char* pCRC = NULL;
             pCRC = (char*)CUtil::GetNextWord(m_PacketBuffer_Video, "crc=",m_dwTotalBytes_Video);

             if (!pCRC)
             {
                   return 0; //No CRC stored in the data
             }

             //continue
             pCRC+=4;

             char szCRC[50] = { 0 };
             DWORD dwCRC = 0;
             
             nCounter = 0;
             nCounter = CUtil::GetNextWhiteSpace(pCRC);

             if (nCounter <= 0 || nCounter > 20)
             {
                   return 0; // bad len
             }
            
              //copy to the crc to the buffer
             memcpy(szCRC, pCRC, nCounter);
             dwCRC = atol(szCRC);



             //Now we have the length of the video data we need to skip the tag and get the video data
             char* pVideoData = CUtil::GetNextWord(pFound,"/>",m_dwTotalBytes_Video);    
 
             if (!pVideoData)
             {
                   return 0 ;
             }
            
             //continue
             pVideoData+=strlen("/>"); //pointer to the start of the data!!!
     
             
            

            
             //convert to a numeric format
             DWORD dwVideoLen = atol(szLength);
     
             //Create a buffer for the video data
             char* pVideoBuffer = new char[dwVideoLen];


             if (pVideoBuffer)
             {
         
                         //Clean new buffer
                        memset(pVideoBuffer, 0, dwVideoLen);
                        //copy the data to the new buffer
                        memcpy(pVideoBuffer, pVideoData, dwVideoLen);
                  
                  
                   //Check the CRC for the data is correct
                   CCRC crc;
                   DWORD dwNewCRC = 0;
                   crc.StringCrc32(pVideoBuffer,dwNewCRC);
                  
                   if (dwNewCRC == dwCRC) //CRC Correct
                   {
                         m_pVideo->SetRemoteData(pVideoBuffer,dwVideoLen);
                         ::SendMessage(g_hWnd,DO_VIDEO_FRAME,0,0);      
                   }else{
                        //Bad Frame, let the buffer clean up and return!!!!
                        ERR_OUT("BAD CRC");
                   }

               
             
             
                    //Clean up the allocated memory
                    delete [] pVideoBuffer;
                    pVideoBuffer = NULL;

             
             
             }


             //Circulate the buffer ... copy
            int dwResult =(int)(m_dwTotalBytes_Video - m_dwPacketLen);
             if ( dwResult > 0)
             {
                  memcpy(m_PacketBuffer_Video, m_PacketBuffer_Video + m_dwPacketLen, m_dwTotalBytes_Video - m_dwPacketLen);
                  m_dwTotalBytes_Video = m_dwTotalBytes_Video - m_dwPacketLen;
             }else if ((m_dwTotalBytes_Video - m_dwPacketLen) == 0) { //all data copied
                  memset(m_PacketBuffer_Video,0,VIDEO_READ_BUFFER_SIZE);
                  m_dwTotalBytes_Video = 0;
             }



      return 0;
}  
0
 
_corey_Commented:
Yes, but at the very very bottom you also delete it inside the while loop (if I'm counting {} correctly)
0
Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
windows_programingAuthor Commented:
It's probably a Cut and paste error on my part. Sorry.  It's not inside the loop in my cpp file. Thanks!
0
 
_corey_Commented:
       }else{
               m_dwTotalBytes_Video = 0;
               //Take the chance to keen up the buffer
               memset(m_PacketBuffer_Video,0,VIDEO_READ_BUFFER_SIZE); //Clean it out
               //Fill it up with the incomming data
               memcpy(m_PacketBuffer_Video + m_dwTotalBytes_Video, (char*)pData, dwLen);
               return 0;
          }

---

This is where you fill up the buffer until you get the whole packet, correct?  If so, why do you clear the buffer everytime before you copy the data to it?
0
 
grg99Commented:
Simplest fix:  go back to using reliable connections.  
The stuttering problem you can fix, just like everybody else does, by buffering up a few seconds on the receiving end before you start to play it.

Harder fixes:
    Don't dispose of your data before you're sure UDP has sent the whole packet.

( Sorry, but I couldnt follow your code at all )

0
 
_corey_Commented:
Do you get any more reliable connection by providing a small wait between packets?
0
 
windows_programingAuthor Commented:
corey,

I zero the memory here because the size of the incomming data + what is in the buffer is bigger than the buffer. so I zero the buffer and copy the new data to the start of the buffer.
0
 
_corey_Commented:
Do you start to process when you get partial packets or do you buffer up until you get a whole packet/crc and then try to process it?  I wasn't sure.

That's what I thought that part was doing, but you'll have to clarify if it waits until it gets all the packets or not.
0
 
windows_programingAuthor Commented:
corey,

I define a full packet (frame) like this ... (xxxx being the actual len of data and crc)

<video datalen=xxxx crc=xxxx />
data.data.data.data.data.data.data.data.data.data.data.
</video/>

So what I do is read in 1k at a time ... then copy it to the PacketBuffer_Video (which is about 50k) until I get a full packet frame, I then process it.  I use the crc to check that the data is the same as before it got sent.  I compare the stored crc with one I calculate on the data between <video datalen=xxxx crc=xxxx /> & </video/>.  If it's the same then I know that the data has not been corrupted so I process it.  the ERR_OUT is a macro "#define ERR_OUT(x) MessageBox(g_hWnd,x,"ERROR...",MB_OK | MB_ICONEXCLAMATION)" ... This way I know when the CRC are different.
0
 
grg99Commented:
>Do you get any more reliable connection by providing a small wait between packets?

No, there's not much you can do about packet droppage, but you can get rid of the choppyness by buffering up a few seconds of received data, so you have a cushion.

Also, how are you handling dropped UDP packets, or duplicated ones,  or ones delivered out of sequence?

All those things can and do happen.



0
 
windows_programingAuthor Commented:
grg99,

The only real check is on the Crc ... if the crc is good then the frame is processed.  I'm not doing any other resends or checks as the crc will be invalid if the packets are out of order or missing.

I really don't want to be using TCP because I'm sending other "control" data via TCP and it can get backlogged if there is a lot of video waiting to come in.

On another note would it be better use RTSP I've heard about it but cannot really find any examples.  
0
 
grg99Commented:
How are you doing the crc?  

If it's calculated on each packet, then the CRc won't detect duplicate or out-of-order packets.

If it's calculated from the beginning, a dropped packet is going to mess up all future CRC's.

0
 
windows_programingAuthor Commented:
The crc is calculated on the whole frame before it's split up into 512bytes and sent.  On the recieve side I then calculate the crc on the whole frame again (or the data between the <video datalen=xxxx crc=xxxx /> & </video/> identifiers).  A crc is calculated on every frame that's captured.

Maybe this is not a good way to do it? I'm open for suggestions :)
0
 
grg99Commented:
You need some kind of sequence number in each packet, as packets can arrive in ANY order, some will never show up, some will show up more than once.

You probably don't need explicit CRC's, as UDP has a checksum in each packet (usually, except on very old Sun systems), and the physical packet has a checksum.  If you get a packet at all, you can probably assume it's okay.  Even if it got burbled somehow, with video it's probably better to display a frame, even if it has a few bad pixels, than to not display it at all.  Your call.

0
 
windows_programingAuthor Commented:
I found a problem with the CRC class, the function actually calculates a crc on all data until if null terminates. I think what was happening was only a small part of the stream was being CRC'd. I think that I'll drop the crc and use a sequence number like you said.  

I’m still getting a problem, but now it’s different.  I get is an occasional WSAEWOULDBLOCK ? Do you know what that’s about?


0
 
grg99Commented:
>WSAEWOULDBLOCK ?

That usually means that I/O operation woul dblock,
on reading, obviously, if there's no thing to be read, or not enough,
on writing, you're writing stuff faster than the OS can ship it out.

0
 
windows_programingAuthor Commented:
grg,

Think I have it... The function CApplication::SplitUDPVideo is called from my VideoCallback Proc (VFW).  I think what is happening is when I get a new frame to process I'm already processing one in CApplication::SplitUDPVideo so it gives me WSAEWOULDBLOCK.  I verified this by droping the capture frame rate down.  I guess there are a few options :-  Which would recommend for sending the data?

1. Drop the frame rate down (which I really don't want to do)
2. Change the spliting of the data in  CApplication::SplitUDPVideo to ASM (so it processes faster)
2. Create a new thread everytime I get a call to CApplication::SplitUDPVideo
0
 
mxjijoCommented:

windows_programing,
     How did you pick the number 512 as your packet size ? Did you check the MTU ? Because if I remember it correct it varies from OS to OS and I have seend numbers like 1470 on some windows machines. The reason why I'm asking this is if you utilize this max limit, you get less number of send()'s and it gives you better performance. Especially if you are streaming, its very important. For example you might need a 100ms timer if you are sending 512 bytes packet but you can reduce it to 200ms timer if you make the size 1024 bytes. Btw, do you support re-send ?
0
 
windows_programingAuthor Commented:
mxjijo,

I got the number from MSDN "sendto"... here's what it says.

For sockets using IP (version 4):

To send a broadcast (on a SOCK_DGRAM only), the address in the to parameter should be constructed using the special IP address INADDR_BROADCAST (defined in WINSOCK2.H), together with the intended port number. It is generally inadvisable for a broadcast datagram to exceed the size at which fragmentation can occur, which implies that the data portion of the datagram (excluding headers) should not exceed 512 bytes.

Thanks :)
0
 
mxjijoCommented:

Well, 512 (576 infact) is the rock bottom limit of IP (not UDP) in RFC's.
But as I mentioned before, it varies - infact most of the implementations support more than that. (1472 typically)
Call getsockopt() with SO_MAX_MSG_SIZE flag to check this out.

I am not saying anyone "have to" do this but sending big buffers is always a good idea.
0
 
windows_programingAuthor Commented:
There seems to be alot of stuff here I need to work through!  Thanks for all the responses and comments !!!! :)
0

Featured Post

Free Tool: IP Lookup

Get more info about an IP address or domain name, such as organization, abuse contacts and geolocation.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

  • 10
  • 5
  • 5
  • +1
Tackle projects and never again get stuck behind a technical roadblock.
Join Now