[alsa-devel] [PATCH] Improved support for different bt87x board configurations

[Mauro Carvalho Chehab]

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