Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1443791 > unrolled thread

Re: [RFC PATCH 0/5] USB Audio Gadget refactoring

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 0/5] USB Audio Gadget refactoring Ruslan Bilovol <ruslan.bilovol@gmail.com> - 2016-07-14 23:40 +0200
    Re: [RFC PATCH 0/5] USB Audio Gadget refactoring Clemens Ladisch <clemens@ladisch.de> - 2016-07-15 10:00 +0200
      Re: [RFC PATCH 0/5] USB Audio Gadget refactoring Ruslan Bilovol <ruslan.bilovol@gmail.com> - 2016-07-26 03:40 +0200
        Re: [RFC PATCH 0/5] USB Audio Gadget refactoring Clemens Ladisch <clemens@ladisch.de> - 2016-07-26 10:10 +0200
          Re: [RFC PATCH 0/5] USB Audio Gadget refactoring Ruslan Bilovol <ruslan.bilovol@gmail.com> - 2016-07-26 23:30 +0200
        Re: [RFC PATCH 0/5] USB Audio Gadget refactoring Jassi Brar <jassisinghbrar@gmail.com> - 2016-07-26 11:00 +0200
          Re: [RFC PATCH 0/5] USB Audio Gadget refactoring Krzysztof Opasiak <k.opasiak@samsung.com> - 2016-07-26 14:10 +0200
            Re: [RFC PATCH 0/5] USB Audio Gadget refactoring Ruslan Bilovol <ruslan.bilovol@gmail.com> - 2016-07-27 00:00 +0200
          Re: [RFC PATCH 0/5] USB Audio Gadget refactoring Ruslan Bilovol <ruslan.bilovol@gmail.com> - 2016-07-26 23:40 +0200

#1443791 — Re: [RFC PATCH 0/5] USB Audio Gadget refactoring

Ping?

On Wed, Jun 8, 2016 at 11:03 AM, Ruslan Bilovol
<ruslan.bilovol@gmail.com> wrote:
> Hi guys,
>
> Any feedback on this patch series? Has anybody had a chance to test it?
>
> Regards,
> Ruslan
>
> On Tue, May 24, 2016 at 2:50 AM, Ruslan Bilovol
> <ruslan.bilovol@gmail.com> wrote:
>> 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-specfic USB descriptors, some
>> boilerplate for configfs, binding and few USB
>> config request handling.
>>
>> Major change to f_uac1 it that it can'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's RFC).
>>
>> Luckily, 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 is not possible with previous
>> "direct play to ALSA card" approach in f_uac1.
>>
>> While here, also dropped redundant platform
>> driver/device creation in f_uac2 driver as well as
>> "never implemented" volume/mute functionality in f_uac1
>> 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.
>>
>> Some changes may have lack of good description that may
>> be obvious for me but not so clear for others, but I
>> hope to fix it in next versions.
>>
>> Comments, testing are welcome.
>>
>> Ruslan Bilovol (5):
>>   usb: gadget: f_uac2: remove platform driver/device creation
>>   usb: gadget: f_uac2: split out audio core
>>   usb: gadget: f_uac1: drop volume/mute functionality
>>   usb: gadget: f_uac1: switch to u_audio core utilities
>>   usb: gadget: f_uac1: add capture support
>>
>>  drivers/usb/gadget/Kconfig            |  13 +-
>>  drivers/usb/gadget/function/Makefile  |   3 +-
>>  drivers/usb/gadget/function/f_uac1.c  | 842 +++++++++++++---------------------
>>  drivers/usb/gadget/function/f_uac2.c  | 778 ++++---------------------------
>>  drivers/usb/gadget/function/u_audio.c | 632 +++++++++++++++++++++++++
>>  drivers/usb/gadget/function/u_audio.h |  93 ++++
>>  drivers/usb/gadget/function/u_uac1.c  | 314 -------------
>>  drivers/usb/gadget/function/u_uac1.h  |  71 +--
>>  drivers/usb/gadget/legacy/Kconfig     |   1 +
>>  drivers/usb/gadget/legacy/audio.c     |  54 ++-
>>  10 files changed, 1208 insertions(+), 1593 deletions(-)
>>  create mode 100644 drivers/usb/gadget/function/u_audio.c
>>  create mode 100644 drivers/usb/gadget/function/u_audio.h
>>  delete mode 100644 drivers/usb/gadget/function/u_uac1.c
>>
>> --
>> 1.9.1
>>

[toc] | [next] | [standalone]

#1444025

>> On Tue, May 24, 2016 at 2:50 AM, Ruslan Bilovol
>> <ruslan.bilovol@gmail.com> wrote:
>>> it may break current usecase for some people

And what are the benefits that justify breaking the kernel API?


Regards,
Clemens

[toc] | [prev] | [next] | [standalone]

#1450315

On Fri, Jul 15, 2016 at 10:43 AM, Clemens Ladisch <clemens@ladisch.de> wrote:
>>> On Tue, May 24, 2016 at 2:50 AM, Ruslan Bilovol
>>> <ruslan.bilovol@gmail.com> wrote:
>>>> it may break current usecase for some people
>
> And what are the benefits that justify breaking the kernel API?


Main limitation with current f_uac1 design is - it can be used only on systems
with real ALSA card present and can have only exact number of
channels / sampling rate as sink card has.
Yet it is not flexible - can't do audio processing between f_uac1 and the card.
Also if someone wants to bind f_uac1 it to another sound card he has to
unload g_audio or reconfigure it through configfs - that means USB
reenumeration on host device.

If you have a "virtual sound card", audio processing is done in userspace
and is more flexible. You even don't need to have a real sound card and
can use some userspace application for playing/capturing audio samples.
Moreover, existing f_uac2 (that is USB Audio Class 2.0 function
implementation) already uses approach of "virtual sound card"

A real cases when it's required to have UAC1 gadget represented as virtual
sound card on gadget side:
 - android accessory f_audio_source.c implementation, android plays audio
   directly to UAC1 "virtual sound card"
 - some 3G/LTE voice USB sticks with Linux as firmware have user-space
   application inside that receives audio from network and need to source/sink
   it to some sound card (UAC1)
 - USB sound card with complex audio processing inside, made on small
   Linux-powered device ("sound-studio")

What's annoying is we have two quite similar USB Audio Classes
implementations(f_uac1 and f_uac2) that have opposite audio
representations: first transfers audio samples directly to real ALSA card,
second exposes virtual ALSA card. That means you have to implement
similar things (capture/playback, etc) in different way. With new design
both implementations provide same "API" (virtual sound card), allowing
us to reuse a lot of code and implement new features much easier (look
at adding of capture support to f_uac1 in PATCH 5/5 - that was very simple
and consists almost from adding new USB descriptors and reusing existing
code from newly created u_audio.c)

Also new USB Audio Gadget design follows existing approach for
another USB Classes:
 - serial gadgets use u_serial and expose virtual TTY port
 - networking gadgets use u_ether and expose virtual network interface
 - uvc gadget exposes virtual v4l device
 - midi gadget exposes virtual sound card
 - etc, etc

Of course disadvantage of new approach for UAC1 gadget is you need to
use some userspace application for routing audio from virtual to real
sound card, like in case of UAC2 gadget. But thanks to existing
applications like alsaloop it's not difficult nowadays.

The answer I want to get in this RFC - can we drop current f_uac1 approach,
simplify USB Audio Gadget by reusing common code.
Or "we do not break userspace" (or API) and have to live with it forever.

Best regards,
Ruslan

[toc] | [prev] | [next] | [standalone]

#1450459

Ruslan Bilovol wrote:
> On Fri, Jul 15, 2016 at 10:43 AM, Clemens Ladisch <clemens@ladisch.de> wrote:
>>>> On Tue, May 24, 2016 at 2:50 AM, Ruslan Bilovol
>>>> <ruslan.bilovol@gmail.com> wrote:
>>>>> it may break current usecase for some people
>>
>> And what are the benefits that justify breaking the kernel API?
>
> Main limitation with current f_uac1 design is - it can be used only on systems
> with real ALSA card present and can have only exact number of
> channels / sampling rate as sink card has. [...]
> A real cases when it's required to have UAC1 gadget represented as virtual
> sound card on gadget side: [...]

Thanks.

> Of course disadvantage of new approach for UAC1 gadget is you need to
> use some userspace application for routing audio from virtual to real
> sound card, like in case of UAC2 gadget. But thanks to existing
> applications like alsaloop it's not difficult nowadays.

I don't know what the maintainer will say, but you would increase the
chances of this change being accepted when you show a concrete example
of how to change the userspace configuration from the old to the new
driver.  (And add it to the documentation.)


Regards,
Clemens

[toc] | [prev] | [next] | [standalone]

#1450922

On Tue, Jul 26, 2016 at 11:06 AM, Clemens Ladisch <clemens@ladisch.de> wrote:
> Ruslan Bilovol wrote:
>> On Fri, Jul 15, 2016 at 10:43 AM, Clemens Ladisch <clemens@ladisch.de> wrote:
>>>>> On Tue, May 24, 2016 at 2:50 AM, Ruslan Bilovol
>>>>> <ruslan.bilovol@gmail.com> wrote:
>>>>>> it may break current usecase for some people
>>>
>>> And what are the benefits that justify breaking the kernel API?
>>
>> Main limitation with current f_uac1 design is - it can be used only on systems
>> with real ALSA card present and can have only exact number of
>> channels / sampling rate as sink card has. [...]
>> A real cases when it's required to have UAC1 gadget represented as virtual
>> sound card on gadget side: [...]
>
> Thanks.
>
>> Of course disadvantage of new approach for UAC1 gadget is you need to
>> use some userspace application for routing audio from virtual to real
>> sound card, like in case of UAC2 gadget. But thanks to existing
>> applications like alsaloop it's not difficult nowadays.
>
> I don't know what the maintainer will say, but you would increase the
> chances of this change being accepted when you show a concrete example
> of how to change the userspace configuration from the old to the new
> driver.  (And add it to the documentation.)

Good point, need to update documentation in next files (make similar to UAC2):
  Documentation/usb/gadget-testing.txt
  Documentation/ABI/testing/configfs-usb-gadget-uac1

Best regards,
Ruslan

[toc] | [prev] | [next] | [standalone]

#1450481

On Tue, Jul 26, 2016 at 7:01 AM, Ruslan Bilovol
<ruslan.bilovol@gmail.com> wrote:
> On Fri, Jul 15, 2016 at 10:43 AM, Clemens Ladisch <clemens@ladisch.de> wrote:
>>>> On Tue, May 24, 2016 at 2:50 AM, Ruslan Bilovol
>>>> <ruslan.bilovol@gmail.com> wrote:
>>>>> it may break current usecase for some people
>>
>> And what are the benefits that justify breaking the kernel API?
>
>
> Main limitation with current f_uac1 design is - it can be used only on systems
> with real ALSA card present and can have only exact number of
> channels / sampling rate as sink card has.
> Yet it is not flexible - can't do audio processing between f_uac1 and the card.
> Also if someone wants to bind f_uac1 it to another sound card he has to
> unload g_audio or reconfigure it through configfs - that means USB
> reenumeration on host device.
>
> If you have a "virtual sound card", audio processing is done in userspace
> and is more flexible. You even don't need to have a real sound card and
> can use some userspace application for playing/capturing audio samples.
> Moreover, existing f_uac2 (that is USB Audio Class 2.0 function
> implementation) already uses approach of "virtual sound card"
>
While I agree the virtual sound card approach is the right way, I am
not sure if we should break the userspace api that the existing UAC1
driver exposes. Maybe we should add another virtual-sound-card
exposing UAC1 driver ... and hopefully very similar to (or just port
of) the f_audio_source.c from android.

[toc] | [prev] | [next] | [standalone]

#1450548

On 07/26/2016 10:53 AM, Jassi Brar wrote:
> On Tue, Jul 26, 2016 at 7:01 AM, Ruslan Bilovol
> <ruslan.bilovol@gmail.com> wrote:
>> On Fri, Jul 15, 2016 at 10:43 AM, Clemens Ladisch <clemens@ladisch.de> wrote:
>>>>> On Tue, May 24, 2016 at 2:50 AM, Ruslan Bilovol
>>>>> <ruslan.bilovol@gmail.com> wrote:
>>>>>> it may break current usecase for some people
>>>
>>> And what are the benefits that justify breaking the kernel API?
>>
>>
>> Main limitation with current f_uac1 design is - it can be used only on systems
>> with real ALSA card present and can have only exact number of
>> channels / sampling rate as sink card has.
>> Yet it is not flexible - can't do audio processing between f_uac1 and the card.
>> Also if someone wants to bind f_uac1 it to another sound card he has to
>> unload g_audio or reconfigure it through configfs - that means USB
>> reenumeration on host device.
>>
>> If you have a "virtual sound card", audio processing is done in userspace
>> and is more flexible. You even don't need to have a real sound card and
>> can use some userspace application for playing/capturing audio samples.
>> Moreover, existing f_uac2 (that is USB Audio Class 2.0 function
>> implementation) already uses approach of "virtual sound card"
>>
> While I agree the virtual sound card approach is the right way, I am
> not sure if we should break the userspace api that the existing UAC1
> driver exposes. Maybe we should add another virtual-sound-card
> exposing UAC1 driver ... and hopefully very similar to (or just port
> of) the f_audio_source.c from android.

Definitely agree with this opinion. I don't see any benefits of breaking
the API here instead of adding just another USB function. Maybe even
some pieces of code could be shared with f_uac1.c but I think that this
should be a brand new function.

Best regards,
-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics

[toc] | [prev] | [next] | [standalone]

#1450937

On Tue, Jul 26, 2016 at 3:02 PM, Krzysztof Opasiak
<k.opasiak@samsung.com> wrote:
>
>
> On 07/26/2016 10:53 AM, Jassi Brar wrote:
>> On Tue, Jul 26, 2016 at 7:01 AM, Ruslan Bilovol
>> <ruslan.bilovol@gmail.com> wrote:
>>> On Fri, Jul 15, 2016 at 10:43 AM, Clemens Ladisch <clemens@ladisch.de> wrote:
>>>>>> On Tue, May 24, 2016 at 2:50 AM, Ruslan Bilovol
>>>>>> <ruslan.bilovol@gmail.com> wrote:
>>>>>>> it may break current usecase for some people
>>>>
>>>> And what are the benefits that justify breaking the kernel API?
>>>
>>>
>>> Main limitation with current f_uac1 design is - it can be used only on systems
>>> with real ALSA card present and can have only exact number of
>>> channels / sampling rate as sink card has.
>>> Yet it is not flexible - can't do audio processing between f_uac1 and the card.
>>> Also if someone wants to bind f_uac1 it to another sound card he has to
>>> unload g_audio or reconfigure it through configfs - that means USB
>>> reenumeration on host device.
>>>
>>> If you have a "virtual sound card", audio processing is done in userspace
>>> and is more flexible. You even don't need to have a real sound card and
>>> can use some userspace application for playing/capturing audio samples.
>>> Moreover, existing f_uac2 (that is USB Audio Class 2.0 function
>>> implementation) already uses approach of "virtual sound card"
>>>
>> While I agree the virtual sound card approach is the right way, I am
>> not sure if we should break the userspace api that the existing UAC1
>> driver exposes. Maybe we should add another virtual-sound-card
>> exposing UAC1 driver ... and hopefully very similar to (or just port
>> of) the f_audio_source.c from android.
>
> Definitely agree with this opinion. I don't see any benefits of breaking
> the API here instead of adding just another USB function. Maybe even
> some pieces of code could be shared with f_uac1.c but I think that this
> should be a brand new function.
>

So if we want to keep old API working, easiest (and cleanest) way is
to create a new f_uac1.c version and kconfig symbol, for example
f_uac1_newapi.c and CONFIG_USB_F_UAC1_NEWAPI
There is no sence to share some pieces of code with f_uac1.c just
because it is changed too drastically.

So I'll implement it in v2 if there is no any objections

Best regards,
Ruslan Bilovol

[toc] | [prev] | [next] | [standalone]

#1450930

On Tue, Jul 26, 2016 at 11:53 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> On Tue, Jul 26, 2016 at 7:01 AM, Ruslan Bilovol
> <ruslan.bilovol@gmail.com> wrote:
>> On Fri, Jul 15, 2016 at 10:43 AM, Clemens Ladisch <clemens@ladisch.de> wrote:
>>>>> On Tue, May 24, 2016 at 2:50 AM, Ruslan Bilovol
>>>>> <ruslan.bilovol@gmail.com> wrote:
>>>>>> it may break current usecase for some people
>>>
>>> And what are the benefits that justify breaking the kernel API?
>>
>>
>> Main limitation with current f_uac1 design is - it can be used only on systems
>> with real ALSA card present and can have only exact number of
>> channels / sampling rate as sink card has.
>> Yet it is not flexible - can't do audio processing between f_uac1 and the card.
>> Also if someone wants to bind f_uac1 it to another sound card he has to
>> unload g_audio or reconfigure it through configfs - that means USB
>> reenumeration on host device.
>>
>> If you have a "virtual sound card", audio processing is done in userspace
>> and is more flexible. You even don't need to have a real sound card and
>> can use some userspace application for playing/capturing audio samples.
>> Moreover, existing f_uac2 (that is USB Audio Class 2.0 function
>> implementation) already uses approach of "virtual sound card"
>>
> While I agree the virtual sound card approach is the right way, I am
> not sure if we should break the userspace api that the existing UAC1
> driver exposes. Maybe we should add another virtual-sound-card
> exposing UAC1 driver

This approach is quite easy to implement and I though about it when
started to work on this patch series, but due to my lazyness I wanted
to get some comments before doing extra work.

> ... and hopefully very similar to (or just port
> of) the f_audio_source.c from android.

Current patch series reuses code from f_uac2 (in u_audio) and
makes it in some sense similar to f_audio_source.c.

The f_audio_source.c implementation has additional functionality
like more accurate data transferring (w.r.t timings), that can be
ported to u_audio separately.

Best regards,
Ruslan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web