On Mon, 2008-11-10 at 10:33 -0500, Jeff Campbell wrote:
> On Fri, Nov 7, 2008 at 7:46 AM, Andy Walls <awalls@radix.net> wrote:
>
> We set up the cx18 AV core to do the proper sample rates
> similar to the
> cx25840 driver. I made a change where I actually made sure
> the audio
> sample clock (the AUX_PLL) is over the long term locked to the
> video
> sample clock in an effort to fix this problem. So going into
> the
> encoder the audio samples are going at the nominal rate with
> minor
> jittering by the hardware to stay locked to the video samples
> to
> maintain a constant rate of audio/video samples on a frame by
> frame
> basis. See the CX25840/1/2/3 data sheet and cx18-av-audio.c
> and look
> for the EN_AV_LOCK comments.
>
> The only real improvement here could be to run the PLL's in
> the middle
> of there range at 400 MHz (200-600 MHz) as opposed to the 300
> MHz or so
> I know we run the AUX_PLL when using 32 ksps.
>
> After that, it's up to the APU and CPU on the CX23418 to
> handle it
> properly.
>
> I've got some great news.
>
> A colleague and I have been working on this for the past week or so
> and we believe we have isolated and corrected the problem.
>
> I apologize, but we are not familiar with the proper way to diff this
> and add it with mercurial
If you cloned it with hg (hg clone
http://linuxtv.org/hg/...) then "hg
diff" will show you a diff.
> so I'm going to submit the complete file here as an attachment and
> hopefully someone can merge it in to the tree.
That's for looking into this and taking initiative!
Now, just working my way through the differences and thinking out
loud...
1. The VIDIOC_G_CTRL ioctl case framed with the same code as
VIDIOC_INT_AUDIO_CLOCK_FREQ. This looks like an error to me. Stopping
the microcontroller and muting the audio system doesn't seem necessarily
or desirable for checking the state of audio controls.
2. No need to comment out the setting of AUD_COUNT or VID_COUNT, simply
setting the ENV_AV_LOCK bit to 0 in register 0x12b should do. For
example instead of this:
//cx18_av_write4(cx, 0x128, 0xa11d4bf8);
just do this:
cx18_av_write4(cx, 0x128, 0xa01d4bf8);
3. The changes of cx18_av_write(cx, reg, val) to
cx18_av_and_or(cx,reg,0xff,val) like this:
- cx18_av_write(cx, 0x127, 0x54);
+ cx18_av_and_or(cx, 0x127, 0xff, 0x54); // wmdb
make no difference AFAICT, as 0xff means change all the bits in this
byte register and all the bits are always explicitly set to 0x50 (a
subset of 0x50 and 0x54) at the beginning of the function.
4. The tweaks of the AUX_PLL_FRAC-tional part a really minor and depend
really on what value you assume for the crystal
case 48000:
/* VID_PLL and AUX_PLL */
cx18_av_write4(cx, 0x108, 0x100a040f);
/* AUX_PLL_FRAC */
/* 0xa.4c6b6ea * 28,636,363.63 / 0x10 = 48000 * 384 */
- cx18_av_write4(cx, 0x110, 0x0098d6dd);
+ cx18_av_write4(cx, 0x110, 0x0098D6E5);
When computing the original value I used the ideal crystal value of:
4.5 MHz/286*455/2*8 = 28.63636363... MHz
But due to rounding, you can back figure the "original" crystal value to
be:
28.636363661 MHz (about .025 Hz off the ideal value)
And back figuring the "new" crystal value from the updated number you
selected gives:
28.636362998 MHz (about .638 Hz off the ideal value)
In other words the Windows driver writers used an improperly rounded
crystal value of 28.63636300 MHz to compute their fractional divider
value.
But either way, the values are so close and probably well within the
error tolerance of an off-the-shelf crystal, that these refinements
shouldn't make a difference.
(I have attached a spreadsheet that you may find useful [or not]. N.B.
we only use BT.656 mode and not the square pixel modes.)
> Essentially, the clock timing mentioned above was causing the issue.
> Once removed, the TS and PS streams off the driver are stable and
> usable in real time with no audio drift or dropouts. We also adjusted
> some of the clock timings to match the windows driver and some other
> minor trimming.
So the only relevant change I see in the patch is the disabling
(actually not enabling) of EN_AV_LOCK in the high byte of register 0x128
(i.e. register 0x12b).
So now to get the changes you really that really fix you problem into
the driver, could you be so kind as to isolate which of the changes you
made actually fixes your problem? If you could start with the original
file and incrementally add in changes, testing to see which ones really
effect a solution, maybe going in this order:
1. Disabling EN_AV_LOCK
2. Changing the fractional divider values
3. Not changing AUD_COUNT and VID_COUNT from the chip defaults
4. Using cx18_av_and_or() instead of cx18_av_write() to set register
0x127
5. Stopping and muting the audio section to check the status of audio
controls
Thanks.
> There is appears to be an occasional hiccup with packet ordering or
> missing audio but its so minor all the players and set top boxes we
> tested against could adjust for it.
Please continue to test (I will too). I've have been lulled into
thinking my actions have fixed the problem before.
My impression is that the problem is dependent on the video content and
the MPEG encoder's internal state. Video that doesn't compress well
(snowy background, lots of things changing) seems to have less of an
issue than video that compresses extremely well (news anchor behind a
desk, talk show host & guest) in my experience. Also it seems like once
the encoder has been working on a particular scene for a while, the
audio jitter symptom doesn't seem as bad (maybe the application is
smoothing things out though).
If you have a canned video source, so you can replay the same video
again and again into the encoder while trying different changes, that
may help.
> Please review the changes, test and hopefully include in a future
> release.
Again thanks for working on this. Don't get discouraged. I appreciate
the help.
Regards,
Andy
> Thanks,
>
> -Jeff
