Hi,
On Sun, Dec 18, 2011 at 1:52 PM, Nathan Maxson <[email protected]>wrote:
> [..]
> -#define BY ( (int)(0.114*219/255*(1<<RGB2YUV_SHIFT)+0.5))
> -#define BV (-(int)(0.081*224/255*(1<<RGB2YUV_SHIFT)+0.5))
> -#define BU ( (int)(0.500*224/255*(1<<RGB2YUV_SHIFT)+0.5))
> -#define GY ( (int)(0.587*219/255*(1<<RGB2YUV_SHIFT)+0.5))
> -#define GV (-(int)(0.419*224/255*(1<<RGB2YUV_SHIFT)+0.5))
> -#define GU (-(int)(0.331*224/255*(1<<RGB2YUV_SHIFT)+0.5))
> -#define RY ( (int)(0.299*219/255*(1<<RGB2YUV_SHIFT)+0.5))
> -#define RV ( (int)(0.500*224/255*(1<<RGB2YUV_SHIFT)+0.5))
> -#define RU (-(int)(0.169*224/255*(1<<RGB2YUV_SHIFT)+0.5))
> -
> -static void fillPlane(uint8_t* plane, int stride, int width, int height,
int y, uint8_t val)
> +#define BY ( (int) (0.114 * 219 / 255 * (1 << RGB2YUV_SHIFT) + 0.5))
> +#define BV ( -(int) (0.081 * 224 / 255 * (1 << RGB2YUV_SHIFT) + 0.5))
> +#define BU ( (int) (0.500 * 224 / 255 * (1 << RGB2YUV_SHIFT) + 0.5))
> +#define GY ( (int) (0.587 * 219 / 255 * (1 << RGB2YUV_SHIFT) + 0.5))
> +#define GV ( -(int) (0.419 * 224 / 255 * (1 << RGB2YUV_SHIFT) + 0.5))
> +#define GU ( -(int) (0.331 * 224 / 255 * (1 << RGB2YUV_SHIFT) + 0.5))
> +#define RY ( (int) (0.299 * 219 / 255 * (1 << RGB2YUV_SHIFT) + 0.5))
> +#define RV ( (int) (0.500 * 224 / 255 * (1 << RGB2YUV_SHIFT) + 0.5))
> +#define RU ( -(int) (0.169 * 224 / 255 * (1 << RGB2YUV_SHIFT) + 0.5))
Remove the space between "(" and "-" here, it's a negator of one variable
("x = -a;"), not a subtract between 2 ("x = a - b;").
> - const uint8_t *srcPtr= src[0];
> - uint8_t *dstPtr= dst[0];
> - if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat ==
PIX_FMT_BGR32_1) && !isRGBA32(dstFormat))
> + const uint8_t *srcPtr = src[0];
> + uint8_t *dstPtr = dst[0];
> + if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat ==
PIX_FMT_BGR32_1) &&
> + !isRGBA32(dstFormat))
The indenting here is off. The "!" should be right under the second "(".
> - if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat ==
PIX_FMT_BGR32_1) && !isRGBA32(srcFormat))
> + if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat ==
PIX_FMT_BGR32_1) &&
> + !isRGBA32(srcFormat))
Same.
> - if (dstStride[0]*srcBpp == srcStride[0]*dstBpp && srcStride[0] >
0 && !(srcStride[0] % srcBpp))
> - conv(srcPtr, dstPtr + dstStride[0]*srcSliceY,
srcSliceH*srcStride[0]);
> + if (dstStride[0] * srcBpp == srcStride[0] * dstBpp &&
srcStride[0] > 0 &&
> + !(srcStride[0] % srcBpp))
Same here (well there's no second "(", so it'd be the character after the
first "(").
> +static int packedCopyWrapper(SwsContext *c, const uint8_t *src[],
> + int srcStride[], int srcSliceY, int
srcSliceH,
> + uint8_t *dst[], int dstStride[])
[..]
> /* universal length finder */
> - while(length+c->srcW <= FFABS(dstStride[0])
> - && length+c->srcW <= FFABS(srcStride[0])) length+= c->srcW;
> - assert(length!=0);
> + while (length + c->srcW <= FFABS(dstStride[0]) &&
> + length + c->srcW <= FFABS(srcStride[0]))
> + length += c->srcW;
This is wrong, and you can see why: it looks like length += ... is part of
the if(), but really length += ... is the thing under the while, so it
needs only 4 spaces indenting beyond the "w" in while.
> +static int planarCopyWrapper(SwsContext *c, const uint8_t *src[],
> + int srcStride[], int srcSliceY, int
srcSliceH,
> + uint8_t *dst[], int dstStride[])
[..]
> #define COPY9_OR_10TO16(rfunc, wfunc) \
> for (i = 0; i < height; i++) { \
> for (j = 0; j < length; j++) { \
> int srcpx = rfunc(&srcPtr2[j]); \
> - wfunc(&dstPtr2[j], (srcpx<<(16-src_depth)) |
(srcpx>>(2*src_depth-16))); \
> + wfunc(&dstPtr2[j], (srcpx << (16-src_depth))
| (srcpx >> (2 * src_depth - 16))); \
Spaces around the '-'. in "16-src_depth".
> - if(srcFormat == PIX_FMT_YUYV422 && (dstFormat == PIX_FMT_YUV420P ||
dstFormat == PIX_FMT_YUVA420P))
> - c->swScale= yuyvToYuv420Wrapper;
> - if(srcFormat == PIX_FMT_UYVY422 && (dstFormat == PIX_FMT_YUV420P ||
dstFormat == PIX_FMT_YUVA420P))
> - c->swScale= uyvyToYuv420Wrapper;
> + if(srcFormat == PIX_FMT_YUYV422 &&
> + (dstFormat == PIX_FMT_YUV420P || dstFormat == PIX_FMT_YUVA420P))
> + c->swScale = yuyvToYuv420Wrapper;
> + if(srcFormat == PIX_FMT_UYVY422 &&
> + (dstFormat == PIX_FMT_YUV420P || dstFormat == PIX_FMT_YUVA420P))
> + c->swScale = uyvyToYuv420Wrapper;
> if(srcFormat == PIX_FMT_YUYV422 && dstFormat == PIX_FMT_YUV422P)
> - c->swScale= yuyvToYuv422Wrapper;
> + c->swScale = yuyvToYuv422Wrapper;
> if(srcFormat == PIX_FMT_UYVY422 && dstFormat == PIX_FMT_YUV422P)
> - c->swScale= uyvyToYuv422Wrapper;
> + c->swScale = uyvyToYuv422Wrapper;
if (, i.e. space between "if" and "(".
> /* simple copy */
> - if ( srcFormat == dstFormat
> - || (srcFormat == PIX_FMT_YUVA420P && dstFormat ==
PIX_FMT_YUV420P)
> - || (srcFormat == PIX_FMT_YUV420P && dstFormat ==
PIX_FMT_YUVA420P)
> - || (isPlanarYUV(srcFormat) && isGray(dstFormat))
> - || (isPlanarYUV(dstFormat) && isGray(srcFormat))
> - || (isGray(dstFormat) && isGray(srcFormat))
> - || (isPlanarYUV(srcFormat) && isPlanarYUV(dstFormat)
> - && c->chrDstHSubSample == c->chrSrcHSubSample
> - && c->chrDstVSubSample == c->chrSrcVSubSample
> - && dstFormat != PIX_FMT_NV12 && dstFormat != PIX_FMT_NV21
> - && srcFormat != PIX_FMT_NV12 && srcFormat != PIX_FMT_NV21))
> + if ( srcFormat == dstFormat ||
> + (srcFormat == PIX_FMT_YUVA420P && dstFormat == PIX_FMT_YUV420P)
||
> + (srcFormat == PIX_FMT_YUV420P && dstFormat == PIX_FMT_YUVA420P)
||
> + (isPlanarYUV(srcFormat) && isGray(dstFormat)) ||
> + (isPlanarYUV(dstFormat) && isGray(srcFormat)) ||
> + (isGray(dstFormat) && isGray(srcFormat)) ||
> + (isPlanarYUV(srcFormat) && isPlanarYUV(dstFormat) &&
> + c->chrDstHSubSample == c->chrSrcHSubSample &&
> + c->chrDstVSubSample == c->chrSrcVSubSample &&
> + dstFormat != PIX_FMT_NV12 && dstFormat != PIX_FMT_NV21 &&
> + srcFormat != PIX_FMT_NV12 && srcFormat != PIX_FMT_NV21))
These last 4 lines have 2 too many spaces indenting, I think.
> +int attribute_align_arg sws_scale(struct SwsContext *c,
> + const uint8_t * const srcSlice[],
> + const int srcStride[], int srcSliceY,
> + int srcSliceH, uint8_t *const dst[],
> + const int dstStride[])
> {
> int i;
> - const uint8_t* src2[4]= {srcSlice[0], srcSlice[1], srcSlice[2],
srcSlice[3]};
> - uint8_t* dst2[4]= {dst[0], dst[1], dst[2], dst[3]};
> + const uint8_t *src2[4]= { srcSlice[0], srcSlice[1], srcSlice[2],
srcSlice[3] };
> + uint8_t *dst2[4]= { dst[0], dst[1], dst[2], dst[3] };
Space before '='.
> if (usePal(c->srcFormat)) {
> - for (i=0; i<256; i++) {
> - int p, r, g, b,y,u,v;
> + for (i = 0; i < 256; i++) {
> + int p, r, g, b, y, u, v;
> if(c->srcFormat == PIX_FMT_PAL8) {
> - p=((const uint32_t*)(srcSlice[1]))[i];
> - r= (p>>16)&0xFF;
> - g= (p>> 8)&0xFF;
> - b= p &0xFF;
> + p = ((const uint32_t *)(srcSlice[1]))[i];
> + r = (p >> 16) & 0xFF;
> + g = (p >> 8) & 0xFF;
> + b = p & 0xFF;
> } else if(c->srcFormat == PIX_FMT_RGB8) {
Space between "if" and "(", same below.
> - int srcStride2[4]= {srcStride[0], srcStride[1], srcStride[2],
srcStride[3]};
> - int dstStride2[4]= {dstStride[0], dstStride[1], dstStride[2],
dstStride[3]};
> + int srcStride2[4] = { srcStride[0], srcStride[1], srcStride[2],
> + srcStride[3] };
> + int dstStride2[4] = { dstStride[0], dstStride[1], dstStride[2],
> + dstStride[3] };
Please indent the [3] lines with 2 more spaces so it vertially aligns with
the [0] lines.
> - int srcStride2[4]= {-srcStride[0], -srcStride[1], -srcStride[2],
-srcStride[3]};
> - int dstStride2[4]= {-dstStride[0], -dstStride[1], -dstStride[2],
-dstStride[3]};
> + int srcStride2[4] = { -srcStride[0], -srcStride[1],
-srcStride[2],
> + -srcStride[3] };
> + int dstStride2[4] = { -dstStride[0], -dstStride[1],
-dstStride[2],
> + -dstStride[3] };
Same.
Great work overall, I think after this we're done.
Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
