Em Tue, 15 Jan 2013 16:49:07 +0200 Antti Palosaari <crope@xxxxxx> escreveu: > On 01/15/2013 03:10 PM, Mauro Carvalho Chehab wrote: > > Em Tue, 15 Jan 2013 11:34:37 +0200 > > Antti Palosaari <crope@xxxxxx> escreveu: > > > >> On 01/15/2013 04:30 AM, Mauro Carvalho Chehab wrote: > >> > >>> v6: Add DocBook documentation. > >>> v7: Some fixes as suggested by Antti > >>> v8: Documentation fix, compilation fix and name the stats struct, > >>> for its reusage inside the core > >>> v9: counters need 32 bits. So, change the return data types to > >>> s32/u32 types > >>> v10: Counters changed to 64 bits for monotonic increment > >>> Don't create a separate get_stats callback. get_frontend > >>> is already good enough for it. > >> > >> Is there way to return BER as rate, or should it be calculated by the > >> application (from total and error bit counts)? > > > > I don't think it makes sense to let such calculus happen inside Kernel. > > It is very easy for userspace to get both numbers at the same ioctl call, > > convert from u64 to float and do a float point division in userspace. > > There may be some devices, having firmware, which calculates BER > directly instead of returning bit counts. Anyhow, returning bit counts > is clearly most common and it is always possible to calculate some > average bit counts from the BER. Yeah, converting from BER to bit count should be trivial. > > Also, BER is calculated before and after the inner coding (pre-BER and > post-BER). But lets the other (post-BER?) later if there is really need. Yes. > I am fine with that. > > > In order to handling tose two u64 numbers in kernelspace, the math will > > be tricky to avoid overflow. It would also require some scale for BER, > > as BER is always a fractional number, generally expressed in E-06 or E-09. > > > >> You seems to change value to 64 bit already, which is enough. 32bit is > >> absolutely too small, it will overflow in seconds (practically around > >> 10sec when there is radio channel of 32MHz and quite optimal conditions). > > > > Yes, 32 bits can cause overflow very quick. > > > >> It is 64bit returned to userspace, is it? > > > > Yes. > > > >> Does 64bit calculations causes any complexity of Kernel or app space? > > > > On the Kernel side, nothing complex was introduced on the frontend I > > added it. See the mb86a20s_get_stats function there: > > http://git.linuxtv.org/mchehab/experimental.git/blob/refs/heads/stats:/drivers/media/dvb-frontends/mb86a20s.c#l934 > > > > The logic that fills it is here, at line 953: > > ... > > rc = mb86a20s_get_ber_before_vterbi(fe, i, &bit_error, &bit_count); > > if (rc >= 0) { > > c->bit_error.stat[1 + i].scale = FE_SCALE_COUNTER; > > c->bit_error.stat[1 + i].uvalue += bit_error; > > c->bit_count.stat[1 + i].scale = FE_SCALE_COUNTER; > > c->bit_count.stat[1 + i].uvalue += bit_count; > > ... > > > > What it was a little more complex were the calculus of a "global" BER measure > > for the (up to) 3 layers. I rewrote that code a few times, until I got > > satisfied with it. The thing is that, as each layer works like an independent > > channel, the BER measure for each layer happens on a different moment. > > > > So, I was in doubt if total BER measure should wait for all layer stats to > > be measured or if it should start to appear when the first layer starts to > > have statistics. I decided for the second. > > > > On the userspace, I did just a very quick hack at late night yesterday > > in order to be able to better see the statistics measures together. > > I don't expect any problem to handle those measures there through. > > As you can see, the patch that gets all stats is very simple: > > > > http://git.linuxtv.org/mchehab/experimental-v4l-utils.git/commitdiff/fffeedfd683033c3d97e0b8c781e7486203a0568 > > > > What it is missed there is to do the division to convert bit error into BER, > > and a logic that would display "dB" or "dBm" if the signal strength/CNR > > measures are scaled in dB. > > > > Btw, I'm even in doubt if we should implement the stats ENUM property. > > Currently, all unsupported properties are returned with len=0. So, I don't > > see any need to have a separate ioctl for that. Perhaps we can just get > > rid of it, in order to simplify the API. > > > > Displaying the per-layer stats there can be a little tricky. It probably > > only makes sense to display one layer at dvbv5-zap application: the layer > > that matches the filtered channel. I need to investigate a little more to > > check how to do such match. Maybe the dvbv5-scan application will need to > > be able to parse some MPEG descriptor to get such data. > > > >> Basically, that API is more complex that I would like to see, but I can > >> live with it. I still fear making too complex API causes same problems > >> as we has currently... lack of app support. > > > > While coding both drivers and userspace, I didn't fill it to be complex. > > At kernelspace, all it was needed were to fill the len for those stats > > measures that would be used. Then, to fill the value and the scale when > > the measure get available, or to mark them as unavailable, if they > > disappear (for example, broadcaster may dynamically change the layers > > layout, so one layer measure could disappear at runtime). > > > > at userspace, just one ioctl is enough to get all stats: > > > > dvb_prop[0].cmd = DTV_QOS_SIGNAL_STRENGTH; > > dvb_prop[1].cmd = DTV_QOS_CNR; > > dvb_prop[2].cmd = DTV_QOS_BIT_ERROR_COUNT; > > dvb_prop[3].cmd = DTV_QOS_TOTAL_BITS_COUNT; > > dvb_prop[4].cmd = DTV_QOS_ERROR_BLOCK_COUNT; > > dvb_prop[5].cmd = DTV_QOS_TOTAL_BLOCKS_COUNT; > > props.num = 6; > > props.props = dvb_prop; > > > > if (ioctl(parms->fd, FE_GET_PROPERTY, &props) == -1) > > perror("FE_GET_PROPERTY"); > > > > A simple display mechanism to display all values would be this one: > > > > for (i = 0; i < 6; i++) { > > for (j = 0; j < dvb_prop[i].u.st.len; j++) { > > if (dvb_prop[i].u.st.stat[j].scale != FE_SCALE_NOT_AVAILABLE) > > printf("%s[%d] = %u\n", dvb_v5_name[dvb_prop[i].cmd], j, (unsigned int)dvb_prop[i].u.st.stat[j].uvalue); > > } > > } > > > > Or, if just the global value is enough: > > > > for (i = 0; i < 6; i++) { > > if (dvb_prop[i].u.st.stat[0].scale != FE_SCALE_NOT_AVAILABLE) > > printf("%s = %u\n", dvb_v5_name[dvb_prop[i].cmd], (unsigned int)dvb_prop[i].u.st.stat[0].uvalue); > > } > > > > > > Of course, for BER, we would do, instead: > > > > double BER = ((double)dvb_prop[i].u.st.stat[2].uvalue) / dvb_prop[i].u.st.stat[5].uvalue; > > > > I am a little bit lazy to read all those patches, but I assume it is > possible: > * return SNR (CNR) as both dB and linear? > * return signal strength as both dBm and linear? The current API doesn't allow. Driver should report it either as dB or as linear. That shouldn't be a problem on userspace, as doing a log()/exp() on userspace is trivial. > And what happens when when multiple statistics are queried, but fronted > cannot perform all those? > > Lets say SS, SNR, BER, UCB are queried, but only SS and SNR are ready to > be returned, whilst rest are not possible? As I remember DVBv5 API is > broken by design and cannot return error code per request. The one(s) not available will have "FE_SCALE_NOT_AVAILABLE" as scale, and its value is undefined. That actually happens on mb86a20s: signal strength is available all the time, but BER measures take some time. When I start the application, I receive a few measures with just signal strength, then layer A or layer B BER measures (together with global) appear, and finally, the other layer BER measure pops up. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
