[FFmpeg-devel] [PATCH v1 3/3] avfilter/vf_yadif: remove duplicate code with macro-based function
Limin Wang
lance.lmwang at gmail.com
Thu Dec 26 03:15:27 EET 2019
On Wed, Dec 25, 2019 at 05:57:58PM +0100, Michael Niedermayer wrote:
> On Tue, Dec 24, 2019 at 09:13:39AM +0800, Limin Wang wrote:
> > On Thu, Nov 28, 2019 at 09:34:18AM +0800, lance.lmwang at gmail.com wrote:
> > > From: Limin Wang <lance.lmwang at gmail.com>
> > >
> > > Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> > > ---
> > > libavfilter/vf_yadif.c | 147 +++++++++++++++--------------------------
> > > 1 file changed, 54 insertions(+), 93 deletions(-)
> > >
> > > diff --git a/libavfilter/vf_yadif.c b/libavfilter/vf_yadif.c
> > > index f53bb77008..15d8c47b0b 100644
> > > --- a/libavfilter/vf_yadif.c
> > > +++ b/libavfilter/vf_yadif.c
> > > @@ -91,101 +91,62 @@ typedef struct ThreadData {
> > > next2++; \
> > > }
> > >
> > > -static void filter_line_c(void *dst1,
> > > - void *prev1, void *cur1, void *next1,
> > > - int w, int prefs, int mrefs, int parity, int mode)
> > > -{
> > > - uint8_t *dst = dst1;
> > > - uint8_t *prev = prev1;
> > > - uint8_t *cur = cur1;
> > > - uint8_t *next = next1;
> > > - int x;
> > > - uint8_t *prev2 = parity ? prev : cur ;
> > > - uint8_t *next2 = parity ? cur : next;
> > > -
> > > - /* The function is called with the pointers already pointing to data[3] and
> > > - * with 6 subtracted from the width. This allows the FILTER macro to be
> > > - * called so that it processes all the pixels normally. A constant value of
> > > - * true for is_not_edge lets the compiler ignore the if statement. */
> > > - FILTER(0, w, 1)
> > > +#define FILTER_LINE_FUNC(type, depth, bps) \
> > > +static void filter_line_c_##depth##bit(void *dst1, \
> > > + void *prev1, void *cur1, void *next1, \
> > > + int w, int prefs, int mrefs, int parity, int mode) \
> > > +{ \
> > > + type *dst = dst1; \
> > > + type *prev = prev1; \
> > > + type *cur = cur1; \
> > > + type *next = next1; \
> > > + int x; \
> > > + type *prev2 = parity ? prev : cur ; \
> > > + type *next2 = parity ? cur : next; \
> > > + mrefs /= bps; \
> > > + prefs /= bps; \
> > > + \
> > > + /* The function is called with the pointers already pointing to data[3] and */ \
> > > + /* with 6 subtracted from the width. This allows the FILTER macro to be */ \
> > > + /* called so that it processes all the pixels normally. A constant value of */ \
> > > + /* true for is_not_edge lets the compiler ignore the if statement. */ \
> > > + FILTER(0, w, 1) \
> > > }
> > > +FILTER_LINE_FUNC(uint16_t, 16, 2);
> > > +FILTER_LINE_FUNC(uint8_t, 8, 1);
> > >
> > > #define MAX_ALIGN 8
> > > -static void filter_edges(void *dst1, void *prev1, void *cur1, void *next1,
> > > - int w, int prefs, int mrefs, int parity, int mode)
> > > -{
> > > - uint8_t *dst = dst1;
> > > - uint8_t *prev = prev1;
> > > - uint8_t *cur = cur1;
> > > - uint8_t *next = next1;
> > > - int x;
> > > - uint8_t *prev2 = parity ? prev : cur ;
> > > - uint8_t *next2 = parity ? cur : next;
> > > -
> > > - const int edge = MAX_ALIGN - 1;
> > > -
> > > - /* Only edge pixels need to be processed here. A constant value of false
> > > - * for is_not_edge should let the compiler ignore the whole branch. */
> > > - FILTER(0, 3, 0)
> > > -
> > > - dst = (uint8_t*)dst1 + w - edge;
> > > - prev = (uint8_t*)prev1 + w - edge;
> > > - cur = (uint8_t*)cur1 + w - edge;
> > > - next = (uint8_t*)next1 + w - edge;
> > > - prev2 = (uint8_t*)(parity ? prev : cur);
> > > - next2 = (uint8_t*)(parity ? cur : next);
> > > -
> > > - FILTER(w - edge, w - 3, 1)
> > > - FILTER(w - 3, w, 0)
> > > -}
> > > -
> > > -
> > > -static void filter_line_c_16bit(void *dst1,
> > > - void *prev1, void *cur1, void *next1,
> > > - int w, int prefs, int mrefs, int parity,
> > > - int mode)
> > > -{
> > > - uint16_t *dst = dst1;
> > > - uint16_t *prev = prev1;
> > > - uint16_t *cur = cur1;
> > > - uint16_t *next = next1;
> > > - int x;
> > > - uint16_t *prev2 = parity ? prev : cur ;
> > > - uint16_t *next2 = parity ? cur : next;
> > > - mrefs /= 2;
> > > - prefs /= 2;
> > > -
> > > - FILTER(0, w, 1)
> > > -}
> > > -
> > > -static void filter_edges_16bit(void *dst1, void *prev1, void *cur1, void *next1,
> > > - int w, int prefs, int mrefs, int parity, int mode)
> > > -{
> > > - uint16_t *dst = dst1;
> > > - uint16_t *prev = prev1;
> > > - uint16_t *cur = cur1;
> > > - uint16_t *next = next1;
> > > - int x;
> > > - uint16_t *prev2 = parity ? prev : cur ;
> > > - uint16_t *next2 = parity ? cur : next;
> > > -
> > > - const int edge = MAX_ALIGN / 2 - 1;
> > > -
> > > - mrefs /= 2;
> > > - prefs /= 2;
> > > -
> > > - FILTER(0, 3, 0)
> > > -
> > > - dst = (uint16_t*)dst1 + w - edge;
> > > - prev = (uint16_t*)prev1 + w - edge;
> > > - cur = (uint16_t*)cur1 + w - edge;
> > > - next = (uint16_t*)next1 + w - edge;
> > > - prev2 = (uint16_t*)(parity ? prev : cur);
> > > - next2 = (uint16_t*)(parity ? cur : next);
> > > -
> > > - FILTER(w - edge, w - 3, 1)
> > > - FILTER(w - 3, w, 0)
> > > +#define EDGES_FUNC(type, depth, bps) \
> > > +static void filter_edges_##depth##bit(void *dst1, void *prev1, void *cur1, \
> > > + void *next1, int w, int prefs, int mrefs, int parity, int mode) \
> > > +{ \
> > > + type *dst = dst1; \
> > > + type *prev = prev1; \
> > > + type *cur = cur1; \
> > > + type *next = next1; \
> > > + int x; \
> > > + type *prev2 = parity ? prev : cur ; \
> > > + type *next2 = parity ? cur : next; \
> > > + \
> > > + const int edge = MAX_ALIGN / bps - 1; \
> > > + \
> > > + mrefs /= bps; \
> > > + prefs /= bps; \
> > > + \
> > > + FILTER(0, 3, 0) \
> > > + \
> > > + dst = (type*)dst1 + w - edge; \
> > > + prev = (type*)prev1 + w - edge; \
> > > + cur = (type*)cur1 + w - edge; \
> > > + next = (type*)next1 + w - edge; \
> > > + prev2 = (type*)(parity ? prev : cur); \
> > > + next2 = (type*)(parity ? cur : next); \
> > > + \
> > > + FILTER(w - edge, w - 3, 1) \
> > > + FILTER(w - 3, w, 0) \
> > > }
> > > +EDGES_FUNC(uint16_t, 16, 2);
> > > +EDGES_FUNC(uint8_t, 8, 1);
> > >
> > > static int filter_slice(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs)
> > > {
> > > @@ -313,8 +274,8 @@ static int config_output(AVFilterLink *outlink)
> > > s->filter_line = filter_line_c_16bit;
> > > s->filter_edges = filter_edges_16bit;
> > > } else {
> > > - s->filter_line = filter_line_c;
> > > - s->filter_edges = filter_edges;
> > > + s->filter_line = filter_line_c_8bit;
> > > + s->filter_edges = filter_edges_8bit;
> > > }
> >
> > ping
>
> the patch is fine, iam just a bit undecided if a template file would be
> cleaner, maybe others have a preferance ?
I think avfilter prefer to use macro-based function, only three templates are
used.
[lmwang at vpn libavfilter]$ ls *template*
colorspacedsp_template.c colorspacedsp_yuv2yuv_template.c median_template.c
>
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> What does censorship reveal? It reveals fear. -- Julian Assange
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list