Em Ter, 2007-09-04 às 14:04 -0700, Trent Piepho escreveu:
On Tue, 4 Sep 2007, Takashi Iwai wrote:
At Tue, 04 Sep 2007 14:54:39 +0100, Mauro Carvalho Chehab wrote:
SND_BT87X_BOARD_HAUPPAUGE878,SND_BT87X_BOARD_OSPREY2x0,SND_BT87X_BOARD_OSPREY440,SND_BT87X_BOARD_ATI_TVWONDER,SND_BT87X_BOARD_WINFAST2000,SND_BT87X_BOARD_VOODOOTV_200,SND_BT87X_BOARD_AVPHONE98,Hmm... Maybe it would be a good idea to move the board numbers from linux/drivers/media/video/bt8xx/bttv.h to a separate header (maybe something like /include/media/bt8xx-cards.h) and use the same board IDs for both -alsa and -v4l drivers.
This will make easier to associate the audio and video parts for troubleshooting and newer developments.
Yes, it sounds like a good idea.
There are few drawbacks.
First, there are like 20x more cards supported by the bttv driver than by the bt87x driver. It's going to produce a very sparse card database in the ALSA driver. Note that for the most part, __initdata doesn't work, so you can't say "it'll just be dropped after the driver loads."
You should notice that even the way you've mapped is a a little sparse, since just two boards use digital_fmt.
Maybe one solution for that is to use a different code or data structure.
For example, instead of using a data struct, you could use some sort of code like (*) (**):
static void fill_board (struct snd_bt87x_board &board, int board_id) { board->dig_rate = 32000; switch (board_id) { case BTTV_BOARD_AVPHONE98: board.dig_rate = 48000, break; case BTTV_BOARD_OSPREY2xx: board->dig_rate =44100; board->digital_fmt = CTL_DA_LRI | (1 << CTL_DA_LRD_SHIFT); break; case BTTV_BOARD_OSPREY440: board->digital_fmt = CTL_DA_LRI | (1 << CTL_DA_LRD_SHIFT); break; } }
(*) GCC optimizer probably will even merge the two digital_fmt changes into just one line of object code, and even transform this into an inline. I suspect that this code will be smaller than the data struct.
(**) I didn't found where SND_BT87X_BOARD_ANALOG is used. An usage would be just a case for each analog board, like: case BTTV_BOARD_PV_BT878P_PLUS: case BTTV_BOARD_PV_BT878P_9B: board->no_digital=1;
Secondly, before this patch there was virtually no difference in the configuration for the different supported cards. After it, most of the cards are still exactly the same. It possible to re-use the same generic configuration entry for most of the supported cards. If you use the bttv IDs, it's not.
See above.
And finally, bt87x is under ALSA but bttv is under linuxtv. What would include/media/bt8xx-cards.h be under, alsa or linuxtv? Will people have to submit patches to both projects (which use different repository formats locally, so you can't just CC the same patch)? I did one patch that touched both linuxtv and alsa files, and it was a PITA, for a very simple patch. Took a month. Emails to different people. Merge conflicts.
This seems quite simple to me: it makes not much sense to provide ALSA support if the video is not supported. So, adding a newer board will always add a new entry at the video support. The opposite is not true, since most boards have just an analog card to be connected to the video device. Also, as you said, most boards will just use the default config.
So, the file will be touched more frequently when adding the video entries. My suggestion is that this file would be maintained together with the video counterpart.
And if both linuxtv and alsa use the same header, what happens if they don't push the changes to GIT at the exact same time? You'll have patches in one subsystem that are dependant on the other subsystem. How will they make the same merge window?
This should be coordinated between me and Takashi. I have no objections if he pull such patch, if he is willing to send the pull request before myself.
I don't see this as a big trouble. There are other cases like this already (pci_ids.h, i2c_id.h). Also, currently, there are not much bttv board additions anymore.
Cheers, Mauro