v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C

v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C

[Manu Abraham]

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



[-- Attachment #2: 0006-DVB-Use-a-unique-delivery-system-identifier-for-DVBC.patch --]
[-- Type: text/x-patch, Size: 1171 bytes --]

From 92a79a1e0a1b5403f06f60661f00ede365b10108 Mon Sep 17 00:00:00 2001
From: Manu Abraham <abraham.manu@gmail.com>
Date: Thu, 24 Nov 2011 17:09:09 +0530
Subject: [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C,
 just like any other. DVBC_ANNEX_A and DVBC_ANNEX_C have slightly
 different parameters and used in 2 geographically different
 locations.

Signed-off-by: Manu Abraham <abraham.manu@gmail.com>
---
 include/linux/dvb/frontend.h |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/include/linux/dvb/frontend.h b/include/linux/dvb/frontend.h
index f80b863..a3c7623 100644
--- a/include/linux/dvb/frontend.h
+++ b/include/linux/dvb/frontend.h
@@ -335,7 +335,7 @@ typedef enum fe_rolloff {
 
 typedef enum fe_delivery_system {
 	SYS_UNDEFINED,
-	SYS_DVBC_ANNEX_AC,
+	SYS_DVBC_ANNEX_A,
 	SYS_DVBC_ANNEX_B,
 	SYS_DVBT,
 	SYS_DSS,
@@ -352,8 +352,13 @@ typedef enum fe_delivery_system {
 	SYS_DAB,
 	SYS_DVBT2,
 	SYS_TURBO,
+	SYS_DVBC_ANNEX_C,
 } fe_delivery_system_t;
 
+
+#define SYS_DVBC_ANNEX_AC	SYS_DVBC_ANNEX_A
+
+
 struct dtv_cmds_h {
 	char	*name;		/* A display name for debugging purposes */
 
-- 
1.7.1

Re: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C

[Mauro Carvalho Chehab]

On 10-12-2011 02:43, Manu Abraham wrote:


> From 92a79a1e0a1b5403f06f60661f00ede365b10108 Mon Sep 17 00:00:00 2001
> From: Manu Abraham <abraham.manu@gmail.com>
> Date: Thu, 24 Nov 2011 17:09:09 +0530
> Subject: [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C,
>  just like any other. DVBC_ANNEX_A and DVBC_ANNEX_C have slightly
>  different parameters and used in 2 geographically different
>  locations.
>
> Signed-off-by: Manu Abraham <abraham.manu@gmail.com>
> ---
>  include/linux/dvb/frontend.h |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/dvb/frontend.h b/include/linux/dvb/frontend.h
> index f80b863..a3c7623 100644
> --- a/include/linux/dvb/frontend.h
> +++ b/include/linux/dvb/frontend.h
> @@ -335,7 +335,7 @@ typedef enum fe_rolloff {
>
>  typedef enum fe_delivery_system {
>  	SYS_UNDEFINED,
> -	SYS_DVBC_ANNEX_AC,
> +	SYS_DVBC_ANNEX_A,
>  	SYS_DVBC_ANNEX_B,
>  	SYS_DVBT,
>  	SYS_DSS,
> @@ -352,8 +352,13 @@ typedef enum fe_delivery_system {
>  	SYS_DAB,
>  	SYS_DVBT2,
>  	SYS_TURBO,
> +	SYS_DVBC_ANNEX_C,
>  } fe_delivery_system_t;
>
> +
> +#define SYS_DVBC_ANNEX_AC	SYS_DVBC_ANNEX_A
> +
> +
>  struct dtv_cmds_h {
>  	char	*name;		/* A display name for debugging purposes */

This patch conflicts with the approach given by this patch:

	http://www.mail-archive.com/linux-media@vger.kernel.org/msg39262.html

merged as commit 39ce61a846c8e1fa00cb57ad5af021542e6e8403.

The approach there were to allow calls to SYS_DVBC_ANNEX_AC to replace the Annex A
roll-off factor of 0.15 by the one used on Annex C (0.13).

As this patch didn't show-up at an stable version, we can still change it to use a
separate delivery system for DVB-C annex C, but this patch needs to be reverted, and
a few changes on existing drivers are needed (drxk, xc5000 and tda18271c2dd explicitly
supports both standards).

So, let's discuss it a little more, and then take a decision on what approach to take.
In any case, I suggest to remove this patch from the series, to not impact on merging
the remaining ones, and add it on another patch series that would contain the needed
changes on other places, if we decide to go on that direction.

-

What ITU-T J83 04/97 defines for Annex C is:

	The system employs the transport multiplexing based on MPEG-2 (see Reference [2]),
	guaranteeing interoperability with other media such as digital broadcasting,
	ISDN networks or packaged media. The framing structure and the channel
	coding are the same as in Annex A. The modulation is 64-QAM, and the QAM symbol
	rate and the roll-off factor are optimized for the 6 MHz channel plan.

So, as stated there, Annex C is a sub-set of Annex A, in terms of modulation, and uses a
smaller roll-off factor (0.13, instead of 0.15).

Also, all Annex A demods need to support QAM64, so it handles Annex C. Several of
them explicitly says that.

For the tuner, the only difference is how to estimate the needed bandwidth,
given a desired signal rate, in order to select the low-pass filter between 6MHz and
7MHz (in practice, affecting only Annex A, as, Annex C is not used on any Country
using more than 6MHz bandwidth).

In other words, SYS_DVBC_ANNEX_AC actually makes sense, as this covers the range of
devices that are able to work with both ways. I'm not aware of any pure Annex C demod.
If we ever found any, then I think we'll need a SYS_DVBC_ANNEX_C. Otherwise, it seems
to be an over-design to add it, while we are not aware of any driver needing it.

If we agree to replace SYS_DVBC_ANNEX_AC by SYS_DVBC_ANNEX_A/SYS_DVBC_ANNEX_C,
this means that it is needed to review every driver that supports SYS_DVBC_ANNEX_AC and
the FE_QAM logic, in order to be sure that the support for SYS_DVBC_ANNEX_C would be
there for all those drivers that support it, otherwise it would be a regression.

Regards,
Mauro

Re: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C

[Manu Abraham]

On Sat, Dec 10, 2011 at 5:59 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> On 10-12-2011 02:43, Manu Abraham wrote:
>
>
>> From 92a79a1e0a1b5403f06f60661f00ede365b10108 Mon Sep 17 00:00:00 2001
>> From: Manu Abraham <abraham.manu@gmail.com>
>> Date: Thu, 24 Nov 2011 17:09:09 +0530
>> Subject: [PATCH 06/10] DVB: Use a unique delivery system identifier for
>> DVBC_ANNEX_C,
>>  just like any other. DVBC_ANNEX_A and DVBC_ANNEX_C have slightly
>>  different parameters and used in 2 geographically different
>>  locations.
>>
>> Signed-off-by: Manu Abraham <abraham.manu@gmail.com>
>> ---
>>  include/linux/dvb/frontend.h |    7 ++++++-
>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/dvb/frontend.h b/include/linux/dvb/frontend.h
>> index f80b863..a3c7623 100644
>> --- a/include/linux/dvb/frontend.h
>> +++ b/include/linux/dvb/frontend.h
>> @@ -335,7 +335,7 @@ typedef enum fe_rolloff {
>>
>>  typedef enum fe_delivery_system {
>>        SYS_UNDEFINED,
>> -       SYS_DVBC_ANNEX_AC,
>> +       SYS_DVBC_ANNEX_A,
>>        SYS_DVBC_ANNEX_B,
>>        SYS_DVBT,
>>        SYS_DSS,
>> @@ -352,8 +352,13 @@ typedef enum fe_delivery_system {
>>        SYS_DAB,
>>        SYS_DVBT2,
>>        SYS_TURBO,
>> +       SYS_DVBC_ANNEX_C,
>>  } fe_delivery_system_t;
>>
>> +
>> +#define SYS_DVBC_ANNEX_AC      SYS_DVBC_ANNEX_A
>> +
>> +
>>  struct dtv_cmds_h {
>>        char    *name;          /* A display name for debugging purposes */
>
>
> This patch conflicts with the approach given by this patch:
>
>        http://www.mail-archive.com/linux-media@vger.kernel.org/msg39262.html
>
> merged as commit 39ce61a846c8e1fa00cb57ad5af021542e6e8403.
>

- For correct delivery system handling, the delivery system identifier
should be unique. Otherwise patch 01/10 is meaningless for devices
with DVBC_ANNEX_C, facing the same situations.

- Rolloff is provided only in the SI table and is not known prior to a
tune. So users must struggle to find the correct rolloff. So users
must know beforehand their experience what rolloff it is rather than
reading Service Information, which is broken by approach. It is much
easier for a user to state that he is living in Japan or some other
place which is using ANNEX_C, rather than creating confusion that
he has to use DVBC and rolloff of 0.15 or is it multiplied by a factor
of 10 or was it 100 ? (Oh, my god my application Y uses a factor
of 10, the X application uses 100 and the Z application uses 1000).
What a lovely confusing scenario. ;-) (Other than for the mentioned
issue that the rolloff can be read from the SI, which is available after
tuning; for tuning you need rolloff).

Really sexy setup indeed. ;-)

One thing that I should warn/mention to you is the lack of clarity on
what you say. You say that you want more discussion, but you
whack in patches which is never discussed, breaking many logical
concepts and ideas and broken by nature. How do you justify
yourself ? I don't think you can justify yourself.


> The approach there were to allow calls to SYS_DVBC_ANNEX_AC to replace the
> Annex A
> roll-off factor of 0.15 by the one used on Annex C (0.13).
>
> As this patch didn't show-up at an stable version, we can still change it to
> use a
> separate delivery system for DVB-C annex C, but this patch needs to be
> reverted, and
> a few changes on existing drivers are needed (drxk, xc5000 and tda18271c2dd
> explicitly
> supports both standards).
>

As I mentioned earlier, the patches were sent in the order that was
being worked upon. It is not complete, for all devices that are using
DVBC_ANNEX_C. Only the TDA18271, TDA18271DD were worked upon
initially.

Re: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C

[Mauro Carvalho Chehab]

On 12-12-2011 01:59, Manu Abraham wrote:
> On Sat, Dec 10, 2011 at 5:59 PM, Mauro Carvalho Chehab
> <mchehab@redhat.com>  wrote:
>> On 10-12-2011 02:43, Manu Abraham wrote:
>>
>>
>>>  From 92a79a1e0a1b5403f06f60661f00ede365b10108 Mon Sep 17 00:00:00 2001
>>> From: Manu Abraham<abraham.manu@gmail.com>
>>> Date: Thu, 24 Nov 2011 17:09:09 +0530
>>> Subject: [PATCH 06/10] DVB: Use a unique delivery system identifier for
>>> DVBC_ANNEX_C,
>>>   just like any other. DVBC_ANNEX_A and DVBC_ANNEX_C have slightly
>>>   different parameters and used in 2 geographically different
>>>   locations.
>>>
>>> Signed-off-by: Manu Abraham<abraham.manu@gmail.com>
>>> ---
>>>   include/linux/dvb/frontend.h |    7 ++++++-
>>>   1 files changed, 6 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/include/linux/dvb/frontend.h b/include/linux/dvb/frontend.h
>>> index f80b863..a3c7623 100644
>>> --- a/include/linux/dvb/frontend.h
>>> +++ b/include/linux/dvb/frontend.h
>>> @@ -335,7 +335,7 @@ typedef enum fe_rolloff {
>>>
>>>   typedef enum fe_delivery_system {
>>>         SYS_UNDEFINED,
>>> -       SYS_DVBC_ANNEX_AC,
>>> +       SYS_DVBC_ANNEX_A,
>>>         SYS_DVBC_ANNEX_B,
>>>         SYS_DVBT,
>>>         SYS_DSS,
>>> @@ -352,8 +352,13 @@ typedef enum fe_delivery_system {
>>>         SYS_DAB,
>>>         SYS_DVBT2,
>>>         SYS_TURBO,
>>> +       SYS_DVBC_ANNEX_C,
>>>   } fe_delivery_system_t;
>>>
>>> +
>>> +#define SYS_DVBC_ANNEX_AC      SYS_DVBC_ANNEX_A
>>> +
>>> +
>>>   struct dtv_cmds_h {
>>>         char    *name;          /* A display name for debugging purposes */
>>
>>
>> This patch conflicts with the approach given by this patch:
>>
>>         http://www.mail-archive.com/linux-media@vger.kernel.org/msg39262.html
>>
>> merged as commit 39ce61a846c8e1fa00cb57ad5af021542e6e8403.
>>
>
> - For correct delivery system handling, the delivery system identifier
> should be unique. Otherwise patch 01/10 is meaningless for devices
> with DVBC_ANNEX_C, facing the same situations.

This is a good point.
>
> - Rolloff is provided only in the SI table and is not known prior to a
> tune. So users must struggle to find the correct rolloff. So users
> must know beforehand their experience what rolloff it is rather than
> reading Service Information, which is broken by approach. It is much
> easier for a user to state that he is living in Japan or some other
> place which is using ANNEX_C, rather than creating confusion that
> he has to use DVBC and rolloff of 0.15

Userspace can present it as Japan and hide the technical details. Most
applications do that already: kaffeine, w_scan, ...

The dvb-apps utils don't do it, but the scan file format it produces
doesn't support anything besides DVB-C/DVB-S/DVB-T/ATSC anyway.

> or is it multiplied by a factor
> of 10 or was it 100 ? (Oh, my god my application Y uses a factor
> of 10, the X application uses 100 and the Z application uses 1000).
> What a lovely confusing scenario. ;-) (Other than for the mentioned
> issue that the rolloff can be read from the SI, which is available after
> tuning; for tuning you need rolloff).

Sorry, but this argument doesn't make any sense to me. The same problem
exists on DVB-S2 already, where several rolloffs are supported. Except
if someone would code a channels.conf line in hand, the roll-off is not
visible by the end user.

> Really sexy setup indeed. ;-)
>
> One thing that I should warn/mention to you is the lack of clarity on
> what you say. You say that you want more discussion, but you
> whack in patches which is never discussed, breaking many logical
> concepts and ideas and broken by nature. How do you justify
> yourself ? I don't think you can justify yourself.

If we have a consensus around your approach, I'm OK to move for it,
provided that it doesn't cause regressions upstream.

As I said, this requires reviewing all DVB frontends to be sure that
they won't break, especially if is there any that it is capable of
auto-detecting the roll-off factor.

Both approaches have advantages and disadvantages.

The main advantage of my approach is that it is coherent with the current
DVBv5 API (e. g. SYS_DVBC_ANNEX_AC). So, the only changes are at the
frontends that need to decide between Annex A and Annex C (currently, only
drx-k - and the tuners used with it).

Advantages on your approach:
	- Cleaner for the userspace API;
	- It is possible to add Annex C only devices.
Disadvantages:
	- Need to deprecate the existing SYS_DVBC_ANNEX_AC;
	- The alias that SYS_DVBC_ANNEX_AC means only SYS_DVBC_ANNEX_A might
	  break some thing;
	- Requires further changes at the DocBook API description;
	- Need to review all DVB-C frontends.

If we're willing to take your approach, we need a patch series that addresses
all DVB-C frontends, to be sure that no regressions were added due to the change
ofSYS_DVBC_ANNEX_AC meaning.

It also requires that FE_QAM to be mapped to be both SYS_DVBC_ANNEX_A and SYS_DVBC_ANNEX_C,
if isn't there any issue for it to work with Annex C mode, or to just
SYS_DVBC_ANNEX_A, if there is enough confidence that such device doesn't
work at allwith annex C.

>> The approach there were to allow calls to SYS_DVBC_ANNEX_AC to replace the
>> Annex A
>> roll-off factor of 0.15 by the one used on Annex C (0.13).
>>
>> As this patch didn't show-up at an stable version, we can still change it to
>> use a
>> separate delivery system for DVB-C annex C, but this patch needs to be
>> reverted, and
>> a few changes on existing drivers are needed (drxk, xc5000 and tda18271c2dd
>> explicitly
>> supports both standards).
>>
>
> As I mentioned earlier, the patches were sent in the order that was
> being worked upon. It is not complete, for all devices that are using
> DVBC_ANNEX_C. Only the TDA18271, TDA18271DD were worked upon
> initially.

Ok. As I said, it is possible to change to your approach, but it requires
a patch series that addresses the frontends that currently supports DVB-C.
There aren't many, so maybe this is not much work.

$ git grep -l FE_QAM drivers/media/dvb/frontends/ drivers/media/common/tuners/
drivers/media/common/tuners/tda18271-fe.c
drivers/media/common/tuners/tda827x.c
drivers/media/common/tuners/tuner-xc2028.c
drivers/media/common/tuners/xc5000.c
drivers/media/dvb/frontends/cxd2820r_core.c
drivers/media/dvb/frontends/drxk_hard.c
drivers/media/dvb/frontends/dvb_dummy_fe.c
drivers/media/dvb/frontends/stv0297.c
drivers/media/dvb/frontends/stv0367.c
drivers/media/dvb/frontends/tda10021.c
drivers/media/dvb/frontends/tda10023.c
drivers/media/dvb/frontends/tda18271c2dd.c
drivers/media/dvb/frontends/ves1820.c

I did a quick research at the Internet for the above:
	- tda827x has just a frequency table. Nothing needs to change
	  there;
	- xc2028 is a false hit: itdoesn't implement DVB-C;
	- cxd2820r says: Integrated matched filter 0.15 roll-off factor
		http://www.framos-imaging.com/fileadmin/img/sony_cxd2820r.pdf
	- dvb_dummy_fe doesn't need changes;
	- tda18271, xc5000, drxk and tda18271c2dd for sure require changes;
	- stv0297 is fully compliant with ITU-T J.83 Annexes A/C, according
	  to:
		http://www.st.com/internet/imag_video/product/159180.jsp
	- stv0367 is fully compliant with ITU-T J.83 Annexes A/C, according
	  with its data brief:
		http://www.st.com/internet/com/TECHNICAL_RESOURCES/TECHNICAL_LITERATURE/DATA_BRIEF/DM00030322.pdf

	- tda10021 datasheet says that it has a programmable half
	Nyquist filter (roll off = 0.15 or 0.13):
		http://www.datasheetcatalog.org/datasheet/philips/TDA10021.pdf
	- tda10023 factsheet says that it is Fully compliant DVB-C (Annex A and C)
	  and MCNS (Annex B) decoders:
		http://www.datasheetarchive.com/indexdl/Datasheets-DAV2/DSADA0022343.pdf
	- vez1820 says Half Nyquist filters (roll off = 15 %).
	- xc5000, drxk and tda18271c2dd drivers are prepared to support both
	  standards;
	- tda18271 is a worldwide tuner that supports all ITU-T J.83 annexes.
	  it requires the same approach used on xc5000/tda18271c2dd drivers to
	  adjust the low-pass filter based on signal rate and roll-off factor.

In summary:

It seems that there are two Annex A-only frontends: cxd2820r and ves1820.
All the others are dual Annex A/Annex C.

This is actually a very good reason to implement your approach,
as otherwise, it would be hard for userspace to detect that those
two devices that lack Annex C.

This also means that just doing an alias from FE_QAM and SYS_DVBC_ANNEX_AC to
SYS_DVBC_ANNEX_A may break something, as, for most devices, SYS_DVBC_ANNEX_AC
really means both Annex A and C.

I didn't look inside the drivers for stv0297, stv0367, tda10021 and tda10023.
I suspect that some will need an additional code to change the roll-off, based on
the delivery system.


Regards,
Mauro

Re: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C

[Manu Abraham]

On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> On 12-12-2011 01:59, Manu Abraham wrote:
>>
>> On Sat, Dec 10, 2011 at 5:59 PM, Mauro Carvalho Chehab
>> <mchehab@redhat.com>  wrote:
>>>
>>> On 10-12-2011 02:43, Manu Abraham wrote:
>>>
>>>
>>>>  From 92a79a1e0a1b5403f06f60661f00ede365b10108 Mon Sep 17 00:00:00 2001
>>>> From: Manu Abraham<abraham.manu@gmail.com>
>>>> Date: Thu, 24 Nov 2011 17:09:09 +0530
>>>> Subject: [PATCH 06/10] DVB: Use a unique delivery system identifier for
>>>> DVBC_ANNEX_C,
>>>>  just like any other. DVBC_ANNEX_A and DVBC_ANNEX_C have slightly
>>>>  different parameters and used in 2 geographically different
>>>>  locations.
>>>>
>>>> Signed-off-by: Manu Abraham<abraham.manu@gmail.com>
>>>> ---
>>>>  include/linux/dvb/frontend.h |    7 ++++++-
>>>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/include/linux/dvb/frontend.h b/include/linux/dvb/frontend.h
>>>> index f80b863..a3c7623 100644
>>>> --- a/include/linux/dvb/frontend.h
>>>> +++ b/include/linux/dvb/frontend.h
>>>> @@ -335,7 +335,7 @@ typedef enum fe_rolloff {
>>>>
>>>>  typedef enum fe_delivery_system {
>>>>        SYS_UNDEFINED,
>>>> -       SYS_DVBC_ANNEX_AC,
>>>> +       SYS_DVBC_ANNEX_A,
>>>>        SYS_DVBC_ANNEX_B,
>>>>        SYS_DVBT,
>>>>        SYS_DSS,
>>>> @@ -352,8 +352,13 @@ typedef enum fe_delivery_system {
>>>>        SYS_DAB,
>>>>        SYS_DVBT2,
>>>>        SYS_TURBO,
>>>> +       SYS_DVBC_ANNEX_C,
>>>>  } fe_delivery_system_t;
>>>>
>>>> +
>>>> +#define SYS_DVBC_ANNEX_AC      SYS_DVBC_ANNEX_A
>>>> +
>>>> +
>>>>  struct dtv_cmds_h {
>>>>        char    *name;          /* A display name for debugging purposes
>>>> */
>>>
>>>
>>>
>>> This patch conflicts with the approach given by this patch:
>>>
>>>
>>>  http://www.mail-archive.com/linux-media@vger.kernel.org/msg39262.html
>>>
>>> merged as commit 39ce61a846c8e1fa00cb57ad5af021542e6e8403.
>>>
>>
>> - For correct delivery system handling, the delivery system identifier
>> should be unique. Otherwise patch 01/10 is meaningless for devices
>> with DVBC_ANNEX_C, facing the same situations.
>
>
> This is a good point.
>
>>
>> - Rolloff is provided only in the SI table and is not known prior to a
>> tune. So users must struggle to find the correct rolloff. So users
>> must know beforehand their experience what rolloff it is rather than
>> reading Service Information, which is broken by approach. It is much
>> easier for a user to state that he is living in Japan or some other
>> place which is using ANNEX_C, rather than creating confusion that
>> he has to use DVBC and rolloff of 0.15
>
>
> Userspace can present it as Japan and hide the technical details. Most
> applications do that already: kaffeine, w_scan, ...
>
> The dvb-apps utils don't do it, but the scan file format it produces
> doesn't support anything besides DVB-C/DVB-S/DVB-T/ATSC anyway.
>
>
>> or is it multiplied by a factor
>> of 10 or was it 100 ? (Oh, my god my application Y uses a factor
>> of 10, the X application uses 100 and the Z application uses 1000).
>> What a lovely confusing scenario. ;-) (Other than for the mentioned
>> issue that the rolloff can be read from the SI, which is available after
>> tuning; for tuning you need rolloff).
>
>
> Sorry, but this argument doesn't make any sense to me. The same problem
> exists on DVB-S2 already, where several rolloffs are supported. Except
> if someone would code a channels.conf line in hand, the roll-off is not
> visible by the end user.



DVB-S2 as what we see as broadcast has just a single rolloff. The same
rolloff is used in the SI alone. It's a mistake to handle rollolff as a user
input field. The other rolloff's are used for very specific applications,
such as DSNG, DVB-RCS etc, where bandwidth has to be really
conserved considering uplinks from trucks, vans etc; for which we don't
even have applications or users.



>
>> Really sexy setup indeed. ;-)
>>
>> One thing that I should warn/mention to you is the lack of clarity on
>> what you say. You say that you want more discussion, but you
>> whack in patches which is never discussed, breaking many logical
>> concepts and ideas and broken by nature. How do you justify
>> yourself ? I don't think you can justify yourself.
>
>
> If we have a consensus around your approach, I'm OK to move for it,
> provided that it doesn't cause regressions upstream.
>
> As I said, this requires reviewing all DVB frontends to be sure that
> they won't break, especially if is there any that it is capable of
> auto-detecting the roll-off factor.
>
> Both approaches have advantages and disadvantages.
>
> The main advantage of my approach is that it is coherent with the current
> DVBv5 API (e. g. SYS_DVBC_ANNEX_AC). So, the only changes are at the
> frontends that need to decide between Annex A and Annex C (currently, only
> drx-k - and the tuners used with it).
>
> Advantages on your approach:
>        - Cleaner for the userspace API;
>        - It is possible to add Annex C only devices.
> Disadvantages:
>        - Need to deprecate the existing SYS_DVBC_ANNEX_AC;
>        - The alias that SYS_DVBC_ANNEX_AC means only SYS_DVBC_ANNEX_A might
>          break some thing;
>        - Requires further changes at the DocBook API description;
>        - Need to review all DVB-C frontends.
>
> If we're willing to take your approach, we need a patch series that
> addresses
> all DVB-C frontends, to be sure that no regressions were added due to the
> change
> ofSYS_DVBC_ANNEX_AC meaning.
>
> It also requires that FE_QAM to be mapped to be both SYS_DVBC_ANNEX_A and
> SYS_DVBC_ANNEX_C,
> if isn't there any issue for it to work with Annex C mode, or to just
> SYS_DVBC_ANNEX_A, if there is enough confidence that such device doesn't
> work at allwith annex C.
>
>
>>> The approach there were to allow calls to SYS_DVBC_ANNEX_AC to replace
>>> the
>>> Annex A
>>> roll-off factor of 0.15 by the one used on Annex C (0.13).
>>>
>>> As this patch didn't show-up at an stable version, we can still change it
>>> to
>>> use a
>>> separate delivery system for DVB-C annex C, but this patch needs to be
>>> reverted, and
>>> a few changes on existing drivers are needed (drxk, xc5000 and
>>> tda18271c2dd
>>> explicitly
>>> supports both standards).
>>>
>>
>> As I mentioned earlier, the patches were sent in the order that was
>> being worked upon. It is not complete, for all devices that are using
>> DVBC_ANNEX_C. Only the TDA18271, TDA18271DD were worked upon
>> initially.
>
>
> Ok. As I said, it is possible to change to your approach, but it requires
> a patch series that addresses the frontends that currently supports DVB-C.
> There aren't many, so maybe this is not much work.
>
> $ git grep -l FE_QAM drivers/media/dvb/frontends/
> drivers/media/common/tuners/
> drivers/media/common/tuners/tda18271-fe.c
> drivers/media/common/tuners/tda827x.c
> drivers/media/common/tuners/tuner-xc2028.c
> drivers/media/common/tuners/xc5000.c
> drivers/media/dvb/frontends/cxd2820r_core.c
> drivers/media/dvb/frontends/drxk_hard.c
> drivers/media/dvb/frontends/dvb_dummy_fe.c
> drivers/media/dvb/frontends/stv0297.c
> drivers/media/dvb/frontends/stv0367.c
> drivers/media/dvb/frontends/tda10021.c
> drivers/media/dvb/frontends/tda10023.c
> drivers/media/dvb/frontends/tda18271c2dd.c
> drivers/media/dvb/frontends/ves1820.c
>
> I did a quick research at the Internet for the above:
>        - tda827x has just a frequency table. Nothing needs to change
>          there;
>        - xc2028 is a false hit: itdoesn't implement DVB-C;
>        - cxd2820r says: Integrated matched filter 0.15 roll-off factor
>                http://www.framos-imaging.com/fileadmin/img/sony_cxd2820r.pdf
>        - dvb_dummy_fe doesn't need changes;
>        - tda18271, xc5000, drxk and tda18271c2dd for sure require changes;
>        - stv0297 is fully compliant with ITU-T J.83 Annexes A/C, according
>          to:
>                http://www.st.com/internet/imag_video/product/159180.jsp
>        - stv0367 is fully compliant with ITU-T J.83 Annexes A/C, according
>          with its data brief:
>
>  http://www.st.com/internet/com/TECHNICAL_RESOURCES/TECHNICAL_LITERATURE/DATA_BRIEF/DM00030322.pdf
>
>        - tda10021 datasheet says that it has a programmable half
>        Nyquist filter (roll off = 0.15 or 0.13):
>
>  http://www.datasheetcatalog.org/datasheet/philips/TDA10021.pdf
>        - tda10023 factsheet says that it is Fully compliant DVB-C (Annex A
> and C)
>          and MCNS (Annex B) decoders:
>
>  http://www.datasheetarchive.com/indexdl/Datasheets-DAV2/DSADA0022343.pdf
>        - vez1820 says Half Nyquist filters (roll off = 15 %).
>        - xc5000, drxk and tda18271c2dd drivers are prepared to support both
>          standards;
>        - tda18271 is a worldwide tuner that supports all ITU-T J.83 annexes.
>          it requires the same approach used on xc5000/tda18271c2dd drivers
> to
>          adjust the low-pass filter based on signal rate and roll-off
> factor.
>
> In summary:
>
> It seems that there are two Annex A-only frontends: cxd2820r and ves1820.
> All the others are dual Annex A/Annex C.
>
> This is actually a very good reason to implement your approach,
> as otherwise, it would be hard for userspace to detect that those
> two devices that lack Annex C.



Ah, it's good to know that you've collected the list of drivers to be fixed. :-)



> This also means that just doing an alias from FE_QAM and SYS_DVBC_ANNEX_AC
> to
> SYS_DVBC_ANNEX_A may break something, as, for most devices,
> SYS_DVBC_ANNEX_AC
> really means both Annex A and C.



With the current approach, the application can determine whether
the hardware supports through the DELSYS enumeration.

So, if you have a device that needs to support both ANNEX_A and
ANNEX_C, it should be rather doing

case DTV_ENUM_DELSYS:
         buffer.data[0] = SYS_DVBC_ANNEX_A;
         buffer.data[1] = SYS_DVBC_ANNEX_C;
         break;


> I didn't look inside the drivers for stv0297, stv0367, tda10021 and
> tda10023.
> I suspect that some will need an additional code to change the roll-off,
> based on
> the delivery system.



Of course, yes this would need to make the change across multiple
drivers.

We can fix the drivers, that's no issue at all, as it is a small change.

Regards,
Manu

Re: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C

[Mauro Carvalho Chehab]

On 12-12-2011 11:40, Manu Abraham wrote:
> On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab

>> This also means that just doing an alias from FE_QAM and SYS_DVBC_ANNEX_AC
>> to
>> SYS_DVBC_ANNEX_A may break something, as, for most devices,
>> SYS_DVBC_ANNEX_AC
>> really means both Annex A and C.
>
>
>
> With the current approach, the application can determine whether
> the hardware supports through the DELSYS enumeration.
>
> So, if you have a device that needs to support both ANNEX_A and
> ANNEX_C, it should be rather doing
>
> case DTV_ENUM_DELSYS:
>           buffer.data[0] = SYS_DVBC_ANNEX_A;
>           buffer.data[1] = SYS_DVBC_ANNEX_C;
>           break;

Sure, but we'll need a logic to handle queries for SYS_DVBC_ANNEX_AC
anyway, if any of the existing DVB-C drivers is currently prepared to
support both.

I'm not concerned with drx-k. The support for both standards are for
kernel 3.3. So, no backward compatibility is needed here.

While there is no explicit option, the code for stv0297, stv0367,
tda10021 and tda10023 drivers are not clear if they support both
(maybe roll-off might be auto-detected?) or just SYS_DVBC_ANNEX_A.

That's said, the difference between a 0.15 and a 0.13 rolloff is not big.
I won't doubt that a demod set to 0.15 rolloff would be capable of working
(non-optimized) with a 0.13 rolloff.

What I'm saing is that, if any of the existing drivers currently works
with both Annex A and Annex C, we'll need something equivalent to:

if (delsys == SYS_DVBC_ANNEX_AC) {
	int ret = try_annex_a();
	if (ret < 0)
		ret = try_annex_c();
}

For FE_SET_FRONTEND (and the corresponding v5 logic), in order to avoid
regressions.

>
>
>> I didn't look inside the drivers for stv0297, stv0367, tda10021 and
>> tda10023.
>> I suspect that some will need an additional code to change the roll-off,
>> based on
>> the delivery system.
>
>
>
> Of course, yes this would need to make the change across multiple
> drivers.
>
> We can fix the drivers, that's no issue at all, as it is a small change.

Indeed, it is a small change. Tuners are trivial to change, but, at the
demod, we need to discover if roll-off is auto-detected somehow, or if
they require manual settings, in order to fix the demod drivers.

>
> Regards,
> Manu

Re: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C

[Manu Abraham]

On Mon, Dec 12, 2011 at 7:26 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> On 12-12-2011 11:40, Manu Abraham wrote:
>>
>> On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab
>
>
>>> This also means that just doing an alias from FE_QAM and
>>> SYS_DVBC_ANNEX_AC
>>> to
>>> SYS_DVBC_ANNEX_A may break something, as, for most devices,
>>> SYS_DVBC_ANNEX_AC
>>> really means both Annex A and C.
>>
>>
>>
>>
>> With the current approach, the application can determine whether
>> the hardware supports through the DELSYS enumeration.
>>
>> So, if you have a device that needs to support both ANNEX_A and
>> ANNEX_C, it should be rather doing
>>
>> case DTV_ENUM_DELSYS:
>>          buffer.data[0] = SYS_DVBC_ANNEX_A;
>>          buffer.data[1] = SYS_DVBC_ANNEX_C;
>>          break;
>
>
> Sure, but we'll need a logic to handle queries for SYS_DVBC_ANNEX_AC
> anyway, if any of the existing DVB-C drivers is currently prepared to
> support both.
>
> I'm not concerned with drx-k. The support for both standards are for
> kernel 3.3. So, no backward compatibility is needed here.
>
> While there is no explicit option, the code for stv0297, stv0367,
> tda10021 and tda10023 drivers are not clear if they support both
> (maybe roll-off might be auto-detected?) or just SYS_DVBC_ANNEX_A.
>
> That's said, the difference between a 0.15 and a 0.13 rolloff is not big.
> I won't doubt that a demod set to 0.15 rolloff would be capable of working
> (non-optimized) with a 0.13 rolloff.
>
> What I'm saing is that, if any of the existing drivers currently works
> with both Annex A and Annex C, we'll need something equivalent to:
>
> if (delsys == SYS_DVBC_ANNEX_AC) {
>        int ret = try_annex_a();
>        if (ret < 0)
>                ret = try_annex_c();
> }
>
> For FE_SET_FRONTEND (and the corresponding v5 logic), in order to avoid
> regressions.


What I was implying:

set_frontend/search()
{
     case SYS_DVBC_ANNEX_A:
              // do whatever you need to do for annex A tuning and return
              break;
     case SYS_DVBC_ANNEX_C:
              // do whatever you need to do for annex C tuning and return
              break;
}


ANNEX_AC is a link to ANNEX_A; We never had any ? users to ANNEX_C, so
that issue might be simple to ignore.

Re: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C

[Mauro Carvalho Chehab]

On 12-12-2011 13:00, Manu Abraham wrote:
> On Mon, Dec 12, 2011 at 7:26 PM, Mauro Carvalho Chehab
> <mchehab@redhat.com>  wrote:
>> On 12-12-2011 11:40, Manu Abraham wrote:
>>>
>>> On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab
>>
>>
>>>> This also means that just doing an alias from FE_QAM and
>>>> SYS_DVBC_ANNEX_AC
>>>> to
>>>> SYS_DVBC_ANNEX_A may break something, as, for most devices,
>>>> SYS_DVBC_ANNEX_AC
>>>> really means both Annex A and C.
>>>
>>>
>>>
>>>
>>> With the current approach, the application can determine whether
>>> the hardware supports through the DELSYS enumeration.
>>>
>>> So, if you have a device that needs to support both ANNEX_A and
>>> ANNEX_C, it should be rather doing
>>>
>>> case DTV_ENUM_DELSYS:
>>>           buffer.data[0] = SYS_DVBC_ANNEX_A;
>>>           buffer.data[1] = SYS_DVBC_ANNEX_C;
>>>           break;
>>
>>
>> Sure, but we'll need a logic to handle queries for SYS_DVBC_ANNEX_AC
>> anyway, if any of the existing DVB-C drivers is currently prepared to
>> support both.
>>
>> I'm not concerned with drx-k. The support for both standards are for
>> kernel 3.3. So, no backward compatibility is needed here.
>>
>> While there is no explicit option, the code for stv0297, stv0367,
>> tda10021 and tda10023 drivers are not clear if they support both
>> (maybe roll-off might be auto-detected?) or just SYS_DVBC_ANNEX_A.
>>
>> That's said, the difference between a 0.15 and a 0.13 rolloff is not big.
>> I won't doubt that a demod set to 0.15 rolloff would be capable of working
>> (non-optimized) with a 0.13 rolloff.
>>
>> What I'm saing is that, if any of the existing drivers currently works
>> with both Annex A and Annex C, we'll need something equivalent to:
>>
>> if (delsys == SYS_DVBC_ANNEX_AC) {
>>         int ret = try_annex_a();
>>         if (ret<  0)
>>                 ret = try_annex_c();
>> }
>>
>> For FE_SET_FRONTEND (and the corresponding v5 logic), in order to avoid
>> regressions.
>
>
> What I was implying:
>
> set_frontend/search()
> {
>       case SYS_DVBC_ANNEX_A:
>                // do whatever you need to do for annex A tuning and return
>                break;
>       case SYS_DVBC_ANNEX_C:
>                // do whatever you need to do for annex C tuning and return
>                break;
> }
>
>
> ANNEX_AC is a link to ANNEX_A;

Yes, I saw your approach.

> We never had any ? users to ANNEX_C, so
> that issue might be simple to ignore.

This is hard to say. What I'm saying is that, if any of the current
drivers works as-is with Annex C, we should assume that someone is using,
as we don't have any evidence otherwise.

I'm sure there are lots of people running Linux in Japan.

How many of them are using the DVB subsystem is hard to say. The low message
traffic at the ML for people *.jp is not a good measure, as due to language
barriers, people may not be posting things there.

A quick grep here on my local copy of the ML traffic (it currently has stored
about 380 days of email, as I moved the older ones to a separate storage space)
still shows 90 messages that has ".jp" inside:

$ grep -l "\.jp" * |wc -l
      90

41 of them has the word DVB inside. Ok, there are some false positives there
too (due to *.jpg), but there are some valid hits also,

Including a commit on this changeset: e38030f3ff02684eb9e25e983a03ad318a10a2ea.
As the cx23885 driver does support DVB-C with stv0367, maybe the committer
might be using it for DVB-C.

Even if not, I suspect that it is likely to have some DVB-C Annex C users
out there.

Regards,
Mauro.

Re: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C

[Manu Abraham]

On Mon, Dec 12, 2011 at 9:52 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> On 12-12-2011 13:00, Manu Abraham wrote:
>>
>> On Mon, Dec 12, 2011 at 7:26 PM, Mauro Carvalho Chehab
>> <mchehab@redhat.com>  wrote:
>>>
>>> On 12-12-2011 11:40, Manu Abraham wrote:
>>>>
>>>>
>>>> On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab
>>>
>>>
>>>
>>>>> This also means that just doing an alias from FE_QAM and
>>>>> SYS_DVBC_ANNEX_AC
>>>>> to
>>>>> SYS_DVBC_ANNEX_A may break something, as, for most devices,
>>>>> SYS_DVBC_ANNEX_AC
>>>>> really means both Annex A and C.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> With the current approach, the application can determine whether
>>>> the hardware supports through the DELSYS enumeration.
>>>>
>>>> So, if you have a device that needs to support both ANNEX_A and
>>>> ANNEX_C, it should be rather doing
>>>>
>>>> case DTV_ENUM_DELSYS:
>>>>          buffer.data[0] = SYS_DVBC_ANNEX_A;
>>>>          buffer.data[1] = SYS_DVBC_ANNEX_C;
>>>>          break;
>>>
>>>
>>>
>>> Sure, but we'll need a logic to handle queries for SYS_DVBC_ANNEX_AC
>>> anyway, if any of the existing DVB-C drivers is currently prepared to
>>> support both.
>>>
>>> I'm not concerned with drx-k. The support for both standards are for
>>> kernel 3.3. So, no backward compatibility is needed here.
>>>
>>> While there is no explicit option, the code for stv0297, stv0367,
>>> tda10021 and tda10023 drivers are not clear if they support both
>>> (maybe roll-off might be auto-detected?) or just SYS_DVBC_ANNEX_A.
>>>
>>> That's said, the difference between a 0.15 and a 0.13 rolloff is not big.
>>> I won't doubt that a demod set to 0.15 rolloff would be capable of
>>> working
>>> (non-optimized) with a 0.13 rolloff.
>>>
>>> What I'm saing is that, if any of the existing drivers currently works
>>> with both Annex A and Annex C, we'll need something equivalent to:
>>>
>>> if (delsys == SYS_DVBC_ANNEX_AC) {
>>>        int ret = try_annex_a();
>>>        if (ret<  0)
>>>                ret = try_annex_c();
>>> }
>>>
>>> For FE_SET_FRONTEND (and the corresponding v5 logic), in order to avoid
>>> regressions.
>>
>>
>>
>> What I was implying:
>>
>> set_frontend/search()
>> {
>>      case SYS_DVBC_ANNEX_A:
>>               // do whatever you need to do for annex A tuning and return
>>               break;
>>      case SYS_DVBC_ANNEX_C:
>>               // do whatever you need to do for annex C tuning and return
>>               break;
>> }
>>
>>
>> ANNEX_AC is a link to ANNEX_A;
>
>
> Yes, I saw your approach.
>
>
>> We never had any ? users to ANNEX_C, so
>> that issue might be simple to ignore.
>
>
> This is hard to say. What I'm saying is that, if any of the current
> drivers works as-is with Annex C, we should assume that someone is using,
> as we don't have any evidence otherwise.
>
> I'm sure there are lots of people running Linux in Japan.
>
> How many of them are using the DVB subsystem is hard to say. The low message
> traffic at the ML for people *.jp is not a good measure, as due to language
> barriers, people may not be posting things there.
>
> A quick grep here on my local copy of the ML traffic (it currently has
> stored
> about 380 days of email, as I moved the older ones to a separate storage
> space)
> still shows 90 messages that has ".jp" inside:
>
> $ grep -l "\.jp" * |wc -l
>     90
>
> 41 of them has the word DVB inside. Ok, there are some false positives there
> too (due to *.jpg), but there are some valid hits also,
>
> Including a commit on this changeset:
> e38030f3ff02684eb9e25e983a03ad318a10a2ea.
> As the cx23885 driver does support DVB-C with stv0367, maybe the committer
> might be using it for DVB-C.
>
> Even if not, I suspect that it is likely to have some DVB-C Annex C users
> out there.


As far as I am aware, most of the services use BCAS2 encryption. There
is no BCAS2 support available as Open Source, other than with sundtek.


Regards,
Manu

Re: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C

[Andreas Oberritter]

On 12.12.2011 14:56, Mauro Carvalho Chehab wrote:
> On 12-12-2011 11:40, Manu Abraham wrote:
>> On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab
> 
>>> This also means that just doing an alias from FE_QAM and
>>> SYS_DVBC_ANNEX_AC
>>> to
>>> SYS_DVBC_ANNEX_A may break something, as, for most devices,
>>> SYS_DVBC_ANNEX_AC
>>> really means both Annex A and C.
>>
>>
>>
>> With the current approach, the application can determine whether
>> the hardware supports through the DELSYS enumeration.
>>
>> So, if you have a device that needs to support both ANNEX_A and
>> ANNEX_C, it should be rather doing
>>
>> case DTV_ENUM_DELSYS:
>>           buffer.data[0] = SYS_DVBC_ANNEX_A;
>>           buffer.data[1] = SYS_DVBC_ANNEX_C;
>>           break;
> 
> Sure, but we'll need a logic to handle queries for SYS_DVBC_ANNEX_AC
> anyway, if any of the existing DVB-C drivers is currently prepared to
> support both.
> 
> I'm not concerned with drx-k. The support for both standards are for
> kernel 3.3. So, no backward compatibility is needed here.
> 
> While there is no explicit option, the code for stv0297, stv0367,
> tda10021 and tda10023 drivers are not clear if they support both
> (maybe roll-off might be auto-detected?) or just SYS_DVBC_ANNEX_A.

tda10021: Driver sets roll-off to 0.15. No auto-detection.
tda10023: Driver sets roll-off to 0.18. No auto-detection.

In general, auto-detection seems unlikely. Do you know any chip that
does it? Unless you do, we shouldn't expect it to exist. stv0297 doesn't
even detect IQ inversion.

> That's said, the difference between a 0.15 and a 0.13 rolloff is not big.
> I won't doubt that a demod set to 0.15 rolloff would be capable of working
> (non-optimized) with a 0.13 rolloff.
> 
> What I'm saing is that, if any of the existing drivers currently works
> with both Annex A and Annex C, we'll need something equivalent to:
> 
> if (delsys == SYS_DVBC_ANNEX_AC) {
>     int ret = try_annex_a();
>     if (ret < 0)
>         ret = try_annex_c();
> }

I'd prefer treating ANNEX_AC just like ANNEX_A. It won't break anything,
because register writes for ANNEX_A will be the same. i.e. applications
using SYS_DVBC_ANNEX_AC will still get the same result as before.

What may change for a user: An updated application may allow him to
select between A and C, if the frontend advertises both. In this case,
both A and C are supposed to work, depending on the location of the
user. Someone who successfully used his tuner in Japan before, and who's
frontend doesn't advertise C, will still be able to choose A and thus
use the same register settings as before.

>>> I didn't look inside the drivers for stv0297, stv0367, tda10021 and
>>> tda10023.
>>> I suspect that some will need an additional code to change the roll-off,
>>> based on
>>> the delivery system.
>>
>>
>>
>> Of course, yes this would need to make the change across multiple
>> drivers.
>>
>> We can fix the drivers, that's no issue at all, as it is a small change.
> 
> Indeed, it is a small change. Tuners are trivial to change, but, at the
> demod, we need to discover if roll-off is auto-detected somehow, or if
> they require manual settings, in order to fix the demod drivers.

tda10021: Register 0x3d & 0x01: 0 -> 0.15; 1 -> 0.13
tda10023: Register 0x3d & 0x03: 2 -> 0.15; 3 -> 0.13

Regards,
Andreas

Re: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C

[Mauro Carvalho Chehab]

Em 12-12-2011 19:24, Andreas Oberritter escreveu:
> On 12.12.2011 14:56, Mauro Carvalho Chehab wrote:
>> On 12-12-2011 11:40, Manu Abraham wrote:
>>> On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab
>>
>>>> This also means that just doing an alias from FE_QAM and
>>>> SYS_DVBC_ANNEX_AC
>>>> to
>>>> SYS_DVBC_ANNEX_A may break something, as, for most devices,
>>>> SYS_DVBC_ANNEX_AC
>>>> really means both Annex A and C.
>>>
>>>
>>>
>>> With the current approach, the application can determine whether
>>> the hardware supports through the DELSYS enumeration.
>>>
>>> So, if you have a device that needs to support both ANNEX_A and
>>> ANNEX_C, it should be rather doing
>>>
>>> case DTV_ENUM_DELSYS:
>>>           buffer.data[0] = SYS_DVBC_ANNEX_A;
>>>           buffer.data[1] = SYS_DVBC_ANNEX_C;
>>>           break;
>>
>> Sure, but we'll need a logic to handle queries for SYS_DVBC_ANNEX_AC
>> anyway, if any of the existing DVB-C drivers is currently prepared to
>> support both.
>>
>> I'm not concerned with drx-k. The support for both standards are for
>> kernel 3.3. So, no backward compatibility is needed here.
>>
>> While there is no explicit option, the code for stv0297, stv0367,
>> tda10021 and tda10023 drivers are not clear if they support both
>> (maybe roll-off might be auto-detected?) or just SYS_DVBC_ANNEX_A.
> 
> tda10021: Driver sets roll-off to 0.15. No auto-detection.
> tda10023: Driver sets roll-off to 0.18. No auto-detection.
> 
> In general, auto-detection seems unlikely. Do you know any chip that
> does it? Unless you do, we shouldn't expect it to exist. stv0297 doesn't
> even detect IQ inversion.
> 
>> That's said, the difference between a 0.15 and a 0.13 rolloff is not big.
>> I won't doubt that a demod set to 0.15 rolloff would be capable of working
>> (non-optimized) with a 0.13 rolloff.
>>
>> What I'm saing is that, if any of the existing drivers currently works
>> with both Annex A and Annex C, we'll need something equivalent to:
>>
>> if (delsys == SYS_DVBC_ANNEX_AC) {
>>     int ret = try_annex_a();
>>     if (ret < 0)
>>         ret = try_annex_c();
>> }
> 
> I'd prefer treating ANNEX_AC just like ANNEX_A. It won't break anything,
> because register writes for ANNEX_A will be the same. i.e. applications
> using SYS_DVBC_ANNEX_AC will still get the same result as before.
> 
> What may change for a user: An updated application may allow him to
> select between A and C, if the frontend advertises both. In this case,
> both A and C are supposed to work, depending on the location of the
> user. Someone who successfully used his tuner in Japan before, and who's
> frontend doesn't advertise C, will still be able to choose A and thus
> use the same register settings as before.


As all existing DVB-C drivers currently upstream seem to be assuming a
Annex A, I don't have any troubles on doing that.


> 
>>>> I didn't look inside the drivers for stv0297, stv0367, tda10021 and
>>>> tda10023.
>>>> I suspect that some will need an additional code to change the roll-off,
>>>> based on
>>>> the delivery system.
>>>
>>>
>>>
>>> Of course, yes this would need to make the change across multiple
>>> drivers.
>>>
>>> We can fix the drivers, that's no issue at all, as it is a small change.
>>
>> Indeed, it is a small change. Tuners are trivial to change, but, at the
>> demod, we need to discover if roll-off is auto-detected somehow, or if
>> they require manual settings, in order to fix the demod drivers.
> 
> tda10021: Register 0x3d & 0x01: 0 -> 0.15; 1 -> 0.13
> tda10023: Register 0x3d & 0x03: 2 -> 0.15; 3 -> 0.13

Thanks for the info!

I'll prepare a patchset with Manu's patch 06 on it, plus the required
changes at the DocBook specs and the fixes for the drx-k based drivers
and for tda10021/tda10023. I should be sending the patches to the ML
later today.

> 
> 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: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C

[Manu Abraham]

On Tue, Dec 13, 2011 at 1:09 AM, Thomas Kernen <tkernen@deckpoint.ch> wrote:
> On 12/12/11 2:40 PM, Manu Abraham wrote:
>
>>>> or is it multiplied by a factor
>>>> of 10 or was it 100 ? (Oh, my god my application Y uses a factor
>>>> of 10, the X application uses 100 and the Z application uses 1000).
>>>> What a lovely confusing scenario. ;-) (Other than for the mentioned
>>>> issue that the rolloff can be read from the SI, which is available after
>>>> tuning; for tuning you need rolloff).
>>>
>>>
>>>
>>> Sorry, but this argument doesn't make any sense to me. The same problem
>>> exists on DVB-S2 already, where several rolloffs are supported. Except
>>> if someone would code a channels.conf line in hand, the roll-off is not
>>> visible by the end user.
>>
>>
>>
>>
>> DVB-S2 as what we see as broadcast has just a single rolloff. The same
>> rolloff is used in the SI alone. It's a mistake to handle rollolff as a
>> user
>> input field. The other rolloff's are used for very specific applications,
>> such as DSNG, DVB-RCS etc, where bandwidth has to be really
>> conserved considering uplinks from trucks, vans etc; for which we don't
>> even have applications or users.
>
>
> AFAIK there is at least one card (TBS 6925) that is supporting DVB-S2
> applications aimed normally at contribution markets and whereby the rolloff
> may need to be specified.

As far as I am aware, that card uses a STV0900 or a 903,
more likely it is a STV0903 being a single input device.
The STV0900/903 chips are capable of auto detecting the
rolloff. All it needs is frequency and symbol rate.

Even if it is another different demodulator:

All the devices that I have seen which support the advanced
MODCOD's, they support auto detection of rolloff. AFAIK,
this is readable of the BBHEADER: MATYPE1, represented
by 2 bits, as specified as in the DVB-S2 specification.
There are other fields along with such as Single/Multiple
Input Streams etc.

Therefore no user intervention is required to determine
rolloff on such devices. (It is read directly off the BBHEADER
by the demod) and is available to the driver.

Regards,
Manu