[PATCH] zr36067 driver - correct jpeg app/com markers

Attached patch fixes the writing of APP (application-specific) and COM
(comment) markers in the JPEG stream as generated by the zr36050 or zr36060
encoders, both part of the zr36067 driver.

During separating those bits out of the main zoran driver, we accidently
removed the interface through which applications can adapt those markers,
replacing them by unchangeable (static) markers.  Some video formats,
particularly Quicktime and AVI, require specific container-specific markers in
the stream for interlaced streams.  Those markers specify field order and
such.

The attached patch re-adds this feature so that decoders depending on those
container-specific markers for correct playback will work correctly.
Signed-off-by: Ronald S. Bultje <rbultje@ronald.bitfreak.net>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

drivers/media/video/videocodec.h

@@ -248,6 +248,17 @@ struct tvnorm {
 	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 -- */

drivers/media/video/zoran_device.c

@@ -1033,7 +1033,10 @@ zr36057_enable_jpg (struct zoran          *zr,
 
 	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.
 		 */
@@ -1043,6 +1046,19 @@ zr36057_enable_jpg (struct zoran          *zr,
 
 		/* 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);
@@ -1066,6 +1082,7 @@ zr36057_enable_jpg (struct zoran          *zr,
 		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

drivers/media/video/zr36050.c

@@ -325,32 +325,6 @@ static const char zr36050_dht[0x1a4] = {
 	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 @@ zr36050_init (struct zr36050 *ptr)
 				      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 + 4, 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 + 4, 60,
+				      ptr->com.data) + 4;
 
 		/* do the internal huffman table preload */
 		zr36050_write(ptr, ZR050_MARKERS_EN, ZR050_ME_DHTI);
@@ -553,8 +535,9 @@ zr36050_init (struct zr36050 *ptr)
 
 		/* 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 @@ zr36050_control (struct videocodec *codec,
 			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 @@ zr36050_setup (struct videocodec *codec)
 	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",

drivers/media/video/zr36050.h

@@ -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 @@ struct zr36050 {
 	__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 */

drivers/media/video/zr36060.c

@@ -315,32 +315,6 @@ static const char zr36060_dht[0x1a4] = {
 	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 @@ zr36060_init (struct zr36060 *ptr)
 		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 @@ zr36060_init (struct zr36060 *ptr)
 
 		/* 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 @@ zr36060_control (struct videocodec *codec,
 			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 @@ zr36060_setup (struct videocodec *codec)
 	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",

drivers/media/video/zr36060.h

@@ -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 @@ struct zr36060 {
 	__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 */