Re: [PATCH] Re: [PATCH] Multi protocol support (stage #1)

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

 



On Sun, 30 Apr 2006, Manu Abraham wrote:
> Updated the patch again, keeping in mind all the comments. Thanks to all
> who provided feedback to improve it further.
> Attached is the latest changes.

The modcod stuff..

+struct modcod_table {
+struct modcod_table dvbs2_modcod_lookup[] = {
+       { FE_MODCOD_DUMMY_PLFRAME,      FE_MOD_NONE,    FE_FECRATE_NONE },
+int decode_dvbs2_modcod(struct dvb_frontend *fe, enum fe_modcod modcod)

It looks like the assignement of certain values to modulation-fec rate
combinations is specific one demodulator's registers.  Is there anything in
the standard that says the value 3 corrisponds to QPSK and 2/5?

IMHO, the modcod_table stuff should go in the driver for the hardware that
uses it.  If a new drivers appears that can make use of the same code, then it
can be moved into the core at that time.  The modcod enum doesn't belong in
the public header file since nothing in the public interface uses it.

+ *     FE_MOD_AUTO, Some devices/drivers, might support
+ *     auto detecting the modulation type, depending on
+ *     some other settings and or hardware features.
+ */
+enum fe_modulations {
+       FE_MOD_IGNORE                   = (0 <<  0),
+       FE_MOD_NONE                     = (1 <<  0),
+       FE_MOD_BPSK                     = (1 <<  1),

First, how about adding QAM_512 and QAM_1024 in the expected spot.  There is
already DOCSIS cable modem cardware than can use these modulations.  You might
want to add VSB_16 too, it's part of the ATSC spec.

Second, what's FE_MOD_NONE for?  Is it used for input to the kernel, output
from the kernel, or both?

Third, FE_MOD_AUTO isn't enough.  Take the or51132 demodulator.  It can
demodulate VSB-16, QAM-64, and QAM-256.  It can automatically detect QAM-64 vs
QAM-256, but not QAM vs VSB.  If an app sets the demodulator to MOD_AUTO, what
firmware should it load, qam or vsb?  There is no way for the driver to know.
There is no way for an app to know that it can't just say 'AUTO', it needs to
choose QAM or VSB.  Maybe it would be enough to add QAM_AUTO and PSK_AUTO
demodulations?


+ *     NOTE: All current delivery systems need inversion in some
+ *     way or the other, due to hardware implementation specifics.
+ *     and/or others.

I thought inversion was caused the hardware design and not by the transmitter?
It was something that the card drivers should set, not user apps?

+struct dvbs2_params {
+       enum fe_fecrates                fecrate;
+       enum fe_fecrates                coderate_HP;
+       enum fe_fecrates                coderate_LP;

What is the difference between fecrate and coderate?

+/**
+ *     DVB-T parameters
+ */
+struct dvbt_params {
+       enum fe_modulations             constellation;
+       enum fe_bandwidths              bandwidth;
+       enum fe_fecrates                code_rate_HP;
+       enum fe_fecrates                code_rate_LP;

This uses 'constellation' where everywhere else 'modulation' is used.  Also
'code_rate' vs 'fecrate' or 'coderate'.

+/**
+ *     ATSC parameters
+ */
+struct atsc_params {
+       enum fe_modulations             modulation;
+       enum fe_fecrates                fecrate;
+};

I don't think ATSC supports multiple fec rates?  It only allows for 2/3.

+
+/**
+ *     Padding to handle future binary compatibility issues.
+ *
+ *     NOTE: The padding is a dummy parameter !
+ */
+struct pad_params {
+       __u8 pad[512];
+};

You could just make __u8 pad[512] one of members of the delsys union, then you
wouldn't have to define this struct at all.

_______________________________________________

linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

[Index of Archives]     [Linux Media]     [Video 4 Linux]     [Asterisk]     [Samba]     [Xorg]     [Xfree86]     [Linux USB]

  Powered by Linux