Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#5929 - FM tuner: Deemphasis cannot be changed

Attached to Project: Rockbox
Opened by Horst Maier (HorstIriver) - Saturday, 02 September 2006, 17:21 GMT+2
Last edited by Dominik Riebeling (bluebrother) - Tuesday, 05 September 2006, 17:27 GMT+2
Task Type Patches
Category FM Tuner
Status Closed
Assigned To No-one
Player Type Iriver H100 series
Severity Medium
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

USA and Europe use different deemphasis time constants. In the US, it is 75us, while in Europe we have 50us.
I don't know the setting of the current Rockbox firmware. If it is 75us (US) and used in Europe, the treble will
be much too low. If it is 50us (Europe) and used in the US, the treble will be overemphasized, and there will
be an audible noise floor, even at good reception. The iriver firmware does the switching depending on the
firmware region.
This task depends upon

Closed by  Dominik Riebeling (bluebrother)
Thursday, 05 October 2006, 14:56 GMT+2
Reason for closing:  Fixed
Additional comments about closing:  commited to cvs
Comment by Dominik Riebeling (bluebrother) - Sunday, 03 September 2006, 00:44 GMT+2
The attached patch adds a new option in the radio context menu which allows changing the emphasis. Please try and give some feedback -- from what I can hear (and according to your description) this should work correctly.
   fm-radio-emphasis.patch (4.3 KiB) 
 apps/settings.c          |    2 +-
 apps/lang/english.lang   |   40 ++++++++++++++++++++++++++++++++++++++++
 apps/recorder/radio.c    |   26 ++++++++++++++++++++++++++
 firmware/tuner_philips.c |    5 +++++
 firmware/export/tuner.h  |    1 +
 5 files changed, 73 insertions(+), 1 deletion(-)

Comment by Dominik Riebeling (bluebrother) - Sunday, 03 September 2006, 01:09 GMT+2
whoops, the patch is missing the change in settings.h -- fixed.
   fm-radio-emphasis2.patch (4.8 KiB) 
 apps/settings.c          |    2 +-
 apps/settings.h          |    1 +
 apps/lang/english.lang   |   40 ++++++++++++++++++++++++++++++++++++++++
 apps/recorder/radio.c    |   26 ++++++++++++++++++++++++++
 firmware/tuner_philips.c |    5 +++++
 firmware/export/tuner.h  |    1 +
 6 files changed, 74 insertions(+), 1 deletion(-)

Comment by Horst Maier (HorstIriver) - Tuesday, 05 September 2006, 16:35 GMT+2
Dominik, thanks a lot for your quick reaction. Has the patch already been integrated in the current daily built,
or do I have to apply the patch manually? If yes, how is it done?
Horst
Comment by Dominik Riebeling (bluebrother) - Tuesday, 05 September 2006, 17:19 GMT+2
Horst,

it is still pending. I even don't know if the core devs think this is done "the right way", but it works and IMO integrates nicely. I asked around a few times on IRC if somebody wants to look into it but it seems noone who is familiar with the radio (and / or free time) was around. Also I was only able trying this on h120, so probably there will arise problems on the other targets (though I tried to make this a way it shouldn't do so). Maybe you want to ask around yourself.

To apply a patch, see the wiki: http://www.rockbox.org/twiki/bin/view/Main/WorkingWithPatches. You'll need a development environment to do so (but this is explained in the wiki as well).
Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 05 September 2006, 21:27 GMT+2
I just had a look at the patch. I have a few comments:

1) I believe it's called de-emphasis, not emphasis

2) It didn't compile cleanly on all targets

3) I believe we should use a region setting instead, since there are more parameters, like frequency range and frequency step

I have attached an updated patch for issue (1) and (2) above.
   fm-radio-deemphasis.patch (5.9 KiB) 
 apps/settings.c          |    4 +++-
 apps/settings.h          |    3 +++
 apps/lang/english.lang   |   40 ++++++++++++++++++++++++++++++++++++++++
 apps/recorder/radio.c    |   26 ++++++++++++++++++++++++++
 firmware/tuner_philips.c |    7 ++++++-
 firmware/export/tuner.h  |    3 ++-
 6 files changed, 80 insertions(+), 3 deletions(-)

Comment by Horst Maier (HorstIriver) - Tuesday, 05 September 2006, 22:13 GMT+2
Linus, adding region settings was my first idea, too, but I didn't dare to propose it because of the work that is behind that task. You are right, it is called de-emphasis or deemphasis. It is a trick to improve the S/N ratio. At the transmitter, the treble is emphasized. At the receiver, it is de-emphasized with the same time constant (roll-off frequency). The de-emphasis not only attenuates the treble down to its correct level, but also attenuates the noise. Testing: I'd really love to start to test the patches immediately, but as a hardware developer, I have never used CVS before, let alone compiled source code, so it will take me some time to get the entire development environment working. The question is: Will my results be reliable enough? If it doesn't work, chances are that it is because I did something wrong. I can test it on an H120 and H140 only because I don't have other players...
Comment by Dominik Riebeling (bluebrother) - Tuesday, 05 September 2006, 22:26 GMT+2
Linus,

thanks for looking in.
1. after looking the term up IIUC emphasis is the process of both emphasis and deemphasis. Seems I've got a bit mixed up with the terms, it had gotten late that day ;)
2. The only difference I spotted between the patches (apart from the naming changes) was the fact that I forgot to #ifdef the changes in settings.[ch] but I don't understand why this should break compiling (but of course this should be ifdef'ed). Can you please enlighten me?
3. I agree with you. This also would solve  FS#5928 . I started collecting information about the different locations, but I wasn't able gathering complete informations. I hope somebody could correct / fill this information (and, if I'm missing some different location, add that -- unfortunately Wikipedia wasn't exhaustive at all).

Location, Frequency (MHz), Step Size (kHz), Deemphasis (µs)
- Europe, 87.5 - 108.0, 50, 50
- US / Canada, 87.9 - 107.9, 200, 75
- GB / Ireland, 88.0 - 97.6, 100?, 50?
- Japan, 76.0 - 90.0, ?, ?
- China, 92.0 - 108.0, ?, ?
Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 05 September 2006, 22:33 GMT+2
2) You used #ifdef TEA5767 instead of #if CONFIG_TUNER == TEA5767

3) A good start. Maybe we should discuss that in the forum?
Comment by Dominik Riebeling (bluebrother) - Tuesday, 05 September 2006, 22:51 GMT+2
2) D'oh! My bad ...
3) I started reworking the patch. As I'm missing the data I currently use guessed values but I'm pretty confident to get this ready in a not-so-long time. I'm going to ask in the forums for input on correct values.
Comment by Steve Bavin (pondlife) - Wednesday, 06 September 2006, 07:46 GMT+2
GB uses a range of 87.5-108.0 MHz with a step size of 50kHz. No idea on the de-emphasis, but seeing as the other parameters match Europe then it's likely to be 50 too and you can remove the GB setting.

No idea about Ireland, but I'd expect it's the same.
Comment by Dominik Riebeling (bluebrother) - Wednesday, 06 September 2006, 11:32 GMT+2
Ok, I reworked the patch to a country setting.

- I still need more country data, so far I only included Europe, US and Japan
- now the setting applies to all targets. I wasn't able trying for the archos devices.
- the samsung tuner seems to be unable changing the deemphasis so deemphasis only gets applied for TEA.
- now the frequency is displayed with two digits after the decimal dot as for europe we need this.

Apart from the missing country data it should work fine.
   fmradio-countrysetting.patch (12 KiB) 
 apps/settings.c          |    1 
 apps/settings.h          |    1 
 apps/lang/english.lang   |   65 +++++++++++++++++++++++++++++---
 apps/recorder/radio.c    |   94 ++++++++++++++++++++++++++++++++++-------------
 apps/recorder/radio.h    |    9 ++++
 firmware/tuner_philips.c |    5 ++
 firmware/export/tuner.h  |    1 
 7 files changed, 144 insertions(+), 32 deletions(-)

Comment by Dominik Riebeling (bluebrother) - Wednesday, 06 September 2006, 19:11 GMT+2
new version:
- TEA needs a Band Limits setting to tune into Japanese FM band, added. As I'm outside of Japan I can't test this.
- #ifdef the settings entry so it only gets included if the dap actually has a tuner. Missed this in the previous version.
- #ifdef the deemphasis (and band limit) values for TEA5767 -- the samsung tuner doesn't need the band limit setting and I still haven't found a setting for the deemphasis on that chip.

The samsung tuner can tune a pretty much wider range than the philips. Should we include include OIRT and other bands that are only available to the samsung tuner? Or should we prefer to go with the "standard" limits, so all players have the same range?
Also, currently with a preset file can tune to any frequency, discarding the country setting. Should this be checked?

Builds fine for x5 and h120.
   fmradio-countrysetting2.patch (12.8 KiB) 
 apps/settings.c          |    3 +
 apps/settings.h          |    3 +
 apps/lang/english.lang   |   65 +++++++++++++++++++++++++---
 apps/recorder/radio.c    |  109 +++++++++++++++++++++++++++++++++++------------
 apps/recorder/radio.h    |   12 +++++
 firmware/tuner_philips.c |    8 +++
 firmware/export/tuner.h  |    4 +
 7 files changed, 172 insertions(+), 32 deletions(-)

Comment by Dominik Riebeling (bluebrother) - Saturday, 09 September 2006, 17:04 GMT+2
small updates:
- add setting for korea
- make sure the frequency matches a valid frequency when changing country settings (i.e. prevent the tuner from starting at something like 98.05 MHz when changing from Europe to US -- US step size is 0.1 MHz, so this gets corrected to 98.0 MHz)

Building tried for h120 and x5.
   fmradio-countrysetting3.patch (13.2 KiB) 
 apps/settings.c          |    3 +
 apps/settings.h          |    3 +
 apps/lang/english.lang   |   77 +++++++++++++++++++++++++++++---
 apps/recorder/radio.c    |  113 ++++++++++++++++++++++++++++++++++++-----------
 apps/recorder/radio.h    |   12 ++++
 firmware/tuner_philips.c |    8 +++
 firmware/export/tuner.h  |    4 +
 7 files changed, 188 insertions(+), 32 deletions(-)

Comment by Rani Hod (RaeNye) - Monday, 18 September 2006, 21:30 GMT+2
Tried this on my X5, works well. Why not commit?

Two suggestions:
1. Maybe allow tweaking the separate lower-level parameters as well as the country presets.
2. Consider adding the sensitivity parameter of response level for deciding whether a given frequency is an actual station; here it misses perfectly good ones and in other places it just finds a whole lot of random presets.

BTW: why won't we put FM settings in the main menu?
Comment by Dominik Riebeling (bluebrother) - Monday, 18 September 2006, 22:26 GMT+2
I'm for committing this (guess why ;-) but don't want to do it ATM myself for some reasons: 1. I was given cvs access for working on the manual, thus I don't want to mess around with the code in cvs itself unless a core developer tells me it's ok to do so. 2. My free time is rather limited for the next ~3 weeks, so I'd prefer waiting a bit (unless someone who is coding more regularly for rockbox adopts this). 3. I haven't heard of anyone testing this on archos devices and would like to get some feedback first (most important).

On your suggestions:
1. What parameters do you mean exactly? The values set by the country presets? I think allowing to change them would cause more confusion than it's actually worth. Users usually want to have the correct values for the location they're currently in and not bother with the details. So I don't think the added complexity is worth it. I haven't looked deeper into this, but IIRC there are some other parameters that could get adjusted (at least for TEA) Also, I don't think Rockbox should allow to tune to non-fm-radio-broadcast frequencies (like  FS#5448  allows).
2. noted on my todo list. In fact I thought about looking what the tuner itself has as additional capabilities. Still waiting, though.
3. I started thinking about improving the radio screen recently. There is still the lower volume issue which also needs a setting. Possibly some more settings to come. I thought it may be better putting the settings somewhere into Main Menu / System / FM tuner (as they are usually only set once) but I haven't done any work on this. Also, I'd prefer doing this as separate changes.
Comment by Horst Maier (HorstIriver) - Thursday, 21 September 2006, 15:59 GMT+2
Dominik, I am not a core developer, but because I raised that topic, I dare to answer you ;-)
Your comments make sense. Actually one region setting in the main menue that sets all parameters
automatically to the correct values would be the best solution. Many users don't know anything about
deemphasis or the correct fm frequency range of their region, and they could be irritated if they
by chance receive the nearby police station instead of their favourite hard metal fm station :-)
Let alone legal aspects... I don't have any problem because you want to wait a bit - I am glad that
someone wants to fix those issues at all. Please drop a comment when you have done any fixes so that
I am notified.
Horst
Comment by Dominik Riebeling (bluebrother) - Thursday, 21 September 2006, 18:57 GMT+2
Updated version:
- as suggested by Linus rename the setting to "region". Don't ask me why I used "country" in the first place, "region" is much more logical :o
- make sure when switching between regions to have a valid frequency set (i.e. check the minimum frequency as well, not only the step size. Previously it was possible to tune to 90.05 MHz (EU) and then switch to US, which should change it to 89.9 MHz but was changed to 90.0 MHz (as step size of US is 0.2 MHz _and_ the frequencies have an odd fraction)
- found some tabs in the source. Should be fixed now.
- change the step size for US to 0.2 MHz (as this is the correct value afaik)
- fix building on sh1 (was broken by a define)
- ondio is available with samsung and philips tuner. Adjusted some defines to have the deemphasis also working in that case

building tried for x5, h120 and ondio fm target.
   fmradio-regionsetting4b.patch (13.2 KiB) 
 apps/settings.c          |    4 +
 apps/settings.h          |    3 +
 apps/lang/english.lang   |   77 +++++++++++++++++++++++++++++--
 apps/recorder/radio.c    |  114 ++++++++++++++++++++++++++++++++++++-----------
 apps/recorder/radio.h    |   12 ++++
 firmware/tuner_philips.c |    8 +++
 firmware/export/tuner.h  |    4 +
 7 files changed, 190 insertions(+), 32 deletions(-)

Comment by Dominik Riebeling (bluebrother) - Thursday, 21 September 2006, 19:03 GMT+2
oops, missed one line (which would result in the ondio not setting the deemphasis at all). Should be fixed now.
   fmradio-regionsetting4c.patch (13.2 KiB) 
 apps/settings.c          |    4 +
 apps/settings.h          |    3 +
 apps/lang/english.lang   |   77 +++++++++++++++++++++++++++++--
 apps/recorder/radio.c    |  114 ++++++++++++++++++++++++++++++++++++-----------
 apps/recorder/radio.h    |   12 ++++
 firmware/tuner_philips.c |    8 +++
 firmware/export/tuner.h  |    4 +
 7 files changed, 190 insertions(+), 32 deletions(-)

Comment by Linus Nielsen Feltzing (linusnielsen) - Thursday, 21 September 2006, 20:13 GMT+2
Seems to work fine on the Archos FM recorder.
Comment by Dominik Riebeling (bluebrother) - Thursday, 05 October 2006, 10:31 GMT+2
updated to latest cvs
   fmradio-regionsetting4e.patch (13.3 KiB) 
 apps/settings.c          |    3 +
 apps/settings.h          |    4 +
 apps/lang/english.lang   |   77 +++++++++++++++++++++++++++++--
 apps/recorder/radio.c    |  114 ++++++++++++++++++++++++++++++++++++-----------
 apps/recorder/radio.h    |   12 ++++
 firmware/tuner_philips.c |    8 +++
 firmware/export/tuner.h  |    4 +
 7 files changed, 190 insertions(+), 32 deletions(-)

Loading...