Johannes Stezenbach wrote:
On Tue, May 16, 2006, Manu Abraham wrote:
Updated the patch again
The main problem I see is that you still try to cram
too much into the DVB-S2 API.
Well, i was just using the minimal fields from the
s2_satellite_delivery_system. The rest of the stuff is found from the
pilot tones. I mean the physical layer fields. If we don't use those
minimal fields, then we can't call it DVB-S2 anymore. Can we ? To me it
looks like DVB-S then, with additional modulations.
Well, 8PSK modulation != DVB-S2
Adding stuff which might be missing is easy.
Removing stuff which might turn out to be wrong is
just *not possible* without destroying both source
and binary compatibility.
That's why plead again to remove everything which
isn't necessary for receiving the DVB-S2 transmissions
in use today.
The SIS/MIS flag will be needed anyway. Other than that for Optional
Backward Compatibility, rolloff will be needed as well, but in any case
rolloff is queried back from the Physical layer.
+enum dvbfe_modulation {
+ DVBFE_MOD_IGNORE = (0 << 0),
+ DVBFE_MOD_UNKNOWN = (1 << 0),
Both IGNORE and UNKNOWN?
test_api.c doesn't use IGNORE, nor is it documented in frontend.h.
Maybe you just forgot to remove them IGNOREs?
I just did a quick test_api, to show how it can be queried, rather than
an a detailed one, since there was a request for one.
UNKNOWN is needed elsewhere. If it is removed, using ignore in those
places sound a bit ugly. At least as far as a API is concerned , it
looks bad, since it looks a bit abnormal.
IGNORE is used not to query. whereas UNKNOWN, is that which is not
defined, that which it says is RESERVED for example according to MODCOD,
where it uses both MODULATION and FEC, so using IGNORE in there instead
of UNKNOWN modulation, does sound indeed a bit strange.
Anyway UNKNOWN is the only one that can be returned from a multistandard
tuner in an UNKNOWN state. returning IGNORE doesn't make sense in there too.
+ DVBFE_MOD_AUTO = (1 << 30),
+ DVBFE_MOD_DUMMY = (1 << 31)
As per my other mail, DUMMY is not neccessary here.
I agree to the fact that AUTO can be substituted for DUMMY, but it does
cause inconsistencies. One place there will be a DUMMY etc. At least it
should be helpful for people to remember. Well i don't object either to
putting AUTO instead of DUMMY, except for that it looks different at
different locations.
+ * Frontend Inversion (I/Q Swap)
+ */
+enum dvbfe_iqswap {
Why this gratuitiuos rename? What's wrong with "inversion"?
:) Ok, thought a name change might help. Thought maybe some bad omen
with that name. ;-)
+struct dvbfe_dvbs_info {
+struct dvbfe_dss_info {
+struct dvbfe_dvbc_info {
+struct dvbfe_dvbt_info {
+struct dvbfe_dvbh_info {
All this I don't believe are useful. If someone writing
an application can plausibly explain to me how they
would be used, then OK. Until then I vote for leaving them
out of the API. We managed without them quite good so far.
?
Do you mean go back to the first patch where there were 3 systems
basically ? Well that sounds pretty much broken, since first of all
there were huge shouts saying that it was quite "Eurocentric"
But the main argument to go for such a split was that earlier, we had it
based on modulations, but with new delivery systems, it is no more a
single modulation, but a mix of them. (for example ATSC and DVB-S2 are
mismatches in that sense in that outlook)
DVB-T/H is the only one that can be said to have a single modulation.
So in any case this split does really make sense.
+struct dvbfe_dvbs2_info {
This one, maybe, in the future, when we know what the
different capabilities of DVB-S2 frontends actually are.
well, according to the specs, they can detect physical layer info from
the physical later info itself. hence many of those things are not
explained in detail. For example hierarchial modulations and or layered
modulations.
If dvbs2 were to have it's own struct, then i would very well say that
datattype for other delivery systems too do exists, such that it looks
consistent , rather than in one place there is dvbs2_struct, in another
place there is a modulation struct.
+/*
+ * ATSC capability bitfields
+ */
+struct dvbfe_atsc_info {
+ enum dvbfe_modulation modulation;
+
+ __u8 pad[32];
+};
And this might better be ATSC-C vs. ATSC-T?
I've no idea, but yes some kind of way to distinguish
different ATSC delivery systems is necessary.
Or do ATSC people actually prefer it that way?
Well, it depends only on the modulation, nothing else. It came to this
way, since the ATSC people really argued for this. The other one is FEC,
but that which is a single fixed one, so ATSC guys don't really like the
FEC field in there for ATSC.
+struct dvbfe_caps {
+ union {
+ struct dvbfe_dvbs_info dvbs;
...
+struct dvbfe_info {
This mixture of "caps" and "info" is suboptimal.
What would be your suggestion ?
Thanks,
Manu
_______________________________________________
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
