Hi all,

On Mon, 2004-08-30 at 00:02, Ronald S. Bultje wrote:
> I'll write up a patch and have you and some other people test it. Should
> fix the zr36050 driver as well. Interestingly, the zr36050 driver has a
> appn default array with a 60-byte length marker in a 40-byte array, so
> that's another bug. Review is good. ;).
> 
> Give me a few days and I'll send the patch to the list.

See attachment. It should now write the correct APP/COM markers again,
like 0.7.x used to do. Please test and report if it works, breaks or
anything else. If I get no serious complaints, I'll send it off to LKML
in a few days. I might even release driver version 0.9.5 then.

Thanks,

Ronald
-- 
Ronald S. Bultje <[EMAIL PROTECTED]>
? .tmp_versions
Index: videocodec.h
===================================================================
RCS file: /cvsroot/mjpeg/driver-zoran/Attic/videocodec.h,v
retrieving revision 1.1.2.5
diff -u -r1.1.2.5 videocodec.h
--- videocodec.h	26 Feb 2004 16:35:22 -0000	1.1.2.5
+++ videocodec.h	31 Aug 2004 10:23:58 -0000
@@ -248,6 +248,17 @@
 	u16 Wt, Wa, HStart, HSyncStart, Ht, Ha, VStart;
 };
 
+struct jpeg_com_marker {
+	int len; /* number of usable bytes in data */
+	char data[60];
+};
+
+struct jpeg_app_marker {
+	int appn; /* number app segment */
+	int len; /* number of usable bytes in data */
+	char data[60];
+};
+
 struct videocodec {
 	struct module *owner;
 	/* -- filled in by slave device during register -- */
Index: zoran_device.c
===================================================================
RCS file: /cvsroot/mjpeg/driver-zoran/Attic/zoran_device.c,v
retrieving revision 1.1.2.19
diff -u -r1.1.2.19 zoran_device.c
--- zoran_device.c	29 Aug 2004 19:10:50 -0000	1.1.2.19
+++ zoran_device.c	31 Aug 2004 10:23:59 -0000
@@ -1036,7 +1036,10 @@
 
 	switch (mode) {
 
-	case BUZ_MODE_MOTION_COMPRESS:
+	case BUZ_MODE_MOTION_COMPRESS: {
+		struct jpeg_app_marker app;
+		struct jpeg_com_marker com;
+
 		/* In motion compress mode, the decoder output must be enabled, and
 		 * the video bus direction set to input.
 		 */
@@ -1046,6 +1049,19 @@
 
 		/* Take the JPEG codec and the VFE out of sleep */
 		jpeg_codec_sleep(zr, 0);
+
+		/* set JPEG app/com marker */
+		app.appn = zr->jpg_settings.jpg_comp.APPn;
+		app.len = zr->jpg_settings.jpg_comp.APP_len;
+		memcpy(app.data, zr->jpg_settings.jpg_comp.APP_data, 60);
+		zr->codec->control(zr->codec, CODEC_S_JPEG_APP_DATA,
+				   sizeof(struct jpeg_app_marker), &app);
+
+		com.len = zr->jpg_settings.jpg_comp.COM_len;
+		memcpy(com.data, zr->jpg_settings.jpg_comp.COM_data, 60);
+		zr->codec->control(zr->codec, CODEC_S_JPEG_COM_DATA,
+				   sizeof(struct jpeg_com_marker), &com);
+
 		/* Setup the JPEG codec */
 		zr->codec->control(zr->codec, CODEC_S_JPEG_TDS_BYTE,
 				   sizeof(int), &field_size);
@@ -1069,6 +1085,7 @@
 		dprintk(2, KERN_INFO "%s: enable_jpg(MOTION_COMPRESS)\n",
 			ZR_DEVNAME(zr));
 		break;
+	}
 
 	case BUZ_MODE_MOTION_DECOMPRESS:
 		/* In motion decompression mode, the decoder output must be disabled, and
Index: zr36050.c
===================================================================
RCS file: /cvsroot/mjpeg/driver-zoran/Attic/zr36050.c,v
retrieving revision 1.1.2.16
diff -u -r1.1.2.16 zr36050.c
--- zr36050.c	26 Feb 2004 16:35:22 -0000	1.1.2.16
+++ zr36050.c	31 Aug 2004 10:23:59 -0000
@@ -325,32 +325,6 @@
 	0xF9, 0xFA
 };
 
-static const char zr36050_app[0x40] = {
-	0xff, 0xe0,		//Marker: APP0
-	0x00, 0x3e,		//Length: 60+2
-	' ', 'A', 'V', 'I', '1', 0, 0, 0,	// 'AVI' field
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0
-};
-
-static const char zr36050_com[0x40] = {
-	0xff, 0xfe,		//Marker: COM
-	0x00, 0x3e,		//Length: 60+2
-	' ', 'C', 'O', 'M', 0, 0, 0, 0,	// 'COM' field
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0
-};
-
 /* jpeg baseline setup, this is just fixed in this driver (YUV pictures) */
 #define NO_OF_COMPONENTS          0x3	//Y,U,V
 #define BASELINE_PRECISION        0x8	//MCU size (?)
@@ -499,10 +473,18 @@
 				      sizeof(zr36050_dqt), zr36050_dqt);
 		sum += zr36050_pushit(ptr, ZR050_DHT_IDX,
 				      sizeof(zr36050_dht), zr36050_dht);
-		sum += zr36050_pushit(ptr, ZR050_APP_IDX,
-				      sizeof(zr36050_app), zr36050_app);
-		sum += zr36050_pushit(ptr, ZR050_COM_IDX,
-				      sizeof(zr36050_com), zr36050_com);
+		zr36050_write(ptr, ZR050_APP_IDX, 0xff);
+		zr36050_write(ptr, ZR050_APP_IDX + 1, 0xe0 + ptr->app.appn);
+		zr36050_write(ptr, ZR050_APP_IDX + 2, 0x00);
+		zr36050_write(ptr, ZR050_APP_IDX + 3, ptr->app.len + 2);
+		sum += zr36050_pushit(ptr, ZR050_APP_IDX, 60,
+				      ptr->app.data) + 4;
+		zr36050_write(ptr, ZR050_COM_IDX, 0xff);
+		zr36050_write(ptr, ZR050_COM_IDX + 1, 0xfe);
+		zr36050_write(ptr, ZR050_COM_IDX + 2, 0x00);
+		zr36050_write(ptr, ZR050_COM_IDX + 3, ptr->com.len + 2);
+		sum += zr36050_pushit(ptr, ZR050_COM_IDX, 60,
+				      ptr->com.data) + 4;
 
 		/* do the internal huffman table preload */
 		zr36050_write(ptr, ZR050_MARKERS_EN, ZR050_ME_DHTI);
@@ -553,8 +535,9 @@
 
 		/* this headers seem to deliver "valid AVI" jpeg frames */
 		zr36050_write(ptr, ZR050_MARKERS_EN,
-			      ZR050_ME_APP | ZR050_ME_DQT | ZR050_ME_DHT |
-			      ZR050_ME_COM);
+			      ZR050_ME_DQT | ZR050_ME_DHT |
+			      ((ptr->app.len > 0) ? ZR050_ME_APP : 0) |
+			      ((ptr->com.len > 0) ? ZR050_ME_COM : 0));
 	} else {
 		dprintk(2, "%s: EXPANSION SETUP\n", ptr->name);
 
@@ -733,6 +716,47 @@
 			return -EFAULT;
 		ptr->scalefact = *ival;
 		break;
+
+	case CODEC_G_JPEG_APP_DATA: {	/* get appn marker data */
+		struct jpeg_app_marker *app = data;
+
+		if (size != sizeof(struct jpeg_app_marker))
+			return -EFAULT;
+
+		*app = ptr->app;
+		break;
+	}
+
+	case CODEC_S_JPEG_APP_DATA: {	 /* set appn marker data */
+		struct jpeg_app_marker *app = data;
+
+		if (size != sizeof(struct jpeg_app_marker))
+			return -EFAULT;
+
+		ptr->app = *app;
+		break;
+	}
+
+	case CODEC_G_JPEG_COM_DATA: {	/* get comment marker data */
+		struct jpeg_com_marker *com = data;
+
+		if (size != sizeof(struct jpeg_com_marker))
+			return -EFAULT;
+
+		*com = ptr->com;
+		break;
+	}
+
+	case CODEC_S_JPEG_COM_DATA: {	/* set comment marker data */
+		struct jpeg_com_marker *com = data;
+
+		if (size != sizeof(struct jpeg_com_marker))
+			return -EFAULT;
+
+		ptr->com = *com;
+		break;
+	}
+
 	default:
 		return -EINVAL;
 	}
@@ -821,6 +845,12 @@
 	ptr->max_block_vol = 240;
 	ptr->scalefact = 0x100;
 	ptr->dri = 1;
+
+	/* no app/com marker by default */
+	ptr->app.appn = 0;
+	ptr->app.len = 0;
+	ptr->com.len = 0;
+
 	zr36050_init(ptr);
 
 	dprintk(1, KERN_INFO "%s: codec attached and running\n",
Index: zr36050.h
===================================================================
RCS file: /cvsroot/mjpeg/driver-zoran/Attic/zr36050.h,v
retrieving revision 1.1.2.4
diff -u -r1.1.2.4 zr36050.h
--- zr36050.h	23 Feb 2004 20:00:44 -0000	1.1.2.4
+++ zr36050.h	31 Aug 2004 10:24:00 -0000
@@ -27,6 +27,8 @@
 #ifndef ZR36050_H
 #define ZR36050_H
 
+#include "videocodec.h"
+
 /* data stored for each zoran jpeg codec chip */
 struct zr36050 {
 	char name[32];
@@ -51,6 +53,10 @@
 	__u8 v_samp_ratio[8];
 	__u16 scalefact;
 	__u16 dri;
+
+	/* com/app marker */
+	struct jpeg_com_marker com;
+	struct jpeg_app_marker app;
 };
 
 /* zr36050 register addresses */
Index: zr36060.c
===================================================================
RCS file: /cvsroot/mjpeg/driver-zoran/Attic/zr36060.c,v
retrieving revision 1.1.2.23
diff -u -r1.1.2.23 zr36060.c
--- zr36060.c	26 Feb 2004 16:35:22 -0000	1.1.2.23
+++ zr36060.c	31 Aug 2004 10:24:00 -0000
@@ -315,32 +315,6 @@
 	0xF9, 0xFA
 };
 
-static const char zr36060_app[0x40] = {
-	0xff, 0xe0,		//Marker: APP0
-	0x00, 0x07,		//Length: 7
-	' ', 'A', 'V', 'I', '1', 0, 0, 0,	// 'AVI' field
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0
-};
-
-static const char zr36060_com[0x40] = {
-	0xff, 0xfe,		//Marker: COM
-	0x00, 0x06,		//Length: 6
-	' ', 'C', 'O', 'M', 0, 0, 0, 0,	// 'COM' field
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0
-};
-
 /* jpeg baseline setup, this is just fixed in this driver (YUV pictures) */
 #define NO_OF_COMPONENTS          0x3	//Y,U,V
 #define BASELINE_PRECISION        0x8	//MCU size (?)
@@ -498,12 +472,18 @@
 		sum +=
 		    zr36060_pushit(ptr, ZR060_DHT_IDX, sizeof(zr36060_dht),
 				   zr36060_dht);
-		sum +=
-		    zr36060_pushit(ptr, ZR060_APP_IDX, sizeof(zr36060_app),
-				   zr36060_app);
-		sum +=
-		    zr36060_pushit(ptr, ZR060_COM_IDX, sizeof(zr36060_com),
-				   zr36060_com);
+		zr36060_write(ptr, ZR060_APP_IDX, 0xff);
+		zr36060_write(ptr, ZR060_APP_IDX + 1, 0xe0 + ptr->app.appn);
+		zr36060_write(ptr, ZR060_APP_IDX + 2, 0x00);
+		zr36060_write(ptr, ZR060_APP_IDX + 3, ptr->app.len + 2);
+		sum += zr36060_pushit(ptr, ZR060_APP_IDX + 4, 60,
+				      ptr->app.data) + 4;
+		zr36060_write(ptr, ZR060_COM_IDX, 0xff);
+		zr36060_write(ptr, ZR060_COM_IDX + 1, 0xfe);
+		zr36060_write(ptr, ZR060_COM_IDX + 2, 0x00);
+		zr36060_write(ptr, ZR060_COM_IDX + 3, ptr->com.len + 2);
+		sum += zr36060_pushit(ptr, ZR060_COM_IDX + 4, 60,
+				      ptr->com.data) + 4;
 
 		/* setup misc. data for compression (target code sizes) */
 
@@ -535,8 +515,9 @@
 
 		/* JPEG markers to be included in the compressed stream */
 		zr36060_write(ptr, ZR060_MER,
-			      ZR060_MER_App | ZR060_MER_Com | ZR060_MER_DQT
-			      | ZR060_MER_DHT);
+			      ZR060_MER_DQT | ZR060_MER_DHT |
+			      ((ptr->com.len > 0) ? ZR060_MER_Com : 0) |
+			      ((ptr->app.len > 0) ? ZR060_MER_App : 0));
 
 		/* Setup the Video Frontend */
 		/* Limit pixel range to 16..235 as per CCIR-601 */
@@ -841,6 +822,47 @@
 			return -EFAULT;
 		ptr->scalefact = *ival;
 		break;
+
+	case CODEC_G_JPEG_APP_DATA: {	/* get appn marker data */
+		struct jpeg_app_marker *app = data;
+
+		if (size != sizeof(struct jpeg_app_marker))
+			return -EFAULT;
+
+		*app = ptr->app;
+		break;
+	}
+
+	case CODEC_S_JPEG_APP_DATA: {	/* set appn marker data */
+		struct jpeg_app_marker *app = data;
+
+		if (size != sizeof(struct jpeg_app_marker))
+			return -EFAULT;
+
+		ptr->app = *app;
+		break;
+	}
+
+	case CODEC_G_JPEG_COM_DATA: {	/* get comment marker data */
+		struct jpeg_com_marker *com = data;
+
+		if (size != sizeof(struct jpeg_com_marker))
+			return -EFAULT;
+
+		*com = ptr->com;
+		break;
+	}
+
+	case CODEC_S_JPEG_COM_DATA: {	/* set comment marker data */
+		struct jpeg_com_marker *com = data;
+
+		if (size != sizeof(struct jpeg_com_marker))
+			return -EFAULT;
+
+		ptr->com = *com;
+		break;
+	}
+
 	default:
 		return -EINVAL;
 	}
@@ -930,6 +952,12 @@
 	ptr->max_block_vol = 240;	/* CHECKME, was 120 is 240 */
 	ptr->scalefact = 0x100;
 	ptr->dri = 1;		/* CHECKME, was 8 is 1 */
+
+	/* by default, no COM or APP markers - app should set those */
+	ptr->com.len = 0;
+	ptr->app.appn = 0;
+	ptr->app.len = 0;
+
 	zr36060_init(ptr);
 
 	dprintk(1, KERN_INFO "%s: codec attached and running\n",
Index: zr36060.h
===================================================================
RCS file: /cvsroot/mjpeg/driver-zoran/zr36060.h,v
retrieving revision 1.1.1.1.2.3
diff -u -r1.1.1.1.2.3 zr36060.h
--- zr36060.h	14 Jan 2003 21:18:47 -0000	1.1.1.1.2.3
+++ zr36060.h	31 Aug 2004 10:24:00 -0000
@@ -27,6 +27,8 @@
 #ifndef ZR36060_H
 #define ZR36060_H
 
+#include "videocodec.h"
+
 /* data stored for each zoran jpeg codec chip */
 struct zr36060 {
 	char name[32];
@@ -51,6 +53,10 @@
 	__u8 v_samp_ratio[8];
 	__u16 scalefact;
 	__u16 dri;
+
+	/* app/com marker data */
+	struct jpeg_app_marker app;
+	struct jpeg_com_marker com;
 };
 
 /* ZR36060 register addresses */

Reply via email to