[FFmpeg-devel] [PATCH] pthread: Fix ff_thread_get_formatissues when called outside frame decode.

[Reimar Döffinger]

On 9 March 2015 13:34:24 CET, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>On Mon, Mar 9, 2015 at 7:38 AM, Rainer Hochecker
><fernetmenta at online.de> wrote:
>> Reimar Döffinger <Reimar.Doeffinger <at> gmx.de> writes:
>>
>>>
>>> Any reason to believe this patch causes it?
>>> Because I can't see how it would.
>>> Maybe it's just a bug with DXVA and multithreading in the HEVC code?
>>> Can you provide some more information like a stacktrace, possibly
>using a
>> tool like DrMemory?
>>
>> I don't think the patch itself is the root cause of the issue, it
>just
>> triggers it. get_format is called 4 times, something seems to get
>corrupted
>> in opening our hw decoder.
>> Do you have an explanation why it works with thread_safe_callbacks
>set to 1?
>>
>> I am wondering if hevc misses the multithreading fix done for other
>codecs:
>> http://ffmpeg.org/pipermail/ffmpeg-cvslog/2013-March/062620.html
>>
>> What do you think?
>>
>
>I build the HEVC HWAccel support, and I consider frame-threading with
>hwaccel an abomination that should not be supported, so I didn't spend
>any time thinking about it.
>
>Honestly, there are several issues with MT+HWaccel in H264 as well, at
>least on Intel hardware, so I can only strongly urge everyone to not
>combine it. There is zero advantages, and only downsides!

Only if you completely ignore the practical issues that anyone trying to use it will have.
In many cases applications will not know whether hardware supports decoding the video until there get_format call. Worse, it can even change from supported to not supported.
Do you expect everyone to disable multithreading "just in case"? Buffer the last n bits of data to be able to restart decoding? Hack things in get_format to trick FFmpeg into single threaded?
All these have been tried and didn't work. (Kind of) supporting frame multithreading with hwaccel in contrast has worked and is convenient to our users.
Unless you have a simple solution to these real world problems I very much think supporting it is a requirement.
That is besides the fact that causing memory corruption it's not "not supporting" it but a bug.
If it's not supported it should return an appropriate error.