diff mbox

[2/3] femon: Display SNR in dB

Message ID 20130603172150.1aaf1904@endymion.delvare (mailing list archive)
State New, archived
Headers show

Commit Message

Jean Delvare June 3, 2013, 3:21 p.m. UTC
SNR is supposed to be reported by the frontend drivers in dB, so print
it that way for drivers which implement it properly.
---
 util/femon/femon.c |   19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

Comments

Manu Abraham Nov. 24, 2013, 5:21 p.m. UTC | #1
Hi Jean,

Sorry, that I came upon this patch quite late.

On Mon, Jun 3, 2013 at 8:51 PM, Jean Delvare <khali@linux-fr.org> wrote:
> SNR is supposed to be reported by the frontend drivers in dB, so print
> it that way for drivers which implement it properly.


Not all frontends do report report the SNR in dB. Well, You can say quite
some frontends do report it that way. Making the application report it in
dB for frontends which do not will show up as incorrect results, from what
I can say.

Best Regards,

Manu
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chris Lee Nov. 24, 2013, 6:02 p.m. UTC | #2
This is a frustration of mine. Some report it in SNR others report it
in terms of % (current snr / (max_snr-min_snr)) others its completely
random.

Seems many dvb-s report arbitrary % which is stupid and many atsc
report snr by 123 would be 12.3db. But there isnt any standardization
around.

imo everything should be reported in terms of db, why % was ever
chosen is beyond logic.

Is this something we can get ratified ?

Chris Lee

On Sun, Nov 24, 2013 at 10:21 AM, Manu Abraham <abraham.manu@gmail.com> wrote:
> Hi Jean,
>
> Sorry, that I came upon this patch quite late.
>
> On Mon, Jun 3, 2013 at 8:51 PM, Jean Delvare <khali@linux-fr.org> wrote:
>> SNR is supposed to be reported by the frontend drivers in dB, so print
>> it that way for drivers which implement it properly.
>
>
> Not all frontends do report report the SNR in dB. Well, You can say quite
> some frontends do report it that way. Making the application report it in
> dB for frontends which do not will show up as incorrect results, from what
> I can say.
>
> Best Regards,
>
> Manu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Manu Abraham Nov. 24, 2013, 6:14 p.m. UTC | #3
On Sun, Nov 24, 2013 at 11:32 PM, Chris Lee <updatelee@gmail.com> wrote:
> This is a frustration of mine. Some report it in SNR others report it
> in terms of % (current snr / (max_snr-min_snr)) others its completely
> random.
>
> Seems many dvb-s report arbitrary % which is stupid and many atsc
> report snr by 123 would be 12.3db. But there isnt any standardization
> around.
>
> imo everything should be reported in terms of db, why % was ever
> chosen is beyond logic.


Because dB terminology did not fit all frontends. For some it does
fit, but again not all. Some frontends by default don't have a dB
specification; some reverse engineered ones unable to.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Devin Heitmueller Nov. 24, 2013, 6:20 p.m. UTC | #4
On Sun, Nov 24, 2013 at 1:02 PM, Chris Lee <updatelee@gmail.com> wrote:
> This is a frustration of mine. Some report it in SNR others report it
> in terms of % (current snr / (max_snr-min_snr)) others its completely
> random.
>
> Seems many dvb-s report arbitrary % which is stupid and many atsc
> report snr by 123 would be 12.3db. But there isnt any standardization
> around.
>
> imo everything should be reported in terms of db, why % was ever
> chosen is beyond logic.
>
> Is this something we can get ratified ?

I wouldn't hold your breath.  We've been arguing about this for years.
 You can check the archives for the dozens of messages exchanged on
the topic.

Given almost all the Linux drivers for ATSC/ClearQAM devices sold
today report in 0.1 dB increments, I'm tempted to put a hack in the
various applications to assume all ATSC devices are in that format.
I've essentially given up on any hope that there will be any agreement
on a kernel API which applications can rely on for a uniform format.

Devin
Chris Lee Nov. 24, 2013, 6:40 p.m. UTC | #5
I made an exception in my app if the system is ATSC/QAM it uses the
snr = snr * 10.0 and havent found a card yet that it doesnt work with.
Ive also converted quite a few of my dvb-s tuners to report db in the
same way. Havent found a card yet that doesnt have the ability to
report snr in db. Im sure there is one, but I wonder how old it is and
if anyone still uses them.

I have found a few tuner/demods that dont have a method of reporting
signal strength and just use a calc based off the snr in db to make a
fake strength.

How I look at is if snr in % is completely arbitrary and means nothing
when compared from one tuner to another, whats the harm in that
particularly weird tuner/demod of reporting a fake SNR that is
arbitrary and have every other device in Linux report something
useful. Seems dumb to have every device in Linux report an arbitrary
useless value just because one or two devices cant report anything
useful.

I just hate seeing every device reporting useless values just because
one or two tuner/demods are reporting useless values. Why destroy that
useful data for the sake of making all data uniformly useless.

Chris Lee

On Sun, Nov 24, 2013 at 11:20 AM, Devin Heitmueller
<dheitmueller@kernellabs.com> wrote:
> On Sun, Nov 24, 2013 at 1:02 PM, Chris Lee <updatelee@gmail.com> wrote:
>> This is a frustration of mine. Some report it in SNR others report it
>> in terms of % (current snr / (max_snr-min_snr)) others its completely
>> random.
>>
>> Seems many dvb-s report arbitrary % which is stupid and many atsc
>> report snr by 123 would be 12.3db. But there isnt any standardization
>> around.
>>
>> imo everything should be reported in terms of db, why % was ever
>> chosen is beyond logic.
>>
>> Is this something we can get ratified ?
>
> I wouldn't hold your breath.  We've been arguing about this for years.
>  You can check the archives for the dozens of messages exchanged on
> the topic.
>
> Given almost all the Linux drivers for ATSC/ClearQAM devices sold
> today report in 0.1 dB increments, I'm tempted to put a hack in the
> various applications to assume all ATSC devices are in that format.
> I've essentially given up on any hope that there will be any agreement
> on a kernel API which applications can rely on for a uniform format.
>
> Devin
>
> --
> Devin J. Heitmueller - Kernel Labs
> http://www.kernellabs.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Devin Heitmueller Nov. 24, 2013, 6:52 p.m. UTC | #6
On Sun, Nov 24, 2013 at 1:40 PM, Chris Lee <updatelee@gmail.com> wrote:
> I made an exception in my app if the system is ATSC/QAM it uses the
> snr = snr * 10.0 and havent found a card yet that it doesnt work with.
> Ive also converted quite a few of my dvb-s tuners to report db in the
> same way. Havent found a card yet that doesnt have the ability to
> report snr in db. Im sure there is one, but I wonder how old it is and
> if anyone still uses them.

The devices which have the ldgt3303 demodulator report in (SNR *
1/256), and there are still quite a few of those out there and in use.
 But yeah, since most of the ATSC/ClearQAM devices out there nowadays
have a demod driver written by one of three people, all who agreed on
the same format, you won't find many devices out there nowadays which
don't use 0.1 dB increments.

That said, everything in the previous paragraph applies exclusively to
ATSC/ClearQAM devices.  There is much more variation once you start
talking about DVB-T/S/S2, etc.

> I have found a few tuner/demods that dont have a method of reporting
> signal strength and just use a calc based off the snr in db to make a
> fake strength.

Yup, there is definitely more ambiguity across demods with the signal
strength field.  In some cases it reports the same as the SNR field,
in other cases it scales the SNR to a 0-65535 range, in yet other
cases it returns garbage or no value at all.

> How I look at is if snr in % is completely arbitrary and means nothing
> when compared from one tuner to another, whats the harm in that
> particularly weird tuner/demod of reporting a fake SNR that is
> arbitrary and have every other device in Linux report something
> useful. Seems dumb to have every device in Linux report an arbitrary
> useless value just because one or two devices cant report anything
> useful.

The whole argument over the years is what that "useful format" should
be.  The problem is bad enough where a whole new API has been proposed
which allows the demod to specify its reporting format explicitly.
That proposed new API has a whole host of *other* problems, but that
was the last attempt to bring some clarity to the problem.

> I just hate seeing every device reporting useless values just because
> one or two tuner/demods are reporting useless values. Why destroy that
> useful data for the sake of making all data uniformly useless.

Unfortunately we're not talking about one or two - we're talking about
dozens.  I wouldn't be remotely surprised to see that more than 50% of
devices out there do not report in 0.1dB increments.  Certainly a
large part of the problem is that users such as yourself have a
slightly skewed viewpoint because your experience is based on the
handful of tuners you own (if you actually own a handful, that's
comparatively quite a lot - most people only own a single tuner).

Yeah, the situation is frustrating.  No easy answers here.

Devin
Jean Delvare Nov. 25, 2013, 9:23 a.m. UTC | #7
Hi Manu,

On Sun, 24 Nov 2013 22:51:33 +0530, Manu Abraham wrote:
> Sorry, that I came upon this patch quite late.

No problem, better late than never! :)

> On Mon, Jun 3, 2013 at 8:51 PM, Jean Delvare <khali@linux-fr.org> wrote:
> > SNR is supposed to be reported by the frontend drivers in dB, so print
> > it that way for drivers which implement it properly.
> 
> Not all frontends do report report the SNR in dB. Well, You can say quite
> some frontends do report it that way.

Last time I discussed this, I was told that this was the preferred way
for frontends to report the SNR. I also referred to this document:
  http://palosaari.fi/linux/v4l-dvb/snr_2012-05-21.txt
I don't know now up-to-date it is by now, but back then it showed a
significant number of frontends reporting in .1 dB already, including
the ones I'm using right now (drx-3916k and drx-3913k.) With the
current version of femon, "femon -H" reports it as 0%, which is quite
useless. Thus my patch.

> Making the application report it in
> dB for frontends which do not will show up as incorrect results, from what
> I can say.

My code has a conditional to interpret high values (>= 1000) as % and
low values (< 1000) as .1 dB. This is a heuristic which works fine for
me in practice (tested on drxk, cx22702 and dib3000mc.) I would
certainly welcome testing on other DVB cards. I seem to recall that
Mauro suggested that values above 40 dB (?) were not possible so we
could probably lower the threshold from 1000 to 400 or so if the
current value causes trouble. I think the maximum I've see on my card
was ~32 dB.

If you find a significant number of frontend drivers for which the
heuristic doesn't work, and lowering the threshold doesn't help, then
I'm fine considering a different approach such as an extra command line
parameter. But if only a small number of drivers cause trouble, I'd say
these drivers should simply be fixed.

Thanks,
Stefan Richter Nov. 25, 2013, 1:43 p.m. UTC | #8
On Nov 25 Jean Delvare wrote:
> Hi Manu,
> 
> On Sun, 24 Nov 2013 22:51:33 +0530, Manu Abraham wrote:
> > Sorry, that I came upon this patch quite late.
> 
> No problem, better late than never! :)
> 
> > On Mon, Jun 3, 2013 at 8:51 PM, Jean Delvare <khali@linux-fr.org> wrote:
> > > SNR is supposed to be reported by the frontend drivers in dB, so print
> > > it that way for drivers which implement it properly.
> > 
> > Not all frontends do report report the SNR in dB. Well, You can say quite
> > some frontends do report it that way.
> 
> Last time I discussed this, I was told that this was the preferred way
> for frontends to report the SNR. I also referred to this document:
>   http://palosaari.fi/linux/v4l-dvb/snr_2012-05-21.txt
> I don't know now up-to-date it is by now, but back then it showed a
> significant number of frontends reporting in .1 dB already, including
> the ones I'm using right now (drx-3916k and drx-3913k.) With the
> current version of femon, "femon -H" reports it as 0%, which is quite
> useless. Thus my patch.
[...]

Hi,

I inherited this in drivers/media/firewire/firedtv-fe.c:

static int fdtv_read_snr(struct dvb_frontend *fe, u16 *snr)
{
	struct firedtv *fdtv = fe->sec_priv;
	struct firedtv_tuner_status stat;

	if (avc_tuner_status(fdtv, &stat))
		return -EINVAL;

	/* C/N[dB] = -10 * log10(snr / 65535) */
	*snr = stat.carrier_noise_ratio * 257;
	return 0;
}

As far as I understand, the comment should have been written with a "FIXME"
prefix.

I have no documentation and no personal manufacturer contact (and the
devices are EOL).  All I know from the driver source is that we do get a 16
bits wide carrier_noise_ratio.  So it appears to be something on a scale
from 0x0000 to 0xffff, and the comment makes it look like being on a linear
scale originally.

I could cross-check with a Windows based TV viewer application what signal
strength value is presented there to the user with DVB-T and DVB-S2
incarnations of FireDTV devices.  Right now I don't remember how that
application presents it (i.e. as percentage or dB or whatever...).
When I looked at that application and at kaffeine some years ago, they
displayed grossly different values.  I did not research back then whether
the Linux driver or kaffeine or both treated it wrong.

Any advice for the quoted kernel driver code?

Thanks,
Stefan Richter Nov. 25, 2013, 2 p.m. UTC | #9
On Nov 25 Stefan Richter wrote:
> On Nov 25 Jean Delvare wrote:
> > Hi Manu,
> > 
> > On Sun, 24 Nov 2013 22:51:33 +0530, Manu Abraham wrote:
> > > Sorry, that I came upon this patch quite late.
> > 
> > No problem, better late than never! :)
> > 
> > > On Mon, Jun 3, 2013 at 8:51 PM, Jean Delvare <khali@linux-fr.org> wrote:
> > > > SNR is supposed to be reported by the frontend drivers in dB, so print
> > > > it that way for drivers which implement it properly.
> > > 
> > > Not all frontends do report report the SNR in dB. Well, You can say quite
> > > some frontends do report it that way.
> > 
> > Last time I discussed this, I was told that this was the preferred way
> > for frontends to report the SNR. I also referred to this document:
> >   http://palosaari.fi/linux/v4l-dvb/snr_2012-05-21.txt
> > I don't know now up-to-date it is by now, but back then it showed a
> > significant number of frontends reporting in .1 dB already, including
> > the ones I'm using right now (drx-3916k and drx-3913k.) With the
> > current version of femon, "femon -H" reports it as 0%, which is quite
> > useless. Thus my patch.
> [...]
> 
> Hi,
> 
> I inherited this in drivers/media/firewire/firedtv-fe.c:
> 
> static int fdtv_read_snr(struct dvb_frontend *fe, u16 *snr)
> {
> 	struct firedtv *fdtv = fe->sec_priv;
> 	struct firedtv_tuner_status stat;
> 
> 	if (avc_tuner_status(fdtv, &stat))
> 		return -EINVAL;
> 
> 	/* C/N[dB] = -10 * log10(snr / 65535) */
> 	*snr = stat.carrier_noise_ratio * 257;
> 	return 0;
> }
> 
> As far as I understand, the comment should have been written with a "FIXME"
> prefix.
> 
> I have no documentation and no personal manufacturer contact (and the
> devices are EOL).  All I know from the driver source is that we do get a 16
> bits wide carrier_noise_ratio.  So it appears to be something on a scale
> from 0x0000 to 0xffff, and the comment makes it look like being on a linear
> scale originally.

Or I got it wrong and all the comment tries to tell that the value to be
returned in the snr argument is meant to be linear on a scale of 0...ffff,
and the code tells that we get a linear value on a scale of 0...255 from
the firmware.  (Cc'ing Henrik who added the code and the comment.)

> I could cross-check with a Windows based TV viewer application what signal
> strength value is presented there to the user with DVB-T and DVB-S2
> incarnations of FireDTV devices.  Right now I don't remember how that
> application presents it (i.e. as percentage or dB or whatever...).
> When I looked at that application and at kaffeine some years ago, they
> displayed grossly different values.  I did not research back then whether
> the Linux driver or kaffeine or both treated it wrong.
> 
> Any advice for the quoted kernel driver code?
> 
> Thanks,
diff mbox

Patch

--- dvb-apps-3ee111da5b3a.orig/util/femon/femon.c	2013-06-02 14:05:00.988323437 +0200
+++ dvb-apps-3ee111da5b3a/util/femon/femon.c	2013-06-02 14:05:33.560792474 +0200
@@ -102,11 +102,20 @@  int check_frontend (struct dvbfe_handle
 			fe_info.lock ? 'L' : ' ');
 
 		if (human_readable) {
-			printf ("signal %3u%% | snr %3u%% | ber %d | unc %d | ",
-				(fe_info.signal_strength * 100) / 0xffff,
-				(fe_info.snr * 100) / 0xffff,
-				fe_info.ber,
-				fe_info.ucblocks);
+			// SNR should be in units of 0.1 dB but some drivers do
+			// not follow that rule, thus this heuristic.
+			if (fe_info.snr < 1000)
+				printf ("signal %3u%% | snr %4.1fdB | ber %d | unc %d | ",
+					(fe_info.signal_strength * 100) / 0xffff,
+					fe_info.snr / 10.,
+					fe_info.ber,
+					fe_info.ucblocks);
+			else
+				printf ("signal %3u%% | snr %3u%% | ber %d | unc %d | ",
+					(fe_info.signal_strength * 100) / 0xffff,
+					(fe_info.snr * 100) / 0xffff,
+					fe_info.ber,
+					fe_info.ucblocks);
 		} else {
 			printf ("signal %04x | snr %04x | ber %08x | unc %08x | ",
 				fe_info.signal_strength,