[FFmpeg-devel] [PATCH 1/2] pthreads: Fix bug introduced with thread_safe_callbacks

[Alexander Strange]

On Feb 17, 2011, at 3:43 PM, Ronald S. Bultje wrote:

> Hi,
> 
> On Thu, Feb 17, 2011 at 4:10 AM, Alexander Strange
> <astrange at ithinksw.com> wrote:
>> For intra codecs, ff_thread_finish_setup() is called before decoding starts
>> automatically. However, get_buffer can only be used before it's called, so
>> adding this requirement broke frame threading for them. Fixed by moving the
>> call until after get_buffer is finished.
>> ---
>>  libavcodec/pthread.c |    8 ++++++--
>>  1 files changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavcodec/pthread.c b/libavcodec/pthread.c
>> index 0e033d3..096b371 100644
>> --- a/libavcodec/pthread.c
>> +++ b/libavcodec/pthread.c
>> @@ -292,7 +292,8 @@ static attribute_align_arg void *frame_worker_thread(void *arg)
>> 
>>         if (fctx->die) break;
>> 
>> -        if (!codec->update_thread_context) ff_thread_finish_setup(avctx);
>> +        if (!codec->update_thread_context && !avctx->thread_safe_callbacks)
>> +            ff_thread_finish_setup(avctx);
>> 
>>         pthread_mutex_lock(&p->mutex);
>>         avcodec_get_frame_defaults(&p->frame);
>> @@ -779,7 +780,7 @@ int ff_thread_get_buffer(AVCodecContext *avctx, AVFrame *f)
>>         return avctx->get_buffer(avctx, f);
>>     }
>> 
>> -    if (p->state != STATE_SETTING_UP) {
>> +    if (p->state != STATE_SETTING_UP && avctx->codec->update_thread_context) {
>>         av_log(avctx, AV_LOG_ERROR, "get_buffer() cannot be called after ff_thread_finish_setup()\n");
>>         return -1;
>>     }
>> @@ -810,6 +811,9 @@ int ff_thread_get_buffer(AVCodecContext *avctx, AVFrame *f)
>>         err = p->result;
>> 
>>         pthread_mutex_unlock(&p->progress_mutex);
>> +
>> +        if (!avctx->codec->update_thread_context)
>> +            ff_thread_finish_setup(avctx);
>>     }
>> 
>>     pthread_mutex_unlock(&p->parent->buffer_mutex);
>> --
>> 1.7.4.1
> 
> Does this mean in some situations ff_thread_finish_setup will be called twice?

No, but there was a deadlock bug in the first line...

Also made the error condition in the second chunk more complex.
(p->state != STATE_SETTING_UP && !avctx->thread_safe_callbacks) will never happen in current code, but it's the kind of bug that's easy to introduce with an unrelated change.

Reattached both patches here.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-pthreads-Fix-bug-introduced-with-thread_safe_callbac.patch
Type: application/octet-stream
Size: 1903 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110228/8dc96ef1/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-huffyuv-Add-multithreading-support.patch
Type: application/octet-stream
Size: 2561 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110228/8dc96ef1/attachment-0001.obj>
-------------- next part --------------