Re: [PATCH v4 0/3] USB Audio Gadget refactoring

[Greg KH]

On Mon, Jun 05, 2017 at 12:22:13PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Ruslan Bilovol <ruslan.bilovol@xxxxxxxxx> writes:
> >> Ruslan Bilovol <ruslan.bilovol@xxxxxxxxx> writes:
> >>> I came to this patch series when wanted to do two things:
> >>>  - use UAC1 as virtual ALSA sound card on gadget side,
> >>>    just like UAC2 is used so it's possible to do rate
> >>>    resampling
> >>>  - have both playback/capture support in UAC1
> >>>
> >>> Since I wanted to have same behavior for both UAC1/UAC2,
> >>> obviously I've got an utility part (u_audio.c) for
> >>> virtual ALSA sound card handling like we have
> >>> for ethernet(u_ether) or serial(u_serial) functions.
> >>> Function-specific parts (f_uac1/f_uac2) became almost
> >>> as storage for class-specific USB descriptors, some
> >>> boilerplate for configfs, binding and few USB
> >>> config request handling.
> >>>
> >>> Originally in RFC [1] I've posted before, there was
> >>> major change to f_uac1 after that it couldn't do
> >>> direct play to existing ALSA sound card anymore,
> >>> representing audio on gadget side as virtual
> >>> ALSA sound card where audio streams are simply
> >>> sinked to and sourced from it, so it may break
> >>> current usecase for some people (and that's why
> >>> it was RFC).
> >>>
> >>> During RFC discussion, it was agreed to not touch
> >>> existing f_uac1 implementation and create new one
> >>> instead. This patchset (v4) introduced new function
> >>> named f_uac1_acard and doesn't touch current f_uac1
> >>> implementation, so people still can use old behavior
> >>
> >> Do you have a pointer to the original RFC discussion where this was
> >> discussed? If we really *must* keep the old implementation, I would
> >> rather rename that to f_uac1_legacy. Still, I find it unlikely that
> >> anybody will care about the old implementation.
> >
> > It is on LKML (which is down for me) [1] or alternative archive [2]
> >
> >>
> >>> Now, it's possible to use existing user-space
> >>> applications for audio routing between Audio Gadget
> >>> and real sound card. I personally use alsaloop tool
> >>> from alsautils and have ability to create PCM
> >>> loopback between two different ALSA cards using
> >>> rate resampling, which was not possible with previous
> >>> "direct play to ALSA card" approach in f_uac1.
> >>
> >> this is really good result and will actually make it a lot easier for
> >> testing things out.
> >>
> >>> While here, also dropped redundant platform
> >>> driver/device creation in f_uac2 driver (as well as
> >>> didn't add "never implemented" volume/mute functionality
> >>> in f_uac1 to f_uac1_acard) that made this work even
> >>> easier to do.
> >>>
> >>> This series is tested with both legacy g_audio.ko and
> >>> modern configfs approaches under Ubuntu 14.04 (UAC1 and
> >>> UAC2) and under Windows7 x64 (UAC1 only) having
> >>> perfect results in all cases.
> >>>
> >>> Comments, testing are welcome.
> >>>
> >>> v4 changes:
> >>>  - renamed f_uac1_newapi to f_uac1_acard that is
> >>>    more meaningful
> >>
> >> I really don't get why you wanna keep both f_uac1 and f_uac1_acard. Why
> >> do we need to maintain the old uac1 implementation? Why two separate
> >> files?
> >
> > In first RFC ([1],[2]) I did exactly what you wrote here (removed
> > old uac1 implementation and replaced it by new one) but got feedback
> > that it will break things for existing f_uac1 legacy users and it's better to
> > have separate implementation.
> >
> > I'm OK with dropping legacy f_uac1 implementation.
> >
> > Another idea I was thinking about is to implement simple in-kernel
> > driver which will do the same as existing alsaloop tool userspace
> > tool does (so legacy users will need to load two kernel modules
> > and get same functionality). But this seems to be a wrong way,
> > since It known that Linux kernel community doesn't like to take drivers
> > with same functionality as existing userspace tools already have.
> >
> > So bottom line: since I'm not a legacy f_uac1 user, there is no
> > difference for me how to handle it - remove legacy f_uac1 completely,
> > rename it to f_uac1_legacy or add separate f_uac1_acard function.
> >
> > So if dropping of legacy f_uac1 implementation is OK for you,
> > I can do it quickly in next patchset.
> 
> Personally, I don't want duplicated functionality and I think the
> virtual sound card approach is much better. Then again, removing
> functionality we already support is kind of odd.
> 
> Greg, Alan, what do you guys think? Do we keep a duplicated function
> around or do we just tell people to rely on alsaloop? Personally, I
> think we're better off with the flexibility of the virtual sound card,
> what's your take?

If the in-kernel driver will do more things, and we don't break the
existing setups using alsaloop, then I don't see the problem, except
that we now have to maintain two things :)

If you don't mind the maintenance, fine with me...

thanks,

greg k-h