[linux-dvb] [PATCH] add DVB-S2 support to frontend.h

[Marcel Siegert]

On Thursday 02 March 2006 00:53, Johannes Stezenbach wrote:
> On Wed, Mar 01, 2006, Marcel Siegert wrote:
> > On Wednesday 01 March 2006 20:08, Johannes Stezenbach wrote:
> > > On Mon, Feb 27, 2006, Marcel Siegert wrote:
> > > >  typedef enum fe_caps {
> > > >  	FE_IS_STUPID			= 0,
> > > > @@ -62,12 +66,13 @@
> > > >  	FE_CAN_HIERARCHY_AUTO		= 0x100000,
> > > >  	FE_CAN_8VSB			= 0x200000,
> > > >  	FE_CAN_16VSB			= 0x400000,
> > > > +	FE_CAN_DVB_S2			= 0x1000000,
> > > 
> > > This doesn't fit in here. Maybe we can do:
> > what doesn't fit there?
> > how will you act on combo frontends that use e.g. DVB-T and DVB-C?
> > what type is reflected in fe_type_t then?
> 
> fe_caps_t doesn't have FE_CAN_DVB_T etc. so why should it have
> FE_CAN_DVB_S2? This is inconsistent.
ok, acked.

> > >  typedef enum fe_type {
> > >        // values 0 to 3 are reserved for binary compatibilty with old
> > >        // API version
> > >        FE_DVB_S  = (1 << 2),
> > >        FE_DVB_C  = (1 << 3),
> > >        FE_DVB_T  = (1 << 4),
> > >        FE_ATSC   = (1 << 5),
> > >        FE_DVB_S2 = (1 << 6)
> > >  } fe_type_t;
> > these combined? hmm, possible, but this would cause frontend drivers to return FE_QPSK | FE_DVB_S 
> > in case an old application will use the new dvb-core or new apps have to rely on both values because
> > they don't now if kernel dvb-core is an old version. 
> 
> FE_GET_INFO returns the default frontend type, an application
> which wants to use the new features sees the
> FE_HAS_EXTENDED_CAN_VALUES flag (bad name, IMHO), does
> FE_GET_EXTENDED_INFO (or maybe better a more generic FE_GET_CAPS
> in the style of V4 API DVB_VIDEO_GET_CAPS?), and so discovers
> the supported FE types, which it can then switch via FE_SET_STANDARD.
FE_HAS_EXTENDED_CAN_VALUES renamed to FE_HAS_EXTENDED_INFO 
to match the ioctl name.
hmm, FE_GET_CAPS in v4 style would cause 4 single ioctls instead of one.
but, it is more flexible.
i'll think about and come back later on this topic.

 
> Maybe I just didn't get how you suggest apps find out what
> FE types are supported?
> 
> > > and use that in a GET_CAPS return value so apps can see what
> > > values are legal to pass to FE_SET_STANDARD
> > > 
> > > #define FE_QPSK   FE_DVB_S // source compatibility
> > > #define FE_QAM    FE_DVB_C
> > > #define FE_OFDM   FE_DVB_T
> > > 
> > > #define FE_QPSK_OLD 0 // for binary compatibility, handled in drivers,
> > > #define FE_QAM_OLD  1 // should be defined in dvb_frontend.h
> > > #define FE_OFDM_OLD 2
> > > #define FE_ATSC_OLD 3
> > so, we have to overwork all drivers to have FE_QPSK changed to FE_QPSK_OLD? or am i missing a point
> 
> Could be mapped in dvb_frontend.c:dvb_frontend_ioctl(FE_GET_INFO).
> 
> But, Oops, doesn't work for another reason: old binaries would
> expect a different value for FE_QPSK than new ones.
> Next try:
> 
> typedef enum fe_type {
>       FE_QPSK, // legacy, do not use in new applications
>       FE_QAM,  // legacy, do not use in new applications
>       FE_OFDM, // legacy, do not use in new applications
>       FE_ATSC   = 3,
>       FE_DVB_S  = (1 << 2),
>       FE_DVB_C  = (1 << 3),
>       FE_DVB_T  = (1 << 4),
>       FE_DVB_S2 = (1 << 6)
> } fe_type_t;
> 
> A bit ugly, but would work for both old apps and reporting supported
> standards for new apps.
agreed. fixed within my tree, one trap is left :/
if new frontend driver just return e.g. FE_DVB_T automatically old applications would assume that
it is an FE_QPSK (which has enum value 0) frontend and fail. 
i think i will overwork the return values within the frontend drivers to prevent this. 
patch would be supplied afterwards.

> 
> > > > +/* backport from dvb-api v4 - adapted a bit to have the last value at bit 31 so we */
> > > > +/* guarantee that the enum will normally have 32 bits size */
> > > 
> > > What does the "bit 31" comment mean? I think gcc guarantees that
> > > sizeof(enum foo) == sizeof(int), unless you compile with -fshort-enums
> > > (which the kernel doesn't support anyway, I think).
> > should we rely on what gcc developers maybe implement(ed) or optimize?
> > what if someone tries to use the intel c/c++ compiler collection?
> > 
> > on enumeration constants (like we do have for the backported v4 enums) 
> > you're absolutely right. but read what i found on 
> > http://david.tribble.com/text/cdiffs.htm#C99-enum-type
> > 
> > it mainly describes differences between C and C++ but some reasons also just on 
> > c enumerations that may have an effect.
> > my testcase with gcc shows same sizes.
> > 
> > if we should trust gcc, i'll change that.
> 
> You are right with your doubts ;-/
> 
> gcc-3.x info pages are incomplete, but gcc-4.0 info pages say:
> 
>    * `The integer type compatible with each enumerated type (C90
>      6.5.2.2, C99 6.7.2.2).'
> 
>      Normally, the type is `unsigned int' if there are no negative
>      values in the enumeration, otherwise `int'.  If `-fshort-enums' is
>      specified, then if there are negative values it is the first of
>      `signed char', `short' and `int' that can represent all the
>      values, otherwise it is the first of `unsigned char', `unsigned
>      short' and `unsigned int' that can represent all the values.
> 
>      On some targets, `-fshort-enums' is the default; this is
>      determined by the ABI.
> 
> The System V generic ABI http://www.caldera.com/developers/gabi/
> says enums sizes are processor specific, the psABI for i386 and MIPS
> say enums have the same size as int. For other platforms I don't know
> but according to google ARM seems to have -fshort-enums as default.
> Sigh...
> 
so, what are your thoughts on this?
take the v3 e.g. fe_modulation and let them be enum constants?
e.g.
typedef enum fe_code_rate {
	FEC_NONE = 0,
	FEC_1_2 = 1,
	FEC_2_3 = 2,
	FEC_3_4 = 3,
	FEC_4_5 = 4,
	FEC_5_6 = 5,
	FEC_6_7 = 6,
	FEC_7_8 = 7,
	FEC_8_9 = 8,
	FEC_AUTO = 9
} fe_code_rate_t;

of course i would extend them also to contain the new given modulations fec rates ect.

i think that would be fine.

regards
marcel