On Tue, 18 Jun 2019 10:02:59 +0200,
Pierre-Louis Bossart wrote:
>
> On 6/18/19 9:44 AM, bgoswami@xxxxxxxxxxxxxx wrote:
> > From: Banajit Goswami <bgoswami@xxxxxxxxxxxxxx>
> >
> > Currently a 32 bit variable is used for storing number of bytes
> > copied to DSP, which can overflow when playback continues for a long
> > duration.
> > Change data type for this variable to __u64 to prevent overflow.
>
> This clearly looks like a bug, the number of bytes transferred is
> stored as u64 in the runtime. I have no memories of this being
> intentional.
>
> That said, it seems odd to me that you have an overflow on the number
> of bytes but not on the number of PCM frames. Shouldn't both the
> pcm_frames and pcm_io_frames fields also be changed to u64 while we
> are at it?
>
> And the second issue is that this may impact apps. This is a ABI
> change, isn't it?
Yes, it breaks ABI, and we can't take this as is.
If we need 64bit values, a new ioctl must be provided while keeping
the old one still working.
thanks,
Takashi
> >
> > Signed-off-by: Dhananjay Kumar <dhakumar@xxxxxxxxxxxxxx>
> > Signed-off-by: Banajit Goswami <bgoswami@xxxxxxxxxxxxxx>
> > ---
> > include/uapi/sound/compress_offload.h | 2 +-
> > sound/core/compress_offload.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
> > index 56d9567..db5edf3 100644
> > --- a/include/uapi/sound/compress_offload.h
> > +++ b/include/uapi/sound/compress_offload.h
> > @@ -67,7 +67,7 @@ struct snd_compr_params {
> > */
> > struct snd_compr_tstamp {
> > __u32 byte_offset;
> > - __u32 copied_total;
> > + __u64 copied_total;
> > __u32 pcm_frames;
> > __u32 pcm_io_frames;
> > __u32 sampling_rate;
> > diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> > index a1a6fd7..2d0a709 100644
> > --- a/sound/core/compress_offload.c
> > +++ b/sound/core/compress_offload.c
> > @@ -184,7 +184,7 @@ static int snd_compr_update_tstamp(struct snd_compr_stream *stream,
> > if (!stream->ops->pointer)
> > return -ENOTSUPP;
> > stream->ops->pointer(stream, tstamp);
> > - pr_debug("dsp consumed till %d total %d bytes\n",
> > + pr_debug("dsp consumed till %d total %llu bytes\n",
> > tstamp->byte_offset, tstamp->copied_total);
> > if (stream->direction == SND_COMPRESS_PLAYBACK)
> > stream->runtime->total_bytes_transferred = tstamp->copied_total;
> >
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@xxxxxxxxxxxxxxxx
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
