[FFmpeg-devel] [PATCH] PPC64: Add versions of functions in libswscale/input.c optimized for POWER8 VSX SIMD.

[James Almer]

On 7/10/2016 8:36 AM, Michael Niedermayer wrote:
> On Sun, Jul 10, 2016 at 07:08:19AM -0400, Ronald S. Bultje wrote:
>> Hi,
>>
>> On Thu, Jul 7, 2016 at 8:51 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
>>
>>> Hi,
>>>
>>> On Thu, Jul 7, 2016 at 7:38 AM, Michael Niedermayer <
>>> michael at niedermayer.cc> wrote:
>>>
>>>> On Thu, Jul 07, 2016 at 07:14:43AM -0400, Ronald S. Bultje wrote:
>>>>> Hi,
>>>>>
>>>>> On Thu, Jul 7, 2016 at 7:07 AM, Michael Niedermayer
>>>> <michael at niedermayer.cc>
>>>>> wrote:
>>>>>
>>>>>> On Wed, Jul 06, 2016 at 07:28:27AM -0500, Dan Parrot wrote:
>>>>>>> On Wed, 2016-07-06 at 09:07 +0200, Hendrik Leppkes wrote:
>>>>>> [...]
>>>>>>
>>>>>>>
>>>>>>> One other thing: why didn't this come up when the earlier patch was
>>>>>>> submitted and applied?
>>>>>>
>>>>>> community patch review is not a reproduceable process, depending on
>>>>>> who has time and does the review, different things can be found and
>>>>>> pointed out, and people have also different oppinions.
>>>>>> Real consistency can possibly only be achived by having an active
>>>>>> maintainer that does all review ...
>>>>>>
>>>>>> To be more precisse the other patch was applied due to this comment
>>>>>> IIRC:
>>>>>>  "If this patch works (FATE passes on ppc64) and is faster than
>>>>>>   the plain c functions then it can be committed as is"
>>>>>
>>>>>
>>>>> How much faster was it?
>>>>
>>>> There where several benchmarks posted, one is here:
>>>> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2016-June/196022.html
>>>> it also contains some arguments why the speedup is less than on x86
>>>
>>>
>>> I don't think these numbers are very convincing...
>>>
>>> The arguments, on the other hand, are not facts, they are hunches, so they
>>> are essentially meaningless.
>>>
>>> I would suggest to revert the patch
>>>
>> [..]
>>
>> So, this hasn't been reverted yet, is there any particular reason why it
>> hasn't?
>>
>> Again, the speedup is practically meaningless, the code unreviewed, and it
>> will have to be rewritten by whoever finishes #5570. Can we please agree
>> reverting is the best option - and then revert?
> 
> i think if you want to "mentor" this you should just do what you need
> to do to mentor this ...

Ronald is asking for people's opinion, since reverting it without
consulting others first (especially the committer) is not nice.
It was partly my fault that it got in since i was the one that said "if
it works and is faster than c then it can be committed", even though it
had no real review to notice all the problems in the approach. And it
ultimately wasn't really faster in most functions.

This code barely made a difference, it's effectively unmaintained now,
and will get in the way of anyone trying to give ticket #5570 another
try using a different, more efficient approach. So I'm with Ronald in
that it should be reverted.