[FFmpeg-devel] [PATCH] pthread_frame: allow per-field ThreadFrame owners.

[Wan-Teh Chang]

Hi Ronald,

Thank you for the quick reply.

On Tue, Jul 11, 2017 at 2:09 PM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> Hi Wan-Teh,
>
> On Tue, Jul 11, 2017 at 4:53 PM, Wan-Teh Chang <wtc at google.com> wrote:
>>
>> Hi Ronald,
>>
>> I have a question about this patch.
>>
>> On Mon, Apr 3, 2017 at 7:24 AM, Ronald S. Bultje <rsbultje at gmail.com>
>> wrote:
[...]
>> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> > index 3e8677d..0c68836 100644
>> > --- a/libavcodec/utils.c
>> > +++ b/libavcodec/utils.c
>> > @@ -3971,7 +3971,8 @@ int ff_thread_ref_frame(ThreadFrame *dst,
>> > ThreadFrame *src)
>> >  {
>> >      int ret;
>> >
>> > -    dst->owner = src->owner;
>> > +    dst->owner[0] = src->owner[0];
>> > +    dst->owner[1] = src->owner[1];
>> >
>> >      ret = av_frame_ref(dst->f, src->f);
>> >      if (ret < 0)
>> > @@ -3981,7 +3982,7 @@ int ff_thread_ref_frame(ThreadFrame *dst,
>> > ThreadFrame *src)
>> >
>> >      if (src->progress &&
>> >          !(dst->progress = av_buffer_ref(src->progress))) {
>> > -        ff_thread_release_buffer(dst->owner, dst);
>> > +        ff_thread_release_buffer(dst->owner[0], dst);
>> >          return AVERROR(ENOMEM);
>> >      }
>> >
>> [...]
>>
>> I don't understand why we pass dst->owner[0], not dst->owner[1], to
>> the ff_thread_release_buffer() call. Does this assume dst->owner[0] ==
>> dst->owner[1]?
>
> Neither is perfect, ideally (from an API programming PoV) you'd want to use
> both. But that's not possible...

Can ff_thread_release_buffer() use f->owner[0] and f->owner[1] and
ignore the avctx argument?

This should work for the ff_thread_release_buffer() call in
ff_thread_ref_frame(), but I don't know if this is correct in general.

> In practice, I don't think it matters much,
> since all it does is decide which release_buffer-queue it'll be appended to
> (if we do delayed free()'ing). Are there problems with this?

No, there aren't. With your explanation, I roughly understand the
ff_thread_release_buffer() code now. Thanks.

Note: I am cherry-picking tsan warning fixes to our copy of ffmpeg,
which is several months behind the tip. As due diligence, I review the
fixes before I merge them.

Wan-Teh Chang