[FFmpeg-devel] [PATCH] avcodec: only warn about hwaccel with frame threads

[Hendrik Leppkes]

On Sat, Jan 23, 2016 at 4:55 PM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> Hi,
>
> On Sat, Jan 23, 2016 at 8:51 AM, Ronald S. Bultje <rsbultje at gmail.com>
> wrote:
>
>> Hi,
>>
>> On Sat, Jan 23, 2016 at 8:45 AM, Hendrik Leppkes <h.leppkes at gmail.com>
>> wrote:
>>
>>> On Sat, Jan 23, 2016 at 2:38 PM, Ronald S. Bultje <rsbultje at gmail.com>
>>> wrote:
>>> > Hi,
>>> >
>>> > On Sat, Jan 23, 2016 at 8:28 AM, wm4 <nfxjfg at googlemail.com> wrote:
>>> >
>>> >> On Sat, 23 Jan 2016 13:50:32 +0100
>>> >> Michael Niedermayer <michael at niedermayer.cc> wrote:
>>> >>
>>> >> > On Sat, Jan 23, 2016 at 11:42:54AM +0100, Hendrik Leppkes wrote:
>>> >> > > On Sat, Jan 23, 2016 at 10:12 AM, Andreas Cadhalpun
>>> >> > > <andreas.cadhalpun at googlemail.com> wrote:
>>> >> > > > VLC uses hwaccel with frame threads and it works fine, but
>>> returning
>>> >> > > > an error here made it fail.
>>> >> > > >
>>> >> > > > This regression was introduced in commit 31741ae.
>>> >> > > >
>>> >> > >
>>> >> > > I'm still opposed to this, and so is everyone else that commented
>>> on
>>> >> the issue.
>>> >> >
>>> >> > I have no oppinion on the patch itself but i think
>>> >> > if MT+HWaccel works in some case(s) and is faster then there should
>>> be
>>> >> > a way to enable that. We have optional "fast" decoding support too
>>> >> > that skips some checks and could crash or uses simpler dequantization
>>> >> > or our skip loopfilter support, ...
>>> >>
>>> >> Whether it's faster is an entirely different question.
>>> >>
>>> >> The problem is that we don't want to enable hwaccel with MT because
>>> >> it's much pain for little gain, but then if fallback to software
>>> >> happens, decoding will remain single-threaded.
>>> >>
>>> >> Possible solutions:
>>> >> - somehow allow changing the number of frame threading threads during
>>> >>   fallback
>>> >> - add a wrapper codec, which behaves like the old one, but reopens the
>>> >>   codec behind the scenes in order to change the number of threads on
>>> >>   fallback
>>> >> - making sure hwaccels actually work with MT (complicated => not worth
>>> >>   the trouble)
>>> >
>>> >
>>> > Why don't we ignore threading (set active_thread_type to NONE) if
>>> hwaccel
>>> > is enabled? Doesn't that fix everything in one single place?
>>> >
>>>
>>> Threading is already setup and running when the hwaccel gets
>>> activated, and its not designed to go in and out of threading mode at
>>> will.
>>
>>
>> Ok, so we can change how threading works. A fairly simple solution may be
>> to have a hwaccel flag that says whether it's threadsafe (whitelist style),
>> and if that's not set and hwaccel is enabled and active, have either
>> hwaccel or threading code "throttle" the number of iterating threads to 1
>> (i.e. you have N running threads, but N-1 idle and only 1 is ever accessed
>> while throttle-mode is active). This way, software fallback still works.
>>
>
> I'd want to poke this again before it gets lost. I'd like software fallback
> with threading to work, and hwaccel to be used by default (w/o threading,
> if needed). This proposal still seems the only one that accomplishes that.
> Can I get votes on this? I can implement if I get access to systems with
> hardware support (or examples for local reproduction).
>

Fully functional switch between hwaccel and threading would of course
also be nice.

Because of thread safety issues, the MT code was already "throttled"
to never execute more than one decoding thread at a time with hwaccel.
It does this by keeping the thread in "setup" mode until its done
executing, so any later threads do not get executed yet.

(This is why I don't see how hwaccel+MT could yield significant
performance gains)

The concept in itself isn't too bad, and it avoided the majority of
problems with thread safety and hwaccels.
However, here is the problem: The threads still execute async, which
means the user code has no way to sync to them and avoid calling
hardware APIs at the same time as the decode thread.

A start would be that the threads would be further synchronized so
that threads only run while avcodec_decode_video2 is executed, and no
longer run once that function returns, so that the user code can be
sure to not run into thread safety issues.
This solves the main problem of image corruption due to parallel use
of image surfaces, and the rare crashes.

Unfortunately that doesn't alleviate the other issues, like the
complexity needed in the decoders during frame threading, or the extra
resources needed (extra image surfaces for every thread).

What I would really *love* to see here would be the ability to
entirely and completely switch in and out of threading mode at will.
If hwaccel turns on, terminate the worker threads, sync their contexts
back to the main context, and proceed with single threaded decoding.
And vice-versa when the hwaccel goes away for some reason.
Is this full switch feasible? Its certainly not trivial and may have a
few roadblocks along the way. But maybe it could be done. Having a m:n
decode  API would certainly make it easier, however.

- Hendrik