Message ID | 20171001161036.6133-1-michael@niedermayer.cc |
---|---|
State | New |
Headers | show |
On Sun, Oct 01, 2017 at 06:10:35PM +0200, Michael Niedermayer wrote: > 32% faster loop > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/dvbsubdec.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c > index b683109643..6a40f2376f 100644 > --- a/libavcodec/dvbsubdec.c > +++ b/libavcodec/dvbsubdec.c > @@ -653,8 +653,9 @@ static void compute_default_clut(AVSubtitleRect *rect, int w, int h) > uint8_t list_inv[256]; > int counttab[256] = {0}; > int count, i, x, y; > + ptrdiff_t ls = rect->linesize[0]; You use this variable in 3 places where the length doesn't matter; please use a longer name (maybe "lsize", "stride", or even "linesize"). > > -#define V(x,y) rect->data[0][(x) + (y)*rect->linesize[0]] > +#define V(x,y) rect->data[0][(x) + (y)*ls] > for (y = 0; y<h; y++) { > for (x = 0; x<w; x++) { > int v = V(x,y) + 1; > @@ -665,7 +666,7 @@ static void compute_default_clut(AVSubtitleRect *rect, int w, int h) > counttab[v-1] += !!((v!=vl) + (v!=vr) + (v!=vt) + (v!=vb)); > } > } > -#define L(x,y) list[ rect->data[0][(x) + (y)*rect->linesize[0]] ] > +#define L(x,y) list[ d[(x) + (y)*ls] ] since you modify this line, feel free to drop the extra spacing after and before the []. > > for (i = 0; i<256; i++) { > int scoretab[256] = {0}; > @@ -673,12 +674,13 @@ static void compute_default_clut(AVSubtitleRect *rect, int w, int h) > int bestv = 0; > for (y = 0; y<h; y++) { > for (x = 0; x<w; x++) { > - int v = rect->data[0][x + y*rect->linesize[0]]; > + uint8_t *d = &rect->data[0][x + y*ls]; > + int v = *d; > int l_m = list[v]; > - int l_l = x ? L(x-1, y) : 1; > - int l_r = x+1<w ? L(x+1, y) : 1; > - int l_t = y ? L(x, y-1) : 1; > - int l_b = y+1<h ? L(x, y+1) : 1; > + int l_l = x ? L(-1, 0) : 1; > + int l_r = x+1<w ? L(+1, 0) : 1; > + int l_t = y ? L(0, -1) : 1; > + int l_b = y+1<h ? L(0, +1) : 1; I'd be more comfortable with: int l_l = x ? L(-1, 0) : 1; int l_r = x+1<w ? L( 1, 0) : 1; int l_t = y ? L( 0,-1) : 1; int l_b = y+1<h ? L( 0, 1) : 1; I feel like at some point in the future compilers will complain about these "(+3)*x" Should be fine otherwise
On Sun, Oct 01, 2017 at 06:25:14PM +0200, Clément Bœsch wrote: > On Sun, Oct 01, 2017 at 06:10:35PM +0200, Michael Niedermayer wrote: > > 32% faster loop > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/dvbsubdec.c | 16 +++++++++------- > > 1 file changed, 9 insertions(+), 7 deletions(-) > > > > diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c > > index b683109643..6a40f2376f 100644 > > --- a/libavcodec/dvbsubdec.c > > +++ b/libavcodec/dvbsubdec.c > > @@ -653,8 +653,9 @@ static void compute_default_clut(AVSubtitleRect *rect, int w, int h) > > uint8_t list_inv[256]; > > int counttab[256] = {0}; > > int count, i, x, y; > > + ptrdiff_t ls = rect->linesize[0]; > > You use this variable in 3 places where the length doesn't matter; please > use a longer name (maybe "lsize", "stride", or even "linesize"). > > > > > -#define V(x,y) rect->data[0][(x) + (y)*rect->linesize[0]] > > +#define V(x,y) rect->data[0][(x) + (y)*ls] > > for (y = 0; y<h; y++) { > > for (x = 0; x<w; x++) { > > int v = V(x,y) + 1; > > @@ -665,7 +666,7 @@ static void compute_default_clut(AVSubtitleRect *rect, int w, int h) > > counttab[v-1] += !!((v!=vl) + (v!=vr) + (v!=vt) + (v!=vb)); > > } > > } > > -#define L(x,y) list[ rect->data[0][(x) + (y)*rect->linesize[0]] ] > > > +#define L(x,y) list[ d[(x) + (y)*ls] ] > > since you modify this line, feel free to drop the extra spacing > after and before the []. > > > > > for (i = 0; i<256; i++) { > > int scoretab[256] = {0}; > > @@ -673,12 +674,13 @@ static void compute_default_clut(AVSubtitleRect *rect, int w, int h) > > int bestv = 0; > > for (y = 0; y<h; y++) { > > for (x = 0; x<w; x++) { > > - int v = rect->data[0][x + y*rect->linesize[0]]; > > + uint8_t *d = &rect->data[0][x + y*ls]; > > + int v = *d; > > int l_m = list[v]; > > - int l_l = x ? L(x-1, y) : 1; > > - int l_r = x+1<w ? L(x+1, y) : 1; > > - int l_t = y ? L(x, y-1) : 1; > > - int l_b = y+1<h ? L(x, y+1) : 1; > > > + int l_l = x ? L(-1, 0) : 1; > > + int l_r = x+1<w ? L(+1, 0) : 1; > > + int l_t = y ? L(0, -1) : 1; > > + int l_b = y+1<h ? L(0, +1) : 1; > > I'd be more comfortable with: > > int l_l = x ? L(-1, 0) : 1; > int l_r = x+1<w ? L( 1, 0) : 1; > int l_t = y ? L( 0,-1) : 1; > int l_b = y+1<h ? L( 0, 1) : 1; > > I feel like at some point in the future compilers will complain about > these "(+3)*x" > > Should be fine otherwise will apply with the suggested changes thx [...]
diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c index b683109643..6a40f2376f 100644 --- a/libavcodec/dvbsubdec.c +++ b/libavcodec/dvbsubdec.c @@ -653,8 +653,9 @@ static void compute_default_clut(AVSubtitleRect *rect, int w, int h) uint8_t list_inv[256]; int counttab[256] = {0}; int count, i, x, y; + ptrdiff_t ls = rect->linesize[0]; -#define V(x,y) rect->data[0][(x) + (y)*rect->linesize[0]] +#define V(x,y) rect->data[0][(x) + (y)*ls] for (y = 0; y<h; y++) { for (x = 0; x<w; x++) { int v = V(x,y) + 1; @@ -665,7 +666,7 @@ static void compute_default_clut(AVSubtitleRect *rect, int w, int h) counttab[v-1] += !!((v!=vl) + (v!=vr) + (v!=vt) + (v!=vb)); } } -#define L(x,y) list[ rect->data[0][(x) + (y)*rect->linesize[0]] ] +#define L(x,y) list[ d[(x) + (y)*ls] ] for (i = 0; i<256; i++) { int scoretab[256] = {0}; @@ -673,12 +674,13 @@ static void compute_default_clut(AVSubtitleRect *rect, int w, int h) int bestv = 0; for (y = 0; y<h; y++) { for (x = 0; x<w; x++) { - int v = rect->data[0][x + y*rect->linesize[0]]; + uint8_t *d = &rect->data[0][x + y*ls]; + int v = *d; int l_m = list[v]; - int l_l = x ? L(x-1, y) : 1; - int l_r = x+1<w ? L(x+1, y) : 1; - int l_t = y ? L(x, y-1) : 1; - int l_b = y+1<h ? L(x, y+1) : 1; + int l_l = x ? L(-1, 0) : 1; + int l_r = x+1<w ? L(+1, 0) : 1; + int l_t = y ? L(0, -1) : 1; + int l_b = y+1<h ? L(0, +1) : 1; int score; if (l_m) continue;
32% faster loop Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/dvbsubdec.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)