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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12-12-2011 01:59, Manu Abraham wrote:
On Sat, Dec 10, 2011 at 5:59 PM, Mauro Carvalho Chehab
<mchehab@xxxxxxxxxx>  wrote:
On 10-12-2011 02:43, Manu Abraham wrote:


 From 92a79a1e0a1b5403f06f60661f00ede365b10108 Mon Sep 17 00:00:00 2001
From: Manu Abraham<abraham.manu@xxxxxxxxx>
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@xxxxxxxxx>
---
  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@xxxxxxxxxxxxxxx/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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux