[FFmpeg-devel] [PATCH] Fix a bug in ff_thread_report_progress in updating progress[field].

[Ronald S. Bultje]

Hi,

On Mon, Feb 29, 2016 at 5:41 PM, Wan-Teh Chang <wtc-at-google.com at ffmpeg.org
> wrote:

> This bug was found by Dmitry Vyukov. If two threads may call
> ff_thread_report_progress at the same time, progress[field] may
> decrease. For example, suppose progress[field] is 10 and two threads
> call ff_thread_report_progress to update progress[field] to 11 and
> 12, respectively. If the second thread acquires progress_mutex first
> and updates progress[field] to 12, then the first thread will update
> progress[field] to 11, causing progress[field] to decrease.
> ---
>  libavcodec/pthread_frame.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> index b77dd1e..a43e8fe 100644
> --- a/libavcodec/pthread_frame.c
> +++ b/libavcodec/pthread_frame.c
> @@ -482,8 +482,10 @@ void ff_thread_report_progress(ThreadFrame *f, int n,
> int field)
>          av_log(f->owner, AV_LOG_DEBUG, "%p finished %d field %d\n",
> progress, n, field);
>
>      pthread_mutex_lock(&p->progress_mutex);
> -    progress[field] = n;
> -    pthread_cond_broadcast(&p->progress_cond);
> +    if (progress[field] < n) {
> +        progress[field] = n;
> +        pthread_cond_broadcast(&p->progress_cond);
> +    }
>      pthread_mutex_unlock(&p->progress_mutex);
>  }


Do you have a sample+commandline to reproduce? The thing is, in all cases
where we use this, only one thread writes to a specific progress[n]. Two
threads may write to progress[], one per field, but one will write to
progress[0] and the other to progress[1]. If this happens, I don't mind the
patch, but I'd like to know how exactly this happens (also for posterity
documentation purposes).

Ronald