[alsa-devel] [PATCH 5/6] ASoC: add Component level struct snd_compr_ops

[Charles Keepax]

On Wed, Dec 21, 2016 at 12:42:21AM +0000, Kuninori Morimoto wrote:
> 
> Hi Charles
> 
> Thank you for your feedback
> 
> > > 	for_each_component(component, rtd->card) {
> > > 		const struct snd_compr_ops *compr_ops = component->compr_ops;
> > > 
> > > 		/* FIXME: 1st compr_ops only at this point */
> > > 		if (compr_ops && compr_ops->open) {
> > 
> > But how do you know this is the correct compressed open here? The
> > system could have multiple platforms providing compressed ops and
> > you have just picked the first one in the card list. I think you
> > are making the assumption that there is only one platform
> > providing compressed ops and that seems like a very dangerous
> > assumption to me. Our CODECs provide some compressed features as
> > do many applications processors, which would easily give you at
> > two platforms. But I could even imagine APs registering multiple
> > compressed platforms for different DSPs or some such.
> 
> I agree your opinion, and yes it can be issue in the future.
> But, this is the reason why it has /* FIXME */ comment here
> (will has in v2, not yet in v1).
> I don't know which one should be solved first, but this
> patch is focusing to part of platform -> component conversion now.
> No existing code/card get damage at this point, I think ?
> 
> So, how about this in v2 ?
> In comment and git log indicates
> "It is still assumeing that system has 1 compressed,
> same as current situation. Multi-compress is TBD."

Admittedly there are no machines currently in mainline that do
this but it will basically break on every single one of our
customer integrations, as most AP systems are supporting some
sort of compressed stream stuff now and our CODEC does as well.

I would really rather see this issue fixed before these patches
are merged, as if it gets merged I basically have to fix it
straight away anyway, or face some very painful integrations in
the future. I will try to find some time to look at this and see
if I can come up with a better solution for these parts of the
code but given the current time frame it is likely to push into
the new year.

Thanks,
Charles