hi andreas,
On Tuesday 28 February 2006 20:01, Andreas Oberritter wrote:
> Hi Marcel,
>
> On Mon, 2006-02-27 at 22:59 +0100, Marcel Siegert wrote:
> > +/*! describes the available modulation types. */
> > +enum dvb_fe_modulation {
> > + DVB_FE_MOD_QPSK = (1 << 0),
> > + DVB_FE_MOD_QAM_16 = (1 << 1),
> > + DVB_FE_MOD_QAM_32 = (1 << 2),
> > + DVB_FE_MOD_QAM_64 = (1 << 3),
> > + DVB_FE_MOD_QAM_128 = (1 << 4),
> > + DVB_FE_MOD_QAM_256 = (1 << 5),
> > + DVB_FE_MOD_QAM_AUTO = (1 << 6),
> > + DVB_FE_MOD_2_VSB = (1 << 7),
> > + DVB_FE_MOD_4_VSB = (1 << 8),
> > + DVB_FE_MOD_8_VSB = (1 << 9),
> > + DVB_FE_MOD_16_VSB = (1 << 10),
> > + DVB_FE_MOD_8_PSK = (1 << 11),
> > + DVB_FE_MOD_BPSK = (1 << 12),
> > + DVB_FE_MOD_MOD_UNKNOWN = (1 << 31)
> > +};
>
> why do we need DVB_FE_MOD_QAM_AUTO and DVB_FE_MOD_MOD_UNKNOWN? I suggest
> replacing them with DVB_FE_MOD_AUTO. Maybe MOD_UNKNOWN was meant to be
> returned by FE_GET_FRONTEND, but if the modulation is unknown, then it
> is very unlikely that it was possible to lock the frontend and therefore
> calling FE_GET_FRONTEND in such a case seems to be useless. I think
> FE_GET_FRONTEND should only replace the known bits of the tuning
> parameters as it does today.
as i mentioned some lines above within the patch, i took those structs via c&p from the api v4 cvs.
i think that michael had reasons to name them like this, but i do agree with your argumentation.
i changed that inside my tree now.
removed DVB_FE_MOD_QAM_AUTO and DVB_FE_MOD_MOD_UNKNOWN
added DVB_FE_MOD_AUTO
>
> I also wonder why QAM and VSB values are sorted by size, but PSK values
> are sorted alphanumerically.
hmm, within this thread there was a small part, i think johannes did that, that
stated we should name the modulations according to the standard.
the standard calls them 8PSK. i did a small change in ordering these enumeration values
in my tree, but that will occur next time also, when a new modulation type is introduced.
> > +/*! describes common supported frontend capabilities. */
> > +enum dvb_fe_common_cap {
> > + DVB_FE_CAN_INVERSION_AUTO = (1 << 0), /*!< fixme */
> > + DVB_FE_CAN_RECOVER = (1 << 1), /*!< frontend can recover from a cable unplug automatically */
> > + DVB_FE_CAN_MUTE_TS = (1 << 2), /*!< frontend can stop spurious TS data output */
> > + DVB_FE_SUP_HIGH_LNB_VOLTAGE = (1 << 31), /*!< fixme, frontend can deliver higher lnb voltages */
> > +};
>
> Why are there FIXMEs?
as told before (c&p) from api v4 tree - removed "fixme" in comments within my tree.
> > +/*! describes other available, frontend type dependent capabilities. */
> > +enum dvb_fe_other_cap {
> > + DVB_FE_CAN_TRANSMISSION_MODE_AUTO = (1 << 0), /*!< fixme (DVB-T specific) */
> > + DVB_FE_CAN_BANDWIDTH_AUTO = (1 << 1), /*!< fixme (DVB-T specific) */
> > + DVB_FE_CAN_GUARD_INTERVAL_AUTO = (1 << 2), /*!< fixme (DVB-T specific) */
> > + DVB_FE_CAN_HIERARCHY_AUTO = (1 << 31), /*!< fixme (DVB-T specific) */
> > +};
>
> Same here. Well, I know these are specific to DVB-T and proably DVB-H,
> but how are these FIXMEs supposed to get fixed?
i removed the comments. i will ask michael why he tagged them fixme.
> > @@ -126,7 +194,8 @@
> > FE_HAS_SYNC = 0x08, /* found sync bytes */
> > FE_HAS_LOCK = 0x10, /* everything's working... */
> > FE_TIMEDOUT = 0x20, /* no lock within the last ~2 seconds */
> > - FE_REINIT = 0x40 /* frontend was reinitialized, */
> > + FE_REINIT = 0x40, /* frontend was reinitialized, */
> > + FE_EXTENDED_TP = 0x80 /* providing new informations */
> > } fe_status_t; /* application is recommended to reset */
>
> First, it's "new information". What shall be reset and what new kind of
> information is provided through which interface?
comments were mixed up. sorry for that. fixed that - and changed the "new information" comment to
a more usesable description.
-----------snip----------------------
FE_TIMEDOUT = 0x20, /* no lock within the last ~2 seconds */
FE_REINIT = 0x40, /* frontend was reinitialized, */
/* application is recommended to reset */
/* DiSEqC, tone and parameters */
FE_EXTENDED_TP = 0x80 /* dvb_frontend_event does contain the fe_type member and */
/* the dvb_frontend_parameters in addition of __dvb_fronend_parameters_old */
} fe_status_t;
----------snip-----------------------
> > +struct dvb_dvbs2_parameters {
> > + __u32 symbol_rate; /* symbol rate in Symbols per second */
>
> It's symbols, not Symbols.
typo fixed. - also fixed that typo within the dvbs/dvbc/dvbt structs that kept this typo already a long time in cvs ;)
thanks for your participation :)
best regards
marcel
_______________________________________________
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
