> James Almer <jamr...@gmail.com> schrieb am Sa, 11.3.2017:
>>On 3/11/2017 12:39 PM, Thomas Mundt wrote:
>>> James Almer <jamrial at gmail.com> schrieb am Fr, 10.3.2017:
>>>> On 3/8/2017 1:58 PM, Thomas Mundt wrote:
>>>> Hi,
>>>>
>>>> attached patch adds a complex (-1 2 6 2 -1) vertcal lowpassfilter to
>>>> vf_interlace. This will better retain detail and reduce blurring compared
>>>> to the existing (1 2 1) filter.
>>>>
>>>> Please comment.
>>>> diff --git a/libavfilter/interlace.h b/libavfilter/interlace.h
>>>> index da073ae..7ad457e 100644
>>>> --- a/libavfilter/interlace.h
>>>> +++ b/libavfilter/interlace.h
>>>> @@ -51,6 +51,8 @@ typedef struct InterlaceContext {
>>>> AVFrame *cur, *next; // the two frames from which the new one is
>>>> obtained
>>>> void (*lowpass_line)(uint8_t *dstp, ptrdiff_t linesize, const uint8_t
>>>> *srcp,
>>>> const uint8_t *srcp_above, const uint8_t
>>>> *srcp_below);
>>>> + void (*lowpass_line_complex)(uint8_t *dstp, ptrdiff_t linesize,
>>>> + const uint8_t *srcp, int mref, int pref);
>>>
>>> Why not keep a single prototype, passing mref and pref for both linear
>>> and complex? You can calculate srcp_above and srcp_below for linear like
>>> you're doing it for complex in both the c and asm versions.
>>>
>>
>> This would make sense. I just didn´t wanted to change more than necessary in
>> one patch and I´m not such an expert in simd programming.
>> This is my second attempt ever.
>> Also there is the same function in tinterlace filter that also uses this
>> asm. So I would also need to change the function in tinterlace.
>
> Yes, this would need separate patches. One to change the prototype,
> c and asm versions of the linear function for both filters, then
> another (One per interlace filter or one for both) adding the
> complex lowpass filter.
>
OK, c is fine, but I´m stumbling with asm since I´m a newbie there.
I changed in all files:
lowpass_line_c(uint8_t *dstp, ptrdiff_t linesize,
const uint8_t *srcp, const uint8_t *srcp_above,
const uint8_t *srcp_below)
to
lowpass_line_c(uint8_t *dstp, ptrdiff_t linesize,
const uint8_t *srcp, ptrdiff_t mref, ptrdiff_t pref)
But when I change asm to
cglobal lowpass_line, 5, 5, 7
add r0, r1
add r2, r1
neg r1
pcmpeqb m6, m6
.loop:
mova m0, [r2+r1+r3]
mova m1, [r2+r1+r3+mmsize]
pavgb m0, [r2+r1+r4]
pavgb m1, [r2+r1+r4+mmsize]
pxor m0, m6
pxor m1, m6
pxor m2, m6, [r2+r1]
pxor m3, m6, [r2+r1+mmsize]
pavgb m0, m2
pavgb m1, m3
pxor m0, m6
pxor m1, m6
mova [r0+r1], m0
mova [r0+r1+mmsize], m1
add r1, 2*mmsize
jl .loop
REP_RET
I get error: invalid effective address
It works when I use the same scheme as I use in complex asm, but this adds some
extra calculations which might not be OK.
>> I could do all this as a subsequent patch. Do you have a suggestion how to
>> deal with the option name of complex filter in tinterlace?
>> There it´s a flag with the names "low_pass_filter" and "vlpf", so I could
>> add "complex_low_pass_filter" and "vclpf"?
>
> With flags you could in theory enable both linear and complex at
> the same time, so you'd need to add code to abort in such cases
> or default to one of them.
> I'd say add a lowpass option similar to vf_interlace's, and leave
> the flags as is.
I´m not sure how to add an additional lowpass option without breaking the flag.
I might need a hint.
Simplest to me would be adding the complex flag and when both flags are set
using the complex function.
Otherwise the user won´t have set it.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
