From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f47.google.com (mail-ej1-f47.google.com [209.85.218.47]) by mx.groups.io with SMTP id smtpd.web10.142.1618595135111002670 for ; Fri, 16 Apr 2021 10:45:35 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20161025 header.b=iKmIJcFn; spf=pass (domain: gmail.com, ip: 209.85.218.47, mailfrom: harlydavidsen@gmail.com) Received: by mail-ej1-f47.google.com with SMTP id r9so43320553ejj.3 for ; Fri, 16 Apr 2021 10:45:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=e3hzlfNfuO+KnDvMswDt4/pcury4E9pfmbaTD66cq1I=; b=iKmIJcFnZRNVGXw2DvJ94TrDvQKznMJgYEAYQo1pRetnGUSPS2/Ic7CyAkvjIvbwXY 8TSIvee062GvMsDsQi70mXj+ggE456c48kpnBQ5Z9iUIs3+aq1Bwzg0gTZG6CmVveOiw 0uscRWsHfkZgdlIfRO13Og5q2VAAGacMCq6IBGrvG8SZTk67kAuLf23Fk9LzEOdIlteR VRmSwmSJustKHx79iAbdVBf20W9jOwIN0r4R8QO5DSLAoDSnfZAgGQ11XQ2zPH3eQ2cj CoD9k54I6JcZ3SQFz5C7S9xHPR52zod5g7pQfQJZ1ZPBn0Xq2vLSwxoX38zbUamzqVA9 8Chg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=e3hzlfNfuO+KnDvMswDt4/pcury4E9pfmbaTD66cq1I=; b=LZzVV2YDSq89ejK7hPkLwBCP3nIETy3iINHVKqP8ZflbJJOCmQ1+cqotkqiuKM6AR+ TnuneOJV1o25flV9/shZAtMnGCiPrh9drCirvcjkCiL+zzLTO83n4l6OCE2OWhlWMQDg KxpTebTc5+cBEfUpVAI3mo5IOWmBwpWHJXW8QxcrtU6xxfC+DRiCXJk+7YqxBO5Zay3i nO7i2q8SZU+XVDzlYhov3VAtQEaHFIXZIMAcNgF036vH16K/PVeQI+7L1NRIju443XA4 W2z8wge31pRr8bxAG8ElS9CDNzZkDvzxzryS6REKlwBwsflH2y8ntckGoUSM0c5wvy7y UoxA== X-Gm-Message-State: AOAM532xd5YyM00PDGVKIXHV7AYwc4m5cyyHhY2AG3Cxd0rewb1zsPht WU5VN73Ui6Z5cbjxiTDxhstWkfr3Dugwi3KaD5U= X-Google-Smtp-Source: ABdhPJzK7LeJefLyb77D1Vd67yFklFFjUWg4O6R8wr7jP9B/RQs8UUtS39WAe3lz08tGZ9cpwKWDTa7PXKkbYdT/Ago= X-Received: by 2002:a17:906:f155:: with SMTP id gw21mr9310454ejb.170.1618595133553; Fri, 16 Apr 2021 10:45:33 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:ab4:a64c:0:0:0:0:0 with HTTP; Fri, 16 Apr 2021 10:45:32 -0700 (PDT) In-Reply-To: <10E3436C-D743-4B2F-8E4B-7AD93B82FC92@apple.com> 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> From: "Ethin Probst" Date: Fri, 16 Apr 2021 12:45:32 -0500 Message-ID: Subject: Re: [edk2-devel] VirtIO Sound Driver (GSoC 2021) To: Andrew Fish Cc: edk2-devel-groups-io , Leif Lindholm , Michael Brown , Mike Kinney , Laszlo Ersek , "Desimone, Nathaniel L" , Rafael Rodrigues Machado , Gerd Hoffmann Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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: 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? On 4/16/21, Andrew Fish wrote: > > >> On Apr 16, 2021, at 4:34 AM, Leif Lindholm 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 should be pretty straightforward). >> > > Leif, > > I=E2=80=99m think if we have an API to load the buffer and a 2nd API to = play the > buffer an optional 3rd API could configure the streams. > >> 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 GRUB >> would more likely be a 44.1 kHz mp3/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 from enabling the >> output. I would much rather see USB *and* HDA support able to play pcm >> streams before worrying about decoding. >> > > I agree it might turn out it is easier to have the text to speech code j= ust > encode a PCM directly. > > Thanks, > > Andrew Fish > >> / >> Leif >> >> On Fri, Apr 16, 2021 at 00:33:06 -0500, Ethin Probst wrote: >>> Thanks for that explanation (I missed Mike'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 stream function. Now that I fully understand the >>> ramifications of 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've >>> 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. >>> >>> On 4/16/21, Andrew Fish via groups.io wr= ote: >>>> >>>> >>>>> On Apr 15, 2021, at 5:59 PM, Michael Brown 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 poi= nt >>>>>> 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 requiremen= t >>>>> to >>>>> be able to play audio files in arbitrary formats. This requirement >>>>> does >>>>> not exist. >>>>> >>>>> With a protocol-mandated fixed baseline set of audio parameters >>>>> (sample >>>>> rate etc), what would happen in practice is that the audio files wou= ld >>>>> be >>>>> encoded in that format at *build* time, using tools entirely externa= l >>>>> to >>>>> UEFI. The application code is then trivially simple: it just does >>>>> "load >>>>> blob, pass blob to audio protocol". >>>>> >>>> >>>> >>>> Ethin, >>>> >>>> Given the goal is an industry standard we value interoperability more >>>> that >>>> flexibility. >>>> >>>> How about another use case. Lets say the Linux OS loader (Grub) wants >>>> 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 t= he >>>> OS >>>> loader GUI. So that version of Grub needs to work on 1,000 of differe= nt >>>> 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 lot = of >>>> complexity and try to encode the audio on the fly, maybe even in Linu= x >>>> proper but that falls down if you are booting from read only media li= ke >>>> a >>>> DVD or backup tape (yes people still do that in server land). >>>> >>>> The other problem with flexibility is you just made the test matrix >>>> very >>>> large for every driver that needs to get implemented. For something a= s >>>> complex as Intel HDA how you hook 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 get 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 th= e quality >>>> of >>>> the boot bong on our systems. >>>> >>>> >>>>>> 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-tim= e >>>>> firmware. :) >>>>> >>>> >>>> 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 API in the future to twiddle knobs). That >>>> API >>>> also 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 values set in the OS. Or some one is goi= ng >>>> 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 i= nto >>>> the >>>> audio hardware that probably means we should have a library that does >>>> that >>>> work, so other Audio drivers can share that code. Also having a libra= ry >>>> makes it easier to write a unit test. We need to be security consciou= s >>>> as we >>>> need to treat the Audo file as attacker controlled data. >>>> >>>> Thanks, >>>> >>>> Andrew Fish >>>> >>>>> Michael >>>>> >>>>> >>>>> >>>>> >>>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>> >>> >>> -- >>> Signed, >>> Ethin D. Probst >> >> >>=20 >> >> > > --=20 Signed, Ethin D. Probst