Hi,
Ruslan Bilovol <[email protected]> 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.
> 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?
--
balbi
Hi,
On Fri, Jun 2, 2017 at 12:42 PM, Felipe Balbi <[email protected]> wrote:
>
> Hi,
>
> Ruslan Bilovol <[email protected]> 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.
[1] https://lkml.org/lkml/2016/5/23/649
[2] https://marc.info/?t=146404758800001&r=1&w=4
Regards,
Ruslan
Hi,
Ruslan Bilovol <[email protected]> writes:
>> Ruslan Bilovol <[email protected]> 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?
--
balbi
On Mon, Jun 05, 2017 at 12:22:13PM +0300, Felipe Balbi wrote:
>
> Hi,
>
> Ruslan Bilovol <[email protected]> writes:
> >> Ruslan Bilovol <[email protected]> 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
Hi,
Greg KH <[email protected]> writes:
>> > 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...
Okay, I don't think many will continue to use f_uac1.c. Ruslan, can you
update your series so that current f_uac1.c gets renamed to
f_uac1_legacy.c and you introduce a *new* f_uac1.c instead?
Thanks a lot
--
balbi
On Tue, Jun 6, 2017 at 12:41 PM, Felipe Balbi <[email protected]> wrote:
>
> Hi,
>
> Greg KH <[email protected]> writes:
>>> > 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...
>
> Okay, I don't think many will continue to use f_uac1.c. Ruslan, can you
> update your series so that current f_uac1.c gets renamed to
> f_uac1_legacy.c and you introduce a *new* f_uac1.c instead?
Yes sure, I'll post an updated patch series soon
Best regards,
Ruslan