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

[Ronald S. Bultje]

Hi Wan-Teh,

On Tue, Jul 11, 2017 at 6:30 PM, Wan-Teh Chang <wtc at google.com> wrote:

> 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.


I guess. I'm guessing that the prototype was modeled after the
get_buffer2() (and release_buffer(), before refcounting) callback prototype
in AVCodecContext. I wouldn't object to a patch to fix that.

Ronald