[alsa-devel] PATCH - ESI Juli driver
Takashi Iwai
tiwai at suse.de
Mon Mar 17 14:04:11 CET 2008
At Mon, 17 Mar 2008 10:37:22 +0100,
Pavel Hofman wrote:
>
> >>
> >> I did not want to copy/paste any algorithmical code from ice1724.c to
> >> juli.c as it complicates future maintenance. Now, the vt1724 code in
> >> ice1724.c is pretty much about general algorhitms, specific routines are
> >> in stdclock_... I tested the original clock with Prodigy192 which
> >> provided testing analog/SPDIF signals. There are no functional changes.
> >
> > I see the point that you want to make it generic. But, too many
> > callbacks, in other words, too many tuning parameters make the code
> > difficult to follow. It's a question of balance.
>
> Takashi, I understand your concern. On the other hand, if the callbacks
> are well documented, it actually makes the code easier to follow, as it
> separates general algorithms from detailed HW implementation. I can
> certainly add more comments to the callback functions.
>
> Honestly, I do not know which callbacks to remove (apart of those two
> not redefined in Juli). Otherwise I will have to copy/paste serious
> portions of code to juli.c which I would really want to avoid.
>
> As I see it, the clocking code in ice1724 was written for one specific
> implementation. There was no need to do so in a general way and I
> completely understand that. Nevertheless, juli uses a different clock
> implementation and I believe the time has come to make the code in
> ice1724 more general. Unfortunately, I do not know of any other
> technology but callbacks.
>
> What would you recommend?
IMO, rate_code can be avoided. Instead of exposing the encoded value,
better to use the raw rate value as parameters.
And, the texts inf rates_info can be generated dynamically.
So, what we need primarily are callbacks to get and set the current
rate setting. Suppose rate=0 as SPDIF-in, we can pass the raw rate
value. Then snd_vt1724_pro_internal_clock_get() would just a function
to get the current rate and compares it with the given rate_info[]
value, returns the index.
How to set stream-specific hw_params is another question. But surely
we can cut off a bit more.
thanks,
Takashi
More information about the Alsa-devel
mailing list