diff mbox series

snd_pcm_status() does not update status->avail

Message ID d9c1f37e-5c8d-f289-270e-c6cda7a56ce3@axis.com (mailing list archive)
State New, archived
Headers show
Series snd_pcm_status() does not update status->avail | expand

Commit Message

Jonas Holmberg Oct. 9, 2020, 2:02 p.m. UTC
Hi,
I have a problem with status->avail not being updated when using 
multiple extplug plugins and softvol. I managed to get it to work with 
the following patch:


My question is if this is the correct solution?

BR
/Jonas

Comments

Jaroslav Kysela Oct. 9, 2020, 2:30 p.m. UTC | #1
Dne 09. 10. 20 v 16:02 Jonas Holmberg napsal(a):
> Hi,
> I have a problem with status->avail not being updated when using 
> multiple extplug plugins and softvol. I managed to get it to work with 
> the following patch:
> 
> diff --git a/src/pcm/pcm_plugin.c b/src/pcm/pcm_plugin.c
> index ea60eb98..89e819d6 100644
> --- a/src/pcm/pcm_plugin.c
> +++ b/src/pcm/pcm_plugin.c
> @@ -551,6 +551,8 @@ static int snd_pcm_plugin_status(snd_pcm_t *pcm, 
> snd_pcm_status_t * status)
>                  return err;
>          status->appl_ptr = *pcm->appl.ptr;
>          status->hw_ptr = *pcm->hw.ptr;
> +       status->avail = snd_pcm_mmap_capture_avail(pcm);
> +       status->delay = snd_pcm_mmap_capture_delay(pcm);
>          return 0;
>   }
> 
> My question is if this is the correct solution?

It seems that nobody is using those status fields. Usually,
snd_pcm_avail_update() is used by apps. Anyway, this should be fixed for both
directions. The avail should be probably synced to
snd_pcm_plugin_avail_update() output:


diff --git a/src/pcm/pcm_plugin.c b/src/pcm/pcm_plugin.c
index ea60eb98..5739cfc2 100644
--- a/src/pcm/pcm_plugin.c
+++ b/src/pcm/pcm_plugin.c
@@ -541,16 +541,20 @@ static snd_pcm_sframes_t
snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
 static int snd_pcm_plugin_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
 {
        snd_pcm_plugin_t *plugin = pcm->private_data;
-       snd_pcm_sframes_t err;
+       snd_pcm_sframes_t err, avail;

        /* sync with the latest hw and appl ptrs */
-       snd_pcm_plugin_avail_update(pcm);
+       avail = snd_pcm_plugin_avail_update(pcm);
+       if (avail < 0)
+               return avail;

        err = snd_pcm_status(plugin->gen.slave, status);
        if (err < 0)
                return err;
        status->appl_ptr = *pcm->appl.ptr;
        status->hw_ptr = *pcm->hw.ptr;
+       status->avail = avail;
+       status->delay = snd_pcm_mmap_delay(pcm);
        return 0;
 }

					Jaroslav

> 
> BR
> /Jonas
>
Jonas Holmberg Oct. 9, 2020, 3:10 p.m. UTC | #2
On 2020-10-09 16:30, Jaroslav Kysela wrote:

> It seems that nobody is using those status fields. Usually,
> snd_pcm_avail_update() is used by apps. Anyway, this should be fixed for 
> both
> directions. The avail should be probably synced to
> snd_pcm_plugin_avail_update() output:
> 
> 
> diff --git a/src/pcm/pcm_plugin.c b/src/pcm/pcm_plugin.c
> index ea60eb98..5739cfc2 100644
> --- a/src/pcm/pcm_plugin.c
> +++ b/src/pcm/pcm_plugin.c
> @@ -541,16 +541,20 @@ static snd_pcm_sframes_t
> snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
>   static int snd_pcm_plugin_status(snd_pcm_t *pcm, snd_pcm_status_t * 
> status)
>   {
>          snd_pcm_plugin_t *plugin = pcm->private_data;
> -       snd_pcm_sframes_t err;
> +       snd_pcm_sframes_t err, avail;
> 
>          /* sync with the latest hw and appl ptrs */
> -       snd_pcm_plugin_avail_update(pcm);
> +       avail = snd_pcm_plugin_avail_update(pcm);
> +       if (avail < 0)
> +               return avail;
> 
>          err = snd_pcm_status(plugin->gen.slave, status);
>          if (err < 0)
>                  return err;
>          status->appl_ptr = *pcm->appl.ptr;
>          status->hw_ptr = *pcm->hw.ptr;
> +       status->avail = avail;
> +       status->delay = snd_pcm_mmap_delay(pcm);
>          return 0;
>   }
> 
>                                          Jaroslav
> 

I have tested your patch and it solves my problem. Are you going to push it?

Thanks!
/Jonas
Jaroslav Kysela Oct. 9, 2020, 6:04 p.m. UTC | #3
Dne 09. 10. 20 v 17:10 Jonas Holmberg napsal(a):
> On 2020-10-09 16:30, Jaroslav Kysela wrote:
> 
>> It seems that nobody is using those status fields. Usually,
>> snd_pcm_avail_update() is used by apps. Anyway, this should be fixed for 
>> both
>> directions. The avail should be probably synced to
>> snd_pcm_plugin_avail_update() output:
>>
>>
>> diff --git a/src/pcm/pcm_plugin.c b/src/pcm/pcm_plugin.c
>> index ea60eb98..5739cfc2 100644
>> --- a/src/pcm/pcm_plugin.c
>> +++ b/src/pcm/pcm_plugin.c
>> @@ -541,16 +541,20 @@ static snd_pcm_sframes_t
>> snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
>>   static int snd_pcm_plugin_status(snd_pcm_t *pcm, snd_pcm_status_t * 
>> status)
>>   {
>>          snd_pcm_plugin_t *plugin = pcm->private_data;
>> -       snd_pcm_sframes_t err;
>> +       snd_pcm_sframes_t err, avail;
>>
>>          /* sync with the latest hw and appl ptrs */
>> -       snd_pcm_plugin_avail_update(pcm);
>> +       avail = snd_pcm_plugin_avail_update(pcm);
>> +       if (avail < 0)
>> +               return avail;
>>
>>          err = snd_pcm_status(plugin->gen.slave, status);
>>          if (err < 0)
>>                  return err;
>>          status->appl_ptr = *pcm->appl.ptr;
>>          status->hw_ptr = *pcm->hw.ptr;
>> +       status->avail = avail;
>> +       status->delay = snd_pcm_mmap_delay(pcm);
>>          return 0;
>>   }
>>
>>                                          Jaroslav
>>
> 
> I have tested your patch and it solves my problem. Are you going to push it?

Yes, applied to alsa-lib's repo:

https://lore.kernel.org/alsa-devel/d9c1f37e-5c8d-f289-270e-c6cda7a56ce3@axis.com/

Thank you for your report.

				Jaroslav

> 
> Thanks!
> /Jonas
>
diff mbox series

Patch

diff --git a/src/pcm/pcm_plugin.c b/src/pcm/pcm_plugin.c
index ea60eb98..89e819d6 100644
--- a/src/pcm/pcm_plugin.c
+++ b/src/pcm/pcm_plugin.c
@@ -551,6 +551,8 @@  static int snd_pcm_plugin_status(snd_pcm_t *pcm, 
snd_pcm_status_t * status)
                 return err;
         status->appl_ptr = *pcm->appl.ptr;
         status->hw_ptr = *pcm->hw.ptr;
+       status->avail = snd_pcm_mmap_capture_avail(pcm);
+       status->delay = snd_pcm_mmap_capture_delay(pcm);
         return 0;
  }