Hi,
On Sun, Apr 1, 2012 at 4:55 AM, Diego Biurrun <[email protected]> wrote:
> DECLARE_ALIGNED(8, const uint8_t, dither_8x8_128)[8][8] = {
> -{ 36, 68, 60, 92, 34, 66, 58, 90,},
> -{ 100, 4,124, 28, 98, 2,122, 26,},
> -{ 52, 84, 44, 76, 50, 82, 42, 74,},
> -{ 116, 20,108, 12,114, 18,106, 10,},
> -{ 32, 64, 56, 88, 38, 70, 62, 94,},
> -{ 96, 0,120, 24,102, 6,126, 30,},
> -{ 48, 80, 40, 72, 54, 86, 46, 78,},
> -{ 112, 16,104, 8,118, 22,110, 14,},
> + { 36, 68, 60, 92, 34, 66, 58, 90, },
> + { 100, 4, 124, 28, 98, 2, 122, 26, },
> + { 52, 84, 44, 76, 50, 82, 42, 74, },
> + { 116, 20, 108, 12, 114, 18, 106, 10, },
> + { 32, 64, 56, 88, 38, 70, 62, 94, },
> + { 96, 0, 120, 24, 102, 6, 126, 30, },
> + { 48, 80, 40, 72, 54, 86, 46, 78, },
> + { 112, 16, 104, 8, 118, 22, 110, 14, },
> };
Please right-align integers:
0, x
64, x
128, x
instead of
0, x
64, x
128, x
> - for (j = 0; j < filterSize; j++) {
> + for (j = 0; j < filterSize; j++)
> val += src[srcPos + j] * filter[filterSize * i + j];
> - }
I've seen you do this in a few places now, I really don't know if it's
a good idea to remove these brackets unconditionally... This isn't
K&R, this is third/first person bikesheddery. I want blue.
> - for (j = 0; j < filterSize; j++) {
> + for (j = 0; j < filterSize; j++)
> val += src[srcPos + j] * filter[filterSize * i + j];
> - }
Blue.
> - for (j=0; j<filterSize; j++) {
> - val += ((int)src[srcPos + j])*filter[filterSize*i + j];
> - }
> + for (j = 0; j < filterSize; j++)
> + val += ((int)src[srcPos + j]) * filter[filterSize * i + j];
Blue.
> - //filter += hFilterSize;
> - dst[i] = FFMIN(val>>7, (1<<15)-1); // the cubic equation does
> overflow ...
> - //dst[i] = val>>7;
> + // filter += hFilterSize;
> + dst[i] = FFMIN(val >> 7, (1 << 15) - 1); // the cubic equation does
> overflow ...
> + // dst[i] = val>>7;
Just remove these two comment lines (filter += ... and dst[i] = ...).
> - for (j=0; j<filterSize; j++) {
> - val += ((int)src[srcPos + j])*filter[filterSize*i + j];
> - }
> + for (j = 0; j < filterSize; j++)
> + val += ((int)src[srcPos + j]) * filter[filterSize * i + j];
Blue.
> - //filter += hFilterSize;
> - dst[i] = FFMIN(val>>3, (1<<19)-1); // the cubic equation does
> overflow ...
> - //dst[i] = val>>7;
> + // filter += hFilterSize;
> + dst[i] = FFMIN(val >> 3, (1 << 19) - 1); // the cubic equation does
> overflow ...
> + // dst[i] = val>>7;
Remove two comment lines.
> + const int vChrBufSize = c->vChrBufSize;
> + uint8_t *formatConvBuffer = c->formatConvBuffer;
> + const int chrSrcSliceY = srcSliceY >> c->chrSrcVSubSample;
> + const int chrSrcSliceH = -((-srcSliceH) >> c->chrSrcVSubSample);
?
> + yuv2packed2_fn yuv2packed2 = c->yuv2packed2;
> + yuv2packedX_fn yuv2packedX = c->yuv2packedX;
> int should_dither = is9_OR_10BPS(c->srcFormat) || is16BPS(c->srcFormat);
?
> + for (; dstY < dstH; dstY++) {
> + const int chrDstY = dstY >> c->chrDstVSubSample;
> + uint8_t *dest[4] = {
> dst[0] + dstStride[0] * dstY,
> dst[1] + dstStride[1] * chrDstY,
> dst[2] + dstStride[2] * chrDstY,
> - (CONFIG_SWSCALE_ALPHA && alpPixBuf) ? dst[3] + dstStride[3] *
> dstY : NULL,
> + (CONFIG_SWSCALE_ALPHA && alpPixBuf) ? dst[3] + dstStride[3] *
> dstY
> + : NULL,
Probably don't break this line.
> // Do we have enough lines in this slice to output the dstY line
> - enough_lines = lastLumSrcY2 < srcSliceY + srcSliceH && lastChrSrcY <
> -((-srcSliceY - srcSliceH)>>c->chrSrcVSubSample);
> + enough_lines = lastLumSrcY2 < srcSliceY + srcSliceH &&
> + lastChrSrcY < -((-srcSliceY - srcSliceH) >>
> + c->chrSrcVSubSample);
Don't break this line.
> int lumAlpha = vLumFilter[2 * dstY + 1];
> int chrAlpha = vChrFilter[2 * dstY + 1];
> - lumMmxFilter[2] =
> - lumMmxFilter[3] = vLumFilter[2 * dstY ] * 0x10001;
> - chrMmxFilter[2] =
> - chrMmxFilter[3] = vChrFilter[2 * chrDstY] * 0x10001;
> + lumMmxFilter[2] =
> + lumMmxFilter[3] = vLumFilter[2 * dstY] * 0x10001;
> + chrMmxFilter[2] =
> + chrMmxFilter[3] = vChrFilter[2 * chrDstY] * 0x10001;
This looks wrong.
> diff --git a/libswscale/utils.c b/libswscale/utils.c
[..]
This not reviewed. Let's do one file at a time.
Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel