From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from rn-mailsvcp-ppex-lapp35.apple.com (rn-mailsvcp-ppex-lapp35.apple.com [17.179.253.44]) by mx.groups.io with SMTP id smtpd.web10.1133.1618680699999429604 for ; Sat, 17 Apr 2021 10:31:40 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=PY2QxUnp; spf=pass (domain: apple.com, ip: 17.179.253.44, mailfrom: afish@apple.com) Received: from pps.filterd (rn-mailsvcp-ppex-lapp35.rno.apple.com [127.0.0.1]) by rn-mailsvcp-ppex-lapp35.rno.apple.com (8.16.1.2/8.16.1.2) with SMTP id 13HHS2Ju009670; Sat, 17 Apr 2021 10:31:39 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=from : message-id : content-type : mime-version : subject : date : in-reply-to : cc : to : references; s=20180706; bh=jOCOUhL+fPa44mWY8s1iJ66WDe/BrdaTsrnJQZruL+E=; b=PY2QxUnpbrMP0PR/ESgRMTtxpLzrW2X+sHooxGvozUh7ZXxJk3Y5Grhr2LfWnm2G3yXN 77fun+6C5Q/Ecjop5oSIdY3GzJ/8ZhBJLInP6apQ+vlDX7UbotMFHPNTm8WrgQ1YyeK4 hqSC7v5tECWzMSDoSlgREGunUszUydtAGSf4EDc5znTRNtSxQBWhDgtuQcpj532POD8t lAH3sLMjYpLS1iVwnIIrPsfr6UY7HpVHMeZJNJmA9zu3esuHgVR39lJete74eHUo/aXb qYGuoGKxiqeu0uXMpvgHWWyH7zt3oQluInyJjNPa9FJBb1CZLQKiS0/wAzhrLVp3vWWy LA== Received: from rn-mailsvcp-mta-lapp04.rno.apple.com (rn-mailsvcp-mta-lapp04.rno.apple.com [10.225.203.152]) by rn-mailsvcp-ppex-lapp35.rno.apple.com with ESMTP id 3800839xpk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Sat, 17 Apr 2021 10:31:39 -0700 Received: from rn-mailsvcp-mmp-lapp01.rno.apple.com (rn-mailsvcp-mmp-lapp01.rno.apple.com [17.179.253.14]) by rn-mailsvcp-mta-lapp04.rno.apple.com (Oracle Communications Messaging Server 8.1.0.7.20201203 64bit (built Dec 3 2020)) with ESMTPS id <0QRP00HM8Y0RNU00@rn-mailsvcp-mta-lapp04.rno.apple.com>; Sat, 17 Apr 2021 10:31:39 -0700 (PDT) Received: from process_milters-daemon.rn-mailsvcp-mmp-lapp01.rno.apple.com by rn-mailsvcp-mmp-lapp01.rno.apple.com (Oracle Communications Messaging Server 8.1.0.7.20201203 64bit (built Dec 3 2020)) id <0QRP00H00XE0KD00@rn-mailsvcp-mmp-lapp01.rno.apple.com>; Sat, 17 Apr 2021 10:31:39 -0700 (PDT) X-Va-A: X-Va-T-CD: 9ad46be6e1c3c1a24e92ea4dad46d58d X-Va-E-CD: 4730c80ee67030d4f2c83e40b4ab0357 X-Va-R-CD: 6f0325faf294bd23a6d751c620be9d51 X-Va-CD: 0 X-Va-ID: cc116ae9-d37b-4ff2-823d-b2521043ea03 X-V-A: X-V-T-CD: 9ad46be6e1c3c1a24e92ea4dad46d58d X-V-E-CD: 4730c80ee67030d4f2c83e40b4ab0357 X-V-R-CD: 6f0325faf294bd23a6d751c620be9d51 X-V-CD: 0 X-V-ID: 578fbdb6-e2eb-4b5c-8276-f8cc27960563 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.761 definitions=2021-04-17_11:2021-04-16,2021-04-17 signatures=0 Received: from [17.235.49.242] (unknown [17.235.49.242]) by rn-mailsvcp-mmp-lapp01.rno.apple.com (Oracle Communications Messaging Server 8.1.0.7.20201203 64bit (built Dec 3 2020)) with ESMTPSA id <0QRP0070MY0OWU00@rn-mailsvcp-mmp-lapp01.rno.apple.com>; Sat, 17 Apr 2021 10:31:38 -0700 (PDT) From: "Andrew Fish" Message-id: <7459B8C0-EDF0-4760-97E7-D3338312B3DF@apple.com> MIME-version: 1.0 (Mac OS X Mail 14.0 \(3654.20.0.2.1\)) Subject: Re: [edk2-devel] VirtIO Sound Driver (GSoC 2021) Date: Sat, 17 Apr 2021 10:31:36 -0700 In-reply-to: Cc: harlydavidsen@gmail.com, Leif Lindholm , Michael Brown , Mike Kinney , Laszlo Ersek , "Desimone, Nathaniel L" , Rafael Rodrigues Machado , Gerd Hoffmann To: edk2-devel-groups-io , =?utf-8?Q?Marvin_H=C3=A4user?= References: <4AEC1784-99AF-47EF-B7DD-77F91EA3D7E9@apple.com> <309cc5ca-2ecd-79dd-b183-eec0572ea982@ipxe.org> <33e37977-2d27-36a0-89a6-36e513d06b2f@ipxe.org> <6F69BEA6-5B7A-42E5-B6DA-D819ECC85EE5@apple.com> <20210416113447.GG1664@vanye> <10E3436C-D743-4B2F-8E4B-7AD93B82FC92@apple.com> X-Mailer: Apple Mail (2.3654.20.0.2.1) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.761 definitions=2021-04-17_11:2021-04-16,2021-04-17 signatures=0 Content-type: multipart/alternative; boundary="Apple-Mail=_75BB1943-58DC-428A-9963-468E3CF3C1F8" --Apple-Mail=_75BB1943-58DC-428A-9963-468E3CF3C1F8 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On Apr 17, 2021, at 9:51 AM, Marvin H=C3=A4user wro= te: >=20 > On 16.04.21 19:45, Ethin Probst wrote: >> Yes, three APIs (maybe like this) would work well: >> - Start, Stop: begin playback of a stream >> - SetVolume, GetVolume, Mute, Unmute: control volume of output and enab= le muting >> - CreateStream, ReleaseStream, SetStreamSampleRate: Control sample >> rate of stream (but not sample format since Signed 16-bit PCM is >> enough) >> Marvin, how do you suggest we make the events then? We need some way >> of notifying the caller that the stream has concluded. We could make >> the driver create the event and pass it back to the caller as an >> event, but you'd still have dangling pointers (this is C, after all). >> We could just make a IsPlaying() function and WaitForCompletion() >> function and allow the driver to do the event handling -- would that >> work? >=20 > I do not know enough about the possible use-cases to tell. Aside from th= e two functions you already mentioned, you could also take in an (optional)= notification function. > Which possible use-cases does determining playback end have? If it's too= much effort, just use EFI_EVENT I guess, just the less code can mess it up= , the better. >=20 In UEFI EFI_EVENT works much better. There is a gBS-WaitForEvent() functio= n that lets a caller wait on an event. That is basically what the UEFI Shel= l is doing at the Shell prompt. A GUI in UEFI/C is basically an event loop.= = =20 Fun fact: I ended up adding gIdleLoopEventGuid to the MdeModulePkg so the = DXE Core could signal gIdleLoopEventGuid if you are sitting in gBS-WaitForE= vent() and no event is signaled. Basically in EFI nothing is going to happe= n until the next timer tick so the gIdleLoopEventGuid lets you idle the CPU= until the next timer tick. I was forced to do this as the 1st MacBook Air = had a bad habit of thermal tripping when sitting at the UEFI Shell prompt. = After all another name for a loop in C code running on bare metal is a powe= r virus.=20 Thanks, Andrew Fish.=20 > If I remember correctly you mentioned the UEFI Talkbox before, if that i= s more convenient for you, I'm there as mhaeuser. >=20 > Best regards, > Marvin >=20 >>=20 >> On 4/16/21, Andrew Fish wrote: >>>=20 >>>> On Apr 16, 2021, at 4:34 AM, Leif Lindholm wrote: >>>>=20 >>>> Hi Ethin, >>>>=20 >>>> I think we also want to have a SetMode function, even if we don't get >>>> around to implement proper support for it as part of GSoC (although I >>>> expect at least for virtio, that should be pretty straightforward). >>>>=20 >>> Leif, >>>=20 >>> I=E2=80=99m think if we have an API to load the buffer and a 2nd API t= o play the >>> buffer an optional 3rd API could configure the streams. >>>=20 >>>> It's quite likely that speech for UI would be stored as 8kHz (or >>>> 20kHz) in some systems, whereas the example for playing a tune in GRU= B >>>> would more likely be a 44.1 kHz mp3/wav/ogg/flac. >>>>=20 >>>> For the GSoC project, I think it would be quite reasonable to >>>> pre-generate pure PCM streams for testing rather than decoding >>>> anything on the fly. >>>>=20 >>>> Porting/writing decoders is really a separate task from enabling the >>>> output. I would much rather see USB *and* HDA support able to play pc= m >>>> streams before worrying about decoding. >>>>=20 >>> I agree it might turn out it is easier to have the text to speech code= just >>> encode a PCM directly. >>>=20 >>> Thanks, >>>=20 >>> Andrew Fish >>>=20 >>>> / >>>> Leif >>>>=20 >>>> On Fri, Apr 16, 2021 at 00:33:06 -0500, Ethin Probst wrote: >>>>> Thanks for that explanation (I missed Mike's message). Earlier I sen= t >>>>> a summary of those things that we can agree on: mainly, that we have >>>>> mute, volume control, a load buffer, (maybe) an unload buffer, and a >>>>> start/stop stream function. Now that I fully understand the >>>>> ramifications of this I don't mind settling for a specific format an= d >>>>> sample rate, and signed 16-bit PCM audio is, I think, the most widel= y >>>>> used one out there, besides 64-bit floating point samples, which I'v= e >>>>> only seen used in DAWs, and that's something we don't need. >>>>> Are you sure you want the firmware itself to handle the decoding of >>>>> WAV audio? I can make a library class for that, but I'll definitely >>>>> need help with the security aspect. >>>>>=20 >>>>> On 4/16/21, Andrew Fish via groups.io = wrote: >>>>>>=20 >>>>>>> On Apr 15, 2021, at 5:59 PM, Michael Brown wrote: >>>>>>>=20 >>>>>>> On 16/04/2021 00:42, Ethin Probst wrote: >>>>>>>> Forcing a particular channel mapping, sample rate and sample form= at >>>>>>>> on >>>>>>>> everyone would complicate application code. From an application p= oint >>>>>>>> of view, one would, with that type of protocol, need to do the >>>>>>>> following: >>>>>>>> 1) Load an audio file in any audio file format from any storage >>>>>>>> mechanism. >>>>>>>> 2) Decode the audio file format to extract the samples and audio >>>>>>>> metadata. >>>>>>>> 3) Resample the (now decoded) audio samples and convert (quantize= ) >>>>>>>> the >>>>>>>> audio samples into signed 16-bit PCM audio. >>>>>>>> 4) forward the samples onto the EFI audio protocol. >>>>>>> You have made an incorrect assumption that there exists a requirem= ent >>>>>>> to >>>>>>> be able to play audio files in arbitrary formats. This requiremen= t >>>>>>> does >>>>>>> not exist. >>>>>>>=20 >>>>>>> With a protocol-mandated fixed baseline set of audio parameters >>>>>>> (sample >>>>>>> rate etc), what would happen in practice is that the audio files w= ould >>>>>>> be >>>>>>> encoded in that format at *build* time, using tools entirely exter= nal >>>>>>> to >>>>>>> UEFI. The application code is then trivially simple: it just does >>>>>>> "load >>>>>>> blob, pass blob to audio protocol". >>>>>>>=20 >>>>>>=20 >>>>>> Ethin, >>>>>>=20 >>>>>> Given the goal is an industry standard we value interoperability mo= re >>>>>> that >>>>>> flexibility. >>>>>>=20 >>>>>> How about another use case. Lets say the Linux OS loader (Grub) wan= ts >>>>>> to >>>>>> have an accessible UI so it decides to sore sound files on the EFI >>>>>> System >>>>>> Partition and use our new fancy UEFI Audio Protocol to add audio to= the >>>>>> OS >>>>>> loader GUI. So that version of Grub needs to work on 1,000 of diffe= rent >>>>>> PCs >>>>>> and a wide range of UEFI Audio driver implementations. It is a much >>>>>> easier >>>>>> world if Wave PCM 16 bit just works every place. You could add a lo= t of >>>>>> complexity and try to encode the audio on the fly, maybe even in Li= nux >>>>>> proper but that falls down if you are booting from read only media = like >>>>>> a >>>>>> DVD or backup tape (yes people still do that in server land). >>>>>>=20 >>>>>> The other problem with flexibility is you just made the test matrix >>>>>> very >>>>>> large for every driver that needs to get implemented. For something= as >>>>>> complex as Intel HDA how you hook up the hardware and what CODECs y= ou >>>>>> use >>>>>> may impact the quality of the playback for a given board. Your EFI = is >>>>>> likely >>>>>> going to pick a single encoding at that will get tested all the tim= e if >>>>>> your >>>>>> system has audio, but all 50 other things you support not so much. = So >>>>>> that >>>>>> will required testing, and some one with audiophile ears (or an AI >>>>>> program) >>>>>> to test all the combinations. I=E2=80=99m not kidding I get BZs on = the quality >>>>>> of >>>>>> the boot bong on our systems. >>>>>>=20 >>>>>>=20 >>>>>>>> typedef struct EFI_SIMPLE_AUDIO_PROTOCOL { >>>>>>>> EFI_SIMPLE_AUDIO_PROTOCOL_RESET Reset; >>>>>>>> EFI_SIMPLE_AUDIO_PROTOCOL_START Start; >>>>>>>> EFI_SIMPLE_AUDIO_PROTOCOL_STOP Stop; >>>>>>>> } EFI_SIMPLE_AUDIO_PROTOCOL; >>>>>>> This is now starting to look like something that belongs in boot-t= ime >>>>>>> firmware. :) >>>>>>>=20 >>>>>> I think that got a little too simple I=E2=80=99d go back and look a= t the example >>>>>> I >>>>>> posted to the thread but add an API to load the buffer, and then pl= ay >>>>>> the >>>>>> buffer (that way we can an API in the future to twiddle knobs). Tha= t >>>>>> API >>>>>> also implements the async EFI interface. Trust me the 1st thing tha= t is >>>>>> going to happen when we add audio is some one is going to complain = in >>>>>> xyz >>>>>> state we should mute audio, or we should honer audio volume and mut= e >>>>>> settings from setup, or from values set in the OS. Or some one is g= oing >>>>>> to >>>>>> want the volume keys on the keyboard to work in EFI. >>>>>>=20 >>>>>> Also if you need to pick apart the Wave PCM 16 byte file to feed it= into >>>>>> the >>>>>> audio hardware that probably means we should have a library that do= es >>>>>> that >>>>>> work, so other Audio drivers can share that code. Also having a lib= rary >>>>>> makes it easier to write a unit test. We need to be security consci= ous >>>>>> as we >>>>>> need to treat the Audo file as attacker controlled data. >>>>>>=20 >>>>>> Thanks, >>>>>>=20 >>>>>> Andrew Fish >>>>>>=20 >>>>>>> Michael >>>>>>>=20 >>>>>>>=20 >>>>>>>=20 >>>>>>>=20 >>>>>>>=20 >>>>>>=20 >>>>>>=20 >>>>>>=20 >>>>>>=20 >>>>>>=20 >>>>>>=20 >>>>>=20 >>>>> -- >>>>> Signed, >>>>> Ethin D. Probst >>>>=20 >>>>=20 >>>>=20 >>>>=20 >>>=20 >>=20 >=20 >=20 >=20 >=20 --Apple-Mail=_75BB1943-58DC-428A-9963-468E3CF3C1F8 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

On Apr 17, 2= 021, at 9:51 AM, Marvin H=C3=A4user <mhaeuser@posteo.de> wrote:

On 16.04.21 19:45, Ethin Probst wrote:
Yes, three APIs (maybe like this) would work = well:
- Start, Stop: begin playback of a stream
- SetVolume, GetVolume, Mute, Unmute: control volume of output and enable = muting
- CreateStream, ReleaseStream, SetStreamSampleRate: Co= ntrol sample
rate of stream (but not sample format since Sign= ed 16-bit PCM is
enough)
Marvin, how do you sug= gest we make the events then? We need some way
of notifying t= he caller that the stream has concluded. We could make
the dr= iver create the event and pass it back to the caller as an
ev= ent, but you'd still have dangling pointers (this is C, after all).
We could just make a IsPlaying() function and WaitForCompletion()function and allow the driver to do the event handling -- would= that
work?

I do not know enough about the possible use-cases t= o tell. Aside from the two functions you already mentioned, you could also = take in an (optional) notification function.
Which possible use-cases does determining playback en= d have? If it's too much effort, just use EFI_EVENT I guess, just the less = code can mess it up, the better.


In UEFI EFI_EVENT works much better. There is a gBS-WaitF= orEvent() function that lets a caller wait on an event. That is basically w= hat the UEFI Shell is doing at the Shell prompt. A GUI in UEFI/C is basical= ly an event loop. 

Fun fact: I end= ed up adding gIdleLoopEventGuid to the MdeModulePkg so the DXE Core co= uld signal gIdleLoopEventGuid if you are sitting in gBS-WaitForEvent() and = no event is signaled. Basically in EFI nothing is going to happen until the= next timer tick so the gIdleLoopEventGuid lets you idle the CPU until the = next timer tick. I was forced to do this as the 1st MacBook Air had a bad h= abit of thermal tripping when sitting at the UEFI Shell prompt. After all a= nother name for a loop in C code running on bare metal is a power virus.&nb= sp;

Thanks,

Andrew Fish. 

If I remember correctly you mentioned the = UEFI Talkbox before, if that is more convenient for you, I'm there as mhaeu= ser.

Best regards,
Marvin


On 4/16/2= 1, Andrew Fish <afish@appl= e.com> wrote:
On Apr 16, 2021, at 4:34 A= M, Leif Lindholm <leif@n= uviainc.com> wrote:

Hi Ethin,

I think we also want to have a SetMode function, even= if we don't get
around to implement proper support for it as= part of GSoC (although I
expect at least for virtio, that sh= ould be pretty straightforward).

= Leif,

I=E2=80=99m think if we have an API to l= oad the buffer and a 2nd API to play the
buffer an optional 3= rd API could configure the streams.

It's quite likely that speech for UI would be st= ored as 8kHz (or
20kHz) in some systems, whereas the example = for playing a tune in GRUB
would more likely be a 44.1 kHz mp= 3/wav/ogg/flac.

For the GSoC project, I think = it would be quite reasonable to
pre-generate pure PCM streams= for testing rather than decoding
anything on the fly.

Porting/writing decoders is really a separate task f= rom enabling the
output. I would much rather see USB *and* HD= A support able to play pcm
streams before worrying about deco= ding.

I agree it might turn out i= t is easier to have the text to speech code just
encode a PCM= directly.

Thanks,

Andrew Fish

/
   Leif

= On Fri, Apr 16, 2021 at 00:33:06 -0500, Ethin Probst wrote:
<= blockquote type=3D"cite" class=3D"">Thanks for that explanation (I missed M= ike's message). Earlier I sent
a summary of those things that= we can agree on: mainly, that we have
mute, volume control, = a load buffer, (maybe) an unload buffer, and a
start/stop str= eam function. Now that I fully understand the
ramifications o= f this I don't mind settling for a specific format and
sample= rate, and signed 16-bit PCM audio is, I think, the most widely
used one out there, besides 64-bit floating point samples, which I'veonly seen used in DAWs, and that's something we don't need.
Are you sure you want the firmware itself to handle the decoding= of
WAV audio? I can make a library class for that, but I'll = definitely
need help with the security aspect.
=
On 4/16/21, Andrew Fish via groups.io <afish=3Dapple.com@groups.io> wrote:

On Apr 15, 2021, at 5:59 PM, Michael Brown <mcb30@ipxe.org> wrote:

On 16/04/2021 00:42, Ethin Probst wrote:
Forcing a particular channel mapping, sample rate = and sample format
on
everyone would complicate = application code. From an application point
of view, one woul= d, with that type of protocol, need to do the
following:
1) Load an audio file in any audio file format from any storagemechanism.
2) Decode the audio file format to ex= tract the samples and audio
metadata.
3) Resamp= le the (now decoded) audio samples and convert (quantize)
the=
audio samples into signed 16-bit PCM audio.
4)= forward the samples onto the EFI audio protocol.
You have made an incorrect assumption that there exists a requirement
to
be able to play audio files in arbitrary format= s.  This requirement
does
not exist.

With a protocol-mandated fixed baseline set of audi= o parameters
(sample
rate etc), what would happ= en in practice is that the audio files would
be
encoded in that format at *build* time, using tools entirely external
to
UEFI.  The application code is then trivial= ly simple: it just does
"load
blob, pass blob t= o audio protocol".


Ethin,

Given the goal is an industry standard= we value interoperability more
that
flexibilit= y.

How about another use case. Lets say the Li= nux OS loader (Grub) wants
to
have an accessibl= e UI so it decides to sore sound files on the EFI
System
Partition and use our new fancy UEFI Audio Protocol to add audio = to the
OS
loader GUI. So that version of Grub n= eeds to work on 1,000 of different
PCs
and a wi= de range of UEFI Audio driver implementations. It is a much
e= asier
world if Wave PCM 16 bit just works every place. You co= uld add a lot of
complexity and try to encode the audio on th= e fly, maybe even in Linux
proper but that falls down if you = are booting from read only media like
a
DVD or = backup tape (yes people still do that in server land).

The other problem with flexibility is you just made the test matri= x
very
large for every driver that needs to get= implemented. For something as
complex as Intel HDA how you h= ook up the hardware and what CODECs you
use
may= impact the quality of the playback for a given board. Your EFI is
likely
going to pick a single encoding at that will ge= t tested all the time if
your
system has audio,= but all 50 other things you support not so much. So
that
will required testing, and some one with audiophile ears (or an = AI
program)
to test all the combinations. I=E2= =80=99m not kidding I get BZs on the quality
of
the boot bong on our systems.


= typedef struct EFI_SIMPLE_AUDIO_PROTOCOL {
 EFI_SIMPLE_A= UDIO_PROTOCOL_RESET Reset;
 EFI_SIMPLE_AUDIO_PROTOCOL_ST= ART Start;
 EFI_SIMPLE_AUDIO_PROTOCOL_STOP Stop;
} EFI_SIMPLE_AUDIO_PROTOCOL;
This is now = starting to look like something that belongs in boot-time
fir= mware.  :)

I think that got = a little too simple I=E2=80=99d go back and look at the example
I
posted to the thread but add an API to load the buffer, = and then play
the
buffer (that way we can an AP= I in the future to twiddle knobs). That
API
als= o implements the async EFI interface. Trust me the 1st thing that is
going to happen when we add audio is some one is going to complain = in
xyz
state we should mute audio, or we should= honer audio volume and mute
settings from setup, or from val= ues set in the OS. Or some one is going
to
want= the volume keys on the keyboard to work in EFI.

Also if you need to pick apart the Wave PCM 16 byte file to feed it into=
the
audio hardware that probably means we shou= ld have a library that does
that
work, so other= Audio drivers can share that code. Also having a library
mak= es it easier to write a unit test. We need to be security conscious
as we
need to treat the Audo file as attacker controll= ed data.

Thanks,

= Andrew Fish

Michael












--
Signed,
Ethin D. Probst




<= /blockquote>




<= br class=3D""> --Apple-Mail=_75BB1943-58DC-428A-9963-468E3CF3C1F8--