From patchwork Wed Aug 2 22:14:30 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carl Eugen Hoyos X-Patchwork-Id: 4605 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.46.211 with SMTP id u202csp76801vsu; Wed, 2 Aug 2017 15:15:02 -0700 (PDT) X-Received: by 10.28.10.131 with SMTP id 125mr4801095wmk.132.1501712102923; Wed, 02 Aug 2017 15:15:02 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1501712102; cv=none; d=google.com; s=arc-20160816; b=yG4MJD8M1TlVe/826Ut2Yyo2vXAbx+p0/TMlTmy8z5wOYT1qL+c8T4ITcSpmnOjoWc vxdE53TjzK3VmE7k9duxFstaJkPDeUXHPtW65UYKTv2QUg2U7dJAoMX++2dYyWItQP41 hHLW37+Yn8PmASJvqczozSs18BMtJFPeUuGOFKaMD+uSkOikOp9MIdJ9k6Pw0Un3nRUv qOK1sqW4yh7n7OHKaQfEwBRZh9mu4qgF1+47hpeVLIfSbeYVtavVl6NLmP6EYzoDpQns gmylU8yqajYwAUv6U70a63taFfxx3bxJOPcqEG6C0rv22RpXQEjNIiRRFyv59L9H6iog MUkA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:to :message-id:date:from:references:in-reply-to:mime-version :dkim-signature:delivered-to:arc-authentication-results; bh=EoBQs8OvDzSeQ+PE1tR53sCCMBo1x40bU1olWzFpMHg=; b=PDsQqHq92C4qnhO7rc+450jMyqVUXRA9y63bpwuNxiV18FtRkzYmy8+C0l+YjnJ3UQ DyYITcuvq3g1HVYqKTDin1A6J6Dvfi3fg1Ql61xTQpEDr/4erB8o0MjjNN+TWSiSnc1x WGwhpquwKAoP0KRFdVXKl61GTh35RbCjTjqabWgDdARNZjuUqLyQHKXCZOwz8Quc5kIi xFJysVFHd9wthzHckJUwOe8ZuVLIgfeo2YKQHmwM9mUeb/2TEjKDIEhdbgPceDMnwYqt jqxTCo0VeiOI6VWfE7psffvIbovDckkJ7/KzVBF6MtSvhTzQ6KalCD3yCV2HPoiBZurR sWlg== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.b=N556yL2r; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=gmail.com Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id i17si253883wrc.474.2017.08.02.15.15.02; Wed, 02 Aug 2017 15:15:02 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.b=N556yL2r; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id BD6146898D9; Thu, 3 Aug 2017 01:14:55 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-vk0-f47.google.com (mail-vk0-f47.google.com [209.85.213.47]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 031286805C5 for ; Thu, 3 Aug 2017 01:14:48 +0300 (EEST) Received: by mail-vk0-f47.google.com with SMTP id g189so23149910vke.5 for ; Wed, 02 Aug 2017 15:14:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=kMTADpKHY1sQJwD3LBxByL/wngWBaFBkEDtykyM/ejI=; b=N556yL2rasy1sl7o8hZm3rq2g5xdsOElm9Ank2vWMsL/9X51YUNtZclVKy+NozxEQz +0aCvnYSW9o+N78ktFvYlfa0PUj42Dtj04BeYb9plFUHOwcyOJfQy//NyuOJ2r5HZcrY IqmZP2ip2EFjfuEDV9KHhzWM0H6fB83W5tLATbjpvj2/Szmh0JYG3hLKwxuJtKgVk0Yj i5+PTT3wLY08oMtHCajO8M9S8LRet65RpY8PxJhGmno6MDQWNZ29Y0xmdw9pPF3k35YK qEssDO1G/rckg+1CY9tquJA7FvRT9TrDXVuUfb+WoC6Ow6J+Jn/thDNK9qLjl6iqFeKd yrLg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=kMTADpKHY1sQJwD3LBxByL/wngWBaFBkEDtykyM/ejI=; b=lFo4fi8PSsLfCAY4M3GaQcfTzaIZi67jMXBFn00wHxb3JmghtDsV413VPhK2I0rTaQ k+WBsiWvM3TVTdaZO6fMAx+lufTOxqy3I6iOcoS3qSIqnhTJFsXMMIyRKigvP14fbat9 uL23hM9hQtXGavqPaoQgloTNnAk213BqxgS6p7uRBK4BTKKM6zoBtebbZH00UQmuGUY8 a5GF3x2WzO9c2K02M33wf9SJmHI//oTlUdAJkIAvAsiXFIstGprNDH7V7nC+TP1TC7IG wNK/72RMn3LrTopcNfF7gTZTOtNssbhKR9dxAJMiAcjarbEFHEnVD88z0Pv9fo1dgNB7 14Cw== X-Gm-Message-State: AIVw111M9fCQR6ml4STiYMFlcqFPX1flkbVlkXPhaz8V2uffl1fgONSu +omLKO328kRSH8MfH9KZDZVxfwROHCwu X-Received: by 10.31.158.71 with SMTP id h68mr13713862vke.52.1501712091029; Wed, 02 Aug 2017 15:14:51 -0700 (PDT) MIME-Version: 1.0 Received: by 10.103.25.2 with HTTP; Wed, 2 Aug 2017 15:14:30 -0700 (PDT) In-Reply-To: <20170725192744.GK3740@nb4> References: <20170725192744.GK3740@nb4> From: Carl Eugen Hoyos Date: Thu, 3 Aug 2017 00:14:30 +0200 Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [RFC]libswscale: Sanitize isRGBinInt() and isBGRinInt() X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 2017-07-25 21:27 GMT+02:00 Michael Niedermayer : > On Mon, Jul 24, 2017 at 08:00:51PM +0200, Carl Eugen Hoyos wrote: >> Attached is a PoC for a code simplification that imo makes a logic in >> libswscale easier to read and also fixes the Big Endian fate failure. >> If this approach is preferred, I will try to fix the five affected asm >> functions. >> >> Please comment, Carl Eugen > > breaks: (blue faces) > ./ffplay -i matrixbench_mpeg2.mpg > -vf format=argb,scale=flags=16,format=rgb565 New PoC attached. This breaks fate, if the change is wanted, 4158fba3 has to be reverted. Please comment, Carl Eugen diff --git a/libswscale/rgb2rgb.c b/libswscale/rgb2rgb.c index f7f8188..a974d64 100644 --- a/libswscale/rgb2rgb.c +++ b/libswscale/rgb2rgb.c @@ -133,8 +133,10 @@ void (*yuyvtoyuv422)(uint8_t *ydst, uint8_t *udst, uint8_t *vdst, av_cold void ff_sws_rgb2rgb_init(void) { rgb2rgb_init_c(); +/* if (ARCH_X86) rgb2rgb_init_x86(); +*/ } void rgb32to24(const uint8_t *src, uint8_t *dst, int src_size) @@ -142,16 +144,9 @@ void rgb32to24(const uint8_t *src, uint8_t *dst, int src_size) int i, num_pixels = src_size >> 2; for (i = 0; i < num_pixels; i++) { -#if HAVE_BIGENDIAN - /* RGB32 (= A,B,G,R) -> BGR24 (= B,G,R) */ - dst[3 * i + 0] = src[4 * i + 1]; - dst[3 * i + 1] = src[4 * i + 2]; - dst[3 * i + 2] = src[4 * i + 3]; -#else - dst[3 * i + 0] = src[4 * i + 2]; + dst[3 * i + 0] = src[4 * i + 0]; dst[3 * i + 1] = src[4 * i + 1]; - dst[3 * i + 2] = src[4 * i + 0]; -#endif + dst[3 * i + 2] = src[4 * i + 2]; } } @@ -160,18 +155,10 @@ void rgb24to32(const uint8_t *src, uint8_t *dst, int src_size) int i; for (i = 0; 3 * i < src_size; i++) { -#if HAVE_BIGENDIAN - /* RGB24 (= R, G, B) -> BGR32 (= A, R, G, B) */ - dst[4 * i + 0] = 255; - dst[4 * i + 1] = src[3 * i + 0]; - dst[4 * i + 2] = src[3 * i + 1]; - dst[4 * i + 3] = src[3 * i + 2]; -#else - dst[4 * i + 0] = src[3 * i + 2]; + dst[4 * i + 0] = src[3 * i + 0]; dst[4 * i + 1] = src[3 * i + 1]; - dst[4 * i + 2] = src[3 * i + 0]; + dst[4 * i + 2] = src[3 * i + 2]; dst[4 * i + 3] = 255; -#endif } } @@ -183,17 +170,10 @@ void rgb16tobgr32(const uint8_t *src, uint8_t *dst, int src_size) while (s < end) { register uint16_t bgr = *s++; -#if HAVE_BIGENDIAN - *d++ = 255; *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2); *d++ = ((bgr&0x07E0)>>3) | ((bgr&0x07E0)>> 9); *d++ = ((bgr&0xF800)>>8) | ((bgr&0xF800)>>13); -#else - *d++ = ((bgr&0xF800)>>8) | ((bgr&0xF800)>>13); - *d++ = ((bgr&0x07E0)>>3) | ((bgr&0x07E0)>> 9); - *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2); *d++ = 255; -#endif } } @@ -258,17 +238,10 @@ void rgb15tobgr32(const uint8_t *src, uint8_t *dst, int src_size) while (s < end) { register uint16_t bgr = *s++; -#if HAVE_BIGENDIAN - *d++ = 255; *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2); *d++ = ((bgr&0x03E0)>>2) | ((bgr&0x03E0)>> 7); *d++ = ((bgr&0x7C00)>>7) | ((bgr&0x7C00)>>12); -#else - *d++ = ((bgr&0x7C00)>>7) | ((bgr&0x7C00)>>12); - *d++ = ((bgr&0x03E0)>>2) | ((bgr&0x03E0)>> 7); - *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2); *d++ = 255; -#endif } } diff --git a/libswscale/rgb2rgb_template.c b/libswscale/rgb2rgb_template.c index 499d25b..479474f 100644 --- a/libswscale/rgb2rgb_template.c +++ b/libswscale/rgb2rgb_template.c @@ -36,19 +36,11 @@ static inline void rgb24tobgr32_c(const uint8_t *src, uint8_t *dst, const uint8_t *end = s + src_size; while (s < end) { -#if HAVE_BIGENDIAN - /* RGB24 (= R, G, B) -> RGB32 (= A, B, G, R) */ - *dest++ = 255; *dest++ = s[2]; *dest++ = s[1]; *dest++ = s[0]; - s += 3; -#else - *dest++ = *s++; - *dest++ = *s++; - *dest++ = *s++; *dest++ = 255; -#endif + s += 3; } } @@ -60,19 +52,11 @@ static inline void rgb32tobgr24_c(const uint8_t *src, uint8_t *dst, const uint8_t *end = s + src_size; while (s < end) { -#if HAVE_BIGENDIAN - /* RGB32 (= A, B, G, R) -> RGB24 (= R, G, B) */ - s++; dest[2] = *s++; dest[1] = *s++; dest[0] = *s++; dest += 3; -#else - *dest++ = *s++; - *dest++ = *s++; - *dest++ = *s++; s++; -#endif } } @@ -120,14 +104,14 @@ static inline void rgb16to15_c(const uint8_t *src, uint8_t *dst, int src_size) } } -static inline void rgb32to16_c(const uint8_t *src, uint8_t *dst, int src_size) +static inline void rgb32tobgr16_c(const uint8_t *src, uint8_t *dst, int src_size) { uint16_t *d = (uint16_t *)dst; const uint8_t *s = src; const uint8_t *end = s + src_size; while (s < end) { - register int rgb = *(const uint32_t *)s; + register int rgb = AV_RL32(s); s += 4; *d++ = ((rgb & 0xFF) >> 3) + ((rgb & 0xFC00) >> 5) + @@ -135,7 +119,7 @@ static inline void rgb32to16_c(const uint8_t *src, uint8_t *dst, int src_size) } } -static inline void rgb32tobgr16_c(const uint8_t *src, uint8_t *dst, +static inline void rgb32to16_c(const uint8_t *src, uint8_t *dst, int src_size) { uint16_t *d = (uint16_t *)dst; @@ -143,7 +127,7 @@ static inline void rgb32tobgr16_c(const uint8_t *src, uint8_t *dst, const uint8_t *end = s + src_size; while (s < end) { - register int rgb = *(const uint32_t *)s; + register int rgb = AV_RL32(s); s += 4; *d++ = ((rgb & 0xF8) << 8) + ((rgb & 0xFC00) >> 5) + @@ -151,14 +135,14 @@ static inline void rgb32tobgr16_c(const uint8_t *src, uint8_t *dst, } } -static inline void rgb32to15_c(const uint8_t *src, uint8_t *dst, int src_size) +static inline void rgb32tobgr15_c(const uint8_t *src, uint8_t *dst, int src_size) { uint16_t *d = (uint16_t *)dst; const uint8_t *s = src; const uint8_t *end = s + src_size; while (s < end) { - register int rgb = *(const uint32_t *)s; + register int rgb = AV_RL32(s); s += 4; *d++ = ((rgb & 0xFF) >> 3) + ((rgb & 0xF800) >> 6) + @@ -166,7 +150,7 @@ static inline void rgb32to15_c(const uint8_t *src, uint8_t *dst, int src_size) } } -static inline void rgb32tobgr15_c(const uint8_t *src, uint8_t *dst, +static inline void rgb32to15_c(const uint8_t *src, uint8_t *dst, int src_size) { uint16_t *d = (uint16_t *)dst; @@ -174,7 +158,7 @@ static inline void rgb32tobgr15_c(const uint8_t *src, uint8_t *dst, const uint8_t *end = s + src_size; while (s < end) { - register int rgb = *(const uint32_t *)s; + register int rgb = AV_RL32(s); s += 4; *d++ = ((rgb & 0xF8) << 7) + ((rgb & 0xF800) >> 6) + @@ -278,17 +262,10 @@ static inline void rgb15to32_c(const uint8_t *src, uint8_t *dst, int src_size) while (s < end) { register uint16_t bgr = *s++; -#if HAVE_BIGENDIAN - *d++ = 255; *d++ = ((bgr&0x7C00)>>7) | ((bgr&0x7C00)>>12); *d++ = ((bgr&0x03E0)>>2) | ((bgr&0x03E0)>> 7); *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2); -#else - *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2); - *d++ = ((bgr&0x03E0)>>2) | ((bgr&0x03E0)>> 7); - *d++ = ((bgr&0x7C00)>>7) | ((bgr&0x7C00)>>12); *d++ = 255; -#endif } } @@ -300,17 +277,10 @@ static inline void rgb16to32_c(const uint8_t *src, uint8_t *dst, int src_size) while (s < end) { register uint16_t bgr = *s++; -#if HAVE_BIGENDIAN - *d++ = 255; *d++ = ((bgr&0xF800)>>8) | ((bgr&0xF800)>>13); *d++ = ((bgr&0x07E0)>>3) | ((bgr&0x07E0)>> 9); *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2); -#else - *d++ = ((bgr&0x001F)<<3) | ((bgr&0x001F)>> 2); - *d++ = ((bgr&0x07E0)>>3) | ((bgr&0x07E0)>> 9); - *d++ = ((bgr&0xF800)>>8) | ((bgr&0xF800)>>13); *d++ = 255; -#endif } } diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h index 0f51df9..16c041b 100644 --- a/libswscale/swscale_internal.h +++ b/libswscale/swscale_internal.h @@ -698,8 +698,8 @@ static av_always_inline int isRGBinInt(enum AVPixelFormat pix_fmt) { return pix_fmt == AV_PIX_FMT_RGB48BE || pix_fmt == AV_PIX_FMT_RGB48LE || - pix_fmt == AV_PIX_FMT_RGB32 || - pix_fmt == AV_PIX_FMT_RGB32_1 || + pix_fmt == AV_PIX_FMT_RGBA || + pix_fmt == AV_PIX_FMT_ARGB || pix_fmt == AV_PIX_FMT_RGB24 || pix_fmt == AV_PIX_FMT_RGB565BE || pix_fmt == AV_PIX_FMT_RGB565LE || @@ -720,8 +720,8 @@ static av_always_inline int isBGRinInt(enum AVPixelFormat pix_fmt) { return pix_fmt == AV_PIX_FMT_BGR48BE || pix_fmt == AV_PIX_FMT_BGR48LE || - pix_fmt == AV_PIX_FMT_BGR32 || - pix_fmt == AV_PIX_FMT_BGR32_1 || + pix_fmt == AV_PIX_FMT_BGRA || + pix_fmt == AV_PIX_FMT_ABGR || pix_fmt == AV_PIX_FMT_BGR24 || pix_fmt == AV_PIX_FMT_BGR565BE || pix_fmt == AV_PIX_FMT_BGR565LE ||