[FFmpeg-devel] [PATCH] frame_thread_encoder: make task indexing deterministic.

[Ronald S. Bultje]

Hi,

On Sat, Apr 1, 2017 at 5:44 PM, Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Sat, Apr 01, 2017 at 08:50:37AM -0400, Ronald S. Bultje wrote:
> > Hi,
> >
> > On Sat, Apr 1, 2017 at 6:15 AM, Michael Niedermayer
> <michael at niedermayer.cc>
> > wrote:
> > [.]
> > >
> > > about the patch itself, the changes done to indexing seem not to
> > > change anything, it makes it possible for the variables to overflow
> > > though.
> > >
> >
> > That's intentional, they're both unsigned and the one is guaranteed to be
> > larger than the other so the difference after subtract would still be
> > guaranteed to be <= n_threads. So although the indices overflow, the
> > subtraction will always be correct.
> >
> >
> > > IIUC the only change your patch does is to remove the outdata check
> > > from the quoted warning
> > >
> >
> > Yes, because if you change the delay to be fixed (and equal to
> n_threads),
> > you no longer need the check. If the difference between indices is
> smaller
> > than n_threads, you don't return a packet. If it's equal, you do return a
> > packet.
> >
>
> > its a while ago that i worked n this code but isnt this just "missing"
> > > a finished_task_mutex lock over the access ?
> >
> >
> > I don't know, it depends on what you're trying to accomplish. If you
> want a
> > fixed-frame delay, you don't need the access and so no mutex is needed. I
> > haven't thought much about what you need to get a variable-frame delay.
> To
> > me, a fixed-frame delay for intra-only codecs (which is what this API
> > currently accomplishes in practice) is exactly what you would expect,
> > variable-frame delay implies you're not actually using all threads. But
> > maybe I misunderstand.
>
> Lower delay is better, so is using fewer threads if theres no need for
> more
> it means less delay for any form of real time commuication
> less memory used, less L2 cache used and consequently better use of
> the CPU.
> If decoding with 1 thread is fast enough for the rate at which packets
> become available using 12 threads will result in more cpu cycles per
> frame, more memory used, more watts, a higher energy bill, ...


We already do automatic thread detection, and it's a user-settable option
also.

Can we please review the patch?

Ronald