All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: James Courtier-Dutton <James@superbug.co.uk>
Cc: Jaroslav Kysela <perex@suse.cz>,
	alsa-devel <alsa-devel@lists.sourceforge.net>
Subject: Re: [PATCH] Adds dB gain to alsa-driver, alsa-lib.
Date: Fri, 16 Dec 2005 13:05:51 +0100	[thread overview]
Message-ID: <s5hirtpz1pc.wl%tiwai@suse.de> (raw)
In-Reply-To: <43A29B2A.1000406@superbug.co.uk>

At Fri, 16 Dec 2005 10:47:06 +0000,
James Courtier-Dutton wrote:
> 
> Takashi Iwai wrote:
> > At Thu, 15 Dec 2005 19:45:57 +0000,
> > James Courtier-Dutton wrote:
> > 
> >>>2) The ca0106 driver is not candidate for such info. The info should
> >>>   be static somewhere in alsa-lib, probably in the src/control/control.c
> >>>   and added only when TLV stuff is requested.
> >>
> >>What "info" are you talking about? If you are talking about the db_scale 
> >>information, then I disagree with you on this point and I explained why 
> >>some time ago in a previous thread. I think Takashi agreed with me in 
> >>the end, once I managed to fully explain the reasons.
> > 
> > 
> > Err, then it's a misunderstanding.  I'm also against inclusion of
> > unnecessary code in the kernel.  That is, if the dB information for
> > each control is static and unchanged, it shouldn't be embedded in the
> > driver.
> > 
> > I agree to add a new ioctl for drivers with dynamic controls or
> > the drivers taking the dB information from the codec chip.
> > The former case is emu10k1, which mixer can be changed via ld10k1.
> > The latter case is usb-audio and hda-intel.
> > 
> > Since the number of drivers requring this ioctl should be relatively
> > small, I suppose that the implementation would be hardware-dependent.
> > So, a generic "hint" and a dedicated parser in alsa-lib would suffice,
> > rather than defining a too complicated struct to cover everyhing about
> > dB.
> > 
> > The case of ac97 needs a further investigation.  I believe it can be
> > implemented well in alsa-lib, but the implementation on the driver
> > side _might_ be more efficient.  Let's see the code.
> > 
> > 
> > Takashi
> > 
> > 
> 
> Ok, somone else can write this. I give up.
> 
> Your solution is going to break down in the edge cases, and mine adds 
> just 4 bytes to each mixer control structure in the kernel. No extra 
> code at all to each driver. Just some lines of extra core code to handle 
> the new ioctl.
> 
> Your suggestions will add considerably to code bloat in the alsa-lib, 
> and probably not work very well due to the edge cases.

The code won't be compliex nor bload so much as you thought.
Imagine that you just move the table of dB information into alsa-lib
like

	static struct db_info_lookup table[] = {
		{ "ca0106", db_info_ca0106 },
		...
	};

The static table could be implemented as an external file, too, of
course.

OTOH, the lookup-table in alsa-lib has a drawback that the table must
includes the whole snd_ctl_elem_id, thus the memory usage about the
control name will be doubled.  So the actual size might be bigger.


... OK, now I've been slowly convinced to add minimal dB information
to the kernel side.  The further question is the implementation.

IMO, your method, everything-about-parser-is-in-alsa-lib, is a bit
exaggerated.  In that way, you'll have to add a different type for
each driver and chip and end up with endless bloat of new types.

As Liam suggested, most cases are linear.  They can be embedded in 4
bytes, including min, max, step and 0dB-offset.  Take a look at
HD-audio codec or USB-audio specification.
(BTW, the floating point expression is (still) prohibitied in the
 kernel code.)

One exception is the volume muting at the lowest value as often seen
in ac97 codecs.  This needs a different type.
More than 90% cases could be coverted by these two types (linear and
linear+mute), I guess.


In addition, some suggestions/questions about your patch:

- The name snd_ctl_misc sounds too ambiguous

- (in kernel) Move the dB-specific stuff out of snd_ctl_elem_misc_user()

- TLV parser (e.g. uint32_to_message) should be more optimized :)

- As Jaroslav pointed, the parsing of dB information should be in the
  mixer layer instead of [h]control.

- How to implement the reverse conversion, from dB to raw?


Takashi


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

  reply	other threads:[~2005-12-16 12:05 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-12-15 18:34 [PATCH] Adds dB gain to alsa-driver, alsa-lib James Courtier-Dutton
2005-12-15 18:58 ` Lee Revell
2005-12-15 19:06   ` James Courtier-Dutton
2005-12-15 19:16     ` Lee Revell
2005-12-15 19:13 ` Jaroslav Kysela
2005-12-15 19:45   ` James Courtier-Dutton
2005-12-16 10:27     ` Takashi Iwai
2005-12-16 10:47       ` James Courtier-Dutton
2005-12-16 12:05         ` Takashi Iwai [this message]
2005-12-16 12:14           ` Jaroslav Kysela
2005-12-16 12:43           ` James Courtier-Dutton
2005-12-16 13:27             ` Takashi Iwai
2005-12-16 13:47               ` James Courtier-Dutton
2005-12-16 14:11                 ` Takashi Iwai
2005-12-16 14:11                   ` Jaroslav Kysela
2005-12-16 14:55                     ` Liam Girdwood
2005-12-16 11:09       ` Liam Girdwood
2005-12-16 12:37     ` Jaroslav Kysela
2005-12-16 13:08       ` James Courtier-Dutton
2005-12-16 14:05         ` Jaroslav Kysela
2005-12-16 14:24   ` Takashi Iwai
2005-12-15 19:32 ` Liam Girdwood
2005-12-15 19:48   ` James Courtier-Dutton
2005-12-16 10:49     ` Liam Girdwood

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=s5hirtpz1pc.wl%tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=James@superbug.co.uk \
    --cc=alsa-devel@lists.sourceforge.net \
    --cc=perex@suse.cz \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.