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

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

 



Hi Manu,

it seems to annoy you that I didn't follow the discussions
about the API closely, and now bother you with my stupid
questions. I'm sorry about that.

But look at it this way: If I'm not able to understand how
to use the API after browsing through the DVB-S2 standard
and reading all the comments in frontend.h, then something
must be wrong -- either with me or with the API. You choose.


On Sat, May 06, 2006, Manu Abraham wrote:
> Johannes Stezenbach wrote:
> >As Trent explained it is used to query supported delivery systems
> >via GET_CAPS. So FE_DELSYS_IGNORE should be renamed to
> >
> >FE_DELSYS_QUERY // use with FE_GET_CAPS to get bit set of delivery systems 
> >supported by hw
> 
> Well, he got it wrong. Well originally in the initial implementation 
> that was there, it was like that, but due to his changes/ideas only it 
> did not make any sense and had to be dropped. I did explain what it was 
> in my last mail. i don't know where Trent got that idea.

I don't get how an application can query the
set of supported delivery systems.

Please explain.

> >>>enum fe_matype isn't used anywhere
> >>>
> >>>also, please always spell out non-obvious acronyms in a comment
> >>>(probably "mode adaption type")
> >>>
> >>>But EN 302 307 defines MATYPE as a two-byte field, this
> >>>enum fe_matype is just the 2bit TS/GS field of the
> >>>whole MATYPE.
> 
> For the frontend we can deduce the params from these values. Basically 
> we need to know the slope (ie, the Rolloff Rate, but it is conveniently 
> described that those slopes will be used for these mentioned types only. 
> So rather than putting numbers, names sounds much better). The rest of 
> the stuff needs to go into the demux and some are just a userspace 
> specific only. (ie, the application tells the demux what it needs, there 
> needs to be other changes than to the demux too. That was the one i 
> talked to you a couple of days along with Ralph, on an earlier mail, for 
> which you said you weren't interested) It has got nothing to do with the 
> demod.
> 
> Well, to put it quite right, to use DVB-S2, Applications have to change 
> , whatever it might be .. There is no shortcut using FRONTEND2 etc, 
> since there are other parameters what userspace will need to tell the 
> DVB-API as a whole, rather than the frontend itself.(Ref: D4 BBHeader EN 
> 302 307, Table D2, p 52). If there are drivers and applications going in 
> for a workaround, they will surely be DVB-S2 incompatible.

Well, you spent more time on DVB-S2 than I did, so probably I am
wrong. Anyway, here is my understanding:

- it is wrong to believe that only the roll off factor is
  relevant for the frontend, and other MATYPE bits are for the demux;
  i.e. the frontend will do frame synchronization, needs to know
  about ACM/CCM, and will output only one stream for MIS transmissions
- there is no 1:1 relationship between the TS/GS field of MATYPE
  and the roll-off factor, these are independent; i.e. your
  enum fe_matype is just named wrong

Certainly the demux would have to be changed for the case where
the demod does not output 188-byte MPEG-2 TS packets. But I
don't think any bits of the two-byte MATYPE field are of
any direct relevance to the demux. They are all for
use by the demod.

I could be totally wrong, of course. What do the data sheets say,
does the demod output the BBHEADER or just the DATA FIELD?

For the common case of HDTV via DVB-S2 I'm quite sure the demod
will just output MPEG-2 TS, otherwise manufacturers would
have to throw away their existing STB chips instead of just
having to stuff a new frontend.

BTW, ARIB in Japan uses a scheme where you have to
configure the frontend with the transport_stream_id. Look at
the V4 API ARIB extensions:
http://linuxtv.org/cgi-bin/viewcvs.cgi/dvb-kernel-v4/linux/include/linux/dvb/frontend.h?rev=1.20.2.2&only_with_tag=ARIB_extension_rev002&view=auto
I believe that the  Input Stream Identifier (MATYPE byte 2)
has a similar function.


> >Maybe fe_mod_types?
> 
> Ok, will change to fe_mod_types.

Duh! I meant fe_mod_type (singular), of course!


> >>>enum fe_modcod isn't used anywhere except the
> >>>decode_dvbs2_modcod() helper function. Either
> >>>use it in the API or move it to dvb_frontend.h.
> >>>      
> >>Compliant drivers uses the modcod straight away. I was still working on 
> >>the STB0899. I was of the thought would add it alongwith it.
> >
> >But it isn't *used* anywhere in the API. Did you mean
> >to add a modcod field to dvbs2_params ?
> >
> >If not, then why is fe_modcod in frontend.h?
> 
> I will be putting it into the dvbs2 struct. Have only 2 hands and a 
> complex device with 2 chips and many people hitting left and right (many 
> thanks to the people who did and are still helping) and there was some 
> posts on the ML, which did not seem right for the same, which would've 
> got me involved sending long posts. (For some people it might be fun 
> just to poke here and there without even going through the specs) So 
> only i did not comment on someone's earlier comment. Did not have the 
> time for that. Sorry, wasn't anything else

Hey, by sending a "Mauro, please merge" mail you advertised
the API as kind of stable. Now you have to deal with my
critisising each and every little mistake ;-)


> >>>I don't fully understand the DVB-S2 spec yet, but I think
> >>>the way to select the stream is by using an 8bit
> >>>input_stream_identifier.
> >>>      
> >>H.3 describes it. 8 bits isn;t needed, anyway boiling down to the demod, 
> >>that doesn't happen, it is a simple knob to select between the streams.

Actually Appendix F is the backwards-compatible HP/LP mode.
However, the discription doesn't make it clear how to
select between the two streams. Maybe this works with
a single-bit HP/LP selection in the hardware, similar
to DVB-T (App. F mentions EN 300 744).

H.3 OTOH uses the merger/slicer desribed in 5.1.5, which to my
understanding uses the Input Stream Identifier to
select the stream.

> >You mean the dvbs2_delivery_system description from EN 300 468
> >is irrelevant for the current hardware? I.e. availbale
> >DVB-S2 tuners only support "normative broadcast mode"?
> 
> No, As i understood from the specs, Normative broadcast mode uses a 
> larger slope and larger Transmit powers. For other modes, to reduce 
> transmit powers, hence a steeper fall off, for the Nyquist filter. 
> (Nyquist Filter Rolloff Rate "Alpha") Continuing just below down..

S2_satellite_delivery_system_descriptor for non-backwards-compatible
mode gives you scrambling_sequence_index and input_stream_identifier.

If your hardware doesn't support these, then I guess your
hardware doesn't support non-backwards-compatible DVB-S2 mode.


> >(I wouldn't be surprised).
> >It also explains why enum fe_matype isn't used. I vote
> >for removing it for now.
> 
> It is used, for setting up the Rolloff rates. I just used it in 
> conjunction with the names so that it looks a bit cleaner.
> Ref: P 18, 302 307.

see above; there is no 1:1 relationship between RO and TS/GS
(Table 3: MATYPE-1 field mapping)

> The STB0899 does support it. It is completely 
> according to the specs and not one step to the left or right.
> It adheres to it completely, to the specs
...
> enum stb0899_rrc_alpha {
>    STB0899_RRC_20,
>    STB0899_RRC_25,
>    STB0899_RRC_35
> };

RO is used in backwards-compatible mode, and given
in satellite delivery system descriptor.

TS/GS is not. Big difference.

> What you were looking at the rest of the Mode Adaptation, it has to be 
> in the demux, so there will be an enumeration in the demux API too. Well 
> as it is for the frontend API itself there has been more noise than 
> useful comments, wondering what it would be for the demux API next.

I think you are wrong on this.


> >How about not deprecating FE_GET_INFO, but just deprecate the "type"
> >and "caps" fields of struct dvb_frontend_info?
> >No need to query name and min/max stuff over and over...
> 
> Why ? He got it wrong ! What if a multi standard tuner has different 
> min/max values ?

Good point.

> But there is enough explanation that i have put in the comments. I don't 
> know why Trent interpret's it differently.

I disagree. You already noticed I wasn't able to figure it out
from the comments.

> Or is more comments necessary ? But i don't know what more can be 
> written for a flag in there, in each struct.

If you can explain to me how it works (querying set of supported
delivery systems) I could suggest some comments which would help other
to understand it, too.


> >>>>+	enum fe_inversion		inversion;
> >
> >What does the inversion do? dvb-core emulates FE_INVERSION_AUTO
> >if the hw can't do it, so caps would always report full
> >capabilities here, right?
> 
> Well, the driver uses it's own algorithm. And cannot use the 
> dvb_frontend_thread.
> It has separate search algorithms for DVB-S and DVB-S2 modes

Doesn't matter who implements it, the important part is that
the application will never have to worry about it, because
either the driver or dvb-core implement FE_INVERSION_AUTO.

(The app can query the actual inversion and supply it next time
to speed up tuning, but it can rely on FE_INVERSION_AUTO
always working.)


> >I'm still not sure about the DVB-S2 API. So I would prefer
> >if the whole change would not be merged into mainline
> >until there is at least one fully functional DVB-S2 driver
> >in the tree (i.e. keep it in a v4l-dvb-s2 repo for now).
> >
> >  
> It will be too large a change and hope i would not have to redo the tree 
> again as it happened earlier. Would be nothing more than hard work 
> alone. Since you say it, i will keep it in the stb0899 repository, in 
> the forests @ linuxtv.org  or at, thadathil.net ;-)
> 
> It will be a huge change though, to merge it that way. IMHO, it should 
> be taken in steps, the individual pieces. Well, i am open to either 
> ways. Maybe marking EXPERIMENTAL is better ?
> 
> >If you think this is too much hassle, then at least
> >mark the DVB-S2 stuff as "EXPERIMENTAL / DO NOT USE / SUBJECT
> >TO CHANGE WITHOUT NOTICE" (in capitals). So that you can
> >break binary and source compatibilty for the DVB-S2 part
> >with clean conscience until it is proven stable.
> >
> >Agreed?
> 
> Even if it is in a separate repo, it has to be marked EXPERIMENTAL, IMHO.
> Almost dead now. ;-)

Sorry for being totally anal about this, but someone has to ;-/

An API change which requires all apps to change sooner or later
is a big deal. By the time this hits mainline it should be
reasonably stable, otherwise the wrath of the application
writers will come upon you. Take it from me, you really don't
want this...


Johannes

_______________________________________________

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