On Tue, 2011-12-13 at 16:01 +0100, Takashi Iwai wrote:
At Tue, 13 Dec 2011 19:54:27 +0530, Vinod Koul wrote:
On Tue, 2011-12-13 at 15:04 +0100, Takashi Iwai wrote:
At Tue, 13 Dec 2011 19:21:30 +0530, Nallasellan, Singaravelan wrote:
> + size_t buffer_size; > + size_t fragment_size; Can we define buffer_size and fragment_size as unsigned items?
size_t is unsigned.
But, it needs consideration whether size_t is the best choice, since size_t is basically a long, thus its size is different between 32bit and 64bit architectures. This may lead to mess in many cases when you think of overlapping.
Sorry for my mess on size_t. I think the assumption here is 32 bits. Should u32 be fine here?
This depends pretty much on the demand and the implementation. Unless a buffer over 4GB is needed, u32 should be OK for buffer_size, etc. (Or, in this case, unsigned int would be fine, too. It's not necessarily be limited to 32bit. u32 or such is used usually for the type that must be 32bit.)
I don't think someone would need buffer of 4GB :)
But, the questions remain for hw_pointer, app_pointer, bytes_* fields. Are these supposed to be 32bit or more? This question is more important than the buffer size. The buffer size would be rarely over 32bit. But, if hw_pointer or such represents the accumulated position like ALSA PCM does, you'll face the overlapping problem. 32bit limit can be reached easily in the real use case.
the pointer are offsets within the ring buffer.
OK, then it's no problem to assume 32bit is enough.
Great :)
there are cumulative counters which will get overlapped after the size_t/u32 value.
Which field specifically? The cumulative counter has been a problem.
There are two fields in snd_compr_runtime, bytes_written and bytes_copied. Both are counting cumulative bytes written to ring buffer and bytes copied by DSP. Assumption is that they will wrap once they reach max. That should not cause issue in core. Then the core will wrap around the timestamp value. My thought was user lib should always take care of kernel wrapping and provide sufficiently large tstamp value to application.
When this changes between 32bit and 64bit, think twice about the 32bit compat-layer on 64bit arch (and the corresponding user-space implementation)...
Is it a fair assumption that userspace compiled for 32 bit should work with 32 bit kernel and same for 64 bit, or does the ABI mandate it be consistent across 32 and 64 bits.
The 32bit user-space must work with 64bit kernel. It means, it'd be much easier if ABI definition is compatible between 32 and 64bit.
That is, using a long type in the ABI definition should be definitely avoided. We had long fields in ALSA PCM field definitions, and this caused lots of really nasty problems. So, I tell you -- don't do it again. Learn from the history :)
Of course, this is about the ABI definition. But, the internal structure (types, etc) usually follows the ABI requirement. So I stated the questions here.
Right. I think we should be pretty decent if we just change the kernel APIs to u32. Based on that all other non cumulative counter should be u32 and cumulative ones u64 :) That was we will not care what bit userland is, and otherthings will work as expected.
Thanks