Re: [ivtv-devel] PVR150 Tinny/fuzzy audio w/ patch?
Am Donnerstag, 31. Dezember 2009 20:40:15 schrieb Andy Walls: On Thu, 2009-12-31 at 14:26 -0500, Andy Walls wrote: On Thu, 2009-12-31 at 18:15 +0100, Martin Dauskardt wrote: Some corrections to errors: My preferences in summary, is that not matter what the digitizer chip: My preferences are, in summary, that no matter what the digitizer chip: a. I'd like to keep the audio clocks always up to avoid tinny audio. b. I'd also like to inhibit the video clock and add the delay after ^^^ refine re-enabling the digitizer to avoid the *potential* for a hung machine. A value smaller than 300 ms should work, but a value smaller than 40 ms may not work, if my hypothesis is correct. c. I do not care to much about the delay after disbaling the video clock, only that it is empirically long enough. Thanks for taking the time to test and comment. Regards, Andy Regards, Andy I tested various sleep values: http://home.arcor-online.de/martin.dauskardt/digitizer_msleep.xls It seems that we only need a total delay of 300ms between the previous actions in ivtv-streams.c and the start of the capture. This also secures that we don't see disturbance from a previous frequency switch. This happens with smaller sleep values: http://home.arcor-online.de/martin.dauskardt/channelswitch.mpg and can lead to the infamous flickering problem (http://www.gossamer- threads.com/lists/ivtv/devel/32970) . The current driver has this problem only with cx23415 (PVR350), not cx23416. My preference is: -let it how it is (300 ms sleep before the firmware call) or -split the 300ms to 150 and 150 Greets, Martin -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ivtv-devel] PVR150 Tinny/fuzzy audio w/ patch?
Hi everybody,
first test results from me:
As expected, the double ivtv_msleep_timeout(300, 1); in ivtv-streams.c
increases the time for stopping/starting a stream. I removed the first call
and it still works fine.
@ Mike:
Previously I suggested to add a msleep(300) in state_eval_decoder_run
(pvrusb2-hdw.c), after calling pvr2_decoder_enable(hdw,!0).
With the change from Andy I now have again sporadic black screens with my
saa7115-based PVRUSB2. So I moved the sleep directly into static int
pvr2_decoder_enable:
--- v4l-dvb-bugfix-7753cdcebd28-orig/v4l-dvb-
bugfix-7753cdcebd28/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c
2009-12-24
17:06:08.0 +0100
+++ v4l-dvb-bugfix-7753cdcebd28-patched/v4l-dvb-
bugfix-7753cdcebd28/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c
2009-12-31
17:19:22.836251706 +0100
@@ -1716,6 +1716,7 @@
(enablefl ? on : off));
v4l2_device_call_all(hdw-v4l2_dev, 0, video, s_stream, enablefl);
v4l2_device_call_all(hdw-v4l2_dev, 0, audio, s_stream, enablefl);
+ if (enablefl != 0) msleep(300);
if (hdw-decoder_client_id) {
/* We get here if the encoder has been noticed. Otherwise
we'll issue a warning to the user (which should
Funny- this seems to work, no more black screens appeared.
The remaining questions are in my opinion:
1.)
What is Hans opinion about the changes, especially the move of the 300ms sleep
from after disabling the digitizer to after enabling it ?
2.)
Do we want to keep disabling the digitizer during the
CX2341X_ENC_INITIALIZE_INPUT call in case the digitizer is a cx25840x ?
It seems to be necessary only for the saa7115.
Note: The cx88-blackbird-driver does also no disabling/enabling of the
digitizer (cx2388x) when doing this firmware call.
3.)
Does Andys Patch solve the tinny audio problem for Argus (who originally
posted the problem and a different solution in the ivtv-devel list). I add him
in cc.
Greets and Happy New Year
Martin
PS:
Readers on the ivtv-devel ML list will miss previous postings (the list was
down a few days). Please have a look in
http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/14151
and
http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/14155
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ivtv-devel] PVR150 Tinny/fuzzy audio w/ patch?
On Thu, 2009-12-31 at 18:15 +0100, Martin Dauskardt wrote:
Hi everybody,
first test results from me:
As expected, the double ivtv_msleep_timeout(300, 1); in ivtv-streams.c
increases the time for stopping/starting a stream. I removed the first call
and it still works fine.
Yes, I knew the initial 300 ms delay before the call to
CX2341X_ENC_INITIALIZE_INPUT was lame, but it should always be safe. I
really want to avoid a video signal from the saa7115 causing the
CX2341[56] to hang the PCI bus.
I was hoping you could find a small number empirically that was safe. I
was thinking at *most* 10 ms for lines to settle: time for audio clocks
to come up to avoid tinny audio and time for video clocks to stop to
avoid PCI bus hangs. It sounds like you determined 0 ms is OK. I
suspect then, that the time to set up and send the
CX2341X_ENC_INITIALIZE_INPUT command to the encoder provides enough
delay.
@ Mike:
Previously I suggested to add a msleep(300) in state_eval_decoder_run
(pvrusb2-hdw.c), after calling pvr2_decoder_enable(hdw,!0).
With the change from Andy I now have again sporadic black screens with my
saa7115-based PVRUSB2.
Hmmm. That's odd. All I did was separate out video and audio clock
enable/disable into two seperate steps for CX2584x units, or add a small
delay for SAA7115 units.
So I moved the sleep directly into static int
pvr2_decoder_enable:
--- v4l-dvb-bugfix-7753cdcebd28-orig/v4l-dvb-
bugfix-7753cdcebd28/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c
2009-12-24
17:06:08.0 +0100
+++ v4l-dvb-bugfix-7753cdcebd28-patched/v4l-dvb-
bugfix-7753cdcebd28/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c
2009-12-31
17:19:22.836251706 +0100
@@ -1716,6 +1716,7 @@
(enablefl ? on : off));
v4l2_device_call_all(hdw-v4l2_dev, 0, video, s_stream, enablefl);
v4l2_device_call_all(hdw-v4l2_dev, 0, audio, s_stream, enablefl);
+ if (enablefl != 0) msleep(300);
if (hdw-decoder_client_id) {
/* We get here if the encoder has been noticed. Otherwise
we'll issue a warning to the user (which should
Funny- this seems to work, no more black screens appeared.
The remaining questions are in my opinion:
1.)
What is Hans opinion about the changes, especially the move of the 300ms
sleep
from after disabling the digitizer to after enabling it ?
My opinion is to use as short a delay as needed before the call to
CX2341X_ENC_INITIALIZE_INPUT, to avoid tinny audio and avoid PCI bus
hangs.
If you found 0 ms delay was fine, then perhaps I'll just remove that
delay or use something like 1 ms, OK?
For the delay after the CX2341X_ENC_INITIALIZE_INPUT, I think it is
good, as there are probably a few things going on:
a. The digitizer clock lines stabilizing as mentioned earlier
b. The CX2341X_ENC_INITIALIZE_INPUT call may still be executing on the
CX23416, when the ivtv_vapi() returns (I haven't disassembeld the
firmware to check if that is the actually case and how long it might
take).
c. before the call to CX2341X_ENC_START_CAPTURE to start the *first*
capture, the CX2341[56] may need a few valid rasters from the digitizer
stored in its memory.
So the delay after enabling the digitzer makes sure at least a few
rasters are available for the CX2341[56] to operate on. A 300 ms delay
is 15 fields for PAL and 18 fields for NTSC. A wait of 100 ms would be
5 fields for PAL and 6 fields for NTSC, and probably enough.
I think this hypothesis is consistent with the pvrusb2 and ivtv hardware
behavior you noticed.
2.)
Do we want to keep disabling the digitizer during the
CX2341X_ENC_INITIALIZE_INPUT call in case the digitizer is a cx25840x ?
It seems to be necessary only for the saa7115.
Note: The cx88-blackbird-driver does also no disabling/enabling of the
digitizer (cx2388x) when doing this firmware call.
I would want to do so.
It is my understanding that, notwithstanding any firmware bugs, the
machines in the CX2341[56] chips have some hardware bugs. I want to
avoid tripping over those bugs and locking up someone's machine
intermittently.
The CX2388[0123] is a differnent type of machine internally, not subject
to the same of hardware bugs that the CX2341[56] have that may hang the
machine/PCI bus. The CX2388x should be providing the DMA engines and
the PCI bus connection. I do not know the full details of the Blackbird
board design, but I assume the CX23416 does not interface directly to
the PCI bus along with the CX2388x (which would require a PCI bridge
chip on the card much like a PVR-500).
However, yes it could be the case the Conexant digitizer (CX2584x and
CX2388x internal) output VIP/BT.656 data in a form that at all times
never causes the CX23416 any problems. I really just don't know what
conditions cause the CX23416 to intermittently hang a machine, so I'd
prefer want to avoid any potential causes of hangs.
3.)
Does Andys Patch solve the tinny audio
Re: [ivtv-devel] PVR150 Tinny/fuzzy audio w/ patch?
On Thu, 2009-12-31 at 14:26 -0500, Andy Walls wrote: On Thu, 2009-12-31 at 18:15 +0100, Martin Dauskardt wrote: Some corrections to errors: My preferences in summary, is that not matter what the digitizer chip: My preferences are, in summary, that no matter what the digitizer chip: a. I'd like to keep the audio clocks always up to avoid tinny audio. b. I'd also like to inhibit the video clock and add the delay after ^^^ refine re-enabling the digitizer to avoid the *potential* for a hung machine. A value smaller than 300 ms should work, but a value smaller than 40 ms may not work, if my hypothesis is correct. c. I do not care to much about the delay after disbaling the video clock, only that it is empirically long enough. Thanks for taking the time to test and comment. Regards, Andy Regards, Andy Greets and Happy New Year Martin PS: Readers on the ivtv-devel ML list will miss previous postings (the list was down a few days). Please have a look in http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/14151 and http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/14155 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ivtv-devel] PVR150 Tinny/fuzzy audio w/ patch?
Martin,
(Moving discussion to the linux-media list since the ivtv-devel list was
unresponsive/dead recently.)
On Sun, 2009-12-06 at 19:44 +0100, Martin Dauskardt wrote:
From: Andy Walls awa...@radix.net
Martin,
It is not turning the digitizer on/off but really enable/disable the
clock output pins of the digitizer. The cause of the tinny audio is
likely that the clock pins from the CX2584x are floating.
This call makes sure the clock output enables are turned on and the pins
are not floating. I suspect no where else are they explciity enabled in
the driver before the first capture (I can go back and verify).
I am only a hobbyist, so please forgive me if there is an error ...
No that's fine too; as long as *somewhere* in the ivtv driver we turn on
the CX2584x's output enables before the first capture.
It really seems that no where else the clock output pins are enabled. I did
not find another s_stream call to the digitizer. But amazing: Even I comment
out the line
v4l2_subdev_call(itv-sd_video, video, s_stream, 1);
the stream starts most of the time (but fails occasionally).
You wrote I suspect that these signals floating cause the CX23416 to make
some bad guesses about audio sample rate when CX2341X_ENC_INTIALIZE_INPUT is
called.
Then we have to enable the digitizer before CX2341X_ENC_INTIALIZE_INPUT.
But the first patch from Argus -which helped him to solve the problem- does
not enable the output. So this is either not necessary or the real problem is
something else. (I made a lot of testing with a PVR150 and an unpatched
driver
and had never any audio problems)
Unfortunately there was a mistake in my last code. The if-statement has to be
in additionally brackets, otherwise it is never true and the digitizer is not
disabled for saa7115-based card.
I tested this code now with PVR150 and PVR350:
if (atomic_read(itv-capturing) == 0) {
/* Clear all Pending Interrupts */
ivtv_set_irq_mask(itv, IVTV_IRQ_MASK_CAPTURE);
clear_bit(IVTV_F_I_EOS, itv-i_flags);
/* Turn off non-cx25840 digitizer to avoid flickering */
if (!(itv-sd_video-grp_id IVTV_HW_CX25840)) {
v4l2_subdev_call(itv-sd_video, video, s_stream, 0);
}
else {
/* make sure the cx25840 clock output pins are enabled and
not
floating */
v4l2_subdev_call(itv-sd_video, video, s_stream, 1);
}
ivtv_vapi(itv, CX2341X_ENC_INITIALIZE_INPUT, 0);
/* Turn on non-cx25840 digitizer and allow clock
output of the digitizer to stabilize before starting
capture */
if (!(itv-sd_video-grp_id IVTV_HW_CX25840)) {
v4l2_subdev_call(itv-sd_video, video, s_stream, 1);
ivtv_msleep_timeout(300, 1);
}
}
You mentioned that the 300ms sleep is probably needed for the clock output of
the digitizer to stabilize. For me it seems more logical if this is done
after ***enabling*** the digitizer output, so I moved this. It seems to work
fine.
I made a similar test with the pvrusb2 driver (which has a black video
problem
and currently no timeout sleep after the digitizer switches). There it works
better when we place the sleep after re-enabling the digitizer. So I think
the
above solution should be right.
I also tested if we can avoid disabling the digitizer for saa7115. Both
pvrusb2 (old model 29xxx) and PVR350 show disturbance (sync problems) when
stopping/starting a stream. This leads to flickering with cx23415 (PVR350).
The cx23416 has no flickering problem, but it is annoying to see sync
problems
during channel switches.
The cx25840 devices (PVR150 and the pvrusb2-based HVR1900) do not show these
problems. The connection to the cx23416 must be different - the new stream
appears always without any snc issues.
Suggestion:
We need more testers to see if a driver change like above is safe for
everybody. Even if it turns out that it is no reliable fix against tinny
audio, it improves the speed for encoder stop/start on cx25840 devices.
I don't know if Myth is stopping/starting the stream for every channel
switch,
but for vdr (pvrinput-plugin) we are doing this. And there I recognize that
channel switching is faster now with cx25840 devices.
And we shouldn't do anything without hearing Hans Hans, where are you?
Greets,
Martin
I have a version of the change for the ivtv/PVR-150 tinny audio fix at
http://linuxtv.org/hg/~awalls/v4l-dvb-bugfix
http://linuxtv.org/hg/~awalls/v4l-dvb-bugfix/rev/7753cdcebd28
It separates out the enable/disable of audio video streaming from each
other for the cx25840 module. Then the ivtv driver can set them
independently to
Re: [ivtv-devel] PVR150 Tinny/fuzzy audio w/ patch?
Hi everybody and Merry Christmas,
Andy Walls wrote:
I have a version of the change for the ivtv/PVR-150 tinny audio fix at
http://linuxtv.org/hg/~awalls/v4l-dvb-bugfix
http://linuxtv.org/hg/~awalls/v4l-dvb-bugfix/rev/7753cdcebd28
It separates out the enable/disable of audio video streaming from each
other for the cx25840 module. Then the ivtv driver can set them
independently to avoid both the unpredictable PCI hang and the tinny
audio in a very generic way. Please test when you can.
@ Andy:
now we have a second 300ms delay in ivtv-streams.c
2.6/* Initialize Digitizer for Capture */
2.7 + /* Avoid tinny audio problem - ensure audio clocks are
going */
2.8 + v4l2_subdev_call(itv-sd_audio, audio, s_stream, 1);
2.9 + /* Avoid unpredictable PCI bus hang - disable video
clocks */
2.10v4l2_subdev_call(itv-sd_video, video, s_stream, 0);
2.11ivtv_msleep_timeout(300, 1);
2.12ivtv_vapi(itv, CX2341X_ENC_INITIALIZE_INPUT, 0);
2.13v4l2_subdev_call(itv-sd_video, video, s_stream, 1);
2.14 + ivtv_msleep_timeout(300, 1);
This would increase the time for channel switching (using encoder stop/start)
noticeable. My suggestion was to move the 300ms sleep at the point after the
stream is re-enabled, so we don't need the first msleep.
I have not tested your code, hope I can do this during the next days.
Mike,
I had to add another subdev call to pvrusb2 to get the same end result
of s_stream calls to the cx25840 module.
@ Mike:
do you remember my posting in the pvrusb2 list about sporadic black screen
after starting a stream? We had the same problem in ivtv, and the 300ms sleep
after disabling the digitizer solved the problem.
I implemented this in a similary way in the pvrusb2 driver and the problem
never appeared again:
--- v4l-dvb-309f16461cf4-orig/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c
2009-12-05 13:34:21.0 +0100
+++ v4l-dvb-309f16461cf4-patched/linux/drivers/media/video/pvrusb2/pvrusb2-
hdw.c 2009-12-24 22:42:49.746899065 +0100
@@ -4689,6 +4689,7 @@
del_timer_sync(hdw-quiescent_timer);
if (hdw-flag_decoder_missed) return 0;
if (pvr2_decoder_enable(hdw,!0) 0) return 0;
+ msleep(300);
hdw-state_decoder_quiescent = 0;
hdw-state_decoder_run = !0;
}
My initial idea was to avoid disabling/enabling the digitizer for devices with
cx2584x-digitizer. Channel changing (using encoder stop/start) with a PVR150
and HVR1900 took always about a second longer than with saa7115-based devices.
Without disabling/enabling the digitizer around the
CX2341X_ENC_INITIALIZE_INPUT call it speeds up noticeable.
So this is the alternate patch for the pvrusb2 driver (similar code for ivtv
was in my last posting which Andy had in his mail):
--- v4l-dvb-309f16461cf4-orig/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c
2009-12-05 13:34:21.0 +0100
+++ v4l-dvb-309f16461cf4-patched/linux/drivers/media/video/pvrusb2/pvrusb2-
hdw.c 2009-12-24 22:48:03.481899379 +0100
@@ -4646,9 +4646,9 @@
!hdw-state_pipeline_pause
hdw-state_pathway_ok) return 0;
}
- if (!hdw-flag_decoder_missed) {
- pvr2_decoder_enable(hdw,0);
- }
+if (hdw-decoder_client_id != PVR2_CLIENT_ID_CX25840 !hdw-
flag_decoder_missed) {
+ pvr2_decoder_enable(hdw,0);
+}
hdw-state_decoder_quiescent = 0;
hdw-state_decoder_run = 0;
/* paranoia - solve race if timer just completed */
@@ -4688,7 +4688,10 @@
!hdw-state_encoder_ok) return 0;
del_timer_sync(hdw-quiescent_timer);
if (hdw-flag_decoder_missed) return 0;
- if (pvr2_decoder_enable(hdw,!0) 0) return 0;
+if (hdw-decoder_client_id != PVR2_CLIENT_ID_CX25840) {
+ if (pvr2_decoder_enable(hdw,!0) 0) return 0;
+msleep(300);
+}
hdw-state_decoder_quiescent = 0;
hdw-state_decoder_run = !0;
}
It works fine with a HVR 1900, but should be tested with a PVRUSB2 model
24xxx.
Greets,
Martin
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
