On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab
<mchehab@xxxxxxxxxx> wrote:
> 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.
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
--
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
