From: Jaroslav Kysela <email@example.com> To: Vinod Koul <firstname.lastname@example.org> Cc: email@example.com, firstname.lastname@example.org, email@example.com, Takashi Iwai <firstname.lastname@example.org>, email@example.com, firstname.lastname@example.org, 'Pierre-Louis Bossart' <email@example.com>, firstname.lastname@example.org, email@example.com, Gyeongtaek Lee <firstname.lastname@example.org>, email@example.com, firstname.lastname@example.org Subject: Re: [PATCH] ALSA: compress: allow pause and resume during draining Date: Mon, 12 Oct 2020 16:10:10 +0200 Message-ID: <email@example.com> (raw) In-Reply-To: <20201012135540.GK2968@vkoul-mobl> Dne 12. 10. 20 v 15:55 Vinod Koul napsal(a): > On 12-10-20, 15:29, Jaroslav Kysela wrote: >> Dne 12. 10. 20 v 14:24 Vinod Koul napsal(a): >>> On 12-10-20, 09:01, Takashi Iwai wrote: >>>> On Mon, 12 Oct 2020 07:25:25 +0200, >>> >>>>> So what if we add another state but keep it in kernel (hidden from >>>>> userspace)...? >>>> >>>> That's fine, then it's just a kernel's business, and it should be >>>> determined which one makes the code better. >>>> >>>> But, there are things to be considered, though: >>>> >>>> - SNDRV_PCM_STATE_* is defined as snd_pcm_state_t with __bitwise. >>>> This indicates that the type has to be defined in that way >>>> explicitly. >>>> >>>> - Having a value over SNDRV_PCM_STATE_LAST internally is hackish. >>>> >>>>> Right now tinycompress does not make use of PCM streams, kernel handles >>>>> these. I am not aware of any other implementation. >>>>> >>>>> So if the scope if within compress then it might work... >>>> >>>> Yes. But currently the API uses SND_PCM_* even for the compress >>>> stuff. Changing this value means to have influence on PCM, even if >>>> PCM stuff doesn't use it yet. (At least you'd need to increase >>>> SND_PCM_STATE_LAST, for example.) >>>> >>>> That said, if we want to change only for compress API by assuming that >>>> the impact must be negligible, the first step would be to move from >>>> SND_PCM_STATE_* to the own state, SND_COMPRESS_STATE_*. The values >>>> should be compatible, but this has to be changed at first. Then you >>>> can introduce a new value there. >>> >>> I think that sounds reasonable to me, we should not have used >>> SNDRV_PCM_STATE_* in the first place and long term fix for this should >>> be SNDRV_COMPRESS_STATE_ >>> >>> I will cook a patch for this >> >> Although the impact is not high, I do think that we should enable the new >> behaviour conditionally (when the user space asks for it) even if the state >> values are split. I think that the whole thread is about 'how to extend the >> current APIs'. The hidden way is really not so nice. >> >> Unfortunately, there are no reserved fields in the snd_compr_params structure >> for this, but I see the similarity with the 'no_wake_mode' field which >> controls the driver behaviour. > > I was not really thinking of exporting the states to userspace. > Tinycompress does not use it, I do not see any uses of it to enable > userspace with it.. Do you think it should be exposed? If so why..? I don't think that it's required to expose the state for the compressed API to add this new feature. I just talk about to activate the new feature conditionally. The question is how to extend the API now. > Worst case we add an ioctl to query the state.. the state transitions > are anyway result of control ops on the stream > > Btw what was the motivation for pcm to expose the stream states..? The driver may change the state when underrun / overrun or an I/O error occurs and there's also mmap write/read mode, so the traditional read/write with an error code handling does not work here. Also, the user space should know the state anyway, so it's better to have all parts synced. Jaroslav -- Jaroslav Kysela <firstname.lastname@example.org> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
next prev parent reply index Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20200929084051epcas2p35fb2228ed1bdfce6a7ddf5b37c944823@epcas2p3.samsung.com> 2020-09-29 8:40 ` Gyeongtaek Lee 2020-09-29 8:54 ` Takashi Iwai 2020-09-29 9:17 ` Gyeongtaek Lee 2020-09-29 14:00 ` Pierre-Louis Bossart 2020-10-01 10:29 ` Vinod Koul 2020-10-01 15:28 ` Pierre-Louis Bossart 2020-10-06 6:21 ` Vinod Koul 2020-10-06 14:57 ` Pierre-Louis Bossart 2020-10-08 9:49 ` Gyeongtaek Lee 2020-10-09 15:13 ` Takashi Iwai 2020-10-09 17:43 ` Jaroslav Kysela 2020-10-10 9:08 ` Takashi Iwai 2020-10-12 5:25 ` Vinod Koul 2020-10-12 7:01 ` Takashi Iwai 2020-10-12 12:24 ` Vinod Koul 2020-10-12 13:29 ` Jaroslav Kysela 2020-10-12 13:55 ` Vinod Koul 2020-10-12 14:10 ` Jaroslav Kysela [this message] 2020-10-12 14:21 ` Takashi Iwai 2020-10-12 14:46 ` Jaroslav Kysela 2020-10-12 14:59 ` Takashi Iwai 2020-10-15 10:47 ` Gyeongtaek Lee 2020-10-20 5:23 ` Gyeongtaek Lee 2020-10-26 9:18 ` Gyeongtaek Lee 2020-10-26 17:01 ` Takashi Iwai 2020-10-27 1:56 ` Gyeongtaek Lee [not found] <CGME20200928105009epcas2p4a65d50d9d09800281395a490d1844ef3@epcas2p4.samsung.com> 2020-09-28 10:50 ` Gyeongtaek Lee 2020-09-28 11:13 ` Jaroslav Kysela 2020-09-28 14:34 ` Pierre-Louis Bossart 2020-09-29 1:51 ` Gyeongtaek Lee 2020-09-29 7:12 ` Takashi Iwai 2020-09-29 17:27 ` Jaroslav Kysela 2020-09-30 9:35 ` Takashi Iwai 2020-09-30 9:57 ` Jaroslav Kysela 2020-09-30 10:33 ` Takashi Iwai 2020-09-30 11:23 ` Jaroslav Kysela 2020-10-01 10:35 ` Vinod Koul
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Alsa-Devel Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \ firstname.lastname@example.org public-inbox-index alsa-devel Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git