PATCH: Query DVB frontend capabilities

PATCH: Query DVB frontend capabilities

[Manu Abraham]

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

Hi,

Currently, for a multi standard frontend it is assumed that it just
has a single standard capability. This is fine in some cases, but
makes things hard when there are incompatible standards in conjuction.
Eg: DVB-S can be seen as a subset of DVB-S2, but the same doesn't hold
the same for DSS. This is not specific to any driver as it is, but a
generic issue. This was handled correctly in the multiproto tree,
while such functionality is missing from the v5 API update.

http://www.linuxtv.org/pipermail/vdr/2008-November/018417.html

Later on a FE_CAN_2G_MODULATION was added as a hack to workaround this
issue in the v5 API, but that hack is incapable of addressing the
issue, as it can be used to simply distinguish between DVB-S and
DVB-S2 alone, or another x vs X2 modulation. If there are more
systems, then you have a potential issue.

In addition to the patch, for illustrative purposes the stb0899 driver
is depicted providing the said capability information.

An application needs to query the device capabilities before
requesting an operation from the device.

If people don't have any objections, Probably other drivers can be
adapted similarly. In fact the change is quite simple.

Comments ?

Regards,
Manu

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

diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.c
--- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Wed Nov 09 19:52:36 2011 +0530
+++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Thu Nov 10 13:51:35 2011 +0530
@@ -973,6 +973,8 @@
 	_DTV_CMD(DTV_GUARD_INTERVAL, 0, 0),
 	_DTV_CMD(DTV_TRANSMISSION_MODE, 0, 0),
 	_DTV_CMD(DTV_HIERARCHY, 0, 0),
+
+	_DTV_CMD(DTV_DELIVERY_CAPS, 0, 0),
 };
 
 static void dtv_property_dump(struct dtv_property *tvp)
@@ -1226,7 +1228,18 @@
 		c = &cdetected;
 	}
 
+	dprintk("%s\n", __func__);
+
 	switch(tvp->cmd) {
+	case DTV_DELIVERY_CAPS:
+		if (fe->ops.delivery_caps) {
+			r = fe->ops.delivery_caps(fe, tvp);
+			if (r < 0)
+				return r;
+			else
+				goto done;
+		}
+		break;
 	case DTV_FREQUENCY:
 		tvp->u.data = c->frequency;
 		break;
@@ -1350,7 +1363,7 @@
 		if (r < 0)
 			return r;
 	}
-
+done:
 	dtv_property_dump(tvp);
 
 	return 0;
diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.h
--- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.h	Wed Nov 09 19:52:36 2011 +0530
+++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.h	Thu Nov 10 13:51:35 2011 +0530
@@ -305,6 +305,8 @@
 
 	int (*set_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
 	int (*get_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
+
+	int (*delivery_caps)(struct dvb_frontend *fe, struct dtv_property *tvp);
 };
 
 #define MAX_EVENT 8
@@ -362,6 +364,8 @@
 
 	/* DVB-T2 specifics */
 	u32                     dvbt2_plp_id;
+
+	fe_delivery_system_t	delivery_caps[32];
 };
 
 struct dvb_frontend {
diff -r b6eb04718aa9 linux/drivers/media/dvb/frontends/stb0899_drv.c
--- a/linux/drivers/media/dvb/frontends/stb0899_drv.c	Wed Nov 09 19:52:36 2011 +0530
+++ b/linux/drivers/media/dvb/frontends/stb0899_drv.c	Thu Nov 10 13:51:35 2011 +0530
@@ -1605,6 +1605,19 @@
 	return DVBFE_ALGO_CUSTOM;
 }
 
+static int stb0899_delivery_caps(struct dvb_frontend *fe, struct dtv_property *caps)
+{
+	struct stb0899_state *state		= fe->demodulator_priv;
+
+	dprintk(state->verbose, FE_DEBUG, 1, "Get caps");
+	caps->u.buffer.data[0] = SYS_DSS;
+	caps->u.buffer.data[1] = SYS_DVBS;
+	caps->u.buffer.data[2] = SYS_DVBS2;
+	caps->u.buffer.len = 3;
+
+	return 0;
+}
+
 static struct dvb_frontend_ops stb0899_ops = {
 
 	.info = {
@@ -1647,6 +1660,8 @@
 	.diseqc_send_master_cmd		= stb0899_send_diseqc_msg,
 	.diseqc_recv_slave_reply	= stb0899_recv_slave_reply,
 	.diseqc_send_burst		= stb0899_send_diseqc_burst,
+
+	.delivery_caps			= stb0899_delivery_caps,
 };
 
 struct dvb_frontend *stb0899_attach(struct stb0899_config *config, struct i2c_adapter *i2c)
diff -r b6eb04718aa9 linux/include/linux/dvb/frontend.h
--- a/linux/include/linux/dvb/frontend.h	Wed Nov 09 19:52:36 2011 +0530
+++ b/linux/include/linux/dvb/frontend.h	Thu Nov 10 13:51:35 2011 +0530
@@ -316,7 +316,9 @@
 
 #define DTV_DVBT2_PLP_ID	43
 
-#define DTV_MAX_COMMAND				DTV_DVBT2_PLP_ID
+#define DTV_DELIVERY_CAPS	44
+
+#define DTV_MAX_COMMAND				DTV_DELIVERY_CAPS
 
 typedef enum fe_pilot {
 	PILOT_ON,

Re: PATCH: Query DVB frontend capabilities

[Andreas Oberritter]

Hello Manu,

please see my comments below:

On 10.11.2011 15:18, Manu Abraham wrote:
> Hi,
> 
> Currently, for a multi standard frontend it is assumed that it just
> has a single standard capability. This is fine in some cases, but
> makes things hard when there are incompatible standards in conjuction.
> Eg: DVB-S can be seen as a subset of DVB-S2, but the same doesn't hold
> the same for DSS. This is not specific to any driver as it is, but a
> generic issue. This was handled correctly in the multiproto tree,
> while such functionality is missing from the v5 API update.
> 
> http://www.linuxtv.org/pipermail/vdr/2008-November/018417.html
> 
> Later on a FE_CAN_2G_MODULATION was added as a hack to workaround this
> issue in the v5 API, but that hack is incapable of addressing the
> issue, as it can be used to simply distinguish between DVB-S and
> DVB-S2 alone, or another x vs X2 modulation. If there are more
> systems, then you have a potential issue.
> 
> In addition to the patch, for illustrative purposes the stb0899 driver
> is depicted providing the said capability information.
> 
> An application needs to query the device capabilities before
> requesting an operation from the device.
> 
> If people don't have any objections, Probably other drivers can be
> adapted similarly. In fact the change is quite simple.
> 
> Comments ?
> 
> Regards,
> Manu
> 
> 
> query_frontend_capabilities.diff
> 
> 
> diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.c
> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Wed Nov 09 19:52:36 2011 +0530
> +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Thu Nov 10 13:51:35 2011 +0530
> @@ -973,6 +973,8 @@
>  	_DTV_CMD(DTV_GUARD_INTERVAL, 0, 0),
>  	_DTV_CMD(DTV_TRANSMISSION_MODE, 0, 0),
>  	_DTV_CMD(DTV_HIERARCHY, 0, 0),
> +
> +	_DTV_CMD(DTV_DELIVERY_CAPS, 0, 0),
>  };
>  
>  static void dtv_property_dump(struct dtv_property *tvp)
> @@ -1226,7 +1228,18 @@
>  		c = &cdetected;
>  	}
>  
> +	dprintk("%s\n", __func__);
> +
>  	switch(tvp->cmd) {
> +	case DTV_DELIVERY_CAPS:
> +		if (fe->ops.delivery_caps) {
> +			r = fe->ops.delivery_caps(fe, tvp);
> +			if (r < 0)
> +				return r;
> +			else
> +				goto done;

What's the reason for introducing that label and goto?

> +		}
> +		break;
>  	case DTV_FREQUENCY:
>  		tvp->u.data = c->frequency;
>  		break;
> @@ -1350,7 +1363,7 @@
>  		if (r < 0)
>  			return r;
>  	}
> -
> +done:
>  	dtv_property_dump(tvp);
>  
>  	return 0;
> diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.h
> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.h	Wed Nov 09 19:52:36 2011 +0530
> +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.h	Thu Nov 10 13:51:35 2011 +0530
> @@ -305,6 +305,8 @@
>  
>  	int (*set_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
>  	int (*get_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
> +
> +	int (*delivery_caps)(struct dvb_frontend *fe, struct dtv_property *tvp);
>  };

I don't think that another function pointer is required. Drivers can
implement this in their get_property callback. The core could provide a
default implementation, returning values derived from fe->ops.info.type
and the 2G flag.

>  #define MAX_EVENT 8
> @@ -362,6 +364,8 @@
>  
>  	/* DVB-T2 specifics */
>  	u32                     dvbt2_plp_id;
> +
> +	fe_delivery_system_t	delivery_caps[32];
>  };

This array seems to be unused.

Regards,
Andreas

>  struct dvb_frontend {
> diff -r b6eb04718aa9 linux/drivers/media/dvb/frontends/stb0899_drv.c
> --- a/linux/drivers/media/dvb/frontends/stb0899_drv.c	Wed Nov 09 19:52:36 2011 +0530
> +++ b/linux/drivers/media/dvb/frontends/stb0899_drv.c	Thu Nov 10 13:51:35 2011 +0530
> @@ -1605,6 +1605,19 @@
>  	return DVBFE_ALGO_CUSTOM;
>  }
>  
> +static int stb0899_delivery_caps(struct dvb_frontend *fe, struct dtv_property *caps)
> +{
> +	struct stb0899_state *state		= fe->demodulator_priv;
> +
> +	dprintk(state->verbose, FE_DEBUG, 1, "Get caps");
> +	caps->u.buffer.data[0] = SYS_DSS;
> +	caps->u.buffer.data[1] = SYS_DVBS;
> +	caps->u.buffer.data[2] = SYS_DVBS2;
> +	caps->u.buffer.len = 3;
> +
> +	return 0;
> +}
> +
>  static struct dvb_frontend_ops stb0899_ops = {
>  
>  	.info = {
> @@ -1647,6 +1660,8 @@
>  	.diseqc_send_master_cmd		= stb0899_send_diseqc_msg,
>  	.diseqc_recv_slave_reply	= stb0899_recv_slave_reply,
>  	.diseqc_send_burst		= stb0899_send_diseqc_burst,
> +
> +	.delivery_caps			= stb0899_delivery_caps,
>  };
>  
>  struct dvb_frontend *stb0899_attach(struct stb0899_config *config, struct i2c_adapter *i2c)
> diff -r b6eb04718aa9 linux/include/linux/dvb/frontend.h
> --- a/linux/include/linux/dvb/frontend.h	Wed Nov 09 19:52:36 2011 +0530
> +++ b/linux/include/linux/dvb/frontend.h	Thu Nov 10 13:51:35 2011 +0530
> @@ -316,7 +316,9 @@
>  
>  #define DTV_DVBT2_PLP_ID	43
>  
> -#define DTV_MAX_COMMAND				DTV_DVBT2_PLP_ID
> +#define DTV_DELIVERY_CAPS	44
> +
> +#define DTV_MAX_COMMAND				DTV_DELIVERY_CAPS
>  
>  typedef enum fe_pilot {
>  	PILOT_ON,
> 

Re: PATCH: Query DVB frontend capabilities

[Manu Abraham]

Hi Andreas,

On Thu, Nov 10, 2011 at 8:14 PM, Andreas Oberritter <obi@linuxtv.org> wrote:
> Hello Manu,
>
> please see my comments below:
>
> On 10.11.2011 15:18, Manu Abraham wrote:
>> Hi,
>>
>> Currently, for a multi standard frontend it is assumed that it just
>> has a single standard capability. This is fine in some cases, but
>> makes things hard when there are incompatible standards in conjuction.
>> Eg: DVB-S can be seen as a subset of DVB-S2, but the same doesn't hold
>> the same for DSS. This is not specific to any driver as it is, but a
>> generic issue. This was handled correctly in the multiproto tree,
>> while such functionality is missing from the v5 API update.
>>
>> http://www.linuxtv.org/pipermail/vdr/2008-November/018417.html
>>
>> Later on a FE_CAN_2G_MODULATION was added as a hack to workaround this
>> issue in the v5 API, but that hack is incapable of addressing the
>> issue, as it can be used to simply distinguish between DVB-S and
>> DVB-S2 alone, or another x vs X2 modulation. If there are more
>> systems, then you have a potential issue.
>>
>> In addition to the patch, for illustrative purposes the stb0899 driver
>> is depicted providing the said capability information.
>>
>> An application needs to query the device capabilities before
>> requesting an operation from the device.
>>
>> If people don't have any objections, Probably other drivers can be
>> adapted similarly. In fact the change is quite simple.
>>
>> Comments ?
>>
>> Regards,
>> Manu
>>
>>
>> query_frontend_capabilities.diff
>>
>>
>> diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.c
>> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c Wed Nov 09 19:52:36 2011 +0530
>> +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c Thu Nov 10 13:51:35 2011 +0530
>> @@ -973,6 +973,8 @@
>>       _DTV_CMD(DTV_GUARD_INTERVAL, 0, 0),
>>       _DTV_CMD(DTV_TRANSMISSION_MODE, 0, 0),
>>       _DTV_CMD(DTV_HIERARCHY, 0, 0),
>> +
>> +     _DTV_CMD(DTV_DELIVERY_CAPS, 0, 0),
>>  };
>>
>>  static void dtv_property_dump(struct dtv_property *tvp)
>> @@ -1226,7 +1228,18 @@
>>               c = &cdetected;
>>       }
>>
>> +     dprintk("%s\n", __func__);
>> +
>>       switch(tvp->cmd) {
>> +     case DTV_DELIVERY_CAPS:
>> +             if (fe->ops.delivery_caps) {
>> +                     r = fe->ops.delivery_caps(fe, tvp);
>> +                     if (r < 0)
>> +                             return r;
>> +                     else
>> +                             goto done;
>
> What's the reason for introducing that label and goto?


The idea was to avoid using get_property callback being called.



>
>> +             }
>> +             break;
>>       case DTV_FREQUENCY:
>>               tvp->u.data = c->frequency;
>>               break;
>> @@ -1350,7 +1363,7 @@
>>               if (r < 0)
>>                       return r;
>>       }
>> -
>> +done:
>>       dtv_property_dump(tvp);
>>
>>       return 0;
>> diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.h
>> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.h Wed Nov 09 19:52:36 2011 +0530
>> +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.h Thu Nov 10 13:51:35 2011 +0530
>> @@ -305,6 +305,8 @@
>>
>>       int (*set_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
>>       int (*get_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
>> +
>> +     int (*delivery_caps)(struct dvb_frontend *fe, struct dtv_property *tvp);
>>  };
>
> I don't think that another function pointer is required. Drivers can
> implement this in their get_property callback. The core could provide a
> default implementation, returning values derived from fe->ops.info.type
> and the 2G flag.


I see your point. While trying to address the issue I had originally
get_property being used. One Issue that came across to me was that:

 - while currently this is just one capability field, things do look
fine. As we move ahead we will possibly have more capability related
information, given the idea that with shared hardware infrastructure
on frontends, this list of properties could grow.

- Once we have a few properties, then we will need to have switches in
that callback, given that it is generic and that length will grow.
Considering different hardware capabilities, we will have quite a bit
of shared code amongst drivers in that large switch, causing code
duplication.
Eg: with hardware having shared infrastructure - while globally it
might have a larger set of capabilities, when a sub part of it is run
in some mode, the other sub parts would have reduced set of
capabilities, which won't be the same as that exists globally for the
device. The driver alone will know the change depending on what's
being used.


- Additionally, though insignificant a separate callback for each
capability will make the drivers look a bit more consistent with
regards to code style.

The basic issue is a switch that will be duplicated across drivers and
that which is expected to grow in length, eventually readability a
question. That said, this shouldn't be an issue at all. Given the
situation at this exact moment, having a generic callback is just
fine, as that will be the only which would be there, right now.


>
>>  #define MAX_EVENT 8
>> @@ -362,6 +364,8 @@
>>
>>       /* DVB-T2 specifics */
>>       u32                     dvbt2_plp_id;
>> +
>> +     fe_delivery_system_t    delivery_caps[32];
>>  };
>
> This array seems to be unused.


That just walked in astray while I worked on it. Didn't notice at all. :-)
I will send an updated patch, do you still think the generic callback
is better ?



>
> Regards,
> Andreas
>
>>  struct dvb_frontend {
>> diff -r b6eb04718aa9 linux/drivers/media/dvb/frontends/stb0899_drv.c
>> --- a/linux/drivers/media/dvb/frontends/stb0899_drv.c Wed Nov 09 19:52:36 2011 +0530
>> +++ b/linux/drivers/media/dvb/frontends/stb0899_drv.c Thu Nov 10 13:51:35 2011 +0530
>> @@ -1605,6 +1605,19 @@
>>       return DVBFE_ALGO_CUSTOM;
>>  }
>>
>> +static int stb0899_delivery_caps(struct dvb_frontend *fe, struct dtv_property *caps)
>> +{
>> +     struct stb0899_state *state             = fe->demodulator_priv;
>> +
>> +     dprintk(state->verbose, FE_DEBUG, 1, "Get caps");
>> +     caps->u.buffer.data[0] = SYS_DSS;
>> +     caps->u.buffer.data[1] = SYS_DVBS;
>> +     caps->u.buffer.data[2] = SYS_DVBS2;
>> +     caps->u.buffer.len = 3;
>> +
>> +     return 0;
>> +}
>> +
>>  static struct dvb_frontend_ops stb0899_ops = {
>>
>>       .info = {
>> @@ -1647,6 +1660,8 @@
>>       .diseqc_send_master_cmd         = stb0899_send_diseqc_msg,
>>       .diseqc_recv_slave_reply        = stb0899_recv_slave_reply,
>>       .diseqc_send_burst              = stb0899_send_diseqc_burst,
>> +
>> +     .delivery_caps                  = stb0899_delivery_caps,
>>  };
>>
>>  struct dvb_frontend *stb0899_attach(struct stb0899_config *config, struct i2c_adapter *i2c)
>> diff -r b6eb04718aa9 linux/include/linux/dvb/frontend.h
>> --- a/linux/include/linux/dvb/frontend.h      Wed Nov 09 19:52:36 2011 +0530
>> +++ b/linux/include/linux/dvb/frontend.h      Thu Nov 10 13:51:35 2011 +0530
>> @@ -316,7 +316,9 @@
>>
>>  #define DTV_DVBT2_PLP_ID     43
>>
>> -#define DTV_MAX_COMMAND                              DTV_DVBT2_PLP_ID
>> +#define DTV_DELIVERY_CAPS    44
>> +
>> +#define DTV_MAX_COMMAND                              DTV_DELIVERY_CAPS
>>
>>  typedef enum fe_pilot {
>>       PILOT_ON,
>>
>
>

Thanks,
Manu

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 10-11-2011 13:30, Manu Abraham escreveu:
> Hi Andreas,
> 
> On Thu, Nov 10, 2011 at 8:14 PM, Andreas Oberritter <obi@linuxtv.org> wrote:
>> Hello Manu,
>>
>> please see my comments below:
>>
>> On 10.11.2011 15:18, Manu Abraham wrote:
>>> Hi,
>>>
>>> Currently, for a multi standard frontend it is assumed that it just
>>> has a single standard capability. This is fine in some cases, but
>>> makes things hard when there are incompatible standards in conjuction.
>>> Eg: DVB-S can be seen as a subset of DVB-S2, but the same doesn't hold
>>> the same for DSS. This is not specific to any driver as it is, but a
>>> generic issue. This was handled correctly in the multiproto tree,
>>> while such functionality is missing from the v5 API update.
>>>
>>> http://www.linuxtv.org/pipermail/vdr/2008-November/018417.html
>>>
>>> Later on a FE_CAN_2G_MODULATION was added as a hack to workaround this
>>> issue in the v5 API, but that hack is incapable of addressing the
>>> issue, as it can be used to simply distinguish between DVB-S and
>>> DVB-S2 alone, or another x vs X2 modulation. If there are more
>>> systems, then you have a potential issue.

For sure something like that is needed.

DVB TURBO is another example where the FE_CAN_2G_MODULATION approach doesn't
properly address.

>>>
>>> In addition to the patch, for illustrative purposes the stb0899 driver
>>> is depicted providing the said capability information.
>>>
>>> An application needs to query the device capabilities before
>>> requesting an operation from the device.
>>>
>>> If people don't have any objections, Probably other drivers can be
>>> adapted similarly. In fact the change is quite simple.
>>>
>>> Comments ?
>>>
>>> Regards,
>>> Manu
>>>
>>>
>>> query_frontend_capabilities.diff
>>>
>>>
>>> diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.c
>>> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c Wed Nov 09 19:52:36 2011 +0530
>>> +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c Thu Nov 10 13:51:35 2011 +0530
>>> @@ -973,6 +973,8 @@
>>>       _DTV_CMD(DTV_GUARD_INTERVAL, 0, 0),
>>>       _DTV_CMD(DTV_TRANSMISSION_MODE, 0, 0),
>>>       _DTV_CMD(DTV_HIERARCHY, 0, 0),
>>> +
>>> +     _DTV_CMD(DTV_DELIVERY_CAPS, 0, 0),
>>>  };
>>>
>>>  static void dtv_property_dump(struct dtv_property *tvp)
>>> @@ -1226,7 +1228,18 @@
>>>               c = &cdetected;
>>>       }
>>>
>>> +     dprintk("%s\n", __func__);
>>> +
>>>       switch(tvp->cmd) {
>>> +     case DTV_DELIVERY_CAPS:
>>> +             if (fe->ops.delivery_caps) {
>>> +                     r = fe->ops.delivery_caps(fe, tvp);
>>> +                     if (r < 0)
>>> +                             return r;
>>> +                     else
>>> +                             goto done;
>>
>> What's the reason for introducing that label and goto?
> 
> 
> The idea was to avoid using get_property callback being called.

Nothing warrants that the only property will be DTV_DELIVERY_CAPS.
calling get_property callback will likely work better.
> 
> 
> 
>>
>>> +             }
>>> +             break;
>>>       case DTV_FREQUENCY:
>>>               tvp->u.data = c->frequency;
>>>               break;
>>> @@ -1350,7 +1363,7 @@
>>>               if (r < 0)
>>>                       return r;
>>>       }
>>> -
>>> +done:
>>>       dtv_property_dump(tvp);
>>>
>>>       return 0;
>>> diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.h
>>> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.h Wed Nov 09 19:52:36 2011 +0530
>>> +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.h Thu Nov 10 13:51:35 2011 +0530
>>> @@ -305,6 +305,8 @@
>>>
>>>       int (*set_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
>>>       int (*get_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
>>> +
>>> +     int (*delivery_caps)(struct dvb_frontend *fe, struct dtv_property *tvp);
>>>  };
>>
>> I don't think that another function pointer is required. Drivers can
>> implement this in their get_property callback. The core could provide a
>> default implementation, returning values derived from fe->ops.info.type
>> and the 2G flag.
> 
> 
> I see your point. While trying to address the issue I had originally
> get_property being used. One Issue that came across to me was that:
> 
>  - while currently this is just one capability field, things do look
> fine. As we move ahead we will possibly have more capability related
> information, given the idea that with shared hardware infrastructure
> on frontends, this list of properties could grow.

There are two DVBv5 calls that are likely designed to address this
need:
	DTV_FE_CAPABILITY_COUNT
	DTV_FE_CAPABILITY

Unfortunately, those were never implemented. I _suspect_ that the original
idea were to allow passing an arbritary count to the driver, and let it fill
each capability.

> - Once we have a few properties, then we will need to have switches in
> that callback, given that it is generic and that length will grow.
> Considering different hardware capabilities, we will have quite a bit
> of shared code amongst drivers in that large switch, causing code
> duplication.
> Eg: with hardware having shared infrastructure - while globally it
> might have a larger set of capabilities, when a sub part of it is run
> in some mode, the other sub parts would have reduced set of
> capabilities, which won't be the same as that exists globally for the
> device. The driver alone will know the change depending on what's
> being used.
> 
> 
> - Additionally, though insignificant a separate callback for each
> capability will make the drivers look a bit more consistent with
> regards to code style.
> 
> The basic issue is a switch that will be duplicated across drivers and
> that which is expected to grow in length, eventually readability a
> question. That said, this shouldn't be an issue at all. Given the
> situation at this exact moment, having a generic callback is just
> fine, as that will be the only which would be there, right now.
> 
> 
>>
>>>  #define MAX_EVENT 8
>>> @@ -362,6 +364,8 @@
>>>
>>>       /* DVB-T2 specifics */
>>>       u32                     dvbt2_plp_id;
>>> +
>>> +     fe_delivery_system_t    delivery_caps[32];
>>>  };
>>
>> This array seems to be unused.
> 
> 
> That just walked in astray while I worked on it. Didn't notice at all. :-)
> I will send an updated patch, do you still think the generic callback
> is better ?
> 
> 
> 
>>
>> Regards,
>> Andreas
>>
>>>  struct dvb_frontend {
>>> diff -r b6eb04718aa9 linux/drivers/media/dvb/frontends/stb0899_drv.c
>>> --- a/linux/drivers/media/dvb/frontends/stb0899_drv.c Wed Nov 09 19:52:36 2011 +0530
>>> +++ b/linux/drivers/media/dvb/frontends/stb0899_drv.c Thu Nov 10 13:51:35 2011 +0530
>>> @@ -1605,6 +1605,19 @@
>>>       return DVBFE_ALGO_CUSTOM;
>>>  }
>>>
>>> +static int stb0899_delivery_caps(struct dvb_frontend *fe, struct dtv_property *caps)
>>> +{
>>> +     struct stb0899_state *state             = fe->demodulator_priv;
>>> +
>>> +     dprintk(state->verbose, FE_DEBUG, 1, "Get caps");
>>> +     caps->u.buffer.data[0] = SYS_DSS;
>>> +     caps->u.buffer.data[1] = SYS_DVBS;
>>> +     caps->u.buffer.data[2] = SYS_DVBS2;
>>> +     caps->u.buffer.len = 3;

This doesn't sound right to me. If userspace passed space for only one delivery system,
you can't just adjust the size here and expect it to work, as the array on userspace
may be smaller than 3 properties.

Also, the other DVBv5 properties may not be DTV_DELIVERY_CAPS type.

In other words, the driver or core needs to check if the type is DTV_DELIVERY_CAPS
before filling it.

Btw, if the "capabilities" here is just the delivery system, it would be better to
name it as something like DTV_GET_SUPPORTED_DELIVERY.

We should also think on a way to enumerate the supported values for each DVB properties,
the enum fe_caps is not enough anymore to store everything. It currently has 30 bits
filled (of a total of 32 bits), and we currently have:
	12 code rates (including AUTO/NONE);
	13 modulation types;
	7 transmission modes;
	7 bandwidths;
	8 guard intervals (including AUTO);
	5 hierarchy names;
	4 rolloff's (probably, we'll need to add 2 more, to distinguish between DVB-C Annex A and Annex C).

So, if we would need to add one CAN_foo for each of the above, we would need 56 to 58
bits, plus 5-6 bits to the other capabilities that currently exists there. So, even
64 bits won't be enough for the current needs (even having the delivery system caps
addressed by something else).

It means that FE_GET_INFO needs to be replaced by something that scales better, e. g.
that would allow to enumerate the supported ranges for each DVBv5 parameter.


>>> +
>>> +     return 0;
>>> +}
>>> +
>>>  static struct dvb_frontend_ops stb0899_ops = {
>>>
>>>       .info = {
>>> @@ -1647,6 +1660,8 @@
>>>       .diseqc_send_master_cmd         = stb0899_send_diseqc_msg,
>>>       .diseqc_recv_slave_reply        = stb0899_recv_slave_reply,
>>>       .diseqc_send_burst              = stb0899_send_diseqc_burst,
>>> +
>>> +     .delivery_caps                  = stb0899_delivery_caps,
>>>  };
>>>
>>>  struct dvb_frontend *stb0899_attach(struct stb0899_config *config, struct i2c_adapter *i2c)
>>> diff -r b6eb04718aa9 linux/include/linux/dvb/frontend.h
>>> --- a/linux/include/linux/dvb/frontend.h      Wed Nov 09 19:52:36 2011 +0530
>>> +++ b/linux/include/linux/dvb/frontend.h      Thu Nov 10 13:51:35 2011 +0530
>>> @@ -316,7 +316,9 @@
>>>
>>>  #define DTV_DVBT2_PLP_ID     43
>>>
>>> -#define DTV_MAX_COMMAND                              DTV_DVBT2_PLP_ID
>>> +#define DTV_DELIVERY_CAPS    44
>>> +
>>> +#define DTV_MAX_COMMAND                              DTV_DELIVERY_CAPS
>>>
>>>  typedef enum fe_pilot {
>>>       PILOT_ON,
>>>
>>
>>
> 
> Thanks,
> Manu

Re: PATCH: Query DVB frontend capabilities

[Manu Abraham]

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

On Fri, Nov 11, 2011 at 2:50 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 10-11-2011 13:30, Manu Abraham escreveu:
>> Hi Andreas,
>>
>> On Thu, Nov 10, 2011 at 8:14 PM, Andreas Oberritter <obi@linuxtv.org> wrote:
>>> Hello Manu,
>>>
>>> please see my comments below:
>>>
>>> On 10.11.2011 15:18, Manu Abraham wrote:
>>>> Hi,
>>>>
>>>> Currently, for a multi standard frontend it is assumed that it just
>>>> has a single standard capability. This is fine in some cases, but
>>>> makes things hard when there are incompatible standards in conjuction.
>>>> Eg: DVB-S can be seen as a subset of DVB-S2, but the same doesn't hold
>>>> the same for DSS. This is not specific to any driver as it is, but a
>>>> generic issue. This was handled correctly in the multiproto tree,
>>>> while such functionality is missing from the v5 API update.
>>>>
>>>> http://www.linuxtv.org/pipermail/vdr/2008-November/018417.html
>>>>
>>>> Later on a FE_CAN_2G_MODULATION was added as a hack to workaround this
>>>> issue in the v5 API, but that hack is incapable of addressing the
>>>> issue, as it can be used to simply distinguish between DVB-S and
>>>> DVB-S2 alone, or another x vs X2 modulation. If there are more
>>>> systems, then you have a potential issue.
>
> For sure something like that is needed.
>
> DVB TURBO is another example where the FE_CAN_2G_MODULATION approach doesn't
> properly address.
>


It is similar to a DVB-S system with a different modulation and FEC.
It was specifically designed to make it proprietary. Nothing wrong
with it as it is now.


>>>>
>>>> In addition to the patch, for illustrative purposes the stb0899 driver
>>>> is depicted providing the said capability information.
>>>>
>>>> An application needs to query the device capabilities before
>>>> requesting an operation from the device.
>>>>
>>>> If people don't have any objections, Probably other drivers can be
>>>> adapted similarly. In fact the change is quite simple.
>>>>
>>>> Comments ?
>>>>
>>>> Regards,
>>>> Manu
>>>>
>>>>
>>>> query_frontend_capabilities.diff
>>>>
>>>>
>>>> diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.c
>>>> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c Wed Nov 09 19:52:36 2011 +0530
>>>> +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c Thu Nov 10 13:51:35 2011 +0530
>>>> @@ -973,6 +973,8 @@
>>>>       _DTV_CMD(DTV_GUARD_INTERVAL, 0, 0),
>>>>       _DTV_CMD(DTV_TRANSMISSION_MODE, 0, 0),
>>>>       _DTV_CMD(DTV_HIERARCHY, 0, 0),
>>>> +
>>>> +     _DTV_CMD(DTV_DELIVERY_CAPS, 0, 0),
>>>>  };
>>>>
>>>>  static void dtv_property_dump(struct dtv_property *tvp)
>>>> @@ -1226,7 +1228,18 @@
>>>>               c = &cdetected;
>>>>       }
>>>>
>>>> +     dprintk("%s\n", __func__);
>>>> +
>>>>       switch(tvp->cmd) {
>>>> +     case DTV_DELIVERY_CAPS:
>>>> +             if (fe->ops.delivery_caps) {
>>>> +                     r = fe->ops.delivery_caps(fe, tvp);
>>>> +                     if (r < 0)
>>>> +                             return r;
>>>> +                     else
>>>> +                             goto done;
>>>
>>> What's the reason for introducing that label and goto?
>>
>>
>> The idea was to avoid using get_property callback being called.
>
> Nothing warrants that the only property will be DTV_DELIVERY_CAPS.
> calling get_property callback will likely work better.

It is indeed called within a case switch:
+	case DTV_DELIVERY_CAPS:
+		if (fe->ops.delivery_caps) {
+			r = fe->ops.delivery_caps(fe, tvp);

So, It does warrant that delivery caps are filled only with the
DTV_DELIVERY_CAPS, just like any other command, no difference in
there. But as I stated earlier, using get_property is just as fine.


>>
>>
>>
>>>
>>>> +             }
>>>> +             break;
>>>>       case DTV_FREQUENCY:
>>>>               tvp->u.data = c->frequency;
>>>>               break;
>>>> @@ -1350,7 +1363,7 @@
>>>>               if (r < 0)
>>>>                       return r;
>>>>       }
>>>> -
>>>> +done:
>>>>       dtv_property_dump(tvp);
>>>>
>>>>       return 0;
>>>> diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.h
>>>> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.h Wed Nov 09 19:52:36 2011 +0530
>>>> +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.h Thu Nov 10 13:51:35 2011 +0530
>>>> @@ -305,6 +305,8 @@
>>>>
>>>>       int (*set_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
>>>>       int (*get_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
>>>> +
>>>> +     int (*delivery_caps)(struct dvb_frontend *fe, struct dtv_property *tvp);
>>>>  };
>>>
>>> I don't think that another function pointer is required. Drivers can
>>> implement this in their get_property callback. The core could provide a
>>> default implementation, returning values derived from fe->ops.info.type
>>> and the 2G flag.
>>
>>
>> I see your point. While trying to address the issue I had originally
>> get_property being used. One Issue that came across to me was that:
>>
>>  - while currently this is just one capability field, things do look
>> fine. As we move ahead we will possibly have more capability related
>> information, given the idea that with shared hardware infrastructure
>> on frontends, this list of properties could grow.
>
> There are two DVBv5 calls that are likely designed to address this
> need:
>        DTV_FE_CAPABILITY_COUNT
>        DTV_FE_CAPABILITY
>
> Unfortunately, those were never implemented. I _suspect_ that the original
> idea were to allow passing an arbritary count to the driver, and let it fill
> each capability.


This was likely meant to be used for the FE_CAN flags as far as I can
see. This is not what I am trying to address.


>
>> - Once we have a few properties, then we will need to have switches in
>> that callback, given that it is generic and that length will grow.
>> Considering different hardware capabilities, we will have quite a bit
>> of shared code amongst drivers in that large switch, causing code
>> duplication.
>> Eg: with hardware having shared infrastructure - while globally it
>> might have a larger set of capabilities, when a sub part of it is run
>> in some mode, the other sub parts would have reduced set of
>> capabilities, which won't be the same as that exists globally for the
>> device. The driver alone will know the change depending on what's
>> being used.
>>
>>
>> - Additionally, though insignificant a separate callback for each
>> capability will make the drivers look a bit more consistent with
>> regards to code style.
>>
>> The basic issue is a switch that will be duplicated across drivers and
>> that which is expected to grow in length, eventually readability a
>> question. That said, this shouldn't be an issue at all. Given the
>> situation at this exact moment, having a generic callback is just
>> fine, as that will be the only which would be there, right now.
>>
>>
>>>
>>>>  #define MAX_EVENT 8
>>>> @@ -362,6 +364,8 @@
>>>>
>>>>       /* DVB-T2 specifics */
>>>>       u32                     dvbt2_plp_id;
>>>> +
>>>> +     fe_delivery_system_t    delivery_caps[32];
>>>>  };
>>>
>>> This array seems to be unused.
>>
>>
>> That just walked in astray while I worked on it. Didn't notice at all. :-)
>> I will send an updated patch, do you still think the generic callback
>> is better ?
>>
>>
>>
>>>
>>> Regards,
>>> Andreas
>>>
>>>>  struct dvb_frontend {
>>>> diff -r b6eb04718aa9 linux/drivers/media/dvb/frontends/stb0899_drv.c
>>>> --- a/linux/drivers/media/dvb/frontends/stb0899_drv.c Wed Nov 09 19:52:36 2011 +0530
>>>> +++ b/linux/drivers/media/dvb/frontends/stb0899_drv.c Thu Nov 10 13:51:35 2011 +0530
>>>> @@ -1605,6 +1605,19 @@
>>>>       return DVBFE_ALGO_CUSTOM;
>>>>  }
>>>>
>>>> +static int stb0899_delivery_caps(struct dvb_frontend *fe, struct dtv_property *caps)
>>>> +{
>>>> +     struct stb0899_state *state             = fe->demodulator_priv;
>>>> +
>>>> +     dprintk(state->verbose, FE_DEBUG, 1, "Get caps");
>>>> +     caps->u.buffer.data[0] = SYS_DSS;
>>>> +     caps->u.buffer.data[1] = SYS_DVBS;
>>>> +     caps->u.buffer.data[2] = SYS_DVBS2;
>>>> +     caps->u.buffer.len = 3;
>
> This doesn't sound right to me. If userspace passed space for only one delivery system,
> you can't just adjust the size here and expect it to work, as the array on userspace
> may be smaller than 3 properties.


You misunderstood.

The userspace requested just a single property, not more than that.
The usage at the kernel side is also for a single property.

This would be what would be running at userspace for the specified patch.

struct dtv_property p[] = {
    { .cmd = DTV_DELIVERY_CAPS },
}

struct dtv_properties cmd = {
     .num = 1,
     .props = p,
}

ioctl(fd, FE_GET_PROPERTY, &cmd);

> Also, the other DVBv5 properties may not be DTV_DELIVERY_CAPS type.
>
> In other words, the driver or core needs to check if the type is DTV_DELIVERY_CAPS
> before filling it.


It is indeed called within a case switch:
+	case DTV_DELIVERY_CAPS:
+		if (fe->ops.delivery_caps) {
+			r = fe->ops.delivery_caps(fe, tvp);


So, It does warrant that delivery caps are filled only with the
DTV_DELIVERY_CAPS, just like any other command, no difference in
there. But as I stated earlier, using get_property is just as fine.


>
> Btw, if the "capabilities" here is just the delivery system, it would be better to
> name it as something like DTV_GET_SUPPORTED_DELIVERY.


Yes that's the intention. But I would like to have a shorter name.
First of all the GET in there is superfluous as it is used in a
FE_GET_PROPERTY. Also you cannot SET from userspace a
SUPPORTED_DELIVERY. Do you feel much of a difference between
DTV_DELIVERY_CAPS and DTV_SUPPORTED_DELIVERY ? It's just a naming
issue. I can live with either of them, but still feel that
DTV_DELIVERY_CAPS is a bit more relevant.

>
> We should also think on a way to enumerate the supported values for each DVB properties,
> the enum fe_caps is not enough anymore to store everything. It currently has 30 bits
> filled (of a total of 32 bits), and we currently have:
>        12 code rates (including AUTO/NONE);
>        13 modulation types;
>        7 transmission modes;
>        7 bandwidths;
>        8 guard intervals (including AUTO);
>        5 hierarchy names;
>        4 rolloff's (probably, we'll need to add 2 more, to distinguish between DVB-C Annex A and Annex C).

These parameters aren't anything related to the current patch.

>
> So, if we would need to add one CAN_foo for each of the above, we would need 56 to 58
> bits, plus 5-6 bits to the other capabilities that currently exists there. So, even
> 64 bits won't be enough for the current needs (even having the delivery system caps
> addressed by something else).
>
> It means that FE_GET_INFO needs to be replaced by something that scales better, e. g.
> that would allow to enumerate the supported ranges for each DVBv5 parameter.


This is not what I am addressing. This is outside the scope of the
proposed patch and requirement. These flags are retrieved correctly as
of now and is used without any issues. The purpose of the patch is to
query DVB delivery system capabilities alone, rather than DVB frontend
info/capability.

Attached is a revised version 2 of the patch, which addresses the
issues that were raised.


Regards,
Manu

[-- Attachment #2: query_frontend_capabilities-v2.diff --]
[-- Type: text/x-patch, Size: 2195 bytes --]

diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.c
--- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Wed Nov 09 19:52:36 2011 +0530
+++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Fri Nov 11 06:05:40 2011 +0530
@@ -973,6 +973,8 @@
 	_DTV_CMD(DTV_GUARD_INTERVAL, 0, 0),
 	_DTV_CMD(DTV_TRANSMISSION_MODE, 0, 0),
 	_DTV_CMD(DTV_HIERARCHY, 0, 0),
+
+	_DTV_CMD(DTV_DELIVERY_CAPS, 0, 0),
 };
 
 static void dtv_property_dump(struct dtv_property *tvp)
@@ -1226,7 +1228,11 @@
 		c = &cdetected;
 	}
 
+	dprintk("%s\n", __func__);
+
 	switch(tvp->cmd) {
+	case DTV_DELIVERY_CAPS:
+		break;
 	case DTV_FREQUENCY:
 		tvp->u.data = c->frequency;
 		break;
@@ -1350,7 +1356,7 @@
 		if (r < 0)
 			return r;
 	}
-
+done:
 	dtv_property_dump(tvp);
 
 	return 0;
diff -r b6eb04718aa9 linux/drivers/media/dvb/frontends/stb0899_drv.c
--- a/linux/drivers/media/dvb/frontends/stb0899_drv.c	Wed Nov 09 19:52:36 2011 +0530
+++ b/linux/drivers/media/dvb/frontends/stb0899_drv.c	Fri Nov 11 06:05:40 2011 +0530
@@ -1605,6 +1605,22 @@
 	return DVBFE_ALGO_CUSTOM;
 }
 
+static int stb0899_get_property(struct dvb_frontend *fe, struct dtv_property *p)
+{
+	switch (p->cmd) {
+	case DTV_DELIVERY_CAPS:
+		p->u.buffer.data[0] = SYS_DSS;
+		p->u.buffer.data[1] = SYS_DVBS;
+		p->u.buffer.data[2] = SYS_DVBS2;
+		p->u.buffer.len = 3;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+
 static struct dvb_frontend_ops stb0899_ops = {
 
 	.info = {
@@ -1647,6 +1663,8 @@
 	.diseqc_send_master_cmd		= stb0899_send_diseqc_msg,
 	.diseqc_recv_slave_reply	= stb0899_recv_slave_reply,
 	.diseqc_send_burst		= stb0899_send_diseqc_burst,
+
+	.get_property			= stb0899_get_property,
 };
 
 struct dvb_frontend *stb0899_attach(struct stb0899_config *config, struct i2c_adapter *i2c)
diff -r b6eb04718aa9 linux/include/linux/dvb/frontend.h
--- a/linux/include/linux/dvb/frontend.h	Wed Nov 09 19:52:36 2011 +0530
+++ b/linux/include/linux/dvb/frontend.h	Fri Nov 11 06:05:40 2011 +0530
@@ -316,7 +316,9 @@
 
 #define DTV_DVBT2_PLP_ID	43
 
-#define DTV_MAX_COMMAND				DTV_DVBT2_PLP_ID
+#define DTV_DELIVERY_CAPS	44
+
+#define DTV_MAX_COMMAND				DTV_DELIVERY_CAPS
 
 typedef enum fe_pilot {
 	PILOT_ON,

FE_CAN-bits (was: Re: PATCH: Query DVB frontend capabilities)

[Patrick Boettcher]

Hi,

On Thursday, November 10, 2011 10:20:46 PM Mauro Carvalho Chehab wrote:
> 
> We should also think on a way to enumerate the supported values for each
> DVB properties, the enum fe_caps is not enough anymore to store
> everything. It currently has 30 bits filled (of a total of 32 bits), and
> we currently have:
> 	12 code rates (including AUTO/NONE);
> 	13 modulation types;
> 	7 transmission modes;
> 	7 bandwidths;
> 	8 guard intervals (including AUTO);
> 	5 hierarchy names;
> 	4 rolloff's (probably, we'll need to add 2 more, to distinguish between
> DVB-C Annex A and Annex C).
> 
> So, if we would need to add one CAN_foo for each of the above, we would
> need 56 to 58 bits, plus 5-6 bits to the other capabilities that
> currently exists there. So, even 64 bits won't be enough for the current
> needs (even having the delivery system caps addressed by something
> else).

IMHO, we don't need such a fine FE_CAN_-bit distinguishing for most 
standards. A well defined sub-standard definition is sufficient, which can be 
handled with a delivery-system-like query as proposed in the original patch. 
This also will be much simpler for most user-space applications and users.

DVB-T means: 
- 8K or 2K, 
- 1/4-1/32 Guard, 
- 1/2, 2/3, 3/4, 5/6 and 7/8 coderate, 
- QPSK, 64QAM or 16QAM

DVB-H (RIP as Remi wrote somewhere) would have meant:
- DVB-T + 4K + in-depth-interleaver mode

The same applies to ISDB-T and ISDB-T 1seg. And for CMMB, CTTB, DVB-SH. 

If there are demods which can't do one particular thing, we should forget 
about them. At least this is what almost all applications I have seen so far 
are doing implicitly. 

Though, I see at least one inconvenience is if someone is using linux-dvb 
for developping dsp-software and wants to deliver things which aren't done. 
But is this a case we want to "support" within the official API.

regards,
--
Patrick Boettcher - KernelLabs
http://www.kernellabs.com/

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 11-11-2011 04:26, Manu Abraham escreveu:
> On Fri, Nov 11, 2011 at 2:50 AM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> Em 10-11-2011 13:30, Manu Abraham escreveu:
>>> Hi Andreas,
>>>
>>> On Thu, Nov 10, 2011 at 8:14 PM, Andreas Oberritter <obi@linuxtv.org> wrote:
>>>> Hello Manu,
>>>>
>>>> please see my comments below:
>>>>
>>>> On 10.11.2011 15:18, Manu Abraham wrote:
>>>>> Hi,
>>>>>
>>>>> Currently, for a multi standard frontend it is assumed that it just
>>>>> has a single standard capability. This is fine in some cases, but
>>>>> makes things hard when there are incompatible standards in conjuction.
>>>>> Eg: DVB-S can be seen as a subset of DVB-S2, but the same doesn't hold
>>>>> the same for DSS. This is not specific to any driver as it is, but a
>>>>> generic issue. This was handled correctly in the multiproto tree,
>>>>> while such functionality is missing from the v5 API update.
>>>>>
>>>>> http://www.linuxtv.org/pipermail/vdr/2008-November/018417.html
>>>>>
>>>>> Later on a FE_CAN_2G_MODULATION was added as a hack to workaround this
>>>>> issue in the v5 API, but that hack is incapable of addressing the
>>>>> issue, as it can be used to simply distinguish between DVB-S and
>>>>> DVB-S2 alone, or another x vs X2 modulation. If there are more
>>>>> systems, then you have a potential issue.
>>
>> For sure something like that is needed.
>>
>> DVB TURBO is another example where the FE_CAN_2G_MODULATION approach doesn't
>> properly address.
>>
> 
> 
> It is similar to a DVB-S system with a different modulation and FEC.
> It was specifically designed to make it proprietary. Nothing wrong
> with it as it is now.

Yes, but DVB_TURBO uses its own FE_CAN flag. The point I'm trying to bold
is that using FE_CAN for delivery system doesn't scale, as there are only 
2 bits left.

>> There are two DVBv5 calls that are likely designed to address this
>> need:
>>        DTV_FE_CAPABILITY_COUNT
>>        DTV_FE_CAPABILITY
>>
>> Unfortunately, those were never implemented. I _suspect_ that the original
>> idea were to allow passing an arbritary count to the driver, and let it fill
>> each capability.
> 
> 
> This was likely meant to be used for the FE_CAN flags as far as I can
> see. This is not what I am trying to address.

Yes, but a few FE_CAN flags are used to indicate delivery systems (CAN_2G, CAN_TURBO).
So, both problems are connected, as future changes should not spend FE_CAN bit anymore 
for delivery systems.

>>>>> +static int stb0899_delivery_caps(struct dvb_frontend *fe, struct dtv_property *caps)
>>>>> +{
>>>>> +     struct stb0899_state *state             = fe->demodulator_priv;
>>>>> +
>>>>> +     dprintk(state->verbose, FE_DEBUG, 1, "Get caps");
>>>>> +     caps->u.buffer.data[0] = SYS_DSS;
>>>>> +     caps->u.buffer.data[1] = SYS_DVBS;
>>>>> +     caps->u.buffer.data[2] = SYS_DVBS2;
>>>>> +     caps->u.buffer.len = 3;
>>
>> This doesn't sound right to me. If userspace passed space for only one delivery system,
>> you can't just adjust the size here and expect it to work, as the array on userspace
>> may be smaller than 3 properties.
> 
> 
> You misunderstood.
> 
> The userspace requested just a single property, not more than that.
> The usage at the kernel side is also for a single property.
> 
> This would be what would be running at userspace for the specified patch.
> 
> struct dtv_property p[] = {
>     { .cmd = DTV_DELIVERY_CAPS },
> }
> 
> struct dtv_properties cmd = {
>      .num = 1,
>      .props = p,
> }
> 
> ioctl(fd, FE_GET_PROPERTY, &cmd);


Ah, now I see. As data is defined as __u8, that means that the DVB core will
be limited by up to 255 delivery systems. Also, as data size is 32, it is limited
up to 32 delivery systems supported by a single frontend.

Hmm... Currently, there is 17 delivery systems (18 with DSS). It seems that it would
scale. If we end by having more than 254 delivery systems, we can still use it, by
encoding it with RLE.

Seems reasonable for me.

>> Also, the other DVBv5 properties may not be DTV_DELIVERY_CAPS type.
>>
>> In other words, the driver or core needs to check if the type is DTV_DELIVERY_CAPS
>> before filling it.
> 
> 
> It is indeed called within a case switch:
> +	case DTV_DELIVERY_CAPS:
> +		if (fe->ops.delivery_caps) {
> +			r = fe->ops.delivery_caps(fe, tvp);
> 
> 
> So, It does warrant that delivery caps are filled only with the
> DTV_DELIVERY_CAPS, just like any other command, no difference in
> there. But as I stated earlier, using get_property is just as fine.
> 
> 
>>
>> Btw, if the "capabilities" here is just the delivery system, it would be better to
>> name it as something like DTV_GET_SUPPORTED_DELIVERY.
> 
> 
> Yes that's the intention. But I would like to have a shorter name.
> First of all the GET in there is superfluous as it is used in a
> FE_GET_PROPERTY. Also you cannot SET from userspace a
> SUPPORTED_DELIVERY. Do you feel much of a difference between
> DTV_DELIVERY_CAPS and DTV_SUPPORTED_DELIVERY ? It's just a naming
> issue. I can live with either of them, but still feel that
> DTV_DELIVERY_CAPS is a bit more relevant.

IMO, CAPS reminds FE_CAN flags. DTV_SUPPORTED_DELIVERY seems a good choice.

>>
>> We should also think on a way to enumerate the supported values for each DVB properties,
>> the enum fe_caps is not enough anymore to store everything. It currently has 30 bits
>> filled (of a total of 32 bits), and we currently have:
>>        12 code rates (including AUTO/NONE);
>>        13 modulation types;
>>        7 transmission modes;
>>        7 bandwidths;
>>        8 guard intervals (including AUTO);
>>        5 hierarchy names;
>>        4 rolloff's (probably, we'll need to add 2 more, to distinguish between DVB-C Annex A and Annex C).
> 
> These parameters aren't anything related to the current patch.
> 
>>
>> So, if we would need to add one CAN_foo for each of the above, we would need 56 to 58
>> bits, plus 5-6 bits to the other capabilities that currently exists there. So, even
>> 64 bits won't be enough for the current needs (even having the delivery system caps
>> addressed by something else).
>>
>> It means that FE_GET_INFO needs to be replaced by something that scales better, e. g.
>> that would allow to enumerate the supported ranges for each DVBv5 parameter.
> 
> 
> This is not what I am addressing. This is outside the scope of the
> proposed patch and requirement.

Ok, but, once we add it, we should not spend flags anymore for delivery system
support capabilities.

> These flags are retrieved correctly as of now and is used without any issues. 

There are some potential issues. If you look at the DVB-T2 patch
(changeset 94d56ffa0a9bf11dfb602dae9223089e09a8e050), you'll see that it 
added 3 new transmission modes, 3 new guard intervals and 3 new bandwidths. 

If some DVB-T2 devices have a limited set of capabilities, the two remaining FE_CAN
bits may not be enough.

> The purpose of the patch is to
> query DVB delivery system capabilities alone, rather than DVB frontend
> info/capability.
> 
> Attached is a revised version 2 of the patch, which addresses the
> issues that were raised.

It looks good for me. I would just rename it to DTV_SUPPORTED_DELIVERY.
Please, when submitting upstream, don't forget to increment DVB version and
add touch at DocBook, in order to not increase the gap between API specs and the
implementation.

Regards,
Mauro

Re: FE_CAN-bits

[Mauro Carvalho Chehab]

Em 11-11-2011 07:55, Patrick Boettcher escreveu:
> Hi,
> 
> On Thursday, November 10, 2011 10:20:46 PM Mauro Carvalho Chehab wrote:
>>
>> We should also think on a way to enumerate the supported values for each
>> DVB properties, the enum fe_caps is not enough anymore to store
>> everything. It currently has 30 bits filled (of a total of 32 bits), and
>> we currently have:
>> 	12 code rates (including AUTO/NONE);
>> 	13 modulation types;
>> 	7 transmission modes;
>> 	7 bandwidths;
>> 	8 guard intervals (including AUTO);
>> 	5 hierarchy names;
>> 	4 rolloff's (probably, we'll need to add 2 more, to distinguish between
>> DVB-C Annex A and Annex C).
>>
>> So, if we would need to add one CAN_foo for each of the above, we would
>> need 56 to 58 bits, plus 5-6 bits to the other capabilities that
>> currently exists there. So, even 64 bits won't be enough for the current
>> needs (even having the delivery system caps addressed by something
>> else).
> 
> IMHO, we don't need such a fine FE_CAN_-bit distinguishing for most 
> standards. A well defined sub-standard definition is sufficient, which can be 
> handled with a delivery-system-like query as proposed in the original patch. 
> This also will be much simpler for most user-space applications and users.
> 
> DVB-T means: 
> - 8K or 2K, 
> - 1/4-1/32 Guard, 
> - 1/2, 2/3, 3/4, 5/6 and 7/8 coderate, 
> - QPSK, 64QAM or 16QAM
> 
> DVB-H (RIP as Remi wrote somewhere) would have meant:
> - DVB-T + 4K + in-depth-interleaver mode
> 
> The same applies to ISDB-T and ISDB-T 1seg. And for CMMB, CTTB, DVB-SH. 

ISDB-T and ISDB-T 1 seg currently are the same delivery system. Btw, I have
here some USB sticks that support:
	- only 1-seg
	- 1-seg and 3-seg
	- full seg.

An userspace application should be capable of detecting it.

I suspect we'll see more stuff like that, as cell phones and tablets start
integrating DVB chips inside. As some may not support HD TV anyway, the
vendor may opt for buying a less-expensive chipset.

> If there are demods which can't do one particular thing, we should forget 
> about them. At least this is what almost all applications I have seen so far 
> are doing implicitly. 
> 
> Though, I see at least one inconvenience is if someone is using linux-dvb 
> for developping dsp-software and wants to deliver things which aren't done. 
> But is this a case we want to "support" within the official API.

I don't care for DVB drivers that aren't submitted upstream, as the vendor
of such drivers explicitly decided to fork, so it is their responsibility
to support its own fork, including any userspace changes that might be
required for his hardware to work, but embedded system vendors (for STB, mobile,
TV, etc) want to send us drivers, we should extend the API to fulfill their
needs.

Regards,
Mauro

Re: FE_CAN-bits

[Antti Palosaari]

On 11/11/2011 11:55 AM, Patrick Boettcher wrote:
> On Thursday, November 10, 2011 10:20:46 PM Mauro Carvalho Chehab wrote:
>>
>> We should also think on a way to enumerate the supported values for each
>> DVB properties, the enum fe_caps is not enough anymore to store
>> everything. It currently has 30 bits filled (of a total of 32 bits), and
>> we currently have:
>> 	12 code rates (including AUTO/NONE);
>> 	13 modulation types;
>> 	7 transmission modes;
>> 	7 bandwidths;
>> 	8 guard intervals (including AUTO);
>> 	5 hierarchy names;
>> 	4 rolloff's (probably, we'll need to add 2 more, to distinguish between
>> DVB-C Annex A and Annex C).
>>
>> So, if we would need to add one CAN_foo for each of the above, we would
>> need 56 to 58 bits, plus 5-6 bits to the other capabilities that
>> currently exists there. So, even 64 bits won't be enough for the current
>> needs (even having the delivery system caps addressed by something
>> else).
>
> IMHO, we don't need such a fine FE_CAN_-bit distinguishing for most
> standards. A well defined sub-standard definition is sufficient, which can be
> handled with a delivery-system-like query as proposed in the original patch.
> This also will be much simpler for most user-space applications and users.

I agree that. Those are totally useless in general. Let driver return 
error to userspace if it cannot handle.

> DVB-T means:
> - 8K or 2K,
> - 1/4-1/32 Guard,
> - 1/2, 2/3, 3/4, 5/6 and 7/8 coderate,
> - QPSK, 64QAM or 16QAM
>
> DVB-H (RIP as Remi wrote somewhere) would have meant:
> - DVB-T + 4K + in-depth-interleaver mode
>
> The same applies to ISDB-T and ISDB-T 1seg. And for CMMB, CTTB, DVB-SH.
>
> If there are demods which can't do one particular thing, we should forget
> about them. At least this is what almost all applications I have seen so far
> are doing implicitly.

I know only one case where device cannot support all standard 
parameters. It is one TDA10023 device and looks like stream goes too 
wide when QAM256 is used.

> Though, I see at least one inconvenience is if someone is using linux-dvb
> for developping dsp-software and wants to deliver things which aren't done.
> But is this a case we want to "support" within the official API.


regards
Antti


-- 
http://palosaari.fi/

Re: FE_CAN-bits

[Mauro Carvalho Chehab]

Em 11-11-2011 09:36, Antti Palosaari escreveu:
> On 11/11/2011 11:55 AM, Patrick Boettcher wrote:
>> On Thursday, November 10, 2011 10:20:46 PM Mauro Carvalho Chehab wrote:
>>>
>>> We should also think on a way to enumerate the supported values for each
>>> DVB properties, the enum fe_caps is not enough anymore to store
>>> everything. It currently has 30 bits filled (of a total of 32 bits), and
>>> we currently have:
>>>     12 code rates (including AUTO/NONE);
>>>     13 modulation types;
>>>     7 transmission modes;
>>>     7 bandwidths;
>>>     8 guard intervals (including AUTO);
>>>     5 hierarchy names;
>>>     4 rolloff's (probably, we'll need to add 2 more, to distinguish between
>>> DVB-C Annex A and Annex C).
>>>
>>> So, if we would need to add one CAN_foo for each of the above, we would
>>> need 56 to 58 bits, plus 5-6 bits to the other capabilities that
>>> currently exists there. So, even 64 bits won't be enough for the current
>>> needs (even having the delivery system caps addressed by something
>>> else).
>>
>> IMHO, we don't need such a fine FE_CAN_-bit distinguishing for most
>> standards. A well defined sub-standard definition is sufficient, which can be
>> handled with a delivery-system-like query as proposed in the original patch.
>> This also will be much simpler for most user-space applications and users.
> 
> I agree that. Those are totally useless in general. Let driver return error to userspace if it cannot handle.
> 
>> DVB-T means:
>> - 8K or 2K,
>> - 1/4-1/32 Guard,
>> - 1/2, 2/3, 3/4, 5/6 and 7/8 coderate,
>> - QPSK, 64QAM or 16QAM
>>
>> DVB-H (RIP as Remi wrote somewhere) would have meant:
>> - DVB-T + 4K + in-depth-interleaver mode
>>
>> The same applies to ISDB-T and ISDB-T 1seg. And for CMMB, CTTB, DVB-SH.
>>
>> If there are demods which can't do one particular thing, we should forget
>> about them. At least this is what almost all applications I have seen so far
>> are doing implicitly.
> 
> I know only one case where device cannot support all standard parameters. It is one TDA10023 device and looks like stream goes too wide when QAM256 is used.
> 
>> Though, I see at least one inconvenience is if someone is using linux-dvb
>> for developping dsp-software and wants to deliver things which aren't done.
>> But is this a case we want to "support" within the official API.
> 

If you take a look at DVB-C, for example, The reference used by the DVB subsystem
seems to be the ITU-T J.83, as the delivery systems are named according to
ITU-T J.83 annexes:
	Annex A - European DVB-C (also defined on EN 300 429)
	Annex B - American DOCSYS
	Annex C - Japanese variant of Annex A, optimized for 6 MHz Bw

According with ITU-T J.83/1997 (from where Annex A, B and C are referenced), we have:

Annex A:
	- modulation: QAM 16, 32 and 64
	  Mentions that QAM 128 and 256 could be used in future
	- rolloff: 0.15

Annex B:
	- Modulation: QAM 64, 256

Annex C:
	- Modulation: QAM 64
	- rolloff: 0.13

ITU-T Annex A is also defined at ETSI as EN 300 429/1998. There, we have:
	- modulation: QAM 16, 32, 64, 128 and 256
	- rolloff: 0.15

As the same delivery system is used for both Annex A and Annex C, the "minimum"
requirement for SYS_DVBC_ANNEX_AC is to support QAM64 (as it can be a device that 
implements only Annex C).

So, just assuming some default from the delivery system is dangerous. Also, as
specs may change with time (as J.83 -> EN 300 429 addition for QAM 128 and 256),
this may lead into troubles in the future.

Btw, DVB-C2, as defined on ITU-T J.122 is even more complex, offering a myriad of
mandatory and optional formats, as shown at item 6.2.3:
	The upstream demodulator MAY support QPSK and 16QAM differential modulation for TDMA.
	The upstream demodulator MUST support QPSK, 16QAM, and 64QAM modulations for TDMA and S-CDMA channels.
	The upstream demodulator MAY support 8QAM and 32QAM modulation for TDMA and S-CDMA channels.
	The upstream demodulator MAY support QPSK, 8QAM, 16QAM, 32QAM, 64QAM, and 128QAM TCM encoded modulations for S-CDMA channels.

What I think we can do is to provide macros for the capabilities, like:

#define FE_CAN_DVBT	FE_CAN_1_2 | FE_CAN_3_4 | ...

With regards to the idea of returning an error, this may not work on all cases.
For example, my 1seg stick is capable of retrieving channel info from 3-seg and full-seg
streams, even not being able of actually watching those. Ideally, userspace should
be capable of disabling the 3seg/full-seg channels if they aren't actually supported
by the plugged device.

Regards,
Mauro

Re: PATCH: Query DVB frontend capabilities

[Andreas Oberritter]

On 11.11.2011 07:26, Manu Abraham wrote:
> diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.c
> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Wed Nov 09 19:52:36 2011 +0530
> +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Fri Nov 11 06:05:40 2011 +0530
> @@ -973,6 +973,8 @@
>  	_DTV_CMD(DTV_GUARD_INTERVAL, 0, 0),
>  	_DTV_CMD(DTV_TRANSMISSION_MODE, 0, 0),
>  	_DTV_CMD(DTV_HIERARCHY, 0, 0),
> +
> +	_DTV_CMD(DTV_DELIVERY_CAPS, 0, 0),
>  };
>  
>  static void dtv_property_dump(struct dtv_property *tvp)
> @@ -1226,7 +1228,11 @@
>  		c = &cdetected;
>  	}
>  
> +	dprintk("%s\n", __func__);
> +
>  	switch(tvp->cmd) {
> +	case DTV_DELIVERY_CAPS:

It would be nice to have a default implementation inserted at this point, e.g. something like:

static void dtv_set_default_delivery_caps(const struct dvb_frontend *fe, struct dtv_property *p)
{
	const struct dvb_frontend_info *info = &fe->ops.info;
	u32 ncaps = 0;

	switch (info->type) {
	case FE_QPSK:
		p->u.buffer.data[ncaps++] = SYS_DVBS;
		if (info->caps & FE_CAN_2G_MODULATION)
			p->u.buffer.data[ncaps++] = SYS_DVBS2;
		if (info->caps & FE_CAN_TURBO_FEC)
			p->u.buffer.data[ncaps++] = SYS_TURBO;
		break;
	case FE_QAM:
		p->u.buffer.data[ncaps++] = SYS_DVBC_ANNEX_AC;
		break;
	case FE_OFDM:
		p->u.buffer.data[ncaps++] = SYS_DVBT;
		if (info->caps & FE_CAN_2G_MODULATION)
			p->u.buffer.data[ncaps++] = SYS_DVBT2;
		break;
	case FE_ATSC:
		if (info->caps & (FE_CAN_8VSB | FE_CAN_16VSB))
			p->u.buffer.data[ncaps++] = SYS_ATSC;
		if (info->caps & (FE_CAN_QAM_16 | FE_CAN_QAM_64 | FE_CAN_QAM_128 | FE_CAN_QAM_256))
			p->u.buffer.data[ncaps++] = SYS_DVBC_ANNEX_B;
	}

	p->u.buffer.len = ncaps;
}

I think this would be sufficient for a lot of drivers and would thus save a lot of work.

> +		break;
>  	case DTV_FREQUENCY:
>  		tvp->u.data = c->frequency;
>  		break;
> @@ -1350,7 +1356,7 @@
>  		if (r < 0)
>  			return r;
>  	}
> -
> +done:

This label is unused now and should be removed.

>  	dtv_property_dump(tvp);
>  
>  	return 0;
> diff -r b6eb04718aa9 linux/drivers/media/dvb/frontends/stb0899_drv.c
> --- a/linux/drivers/media/dvb/frontends/stb0899_drv.c	Wed Nov 09 19:52:36 2011 +0530
> +++ b/linux/drivers/media/dvb/frontends/stb0899_drv.c	Fri Nov 11 06:05:40 2011 +0530
> @@ -1605,6 +1605,22 @@
>  	return DVBFE_ALGO_CUSTOM;
>  }
>  
> +static int stb0899_get_property(struct dvb_frontend *fe, struct dtv_property *p)
> +{
> +	switch (p->cmd) {
> +	case DTV_DELIVERY_CAPS:
> +		p->u.buffer.data[0] = SYS_DSS;
> +		p->u.buffer.data[1] = SYS_DVBS;
> +		p->u.buffer.data[2] = SYS_DVBS2;
> +		p->u.buffer.len = 3;
> +		break;
> +	default:
> +		return -EINVAL;

You should ignore all unhandled properties. Otherwise all properties handled
by the core will have result set to -EINVAL.

> +	}
> +	return 0;
> +}
> +
> +
>  static struct dvb_frontend_ops stb0899_ops = {
>  
>  	.info = {
> @@ -1647,6 +1663,8 @@
>  	.diseqc_send_master_cmd		= stb0899_send_diseqc_msg,
>  	.diseqc_recv_slave_reply	= stb0899_recv_slave_reply,
>  	.diseqc_send_burst		= stb0899_send_diseqc_burst,
> +
> +	.get_property			= stb0899_get_property,
>  };
>  
>  struct dvb_frontend *stb0899_attach(struct stb0899_config *config, struct i2c_adapter *i2c)
> diff -r b6eb04718aa9 linux/include/linux/dvb/frontend.h
> --- a/linux/include/linux/dvb/frontend.h	Wed Nov 09 19:52:36 2011 +0530
> +++ b/linux/include/linux/dvb/frontend.h	Fri Nov 11 06:05:40 2011 +0530
> @@ -316,7 +316,9 @@
>  
>  #define DTV_DVBT2_PLP_ID	43
>  
> -#define DTV_MAX_COMMAND				DTV_DVBT2_PLP_ID
> +#define DTV_DELIVERY_CAPS	44
> +
> +#define DTV_MAX_COMMAND				DTV_DELIVERY_CAPS
>  
>  typedef enum fe_pilot {
>  	PILOT_ON,
> 

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 11-11-2011 12:30, Andreas Oberritter escreveu:
> On 11.11.2011 07:26, Manu Abraham wrote:
>> diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.c
>> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Wed Nov 09 19:52:36 2011 +0530
>> +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Fri Nov 11 06:05:40 2011 +0530
>> @@ -973,6 +973,8 @@
>>  	_DTV_CMD(DTV_GUARD_INTERVAL, 0, 0),
>>  	_DTV_CMD(DTV_TRANSMISSION_MODE, 0, 0),
>>  	_DTV_CMD(DTV_HIERARCHY, 0, 0),
>> +
>> +	_DTV_CMD(DTV_DELIVERY_CAPS, 0, 0),
>>  };
>>  
>>  static void dtv_property_dump(struct dtv_property *tvp)
>> @@ -1226,7 +1228,11 @@
>>  		c = &cdetected;
>>  	}
>>  
>> +	dprintk("%s\n", __func__);
>> +
>>  	switch(tvp->cmd) {
>> +	case DTV_DELIVERY_CAPS:
> 
> It would be nice to have a default implementation inserted at this point, e.g. something like:
> 
> static void dtv_set_default_delivery_caps(const struct dvb_frontend *fe, struct dtv_property *p)
> {
> 	const struct dvb_frontend_info *info = &fe->ops.info;
> 	u32 ncaps = 0;
> 
> 	switch (info->type) {
> 	case FE_QPSK:
> 		p->u.buffer.data[ncaps++] = SYS_DVBS;
> 		if (info->caps & FE_CAN_2G_MODULATION)
> 			p->u.buffer.data[ncaps++] = SYS_DVBS2;
> 		if (info->caps & FE_CAN_TURBO_FEC)
> 			p->u.buffer.data[ncaps++] = SYS_TURBO;
> 		break;
> 	case FE_QAM:
> 		p->u.buffer.data[ncaps++] = SYS_DVBC_ANNEX_AC;
> 		break;
> 	case FE_OFDM:
> 		p->u.buffer.data[ncaps++] = SYS_DVBT;
> 		if (info->caps & FE_CAN_2G_MODULATION)
> 			p->u.buffer.data[ncaps++] = SYS_DVBT2;
> 		break;
> 	case FE_ATSC:
> 		if (info->caps & (FE_CAN_8VSB | FE_CAN_16VSB))
> 			p->u.buffer.data[ncaps++] = SYS_ATSC;
> 		if (info->caps & (FE_CAN_QAM_16 | FE_CAN_QAM_64 | FE_CAN_QAM_128 | FE_CAN_QAM_256))
> 			p->u.buffer.data[ncaps++] = SYS_DVBC_ANNEX_B;
> 	}
> 
> 	p->u.buffer.len = ncaps;
> }
> 
> I think this would be sufficient for a lot of drivers and would thus save a lot of work.
> 
>> +		break;
>>  	case DTV_FREQUENCY:
>>  		tvp->u.data = c->frequency;
>>  		break;
>> @@ -1350,7 +1356,7 @@
>>  		if (r < 0)
>>  			return r;
>>  	}
>> -
>> +done:
> 
> This label is unused now and should be removed.
> 
>>  	dtv_property_dump(tvp);
>>  
>>  	return 0;
>> diff -r b6eb04718aa9 linux/drivers/media/dvb/frontends/stb0899_drv.c
>> --- a/linux/drivers/media/dvb/frontends/stb0899_drv.c	Wed Nov 09 19:52:36 2011 +0530
>> +++ b/linux/drivers/media/dvb/frontends/stb0899_drv.c	Fri Nov 11 06:05:40 2011 +0530
>> @@ -1605,6 +1605,22 @@
>>  	return DVBFE_ALGO_CUSTOM;
>>  }
>>  
>> +static int stb0899_get_property(struct dvb_frontend *fe, struct dtv_property *p)
>> +{
>> +	switch (p->cmd) {
>> +	case DTV_DELIVERY_CAPS:
>> +		p->u.buffer.data[0] = SYS_DSS;
>> +		p->u.buffer.data[1] = SYS_DVBS;
>> +		p->u.buffer.data[2] = SYS_DVBS2;
>> +		p->u.buffer.len = 3;
>> +		break;
>> +	default:
>> +		return -EINVAL;
> 
> You should ignore all unhandled properties. Otherwise all properties handled
> by the core will have result set to -EINVAL.

IMHO, the better is to set all parameters via stb0899_get_property(). We should
work on deprecate the old way, as, by having all frontends implementing the
get/set property ops, we can remove part of the legacy code inside the DVB core.

> 
>> +	}
>> +	return 0;
>> +}
>> +
>> +
>>  static struct dvb_frontend_ops stb0899_ops = {
>>  
>>  	.info = {
>> @@ -1647,6 +1663,8 @@
>>  	.diseqc_send_master_cmd		= stb0899_send_diseqc_msg,
>>  	.diseqc_recv_slave_reply	= stb0899_recv_slave_reply,
>>  	.diseqc_send_burst		= stb0899_send_diseqc_burst,
>> +
>> +	.get_property			= stb0899_get_property,
>>  };
>>  
>>  struct dvb_frontend *stb0899_attach(struct stb0899_config *config, struct i2c_adapter *i2c)
>> diff -r b6eb04718aa9 linux/include/linux/dvb/frontend.h
>> --- a/linux/include/linux/dvb/frontend.h	Wed Nov 09 19:52:36 2011 +0530
>> +++ b/linux/include/linux/dvb/frontend.h	Fri Nov 11 06:05:40 2011 +0530
>> @@ -316,7 +316,9 @@
>>  
>>  #define DTV_DVBT2_PLP_ID	43
>>  
>> -#define DTV_MAX_COMMAND				DTV_DVBT2_PLP_ID
>> +#define DTV_DELIVERY_CAPS	44
>> +
>> +#define DTV_MAX_COMMAND				DTV_DELIVERY_CAPS
>>  
>>  typedef enum fe_pilot {
>>  	PILOT_ON,
>>
> 

Re: PATCH: Query DVB frontend capabilities

[Andreas Oberritter]

On 11.11.2011 15:43, Mauro Carvalho Chehab wrote:
> Em 11-11-2011 12:30, Andreas Oberritter escreveu:
>> On 11.11.2011 07:26, Manu Abraham wrote:
>>> diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.c
>>> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Wed Nov 09 19:52:36 2011 +0530
>>> +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Fri Nov 11 06:05:40 2011 +0530
>>> @@ -973,6 +973,8 @@
>>>  	_DTV_CMD(DTV_GUARD_INTERVAL, 0, 0),
>>>  	_DTV_CMD(DTV_TRANSMISSION_MODE, 0, 0),
>>>  	_DTV_CMD(DTV_HIERARCHY, 0, 0),
>>> +
>>> +	_DTV_CMD(DTV_DELIVERY_CAPS, 0, 0),
>>>  };
>>>  
>>>  static void dtv_property_dump(struct dtv_property *tvp)
>>> @@ -1226,7 +1228,11 @@
>>>  		c = &cdetected;
>>>  	}
>>>  
>>> +	dprintk("%s\n", __func__);
>>> +
>>>  	switch(tvp->cmd) {
>>> +	case DTV_DELIVERY_CAPS:
>>
>> It would be nice to have a default implementation inserted at this point, e.g. something like:
>>
>> static void dtv_set_default_delivery_caps(const struct dvb_frontend *fe, struct dtv_property *p)
>> {
>> 	const struct dvb_frontend_info *info = &fe->ops.info;
>> 	u32 ncaps = 0;
>>
>> 	switch (info->type) {
>> 	case FE_QPSK:
>> 		p->u.buffer.data[ncaps++] = SYS_DVBS;
>> 		if (info->caps & FE_CAN_2G_MODULATION)
>> 			p->u.buffer.data[ncaps++] = SYS_DVBS2;
>> 		if (info->caps & FE_CAN_TURBO_FEC)
>> 			p->u.buffer.data[ncaps++] = SYS_TURBO;
>> 		break;
>> 	case FE_QAM:
>> 		p->u.buffer.data[ncaps++] = SYS_DVBC_ANNEX_AC;
>> 		break;
>> 	case FE_OFDM:
>> 		p->u.buffer.data[ncaps++] = SYS_DVBT;
>> 		if (info->caps & FE_CAN_2G_MODULATION)
>> 			p->u.buffer.data[ncaps++] = SYS_DVBT2;
>> 		break;
>> 	case FE_ATSC:
>> 		if (info->caps & (FE_CAN_8VSB | FE_CAN_16VSB))
>> 			p->u.buffer.data[ncaps++] = SYS_ATSC;
>> 		if (info->caps & (FE_CAN_QAM_16 | FE_CAN_QAM_64 | FE_CAN_QAM_128 | FE_CAN_QAM_256))
>> 			p->u.buffer.data[ncaps++] = SYS_DVBC_ANNEX_B;
>> 	}
>>
>> 	p->u.buffer.len = ncaps;
>> }
>>
>> I think this would be sufficient for a lot of drivers and would thus save a lot of work.
>>
>>> +		break;
>>>  	case DTV_FREQUENCY:
>>>  		tvp->u.data = c->frequency;
>>>  		break;
>>> @@ -1350,7 +1356,7 @@
>>>  		if (r < 0)
>>>  			return r;
>>>  	}
>>> -
>>> +done:
>>
>> This label is unused now and should be removed.
>>
>>>  	dtv_property_dump(tvp);
>>>  
>>>  	return 0;
>>> diff -r b6eb04718aa9 linux/drivers/media/dvb/frontends/stb0899_drv.c
>>> --- a/linux/drivers/media/dvb/frontends/stb0899_drv.c	Wed Nov 09 19:52:36 2011 +0530
>>> +++ b/linux/drivers/media/dvb/frontends/stb0899_drv.c	Fri Nov 11 06:05:40 2011 +0530
>>> @@ -1605,6 +1605,22 @@
>>>  	return DVBFE_ALGO_CUSTOM;
>>>  }
>>>  
>>> +static int stb0899_get_property(struct dvb_frontend *fe, struct dtv_property *p)
>>> +{
>>> +	switch (p->cmd) {
>>> +	case DTV_DELIVERY_CAPS:
>>> +		p->u.buffer.data[0] = SYS_DSS;
>>> +		p->u.buffer.data[1] = SYS_DVBS;
>>> +		p->u.buffer.data[2] = SYS_DVBS2;
>>> +		p->u.buffer.len = 3;
>>> +		break;
>>> +	default:
>>> +		return -EINVAL;
>>
>> You should ignore all unhandled properties. Otherwise all properties handled
>> by the core will have result set to -EINVAL.
> 
> IMHO, the better is to set all parameters via stb0899_get_property(). We should
> work on deprecate the old way, as, by having all frontends implementing the
> get/set property ops, we can remove part of the legacy code inside the DVB core.

I'm not sure what "legacy code" you're referring to. If you're referring
to the big switch in dtv_property_process_get(), which presets output
values based on previously set tuning parameters, then no, please don't
deprecate it. It doesn't improve driver code if you move this switch
down into every driver.

Of course, a driver can and should override any property it knows about
in its get_property callback, if - and only if - the property value
possibly differs from the value set by the default implementation in
dvb_frontend.

However, all of this is out of scope of Manu's patch. Please just remove
the erroneous return statement.

>>
>>> +	}
>>> +	return 0;
>>> +}
>>> +
>>> +
>>>  static struct dvb_frontend_ops stb0899_ops = {
>>>  
>>>  	.info = {
>>> @@ -1647,6 +1663,8 @@
>>>  	.diseqc_send_master_cmd		= stb0899_send_diseqc_msg,
>>>  	.diseqc_recv_slave_reply	= stb0899_recv_slave_reply,
>>>  	.diseqc_send_burst		= stb0899_send_diseqc_burst,
>>> +
>>> +	.get_property			= stb0899_get_property,
>>>  };
>>>  
>>>  struct dvb_frontend *stb0899_attach(struct stb0899_config *config, struct i2c_adapter *i2c)
>>> diff -r b6eb04718aa9 linux/include/linux/dvb/frontend.h
>>> --- a/linux/include/linux/dvb/frontend.h	Wed Nov 09 19:52:36 2011 +0530
>>> +++ b/linux/include/linux/dvb/frontend.h	Fri Nov 11 06:05:40 2011 +0530
>>> @@ -316,7 +316,9 @@
>>>  
>>>  #define DTV_DVBT2_PLP_ID	43
>>>  
>>> -#define DTV_MAX_COMMAND				DTV_DVBT2_PLP_ID
>>> +#define DTV_DELIVERY_CAPS	44
>>> +
>>> +#define DTV_MAX_COMMAND				DTV_DELIVERY_CAPS
>>>  
>>>  typedef enum fe_pilot {
>>>  	PILOT_ON,
>>>
>>
> 

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 11-11-2011 13:06, Andreas Oberritter escreveu:
> On 11.11.2011 15:43, Mauro Carvalho Chehab wrote:
>> Em 11-11-2011 12:30, Andreas Oberritter escreveu:
>>> On 11.11.2011 07:26, Manu Abraham wrote:
>>>> diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.c
>>>> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Wed Nov 09 19:52:36 2011 +0530
>>>> +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	Fri Nov 11 06:05:40 2011 +0530
>>>> @@ -973,6 +973,8 @@
>>>>  	_DTV_CMD(DTV_GUARD_INTERVAL, 0, 0),
>>>>  	_DTV_CMD(DTV_TRANSMISSION_MODE, 0, 0),
>>>>  	_DTV_CMD(DTV_HIERARCHY, 0, 0),
>>>> +
>>>> +	_DTV_CMD(DTV_DELIVERY_CAPS, 0, 0),
>>>>  };
>>>>  
>>>>  static void dtv_property_dump(struct dtv_property *tvp)
>>>> @@ -1226,7 +1228,11 @@
>>>>  		c = &cdetected;
>>>>  	}
>>>>  
>>>> +	dprintk("%s\n", __func__);
>>>> +
>>>>  	switch(tvp->cmd) {
>>>> +	case DTV_DELIVERY_CAPS:
>>>
>>> It would be nice to have a default implementation inserted at this point, e.g. something like:
>>>
>>> static void dtv_set_default_delivery_caps(const struct dvb_frontend *fe, struct dtv_property *p)
>>> {
>>> 	const struct dvb_frontend_info *info = &fe->ops.info;
>>> 	u32 ncaps = 0;
>>>
>>> 	switch (info->type) {
>>> 	case FE_QPSK:
>>> 		p->u.buffer.data[ncaps++] = SYS_DVBS;
>>> 		if (info->caps & FE_CAN_2G_MODULATION)
>>> 			p->u.buffer.data[ncaps++] = SYS_DVBS2;
>>> 		if (info->caps & FE_CAN_TURBO_FEC)
>>> 			p->u.buffer.data[ncaps++] = SYS_TURBO;
>>> 		break;
>>> 	case FE_QAM:
>>> 		p->u.buffer.data[ncaps++] = SYS_DVBC_ANNEX_AC;
>>> 		break;
>>> 	case FE_OFDM:
>>> 		p->u.buffer.data[ncaps++] = SYS_DVBT;
>>> 		if (info->caps & FE_CAN_2G_MODULATION)
>>> 			p->u.buffer.data[ncaps++] = SYS_DVBT2;
>>> 		break;
>>> 	case FE_ATSC:
>>> 		if (info->caps & (FE_CAN_8VSB | FE_CAN_16VSB))
>>> 			p->u.buffer.data[ncaps++] = SYS_ATSC;
>>> 		if (info->caps & (FE_CAN_QAM_16 | FE_CAN_QAM_64 | FE_CAN_QAM_128 | FE_CAN_QAM_256))
>>> 			p->u.buffer.data[ncaps++] = SYS_DVBC_ANNEX_B;
>>> 	}
>>>
>>> 	p->u.buffer.len = ncaps;
>>> }
>>>
>>> I think this would be sufficient for a lot of drivers and would thus save a lot of work.
>>>
>>>> +		break;
>>>>  	case DTV_FREQUENCY:
>>>>  		tvp->u.data = c->frequency;
>>>>  		break;
>>>> @@ -1350,7 +1356,7 @@
>>>>  		if (r < 0)
>>>>  			return r;
>>>>  	}
>>>> -
>>>> +done:
>>>
>>> This label is unused now and should be removed.
>>>
>>>>  	dtv_property_dump(tvp);
>>>>  
>>>>  	return 0;
>>>> diff -r b6eb04718aa9 linux/drivers/media/dvb/frontends/stb0899_drv.c
>>>> --- a/linux/drivers/media/dvb/frontends/stb0899_drv.c	Wed Nov 09 19:52:36 2011 +0530
>>>> +++ b/linux/drivers/media/dvb/frontends/stb0899_drv.c	Fri Nov 11 06:05:40 2011 +0530
>>>> @@ -1605,6 +1605,22 @@
>>>>  	return DVBFE_ALGO_CUSTOM;
>>>>  }
>>>>  
>>>> +static int stb0899_get_property(struct dvb_frontend *fe, struct dtv_property *p)
>>>> +{
>>>> +	switch (p->cmd) {
>>>> +	case DTV_DELIVERY_CAPS:
>>>> +		p->u.buffer.data[0] = SYS_DSS;
>>>> +		p->u.buffer.data[1] = SYS_DVBS;
>>>> +		p->u.buffer.data[2] = SYS_DVBS2;
>>>> +		p->u.buffer.len = 3;
>>>> +		break;
>>>> +	default:
>>>> +		return -EINVAL;
>>>
>>> You should ignore all unhandled properties. Otherwise all properties handled
>>> by the core will have result set to -EINVAL.
>>
>> IMHO, the better is to set all parameters via stb0899_get_property(). We should
>> work on deprecate the old way, as, by having all frontends implementing the
>> get/set property ops, we can remove part of the legacy code inside the DVB core.
> 
> I'm not sure what "legacy code" you're referring to. If you're referring
> to the big switch in dtv_property_process_get(), which presets output
> values based on previously set tuning parameters, then no, please don't
> deprecate it. It doesn't improve driver code if you move this switch
> down into every driver.

What I mean is that drivers should get rid of implementing get_frontend() and 
set_frontend(), restricting the usage of struct dvb_frontend_parameters for DVBv3
calls from userspace.

In other words, it should be part of the core logic to get all the parameters
passed from userspace and passing them via one single call to something similar
to set_property. 

In other words, ideally, the implementation for DTV set should be
like:

static int dtv_property_process_set(struct dvb_frontend *fe,
				    struct dtv_property *tvp,
				    struct file *file)
{
	struct dtv_frontend_properties *c = &fe->dtv_property_cache;

	switch(tvp->cmd) {
	case DTV_CLEAR:
		dvb_frontend_clear_cache(fe);
		break;
	case DTV_FREQUENCY:
		c->frequency = tvp->u.data;
		break;
	case DTV_MODULATION:
		c->modulation = tvp->u.data;
		break;
	case DTV_BANDWIDTH_HZ:
		c->bandwidth_hz = tvp->u.data;
		break;
...
	case DTV_TUNE:
		/* interpret the cache of data */
		if (fe->ops.new_set_frontend) {
			r = fe->ops.new_set_frontend(fe);
			if (r < 0)
				return r;
		}
		break;

E. g. instead of using the struct dvb_frontend_parameters, the drivers would
use struct dtv_frontend_properties (already stored at the frontend
struct as fe->dtv_property_cache).

Btw, with such change, it would actually make sense the original proposal
from Manu of having a separate callback for supported delivery systems.

> Of course, a driver can and should override any property it knows about
> in its get_property callback, if - and only if - the property value
> possibly differs from the value set by the default implementation in
> dvb_frontend.
> 
> However, all of this is out of scope of Manu's patch. 

Yes. The above change would require more work than what the current
patch is trying to address.

Regards,
Mauro

Re: PATCH: Query DVB frontend capabilities

[BOUWSMA Barry]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


On Do (Donnerstag) 10.Nov (November) 2011, 22:20,  Mauro Carvalho Chehab wrote:

> We should also think on a way to enumerate the supported values for each DVB $
> the enum fe_caps is not enough anymore to store everything. It currently has $
> filled (of a total of 32 bits), and we currently have:
> 	12 code rates (including AUTO/NONE);

I'm probably not looking at the correct source, but the numbers
seem to match, so I'll just note that in what I'm looking at,
there are missing the values  1/3  and  2/5 .

But I have to apologise in that I've also not been paying
attention to this conversation, and haven't even been trying
to follow recent developments.


> 	13 modulation types;

Here I see missing  QAM1024  and  QAM4096 .


> 	7 transmission modes;
> 	7 bandwidths;

Apparently DVB-C2 allows us any bandwidth from 8MHz to 450MHz,
rather than the discrete values used by the other systems.
If this is also applicable to other countries with 6MHz rasters,
would it be necessary in addition to specify carrier spacing,
either 2,232kHz or 1,674kHz as opposed to getting this from the
channel bandwidth?


> 	8 guard intervals (including AUTO);

Here I observe the absence of  1/64 .


> 	5 hierarchy names;
> 	4 rolloff's (probably, we'll need to add 2 more, to distinguish between$


Of course, I'm just pointing out what I find, as I really don't
know anything about the transport systems, and someone who 
actually does might be able to say more, and correct my errors.

So just ignore me -- I'd rather see these values added sooner
than later if needed.  Apparently the broadcasts from Borups
Allé scheduled to start sometime around now will be switching
over to use those mentioned to test their increased robustness.


thanks,
barry bouwsma



-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Key found at http://vax.chrillesen.dk/~barry/gpg.key
Comment: Anständige Signaturen und Verschlüsselung gibt es nur mit GNUpg
Comment: und nicht mit De-Mail.

iEYEARECAAYFAk69XqgACgkQPTWIZbDoOFfp0gCcDOxKlVrjbfGtEMLqNZ/Jkqkk
ngsAn3hoMOF5rPkqzZKD2QnDTifA2+of
=vN6k
-----END PGP SIGNATURE-----

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 11-11-2011 15:43, BOUWSMA Barry escreveu:
> 
> On Do (Donnerstag) 10.Nov (November) 2011, 22:20,  Mauro Carvalho Chehab wrote:
> 
>> We should also think on a way to enumerate the supported values for each DVB $
>> the enum fe_caps is not enough anymore to store everything. It currently has $
>> filled (of a total of 32 bits), and we currently have:
>> 	12 code rates (including AUTO/NONE);
> 
> I'm probably not looking at the correct source, but the numbers
> seem to match, so I'll just note that in what I'm looking at,
> there are missing the values  1/3  and  2/5 .

Those were not added yet, as no driver currently uses it.

> 
> But I have to apologise in that I've also not been paying
> attention to this conversation, and haven't even been trying
> to follow recent developments.
> 
> 
>> 	13 modulation types;
> 
> Here I see missing  QAM1024  and  QAM4096 .

Same here.

> 
> 
>> 	7 transmission modes;
>> 	7 bandwidths;
> 
> Apparently DVB-C2 allows us any bandwidth from 8MHz to 450MHz,
> rather than the discrete values used by the other systems.
> If this is also applicable to other countries with 6MHz rasters,
> would it be necessary in addition to specify carrier spacing,
> either 2,232kHz or 1,674kHz as opposed to getting this from the
> channel bandwidth?

There are 3 parameters for Satellite and Cable systems:
	- Roll off factor;
	- Symbol Rate;
	- Bandwidth.

Only two of the tree are independent, as the spec defines:
	Bandwidth = symbol rate * (1  + roll off).

For DVB-C Annex A and C, roll off is fixed (0.15 and 0.13, respectively).

ITU-T J 83 Annex B doesn't say anything about it, but the ANSI SCTE07 spec
says that the roll-off is approx. 0.18 for 256-QAM and approx. 0.12 for
256-QAM.

DVB-S also has a fixed roll-off of 0.35, while DVB-S2 allows configuring it.

Not 100% sure, but ISDB-S also seems to use a per-modulation roll-off factor.

Anyway, when the roll-off is known, only symbol rate is needed, in order
to know the needed bandwidth.

IMHO, we should add some function inside the DVB core to calculate the
bandwidth for DVB-C (and DVB-C2), as the tuner saw filters require it,
in order to filter spurious frequencies from adjacent channels. Some
demods may also need such info.

The DVBv5 API doesn't impose any step for the carrier value, as it is
specified in Hz. So, I don't think that any change would be needed at 
the userspace API, in order to support DVB-C2 "continuous" carrier spacing.

> 
> 
>> 	8 guard intervals (including AUTO);
> 
> Here I observe the absence of  1/64 .

Same here: currently, no driver implements it.

> 
> 
>> 	5 hierarchy names;
>> 	4 rolloff's (probably, we'll need to add 2 more, to distinguish between$
> 
> 
> Of course, I'm just pointing out what I find, as I really don't
> know anything about the transport systems, and someone who 
> actually does might be able to say more, and correct my errors.
> 
> So just ignore me -- I'd rather see these values added sooner
> than later if needed.  Apparently the broadcasts from Borups
> Allé scheduled to start sometime around now will be switching
> over to use those mentioned to test their increased robustness.

Implementing those parameters is not a matter of just adding new stuff at
the core. Developers need DVB-C2 capable hardware, and access to a broadcaster
using it (or access to some testing facility where DVB-C2 could be
simulated).

Regards,
Mauro

Re: PATCH: Query DVB frontend capabilities

[Andreas Oberritter]

On 11.11.2011 18:14, Mauro Carvalho Chehab wrote:
> Em 11-11-2011 13:06, Andreas Oberritter escreveu:
>> On 11.11.2011 15:43, Mauro Carvalho Chehab wrote:
>>> IMHO, the better is to set all parameters via stb0899_get_property(). We should
>>> work on deprecate the old way, as, by having all frontends implementing the
>>> get/set property ops, we can remove part of the legacy code inside the DVB core.
>>
>> I'm not sure what "legacy code" you're referring to. If you're referring
>> to the big switch in dtv_property_process_get(), which presets output
>> values based on previously set tuning parameters, then no, please don't
>> deprecate it. It doesn't improve driver code if you move this switch
>> down into every driver.
> 
> What I mean is that drivers should get rid of implementing get_frontend() and 
> set_frontend(), restricting the usage of struct dvb_frontend_parameters for DVBv3
> calls from userspace.

This would generate quite some work without much benefit.

> In other words, it should be part of the core logic to get all the parameters
> passed from userspace and passing them via one single call to something similar
> to set_property.

That's exactly what we have now with the set_frontend, tune, search and
track callbacks.

> In other words, ideally, the implementation for DTV set should be
> like:
> 
> static int dtv_property_process_set(struct dvb_frontend *fe,
> 				    struct dtv_property *tvp,
> 				    struct file *file)
> {
> 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> 
> 	switch(tvp->cmd) {
> 	case DTV_CLEAR:
> 		dvb_frontend_clear_cache(fe);
> 		break;
> 	case DTV_FREQUENCY:
> 		c->frequency = tvp->u.data;
> 		break;
> 	case DTV_MODULATION:
> 		c->modulation = tvp->u.data;
> 		break;
> 	case DTV_BANDWIDTH_HZ:
> 		c->bandwidth_hz = tvp->u.data;
> 		break;
> ...
> 	case DTV_TUNE:
> 		/* interpret the cache of data */
> 		if (fe->ops.new_set_frontend) {
> 			r = fe->ops.new_set_frontend(fe);
> 			if (r < 0)
> 				return r;
> 		}

set_frontend is called by the frontend thread, multiple times with
alternating parameters if necessary. Depending on the tuning algorithm,
drivers may implement tune or search and track instead. You cannot just
call a "new_set_frontend" driver function from here and expect it to
work as before.

> 		break;
> 
> E. g. instead of using the struct dvb_frontend_parameters, the drivers would
> use struct dtv_frontend_properties (already stored at the frontend
> struct as fe->dtv_property_cache).

Drivers are already free to ignore dvb_frontend_parameters and use the
properties stored in dtv_property_cache today.

> Btw, with such change, it would actually make sense the original proposal
> from Manu of having a separate callback for supported delivery systems.

Why? How does setting parameters relate to querying capabilies?

Regards,
Andreas

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 11-11-2011 18:09, Andreas Oberritter escreveu:
> On 11.11.2011 18:14, Mauro Carvalho Chehab wrote:
>> Em 11-11-2011 13:06, Andreas Oberritter escreveu:
>>> On 11.11.2011 15:43, Mauro Carvalho Chehab wrote:
>>>> IMHO, the better is to set all parameters via stb0899_get_property(). We should
>>>> work on deprecate the old way, as, by having all frontends implementing the
>>>> get/set property ops, we can remove part of the legacy code inside the DVB core.
>>>
>>> I'm not sure what "legacy code" you're referring to. If you're referring
>>> to the big switch in dtv_property_process_get(), which presets output
>>> values based on previously set tuning parameters, then no, please don't
>>> deprecate it. It doesn't improve driver code if you move this switch
>>> down into every driver.
>>
>> What I mean is that drivers should get rid of implementing get_frontend() and 
>> set_frontend(), restricting the usage of struct dvb_frontend_parameters for DVBv3
>> calls from userspace.
> 
> This would generate quite some work without much benefit.

Some effort is indeed needed. There are some benefits, though. Tests I made
when writing a v5 library showed some hard to fix bugs with the current way:

	1) DVB-C Annex C is currently broken, as there's no way to select it
with the current API. A fix for it is simple, but requires adding one more
parameter for example, to represent the roll-off (Annex C uses 0.13 for roll-off, 
instead of 0.15 for Annex A);

	2) The *legacy*() calls at the code don't handle well ATSC x ANNEX B, e. g.
a get after a set returns the wrong delivery system.

Ok, we may be able to find some workarounds, but that means adding some hacks at
the core.

>> In other words, it should be part of the core logic to get all the parameters
>> passed from userspace and passing them via one single call to something similar
>> to set_property.
> 
> That's exactly what we have now with the set_frontend, tune, search and
> track callbacks.

Yes, except that the same information is passed twice at the driver for DVB-C,
DVB-S, DVB-T and ATSC/J.83 Annex B.

The core needs to duplicate everything into the legacy structure, as it assumes
that the drivers could use the legacy stuff.

>> In other words, ideally, the implementation for DTV set should be
>> like:
>>
>> static int dtv_property_process_set(struct dvb_frontend *fe,
>> 				    struct dtv_property *tvp,
>> 				    struct file *file)
>> {
>> 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>>
>> 	switch(tvp->cmd) {
>> 	case DTV_CLEAR:
>> 		dvb_frontend_clear_cache(fe);
>> 		break;
>> 	case DTV_FREQUENCY:
>> 		c->frequency = tvp->u.data;
>> 		break;
>> 	case DTV_MODULATION:
>> 		c->modulation = tvp->u.data;
>> 		break;
>> 	case DTV_BANDWIDTH_HZ:
>> 		c->bandwidth_hz = tvp->u.data;
>> 		break;
>> ...
>> 	case DTV_TUNE:
>> 		/* interpret the cache of data */
>> 		if (fe->ops.new_set_frontend) {
>> 			r = fe->ops.new_set_frontend(fe);
>> 			if (r < 0)
>> 				return r;
>> 		}
> 
> set_frontend is called by the frontend thread, multiple times with
> alternating parameters if necessary. Depending on the tuning algorithm,
> drivers may implement tune or search and track instead. You cannot just
> call a "new_set_frontend" driver function from here and expect it to
> work as before.

I know. The solution is not as simple as the above.

>> 		break;
>>
>> E. g. instead of using the struct dvb_frontend_parameters, the drivers would
>> use struct dtv_frontend_properties (already stored at the frontend
>> struct as fe->dtv_property_cache).
> 
> Drivers are already free to ignore dvb_frontend_parameters and use the
> properties stored in dtv_property_cache today.

True, and they do that already, but this still data needs to be copied
twice. There is also a problem with this approach on get_properties:
as some drivers may be storing returned data into dvb_frontend_parameters, while
others may be storing at dtv_frontend_properties, the code will need to know
what data is reliable and what data should be discarded.

The current code assumes that "legacy" delivery systems will always return data
via dtv_frontend_properties.

Btw, the emulation code is currently broken for ISDB-T and DVB-T2: both emulate
a DVB-T delivery system, so, at the DVB structure info.type = FE_OFDM.

If you look at the code, you'll see things like:

...
	switch (fe->ops.info.type) {
...
	case FE_OFDM:
		c->delivery_system = SYS_DVBT;
		break;
...

static void dtv_property_cache_sync(struct dvb_frontend *fe,
...
case FE_OFDM:
                if (p->u.ofdm.bandwidth == BANDWIDTH_6_MHZ)
                       	c->bandwidth_hz = 6000000;
                else if (p->u.ofdm.bandwidth == BANDWIDTH_7_MHZ)
                        c->bandwidth_hz = 7000000;
                else if (p->u.ofdm.bandwidth == BANDWIDTH_8_MHZ)
                        c->bandwidth_hz = 8000000;
               	else
                    	/* Including BANDWIDTH_AUTO */
                        c->bandwidth_hz = 0;
                c->code_rate_HP = p->u.ofdm.code_rate_HP;
                c->code_rate_LP = p->u.ofdm.code_rate_LP;
                c->modulation = p->u.ofdm.constellation;
                c->transmission_mode = p->u.ofdm.transmission_mode;
                c->guard_interval = p->u.ofdm.guard_interval;
                c->hierarchy = p->u.ofdm.hierarchy_information;
                break;

So, even a pure ISDB-T driver will need to change DVB-T u.ofdm.*, as touching
at c->* will be discarded by dtv_property_cache_sync().

The same thing will also occur with all 2 GEN drivers that fill info.type.

Such behavior is ugly, and not expected, and may lead into hard to detect
bugs, as the driver code will look right, but won't behave as expected.

The thing is that those emulation stuff are broken, and fixing it will probably
be more complex than a simple scriptable change applied at the drivers that would
replace the union by a direct access to the cache info. On most drivers, the change 
is as simple as:
	s/p->u.ofdm./c->/
	s/p->u.qpsk./c->/
	s/p->u.vsm./c->/
	s/p->u.qam./c->/

>> Btw, with such change, it would actually make sense the original proposal
>> from Manu of having a separate callback for supported delivery systems.
> 
> Why? How does setting parameters relate to querying capabilies?

Because, after such change, set_property() could likely be removed.

Anyway, first things first. Let's apply Manu's patches (after the proposed changes),
and then work on it.

> 
> Regards,
> Andreas
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: PATCH: Query DVB frontend capabilities

[Manu Abraham]

On Fri, Nov 11, 2011 at 3:42 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 11-11-2011 04:26, Manu Abraham escreveu:
>> On Fri, Nov 11, 2011 at 2:50 AM, Mauro Carvalho Chehab
>> <mchehab@redhat.com> wrote:
>>> Em 10-11-2011 13:30, Manu Abraham escreveu:
>> The purpose of the patch is to
>> query DVB delivery system capabilities alone, rather than DVB frontend
>> info/capability.
>>
>> Attached is a revised version 2 of the patch, which addresses the
>> issues that were raised.
>
> It looks good for me. I would just rename it to DTV_SUPPORTED_DELIVERY.
> Please, when submitting upstream, don't forget to increment DVB version and
> add touch at DocBook, in order to not increase the gap between API specs and the
> implementation.

Ok, thanks for the feedback, will do that.

The naming issue is trivial. I would like to have a shorter name
rather that SUPPORTED. CAPS would have been ideal, since it refers to
device capability.

Regards,
Manu

Re: PATCH: Query DVB frontend capabilities

[Manu Abraham]

On Fri, Nov 11, 2011 at 8:00 PM, Andreas Oberritter <obi@linuxtv.org> wrote:
> On 11.11.2011 07:26, Manu Abraham wrote:
>> diff -r b6eb04718aa9 linux/drivers/media/dvb/dvb-core/dvb_frontend.c
>> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c Wed Nov 09 19:52:36 2011 +0530
>> +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c Fri Nov 11 06:05:40 2011 +0530
>> @@ -973,6 +973,8 @@
>>       _DTV_CMD(DTV_GUARD_INTERVAL, 0, 0),
>>       _DTV_CMD(DTV_TRANSMISSION_MODE, 0, 0),
>>       _DTV_CMD(DTV_HIERARCHY, 0, 0),
>> +
>> +     _DTV_CMD(DTV_DELIVERY_CAPS, 0, 0),
>>  };
>>
>>  static void dtv_property_dump(struct dtv_property *tvp)
>> @@ -1226,7 +1228,11 @@
>>               c = &cdetected;
>>       }
>>
>> +     dprintk("%s\n", __func__);
>> +
>>       switch(tvp->cmd) {
>> +     case DTV_DELIVERY_CAPS:
>
> It would be nice to have a default implementation inserted at this point, e.g. something like:
>
> static void dtv_set_default_delivery_caps(const struct dvb_frontend *fe, struct dtv_property *p)
> {
>        const struct dvb_frontend_info *info = &fe->ops.info;
>        u32 ncaps = 0;
>
>        switch (info->type) {
>        case FE_QPSK:
>                p->u.buffer.data[ncaps++] = SYS_DVBS;
>                if (info->caps & FE_CAN_2G_MODULATION)
>                        p->u.buffer.data[ncaps++] = SYS_DVBS2;
>                if (info->caps & FE_CAN_TURBO_FEC)
>                        p->u.buffer.data[ncaps++] = SYS_TURBO;
>                break;
>        case FE_QAM:
>                p->u.buffer.data[ncaps++] = SYS_DVBC_ANNEX_AC;
>                break;
>        case FE_OFDM:
>                p->u.buffer.data[ncaps++] = SYS_DVBT;
>                if (info->caps & FE_CAN_2G_MODULATION)
>                        p->u.buffer.data[ncaps++] = SYS_DVBT2;
>                break;
>        case FE_ATSC:
>                if (info->caps & (FE_CAN_8VSB | FE_CAN_16VSB))
>                        p->u.buffer.data[ncaps++] = SYS_ATSC;
>                if (info->caps & (FE_CAN_QAM_16 | FE_CAN_QAM_64 | FE_CAN_QAM_128 | FE_CAN_QAM_256))
>                        p->u.buffer.data[ncaps++] = SYS_DVBC_ANNEX_B;
>        }
>
>        p->u.buffer.len = ncaps;
> }
>
> I think this would be sufficient for a lot of drivers and would thus save a lot of work.


Ahhh, I get what you mean.


>
>> +             break;
>>       case DTV_FREQUENCY:
>>               tvp->u.data = c->frequency;
>>               break;
>> @@ -1350,7 +1356,7 @@
>>               if (r < 0)
>>                       return r;
>>       }
>> -
>> +done:
>
> This label is unused now and should be removed.


True.

>>       dtv_property_dump(tvp);
>>
>>       return 0;
>> diff -r b6eb04718aa9 linux/drivers/media/dvb/frontends/stb0899_drv.c
>> --- a/linux/drivers/media/dvb/frontends/stb0899_drv.c Wed Nov 09 19:52:36 2011 +0530
>> +++ b/linux/drivers/media/dvb/frontends/stb0899_drv.c Fri Nov 11 06:05:40 2011 +0530
>> @@ -1605,6 +1605,22 @@
>>       return DVBFE_ALGO_CUSTOM;
>>  }
>>
>> +static int stb0899_get_property(struct dvb_frontend *fe, struct dtv_property *p)
>> +{
>> +     switch (p->cmd) {
>> +     case DTV_DELIVERY_CAPS:
>> +             p->u.buffer.data[0] = SYS_DSS;
>> +             p->u.buffer.data[1] = SYS_DVBS;
>> +             p->u.buffer.data[2] = SYS_DVBS2;
>> +             p->u.buffer.len = 3;
>> +             break;
>> +     default:
>> +             return -EINVAL;
>
> You should ignore all unhandled properties. Otherwise all properties handled
> by the core will have result set to -EINVAL.


Ah yes, much true. Didn't think of that issue, Will fix.


>
>> +     }
>> +     return 0;
>> +}
>> +
>> +
>>  static struct dvb_frontend_ops stb0899_ops = {
>>
>>       .info = {
>> @@ -1647,6 +1663,8 @@
>>       .diseqc_send_master_cmd         = stb0899_send_diseqc_msg,
>>       .diseqc_recv_slave_reply        = stb0899_recv_slave_reply,
>>       .diseqc_send_burst              = stb0899_send_diseqc_burst,
>> +
>> +     .get_property                   = stb0899_get_property,
>>  };
>>
>>  struct dvb_frontend *stb0899_attach(struct stb0899_config *config, struct i2c_adapter *i2c)
>> diff -r b6eb04718aa9 linux/include/linux/dvb/frontend.h
>> --- a/linux/include/linux/dvb/frontend.h      Wed Nov 09 19:52:36 2011 +0530
>> +++ b/linux/include/linux/dvb/frontend.h      Fri Nov 11 06:05:40 2011 +0530
>> @@ -316,7 +316,9 @@
>>
>>  #define DTV_DVBT2_PLP_ID     43
>>
>> -#define DTV_MAX_COMMAND                              DTV_DVBT2_PLP_ID
>> +#define DTV_DELIVERY_CAPS    44
>> +
>> +#define DTV_MAX_COMMAND                              DTV_DELIVERY_CAPS
>>
>>  typedef enum fe_pilot {
>>       PILOT_ON,
>>


Thanks for the feedback.

Regards,
Manu

Re: PATCH: Query DVB frontend capabilities

[Manu Abraham]

On Sat, Nov 12, 2011 at 12:07 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 11-11-2011 15:43, BOUWSMA Barry escreveu:
>>
>> On Do (Donnerstag) 10.Nov (November) 2011, 22:20,  Mauro Carvalho Chehab wrote:
>>
>>> We should also think on a way to enumerate the supported values for each DVB $
>>> the enum fe_caps is not enough anymore to store everything. It currently has $
>>> filled (of a total of 32 bits), and we currently have:
>>>      12 code rates (including AUTO/NONE);
>>
>> I'm probably not looking at the correct source, but the numbers
>> seem to match, so I'll just note that in what I'm looking at,
>> there are missing the values  1/3  and  2/5 .
>
> Those were not added yet, as no driver currently uses it.
>
>>
>> But I have to apologise in that I've also not been paying
>> attention to this conversation, and haven't even been trying
>> to follow recent developments.
>>
>>
>>>      13 modulation types;
>>
>> Here I see missing  QAM1024  and  QAM4096 .
>
> Same here.
>
>>
>>
>>>      7 transmission modes;
>>>      7 bandwidths;
>>
>> Apparently DVB-C2 allows us any bandwidth from 8MHz to 450MHz,
>> rather than the discrete values used by the other systems.
>> If this is also applicable to other countries with 6MHz rasters,
>> would it be necessary in addition to specify carrier spacing,
>> either 2,232kHz or 1,674kHz as opposed to getting this from the
>> channel bandwidth?
>
> There are 3 parameters for Satellite and Cable systems:
>        - Roll off factor;
>        - Symbol Rate;
>        - Bandwidth.
>
> Only two of the tree are independent, as the spec defines:
>        Bandwidth = symbol rate * (1  + roll off).
>
> For DVB-C Annex A and C, roll off is fixed (0.15 and 0.13, respectively).
>
> ITU-T J 83 Annex B doesn't say anything about it, but the ANSI SCTE07 spec
> says that the roll-off is approx. 0.18 for 256-QAM and approx. 0.12 for
> 256-QAM.
>
> DVB-S also has a fixed roll-off of 0.35, while DVB-S2 allows configuring it.


DVB-S uses 3 different rolloffs just like DVB-S2. In fact the rolloff
doesn't have anything to do with the delivery system at all, but
something that which is independant and physical to the channel,
rather than something logical such as a delivery system, looking at a
satellite transponder's viewpoint.

For general (home) broadcast purposes, we use only 0.35. There are
many other usages, which is not yet applicable with especially Linux
DVB with regards to narrow band operations such as DVB feeds and DSNG.

There are many usages for the second generation delivery systems,
which will likely realize only a very small part.

Eg: there are other aspects to DVB-S2 such as ACM and VCM, which most
likely we wouldn't cover. the reason is that the users of it are most
likely propreitary users of it and neither would they prefer to have
an open source version for it, nor would any Open Source users be
likely to implement it. Eg: Ericson's Director CA system, where they
have complete control over the box, rather than the user. As far as I
am aware they have a return path as well.

>
> Not 100% sure, but ISDB-S also seems to use a per-modulation roll-off factor.
>
> Anyway, when the roll-off is known, only symbol rate is needed, in order
> to know the needed bandwidth.


You need to know FEC, modulation too .. Eg: If you have 16APSK where
you have 4 bits, in comparison to 3 bits as with 8PSK, then you
require lesser bandwidth.

Also, higher the Error correction information bits set in (redundant),
the more bandwidth it needs to occupy. This is the same with any
Digital Channel whether it be Satellite/Cable/Terrestrial, whatever
delivery system it is.

Regards,
Manu

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 11-11-2011 20:07, Manu Abraham escreveu:
> On Fri, Nov 11, 2011 at 3:42 PM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> Em 11-11-2011 04:26, Manu Abraham escreveu:
>>> On Fri, Nov 11, 2011 at 2:50 AM, Mauro Carvalho Chehab
>>> <mchehab@redhat.com> wrote:
>>>> Em 10-11-2011 13:30, Manu Abraham escreveu:
>>> The purpose of the patch is to
>>> query DVB delivery system capabilities alone, rather than DVB frontend
>>> info/capability.
>>>
>>> Attached is a revised version 2 of the patch, which addresses the
>>> issues that were raised.
>>
>> It looks good for me. I would just rename it to DTV_SUPPORTED_DELIVERY.
>> Please, when submitting upstream, don't forget to increment DVB version and
>> add touch at DocBook, in order to not increase the gap between API specs and the
>> implementation.
> 
> Ok, thanks for the feedback, will do that.
> 
> The naming issue is trivial. I would like to have a shorter name
> rather that SUPPORTED. CAPS would have been ideal, since it refers to
> device capability.

CAPS is not a good name, as there are those two CAPABILITIES calls there
(well, currently not implemented). So, it can lead in to some confusion.

DTV_ENUM_DELIVERY could be an alternative for a short name to be used there.

> 
> Regards,
> Manu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: PATCH: Query DVB frontend capabilities

[Andreas Oberritter]

On 11.11.2011 23:38, Mauro Carvalho Chehab wrote:
> Em 11-11-2011 20:07, Manu Abraham escreveu:
>> On Fri, Nov 11, 2011 at 3:42 PM, Mauro Carvalho Chehab
>> <mchehab@redhat.com> wrote:
>>> Em 11-11-2011 04:26, Manu Abraham escreveu:
>>>> On Fri, Nov 11, 2011 at 2:50 AM, Mauro Carvalho Chehab
>>>> <mchehab@redhat.com> wrote:
>>>>> Em 10-11-2011 13:30, Manu Abraham escreveu:
>>>> The purpose of the patch is to
>>>> query DVB delivery system capabilities alone, rather than DVB frontend
>>>> info/capability.
>>>>
>>>> Attached is a revised version 2 of the patch, which addresses the
>>>> issues that were raised.
>>>
>>> It looks good for me. I would just rename it to DTV_SUPPORTED_DELIVERY.
>>> Please, when submitting upstream, don't forget to increment DVB version and
>>> add touch at DocBook, in order to not increase the gap between API specs and the
>>> implementation.
>>
>> Ok, thanks for the feedback, will do that.
>>
>> The naming issue is trivial. I would like to have a shorter name
>> rather that SUPPORTED. CAPS would have been ideal, since it refers to
>> device capability.
> 
> CAPS is not a good name, as there are those two CAPABILITIES calls there
> (well, currently not implemented). So, it can lead in to some confusion.
> 
> DTV_ENUM_DELIVERY could be an alternative for a short name to be used there.

I like "enum", because it suggests that it's a read-only property.

DVB calls them "delivery systems", so maybe DTV_ENUM_DELSYS may be an
alternative.

Regards,
Andreas

Re: PATCH: Query DVB frontend capabilities

[Andreas Oberritter]

On 11.11.2011 23:02, Mauro Carvalho Chehab wrote:
> Em 11-11-2011 18:09, Andreas Oberritter escreveu:
>> On 11.11.2011 18:14, Mauro Carvalho Chehab wrote:
>>> Em 11-11-2011 13:06, Andreas Oberritter escreveu:
>>>> On 11.11.2011 15:43, Mauro Carvalho Chehab wrote:
>>>>> IMHO, the better is to set all parameters via stb0899_get_property(). We should
>>>>> work on deprecate the old way, as, by having all frontends implementing the
>>>>> get/set property ops, we can remove part of the legacy code inside the DVB core.
>>>>
>>>> I'm not sure what "legacy code" you're referring to. If you're referring
>>>> to the big switch in dtv_property_process_get(), which presets output
>>>> values based on previously set tuning parameters, then no, please don't
>>>> deprecate it. It doesn't improve driver code if you move this switch
>>>> down into every driver.
>>>
>>> What I mean is that drivers should get rid of implementing get_frontend() and 
>>> set_frontend(), restricting the usage of struct dvb_frontend_parameters for DVBv3
>>> calls from userspace.
>>
>> This would generate quite some work without much benefit.
> 
> Some effort is indeed needed. There are some benefits, though. Tests I made
> when writing a v5 library showed some hard to fix bugs with the current way:
> 
> 	1) DVB-C Annex C is currently broken, as there's no way to select it
> with the current API. A fix for it is simple, but requires adding one more
> parameter for example, to represent the roll-off (Annex C uses 0.13 for roll-off, 
> instead of 0.15 for Annex A);

An alternative would be to rename SYS_DVBC_ANNEX_AC to SYS_DVBC_ANNEX_A,
add SYS_DVBC_ANNEX_C, and add SYS_DVBC_ANNEX_AC as a define for
backwards compatibility.

> 
> 	2) The *legacy*() calls at the code don't handle well ATSC x ANNEX B, e. g.
> a get after a set returns the wrong delivery system.

Do you know what exactly is wrong with it?

> Ok, we may be able to find some workarounds, but that means adding some hacks at
> the core.
> 
>>> In other words, it should be part of the core logic to get all the parameters
>>> passed from userspace and passing them via one single call to something similar
>>> to set_property.
>>
>> That's exactly what we have now with the set_frontend, tune, search and
>> track callbacks.
> 
> Yes, except that the same information is passed twice at the driver for DVB-C,
> DVB-S, DVB-T and ATSC/J.83 Annex B.
> 
> The core needs to duplicate everything into the legacy structure, as it assumes
> that the drivers could use the legacy stuff.

Yes.

>>> In other words, ideally, the implementation for DTV set should be
>>> like:
>>>
>>> static int dtv_property_process_set(struct dvb_frontend *fe,
>>> 				    struct dtv_property *tvp,
>>> 				    struct file *file)
>>> {
>>> 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>>>
>>> 	switch(tvp->cmd) {
>>> 	case DTV_CLEAR:
>>> 		dvb_frontend_clear_cache(fe);
>>> 		break;
>>> 	case DTV_FREQUENCY:
>>> 		c->frequency = tvp->u.data;
>>> 		break;
>>> 	case DTV_MODULATION:
>>> 		c->modulation = tvp->u.data;
>>> 		break;
>>> 	case DTV_BANDWIDTH_HZ:
>>> 		c->bandwidth_hz = tvp->u.data;
>>> 		break;
>>> ...
>>> 	case DTV_TUNE:
>>> 		/* interpret the cache of data */
>>> 		if (fe->ops.new_set_frontend) {
>>> 			r = fe->ops.new_set_frontend(fe);
>>> 			if (r < 0)
>>> 				return r;
>>> 		}
>>
>> set_frontend is called by the frontend thread, multiple times with
>> alternating parameters if necessary. Depending on the tuning algorithm,
>> drivers may implement tune or search and track instead. You cannot just
>> call a "new_set_frontend" driver function from here and expect it to
>> work as before.
> 
> I know. The solution is not as simple as the above.
> 
>>> 		break;
>>>
>>> E. g. instead of using the struct dvb_frontend_parameters, the drivers would
>>> use struct dtv_frontend_properties (already stored at the frontend
>>> struct as fe->dtv_property_cache).
>>
>> Drivers are already free to ignore dvb_frontend_parameters and use the
>> properties stored in dtv_property_cache today.
> 
> True, and they do that already, but this still data needs to be copied
> twice. There is also a problem with this approach on get_properties:
> as some drivers may be storing returned data into dvb_frontend_parameters, while
> others may be storing at dtv_frontend_properties, the code will need to know
> what data is reliable and what data should be discarded.
> 
> The current code assumes that "legacy" delivery systems will always return data
> via dtv_frontend_properties.
> 
> Btw, the emulation code is currently broken for ISDB-T and DVB-T2: both emulate
> a DVB-T delivery system, so, at the DVB structure info.type = FE_OFDM.
> 
> If you look at the code, you'll see things like:
> 
> ...
> 	switch (fe->ops.info.type) {
> ...
> 	case FE_OFDM:
> 		c->delivery_system = SYS_DVBT;
> 		break;
> ...

This code path only gets executed when calling FE_SET_FRONTEND from
userspace. There's no DVB-T2 support in v3, so this should be ok.

> static void dtv_property_cache_sync(struct dvb_frontend *fe,
> ...
> case FE_OFDM:
>                 if (p->u.ofdm.bandwidth == BANDWIDTH_6_MHZ)
>                        	c->bandwidth_hz = 6000000;
>                 else if (p->u.ofdm.bandwidth == BANDWIDTH_7_MHZ)
>                         c->bandwidth_hz = 7000000;
>                 else if (p->u.ofdm.bandwidth == BANDWIDTH_8_MHZ)
>                         c->bandwidth_hz = 8000000;
>                	else
>                     	/* Including BANDWIDTH_AUTO */
>                         c->bandwidth_hz = 0;
>                 c->code_rate_HP = p->u.ofdm.code_rate_HP;
>                 c->code_rate_LP = p->u.ofdm.code_rate_LP;
>                 c->modulation = p->u.ofdm.constellation;
>                 c->transmission_mode = p->u.ofdm.transmission_mode;
>                 c->guard_interval = p->u.ofdm.guard_interval;
>                 c->hierarchy = p->u.ofdm.hierarchy_information;
>                 break;
> 
> So, even a pure ISDB-T driver will need to change DVB-T u.ofdm.*, as touching
> at c->* will be discarded by dtv_property_cache_sync().

Why do ISDB-T drivers pretend to be DVB-T drivers by specifying FE_OFDM?
Even though it's called FE_OFDM, it really means "DVB-T" and not "any
delivery system using OFDM".

ISDB-T doesn't have a v3 interface, so ISDB-T drivers shouldn't
implement the legacy get_frontend callback, in which case
dtv_property_cache_sync won't be called. Furthermore, a driver may set
all properties in its get_property callback, whether
dtv_property_cache_sync was called or not.

> The same thing will also occur with all 2 GEN drivers that fill info.type.
> 
> Such behavior is ugly, and not expected, and may lead into hard to detect
> bugs, as the driver code will look right, but won't behave as expected.
> 
> The thing is that those emulation stuff are broken, and fixing it will probably
> be more complex than a simple scriptable change applied at the drivers that would
> replace the union by a direct access to the cache info. On most drivers, the change 
> is as simple as:
> 	s/p->u.ofdm./c->/
> 	s/p->u.qpsk./c->/
> 	s/p->u.vsm./c->/
> 	s/p->u.qam./c->/
> 
>>> Btw, with such change, it would actually make sense the original proposal
>>> from Manu of having a separate callback for supported delivery systems.
>>
>> Why? How does setting parameters relate to querying capabilies?
> 
> Because, after such change, set_property() could likely be removed.

But how does this relate to get_property? set_property isn't used to
query capabilities.

Regards,
Andreas

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 12-11-2011 01:36, Andreas Oberritter escreveu:
> On 11.11.2011 23:38, Mauro Carvalho Chehab wrote:
>> Em 11-11-2011 20:07, Manu Abraham escreveu:
>>> On Fri, Nov 11, 2011 at 3:42 PM, Mauro Carvalho Chehab
>>> <mchehab@redhat.com> wrote:
>>>> Em 11-11-2011 04:26, Manu Abraham escreveu:
>>>>> On Fri, Nov 11, 2011 at 2:50 AM, Mauro Carvalho Chehab
>>>>> <mchehab@redhat.com> wrote:
>>>>>> Em 10-11-2011 13:30, Manu Abraham escreveu:
>>>>> The purpose of the patch is to
>>>>> query DVB delivery system capabilities alone, rather than DVB frontend
>>>>> info/capability.
>>>>>
>>>>> Attached is a revised version 2 of the patch, which addresses the
>>>>> issues that were raised.
>>>>
>>>> It looks good for me. I would just rename it to DTV_SUPPORTED_DELIVERY.
>>>> Please, when submitting upstream, don't forget to increment DVB version and
>>>> add touch at DocBook, in order to not increase the gap between API specs and the
>>>> implementation.
>>>
>>> Ok, thanks for the feedback, will do that.
>>>
>>> The naming issue is trivial. I would like to have a shorter name
>>> rather that SUPPORTED. CAPS would have been ideal, since it refers to
>>> device capability.
>>
>> CAPS is not a good name, as there are those two CAPABILITIES calls there
>> (well, currently not implemented). So, it can lead in to some confusion.
>>
>> DTV_ENUM_DELIVERY could be an alternative for a short name to be used there.
> 
> I like "enum", because it suggests that it's a read-only property.
> 
> DVB calls them "delivery systems", so maybe DTV_ENUM_DELSYS may be an
> alternative.

DELSYS may give other interpretations. I don't think we should be so short
at the cmd name. There are already some on ISDB with very big names.

I would get either 

DTV_ENUM_DELIVSYS
 or
DTV_ENUM_DELIVERYSYS

(IMHO, DTV_ENUM_DELIVERYSYS is the better choice)

Regards,
Mauro

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 11-11-2011 20:34, Manu Abraham escreveu:
> On Sat, Nov 12, 2011 at 12:07 AM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> Em 11-11-2011 15:43, BOUWSMA Barry escreveu:
>>>
>>> On Do (Donnerstag) 10.Nov (November) 2011, 22:20,  Mauro Carvalho Chehab wrote:
>>>
>>>> We should also think on a way to enumerate the supported values for each DVB $
>>>> the enum fe_caps is not enough anymore to store everything. It currently has $
>>>> filled (of a total of 32 bits), and we currently have:
>>>>      12 code rates (including AUTO/NONE);
>>>
>>> I'm probably not looking at the correct source, but the numbers
>>> seem to match, so I'll just note that in what I'm looking at,
>>> there are missing the values  1/3  and  2/5 .
>>
>> Those were not added yet, as no driver currently uses it.
>>
>>>
>>> But I have to apologise in that I've also not been paying
>>> attention to this conversation, and haven't even been trying
>>> to follow recent developments.
>>>
>>>
>>>>      13 modulation types;
>>>
>>> Here I see missing  QAM1024  and  QAM4096 .
>>
>> Same here.
>>
>>>
>>>
>>>>      7 transmission modes;
>>>>      7 bandwidths;
>>>
>>> Apparently DVB-C2 allows us any bandwidth from 8MHz to 450MHz,
>>> rather than the discrete values used by the other systems.
>>> If this is also applicable to other countries with 6MHz rasters,
>>> would it be necessary in addition to specify carrier spacing,
>>> either 2,232kHz or 1,674kHz as opposed to getting this from the
>>> channel bandwidth?
>>
>> There are 3 parameters for Satellite and Cable systems:
>>        - Roll off factor;
>>        - Symbol Rate;
>>        - Bandwidth.
>>
>> Only two of the tree are independent, as the spec defines:
>>        Bandwidth = symbol rate * (1  + roll off).
>>
>> For DVB-C Annex A and C, roll off is fixed (0.15 and 0.13, respectively).
>>
>> ITU-T J 83 Annex B doesn't say anything about it, but the ANSI SCTE07 spec
>> says that the roll-off is approx. 0.18 for 256-QAM and approx. 0.12 for
>> 256-QAM.
>>
>> DVB-S also has a fixed roll-off of 0.35, while DVB-S2 allows configuring it.
> 
> 
> DVB-S uses 3 different rolloffs just like DVB-S2. In fact the rolloff
> doesn't have anything to do with the delivery system at all, but
> something that which is independant and physical to the channel,
> rather than something logical such as a delivery system, looking at a
> satellite transponder's viewpoint.
> 
> For general (home) broadcast purposes, we use only 0.35. There are
> many other usages, which is not yet applicable with especially Linux
> DVB with regards to narrow band operations such as DVB feeds and DSNG.

Ok.

> 
> There are many usages for the second generation delivery systems,
> which will likely realize only a very small part.
> 
> Eg: there are other aspects to DVB-S2 such as ACM and VCM, which most
> likely we wouldn't cover. the reason is that the users of it are most
> likely propreitary users of it and neither would they prefer to have
> an open source version for it, nor would any Open Source users be
> likely to implement it. Eg: Ericson's Director CA system, where they
> have complete control over the box, rather than the user. As far as I
> am aware they have a return path as well.
> 
>>
>> Not 100% sure, but ISDB-S also seems to use a per-modulation roll-off factor.
>>
>> Anyway, when the roll-off is known, only symbol rate is needed, in order
>> to know the needed bandwidth.
> 
> 
> You need to know FEC, modulation too .. Eg: If you have 16APSK where
> you have 4 bits, in comparison to 3 bits as with 8PSK, then you
> require lesser bandwidth.

Manu, you're probably thinking in terms of the TS bit rate, and not the
modulator symbol rate.

The number of bits don't matter here, as the symbol rate is specified
in bauds (e. g. symbols per second), and not in bits/s.

The conversion from bauds to bits/s is to multiply the number of bits per
symbol by the rate, in bauds.

A higher number of bits for a given modulation just increase the number of 
possible states that the modulation will use. So, it will require a higher
signal to noise relation at the demod, to avoid miss-detection, but this is
a separate issue.

The roll-off, minimal bandwidth (referred as "Nyquist frequency" by the DVB-C
specs) and symbol rate are related by this equation:
	f = symbol_rate * (1 + roll_off)

The f value is the Nyquist frequency, and it will dictate the low-pass filter
needed to confine the entire signal of the channel (with is, basically, the
amount of bandwidth required by the channel).

> Also, higher the Error correction information bits set in (redundant),
> the more bandwidth it needs to occupy.

The error correction algorithm will reduce the bit rate of the TS stream,
but won't affect the symbol rate at the modulator.

> This is the same with any
> Digital Channel whether it be Satellite/Cable/Terrestrial, whatever
> delivery system it is.

Yes.

> 
> Regards,
> Manu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: PATCH: Query DVB frontend capabilities

[Manu Abraham]

On Sun, Nov 13, 2011 at 7:02 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 11-11-2011 20:34, Manu Abraham escreveu:
>> On Sat, Nov 12, 2011 at 12:07 AM, Mauro Carvalho Chehab
>> <mchehab@redhat.com> wrote:
>>> Em 11-11-2011 15:43, BOUWSMA Barry escreveu:
>>>>
>>>> On Do (Donnerstag) 10.Nov (November) 2011, 22:20,  Mauro Carvalho Chehab wrote:
>>>>
>>>>> We should also think on a way to enumerate the supported values for each DVB $
>>>>> the enum fe_caps is not enough anymore to store everything. It currently has $
>>>>> filled (of a total of 32 bits), and we currently have:
>>>>>      12 code rates (including AUTO/NONE);
>>>>
>>>> I'm probably not looking at the correct source, but the numbers
>>>> seem to match, so I'll just note that in what I'm looking at,
>>>> there are missing the values  1/3  and  2/5 .
>>>
>>> Those were not added yet, as no driver currently uses it.
>>>
>>>>
>>>> But I have to apologise in that I've also not been paying
>>>> attention to this conversation, and haven't even been trying
>>>> to follow recent developments.
>>>>
>>>>
>>>>>      13 modulation types;
>>>>
>>>> Here I see missing  QAM1024  and  QAM4096 .
>>>
>>> Same here.
>>>
>>>>
>>>>
>>>>>      7 transmission modes;
>>>>>      7 bandwidths;
>>>>
>>>> Apparently DVB-C2 allows us any bandwidth from 8MHz to 450MHz,
>>>> rather than the discrete values used by the other systems.
>>>> If this is also applicable to other countries with 6MHz rasters,
>>>> would it be necessary in addition to specify carrier spacing,
>>>> either 2,232kHz or 1,674kHz as opposed to getting this from the
>>>> channel bandwidth?
>>>
>>> There are 3 parameters for Satellite and Cable systems:
>>>        - Roll off factor;
>>>        - Symbol Rate;
>>>        - Bandwidth.
>>>
>>> Only two of the tree are independent, as the spec defines:
>>>        Bandwidth = symbol rate * (1  + roll off).
>>>
>>> For DVB-C Annex A and C, roll off is fixed (0.15 and 0.13, respectively).
>>>
>>> ITU-T J 83 Annex B doesn't say anything about it, but the ANSI SCTE07 spec
>>> says that the roll-off is approx. 0.18 for 256-QAM and approx. 0.12 for
>>> 256-QAM.
>>>
>>> DVB-S also has a fixed roll-off of 0.35, while DVB-S2 allows configuring it.
>>
>>
>> DVB-S uses 3 different rolloffs just like DVB-S2. In fact the rolloff
>> doesn't have anything to do with the delivery system at all, but
>> something that which is independant and physical to the channel,
>> rather than something logical such as a delivery system, looking at a
>> satellite transponder's viewpoint.
>>
>> For general (home) broadcast purposes, we use only 0.35. There are
>> many other usages, which is not yet applicable with especially Linux
>> DVB with regards to narrow band operations such as DVB feeds and DSNG.
>
> Ok.
>
>>
>> There are many usages for the second generation delivery systems,
>> which will likely realize only a very small part.
>>
>> Eg: there are other aspects to DVB-S2 such as ACM and VCM, which most
>> likely we wouldn't cover. the reason is that the users of it are most
>> likely propreitary users of it and neither would they prefer to have
>> an open source version for it, nor would any Open Source users be
>> likely to implement it. Eg: Ericson's Director CA system, where they
>> have complete control over the box, rather than the user. As far as I
>> am aware they have a return path as well.
>>
>>>
>>> Not 100% sure, but ISDB-S also seems to use a per-modulation roll-off factor.
>>>
>>> Anyway, when the roll-off is known, only symbol rate is needed, in order
>>> to know the needed bandwidth.
>>
>>
>> You need to know FEC, modulation too .. Eg: If you have 16APSK where
>> you have 4 bits, in comparison to 3 bits as with 8PSK, then you
>> require lesser bandwidth.
>
> Manu, you're probably thinking in terms of the TS bit rate, and not the
> modulator symbol rate.
>
> The number of bits don't matter here, as the symbol rate is specified
> in bauds (e. g. symbols per second), and not in bits/s.


A PSK modulator is a state machine:
where states/symbols are logically represented by bits, given that the
state can either be a 0 or 1

BPSK  states=2    bits=1
QPSK  states=4    bits=2
8PSK  states=8     bits=3
16PSK states=16  bits=4
32PSK states=32  bits=5

http://en.wikipedia.org/wiki/Constellation_diagram
http://en.wikipedia.org/wiki/Gray_code

Symbol Rate is generally specified in SPS, ie Symbols/sec, or in
Bauds. AFAICS, We generally use Symbols Per Second rather than bauds.

http://www.asiasat.com/asiasat/contentView.php?section=69&lang=0
http://www.b4utv.com/subs/technology.shtml
http://www.skynewsinternational.com/watch/satellite-information

I haven't seen a demodulator specification which states Mbaud, but
have seen them stated as MSPS or kSPS.

Now, assuming a 36MHz TP as an example: The given bandwidth is better
or efficiently used by a higher order modulation. This is the reason
why DVB.org states that DVB-S2 saves 30% bandwidth.

Quoting you: "Anyway, when the roll-off is known, only symbol rate is
needed, in order to know the needed bandwidth."

Given a fixed TP CHBW, according to you: Channel Bandwidth can be
calculated by knowing Symbol Rate alone, with a known rolloff.

I say that this is not possible. Since the number of states/symbols
for any given channel depends on the modulation order as well.

I hope that clears up things for you.


> The conversion from bauds to bits/s is to multiply the number of bits per
> symbol by the rate, in bauds.
>
> A higher number of bits for a given modulation just increase the number of
> possible states that the modulation will use. So, it will require a higher
> signal to noise relation at the demod, to avoid miss-detection, but this is
> a separate issue.


That's why for higher order modulations, demodulators use better Error
Correction Schemes (eg: BCH/Turbo) when the modulation order is
higher.

http://en.wikipedia.org/wiki/Modulation_order
http://en.wikipedia.org/wiki/BCH_code


> The roll-off, minimal bandwidth (referred as "Nyquist frequency" by the DVB-C
> specs) and symbol rate are related by this equation:
>        f = symbol_rate * (1 + roll_off)
>
> The f value is the Nyquist frequency, and it will dictate the low-pass filter
> needed to confine the entire signal of the channel (with is, basically, the
> amount of bandwidth required by the channel).
>
>> Also, higher the Error correction information bits set in (redundant),
>> the more bandwidth it needs to occupy.
>
> The error correction algorithm will reduce the bit rate of the TS stream,
> but won't affect the symbol rate at the modulator.


No. That's an incorrect statement. FEC gives the receiver the ability
to correct errors without needing a reverse channel to request
retransmission of data, but at the cost of a fixed, higher forward
channel bandwidth.

http://en.wikipedia.org/wiki/Forward_error_correction


>> This is the same with any
>> Digital Channel whether it be Satellite/Cable/Terrestrial, whatever
>> delivery system it is.
>
> Yes.
>
>>
>> Regards,
>> Manu
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

Hope it helps to clear your understanding.
Manu

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 13-11-2011 13:27, Manu Abraham escreveu:
> On Sun, Nov 13, 2011 at 7:02 PM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> Em 11-11-2011 20:34, Manu Abraham escreveu:
>>> On Sat, Nov 12, 2011 at 12:07 AM, Mauro Carvalho Chehab
>>> <mchehab@redhat.com> wrote:
>>>> Em 11-11-2011 15:43, BOUWSMA Barry escreveu:
>>>>>
>>>>> On Do (Donnerstag) 10.Nov (November) 2011, 22:20,  Mauro Carvalho Chehab wrote:
>>>>>
>>>>>> We should also think on a way to enumerate the supported values for each DVB $
>>>>>> the enum fe_caps is not enough anymore to store everything. It currently has $
>>>>>> filled (of a total of 32 bits), and we currently have:
>>>>>>      12 code rates (including AUTO/NONE);
>>>>>
>>>>> I'm probably not looking at the correct source, but the numbers
>>>>> seem to match, so I'll just note that in what I'm looking at,
>>>>> there are missing the values  1/3  and  2/5 .
>>>>
>>>> Those were not added yet, as no driver currently uses it.
>>>>
>>>>>
>>>>> But I have to apologise in that I've also not been paying
>>>>> attention to this conversation, and haven't even been trying
>>>>> to follow recent developments.
>>>>>
>>>>>
>>>>>>      13 modulation types;
>>>>>
>>>>> Here I see missing  QAM1024  and  QAM4096 .
>>>>
>>>> Same here.
>>>>
>>>>>
>>>>>
>>>>>>      7 transmission modes;
>>>>>>      7 bandwidths;
>>>>>
>>>>> Apparently DVB-C2 allows us any bandwidth from 8MHz to 450MHz,
>>>>> rather than the discrete values used by the other systems.
>>>>> If this is also applicable to other countries with 6MHz rasters,
>>>>> would it be necessary in addition to specify carrier spacing,
>>>>> either 2,232kHz or 1,674kHz as opposed to getting this from the
>>>>> channel bandwidth?
>>>>
>>>> There are 3 parameters for Satellite and Cable systems:
>>>>        - Roll off factor;
>>>>        - Symbol Rate;
>>>>        - Bandwidth.
>>>>
>>>> Only two of the tree are independent, as the spec defines:
>>>>        Bandwidth = symbol rate * (1  + roll off).
>>>>
>>>> For DVB-C Annex A and C, roll off is fixed (0.15 and 0.13, respectively).
>>>>
>>>> ITU-T J 83 Annex B doesn't say anything about it, but the ANSI SCTE07 spec
>>>> says that the roll-off is approx. 0.18 for 256-QAM and approx. 0.12 for
>>>> 256-QAM.
>>>>
>>>> DVB-S also has a fixed roll-off of 0.35, while DVB-S2 allows configuring it.
>>>
>>>
>>> DVB-S uses 3 different rolloffs just like DVB-S2. In fact the rolloff
>>> doesn't have anything to do with the delivery system at all, but
>>> something that which is independant and physical to the channel,
>>> rather than something logical such as a delivery system, looking at a
>>> satellite transponder's viewpoint.
>>>
>>> For general (home) broadcast purposes, we use only 0.35. There are
>>> many other usages, which is not yet applicable with especially Linux
>>> DVB with regards to narrow band operations such as DVB feeds and DSNG.
>>
>> Ok.
>>
>>>
>>> There are many usages for the second generation delivery systems,
>>> which will likely realize only a very small part.
>>>
>>> Eg: there are other aspects to DVB-S2 such as ACM and VCM, which most
>>> likely we wouldn't cover. the reason is that the users of it are most
>>> likely propreitary users of it and neither would they prefer to have
>>> an open source version for it, nor would any Open Source users be
>>> likely to implement it. Eg: Ericson's Director CA system, where they
>>> have complete control over the box, rather than the user. As far as I
>>> am aware they have a return path as well.
>>>
>>>>
>>>> Not 100% sure, but ISDB-S also seems to use a per-modulation roll-off factor.
>>>>
>>>> Anyway, when the roll-off is known, only symbol rate is needed, in order
>>>> to know the needed bandwidth.
>>>
>>>
>>> You need to know FEC, modulation too .. Eg: If you have 16APSK where
>>> you have 4 bits, in comparison to 3 bits as with 8PSK, then you
>>> require lesser bandwidth.
>>
>> Manu, you're probably thinking in terms of the TS bit rate, and not the
>> modulator symbol rate.
>>
>> The number of bits don't matter here, as the symbol rate is specified
>> in bauds (e. g. symbols per second), and not in bits/s.
> 
> 
> A PSK modulator is a state machine:
> where states/symbols are logically represented by bits, given that the
> state can either be a 0 or 1
> 
> BPSK  states=2    bits=1
> QPSK  states=4    bits=2
> 8PSK  states=8     bits=3
> 16PSK states=16  bits=4
> 32PSK states=32  bits=5
> 
> http://en.wikipedia.org/wiki/Constellation_diagram
> http://en.wikipedia.org/wiki/Gray_code
> 
> Symbol Rate is generally specified in SPS, ie Symbols/sec, or in
> Bauds. AFAICS, We generally use Symbols Per Second rather than bauds.
> 
> http://www.asiasat.com/asiasat/contentView.php?section=69&lang=0
> http://www.b4utv.com/subs/technology.shtml
> http://www.skynewsinternational.com/watch/satellite-information
> 
> I haven't seen a demodulator specification which states Mbaud, but
> have seen them stated as MSPS or kSPS.
> 
> Now, assuming a 36MHz TP as an example: The given bandwidth is better
> or efficiently used by a higher order modulation. This is the reason
> why DVB.org states that DVB-S2 saves 30% bandwidth.
> 
> Quoting you: "Anyway, when the roll-off is known, only symbol rate is
> needed, in order to know the needed bandwidth."
> 
> Given a fixed TP CHBW, according to you: Channel Bandwidth can be
> calculated by knowing Symbol Rate alone, with a known rolloff.
> 
> I say that this is not possible. Since the number of states/symbols
> for any given channel depends on the modulation order as well.
> 
> I hope that clears up things for you.
> 
> 
>> The conversion from bauds to bits/s is to multiply the number of bits per
>> symbol by the rate, in bauds.
>>
>> A higher number of bits for a given modulation just increase the number of
>> possible states that the modulation will use. So, it will require a higher
>> signal to noise relation at the demod, to avoid miss-detection, but this is
>> a separate issue.
> 
> 
> That's why for higher order modulations, demodulators use better Error
> Correction Schemes (eg: BCH/Turbo) when the modulation order is
> higher.
> 
> http://en.wikipedia.org/wiki/Modulation_order
> http://en.wikipedia.org/wiki/BCH_code
> 
> 
>> The roll-off, minimal bandwidth (referred as "Nyquist frequency" by the DVB-C
>> specs) and symbol rate are related by this equation:
>>        f = symbol_rate * (1 + roll_off)
>>
>> The f value is the Nyquist frequency, and it will dictate the low-pass filter
>> needed to confine the entire signal of the channel (with is, basically, the
>> amount of bandwidth required by the channel).
>>
>>> Also, higher the Error correction information bits set in (redundant),
>>> the more bandwidth it needs to occupy.
>>
>> The error correction algorithm will reduce the bit rate of the TS stream,
>> but won't affect the symbol rate at the modulator.
> 
> 
> No. That's an incorrect statement. FEC gives the receiver the ability
> to correct errors without needing a reverse channel to request
> retransmission of data, but at the cost of a fixed, higher forward
> channel bandwidth.
> 
> http://en.wikipedia.org/wiki/Forward_error_correction

Manu,

A good reference for working with those stuff is the Symon Haykin book:
	http://www.amazon.com/Communication-Systems-4th-Simon-Haykin/dp/0471178691

I used it a lot during the time I was studying Electrical Engineering it at University,
and during my post-graduation.

There are two ways of engineering with those parameters: 

1) giving that you need an effective bit rate of X, calculate the amount of bandwidth
that it is needed. If you go into this direction, FEC will require more bandwidth. This
is what you're saying. This is the direction used generally by engineers that aren't
limited by a previous bandwidth constraints (like the ones that work with satellite).

2) Work at the reverse way: giving that you have a limited bandwidh of Y, estimate the
effective bit rate that will be available on that channel.

This is the direction where the designers of standards like DVB-C and DVB-T took, as 
they needed to start with a channel with a bandwidth limited by the analog standard.

Assuming that an specific channel allows amount of symbols per second X (btw, Baud is just
an alias for symbols per second), there is a frequency greater than X from where the
entropy of the channel will be zero. Due to the Nyquist theorem, sampling at twice that
frequency is enough to completely recover the original signal, in the absence of noise.

All DVB specs define such frequency via a roll-off factor, as:

	H(freq) = 0 for freq > symbol_rate * (1 + roll_off)

So, the bandwidth is limited by symbol_rate * (1 + roll_off).

This equation is at ITU-T J.83 Annex A.7. The same equation is at on chapter 9 of the 
EN 300 429 v1.2.1 (page 16).

EN 300 429 Annex B estimates the channel bandwidth for a 8 MHz and roll-off factor 0.15
as about 6.96 MBaud.

With the above expression, we can get the same result:
	max_symbol_rate = Bw / (1 + roll_off)
	max_symbol_rate = 8000000 / (1 + 0.15)
	max_symbol_rate = 6956521

It should be noticed that the SNR of the channel and the modulation efficiency may require
using a lower channel rate for the channel. Annex B of EN 300 429 (informative only)
gives some examples on the effective maximum symbol rates for real systems.

For example, instead of using 8 MHz, it uses a guard interval, reducing the occupied bandwidth
to 7.92 MHz for 64-QAM, 7.96 for 32-QAM, and so on.

So, for a 64-QAM, at Annex B examples, using a 7.92 MHz with 0.15 rolloff, the max symbol
rate is 6,886,956 MBaud (7920000 / (1 + .15). as it uses 6 bits/symbol, total bitrate is
41,321,736 (6 * 6,886,956). Unfortunately, the annex B examples don't tell what FEC were used,
but the efficiency of the channel were 38.1/41.34 = 92 %. E. g. 8% of the total bit rate
were wasted by the error correction algorithm, meaning that such channel can transport
streams up to 38.1 Mbits/s.

Regards,
Mauro

Re: PATCH: Query DVB frontend capabilities

[Manu Abraham]

On Mon, Nov 14, 2011 at 5:17 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 13-11-2011 13:27, Manu Abraham escreveu:
>> On Sun, Nov 13, 2011 at 7:02 PM, Mauro Carvalho Chehab
>> <mchehab@redhat.com> wrote:
>>> Em 11-11-2011 20:34, Manu Abraham escreveu:
>>>> On Sat, Nov 12, 2011 at 12:07 AM, Mauro Carvalho Chehab
>>>> <mchehab@redhat.com> wrote:
>>>>> Em 11-11-2011 15:43, BOUWSMA Barry escreveu:
>>>>>>
>>>>>> On Do (Donnerstag) 10.Nov (November) 2011, 22:20,  Mauro Carvalho Chehab wrote:
>>>>>>
>>>>>>> We should also think on a way to enumerate the supported values for each DVB $
>>>>>>> the enum fe_caps is not enough anymore to store everything. It currently has $
>>>>>>> filled (of a total of 32 bits), and we currently have:
>>>>>>>      12 code rates (including AUTO/NONE);
>>>>>>
>>>>>> I'm probably not looking at the correct source, but the numbers
>>>>>> seem to match, so I'll just note that in what I'm looking at,
>>>>>> there are missing the values  1/3  and  2/5 .
>>>>>
>>>>> Those were not added yet, as no driver currently uses it.
>>>>>
>>>>>>
>>>>>> But I have to apologise in that I've also not been paying
>>>>>> attention to this conversation, and haven't even been trying
>>>>>> to follow recent developments.
>>>>>>
>>>>>>
>>>>>>>      13 modulation types;
>>>>>>
>>>>>> Here I see missing  QAM1024  and  QAM4096 .
>>>>>
>>>>> Same here.
>>>>>
>>>>>>
>>>>>>
>>>>>>>      7 transmission modes;
>>>>>>>      7 bandwidths;
>>>>>>
>>>>>> Apparently DVB-C2 allows us any bandwidth from 8MHz to 450MHz,
>>>>>> rather than the discrete values used by the other systems.
>>>>>> If this is also applicable to other countries with 6MHz rasters,
>>>>>> would it be necessary in addition to specify carrier spacing,
>>>>>> either 2,232kHz or 1,674kHz as opposed to getting this from the
>>>>>> channel bandwidth?
>>>>>
>>>>> There are 3 parameters for Satellite and Cable systems:
>>>>>        - Roll off factor;
>>>>>        - Symbol Rate;
>>>>>        - Bandwidth.
>>>>>
>>>>> Only two of the tree are independent, as the spec defines:
>>>>>        Bandwidth = symbol rate * (1  + roll off).
>>>>>
>>>>> For DVB-C Annex A and C, roll off is fixed (0.15 and 0.13, respectively).
>>>>>
>>>>> ITU-T J 83 Annex B doesn't say anything about it, but the ANSI SCTE07 spec
>>>>> says that the roll-off is approx. 0.18 for 256-QAM and approx. 0.12 for
>>>>> 256-QAM.
>>>>>
>>>>> DVB-S also has a fixed roll-off of 0.35, while DVB-S2 allows configuring it.
>>>>
>>>>
>>>> DVB-S uses 3 different rolloffs just like DVB-S2. In fact the rolloff
>>>> doesn't have anything to do with the delivery system at all, but
>>>> something that which is independant and physical to the channel,
>>>> rather than something logical such as a delivery system, looking at a
>>>> satellite transponder's viewpoint.
>>>>
>>>> For general (home) broadcast purposes, we use only 0.35. There are
>>>> many other usages, which is not yet applicable with especially Linux
>>>> DVB with regards to narrow band operations such as DVB feeds and DSNG.
>>>
>>> Ok.
>>>
>>>>
>>>> There are many usages for the second generation delivery systems,
>>>> which will likely realize only a very small part.
>>>>
>>>> Eg: there are other aspects to DVB-S2 such as ACM and VCM, which most
>>>> likely we wouldn't cover. the reason is that the users of it are most
>>>> likely propreitary users of it and neither would they prefer to have
>>>> an open source version for it, nor would any Open Source users be
>>>> likely to implement it. Eg: Ericson's Director CA system, where they
>>>> have complete control over the box, rather than the user. As far as I
>>>> am aware they have a return path as well.
>>>>
>>>>>
>>>>> Not 100% sure, but ISDB-S also seems to use a per-modulation roll-off factor.
>>>>>
>>>>> Anyway, when the roll-off is known, only symbol rate is needed, in order
>>>>> to know the needed bandwidth.
>>>>
>>>>
>>>> You need to know FEC, modulation too .. Eg: If you have 16APSK where
>>>> you have 4 bits, in comparison to 3 bits as with 8PSK, then you
>>>> require lesser bandwidth.
>>>
>>> Manu, you're probably thinking in terms of the TS bit rate, and not the
>>> modulator symbol rate.
>>>
>>> The number of bits don't matter here, as the symbol rate is specified
>>> in bauds (e. g. symbols per second), and not in bits/s.
>>
>>
>> A PSK modulator is a state machine:
>> where states/symbols are logically represented by bits, given that the
>> state can either be a 0 or 1
>>
>> BPSK  states=2    bits=1
>> QPSK  states=4    bits=2
>> 8PSK  states=8     bits=3
>> 16PSK states=16  bits=4
>> 32PSK states=32  bits=5
>>
>> http://en.wikipedia.org/wiki/Constellation_diagram
>> http://en.wikipedia.org/wiki/Gray_code
>>
>> Symbol Rate is generally specified in SPS, ie Symbols/sec, or in
>> Bauds. AFAICS, We generally use Symbols Per Second rather than bauds.
>>
>> http://www.asiasat.com/asiasat/contentView.php?section=69&lang=0
>> http://www.b4utv.com/subs/technology.shtml
>> http://www.skynewsinternational.com/watch/satellite-information
>>
>> I haven't seen a demodulator specification which states Mbaud, but
>> have seen them stated as MSPS or kSPS.
>>
>> Now, assuming a 36MHz TP as an example: The given bandwidth is better
>> or efficiently used by a higher order modulation. This is the reason
>> why DVB.org states that DVB-S2 saves 30% bandwidth.
>>
>> Quoting you: "Anyway, when the roll-off is known, only symbol rate is
>> needed, in order to know the needed bandwidth."
>>
>> Given a fixed TP CHBW, according to you: Channel Bandwidth can be
>> calculated by knowing Symbol Rate alone, with a known rolloff.
>>
>> I say that this is not possible. Since the number of states/symbols
>> for any given channel depends on the modulation order as well.
>>
>> I hope that clears up things for you.
>>
>>
>>> The conversion from bauds to bits/s is to multiply the number of bits per
>>> symbol by the rate, in bauds.
>>>
>>> A higher number of bits for a given modulation just increase the number of
>>> possible states that the modulation will use. So, it will require a higher
>>> signal to noise relation at the demod, to avoid miss-detection, but this is
>>> a separate issue.
>>
>>
>> That's why for higher order modulations, demodulators use better Error
>> Correction Schemes (eg: BCH/Turbo) when the modulation order is
>> higher.
>>
>> http://en.wikipedia.org/wiki/Modulation_order
>> http://en.wikipedia.org/wiki/BCH_code
>>
>>
>>> The roll-off, minimal bandwidth (referred as "Nyquist frequency" by the DVB-C
>>> specs) and symbol rate are related by this equation:
>>>        f = symbol_rate * (1 + roll_off)
>>>
>>> The f value is the Nyquist frequency, and it will dictate the low-pass filter
>>> needed to confine the entire signal of the channel (with is, basically, the
>>> amount of bandwidth required by the channel).
>>>
>>>> Also, higher the Error correction information bits set in (redundant),
>>>> the more bandwidth it needs to occupy.
>>>
>>> The error correction algorithm will reduce the bit rate of the TS stream,
>>> but won't affect the symbol rate at the modulator.
>>
>>
>> No. That's an incorrect statement. FEC gives the receiver the ability
>> to correct errors without needing a reverse channel to request
>> retransmission of data, but at the cost of a fixed, higher forward
>> channel bandwidth.
>>
>> http://en.wikipedia.org/wiki/Forward_error_correction
>
> Manu,
>
> A good reference for working with those stuff is the Symon Haykin book:
>        http://www.amazon.com/Communication-Systems-4th-Simon-Haykin/dp/0471178691
>
> I used it a lot during the time I was studying Electrical Engineering it at University,
> and during my post-graduation.


Mauro,

Well, if the discussion is about know the person whom you are talking
to: I did my Engineering degree in Electronics and Communication;
Simon and Haykin was just one among them in one of the semesters. I
still have those college books somewhere in an old dusty shelf. Later
on, I worked at the (Remote Sensing and CVAI labs) Indian Institute of
Sciences, Bangalore. Further down the lane, did work with media
broadcast organizations.

I am not going to get into an argument session, where you keep on
changing the topic, with unrelated or incorrect stuff altogether.

Let this thread die. :-)


Regards,
Manu

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 14-11-2011 13:02, Manu Abraham escreveu:
> On Mon, Nov 14, 2011 at 5:17 PM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> Em 13-11-2011 13:27, Manu Abraham escreveu:
>>> On Sun, Nov 13, 2011 at 7:02 PM, Mauro Carvalho Chehab
>>> <mchehab@redhat.com> wrote:
>>>> Em 11-11-2011 20:34, Manu Abraham escreveu:
>>>>> On Sat, Nov 12, 2011 at 12:07 AM, Mauro Carvalho Chehab
>>>>> <mchehab@redhat.com> wrote:
>>>>>> Em 11-11-2011 15:43, BOUWSMA Barry escreveu:
>>>>>>>
>>>>>>> On Do (Donnerstag) 10.Nov (November) 2011, 22:20,  Mauro Carvalho Chehab wrote:
>>>>>>>
>>>>>>>> We should also think on a way to enumerate the supported values for each DVB $
>>>>>>>> the enum fe_caps is not enough anymore to store everything. It currently has $
>>>>>>>> filled (of a total of 32 bits), and we currently have:
>>>>>>>>      12 code rates (including AUTO/NONE);
>>>>>>>
>>>>>>> I'm probably not looking at the correct source, but the numbers
>>>>>>> seem to match, so I'll just note that in what I'm looking at,
>>>>>>> there are missing the values  1/3  and  2/5 .
>>>>>>
>>>>>> Those were not added yet, as no driver currently uses it.
>>>>>>
>>>>>>>
>>>>>>> But I have to apologise in that I've also not been paying
>>>>>>> attention to this conversation, and haven't even been trying
>>>>>>> to follow recent developments.
>>>>>>>
>>>>>>>
>>>>>>>>      13 modulation types;
>>>>>>>
>>>>>>> Here I see missing  QAM1024  and  QAM4096 .
>>>>>>
>>>>>> Same here.
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>      7 transmission modes;
>>>>>>>>      7 bandwidths;
>>>>>>>
>>>>>>> Apparently DVB-C2 allows us any bandwidth from 8MHz to 450MHz,
>>>>>>> rather than the discrete values used by the other systems.
>>>>>>> If this is also applicable to other countries with 6MHz rasters,
>>>>>>> would it be necessary in addition to specify carrier spacing,
>>>>>>> either 2,232kHz or 1,674kHz as opposed to getting this from the
>>>>>>> channel bandwidth?
>>>>>>
>>>>>> There are 3 parameters for Satellite and Cable systems:
>>>>>>        - Roll off factor;
>>>>>>        - Symbol Rate;
>>>>>>        - Bandwidth.
>>>>>>
>>>>>> Only two of the tree are independent, as the spec defines:
>>>>>>        Bandwidth = symbol rate * (1  + roll off).
>>>>>>
>>>>>> For DVB-C Annex A and C, roll off is fixed (0.15 and 0.13, respectively).
>>>>>>
>>>>>> ITU-T J 83 Annex B doesn't say anything about it, but the ANSI SCTE07 spec
>>>>>> says that the roll-off is approx. 0.18 for 256-QAM and approx. 0.12 for
>>>>>> 256-QAM.
>>>>>>
>>>>>> DVB-S also has a fixed roll-off of 0.35, while DVB-S2 allows configuring it.
>>>>>
>>>>>
>>>>> DVB-S uses 3 different rolloffs just like DVB-S2. In fact the rolloff
>>>>> doesn't have anything to do with the delivery system at all, but
>>>>> something that which is independant and physical to the channel,
>>>>> rather than something logical such as a delivery system, looking at a
>>>>> satellite transponder's viewpoint.
>>>>>
>>>>> For general (home) broadcast purposes, we use only 0.35. There are
>>>>> many other usages, which is not yet applicable with especially Linux
>>>>> DVB with regards to narrow band operations such as DVB feeds and DSNG.
>>>>
>>>> Ok.
>>>>
>>>>>
>>>>> There are many usages for the second generation delivery systems,
>>>>> which will likely realize only a very small part.
>>>>>
>>>>> Eg: there are other aspects to DVB-S2 such as ACM and VCM, which most
>>>>> likely we wouldn't cover. the reason is that the users of it are most
>>>>> likely propreitary users of it and neither would they prefer to have
>>>>> an open source version for it, nor would any Open Source users be
>>>>> likely to implement it. Eg: Ericson's Director CA system, where they
>>>>> have complete control over the box, rather than the user. As far as I
>>>>> am aware they have a return path as well.
>>>>>
>>>>>>
>>>>>> Not 100% sure, but ISDB-S also seems to use a per-modulation roll-off factor.
>>>>>>
>>>>>> Anyway, when the roll-off is known, only symbol rate is needed, in order
>>>>>> to know the needed bandwidth.
>>>>>
>>>>>
>>>>> You need to know FEC, modulation too .. Eg: If you have 16APSK where
>>>>> you have 4 bits, in comparison to 3 bits as with 8PSK, then you
>>>>> require lesser bandwidth.
>>>>
>>>> Manu, you're probably thinking in terms of the TS bit rate, and not the
>>>> modulator symbol rate.
>>>>
>>>> The number of bits don't matter here, as the symbol rate is specified
>>>> in bauds (e. g. symbols per second), and not in bits/s.
>>>
>>>
>>> A PSK modulator is a state machine:
>>> where states/symbols are logically represented by bits, given that the
>>> state can either be a 0 or 1
>>>
>>> BPSK  states=2    bits=1
>>> QPSK  states=4    bits=2
>>> 8PSK  states=8     bits=3
>>> 16PSK states=16  bits=4
>>> 32PSK states=32  bits=5
>>>
>>> http://en.wikipedia.org/wiki/Constellation_diagram
>>> http://en.wikipedia.org/wiki/Gray_code
>>>
>>> Symbol Rate is generally specified in SPS, ie Symbols/sec, or in
>>> Bauds. AFAICS, We generally use Symbols Per Second rather than bauds.
>>>
>>> http://www.asiasat.com/asiasat/contentView.php?section=69&lang=0
>>> http://www.b4utv.com/subs/technology.shtml
>>> http://www.skynewsinternational.com/watch/satellite-information
>>>
>>> I haven't seen a demodulator specification which states Mbaud, but
>>> have seen them stated as MSPS or kSPS.
>>>
>>> Now, assuming a 36MHz TP as an example: The given bandwidth is better
>>> or efficiently used by a higher order modulation. This is the reason
>>> why DVB.org states that DVB-S2 saves 30% bandwidth.
>>>
>>> Quoting you: "Anyway, when the roll-off is known, only symbol rate is
>>> needed, in order to know the needed bandwidth."
>>>
>>> Given a fixed TP CHBW, according to you: Channel Bandwidth can be
>>> calculated by knowing Symbol Rate alone, with a known rolloff.
>>>
>>> I say that this is not possible. Since the number of states/symbols
>>> for any given channel depends on the modulation order as well.
>>>
>>> I hope that clears up things for you.
>>>
>>>
>>>> The conversion from bauds to bits/s is to multiply the number of bits per
>>>> symbol by the rate, in bauds.
>>>>
>>>> A higher number of bits for a given modulation just increase the number of
>>>> possible states that the modulation will use. So, it will require a higher
>>>> signal to noise relation at the demod, to avoid miss-detection, but this is
>>>> a separate issue.
>>>
>>>
>>> That's why for higher order modulations, demodulators use better Error
>>> Correction Schemes (eg: BCH/Turbo) when the modulation order is
>>> higher.
>>>
>>> http://en.wikipedia.org/wiki/Modulation_order
>>> http://en.wikipedia.org/wiki/BCH_code
>>>
>>>
>>>> The roll-off, minimal bandwidth (referred as "Nyquist frequency" by the DVB-C
>>>> specs) and symbol rate are related by this equation:
>>>>        f = symbol_rate * (1 + roll_off)
>>>>
>>>> The f value is the Nyquist frequency, and it will dictate the low-pass filter
>>>> needed to confine the entire signal of the channel (with is, basically, the
>>>> amount of bandwidth required by the channel).
>>>>
>>>>> Also, higher the Error correction information bits set in (redundant),
>>>>> the more bandwidth it needs to occupy.
>>>>
>>>> The error correction algorithm will reduce the bit rate of the TS stream,
>>>> but won't affect the symbol rate at the modulator.
>>>
>>>
>>> No. That's an incorrect statement. FEC gives the receiver the ability
>>> to correct errors without needing a reverse channel to request
>>> retransmission of data, but at the cost of a fixed, higher forward
>>> channel bandwidth.
>>>
>>> http://en.wikipedia.org/wiki/Forward_error_correction
>>
>> Manu,
>>
>> A good reference for working with those stuff is the Symon Haykin book:
>>        http://www.amazon.com/Communication-Systems-4th-Simon-Haykin/dp/0471178691
>>
>> I used it a lot during the time I was studying Electrical Engineering it at University,
>> and during my post-graduation.
> 
> 
> Mauro,
> 
> Well, if the discussion is about know the person whom you are talking
> to: I did my Engineering degree in Electronics and Communication;
> Simon and Haykin was just one among them in one of the semesters. I
> still have those college books somewhere in an old dusty shelf. Later
> on, I worked at the (Remote Sensing and CVAI labs) Indian Institute of
> Sciences, Bangalore. Further down the lane, did work with media
> broadcast organizations.
> 
> I am not going to get into an argument session, where you keep on
> changing the topic, with unrelated or incorrect stuff altogether.

I'm not changing topic. All I'm saying since the beginning is that
there's no need to add Bandwidth at the DVB API for cable/satellite, as,
for the supported delivery systems, the symbol rate + roll-off can be used
for bandwidth estimation, where needed,

E. g.:
	Bandwidth = max_symbol_rate * (1 + roll_off)

> 
> Let this thread die. :-)

Agreed.

Regards,
Mauro

Re: PATCH: Query DVB frontend capabilities

[Manu Abraham]

On Mon, Nov 14, 2011 at 10:09 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 14-11-2011 13:02, Manu Abraham escreveu:
>> On Mon, Nov 14, 2011 at 5:17 PM, Mauro Carvalho Chehab
>> <mchehab@redhat.com> wrote:
>>> Em 13-11-2011 13:27, Manu Abraham escreveu:
>>>> On Sun, Nov 13, 2011 at 7:02 PM, Mauro Carvalho Chehab
>>>> <mchehab@redhat.com> wrote:
>>>>> Em 11-11-2011 20:34, Manu Abraham escreveu:
>>>>>> On Sat, Nov 12, 2011 at 12:07 AM, Mauro Carvalho Chehab
>>>>>> <mchehab@redhat.com> wrote:
>>>>>>> Em 11-11-2011 15:43, BOUWSMA Barry escreveu:
>>>>>>>>
>>>>>>>> On Do (Donnerstag) 10.Nov (November) 2011, 22:20,  Mauro Carvalho Chehab wrote:
>>>>>>>>
>>>>>>>>> We should also think on a way to enumerate the supported values for each DVB $
>>>>>>>>> the enum fe_caps is not enough anymore to store everything. It currently has $
>>>>>>>>> filled (of a total of 32 bits), and we currently have:
>>>>>>>>>      12 code rates (including AUTO/NONE);
>>>>>>>>
>>>>>>>> I'm probably not looking at the correct source, but the numbers
>>>>>>>> seem to match, so I'll just note that in what I'm looking at,
>>>>>>>> there are missing the values  1/3  and  2/5 .
>>>>>>>
>>>>>>> Those were not added yet, as no driver currently uses it.
>>>>>>>
>>>>>>>>
>>>>>>>> But I have to apologise in that I've also not been paying
>>>>>>>> attention to this conversation, and haven't even been trying
>>>>>>>> to follow recent developments.
>>>>>>>>
>>>>>>>>
>>>>>>>>>      13 modulation types;
>>>>>>>>
>>>>>>>> Here I see missing  QAM1024  and  QAM4096 .
>>>>>>>
>>>>>>> Same here.
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>      7 transmission modes;
>>>>>>>>>      7 bandwidths;
>>>>>>>>
>>>>>>>> Apparently DVB-C2 allows us any bandwidth from 8MHz to 450MHz,
>>>>>>>> rather than the discrete values used by the other systems.
>>>>>>>> If this is also applicable to other countries with 6MHz rasters,
>>>>>>>> would it be necessary in addition to specify carrier spacing,
>>>>>>>> either 2,232kHz or 1,674kHz as opposed to getting this from the
>>>>>>>> channel bandwidth?
>>>>>>>
>>>>>>> There are 3 parameters for Satellite and Cable systems:
>>>>>>>        - Roll off factor;
>>>>>>>        - Symbol Rate;
>>>>>>>        - Bandwidth.
>>>>>>>
>>>>>>> Only two of the tree are independent, as the spec defines:
>>>>>>>        Bandwidth = symbol rate * (1  + roll off).
>>>>>>>
>>>>>>> For DVB-C Annex A and C, roll off is fixed (0.15 and 0.13, respectively).
>>>>>>>
>>>>>>> ITU-T J 83 Annex B doesn't say anything about it, but the ANSI SCTE07 spec
>>>>>>> says that the roll-off is approx. 0.18 for 256-QAM and approx. 0.12 for
>>>>>>> 256-QAM.
>>>>>>>
>>>>>>> DVB-S also has a fixed roll-off of 0.35, while DVB-S2 allows configuring it.
>>>>>>
>>>>>>
>>>>>> DVB-S uses 3 different rolloffs just like DVB-S2. In fact the rolloff
>>>>>> doesn't have anything to do with the delivery system at all, but
>>>>>> something that which is independant and physical to the channel,
>>>>>> rather than something logical such as a delivery system, looking at a
>>>>>> satellite transponder's viewpoint.
>>>>>>
>>>>>> For general (home) broadcast purposes, we use only 0.35. There are
>>>>>> many other usages, which is not yet applicable with especially Linux
>>>>>> DVB with regards to narrow band operations such as DVB feeds and DSNG.
>>>>>
>>>>> Ok.
>>>>>
>>>>>>
>>>>>> There are many usages for the second generation delivery systems,
>>>>>> which will likely realize only a very small part.
>>>>>>
>>>>>> Eg: there are other aspects to DVB-S2 such as ACM and VCM, which most
>>>>>> likely we wouldn't cover. the reason is that the users of it are most
>>>>>> likely propreitary users of it and neither would they prefer to have
>>>>>> an open source version for it, nor would any Open Source users be
>>>>>> likely to implement it. Eg: Ericson's Director CA system, where they
>>>>>> have complete control over the box, rather than the user. As far as I
>>>>>> am aware they have a return path as well.
>>>>>>
>>>>>>>
>>>>>>> Not 100% sure, but ISDB-S also seems to use a per-modulation roll-off factor.
>>>>>>>
>>>>>>> Anyway, when the roll-off is known, only symbol rate is needed, in order
>>>>>>> to know the needed bandwidth.
>>>>>>
>>>>>>
>>>>>> You need to know FEC, modulation too .. Eg: If you have 16APSK where
>>>>>> you have 4 bits, in comparison to 3 bits as with 8PSK, then you
>>>>>> require lesser bandwidth.
>>>>>
>>>>> Manu, you're probably thinking in terms of the TS bit rate, and not the
>>>>> modulator symbol rate.
>>>>>
>>>>> The number of bits don't matter here, as the symbol rate is specified
>>>>> in bauds (e. g. symbols per second), and not in bits/s.
>>>>
>>>>
>>>> A PSK modulator is a state machine:
>>>> where states/symbols are logically represented by bits, given that the
>>>> state can either be a 0 or 1
>>>>
>>>> BPSK  states=2    bits=1
>>>> QPSK  states=4    bits=2
>>>> 8PSK  states=8     bits=3
>>>> 16PSK states=16  bits=4
>>>> 32PSK states=32  bits=5
>>>>
>>>> http://en.wikipedia.org/wiki/Constellation_diagram
>>>> http://en.wikipedia.org/wiki/Gray_code
>>>>
>>>> Symbol Rate is generally specified in SPS, ie Symbols/sec, or in
>>>> Bauds. AFAICS, We generally use Symbols Per Second rather than bauds.
>>>>
>>>> http://www.asiasat.com/asiasat/contentView.php?section=69&lang=0
>>>> http://www.b4utv.com/subs/technology.shtml
>>>> http://www.skynewsinternational.com/watch/satellite-information
>>>>
>>>> I haven't seen a demodulator specification which states Mbaud, but
>>>> have seen them stated as MSPS or kSPS.
>>>>
>>>> Now, assuming a 36MHz TP as an example: The given bandwidth is better
>>>> or efficiently used by a higher order modulation. This is the reason
>>>> why DVB.org states that DVB-S2 saves 30% bandwidth.
>>>>
>>>> Quoting you: "Anyway, when the roll-off is known, only symbol rate is
>>>> needed, in order to know the needed bandwidth."
>>>>
>>>> Given a fixed TP CHBW, according to you: Channel Bandwidth can be
>>>> calculated by knowing Symbol Rate alone, with a known rolloff.
>>>>
>>>> I say that this is not possible. Since the number of states/symbols
>>>> for any given channel depends on the modulation order as well.
>>>>
>>>> I hope that clears up things for you.
>>>>
>>>>
>>>>> The conversion from bauds to bits/s is to multiply the number of bits per
>>>>> symbol by the rate, in bauds.
>>>>>
>>>>> A higher number of bits for a given modulation just increase the number of
>>>>> possible states that the modulation will use. So, it will require a higher
>>>>> signal to noise relation at the demod, to avoid miss-detection, but this is
>>>>> a separate issue.
>>>>
>>>>
>>>> That's why for higher order modulations, demodulators use better Error
>>>> Correction Schemes (eg: BCH/Turbo) when the modulation order is
>>>> higher.
>>>>
>>>> http://en.wikipedia.org/wiki/Modulation_order
>>>> http://en.wikipedia.org/wiki/BCH_code
>>>>
>>>>
>>>>> The roll-off, minimal bandwidth (referred as "Nyquist frequency" by the DVB-C
>>>>> specs) and symbol rate are related by this equation:
>>>>>        f = symbol_rate * (1 + roll_off)
>>>>>
>>>>> The f value is the Nyquist frequency, and it will dictate the low-pass filter
>>>>> needed to confine the entire signal of the channel (with is, basically, the
>>>>> amount of bandwidth required by the channel).
>>>>>
>>>>>> Also, higher the Error correction information bits set in (redundant),
>>>>>> the more bandwidth it needs to occupy.
>>>>>
>>>>> The error correction algorithm will reduce the bit rate of the TS stream,
>>>>> but won't affect the symbol rate at the modulator.
>>>>
>>>>
>>>> No. That's an incorrect statement. FEC gives the receiver the ability
>>>> to correct errors without needing a reverse channel to request
>>>> retransmission of data, but at the cost of a fixed, higher forward
>>>> channel bandwidth.
>>>>
>>>> http://en.wikipedia.org/wiki/Forward_error_correction
>>>
>>> Manu,
>>>
>>> A good reference for working with those stuff is the Symon Haykin book:
>>>        http://www.amazon.com/Communication-Systems-4th-Simon-Haykin/dp/0471178691
>>>
>>> I used it a lot during the time I was studying Electrical Engineering it at University,
>>> and during my post-graduation.
>>
>>
>> Mauro,
>>
>> Well, if the discussion is about know the person whom you are talking
>> to: I did my Engineering degree in Electronics and Communication;
>> Simon and Haykin was just one among them in one of the semesters. I
>> still have those college books somewhere in an old dusty shelf. Later
>> on, I worked at the (Remote Sensing and CVAI labs) Indian Institute of
>> Sciences, Bangalore. Further down the lane, did work with media
>> broadcast organizations.
>>
>> I am not going to get into an argument session, where you keep on
>> changing the topic, with unrelated or incorrect stuff altogether.
>
> I'm not changing topic. All I'm saying since the beginning is that
> there's no need to add Bandwidth at the DVB API for cable/satellite, as,
> for the supported delivery systems, the symbol rate + roll-off can be used
> for bandwidth estimation, where needed,

What you stated:

"Anyway, when the roll-off is known, only symbol rate is needed, in
order to know the needed bandwidth."

What I stated:

"the number of states/symbols for any given channel depends on the
modulation order as well."

When you don't know the modulation, you don't know how many bits are
packed into a given Digital channel. It is that simple. rolloff
provides you only the slope (or 3dB cutoff) of the channel.

Eg: A TP having a b/w of 36 MHz can contain more symbols with a higher
order modulation.
(Bandwidth allocated to a media broadcaster, is fixed in terms of MHz
alone. A media broadcaster, let's say "X", purchases a transponder
with a fixed bandwidth of "Y" for "Z" thousand USD)

ie, bw = f(m) + f(sr)

Generally, the concept with a fixed modulation, it is bw = f(sr)
alone. But this is not the case, when we are dealing with multiple
modulations on the same device. I hope by now, you understand where
you are going wrong.

Regards,
Manu

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 14-11-2011 15:09, Manu Abraham escreveu:
> On Mon, Nov 14, 2011 at 10:09 PM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> Em 14-11-2011 13:02, Manu Abraham escreveu:
>>> On Mon, Nov 14, 2011 at 5:17 PM, Mauro Carvalho Chehab
>>> <mchehab@redhat.com> wrote:
>>>> Em 13-11-2011 13:27, Manu Abraham escreveu:
>>>>> On Sun, Nov 13, 2011 at 7:02 PM, Mauro Carvalho Chehab
>>>>> <mchehab@redhat.com> wrote:
>>>>>> Em 11-11-2011 20:34, Manu Abraham escreveu:
>>>>>>> On Sat, Nov 12, 2011 at 12:07 AM, Mauro Carvalho Chehab
>>>>>>> <mchehab@redhat.com> wrote:
>>>>>>>> Em 11-11-2011 15:43, BOUWSMA Barry escreveu:
>>>>>>>>>
>>>>>>>>> On Do (Donnerstag) 10.Nov (November) 2011, 22:20,  Mauro Carvalho Chehab wrote:
>>>>>>>>>
>>>>>>>>>> We should also think on a way to enumerate the supported values for each DVB $
>>>>>>>>>> the enum fe_caps is not enough anymore to store everything. It currently has $
>>>>>>>>>> filled (of a total of 32 bits), and we currently have:
>>>>>>>>>>      12 code rates (including AUTO/NONE);
>>>>>>>>>
>>>>>>>>> I'm probably not looking at the correct source, but the numbers
>>>>>>>>> seem to match, so I'll just note that in what I'm looking at,
>>>>>>>>> there are missing the values  1/3  and  2/5 .
>>>>>>>>
>>>>>>>> Those were not added yet, as no driver currently uses it.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> But I have to apologise in that I've also not been paying
>>>>>>>>> attention to this conversation, and haven't even been trying
>>>>>>>>> to follow recent developments.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>      13 modulation types;
>>>>>>>>>
>>>>>>>>> Here I see missing  QAM1024  and  QAM4096 .
>>>>>>>>
>>>>>>>> Same here.
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>      7 transmission modes;
>>>>>>>>>>      7 bandwidths;
>>>>>>>>>
>>>>>>>>> Apparently DVB-C2 allows us any bandwidth from 8MHz to 450MHz,
>>>>>>>>> rather than the discrete values used by the other systems.
>>>>>>>>> If this is also applicable to other countries with 6MHz rasters,
>>>>>>>>> would it be necessary in addition to specify carrier spacing,
>>>>>>>>> either 2,232kHz or 1,674kHz as opposed to getting this from the
>>>>>>>>> channel bandwidth?
>>>>>>>>
>>>>>>>> There are 3 parameters for Satellite and Cable systems:
>>>>>>>>        - Roll off factor;
>>>>>>>>        - Symbol Rate;
>>>>>>>>        - Bandwidth.
>>>>>>>>
>>>>>>>> Only two of the tree are independent, as the spec defines:
>>>>>>>>        Bandwidth = symbol rate * (1  + roll off).
>>>>>>>>
>>>>>>>> For DVB-C Annex A and C, roll off is fixed (0.15 and 0.13, respectively).
>>>>>>>>
>>>>>>>> ITU-T J 83 Annex B doesn't say anything about it, but the ANSI SCTE07 spec
>>>>>>>> says that the roll-off is approx. 0.18 for 256-QAM and approx. 0.12 for
>>>>>>>> 256-QAM.
>>>>>>>>
>>>>>>>> DVB-S also has a fixed roll-off of 0.35, while DVB-S2 allows configuring it.
>>>>>>>
>>>>>>>
>>>>>>> DVB-S uses 3 different rolloffs just like DVB-S2. In fact the rolloff
>>>>>>> doesn't have anything to do with the delivery system at all, but
>>>>>>> something that which is independant and physical to the channel,
>>>>>>> rather than something logical such as a delivery system, looking at a
>>>>>>> satellite transponder's viewpoint.
>>>>>>>
>>>>>>> For general (home) broadcast purposes, we use only 0.35. There are
>>>>>>> many other usages, which is not yet applicable with especially Linux
>>>>>>> DVB with regards to narrow band operations such as DVB feeds and DSNG.
>>>>>>
>>>>>> Ok.
>>>>>>
>>>>>>>
>>>>>>> There are many usages for the second generation delivery systems,
>>>>>>> which will likely realize only a very small part.
>>>>>>>
>>>>>>> Eg: there are other aspects to DVB-S2 such as ACM and VCM, which most
>>>>>>> likely we wouldn't cover. the reason is that the users of it are most
>>>>>>> likely propreitary users of it and neither would they prefer to have
>>>>>>> an open source version for it, nor would any Open Source users be
>>>>>>> likely to implement it. Eg: Ericson's Director CA system, where they
>>>>>>> have complete control over the box, rather than the user. As far as I
>>>>>>> am aware they have a return path as well.
>>>>>>>
>>>>>>>>
>>>>>>>> Not 100% sure, but ISDB-S also seems to use a per-modulation roll-off factor.
>>>>>>>>
>>>>>>>> Anyway, when the roll-off is known, only symbol rate is needed, in order
>>>>>>>> to know the needed bandwidth.
>>>>>>>
>>>>>>>
>>>>>>> You need to know FEC, modulation too .. Eg: If you have 16APSK where
>>>>>>> you have 4 bits, in comparison to 3 bits as with 8PSK, then you
>>>>>>> require lesser bandwidth.
>>>>>>
>>>>>> Manu, you're probably thinking in terms of the TS bit rate, and not the
>>>>>> modulator symbol rate.
>>>>>>
>>>>>> The number of bits don't matter here, as the symbol rate is specified
>>>>>> in bauds (e. g. symbols per second), and not in bits/s.
>>>>>
>>>>>
>>>>> A PSK modulator is a state machine:
>>>>> where states/symbols are logically represented by bits, given that the
>>>>> state can either be a 0 or 1
>>>>>
>>>>> BPSK  states=2    bits=1
>>>>> QPSK  states=4    bits=2
>>>>> 8PSK  states=8     bits=3
>>>>> 16PSK states=16  bits=4
>>>>> 32PSK states=32  bits=5
>>>>>
>>>>> http://en.wikipedia.org/wiki/Constellation_diagram
>>>>> http://en.wikipedia.org/wiki/Gray_code
>>>>>
>>>>> Symbol Rate is generally specified in SPS, ie Symbols/sec, or in
>>>>> Bauds. AFAICS, We generally use Symbols Per Second rather than bauds.
>>>>>
>>>>> http://www.asiasat.com/asiasat/contentView.php?section=69&lang=0
>>>>> http://www.b4utv.com/subs/technology.shtml
>>>>> http://www.skynewsinternational.com/watch/satellite-information
>>>>>
>>>>> I haven't seen a demodulator specification which states Mbaud, but
>>>>> have seen them stated as MSPS or kSPS.
>>>>>
>>>>> Now, assuming a 36MHz TP as an example: The given bandwidth is better
>>>>> or efficiently used by a higher order modulation. This is the reason
>>>>> why DVB.org states that DVB-S2 saves 30% bandwidth.
>>>>>
>>>>> Quoting you: "Anyway, when the roll-off is known, only symbol rate is
>>>>> needed, in order to know the needed bandwidth."
>>>>>
>>>>> Given a fixed TP CHBW, according to you: Channel Bandwidth can be
>>>>> calculated by knowing Symbol Rate alone, with a known rolloff.
>>>>>
>>>>> I say that this is not possible. Since the number of states/symbols
>>>>> for any given channel depends on the modulation order as well.
>>>>>
>>>>> I hope that clears up things for you.
>>>>>
>>>>>
>>>>>> The conversion from bauds to bits/s is to multiply the number of bits per
>>>>>> symbol by the rate, in bauds.
>>>>>>
>>>>>> A higher number of bits for a given modulation just increase the number of
>>>>>> possible states that the modulation will use. So, it will require a higher
>>>>>> signal to noise relation at the demod, to avoid miss-detection, but this is
>>>>>> a separate issue.
>>>>>
>>>>>
>>>>> That's why for higher order modulations, demodulators use better Error
>>>>> Correction Schemes (eg: BCH/Turbo) when the modulation order is
>>>>> higher.
>>>>>
>>>>> http://en.wikipedia.org/wiki/Modulation_order
>>>>> http://en.wikipedia.org/wiki/BCH_code
>>>>>
>>>>>
>>>>>> The roll-off, minimal bandwidth (referred as "Nyquist frequency" by the DVB-C
>>>>>> specs) and symbol rate are related by this equation:
>>>>>>        f = symbol_rate * (1 + roll_off)
>>>>>>
>>>>>> The f value is the Nyquist frequency, and it will dictate the low-pass filter
>>>>>> needed to confine the entire signal of the channel (with is, basically, the
>>>>>> amount of bandwidth required by the channel).
>>>>>>
>>>>>>> Also, higher the Error correction information bits set in (redundant),
>>>>>>> the more bandwidth it needs to occupy.
>>>>>>
>>>>>> The error correction algorithm will reduce the bit rate of the TS stream,
>>>>>> but won't affect the symbol rate at the modulator.
>>>>>
>>>>>
>>>>> No. That's an incorrect statement. FEC gives the receiver the ability
>>>>> to correct errors without needing a reverse channel to request
>>>>> retransmission of data, but at the cost of a fixed, higher forward
>>>>> channel bandwidth.
>>>>>
>>>>> http://en.wikipedia.org/wiki/Forward_error_correction
>>>>
>>>> Manu,
>>>>
>>>> A good reference for working with those stuff is the Symon Haykin book:
>>>>        http://www.amazon.com/Communication-Systems-4th-Simon-Haykin/dp/0471178691
>>>>
>>>> I used it a lot during the time I was studying Electrical Engineering it at University,
>>>> and during my post-graduation.
>>>
>>>
>>> Mauro,
>>>
>>> Well, if the discussion is about know the person whom you are talking
>>> to: I did my Engineering degree in Electronics and Communication;
>>> Simon and Haykin was just one among them in one of the semesters. I
>>> still have those college books somewhere in an old dusty shelf. Later
>>> on, I worked at the (Remote Sensing and CVAI labs) Indian Institute of
>>> Sciences, Bangalore. Further down the lane, did work with media
>>> broadcast organizations.
>>>
>>> I am not going to get into an argument session, where you keep on
>>> changing the topic, with unrelated or incorrect stuff altogether.
>>
>> I'm not changing topic. All I'm saying since the beginning is that
>> there's no need to add Bandwidth at the DVB API for cable/satellite, as,
>> for the supported delivery systems, the symbol rate + roll-off can be used
>> for bandwidth estimation, where needed,
> 
> What you stated:
> 
> "Anyway, when the roll-off is known, only symbol rate is needed, in
> order to know the needed bandwidth."
> 
> What I stated:
> 
> "the number of states/symbols for any given channel depends on the
> modulation order as well."

Sure. I never said otherwise.

> When you don't know the modulation, you don't know how many bits are
> packed into a given Digital channel. It is that simple. 

Yes.

> rolloff
> provides you only the slope (or 3dB cutoff) of the channel.

It specifies the slope, but the roll-off is not 3dB cutoff. 

 EN 300 429 v1.2.1 page 17 picture is very clear[1]:

The Nyquist frequency Fn is defined in terms of the -3dB cutoff, but (1 + roll-off) Fn is
defined with a -43dB cut off.

So, in the DVB-C case, where the roll-off is 0.15, we have, from Page 16 equation:
	H(f) = 0 for |f| > 1.15 Fn

[1] http://www.etsi.org/deliver/etsi_en/300400_300499/300429/01.02.01_60/en_300429v010201p.pdf

> Eg: A TP having a b/w of 36 MHz can contain more symbols with a higher
> order modulation.

Sure. The higher order, the bigger is the number of bits at the constellation
diagram.

Yet, a QPSK and a QAM-64 using the same bandwidth will send data
at the same symbols/sec rate. 

So, let's say that such rate is 1 Msymbol/sec (or 1 Mbaud).

A QPSK modulation (without FEC, on a perfect channel, no noise, etc), will
send 2 bits per symbol, so it will send data at 2 Mbps or 1 Mbaud.

A QAM-64 modulation (at the same conditions) will send 6 bits per symbol,
so it will send data at 64 Mbps or 1Mbaud [1].

[2] http://en.wikipedia.org/wiki/Baud#Relationship_to_gross_bit_rate

See? It seems that you're thinking in terms of bits per second, and not
symbols per second.

> (Bandwidth allocated to a media broadcaster, is fixed in terms of MHz
> alone. A media broadcaster, let's say "X", purchases a transponder
> with a fixed bandwidth of "Y" for "Z" thousand USD)
> 
> ie, bw = f(m) + f(sr)

Ok, if you have two sub-carriers, you need to sum the bandwidth
required for each, in order to estimate the total amount of bandwidth
for the transponder. As the modulation on each is independent, each
modulation may have a different roll-off factor.

Yet, this doesn't require any changes at DVB API, as all that the demodulator
need to know is the sub-carrier parameters (frequency, roll-off, symbol
rate, etc).

> Generally, the concept with a fixed modulation, it is bw = f(sr)
> alone. But this is not the case, when we are dealing with multiple
> modulations on the same device. I hope by now, you understand where
> you are going wrong.

Regards,
Mauro

Re: PATCH: Query DVB frontend capabilities

[Manu Abraham]

On Mon, Nov 14, 2011 at 11:38 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Yet, this doesn't require any changes at DVB API, as all that the demodulator
> need to know is the sub-carrier parameters (frequency, roll-off, symbol
> rate, etc).

You do: this is why there were changes to the V3 API to accomodate
DVB-S2, which eventually became V5. The major change that underwent is
the addition of newer modulations. The demodulator need to be
explicitly told of the modulation. With some demodulators, the
modulation order could be detected from the PL signaling, rather than
the user space application telling it.

Regards,
Manu

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 14-11-2011 16:30, Manu Abraham escreveu:
> On Mon, Nov 14, 2011 at 11:38 PM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> Yet, this doesn't require any changes at DVB API, as all that the demodulator
>> need to know is the sub-carrier parameters (frequency, roll-off, symbol
>> rate, etc).
> 
> You do: this is why there were changes to the V3 API to accomodate
> DVB-S2, which eventually became V5. The major change that underwent is
> the addition of newer modulations. The demodulator need to be
> explicitly told of the modulation. With some demodulators, the
> modulation order could be detected from the PL signaling, rather than
> the user space application telling it.

DVB-S2 doesn't require DVB bandwidth to be specified.

Regards,
Mauro

Re: PATCH: Query DVB frontend capabilities

[Manu Abraham]

On Tue, Nov 15, 2011 at 12:12 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 14-11-2011 16:30, Manu Abraham escreveu:
>> On Mon, Nov 14, 2011 at 11:38 PM, Mauro Carvalho Chehab
>> <mchehab@redhat.com> wrote:
>>> Yet, this doesn't require any changes at DVB API, as all that the demodulator
>>> need to know is the sub-carrier parameters (frequency, roll-off, symbol
>>> rate, etc).
>>
>> You do: this is why there were changes to the V3 API to accomodate
>> DVB-S2, which eventually became V5. The major change that underwent is
>> the addition of newer modulations. The demodulator need to be
>> explicitly told of the modulation. With some demodulators, the
>> modulation order could be detected from the PL signaling, rather than
>> the user space application telling it.
>
> DVB-S2 doesn't require DVB bandwidth to be specified.


stb0899:
		switch (state->delsys) {
		case SYS_DVBS:
		case SYS_DSS:
                                  ......
			if (state->config->tuner_set_bandwidth)
				state->config->tuner_set_bandwidth(fe, (13 *
(stb0899_carr_width(state) + SearchRange)) / 10);
			if (state->config->tuner_get_bandwidth)
				state->config->tuner_get_bandwidth(fe, &internal->tuner_bw);
                               .......
    			break;
		case SYS_DVBS2:
                                  ......
			if (state->config->tuner_set_bandwidth)
				state->config->tuner_set_bandwidth(fe, (stb0899_carr_width(state)
+ SearchRange));
			if (state->config->tuner_get_bandwidth)
				state->config->tuner_get_bandwidth(fe, &internal->tuner_bw);
    			break;


cx24116:

	/* Set/Reset B/W */
	cmd.args[0x00] = CMD_BANDWIDTH;
	cmd.args[0x01] = 0x01;
	cmd.len = 0x02;
	ret = cx24116_cmd_execute(fe, &cmd);
	if (ret != 0)
		return ret;


stv090x does a lot of auto detection for almost everything, but still:

stv090x:
			if (state->algo == STV090x_COLD_SEARCH)
				state->tuner_bw = (15 * (stv090x_car_width(state->srate,
state->rolloff) + 10000000)) / 10;
			else if (state->algo == STV090x_WARM_SEARCH)
				state->tuner_bw = stv090x_car_width(state->srate, state->rolloff)
+ 10000000;
		}


Regards,
Manu

Re: PATCH: Query DVB frontend capabilities

[Mauro Carvalho Chehab]

Em 14-11-2011 16:59, Manu Abraham escreveu:
> On Tue, Nov 15, 2011 at 12:12 AM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> Em 14-11-2011 16:30, Manu Abraham escreveu:
>>> On Mon, Nov 14, 2011 at 11:38 PM, Mauro Carvalho Chehab
>>> <mchehab@redhat.com> wrote:
>>>> Yet, this doesn't require any changes at DVB API, as all that the demodulator
>>>> need to know is the sub-carrier parameters (frequency, roll-off, symbol
>>>> rate, etc).
>>>
>>> You do: this is why there were changes to the V3 API to accomodate
>>> DVB-S2, which eventually became V5. The major change that underwent is
>>> the addition of newer modulations. The demodulator need to be
>>> explicitly told of the modulation. With some demodulators, the
>>> modulation order could be detected from the PL signaling, rather than
>>> the user space application telling it.
>>
>> DVB-S2 doesn't require DVB bandwidth to be specified.

What I meant to say is that DVB-S2 doesn't require that the bandwidth to be
provided via DVBv5 API.

> stb0899:
> 		switch (state->delsys) {
> 		case SYS_DVBS:
> 		case SYS_DSS:
>                                   ......
> 			if (state->config->tuner_set_bandwidth)
> 				state->config->tuner_set_bandwidth(fe, (13 *
> (stb0899_carr_width(state) + SearchRange)) / 10);
> 			if (state->config->tuner_get_bandwidth)
> 				state->config->tuner_get_bandwidth(fe, &internal->tuner_bw);
>                                .......
>     			break;
> 		case SYS_DVBS2:
>                                   ......
> 			if (state->config->tuner_set_bandwidth)
> 				state->config->tuner_set_bandwidth(fe, (stb0899_carr_width(state)
> + SearchRange));
> 			if (state->config->tuner_get_bandwidth)
> 				state->config->tuner_get_bandwidth(fe, &internal->tuner_bw);
>     			break;
> 
> 
> cx24116:
> 
> 	/* Set/Reset B/W */
> 	cmd.args[0x00] = CMD_BANDWIDTH;
> 	cmd.args[0x01] = 0x01;
> 	cmd.len = 0x02;
> 	ret = cx24116_cmd_execute(fe, &cmd);
> 	if (ret != 0)
> 		return ret;
> 
> 
> stv090x does a lot of auto detection for almost everything, but still:
> 
> stv090x:
> 			if (state->algo == STV090x_COLD_SEARCH)
> 				state->tuner_bw = (15 * (stv090x_car_width(state->srate,
> state->rolloff) + 10000000)) / 10;
> 			else if (state->algo == STV090x_WARM_SEARCH)
> 				state->tuner_bw = stv090x_car_width(state->srate, state->rolloff)
> + 10000000;
> 		}

Frontends of course need it. The code at stv090x is clear:
	state->tuner_bw = stv090x_car_width(state->srate, state->rolloff)

...

static u32 stv090x_car_width(u32 srate, enum stv090x_rolloff rolloff)
{
	u32 ro;

	switch (rolloff) {
	case STV090x_RO_20:
		ro = 20;
		break;
	case STV090x_RO_25:
		ro = 25;
		break;
	case STV090x_RO_35:
	default:
		ro = 35;
		break;
	}

	return srate + (srate * ro) / 100;
}

Regards,
Mauro