- Fix data alignment issue with glReadPixel call, reported by Guillaume Bougard (#4954)

- Fix data alignment issue in raw AVI recording for weird resolutions (like 1366x768)

code/client/cl_avi.c

@@ -368,9 +368,13 @@ qboolean CL_OpenAVIForWriting( const char *fileName )
   else
     afd.motionJpeg = qfalse;
 
-  // Buffers only need to store RGB pixels
+  // Buffers only need to store RGB pixels.
-  afd.cBuffer = Z_Malloc(afd.width * afd.height * 3);
+  // Allocate a bit more space for the capture buffer to account for possible
-  afd.eBuffer = Z_Malloc(afd.width * afd.height * 3);
+  // padding at the end of pixel lines, and padding for alignment
+  #define MAX_PACK_LEN 16
+  afd.cBuffer = Z_Malloc((afd.width * 3 + MAX_PACK_LEN - 1) * afd.height + MAX_PACK_LEN - 1);
+  // raw avi files have pixel lines start on 4-byte boundaries
+  afd.eBuffer = Z_Malloc(PAD(afd.width * 3, AVI_LINE_PADDING) * afd.height);
 
   afd.a.rate = dma.speed;
   afd.a.format = WAV_FORMAT_PCM;
@@ -468,7 +472,7 @@ void CL_WriteAVIVideoFrame( const byte *imageBuffer, int size )
 {
   int   chunkOffset = afd.fileSize - afd.moviOffset - 8;
   int   chunkSize = 8 + size;
-  int   paddingSize = PAD( size, 2 ) - size;
+  int   paddingSize = PADLEN(size, 2);
   byte  padding[ 4 ] = { 0 };
 
   if( !afd.fileOpen )
@@ -542,7 +546,7 @@ void CL_WriteAVIAudioFrame( const byte *pcmBuffer, int size )
   {
     int   chunkOffset = afd.fileSize - afd.moviOffset - 8;
     int   chunkSize = 8 + bytesInBuffer;
-    int   paddingSize = PAD( bytesInBuffer, 2 ) - bytesInBuffer;
+    int   paddingSize = PADLEN(bytesInBuffer, 2);
     byte  padding[ 4 ] = { 0 };
 
     bufIndex = 0;

code/qcommon/q_shared.h

@@ -177,7 +177,8 @@ typedef int		sfxHandle_t;
 typedef int		fileHandle_t;
 typedef int		clipHandle_t;
 
-#define PAD(x,y) (((x)+(y)-1) & ~((y)-1))
+#define PAD(x,y)	(((x)+(y)-1) & ~((y)-1))
+#define PADLEN(x,y)	(PAD((x), (y)) - (x))
 
 #ifdef __GNUC__
 #define QALIGN(x) __attribute__((aligned(x)))

code/qcommon/qcommon.h

@@ -1008,6 +1008,9 @@ void S_ClearSoundBuffer( void );
 
 void SCR_DebugGraph (float value, int color);	// FIXME: move logging to common?
 
+// AVI files have the start of pixel lines 4 byte-aligned
+#define AVI_LINE_PADDING 4
+
 //
 // server interface
 //

code/renderer/tr_image_jpg.c

@@ -358,7 +358,7 @@ Expects RGB input data
 =================
 */
 size_t RE_SaveJPGToBuffer(byte *buffer, size_t bufSize, int quality,
-    int image_width, int image_height, byte *image_buffer)
+    int image_width, int image_height, byte *image_buffer, int padding)
 {
   struct jpeg_compress_struct cinfo;
   struct jpeg_error_mgr jerr;
@@ -398,7 +398,7 @@ size_t RE_SaveJPGToBuffer(byte *buffer, size_t bufSize, int quality,
 
   /* Step 5: while (scan lines remain to be written) */
   /*           jpeg_write_scanlines(...); */
-  row_stride = image_width * cinfo.input_components; /* JSAMPLEs per row in image_buffer */
+  row_stride = image_width * cinfo.input_components + padding; /* JSAMPLEs per row in image_buffer */
   
   while (cinfo.next_scanline < cinfo.image_height) {
     /* jpeg_write_scanlines expects an array of pointers to scanlines.
@@ -422,7 +422,7 @@ size_t RE_SaveJPGToBuffer(byte *buffer, size_t bufSize, int quality,
   return outcount;
 }
 
-void RE_SaveJPG(char * filename, int quality, int image_width, int image_height, unsigned char *image_buffer)
+void RE_SaveJPG(char * filename, int quality, int image_width, int image_height, byte *image_buffer, int padding)
 {
   byte *out;
   size_t bufSize;
@@ -430,7 +430,7 @@ void RE_SaveJPG(char * filename, int quality, int image_width, int image_height,
   bufSize = image_width * image_height * 3;
   out = ri.Hunk_AllocateTempMemory(bufSize);
 
-  bufSize = RE_SaveJPGToBuffer(out, bufSize, quality, image_width, image_height, image_buffer);
+  bufSize = RE_SaveJPGToBuffer(out, bufSize, quality, image_width, image_height, image_buffer, padding);
   ri.FS_WriteFile(filename, out, bufSize);
 
   ri.Hunk_FreeTempMemory(out);

code/renderer/tr_init.c

@@ -351,17 +351,64 @@ FIXME: the statics don't get a reinit between fs_game changes
 ============================================================================== 
 */ 
 
+/* 
+================== 
+RB_ReadPixels
+
+Reads an image but takes care of alignment issues for reading RGB images.
+
+Reads a minimum offset for where the RGB data starts in the image from
+integer stored at pointer offset. When the function has returned the actual
+offset was written back to address offset. This address will always have an
+alignment of packAlign to ensure efficient copying.
+
+Stores the length of padding after a line of pixels to address padlen
+
+Return value must be freed with ri.Hunk_FreeTempMemory()
+================== 
+*/  
+
+byte *RB_ReadPixels(int x, int y, int width, int height, size_t *offset, int *padlen)
+{
+	byte *buffer, *bufstart;
+	int padwidth, linelen;
+	GLint packAlign;
+	
+	qglGetIntegerv(GL_PACK_ALIGNMENT, &packAlign);
+	
+	linelen = width * 3;
+	padwidth = PAD(linelen, packAlign);
+	
+	// Allocate a few more bytes so that we can choose an alignment we like
+	buffer = ri.Hunk_AllocateTempMemory(padwidth * height + *offset + packAlign - 1);
+	
+	bufstart = (byte *) PAD((intptr_t) buffer + *offset, packAlign);
+	qglReadPixels(x, y, width, height, GL_RGB, GL_UNSIGNED_BYTE, bufstart);
+	
+	*offset = bufstart - buffer;
+	*padlen = padwidth - linelen;
+	
+	return buffer;
+}
+
 /* 
 ================== 
 RB_TakeScreenshot
 ================== 
 */  
-void RB_TakeScreenshot( int x, int y, int width, int height, char *fileName ) {
+void RB_TakeScreenshot(int x, int y, int width, int height, char *fileName)
-	byte		*buffer;
+{
-	int			i, c, temp;
+	byte *allbuf, *buffer;
+	byte *srcptr, *destptr;
+	byte *endline, *endmem;
+	byte temp;
+	
+	int linelen, padlen;
+	size_t offset = 18, memcount;
 		
-	buffer = ri.Hunk_AllocateTempMemory(glConfig.vidWidth*glConfig.vidHeight*3+18);
+	allbuf = RB_ReadPixels(x, y, width, height, &offset, &padlen);
-
+	buffer = allbuf + offset - 18;
+	
 	Com_Memset (buffer, 0, 18);
 	buffer[2] = 2;		// uncompressed type
 	buffer[12] = width & 255;
@@ -370,49 +417,62 @@ void RB_TakeScreenshot( int x, int y, int width, int height, char *fileName ) {
 	buffer[15] = height >> 8;
 	buffer[16] = 24;	// pixel size
 
-	qglReadPixels( x, y, width, height, GL_RGB, GL_UNSIGNED_BYTE, buffer+18 ); 
+	// swap rgb to bgr and remove padding from line endings
+	linelen = width * 3;
+	
+	srcptr = destptr = allbuf + offset;
+	endmem = srcptr + (linelen + padlen) * height;
+	
+	while(srcptr < endmem)
+	{
+		endline = srcptr + linelen;
 
-	// swap rgb to bgr
+		while(srcptr < endline)
-	c = 18 + width * height * 3;
+		{
-	for (i=18 ; i<c ; i+=3) {
+			temp = srcptr[0];
-		temp = buffer[i];
+			*destptr++ = srcptr[2];
-		buffer[i] = buffer[i+2];
+			*destptr++ = srcptr[1];
-		buffer[i+2] = temp;
+			*destptr++ = temp;
+			
+			srcptr += 3;
+		}
+		
+		// Skip the pad
+		srcptr += padlen;
 	}
 
+	memcount = linelen * height;
+
 	// gamma correct
-	if ( glConfig.deviceSupportsGamma ) {
+	if(glConfig.deviceSupportsGamma)
-		R_GammaCorrect( buffer + 18, glConfig.vidWidth * glConfig.vidHeight * 3 );
+		R_GammaCorrect(allbuf + offset, memcount);
-	}
 
-	ri.FS_WriteFile( fileName, buffer, c );
+	ri.FS_WriteFile(fileName, buffer, memcount + 18);
 
-	ri.Hunk_FreeTempMemory( buffer );
+	ri.Hunk_FreeTempMemory(allbuf);
 }
 
 /* 
 ================== 
 RB_TakeScreenshotJPEG
 ================== 
-*/  
+*/
-void RB_TakeScreenshotJPEG( int x, int y, int width, int height, char *fileName ) {
-	byte		*buffer;
-	size_t memcount;
 
-	memcount = glConfig.vidWidth * glConfig.vidHeight * 3;
+void RB_TakeScreenshotJPEG(int x, int y, int width, int height, char *fileName)
+{
+	byte *buffer;
+	size_t offset = 0, memcount;
+	int padlen;
 
-	buffer = ri.Hunk_AllocateTempMemory(memcount);
+	buffer = RB_ReadPixels(x, y, width, height, &offset, &padlen);
-
+	memcount = (width * 3 + padlen) * height;
-	qglReadPixels(x, y, width, height, GL_RGB, GL_UNSIGNED_BYTE, buffer);
 
 	// gamma correct
 	if(glConfig.deviceSupportsGamma)
-		R_GammaCorrect(buffer, memcount);
+		R_GammaCorrect(buffer + offset, memcount);
 
-	ri.FS_WriteFile( fileName, buffer, 1 );		// create path
+	RE_SaveJPG(fileName, r_screenshotJpegQuality->integer, width, height, buffer + offset, padlen);
-	RE_SaveJPG(fileName, r_screenshotJpegQuality->integer, glConfig.vidWidth, glConfig.vidHeight, buffer);
+	ri.Hunk_FreeTempMemory(buffer);
-
-	ri.Hunk_FreeTempMemory( buffer );
 }
 
 /*
@@ -518,8 +578,10 @@ the menu system, sampled down from full screen distorted images
 void R_LevelShot( void ) {
 	char		checkname[MAX_OSPATH];
 	byte		*buffer;
-	byte		*source;
+	byte		*source, *allsource;
 	byte		*src, *dst;
+	size_t			offset = 0;
+	int			padlen;
 	int			x, y;
 	int			r, g, b;
 	float		xScale, yScale;
@@ -527,17 +589,16 @@ void R_LevelShot( void ) {
 
 	Com_sprintf(checkname, sizeof(checkname), "levelshots/%s.tga", tr.world->baseName);
 
-	source = ri.Hunk_AllocateTempMemory( glConfig.vidWidth * glConfig.vidHeight * 3 );
+	allsource = RB_ReadPixels(0, 0, glConfig.vidWidth, glConfig.vidHeight, &offset, &padlen);
+	source = allsource + offset;
 
-	buffer = ri.Hunk_AllocateTempMemory( 128 * 128*3 + 18);
+	buffer = ri.Hunk_AllocateTempMemory(128 * 128*3 + 18);
 	Com_Memset (buffer, 0, 18);
 	buffer[2] = 2;		// uncompressed type
 	buffer[12] = 128;
 	buffer[14] = 128;
 	buffer[16] = 24;	// pixel size
 
-	qglReadPixels( 0, 0, glConfig.vidWidth, glConfig.vidHeight, GL_RGB, GL_UNSIGNED_BYTE, source ); 
-
 	// resample from source
 	xScale = glConfig.vidWidth / 512.0f;
 	yScale = glConfig.vidHeight / 384.0f;
@@ -546,7 +607,8 @@ void R_LevelShot( void ) {
 			r = g = b = 0;
 			for ( yy = 0 ; yy < 3 ; yy++ ) {
 				for ( xx = 0 ; xx < 4 ; xx++ ) {
-					src = source + 3 * ( glConfig.vidWidth * (int)( (y*3+yy)*yScale ) + (int)( (x*4+xx)*xScale ) );
+					src = source + (3 * glConfig.vidWidth + padlen) * (int)((y*3 + yy) * yScale) +
+						3 * (int) ((x*4 + xx) * xScale);
 					r += src[0];
 					g += src[1];
 					b += src[2];
@@ -566,8 +628,8 @@ void R_LevelShot( void ) {
 
 	ri.FS_WriteFile( checkname, buffer, 128 * 128*3 + 18 );
 
-	ri.Hunk_FreeTempMemory( buffer );
+	ri.Hunk_FreeTempMemory(buffer);
-	ri.Hunk_FreeTempMemory( source );
+	ri.Hunk_FreeTempMemory(allsource);
 
 	ri.Printf( PRINT_ALL, "Wrote %s\n", checkname );
 }
@@ -700,37 +762,70 @@ RB_TakeVideoFrameCmd
 const void *RB_TakeVideoFrameCmd( const void *data )
 {
 	const videoFrameCommand_t	*cmd;
-	size_t				memcount;
+	byte				*cBuf;
-	int				i;
+	size_t				memcount, linelen;
+	int				padwidth, avipadwidth, padlen, avipadlen;
+	GLint packAlign;
 	
 	cmd = (const videoFrameCommand_t *)data;
 	
-	qglReadPixels(0, 0, cmd->width, cmd->height, GL_RGB,
+	qglGetIntegerv(GL_PACK_ALIGNMENT, &packAlign);
-		GL_UNSIGNED_BYTE, cmd->captureBuffer);
 
-	memcount = cmd->width * cmd->height * 3;
+	linelen = cmd->width * 3;
+
+	// Alignment stuff for glReadPixels
+	padwidth = PAD(linelen, packAlign);
+	padlen = padwidth - linelen;
+	// AVI line padding
+	avipadwidth = PAD(linelen, AVI_LINE_PADDING);
+	avipadlen = avipadwidth - linelen;
+
+	cBuf = (byte *) PAD((intptr_t) cmd->captureBuffer, packAlign);
+		
+	qglReadPixels(0, 0, cmd->width, cmd->height, GL_RGB,
+		GL_UNSIGNED_BYTE, cBuf);
+
+	memcount = padwidth * cmd->height;
 
 	// gamma correct
-	if( glConfig.deviceSupportsGamma )
+	if(glConfig.deviceSupportsGamma)
-		R_GammaCorrect(cmd->captureBuffer, memcount);
+		R_GammaCorrect(cBuf, memcount);
 
-	if( cmd->motionJpeg )
+	if(cmd->motionJpeg)
 	{
-		memcount = RE_SaveJPGToBuffer(cmd->encodeBuffer, memcount,
+		memcount = RE_SaveJPGToBuffer(cmd->encodeBuffer, linelen * cmd->height,
 			r_aviMotionJpegQuality->integer,
-			cmd->width, cmd->height, cmd->captureBuffer);
+			cmd->width, cmd->height, cBuf, padlen);
 		ri.CL_WriteAVIVideoFrame(cmd->encodeBuffer, memcount);
 	}
 	else
 	{
-		for(i = 0; i < memcount; i += 3)    // swap R and B
+		byte *lineend, *memend;
+		byte *srcptr, *destptr;
+	
+		srcptr = cBuf;
+		destptr = cmd->encodeBuffer;
+		memend = srcptr + memcount;
+		
+		// swap R and B and remove line paddings
+		while(srcptr < memend)
 		{
-			cmd->encodeBuffer[i]     = cmd->captureBuffer[i + 2];
+			lineend = srcptr + linelen;
-			cmd->encodeBuffer[i + 1] = cmd->captureBuffer[i + 1];
+			while(srcptr < lineend)
-			cmd->encodeBuffer[i + 2] = cmd->captureBuffer[i];
+			{
+				*destptr++ = srcptr[2];
+				*destptr++ = srcptr[1];
+				*destptr++ = srcptr[0];
+				srcptr += 3;
+			}
+			
+			Com_Memset(destptr, '\0', avipadlen);
+			destptr += avipadlen;
+			
+			srcptr += padlen;
 		}
-
+		
-		ri.CL_WriteAVIVideoFrame(cmd->encodeBuffer, memcount);
+		ri.CL_WriteAVIVideoFrame(cmd->encodeBuffer, avipadwidth * cmd->height);
 	}
 
 	return (const void *)(cmd + 1);	

code/renderer/tr_local.h

@@ -1695,9 +1695,10 @@ void RE_StretchPic ( float x, float y, float w, float h,
 					  float s1, float t1, float s2, float t2, qhandle_t hShader );
 void RE_BeginFrame( stereoFrame_t stereoFrame );
 void RE_EndFrame( int *frontEndMsec, int *backEndMsec );
-void RE_SaveJPG(char * filename, int quality, int image_width, int image_height, unsigned char *image_buffer);
+void RE_SaveJPG(char * filename, int quality, int image_width, int image_height,
+                unsigned char *image_buffer, int padding);
 size_t RE_SaveJPGToBuffer(byte *buffer, size_t bufSize, int quality,
-		int image_width, int image_height, byte *image_buffer);
+		          int image_width, int image_height, byte *image_buffer, int padding);
 void RE_TakeVideoFrame( int width, int height,
 		byte *captureBuffer, byte *encodeBuffer, qboolean motionJpeg );