linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] dvb-apps: Improve femon
@ 2013-06-03 15:16 Jean Delvare
  2013-06-03 15:17 ` [PATCH 1/3] femon: Share common code Jean Delvare
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Jean Delvare @ 2013-06-03 15:16 UTC (permalink / raw)
  To: Linux Media

Improvements to dvb-apps/femon:
* femon: Share common code
* femon: Display SNR in dB
* femon: Handle -EOPNOTSUPP

-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] femon: Share common code
  2013-06-03 15:16 [PATCH 0/3] dvb-apps: Improve femon Jean Delvare
@ 2013-06-03 15:17 ` Jean Delvare
  2013-06-03 15:21 ` [PATCH 2/3] femon: Display SNR in dB Jean Delvare
  2013-06-03 15:23 ` [PATCH 3/3] femon: Handle -EOPNOTSUPP Jean Delvare
  2 siblings, 0 replies; 13+ messages in thread
From: Jean Delvare @ 2013-06-03 15:17 UTC (permalink / raw)
  To: Linux Media

The status flags are printed the same in standard output mode and
human readable output mode, so use common code.
---
 util/femon/femon.c |   20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

--- dvb-apps-3ee111da5b3a.orig/util/femon/femon.c	2013-06-02 13:56:18.936297146 +0200
+++ dvb-apps-3ee111da5b3a/util/femon/femon.c	2013-06-02 13:59:03.383299584 +0200
@@ -94,25 +94,21 @@ int check_frontend (struct dvbfe_handle
 		}
 
 
+		printf ("status %c%c%c%c%c | ",
+			fe_info.signal ? 'S' : ' ',
+			fe_info.carrier ? 'C' : ' ',
+			fe_info.viterbi ? 'V' : ' ',
+			fe_info.sync ? 'Y' : ' ',
+			fe_info.lock ? 'L' : ' ');
 
 		if (human_readable) {
-                       printf ("status %c%c%c%c%c | signal %3u%% | snr %3u%% | ber %d | unc %d | ",
-				fe_info.signal ? 'S' : ' ',
-				fe_info.carrier ? 'C' : ' ',
-				fe_info.viterbi ? 'V' : ' ',
-				fe_info.sync ? 'Y' : ' ',
-				fe_info.lock ? 'L' : ' ',
+			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 ("status %c%c%c%c%c | signal %04x | snr %04x | ber %08x | unc %08x | ",
-				fe_info.signal ? 'S' : ' ',
-				fe_info.carrier ? 'C' : ' ',
-				fe_info.viterbi ? 'V' : ' ',
-				fe_info.sync ? 'Y' : ' ',
-				fe_info.lock ? 'L' : ' ',
+			printf ("signal %04x | snr %04x | ber %08x | unc %08x | ",
 				fe_info.signal_strength,
 				fe_info.snr,
 				fe_info.ber,

-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/3] femon: Display SNR in dB
  2013-06-03 15:16 [PATCH 0/3] dvb-apps: Improve femon Jean Delvare
  2013-06-03 15:17 ` [PATCH 1/3] femon: Share common code Jean Delvare
@ 2013-06-03 15:21 ` Jean Delvare
  2013-11-24 17:21   ` Manu Abraham
  2013-06-03 15:23 ` [PATCH 3/3] femon: Handle -EOPNOTSUPP Jean Delvare
  2 siblings, 1 reply; 13+ messages in thread
From: Jean Delvare @ 2013-06-03 15:21 UTC (permalink / raw)
  To: Linux Media

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(-)

--- 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,

-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/3] femon: Handle -EOPNOTSUPP
  2013-06-03 15:16 [PATCH 0/3] dvb-apps: Improve femon Jean Delvare
  2013-06-03 15:17 ` [PATCH 1/3] femon: Share common code Jean Delvare
  2013-06-03 15:21 ` [PATCH 2/3] femon: Display SNR in dB Jean Delvare
@ 2013-06-03 15:23 ` Jean Delvare
  2 siblings, 0 replies; 13+ messages in thread
From: Jean Delvare @ 2013-06-03 15:23 UTC (permalink / raw)
  To: Linux Media

Frontend drivers don't have to implement all monitoring callbacks. So
expect -EOPNOTSUPP and handle it properly.
---
 util/femon/femon.c |   75 +++++++++++++++++++++++++++++++++--------------------
 1 file changed, 48 insertions(+), 27 deletions(-)

--- dvb-apps-3ee111da5b3a.orig/util/femon/femon.c	2013-06-03 17:22:56.923398598 +0200
+++ dvb-apps-3ee111da5b3a/util/femon/femon.c	2013-06-03 17:23:16.946398895 +0200
@@ -67,6 +67,7 @@ int check_frontend (struct dvbfe_handle
 	struct dvbfe_info fe_info;
 	unsigned int samples = 0;
 	FILE *ttyFile=NULL;
+	int got_info;
 	
 	// We dont write the "beep"-codes to stdout but to /dev/tty1.
 	// This is neccessary for Thin-Client-Systems or Streaming-Boxes
@@ -89,39 +90,59 @@ int check_frontend (struct dvbfe_handle
 	}
 
 	do {
-		if (dvbfe_get_info(fe, FE_STATUS_PARAMS, &fe_info, DVBFE_INFO_QUERYTYPE_IMMEDIATE, 0) != FE_STATUS_PARAMS) {
+		got_info = dvbfe_get_info(fe, FE_STATUS_PARAMS, &fe_info, DVBFE_INFO_QUERYTYPE_IMMEDIATE, 0);
+		if (got_info & DVBFE_INFO_LOCKSTATUS) {
+			printf ("status %c%c%c%c%c | ",
+				fe_info.signal ? 'S' : ' ',
+				fe_info.carrier ? 'C' : ' ',
+				fe_info.viterbi ? 'V' : ' ',
+				fe_info.sync ? 'Y' : ' ',
+				fe_info.lock ? 'L' : ' ');
+		} else {
 			fprintf(stderr, "Problem retrieving frontend information: %m\n");
+			printf ("status ----- | ");
 		}
 
 
-		printf ("status %c%c%c%c%c | ",
-			fe_info.signal ? 'S' : ' ',
-			fe_info.carrier ? 'C' : ' ',
-			fe_info.viterbi ? 'V' : ' ',
-			fe_info.sync ? 'Y' : ' ',
-			fe_info.lock ? 'L' : ' ');
-
 		if (human_readable) {
-			// 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);
+			if (got_info & DVBFE_INFO_SIGNAL_STRENGTH)
+				printf ("signal %3u%% | ", (fe_info.signal_strength * 100) / 0xffff);
+			else
+				printf ("signal ---%% | ");
+			if (got_info & DVBFE_INFO_SNR) {
+				// 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 ("snr %4.1fdB | ", fe_info.snr / 10.);
+				else
+					printf ("snr %3u%% | ", (fe_info.snr * 100) / 0xffff);
+			} else
+				printf ("snr ---- | ");
+			if (got_info & DVBFE_INFO_BER)
+				printf ("ber %d | ", fe_info.ber);
+			else
+				printf ("ber - | ");
+			if (got_info & DVBFE_INFO_UNCORRECTED_BLOCKS)
+				printf ("unc %d | ", fe_info.ucblocks);
+			else
+				printf ("unc - | ");
 		} else {
-			printf ("signal %04x | snr %04x | ber %08x | unc %08x | ",
-				fe_info.signal_strength,
-				fe_info.snr,
-				fe_info.ber,
-				fe_info.ucblocks);
+			if (got_info & DVBFE_INFO_SIGNAL_STRENGTH)
+				printf ("signal %04x | ", fe_info.signal_strength);
+			else
+				printf ("signal ---- | ");
+			if (got_info & DVBFE_INFO_SNR)
+				printf ("snr %04x | ", fe_info.snr);
+			else
+				printf ("snr ---- | ");
+			if (got_info & DVBFE_INFO_BER)
+				printf ("ber %08x | ", fe_info.ber);
+			else
+				printf ("ber -------- | ");
+			if (got_info & DVBFE_INFO_UNCORRECTED_BLOCKS)
+				printf ("unc %08x | ", fe_info.ucblocks);
+			else
+				printf ("unc -------- | ");
 		}
 
 		if (fe_info.lock)

-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] femon: Display SNR in dB
  2013-06-03 15:21 ` [PATCH 2/3] femon: Display SNR in dB Jean Delvare
@ 2013-11-24 17:21   ` Manu Abraham
  2013-11-24 18:02     ` Chris Lee
  2013-11-25  9:23     ` Jean Delvare
  0 siblings, 2 replies; 13+ messages in thread
From: Manu Abraham @ 2013-11-24 17:21 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Linux Media

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] femon: Display SNR in dB
  2013-11-24 17:21   ` Manu Abraham
@ 2013-11-24 18:02     ` Chris Lee
  2013-11-24 18:14       ` Manu Abraham
  2013-11-24 18:20       ` Devin Heitmueller
  2013-11-25  9:23     ` Jean Delvare
  1 sibling, 2 replies; 13+ messages in thread
From: Chris Lee @ 2013-11-24 18:02 UTC (permalink / raw)
  To: Linux Media Mailing List

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] femon: Display SNR in dB
  2013-11-24 18:02     ` Chris Lee
@ 2013-11-24 18:14       ` Manu Abraham
  2013-11-24 18:20       ` Devin Heitmueller
  1 sibling, 0 replies; 13+ messages in thread
From: Manu Abraham @ 2013-11-24 18:14 UTC (permalink / raw)
  To: Chris Lee; +Cc: Linux Media Mailing List

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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] femon: Display SNR in dB
  2013-11-24 18:02     ` Chris Lee
  2013-11-24 18:14       ` Manu Abraham
@ 2013-11-24 18:20       ` Devin Heitmueller
  2013-11-24 18:40         ` Chris Lee
  1 sibling, 1 reply; 13+ messages in thread
From: Devin Heitmueller @ 2013-11-24 18:20 UTC (permalink / raw)
  To: Chris Lee; +Cc: Linux Media Mailing List

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] femon: Display SNR in dB
  2013-11-24 18:20       ` Devin Heitmueller
@ 2013-11-24 18:40         ` Chris Lee
  2013-11-24 18:52           ` Devin Heitmueller
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Lee @ 2013-11-24 18:40 UTC (permalink / raw)
  To: Linux Media Mailing List

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] femon: Display SNR in dB
  2013-11-24 18:40         ` Chris Lee
@ 2013-11-24 18:52           ` Devin Heitmueller
  0 siblings, 0 replies; 13+ messages in thread
From: Devin Heitmueller @ 2013-11-24 18:52 UTC (permalink / raw)
  To: Chris Lee; +Cc: Linux Media Mailing List

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

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] femon: Display SNR in dB
  2013-11-24 17:21   ` Manu Abraham
  2013-11-24 18:02     ` Chris Lee
@ 2013-11-25  9:23     ` Jean Delvare
  2013-11-25 13:43       ` Stefan Richter
  1 sibling, 1 reply; 13+ messages in thread
From: Jean Delvare @ 2013-11-25  9:23 UTC (permalink / raw)
  To: Manu Abraham; +Cc: Linux Media

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,
-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] femon: Display SNR in dB
  2013-11-25  9:23     ` Jean Delvare
@ 2013-11-25 13:43       ` Stefan Richter
  2013-11-25 14:00         ` Stefan Richter
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Richter @ 2013-11-25 13:43 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Manu Abraham, Linux Media, Chris Lee, Devin Heitmueller

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
-=====-===-= =-== ==--=
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] femon: Display SNR in dB
  2013-11-25 13:43       ` Stefan Richter
@ 2013-11-25 14:00         ` Stefan Richter
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Richter @ 2013-11-25 14:00 UTC (permalink / raw)
  To: Stefan Richter, Henrik Kurelid
  Cc: Jean Delvare, Manu Abraham, Linux Media, Chris Lee, Devin Heitmueller

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,

-- 
Stefan Richter
-=====-===-= =-== ==--=
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-11-25 14:15 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-03 15:16 [PATCH 0/3] dvb-apps: Improve femon Jean Delvare
2013-06-03 15:17 ` [PATCH 1/3] femon: Share common code Jean Delvare
2013-06-03 15:21 ` [PATCH 2/3] femon: Display SNR in dB Jean Delvare
2013-11-24 17:21   ` Manu Abraham
2013-11-24 18:02     ` Chris Lee
2013-11-24 18:14       ` Manu Abraham
2013-11-24 18:20       ` Devin Heitmueller
2013-11-24 18:40         ` Chris Lee
2013-11-24 18:52           ` Devin Heitmueller
2013-11-25  9:23     ` Jean Delvare
2013-11-25 13:43       ` Stefan Richter
2013-11-25 14:00         ` Stefan Richter
2013-06-03 15:23 ` [PATCH 3/3] femon: Handle -EOPNOTSUPP Jean Delvare

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).