[vlc-devel] [PATCH 2/9] RFC: audio_output: fix buffers_lost count

[Rémi Denis-Courmont]

Le torstaina 1. syyskuuta 2016, 10.54.09 EEST Thomas Guillem a écrit :
> aout_FiltersPlay can return NULL without loosing a buffer (the block can
> still be in filters internals)
> 
> This is hackish and I'm not sure this will work in all cases .
> 
> Should we change pf_*_filter to differentiate an error from an absence of
> block ?
> ---
>  src/audio_output/aout_internal.h |  2 ++
>  src/audio_output/dec.c           | 20 ++++++++++++++++----
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/src/audio_output/aout_internal.h
> b/src/audio_output/aout_internal.h index c6edf8e..5592e22 100644
> --- a/src/audio_output/aout_internal.h
> +++ b/src/audio_output/aout_internal.h
> @@ -80,6 +80,8 @@ typedef struct
> 
>      aout_request_vout_t request_vout;
> 
> +    mtime_t last_dts;
> +    unsigned buffers_filtered_count;
>      atomic_uint buffers_lost;
>      atomic_uint buffers_played;
>      atomic_uchar restart;
> diff --git a/src/audio_output/dec.c b/src/audio_output/dec.c
> index c1cc1db..ca2105f 100644
> --- a/src/audio_output/dec.c
> +++ b/src/audio_output/dec.c
> @@ -107,6 +107,8 @@ error:
>      owner->sync.discontinuity = true;
>      aout_OutputUnlock (p_aout);
> 
> +    owner->last_dts = VLC_TS_INVALID;
> +    owner->buffers_filtered_count = 0;
>      atomic_init (&owner->buffers_lost, 0);
>      atomic_init (&owner->buffers_played, 0);
>      return 0;
> @@ -374,9 +376,22 @@ int aout_DecPlay (audio_output_t *aout, block_t *block,
> int input_rate) if (block->i_flags & BLOCK_FLAG_DISCONTINUITY)
>          owner->sync.discontinuity = true;
> 
> +    mtime_t last_dts = block->i_dts;
>      block = aout_FiltersPlay (owner->filters, block, input_rate);
>      if (block == NULL)
> -        goto lost;
> +    {
> +        if (owner->last_dts != VLC_TS_INVALID)
> +            owner->last_dts = last_dts;
> +        owner->buffers_filtered_count++;
> +        goto out;
> +    }
> +    else if (owner->last_dts != VLC_TS_INVALID && block->i_dts >
> owner->last_dts) +    {

AFAICT, that´s broken (too). For instance, speech codecs can have 
discontinuities during silence, i.e. not caused by packet loss.

> +        assert( owner->buffers_filtered_count > 0 );
> +        atomic_fetch_add(&owner->buffers_lost,
> owner->buffers_filtered_count); +    }
> +    owner->last_dts = VLC_TS_INVALID;
> +    owner->buffers_filtered_count = 0;
> 
>      /* Software volume */
>      aout_volume_Amplify (owner->volume, block);
> @@ -395,9 +410,6 @@ out:
>  drop:
>      owner->sync.discontinuity = true;
>      block_Release (block);
> -lost:
> -    atomic_fetch_add(&owner->buffers_lost, 1);
> -    goto out;
>  }
> 
>  void aout_DecGetResetStats(audio_output_t *aout, unsigned *restrict lost,


-- 
Rémi Denis-Courmont
http://www.remlab.net/