Hi Andrew,
[...]
> We should probably use the in-kernel crc32 library...
Opss... I did not know about it ;) New patch attached. I hope I used the
kernel crc routine in the correct way - ffmpeg likes the PATs and PMTs
produced by the new code, hence it seems that the crc is computed
correctly.
> > The patch also fixes two things that I believe are typos:
> > 1) if (params->bitrate_mode == MPEG_BITRATE_MODE_MAX &&
> > params->video_target_bitrate <=
> > params->video_max_bitrate)
> > does not seem to be correct... Maybe is should be
> > if (params->bitrate_mode == MPEG_BITRATE_MODE_VBR &&
> > params->video_target_bitrate >=
> > params->video_max_bitrate)
> >
> > 2) the check on params->video_max_bitrate was performed even in the CBR
> > case
> >
> > I apologize if I misunderstood the code...
>
> Nice one! I'll have a look later; in the middle of fixing DVB stuff right now.
Ok. I noticed it because I was calling the MPEG_SETPARAMS ioctl()
without setting video_max_bitrate (I was setting bitrate_mode =
MPEG_BITRATE_MODE_CBR), and the ioctl was not working.
BTW, there is a bug that I forgot to mention: if the checks in
saa6752hs_init() fail, the ioctl() fails silently (i.e., the return
value is not negative). Since saa6752hs_init() is properly returning a
negative value, I think the problem is somewhere else... Uhm.. I think I
am seeing it: ts_init_encoder() in drivers/media/video/saa7134-ts.c is
always returning 0!!! Can we get a return value from
saa7134_i2c_call_clients()? Ok, I'll try to have a look at it...
> I have a slightly updated version with some better default bitrate parameters
> and tweaks... should I merge our two patches together & repost?
If you think that my patch is ok, that would be good.
> > Now, I am thinking about adding code for setting the video format (it is
> > currently hardcoded to D1). Is it ok to add yet another field to
> > mpeg_params?
>
> Fine by me - if you can, try and make it flexible; I was aiming for something
> that used by other MPEG encoders at a future date.
Unfortunately, I don't know about other MPEG encoders... I see that in
the saa6752hs there is a 1-byte parameter for selecting the video format
(D1, 2/3D1, 1/2D1, or SIF) and I was thinking about adding a field to
mpeg_params for setting the format. But I don't know if this is flexible
enough.
Finally, I also have a patch against saa7134-ts.c, for transforming
TS_NR_PACKETS in a module parameter (very useful for reducing the jitter
in the PCR!!!), and I wanted to submit it to Gerd Knorr, but it seems
that the v4l mailing list is not working (or I have been unsubscribed
for some reason, I don't know). I attach it to this mail; maybe you can
submit it when you will submit the next update to saa6752hs.c?
Thanks,
Luca
--
_____________________________________________________________________________
Copy this in your signature, if you think it is important:
N O W A R ! ! !
diff -urp ../linux-2.4.23/drivers/media/video/saa6752hs.c drivers/media/video/saa6752hs.c
--- ../linux-2.4.23/drivers/media/video/saa6752hs.c 2004-02-10 11:25:16.000000000 +0100
+++ drivers/media/video/saa6752hs.c 2004-05-13 13:13:56.000000000 +0200
@@ -11,6 +11,7 @@
#include <linux/types.h>
#include <linux/videodev.h>
#include <linux/init.h>
+#include <linux/crc32.h>
#include "i2c-compat.h"
#include "id.h"
@@ -233,22 +233,40 @@ static int saa6752hs_init(struct i2c_cli
{
unsigned char buf[3];
void *data;
+ u32 crc;
+ static unsigned int pcr_pid = 260, video_pid = 256 , audio_pid = 259;
// check the bitrate parameters first
if (params != NULL) {
if (params->bitrate_mode >= MPEG_BITRATE_MODE_MAX)
return -EINVAL;
- if (params->video_target_bitrate >= MPEG_VIDEO_TARGET_BITRATE_MAX)
- return -EINVAL;
- if (params->video_max_bitrate >= MPEG_VIDEO_MAX_BITRATE_MAX)
+ if (params->video_target_bitrate >= MPEG_VIDEO_MAX_BITRATE_MAX)
return -EINVAL;
if (params->audio_bitrate >= MPEG_AUDIO_BITRATE_MAX)
return -EINVAL;
if (params->total_bitrate >= MPEG_TOTAL_BITRATE_MAX)
return -EINVAL;
- if (params->bitrate_mode == MPEG_BITRATE_MODE_MAX &&
- params->video_target_bitrate <= params->video_max_bitrate)
- return -EINVAL;
+ if (params->bitrate_mode == MPEG_BITRATE_MODE_VBR &&
+ params->video_target_bitrate >= params->video_max_bitrate)
+ return -EINVAL;
+ if (params->bitrate_mode == MPEG_BITRATE_MODE_VBR &&
+ params->video_max_bitrate >= MPEG_VIDEO_TARGET_BITRATE_MAX)
+ return -EINVAL;
+ if (params->pcr_pid != 0) {
+ if (params->pcr_pid > MPEG_PID_MAX)
+ return -EINVAL;
+ pcr_pid = params->pcr_pid;
+ }
+ if (params->video_pid != 0) {
+ if (params->video_pid > MPEG_PID_MAX)
+ return -EINVAL;
+ video_pid = params->video_pid;
+ }
+ if (params->audio_pid != 0) {
+ if (params->audio_pid > MPEG_PID_MAX)
+ return -EINVAL;
+ audio_pid = params->audio_pid;
+ }
}
// Set GOP structure {3, 13}
@@ -277,12 +295,6 @@ static int saa6752hs_init(struct i2c_cli
buf[1] = 0x05;
i2c_master_send(client,buf,2);
- // Set Audio PID {0x103}
- buf[0] = 0xC1;
- buf[1] = 0x01;
- buf[2] = 0x03;
- i2c_master_send(client,buf,3);
-
// setup bitrate settings
data = i2c_get_clientdata(client);
if (params) {
@@ -292,10 +304,44 @@ static int saa6752hs_init(struct i2c_cli
// parameters were not supplied. use the previous set
saa6752hs_set_bitrate(client, (struct mpeg_params*) data);
}
-
+
+ /* compute CRCs */
+ crc = crc32_be(~0, &PAT[7], sizeof(PAT) - 7 - 4);
+ PAT[sizeof(PAT) - 4] = (crc >> 24) & 0xFF;
+ PAT[sizeof(PAT) - 3] = (crc >> 16) & 0xFF;
+ PAT[sizeof(PAT) - 2] = (crc >> 8) & 0xFF;
+ PAT[sizeof(PAT) - 1] = crc & 0xFF;
+
+ PMT[15] = 0xE0 | ((pcr_pid >> 8) && 0x0F);
+ PMT[16] = pcr_pid & 0xFF;
+ PMT[20] = 0xE0 | ((video_pid >> 8) && 0x0F);
+ PMT[21] = video_pid & 0xFF;
+ PMT[25] = 0xE0 | ((audio_pid >> 8) && 0x0F);
+ PMT[26] = audio_pid & 0xFF;
+ crc = crc32_be(~0, &PMT[7], sizeof(PMT) - 7 - 4);
+ PMT[sizeof(PMT) - 4] = (crc >> 24) & 0xFF;
+ PMT[sizeof(PMT) - 3] = (crc >> 16) & 0xFF;
+ PMT[sizeof(PMT) - 2] = (crc >> 8) & 0xFF;
+ PMT[sizeof(PMT) - 1] = crc & 0xFF;
+ // Set Audio PID
+ buf[0] = 0xC1;
+ buf[1] = (audio_pid >> 8) & 0xFF;
+ buf[2] = audio_pid & 0xFF;
+ i2c_master_send(client,buf,3);
+ // Set Video PID
+ buf[0] = 0xC0;
+ buf[1] = (video_pid >> 8) & 0xFF;
+ buf[2] = video_pid & 0xFF;
+ i2c_master_send(client,buf,3);
+ // Set PCR PID
+ buf[0] = 0xC4;
+ buf[1] = (pcr_pid >> 8) & 0xFF;
+ buf[2] = pcr_pid & 0xFF;
+ i2c_master_send(client,buf,3);
+
// Send SI tables
- i2c_master_send(client,PAT,sizeof(PAT));
- i2c_master_send(client,PMT,sizeof(PMT));
+ i2c_master_send(client,PAT,sizeof(PAT));
+ i2c_master_send(client,PMT,sizeof(PMT));
// mute then unmute audio. This removes buzzing artefacts
buf[0] = 0xa4;
diff -urp ../linux-2.4.23/drivers/media/video/saa6752hs.h drivers/media/video/saa6752hs.h
--- ../linux-2.4.23/drivers/media/video/saa6752hs.h 2004-02-10 11:25:16.000000000 +0100
+++ drivers/media/video/saa6752hs.h 2004-05-13 10:10:03.000000000 +0200
@@ -38,13 +38,17 @@ enum mpeg_audio_bitrate {
#define MPEG_VIDEO_TARGET_BITRATE_MAX 27000
#define MPEG_VIDEO_MAX_BITRATE_MAX 27000
#define MPEG_TOTAL_BITRATE_MAX 27000
-
+#define MPEG_PID_MAX ((1 << 14) - 1)
+
struct mpeg_params {
enum mpeg_bitrate_mode bitrate_mode;
unsigned int video_target_bitrate;
unsigned int video_max_bitrate; // only used for VBR
enum mpeg_audio_bitrate audio_bitrate;
unsigned int total_bitrate;
+ unsigned int video_pid;
+ unsigned int audio_pid;
+ unsigned int pcr_pid;
};
#define MPEG_SETPARAMS _IOW('6',100,struct mpeg_params)
diff -urp ../linux-2.4.23/drivers/media/video/saa7134-ts.c drivers/media/video/saa7134-ts.c
--- ../linux-2.4.23/drivers/media/video/saa7134-ts.c 2004-02-10 11:25:16.000000000 +0100
+++ drivers/media/video/saa7134-ts.c 2004-05-13 10:56:54.000000000 +0200
@@ -42,7 +42,9 @@ MODULE_PARM(tsbufs,"i");
MODULE_PARM_DESC(tsbufs,"number of ts buffers, range 2-32");
#define TS_PACKET_SIZE 188 /* TS packets 188 bytes */
-#define TS_NR_PACKETS 312
+static unsigned int ts_nr_packets = 30;
+MODULE_PARM(ts_nr_packets,"i");
+MODULE_PARM_DESC(ts_nr_packets,"size of a ts buffers (in ts packets)");
#define dprintk(fmt, arg...) if (ts_debug) \
printk(KERN_DEBUG "%s/ts: " fmt, dev->name , ## arg)
@@ -96,7 +98,7 @@ static int buffer_prepare(struct file *f
dprintk("buffer_prepare [%p,%s]\n",buf,v4l2_field_names[field]);
llength = TS_PACKET_SIZE;
- lines = TS_NR_PACKETS;
+ lines = ts_nr_packets;
size = lines * llength;
if (0 != buf->vb.baddr && buf->vb.bsize < size)
@@ -135,7 +137,7 @@ static int buffer_prepare(struct file *f
static int
buffer_setup(struct file *file, unsigned int *count, unsigned int *size)
{
- *size = TS_PACKET_SIZE * TS_NR_PACKETS;
+ *size = TS_PACKET_SIZE * ts_nr_packets;
if (0 == *count)
*count = tsbufs;
*count = saa7134_buffer_count(*size,*count);
@@ -353,7 +355,7 @@ static int ts_do_ioctl(struct inode *ino
f->fmt.pix.width = 720; /* D1 */
f->fmt.pix.height = 576;
f->fmt.pix.pixelformat = V4L2_PIX_FMT_MPEG;
- f->fmt.pix.sizeimage = TS_PACKET_SIZE*TS_NR_PACKETS;
+ f->fmt.pix.sizeimage = TS_PACKET_SIZE*ts_nr_packets;
return 0;
}
@@ -379,7 +381,7 @@ static int ts_do_ioctl(struct inode *ino
f->fmt.pix.width = 720; /* D1 */
f->fmt.pix.height = 576;
f->fmt.pix.pixelformat = V4L2_PIX_FMT_MPEG;
- f->fmt.pix.sizeimage = TS_PACKET_SIZE*TS_NR_PACKETS;
+ f->fmt.pix.sizeimage = TS_PACKET_SIZE*ts_nr_packets;
return 0;
}
@@ -472,9 +474,9 @@ int saa7134_ts_init1(struct saa7134_dev
saa_writeb(SAA7134_TS_SERIAL1, 0x00); /* deactivate TS softreset */
saa_writeb(SAA7134_TS_PARALLEL, 0xec); /* TSSOP high active, TSVAL high active, TSLOCK ignored */
saa_writeb(SAA7134_TS_PARALLEL_SERIAL, (TS_PACKET_SIZE-1));
- saa_writeb(SAA7134_TS_DMA0, ((TS_NR_PACKETS-1)&0xff));
- saa_writeb(SAA7134_TS_DMA1, (((TS_NR_PACKETS-1)>>8)&0xff));
- saa_writeb(SAA7134_TS_DMA2, ((((TS_NR_PACKETS-1)>>16)&0x3f) | 0x00)); /* TSNOPIT=0, TSCOLAP=0 */
+ saa_writeb(SAA7134_TS_DMA0, ((ts_nr_packets-1)&0xff));
+ saa_writeb(SAA7134_TS_DMA1, (((ts_nr_packets-1)>>8)&0xff));
+ saa_writeb(SAA7134_TS_DMA2, ((((ts_nr_packets-1)>>16)&0x3f) | 0x00)); /* TSNOPIT=0, TSCOLAP=0 */
return 0;
}
