From: Mauro Carvalho Chehab <mchehab@redhat.com>
To: Antti Palosaari <crope@iki.fi>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>
Subject: Re: [PATCH RFCv10 00/15] DVB QoS statistics API
Date: Tue, 15 Jan 2013 11:10:41 -0200 [thread overview]
Message-ID: <20130115111041.6b78a935@redhat.com> (raw)
In-Reply-To: <50F522AD.8000109@iki.fi>
Em Tue, 15 Jan 2013 11:34:37 +0200
Antti Palosaari <crope@iki.fi> 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.
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;
--
Cheers,
Mauro
next prev parent reply other threads:[~2013-01-15 13:11 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-15 2:30 [PATCH RFCv10 00/15] DVB QoS statistics API Mauro Carvalho Chehab
2013-01-15 2:30 ` [PATCH RFCv10 01/15] mb86a20s: improve error handling at get_frontend Mauro Carvalho Chehab
2013-01-15 2:30 ` [PATCH RFCv10 02/15] dvb: Add DVBv5 stats properties for Quality of Service Mauro Carvalho Chehab
2013-01-15 2:30 ` [PATCH RFCv10 03/15] dvb: the core logic to handle the DVBv5 QoS properties Mauro Carvalho Chehab
2013-01-15 2:30 ` [PATCH RFCv10 04/15] mb86a20s: Update QoS statistics at FE read_status Mauro Carvalho Chehab
2013-01-15 2:30 ` [PATCH RFCv10 05/15] mb86a20s: functions reorder Mauro Carvalho Chehab
2013-01-15 2:30 ` [PATCH RFCv10 06/15] mb86a20s: Fix i2c gate on error Mauro Carvalho Chehab
2013-01-15 2:30 ` [PATCH RFCv10 07/15] mb86a20s: improve debug for RF level Mauro Carvalho Chehab
2013-01-15 2:30 ` [PATCH RFCv10 08/15] mb86a20s: fix interleaving and FEC retrival Mauro Carvalho Chehab
2013-01-15 2:30 ` [PATCH RFCv10 09/15] mb86a20s: convert it to use dev_info/dev_err/dev_dbg Mauro Carvalho Chehab
2013-01-15 2:30 ` [PATCH RFCv10 10/15] mb86a20s: -EBUSY is expected when getting QoS measures Mauro Carvalho Chehab
2013-01-15 2:30 ` [PATCH RFCv10 11/15] mb86a20s: make AGC work better Mauro Carvalho Chehab
2013-01-15 2:30 ` [PATCH RFCv10 12/15] mb86a20s: Some improvements for BER measurement Mauro Carvalho Chehab
2013-01-15 2:30 ` [PATCH RFCv10 13/15] mb86a20s: improve bit error count for BER Mauro Carvalho Chehab
2013-01-15 2:31 ` [PATCH RFCv10 14/15] dvb: increase API version Mauro Carvalho Chehab
2013-01-15 8:20 ` [PATCH RFCv10 00/15] DVB QoS statistics API Johannes Stezenbach
2013-01-15 8:55 ` Antti Palosaari
2013-01-15 12:23 ` Mauro Carvalho Chehab
2013-01-15 9:34 ` Antti Palosaari
2013-01-15 13:10 ` Mauro Carvalho Chehab [this message]
2013-01-15 14:49 ` Antti Palosaari
2013-01-15 15:21 ` Mauro Carvalho Chehab
2013-01-15 15:47 ` Devin Heitmueller
2013-01-15 17:02 ` Mauro Carvalho Chehab
2013-01-15 15:26 ` Antti Palosaari
2013-01-15 17:12 ` Mauro Carvalho Chehab
2013-01-15 20:37 ` Antti Palosaari
2013-01-16 4:26 ` Manu Abraham
2013-01-16 11:41 ` Luca Olivetti
2013-01-16 13:56 ` Mauro Carvalho Chehab
2013-01-16 15:19 ` Manu Abraham
2013-01-16 17:21 ` Mauro Carvalho Chehab
2013-01-16 18:26 ` Manu Abraham
2013-01-16 19:22 ` Mauro Carvalho Chehab
2013-01-16 21:40 ` Manu Abraham
2013-01-16 19:29 ` Simon Farnsworth
2013-01-16 21:37 ` Manu Abraham
2013-01-16 22:11 ` Mauro Carvalho Chehab
2013-01-17 3:26 ` Manu Abraham
2013-01-16 22:01 ` Mauro Carvalho Chehab
2013-01-17 3:40 ` Manu Abraham
2013-01-17 9:33 ` Antti Palosaari
2013-01-17 16:50 ` Mauro Carvalho Chehab
2013-01-17 17:15 ` Antti Palosaari
2013-01-17 18:11 ` Mauro Carvalho Chehab
2013-01-17 18:27 ` Antti Palosaari
2013-01-17 18:37 ` Manu Abraham
2013-01-17 18:50 ` Mauro Carvalho Chehab
2013-01-17 19:11 ` Antti Palosaari
2013-01-17 19:35 ` Mauro Carvalho Chehab
2013-01-17 21:29 ` Manu Abraham
2013-01-17 22:22 ` Antti Palosaari
2013-01-17 22:46 ` Mauro Carvalho Chehab
2013-01-22 12:16 ` Mauro Carvalho Chehab
2013-01-23 15:08 ` Antti Palosaari
2013-01-23 15:12 ` Antti Palosaari
2013-01-23 18:18 ` Mauro Carvalho Chehab
2013-01-23 18:57 ` Mauro Carvalho Chehab
2013-01-23 19:55 ` Antti Palosaari
2013-01-23 21:00 ` Mauro Carvalho Chehab
2013-01-23 22:02 ` Mauro Carvalho Chehab
2013-01-17 17:16 ` Manu Abraham
2013-01-17 17:22 ` Antti Palosaari
2013-01-17 17:37 ` [linux-media] " Klaus Schmidinger
2013-01-17 17:39 ` [linux-media] " Klaus Schmidinger
2013-01-17 18:36 ` Mauro Carvalho Chehab
2013-01-19 12:04 ` Mauro Carvalho Chehab
2013-01-16 13:24 ` Mauro Carvalho Chehab
2013-01-15 10:38 ` Manu Abraham
2013-01-15 15:23 ` Mauro Carvalho Chehab
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130115111041.6b78a935@redhat.com \
--to=mchehab@redhat.com \
--cc=crope@iki.fi \
--cc=linux-media@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).