On Sat, 22 Apr 2006, Manu Abraham wrote:
> After some rework landed up with a patch like this.
Ok, I've got a few comments.
+typedef enum {
+ FE_TRANSPORT = 0x00000001,
+ FE_GENERIC_PACKET,
+ FE_GENERIC_CONTINUOUS,
+ FE_RESERVED,
+} fe_matype_t;
I take it this was the field that used to be a mirror of fe_delsys_t, but
really was the multiplex stream format? If FE_TRANSPORT is for MPEG / ISO
13818-1 Transport Streams, maybe it should have MPEG in the name? Or is
FE_TRANSPORT for any kind of transport stream?
+ MOD_BPSK_1_4 = 0x40000000,
+ MOD_RESERVED_2 = 0x80000000
+} sat_modcod_t;
There are only two unassigned bits left. I can just picture one year from
now, three new modulation-FEC ratio combos need to be added. Maybe
splitting FEC and modulation, as is done for DVB-C and DVB-T, would make
more sense? I suppose that would make it hard to show the supported
_combinations_ of modulation and FEC ratio.
I also noticed that the same value is used for identical modulation-FEC
combinations for both DVB-S and DVB-S2. If something supported QPSK 1/2
for DVB-S but not DVB-S2, how would that work?
+ MOD_QAM_256 = 0x00000010
+} cab_modcod_t;
+ MOD_COFDM = 0x00000001
+} ter_modcod_t;
I'm guessing 'mod' stands for modulation and 'cod' is for Forward Error
Correction Code Ratio? But these enums don't specify anything about FEC,
just modulation. Also, there are no AUTO modulations, but there is
hardware that can do this.
In the _params for DVB-S, the modulation and FEC ratio are specified as a
single field 'modcod'. For DVB-C, modulation isn't specified at all and
FEC ratio uses a fe_code_rate_t field fec_inner. For DVB-T, there is only
one modulation, but it doesn't use 'modcod' like DVB-S, instead it uses
fe_modulation_t. The FEC ratio isn't specified with 'modcod' like DVB-S,
but uses fe_code_rate_t fields named code_rate_{HP,LP}, instead of
fec_inner like in DVB-C. Or to summarize:
Modulation:
DVB-S sat_modcod_t modcod (modulation-FEC combo field)
DVB-C unspecified
DVB-T fe_modulation_t constellation
FEC ratio(s):
DVB-S sat_modcod_t modcod (modulation-FEC combo field)
DVB-C fe_code_rate_t fec_inner
DVB-T fe_code_rate_t code_rate_HP, code_rate_LP
Why not be consistent?
+struct dvb_frontend_params {
+ __u32 frequency;
+ fe_spectral_inversion_t inversion;
+ struct fe_cap caps;
Why is inversion not part of XXX_params?
+/**
+ * Cable frontend parameters
+ */
+/**
+ * Terrestrial frontend capabilities
+ */
+/**
+ * Satellite frontend parameters
+ */
This is very Eurocentric. The rest of the world has digital Cable,
Terrestrial and Satellite too, but it's not all DVB-C/T/S. Same with the
structure names, ter_params is for your terrestrial parameters, it has
nothing to do with MY terrestrial parameters.
+/**
+ * Case #1
+ * If we make a call FE_GET_PARAMS with modcod=0x00
+ * and do a FE_GET_PARAMS, it will simply return
+ * all the available modulation and coding types used.
+ *
+ * Case #2
+ * calling FE_GET_PARAMS with the corresponding
+ * MODCOD type will return the parameters
+ * available for the corresponding MODCOD
+ */
I don't get it, what is 'modcod'? There is one field named modcod,
(dvb_frontend_params).sat.modcod, that's only for DVB-S. Do you mean
(dvb_frontend_params).caps.delivery? So if you set .caps.delivery=0 then
you get the union of "all the available modulation and coding types" inside
.caps.modulation? That doesn't make sense either. Does it mean that
.caps.delivery will be set to the union of all available delivery systems?
Does FE_GET_PARAMS return the current status of the FE, or does it return
the FE's capabilities? Or both? If both, is it wise to conflate the two
operations? The former requires querying the hardware while the later can
just return a pre-packaged const struct.
_______________________________________________
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
