1. Patchew
  2. linux
  3. [PATCH v4 0/3] ALSA: compress_offload: Add 64-bit safe timestamp API
  4. View patch

[PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl

Joris Verhaegen posted 3 patches 6 months, 1 week ago
There is a newer version of this series
[PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl
Posted by Joris Verhaegen 6 months, 1 week ago 
The previous patch introduced the internal infrastructure for handling
64-bit timestamps. This patch exposes this capability to user-space.

Define the new ioctl command SNDRV_COMPRESS_TSTAMP64, which allows
applications to fetch the overflow-safe struct snd_compr_tstamp64.

The ioctl dispatch table is updated to handle the new command by
calling a new snd_compr_tstamp64 handler, while the legacy path is
renamed to snd_compr_tstamp32 for clarity.

This patch bumps the SNDRV_COMPRESS_VERSION to 0.4.0.

Reviewed-by: Miller Liang <millerliang@google.com>
Tested-by: Joris Verhaegen <verhaegen@google.com>
Signed-off-by: Joris Verhaegen <verhaegen@google.com>
---
 include/uapi/sound/compress_offload.h |  5 +++--
 sound/core/compress_offload.c         | 19 +++++++++++++------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
index abd0ea3f86ee..70b8921601f9 100644
--- a/include/uapi/sound/compress_offload.h
+++ b/include/uapi/sound/compress_offload.h
@@ -13,8 +13,7 @@
 #include <sound/asound.h>
 #include <sound/compress_params.h>
 
-
-#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 3, 0)
+#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 4, 0)
 /**
  * struct snd_compressed_buffer - compressed buffer
  * @fragment_size: size of buffer fragment in bytes
@@ -208,6 +207,7 @@ struct snd_compr_task_status {
  * Note: only codec params can be changed runtime and stream params cant be
  * SNDRV_COMPRESS_GET_PARAMS: Query codec params
  * SNDRV_COMPRESS_TSTAMP: get the current timestamp value
+ * SNDRV_COMPRESS_TSTAMP64: get the current timestamp value in 64 bit format
  * SNDRV_COMPRESS_AVAIL: get the current buffer avail value.
  * This also queries the tstamp properties
  * SNDRV_COMPRESS_PAUSE: Pause the running stream
@@ -230,6 +230,7 @@ struct snd_compr_task_status {
 						 struct snd_compr_metadata)
 #define SNDRV_COMPRESS_TSTAMP		_IOR('C', 0x20, struct snd_compr_tstamp)
 #define SNDRV_COMPRESS_AVAIL		_IOR('C', 0x21, struct snd_compr_avail)
+#define SNDRV_COMPRESS_TSTAMP64		_IOR('C', 0x22, struct snd_compr_tstamp64)
 #define SNDRV_COMPRESS_PAUSE		_IO('C', 0x30)
 #define SNDRV_COMPRESS_RESUME		_IO('C', 0x31)
 #define SNDRV_COMPRESS_START		_IO('C', 0x32)
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index d3164aa07158..445220fdb6a0 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -736,18 +736,23 @@ snd_compr_set_metadata(struct snd_compr_stream *stream, unsigned long arg)
 	return retval;
 }
 
-static inline int
-snd_compr_tstamp(struct snd_compr_stream *stream, unsigned long arg)
+static inline int snd_compr_tstamp(struct snd_compr_stream *stream,
+				   unsigned long arg, bool is_32bit)
 {
 	struct snd_compr_tstamp64 tstamp64 = { 0 };
 	struct snd_compr_tstamp tstamp32 = { 0 };
+	const void *copy_from = &tstamp64;
+	size_t copy_size = sizeof(tstamp64);
 	int ret;
 
 	ret = snd_compr_update_tstamp(stream, &tstamp64);
 	if (ret == 0) {
-		snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
-		ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
-				   &tstamp32, sizeof(tstamp32)) ?
+		if (is_32bit) {
+			snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
+			copy_from = &tstamp32;
+			copy_size = sizeof(tstamp32);
+		}
+		ret = copy_to_user((void __user *)arg, copy_from, copy_size) ?
 			      -EFAULT :
 			      0;
 	}
@@ -1327,7 +1332,9 @@ static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
 
 	switch (cmd) {
 	case SNDRV_COMPRESS_TSTAMP:
-		return snd_compr_tstamp(stream, arg);
+		return snd_compr_tstamp(stream, arg, true);
+	case SNDRV_COMPRESS_TSTAMP64:
+		return snd_compr_tstamp(stream, arg, false);
 	case SNDRV_COMPRESS_AVAIL:
 		return snd_compr_ioctl_avail(stream, arg);
 	case SNDRV_COMPRESS_PAUSE:
-- 
2.50.1.565.gc32cd1483b-goog
Re: [PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl
Posted by Vinod Koul 6 months, 1 week ago 
On 01-08-25, 10:27, Joris Verhaegen wrote:
> The previous patch introduced the internal infrastructure for handling
> 64-bit timestamps. This patch exposes this capability to user-space.
> 
> Define the new ioctl command SNDRV_COMPRESS_TSTAMP64, which allows
> applications to fetch the overflow-safe struct snd_compr_tstamp64.
> 
> The ioctl dispatch table is updated to handle the new command by
> calling a new snd_compr_tstamp64 handler, while the legacy path is
> renamed to snd_compr_tstamp32 for clarity.
> 
> This patch bumps the SNDRV_COMPRESS_VERSION to 0.4.0.
> 
> Reviewed-by: Miller Liang <millerliang@google.com>
> Tested-by: Joris Verhaegen <verhaegen@google.com>
> Signed-off-by: Joris Verhaegen <verhaegen@google.com>
> ---
>  include/uapi/sound/compress_offload.h |  5 +++--
>  sound/core/compress_offload.c         | 19 +++++++++++++------
>  2 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
> index abd0ea3f86ee..70b8921601f9 100644
> --- a/include/uapi/sound/compress_offload.h
> +++ b/include/uapi/sound/compress_offload.h
> @@ -13,8 +13,7 @@
>  #include <sound/asound.h>
>  #include <sound/compress_params.h>
>  
> -
> -#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 3, 0)
> +#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 4, 0)
>  /**
>   * struct snd_compressed_buffer - compressed buffer
>   * @fragment_size: size of buffer fragment in bytes
> @@ -208,6 +207,7 @@ struct snd_compr_task_status {
>   * Note: only codec params can be changed runtime and stream params cant be
>   * SNDRV_COMPRESS_GET_PARAMS: Query codec params
>   * SNDRV_COMPRESS_TSTAMP: get the current timestamp value
> + * SNDRV_COMPRESS_TSTAMP64: get the current timestamp value in 64 bit format
>   * SNDRV_COMPRESS_AVAIL: get the current buffer avail value.
>   * This also queries the tstamp properties
>   * SNDRV_COMPRESS_PAUSE: Pause the running stream
> @@ -230,6 +230,7 @@ struct snd_compr_task_status {
>  						 struct snd_compr_metadata)
>  #define SNDRV_COMPRESS_TSTAMP		_IOR('C', 0x20, struct snd_compr_tstamp)
>  #define SNDRV_COMPRESS_AVAIL		_IOR('C', 0x21, struct snd_compr_avail)
> +#define SNDRV_COMPRESS_TSTAMP64		_IOR('C', 0x22, struct snd_compr_tstamp64)
>  #define SNDRV_COMPRESS_PAUSE		_IO('C', 0x30)
>  #define SNDRV_COMPRESS_RESUME		_IO('C', 0x31)
>  #define SNDRV_COMPRESS_START		_IO('C', 0x32)
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index d3164aa07158..445220fdb6a0 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -736,18 +736,23 @@ snd_compr_set_metadata(struct snd_compr_stream *stream, unsigned long arg)
>  	return retval;
>  }
>  
> -static inline int
> -snd_compr_tstamp(struct snd_compr_stream *stream, unsigned long arg)
> +static inline int snd_compr_tstamp(struct snd_compr_stream *stream,
> +				   unsigned long arg, bool is_32bit)
>  {
>  	struct snd_compr_tstamp64 tstamp64 = { 0 };
>  	struct snd_compr_tstamp tstamp32 = { 0 };
> +	const void *copy_from = &tstamp64;
> +	size_t copy_size = sizeof(tstamp64);
>  	int ret;
>  
>  	ret = snd_compr_update_tstamp(stream, &tstamp64);
>  	if (ret == 0) {
> -		snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> -		ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
> -				   &tstamp32, sizeof(tstamp32)) ?
> +		if (is_32bit) {
> +			snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> +			copy_from = &tstamp32;
> +			copy_size = sizeof(tstamp32);
> +		}

Most of the applications and people would be 32bit right now and we
expect this to progressively change, but then this imposes a penalty as
default path is 64 bit, since we expect this ioctl to be called very
frequently, should we do this optimization for 64bit here?

> +		ret = copy_to_user((void __user *)arg, copy_from, copy_size) ?
>  			      -EFAULT :
>  			      0;
>  	}
> @@ -1327,7 +1332,9 @@ static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
>  
>  	switch (cmd) {
>  	case SNDRV_COMPRESS_TSTAMP:
> -		return snd_compr_tstamp(stream, arg);
> +		return snd_compr_tstamp(stream, arg, true);
> +	case SNDRV_COMPRESS_TSTAMP64:
> +		return snd_compr_tstamp(stream, arg, false);
>  	case SNDRV_COMPRESS_AVAIL:
>  		return snd_compr_ioctl_avail(stream, arg);
>  	case SNDRV_COMPRESS_PAUSE:
> -- 
> 2.50.1.565.gc32cd1483b-goog

-- 
~Vinod
Re: [PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl
Posted by Takashi Iwai 6 months, 1 week ago 
On Tue, 05 Aug 2025 06:47:59 +0200,
Vinod Koul wrote:
> 
> On 01-08-25, 10:27, Joris Verhaegen wrote:
> > The previous patch introduced the internal infrastructure for handling
> > 64-bit timestamps. This patch exposes this capability to user-space.
> > 
> > Define the new ioctl command SNDRV_COMPRESS_TSTAMP64, which allows
> > applications to fetch the overflow-safe struct snd_compr_tstamp64.
> > 
> > The ioctl dispatch table is updated to handle the new command by
> > calling a new snd_compr_tstamp64 handler, while the legacy path is
> > renamed to snd_compr_tstamp32 for clarity.
> > 
> > This patch bumps the SNDRV_COMPRESS_VERSION to 0.4.0.
> > 
> > Reviewed-by: Miller Liang <millerliang@google.com>
> > Tested-by: Joris Verhaegen <verhaegen@google.com>
> > Signed-off-by: Joris Verhaegen <verhaegen@google.com>
> > ---
> >  include/uapi/sound/compress_offload.h |  5 +++--
> >  sound/core/compress_offload.c         | 19 +++++++++++++------
> >  2 files changed, 16 insertions(+), 8 deletions(-)
> > 
> > diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
> > index abd0ea3f86ee..70b8921601f9 100644
> > --- a/include/uapi/sound/compress_offload.h
> > +++ b/include/uapi/sound/compress_offload.h
> > @@ -13,8 +13,7 @@
> >  #include <sound/asound.h>
> >  #include <sound/compress_params.h>
> >  
> > -
> > -#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 3, 0)
> > +#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 4, 0)
> >  /**
> >   * struct snd_compressed_buffer - compressed buffer
> >   * @fragment_size: size of buffer fragment in bytes
> > @@ -208,6 +207,7 @@ struct snd_compr_task_status {
> >   * Note: only codec params can be changed runtime and stream params cant be
> >   * SNDRV_COMPRESS_GET_PARAMS: Query codec params
> >   * SNDRV_COMPRESS_TSTAMP: get the current timestamp value
> > + * SNDRV_COMPRESS_TSTAMP64: get the current timestamp value in 64 bit format
> >   * SNDRV_COMPRESS_AVAIL: get the current buffer avail value.
> >   * This also queries the tstamp properties
> >   * SNDRV_COMPRESS_PAUSE: Pause the running stream
> > @@ -230,6 +230,7 @@ struct snd_compr_task_status {
> >  						 struct snd_compr_metadata)
> >  #define SNDRV_COMPRESS_TSTAMP		_IOR('C', 0x20, struct snd_compr_tstamp)
> >  #define SNDRV_COMPRESS_AVAIL		_IOR('C', 0x21, struct snd_compr_avail)
> > +#define SNDRV_COMPRESS_TSTAMP64		_IOR('C', 0x22, struct snd_compr_tstamp64)
> >  #define SNDRV_COMPRESS_PAUSE		_IO('C', 0x30)
> >  #define SNDRV_COMPRESS_RESUME		_IO('C', 0x31)
> >  #define SNDRV_COMPRESS_START		_IO('C', 0x32)
> > diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> > index d3164aa07158..445220fdb6a0 100644
> > --- a/sound/core/compress_offload.c
> > +++ b/sound/core/compress_offload.c
> > @@ -736,18 +736,23 @@ snd_compr_set_metadata(struct snd_compr_stream *stream, unsigned long arg)
> >  	return retval;
> >  }
> >  
> > -static inline int
> > -snd_compr_tstamp(struct snd_compr_stream *stream, unsigned long arg)
> > +static inline int snd_compr_tstamp(struct snd_compr_stream *stream,
> > +				   unsigned long arg, bool is_32bit)
> >  {
> >  	struct snd_compr_tstamp64 tstamp64 = { 0 };
> >  	struct snd_compr_tstamp tstamp32 = { 0 };
> > +	const void *copy_from = &tstamp64;
> > +	size_t copy_size = sizeof(tstamp64);
> >  	int ret;
> >  
> >  	ret = snd_compr_update_tstamp(stream, &tstamp64);
> >  	if (ret == 0) {
> > -		snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > -		ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
> > -				   &tstamp32, sizeof(tstamp32)) ?
> > +		if (is_32bit) {
> > +			snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > +			copy_from = &tstamp32;
> > +			copy_size = sizeof(tstamp32);
> > +		}
> 
> Most of the applications and people would be 32bit right now and we
> expect this to progressively change, but then this imposes a penalty as
> default path is 64 bit, since we expect this ioctl to be called very
> frequently, should we do this optimization for 64bit here?

Through a quick glance over the patch, I don't think you'll hit the
significant performance loss.  It's merely a few bytes of extra copies
before copy_to_user(), after all.  But, of course, it'd be more
convincing if anyone can test and give the actual numbers.


thanks,

Takashi
Re: [PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl
Posted by Charles Keepax 5 months, 3 weeks ago 
On Tue, Aug 05, 2025 at 07:59:42AM +0200, Takashi Iwai wrote:
> On Tue, 05 Aug 2025 06:47:59 +0200,
> Vinod Koul wrote:
> > On 01-08-25, 10:27, Joris Verhaegen wrote:
> > >  	ret = snd_compr_update_tstamp(stream, &tstamp64);
> > >  	if (ret == 0) {
> > > -		snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > > -		ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
> > > -				   &tstamp32, sizeof(tstamp32)) ?
> > > +		if (is_32bit) {
> > > +			snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > > +			copy_from = &tstamp32;
> > > +			copy_size = sizeof(tstamp32);
> > > +		}
> > 
> > Most of the applications and people would be 32bit right now and we
> > expect this to progressively change, but then this imposes a penalty as
> > default path is 64 bit, since we expect this ioctl to be called very
> > frequently, should we do this optimization for 64bit here?
> 
> Through a quick glance over the patch, I don't think you'll hit the
> significant performance loss.  It's merely a few bytes of extra copies
> before copy_to_user(), after all.  But, of course, it'd be more
> convincing if anyone can test and give the actual numbers.
> 

I am inclined to agree the impact should be very minimal and the
only alternative is a more complex implementation. I would vote
for leaving this as is.

Thanks,
Charles
Re: [PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl
Posted by Vinod Koul 5 months, 3 weeks ago 
On 18-08-25, 15:05, Charles Keepax wrote:
> On Tue, Aug 05, 2025 at 07:59:42AM +0200, Takashi Iwai wrote:
> > On Tue, 05 Aug 2025 06:47:59 +0200,
> > Vinod Koul wrote:
> > > On 01-08-25, 10:27, Joris Verhaegen wrote:
> > > >  	ret = snd_compr_update_tstamp(stream, &tstamp64);
> > > >  	if (ret == 0) {
> > > > -		snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > > > -		ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
> > > > -				   &tstamp32, sizeof(tstamp32)) ?
> > > > +		if (is_32bit) {
> > > > +			snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > > > +			copy_from = &tstamp32;
> > > > +			copy_size = sizeof(tstamp32);
> > > > +		}
> > > 
> > > Most of the applications and people would be 32bit right now and we
> > > expect this to progressively change, but then this imposes a penalty as
> > > default path is 64 bit, since we expect this ioctl to be called very
> > > frequently, should we do this optimization for 64bit here?
> > 
> > Through a quick glance over the patch, I don't think you'll hit the
> > significant performance loss.  It's merely a few bytes of extra copies
> > before copy_to_user(), after all.  But, of course, it'd be more
> > convincing if anyone can test and give the actual numbers.

That would be better

> I am inclined to agree the impact should be very minimal and the
> only alternative is a more complex implementation. I would vote
> for leaving this as is.

But yes, we can for now, go ahead. It is internal kernel flow can be
adapted anytime :-)

-- 
~Vinod
Re: [PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl
Posted by Takashi Iwai 5 months, 3 weeks ago 
On Tue, 19 Aug 2025 07:08:08 +0200,
Vinod Koul wrote:
> 
> On 18-08-25, 15:05, Charles Keepax wrote:
> > On Tue, Aug 05, 2025 at 07:59:42AM +0200, Takashi Iwai wrote:
> > > On Tue, 05 Aug 2025 06:47:59 +0200,
> > > Vinod Koul wrote:
> > > > On 01-08-25, 10:27, Joris Verhaegen wrote:
> > > > >  	ret = snd_compr_update_tstamp(stream, &tstamp64);
> > > > >  	if (ret == 0) {
> > > > > -		snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > > > > -		ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
> > > > > -				   &tstamp32, sizeof(tstamp32)) ?
> > > > > +		if (is_32bit) {
> > > > > +			snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > > > > +			copy_from = &tstamp32;
> > > > > +			copy_size = sizeof(tstamp32);
> > > > > +		}
> > > > 
> > > > Most of the applications and people would be 32bit right now and we
> > > > expect this to progressively change, but then this imposes a penalty as
> > > > default path is 64 bit, since we expect this ioctl to be called very
> > > > frequently, should we do this optimization for 64bit here?
> > > 
> > > Through a quick glance over the patch, I don't think you'll hit the
> > > significant performance loss.  It's merely a few bytes of extra copies
> > > before copy_to_user(), after all.  But, of course, it'd be more
> > > convincing if anyone can test and give the actual numbers.
> 
> That would be better
> 
> > I am inclined to agree the impact should be very minimal and the
> > only alternative is a more complex implementation. I would vote
> > for leaving this as is.
> 
> But yes, we can for now, go ahead. It is internal kernel flow can be
> adapted anytime :-)

OK, Joris, could you submit v5 with correction of what Vinod
suggested in reviews?


thanks,

Takashi
Re: [PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl
Posted by George Verhaegen 5 months, 1 week ago 
On Thu, 21 Aug 2025 at 08:18, Takashi Iwai <tiwai@suse.de> wrote:
> OK, Joris, could you submit v5 with correction of what Vinod
> suggested in reviews?

Submitted v5 with revert of the alignment change. I think that was the
only remaining open item.

Thanks,
  George (Joris)
Re: [PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl
Posted by George Verhaegen 6 months, 1 week ago 
On Tue, 05 Aug 2025 06:47:59 +0200,
Vinod Koul wrote:
>
> On 01-08-25, 10:27, Joris Verhaegen wrote:
> > The previous patch introduced the internal infrastructure for handling
> > 64-bit timestamps. This patch exposes this capability to user-space.
> >
> > Define the new ioctl command SNDRV_COMPRESS_TSTAMP64, which allows
> > applications to fetch the overflow-safe struct snd_compr_tstamp64.
> >
> > The ioctl dispatch table is updated to handle the new command by
> > calling a new snd_compr_tstamp64 handler, while the legacy path is
> > renamed to snd_compr_tstamp32 for clarity.
> >
> > This patch bumps the SNDRV_COMPRESS_VERSION to 0.4.0.
> >
> > Reviewed-by: Miller Liang <millerliang@google.com>
> > Tested-by: Joris Verhaegen <verhaegen@google.com>
> > Signed-off-by: Joris Verhaegen <verhaegen@google.com>
> > ---
> >  include/uapi/sound/compress_offload.h |  5 +++--
> >  sound/core/compress_offload.c         | 19 +++++++++++++------
> >  2 files changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
> > index abd0ea3f86ee..70b8921601f9 100644
> > --- a/include/uapi/sound/compress_offload.h
> > +++ b/include/uapi/sound/compress_offload.h
> > @@ -13,8 +13,7 @@
> >  #include <sound/asound.h>
> >  #include <sound/compress_params.h>
> >
> > -
> > -#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 3, 0)
> > +#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 4, 0)
> >  /**
> >   * struct snd_compressed_buffer - compressed buffer
> >   * @fragment_size: size of buffer fragment in bytes
> > @@ -208,6 +207,7 @@ struct snd_compr_task_status {
> >   * Note: only codec params can be changed runtime and stream params cant be
> >   * SNDRV_COMPRESS_GET_PARAMS: Query codec params
> >   * SNDRV_COMPRESS_TSTAMP: get the current timestamp value
> > + * SNDRV_COMPRESS_TSTAMP64: get the current timestamp value in 64 bit format
> >   * SNDRV_COMPRESS_AVAIL: get the current buffer avail value.
> >   * This also queries the tstamp properties
> >   * SNDRV_COMPRESS_PAUSE: Pause the running stream
> > @@ -230,6 +230,7 @@ struct snd_compr_task_status {
> >                                              struct snd_compr_metadata)
> >  #define SNDRV_COMPRESS_TSTAMP              _IOR('C', 0x20, struct snd_compr_tstamp)
> >  #define SNDRV_COMPRESS_AVAIL               _IOR('C', 0x21, struct snd_compr_avail)
> > +#define SNDRV_COMPRESS_TSTAMP64            _IOR('C', 0x22, struct snd_compr_tstamp64)
> >  #define SNDRV_COMPRESS_PAUSE               _IO('C', 0x30)
> >  #define SNDRV_COMPRESS_RESUME              _IO('C', 0x31)
> >  #define SNDRV_COMPRESS_START               _IO('C', 0x32)
> > diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> > index d3164aa07158..445220fdb6a0 100644
> > --- a/sound/core/compress_offload.c
> > +++ b/sound/core/compress_offload.c
> > @@ -736,18 +736,23 @@ snd_compr_set_metadata(struct snd_compr_stream *stream, unsigned long arg)
> >     return retval;
> >  }
> >
> > -static inline int
> > -snd_compr_tstamp(struct snd_compr_stream *stream, unsigned long arg)
> > +static inline int snd_compr_tstamp(struct snd_compr_stream *stream,
> > +                              unsigned long arg, bool is_32bit)
> >  {
> >     struct snd_compr_tstamp64 tstamp64 = { 0 };
> >     struct snd_compr_tstamp tstamp32 = { 0 };
> > +   const void *copy_from = &tstamp64;
> > +   size_t copy_size = sizeof(tstamp64);
> >     int ret;
> >
> >     ret = snd_compr_update_tstamp(stream, &tstamp64);
> >     if (ret == 0) {
> > -           snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > -           ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
> > -                              &tstamp32, sizeof(tstamp32)) ?
> > +           if (is_32bit) {
> > +                   snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > +                   copy_from = &tstamp32;
> > +                   copy_size = sizeof(tstamp32);
> > +           }
>
> Most of the applications and people would be 32bit right now and we
> expect this to progressively change, but then this imposes a penalty as
> default path is 64 bit, since we expect this ioctl to be called very
> frequently, should we do this optimization for 64bit here?

Valid point about optimizing the common path. But since the underlying
.pointer op was changed in the patch V3 to always return a 64-bit
snd_compr_tstamp64, there is no longer a "native" 32-bit path to fetch.

Any call to the legacy SNDRV_COMPRESS_TSTAMP ioctl must fetch the 64-bit
value and then truncate it down. The current patch reflects this by sharing the
common 64-bit fetch and then conditionally performing the truncation to 32-bit.
Splitting the function would result in two functions that both call
snd_compr_update_tstamp() to get the 64-bit data, so there would be code
duplication with no performance gain.

Thanks,
George

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.