Dne 09. 10. 20 v 17:13 Takashi Iwai napsal(a): > On Thu, 08 Oct 2020 11:49:24 +0200, > Gyeongtaek Lee wrote: >> >> On 10/06/20 11:57 PM, Pierre-Louis Bossart wrote: >>>> The SM in kernel might be bit more convoluted so was wondering if we can >>>> handle this in userland. The changelog for this patch says that for >>>> test case was sending whole file, surely that is not an optimal approach. >>> >>> It's rather common to have to deal with very small files, even with PCM, >>> e.g. for notifications. It's actually a classic test case that exposes >>> design issues in drivers, where e.g. the last part of the notification >>> is not played. >>> >>>> Should we allow folks to send whole file to kernel and then issue >>>> partial drain? >>> >>> I don't think there should be a conceptual limitation here. If the >>> userspace knows that the last part of the file is smaller than a >>> fragment it should be able to issue a drain (or partial drain if it's a >>> gapless solution). >>> >>> However now that I think of it, I am not sure what happens if the file >>> is smaller than a fragment. That may very well be a limitation in the >>> design. >>> >> Thanks for the comments. >> >> Actually, problem can be occurred with big file also. >> Application usually requests draining after sending last frame. >> If user clicks pause button after draining, pause will be failed >> and the file just be played until end. >> If application stop and start playback for this case, >> start of last frame will be heard again because stop sets state to SETUP, >> and write is needed to set the state to PREPARED for start. >> If bitrate of the file is low, time stamp will be reversed and be heard weird. >> I also hope this problem can be handled in userspace easily but I couldn't find a way for now. >> >> I think that this is the time that I should share fixed patch following the comments to help the discussion. >> Following opinions are added to the patch. >> 1. it's be much nicer to have a new state - Takashi > > Well, it wasn't me; I'm not against the new state *iff* it would end > up with cleaner code. Admittedly, the new state can be more > "consistent" regarding the state transition. If we allow the PAUSE > state during DRAINING, it'll lead to multiple states after resuming > the pause. > >> 2. We can add this state to asound.h so the user space can be updated. - Jaroslav >> 3. don't forget to increase the SNDRV_COMPRESS_VERSION - Jaroslav >> >> I'm bit wondering whether it is good to increase SNDRV_COMPRESS_VERSION >> with a change in asound.h not in compress_offload.h. >> Should I increase SNDRV_PCM_VERSION also? > > Yes, if we really add the PCM state, it's definitely needed. > >> And what happened if I request back-porting a patch which changes VERSION to stables? > > If we introduce the new change, it must be safe to the old kernels, > too. The problem is only about the compatibility of the user-space > program, not about the kernel. > > > HOWEVER: I'm still concerned by the addition of a new PCM state. > Jaroslav suggested two steps approach, (1) first add the state only in > the uapi header, then use (2) the new state actually. But, this > doesn't help much, simply because the step 1 won't catch real bugs. > > Even if we add a new state and change the SNDRV_PCM_STATE_LAST, I > guess most of code can be compiled fine. So, at the step 1, no one > notices it and bothered, either. But, at the step 2, you'll hit a > problem. > > Adding a new state is something like to add a new color to the traffic > signal. In some countries, the car turning right at a crossing > doesn't have to stop at a red signal. Suppose that we want to control > it, and change the international rule by introducing a new color (say > magenta) signal to stop the car turning right. That'll be a big > confusion because most drivers are trained for only red, green and > yellow signals. > > Similarly, if we add a new PCM state, every program code that deals > with the PCM state may be confused by the new state. It has to be > reviewed and corrected manually, because it's no syntax problem the > compiler may catch. If there is a handshake between both side, this problem is gone. We can just add another flag / ioctl / whatever to activate the new behaviour. Jaroslav -- Jaroslav Kysela <perex@xxxxxxxx> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
