-
Notifications
You must be signed in to change notification settings - Fork 25
tinycompress: add support for new SNDRV_COMPRESS_TSTAMP64 and _AVAIL64 #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
tinycompress: add support for new SNDRV_COMPRESS_TSTAMP64 and _AVAIL64 #29
Conversation
Kernel has upgraded compress headers and bumped version to 0.4.0, so update this header as well.
Add get_tstamp64 using the new ioctl SNDRV_COMPRESS_TSTAMP64.
} | ||
|
||
static int compress_hw_get_tstamp64(void *data, | ||
unsigned long long *samples, unsigned int *sampling_rate) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to use forced 64-bit type like __u64 instead long long. But it's probably for the discussion. The other exported functions do not force 32-bit types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I retained the C standard types for consistency with the existing API as you mentioned. Long long is guaranteed to be at least 64 bits wide so it won't cause any truncation issues.
Read the version when opened and cache it to reduce ioctl calls.
Refactor compress_hw_get_hpointer to use the new SNDRV_COMPRESS_AVAIL64 which benefits from overflow safety.
248f7fd
to
8f7cb7c
Compare
time = kavail64.tstamp.pcm_io_frames % kavail64.tstamp.sampling_rate; | ||
tstamp->tv_nsec = time * 1000000000 / kavail64.tstamp.sampling_rate; | ||
return 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont see compress_hw_get_tstamp() updated. compress_hw_write() uses AVAIL ioctl, that should be updated too...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment!
I dont see compress_hw_get_tstamp() updated
Correct, I added a parallel compress_hw_get_tstamp64
to get the 64 bit struct.
On the other hand, compress_hw_get_tstamp
has been unchanged, still returning the 32 bit structure via ioctl SNDRV_COMPRESS_TSTAMP
which will stay supported in the kernel forever.
compress_hw_write() uses AVAIL ioctl, that should be updated too...
There is no benefit in using AVAIL64 here since only the avail
field of the structure is being used (was already 64 bit). The only difference between AVAIL and AVAIL64 return value is the tstamp
field which is unused in compress_hw_write
These 2 ioctl are under review to be added to kernel. See https://lore.kernel.org/linux-sound/[email protected]/T/#t
This pull request should be merged after this kernel is available.