multi-thread socket program with packet fragmentation

[thinkerchjf]

Hi, Im new to C. I am trying to encode a series of BMP into JPEG, fragment it(becuz of MTU) and send to a UDP server. The server just echoes everything back to me(client). Then I construct a complete JPEG with enough fragments received from server.

Program structure:
socket send() is done in main thread when a timer goes off. and in WM_CREATE, I start a thread to listen on the socket and receive in an infinite loop, which is recvThread shown below.

Problem:
In debugging mode, I can see that each JPEG frame is received correctly when (recvNum == totalNum, set break point here). However, in run-time(or when I set break point in "jpeg_decode(jpegBuf, bmpBuf, pr->bmpFrameSize);"), each JPEG frame has some wrong data and the image is either incomplete or with some wrong part.

The following is some related code. I will paste more if necessary. New here, thanks in advance.

typedef struct {
	int* len;
	long* fn;
	long* pn;
	SOCKET fd;
	unsigned char *encBuf;
	unsigned char *frameBuf;
	unsigned char *outFrameBuf;
	struct JpegPacketStruct jpacket;
	struct sockaddr_in saddr;
	FIFO *fifo;
	HWND hwnd;
	HANDLE hEvent;
	BOOL bContinue;
	DWORD bmpFrameSize;
} FrameInfo;

void *
StartThread(int(*func)(void *arg), void *arg, int size) {
	DWORD id;
	int *argp;
	if(size > 0) {
		if((argp = malloc(size)) == 0)
			return 0;
		memcpy(argp, arg, size);
	}
	return CreateThread(0, STK_SIZE, (LPTHREAD_START_ROUTINE)func, argp, 0, &id);
}

void *recvThread(void *arg) {
	BYTE recvBuf[2048];
	BYTE bmpBuf[512*1024];
	BYTE jpegBuf[64*1024];
	BYTE *pjpegbuf = jpegBuf;
	BYTE *pbmpBuf = bmpBuf;

	int retval;
	int recvNum = 0;
	long pktNum, totalNum, frameNum;
	static long currentNum = 0;	//save serial number for current frame
	volatile FrameInfo *pr = (FrameInfo *)arg;
	JpegPacket jpacket;
	BYTE *pbuf;
	BYTE *prframebuf = pr->frameBuf;
	int saddr_len = sizeof(pr->saddr);

	while(1) {
		fd_set fds;
		struct timeval tv;
		int msec = 5000;
		FD_ZERO(&fds);
		FD_SET(pr->fd, &fds);
		tv.tv_sec = msec/1000;
		tv.tv_usec = msec%1000*1000;
		if(select((pr->fd)+1, &fds, NULL, NULL, &tv) <= 0)
			return -2;

		memset(recvBuf, 0, sizeof(recvBuf));
		//retval = recv(pr->fd, recvBuf, sizeof(jpacket), 0);
		retval = recvfrom(pr->fd, recvBuf, sizeof(jpacket), 0, (struct sockaddr *)(&pr->saddr), &saddr_len);
        if (retval == SOCKET_ERROR) {
			continue;
        }
		
		//frame serial number
		pbuf = recvBuf;
		pbuf += sizeof(jpacket.header.info);
		frameNum = getlong(pbuf);
		pbuf += sizeof(jpacket.header.frameNum);

		//fragment serial number
		pktNum = getlong(pbuf);
		pbuf += sizeof(jpacket.header.pktNum);

		//number of fragments
		totalNum = getlong(pbuf);
		pbuf += sizeof(jpacket.header.totalNum);
		pbuf += sizeof(jpacket.header.len);	//now pbuf points to the beginning of img data

		//move ptr to correct location according to pktNum and copy data
		pjpegbuf = jpegBuf;
		pjpegbuf += (pktNum)*sizeof(jpacket.payload);
		memcpy(pjpegbuf, pbuf, sizeof(jpacket.payload));
		
		//number of fragments received
		recvNum += 1;		
		if((recvNum == totalNum) || (pktNum == totalNum - 1)) {
			jpeg_decode(jpegBuf, bmpBuf, pr->bmpFrameSize);
			memcpy(pr->outFrameBuf, bmpBuf, pr->bmpFrameSize);
			
			//write to FIFO
			pbmpBuf = bmpBuf;
			pbmpBuf += sizeof(BITMAPFILEHEADER) + sizeof(BITMAPINFOHEADER);
			write_fifo(pr->fifo, pbmpBuf);

			PostMessage(pr->hwnd, WM_FRAME_RECEIVED, 0, 0);
			recvNum = 0;
			currentNum ++;
		}
	}
}

[brewbuck]

This whole segment of code seems pointless, since the call to recvfrom() would block exactly the same way select would:

FD_ZERO(&fds);
		FD_SET(pr->fd, &fds);
		tv.tv_sec = msec/1000;
		tv.tv_usec = msec%1000*1000;
		if(select((pr->fd)+1, &fds, NULL, NULL, &tv) <= 0)
			return -2;

Here, you fail to check that recvfrom() received the correct number of bytes:

retval = recvfrom(pr->fd, recvBuf, sizeof(jpacket), 0, (struct sockaddr *)(&pr->saddr), &saddr_len);
        if (retval == SOCKET_ERROR) {
			continue;
        }

And this whole section of code is pointless. It seems like you are trying to avoid receiving directly into a struct, yet you re-introduce all the problems of that technique by using sizeof() on the struct members to compute the offsets. You have removed ZERO compiler dependence by doing that, and you might as well just cast a pointer to the struct type and access the fields directly:

pbuf = recvBuf;
		pbuf += sizeof(jpacket.header.info);
		frameNum = getlong(pbuf);
		pbuf += sizeof(jpacket.header.frameNum);

		//fragment serial number
		pktNum = getlong(pbuf);
		pbuf += sizeof(jpacket.header.pktNum);

		//number of fragments
		totalNum = getlong(pbuf);
		pbuf += sizeof(jpacket.header.totalNum);
		pbuf += sizeof(jpacket.header.len);	//now pbuf points to the beginning of img data

		//move ptr to correct location according to pktNum and copy data
		pjpegbuf = jpegBuf;
		pjpegbuf += (pktNum)*sizeof(jpacket.payload);
		memcpy(pjpegbuf, pbuf, sizeof(jpacket.payload));

How do you deal with packets arriving out of order (it happens all the time)?

How do you deal with packets that get lost in the network (it happens all the time)?

[thinkerchjf]

Hi brewbuck, thanks for your quick reply. I am really to C, pls excuse my terrible coding. I just want to write a thing that runs correctly first.

the out-of-order and loss of packets are serious problems. so I am testing this with a udp server on localhost, is it still common? Thanks.

I was dealing with out-of-order fragments in the following code: construct JPEG buffer for one frame with all of its fragments according to the pktNum(serial number of fragment). for each frame, pktNum 0 will be placed at the beginning of the buffer, pktNum 2 at the [2] location, and so forth.

//move ptr to correct location according to pktNum and copy data
prframebuf = pr->frameBuf;
prframebuf += (pktNum)*sizeof(jpacket.payload);
memcpy(prframebuf, pbuf, sizeof(jpacket.payload));