linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Is xine DVB broken with Linux 3.3.1?
@ 2012-04-04 23:00 Chris Rankin
  2012-04-04 23:33 ` Chris Rankin
  2012-04-06 20:06 ` [REGRESSION] Linux 3.3 DVB userspace ABI broken for xine (FE_SET_FRONTEND) Chris Rankin
  0 siblings, 2 replies; 9+ messages in thread
From: Chris Rankin @ 2012-04-04 23:00 UTC (permalink / raw)
  To: linux-media@vger.kernel.org

 Hi,

Is anyone else having difficulty watching DVB-T with xine and a 3.3.1 kernel? I have tried the following with several different USB adapters:

1) Plug in the device
2) Launch xine.
3) Select DVB-T channel.


and each time, the results are the same. Namely, xine refuses to acknowledge any of my channels. However, if I execute scandvb before running xine then xine suddenly realises that the exact same channels.conf file is OK after all.

It looks as if scandvb is initialising a piece of the USB DVB device's internal "state", and xine cannot tune the adapter into any channel until it has.

Can anyone else reproduce this, please?
Thanks,
Chris


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Is xine DVB broken with Linux 3.3.1?
  2012-04-04 23:00 Is xine DVB broken with Linux 3.3.1? Chris Rankin
@ 2012-04-04 23:33 ` Chris Rankin
  2012-04-06 10:49   ` DVB ioctl FE_GET_EVENT behaviour broken in 3.3 Chris Rankin
  2012-04-06 20:06 ` [REGRESSION] Linux 3.3 DVB userspace ABI broken for xine (FE_SET_FRONTEND) Chris Rankin
  1 sibling, 1 reply; 9+ messages in thread
From: Chris Rankin @ 2012-04-04 23:33 UTC (permalink / raw)
  To: linux-media

I have just checked the 3.2.14 kernel, and it seems unaffected by this 
issue. So I'm guessing it was introduced recently in the 3.3 kernel.

Cheers,
Chris


^ permalink raw reply	[flat|nested] 9+ messages in thread

* DVB ioctl FE_GET_EVENT behaviour broken in 3.3
  2012-04-04 23:33 ` Chris Rankin
@ 2012-04-06 10:49   ` Chris Rankin
  2012-04-06 11:47     ` Antti Palosaari
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Rankin @ 2012-04-06 10:49 UTC (permalink / raw)
  To: linux-media

The reason that DVB playback with xine is broken in 3.3 is that the userspace 
semantics of FE_GET_EVENT have changed. Xine tunes into a DVB channel as follows:

* discards stale frontend events by calling FE_GET_EVENT until there is none left.
* calls FE_SET_FRONTEND with the new frequency.
* starts polling for new frontend events by calling FE_GET_EVENT again.

Xine assumes that *every* FE_GET_EVENT after calling FE_SET_FRONTEND will have 
dvb_frontend_event.parameters.frequency set to the new frequency, if this 
channel exists. However, under Linux 3.3, at least the first new event with a 
newly-plugged-in device has a frequency parameter of zero. I am assuming that 
Linux is populating the frequency parameter from an internal data structure 
because xine behaves normally once some other DVB application manages to set it 
to something other than zero. And xine then continues to behave normally until I 
unplug the DVB adapter and plug in back in again.

So the question is: why is there no frequency for this first FE_GET_EVENT? Are 
the parameters incomplete, or shouldn't this event have been sent in the first 
place?

Cheers,
Chris

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: DVB ioctl FE_GET_EVENT behaviour broken in 3.3
  2012-04-06 10:49   ` DVB ioctl FE_GET_EVENT behaviour broken in 3.3 Chris Rankin
@ 2012-04-06 11:47     ` Antti Palosaari
  2012-04-06 12:06       ` Chris Rankin
  2012-04-06 14:59       ` Chris Rankin
  0 siblings, 2 replies; 9+ messages in thread
From: Antti Palosaari @ 2012-04-06 11:47 UTC (permalink / raw)
  To: Chris Rankin; +Cc: linux-media

On 06.04.2012 13:49, Chris Rankin wrote:
> The reason that DVB playback with xine is broken in 3.3 is that the
> userspace semantics of FE_GET_EVENT have changed. Xine tunes into a DVB
> channel as follows:
>
> * discards stale frontend events by calling FE_GET_EVENT until there is
> none left.
> * calls FE_SET_FRONTEND with the new frequency.
> * starts polling for new frontend events by calling FE_GET_EVENT again.
>
> Xine assumes that *every* FE_GET_EVENT after calling FE_SET_FRONTEND
> will have dvb_frontend_event.parameters.frequency set to the new
> frequency, if this channel exists. However, under Linux 3.3, at least
> the first new event with a newly-plugged-in device has a frequency
> parameter of zero. I am assuming that Linux is populating the frequency
> parameter from an internal data structure because xine behaves normally
> once some other DVB application manages to set it to something other
> than zero. And xine then continues to behave normally until I unplug the
> DVB adapter and plug in back in again.
>
> So the question is: why is there no frequency for this first
> FE_GET_EVENT? Are the parameters incomplete, or shouldn't this event
> have been sent in the first place?

I have no enough experience, but IMHO all frontend parameters should be 
available just after demod is LOCKed. Before LOCK you cannot know many 
parameters at all and frequency also can be changed a little bit during 
tuning process (ZigZag tuning algo).

Could you try to git bisect to find out patch causing that regression?

I suspect it is some change done for DVB core, git log 
drivers/media/dvb/dvb-core/ shows rather many patches that could be the 
reason.

regards
Antti
-- 
http://palosaari.fi/

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: DVB ioctl FE_GET_EVENT behaviour broken in 3.3
  2012-04-06 11:47     ` Antti Palosaari
@ 2012-04-06 12:06       ` Chris Rankin
  2012-04-06 14:59       ` Chris Rankin
  1 sibling, 0 replies; 9+ messages in thread
From: Chris Rankin @ 2012-04-06 12:06 UTC (permalink / raw)
  To: crope; +Cc: linux-media

 > Before LOCK you cannot know many parameters at all and frequency also
 > can be changed a little bit during tuning process (ZigZag tuning algo).

But surely the point of calling poll() on the front end's descriptor is either 
to be notified once the tuning algorithm has locked, or to be told that LOCK has 
failed? This would certainly seem to have been xine's assumption for the past 
10+ years.

 > Could you try to git bisect to find out patch causing that regression?

I don't have a git tree to bisect with, so I expect I'll have to resort to more 
"old fashioned" methods.

 > I suspect it is some change done for DVB core

Me too, because this bug is affecting *every* DVB adapter that I own. (All USB, 
but anyway...)

Cheers,
Chris

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: DVB ioctl FE_GET_EVENT behaviour broken in 3.3
  2012-04-06 11:47     ` Antti Palosaari
  2012-04-06 12:06       ` Chris Rankin
@ 2012-04-06 14:59       ` Chris Rankin
  1 sibling, 0 replies; 9+ messages in thread
From: Chris Rankin @ 2012-04-06 14:59 UTC (permalink / raw)
  To: crope; +Cc: linux-media

The problem is that the following line was deleted from the FE_SET_FRONTEND 
ioctl logic:

         fepriv->parameters_out = fepriv->parameters_in;

The following dirty little patch restores the correct behaviour:

--- dvb_frontend.c.orig	2012-04-06 13:28:43.000000000 +0100
+++ dvb_frontend.c	2012-04-06 15:42:04.000000000 +0100
@@ -1877,6 +1877,8 @@
  	if (c->hierarchy == HIERARCHY_NONE && c->code_rate_LP == FEC_NONE)
  		c->code_rate_LP = FEC_AUTO;

+	fepriv->parameters_out.frequency = c->frequency;
+
  	/* get frontend-specific tuning settings */
  	memset(&fetunesettings, 0, sizeof(struct dvb_frontend_tune_settings));
  	if (fe->ops.get_tune_settings && (fe->ops.get_tune_settings(fe, 
&fetunesettings) == 0)) {

I'm hoping that someone out there who understands the new logic better than I 
can provide a better patch.

Cheers,
Chris

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [REGRESSION] Linux 3.3 DVB userspace ABI broken for xine (FE_SET_FRONTEND)
  2012-04-04 23:00 Is xine DVB broken with Linux 3.3.1? Chris Rankin
  2012-04-04 23:33 ` Chris Rankin
@ 2012-04-06 20:06 ` Chris Rankin
  2012-04-06 20:21   ` [PATCH] " Chris Rankin
  2012-04-06 22:38   ` [PATCH v2] " Chris Rankin
  1 sibling, 2 replies; 9+ messages in thread
From: Chris Rankin @ 2012-04-06 20:06 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab

Hi,

The following commit has broken the DVB ABI for xine:

http://git.linuxtv.org/linux-2.6.git/commitdiff/e399ce77e6e8f0ff2e0b8ef808cbb88fc824c610

author: Mauro Carvalho Chehab <mchehab@redhat.com>
  Sun, 1 Jan 2012 19:11:16 +0000 (16:11 -0300)
committer: Mauro Carvalho Chehab <mchehab@redhat.com>
  Wed, 4 Jan 2012 19:30:02 +0000 (17:30 -0200)

This var were used during DVBv3 times, in order to keep a copy
of the parameters used by the events. This is not needed anymore,
as the parameters are now dynamically generated from the DVBv5
structure.

So, just get rid of it. That means that a DVBv5 pure call won't
use anymore any DVBv3 parameters.


The problem is that xine is expecting every event after a successful 
FE_SET_FRONTEND ioctl to have a non-zero frequency parameter, regardless of 
whether the tuning process has LOCKed yet. What used to happen is that the 
events inherited the initial tuning parameters from the FE_SET_FRONTEND call. 
However, the fepriv->parameters_out struct is now not initialised until the 
status contains the FE_HAS_LOCK bit.

You might argue that this behaviour is intentional, except that if an 
application other than xine uses the DVB adapter and manages to set the 
parameters_out.frequency field to something other than zero, then xine no longer 
has any problems until either the adapter is replugged or the kernel modules 
reloaded. This can only mean that the fepriv->parameters_out struct still 
contains the (stale) tuning information from the previous application.

So can we please restore the original ABI behaviour, and have 
fepriv->parameters_out initialised by FE_SET_FRONTEND again?

Cheers,
Chris

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] Linux 3.3 DVB userspace ABI broken for xine (FE_SET_FRONTEND)
  2012-04-06 20:06 ` [REGRESSION] Linux 3.3 DVB userspace ABI broken for xine (FE_SET_FRONTEND) Chris Rankin
@ 2012-04-06 20:21   ` Chris Rankin
  2012-04-06 22:38   ` [PATCH v2] " Chris Rankin
  1 sibling, 0 replies; 9+ messages in thread
From: Chris Rankin @ 2012-04-06 20:21 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media

[-- Attachment #1: Type: text/plain, Size: 94 bytes --]

In fact, the following patch works for me.

Signed-off-by: Chris Rankin <rankincj@yahoo.com>


[-- Attachment #2: DVB.diff --]
[-- Type: text/x-patch, Size: 678 bytes --]

--- linux-3.3/drivers/media/dvb/dvb-core/dvb_frontend.c.orig	2012-04-06 20:16:02.000000000 +0100
+++ linux-3.3/drivers/media/dvb/dvb-core/dvb_frontend.c	2012-04-06 21:17:38.000000000 +0100
@@ -1831,6 +1831,13 @@
 		return -EINVAL;
 
 	/*
+	 * Initialize output parameters to match the values given by
+	 * the user. FE_SET_FRONTEND triggers an initial frontend event
+	 * with status = 0, which copies output parameters to userspace.
+	 */
+	dtv_property_legacy_params_sync(fe, &fepriv->parameters_out);
+
+	/*
 	 * Be sure that the bandwidth will be filled for all
 	 * non-satellite systems, as tuners need to know what
 	 * low pass/Nyquist half filter should be applied, in

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] Linux 3.3 DVB userspace ABI broken for xine (FE_SET_FRONTEND)
  2012-04-06 20:06 ` [REGRESSION] Linux 3.3 DVB userspace ABI broken for xine (FE_SET_FRONTEND) Chris Rankin
  2012-04-06 20:21   ` [PATCH] " Chris Rankin
@ 2012-04-06 22:38   ` Chris Rankin
  1 sibling, 0 replies; 9+ messages in thread
From: Chris Rankin @ 2012-04-06 22:38 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media

[-- Attachment #1: Type: text/plain, Size: 278 bytes --]

I've had a closer look at the commit which caused the regression and it looks 
like there were two places where fepriv->parameters_in was assigned to 
fepriv->parameters_out. So I've updated my patch accordingly.

Cheers,
Chris

Signed-off-by: Chris Rankin <rankincj@yahoo.com>

[-- Attachment #2: DVB.diff --]
[-- Type: text/x-patch, Size: 1338 bytes --]

--- linux-3.3/drivers/media/dvb/dvb-core/dvb_frontend.c.orig	2012-04-06 20:16:02.000000000 +0100
+++ linux-3.3/drivers/media/dvb/dvb-core/dvb_frontend.c	2012-04-06 23:16:03.000000000 +0100
@@ -143,6 +143,8 @@
 static void dvb_frontend_wakeup(struct dvb_frontend *fe);
 static int dtv_get_frontend(struct dvb_frontend *fe,
 			    struct dvb_frontend_parameters *p_out);
+static int dtv_property_legacy_params_sync(struct dvb_frontend *fe,
+					   struct dvb_frontend_parameters *p);
 
 static bool has_get_frontend(struct dvb_frontend *fe)
 {
@@ -695,6 +697,7 @@
 					fepriv->algo_status |= DVBFE_ALGO_SEARCH_AGAIN;
 					fepriv->delay = HZ / 2;
 				}
+				dtv_property_legacy_params_sync(fe, &fepriv->parameters_out);
 				fe->ops.read_status(fe, &s);
 				if (s != fepriv->status) {
 					dvb_frontend_add_event(fe, s); /* update event list */
@@ -1831,6 +1834,13 @@
 		return -EINVAL;
 
 	/*
+	 * Initialize output parameters to match the values given by
+	 * the user. FE_SET_FRONTEND triggers an initial frontend event
+	 * with status = 0, which copies output parameters to userspace.
+	 */
+	dtv_property_legacy_params_sync(fe, &fepriv->parameters_out);
+
+	/*
 	 * Be sure that the bandwidth will be filled for all
 	 * non-satellite systems, as tuners need to know what
 	 * low pass/Nyquist half filter should be applied, in

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-04-06 22:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-04 23:00 Is xine DVB broken with Linux 3.3.1? Chris Rankin
2012-04-04 23:33 ` Chris Rankin
2012-04-06 10:49   ` DVB ioctl FE_GET_EVENT behaviour broken in 3.3 Chris Rankin
2012-04-06 11:47     ` Antti Palosaari
2012-04-06 12:06       ` Chris Rankin
2012-04-06 14:59       ` Chris Rankin
2012-04-06 20:06 ` [REGRESSION] Linux 3.3 DVB userspace ABI broken for xine (FE_SET_FRONTEND) Chris Rankin
2012-04-06 20:21   ` [PATCH] " Chris Rankin
2012-04-06 22:38   ` [PATCH v2] " Chris Rankin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).