diff mbox

[FFmpeg-devel,3/5] swscale: use a function for isBayer

Message ID 20170319143343.7298-4-u@pkh.me
State Accepted
Headers show

Commit Message

Clément Bœsch March 19, 2017, 2:33 p.m. UTC 
---
 libswscale/swscale_internal.h | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

Comments

Michael Niedermayer March 19, 2017, 3:35 p.m. UTC | #1 
On Sun, Mar 19, 2017 at 03:33:41PM +0100, Clément Bœsch wrote:
> ---
>  libswscale/swscale_internal.h | 20 ++++++--------------
>  1 file changed, 6 insertions(+), 14 deletions(-)
> 
> diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
> index 6bcb4640ee..b3bb1695fb 100644
> --- a/libswscale/swscale_internal.h
> +++ b/libswscale/swscale_internal.h
> @@ -760,20 +760,12 @@ static av_always_inline int isGray(enum AVPixelFormat pix_fmt)
>          || (x) == AV_PIX_FMT_BGR24       \
>      )
>  
> -#define isBayer(x) ( \
> -           (x)==AV_PIX_FMT_BAYER_BGGR8    \
> -        || (x)==AV_PIX_FMT_BAYER_BGGR16LE \
> -        || (x)==AV_PIX_FMT_BAYER_BGGR16BE \
> -        || (x)==AV_PIX_FMT_BAYER_RGGB8    \
> -        || (x)==AV_PIX_FMT_BAYER_RGGB16LE \
> -        || (x)==AV_PIX_FMT_BAYER_RGGB16BE \
> -        || (x)==AV_PIX_FMT_BAYER_GBRG8    \
> -        || (x)==AV_PIX_FMT_BAYER_GBRG16LE \
> -        || (x)==AV_PIX_FMT_BAYER_GBRG16BE \
> -        || (x)==AV_PIX_FMT_BAYER_GRBG8    \
> -        || (x)==AV_PIX_FMT_BAYER_GRBG16LE \
> -        || (x)==AV_PIX_FMT_BAYER_GRBG16BE \
> -    )
> +static av_always_inline int isBayer(enum AVPixelFormat pix_fmt)
> +{
> +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
> +    av_assert0(desc);
> +    return !strncmp(desc->name, "bayer_", 6);

iam not sure strncmp() is a good idea speed wise

[...]
Clément Bœsch March 19, 2017, 3:50 p.m. UTC | #2 
On Sun, Mar 19, 2017 at 04:35:12PM +0100, Michael Niedermayer wrote:
> On Sun, Mar 19, 2017 at 03:33:41PM +0100, Clément Bœsch wrote:
> > ---
> >  libswscale/swscale_internal.h | 20 ++++++--------------
> >  1 file changed, 6 insertions(+), 14 deletions(-)
> > 
> > diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
> > index 6bcb4640ee..b3bb1695fb 100644
> > --- a/libswscale/swscale_internal.h
> > +++ b/libswscale/swscale_internal.h
> > @@ -760,20 +760,12 @@ static av_always_inline int isGray(enum AVPixelFormat pix_fmt)
> >          || (x) == AV_PIX_FMT_BGR24       \
> >      )
> >  
> > -#define isBayer(x) ( \
> > -           (x)==AV_PIX_FMT_BAYER_BGGR8    \
> > -        || (x)==AV_PIX_FMT_BAYER_BGGR16LE \
> > -        || (x)==AV_PIX_FMT_BAYER_BGGR16BE \
> > -        || (x)==AV_PIX_FMT_BAYER_RGGB8    \
> > -        || (x)==AV_PIX_FMT_BAYER_RGGB16LE \
> > -        || (x)==AV_PIX_FMT_BAYER_RGGB16BE \
> > -        || (x)==AV_PIX_FMT_BAYER_GBRG8    \
> > -        || (x)==AV_PIX_FMT_BAYER_GBRG16LE \
> > -        || (x)==AV_PIX_FMT_BAYER_GBRG16BE \
> > -        || (x)==AV_PIX_FMT_BAYER_GRBG8    \
> > -        || (x)==AV_PIX_FMT_BAYER_GRBG16LE \
> > -        || (x)==AV_PIX_FMT_BAYER_GRBG16BE \
> > -    )
> > +static av_always_inline int isBayer(enum AVPixelFormat pix_fmt)
> > +{
> > +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
> > +    av_assert0(desc);
> > +    return !strncmp(desc->name, "bayer_", 6);
> 
> iam not sure strncmp() is a good idea speed wise
> 

In a non-bayer case, the function will return in the worst case after the
2nd character, I have high doubt about this being a speed issue. We can
introduce a flag for this, but I don't think it's worth.
Michael Niedermayer March 19, 2017, 9:13 p.m. UTC | #3 
On Sun, Mar 19, 2017 at 04:50:24PM +0100, Clément Bœsch wrote:
> On Sun, Mar 19, 2017 at 04:35:12PM +0100, Michael Niedermayer wrote:
> > On Sun, Mar 19, 2017 at 03:33:41PM +0100, Clément Bœsch wrote:
> > > ---
> > >  libswscale/swscale_internal.h | 20 ++++++--------------
> > >  1 file changed, 6 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
> > > index 6bcb4640ee..b3bb1695fb 100644
> > > --- a/libswscale/swscale_internal.h
> > > +++ b/libswscale/swscale_internal.h
> > > @@ -760,20 +760,12 @@ static av_always_inline int isGray(enum AVPixelFormat pix_fmt)
> > >          || (x) == AV_PIX_FMT_BGR24       \
> > >      )
> > >  
> > > -#define isBayer(x) ( \
> > > -           (x)==AV_PIX_FMT_BAYER_BGGR8    \
> > > -        || (x)==AV_PIX_FMT_BAYER_BGGR16LE \
> > > -        || (x)==AV_PIX_FMT_BAYER_BGGR16BE \
> > > -        || (x)==AV_PIX_FMT_BAYER_RGGB8    \
> > > -        || (x)==AV_PIX_FMT_BAYER_RGGB16LE \
> > > -        || (x)==AV_PIX_FMT_BAYER_RGGB16BE \
> > > -        || (x)==AV_PIX_FMT_BAYER_GBRG8    \
> > > -        || (x)==AV_PIX_FMT_BAYER_GBRG16LE \
> > > -        || (x)==AV_PIX_FMT_BAYER_GBRG16BE \
> > > -        || (x)==AV_PIX_FMT_BAYER_GRBG8    \
> > > -        || (x)==AV_PIX_FMT_BAYER_GRBG16LE \
> > > -        || (x)==AV_PIX_FMT_BAYER_GRBG16BE \
> > > -    )
> > > +static av_always_inline int isBayer(enum AVPixelFormat pix_fmt)
> > > +{
> > > +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
> > > +    av_assert0(desc);
> > > +    return !strncmp(desc->name, "bayer_", 6);
> > 
> > iam not sure strncmp() is a good idea speed wise
> > 
> 
> In a non-bayer case, the function will return in the worst case after the
> 2nd character, I have high doubt about this being a speed issue. We can
> introduce a flag for this, but I don't think it's worth.

i think needing to call libc is a bit ugly here

[...]
diff mbox

Patch

diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
index 6bcb4640ee..b3bb1695fb 100644
--- a/libswscale/swscale_internal.h
+++ b/libswscale/swscale_internal.h
@@ -760,20 +760,12 @@  static av_always_inline int isGray(enum AVPixelFormat pix_fmt)
         || (x) == AV_PIX_FMT_BGR24       \
     )
 
-#define isBayer(x) ( \
-           (x)==AV_PIX_FMT_BAYER_BGGR8    \
-        || (x)==AV_PIX_FMT_BAYER_BGGR16LE \
-        || (x)==AV_PIX_FMT_BAYER_BGGR16BE \
-        || (x)==AV_PIX_FMT_BAYER_RGGB8    \
-        || (x)==AV_PIX_FMT_BAYER_RGGB16LE \
-        || (x)==AV_PIX_FMT_BAYER_RGGB16BE \
-        || (x)==AV_PIX_FMT_BAYER_GBRG8    \
-        || (x)==AV_PIX_FMT_BAYER_GBRG16LE \
-        || (x)==AV_PIX_FMT_BAYER_GBRG16BE \
-        || (x)==AV_PIX_FMT_BAYER_GRBG8    \
-        || (x)==AV_PIX_FMT_BAYER_GRBG16LE \
-        || (x)==AV_PIX_FMT_BAYER_GRBG16BE \
-    )
+static av_always_inline int isBayer(enum AVPixelFormat pix_fmt)
+{
+    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
+    av_assert0(desc);
+    return !strncmp(desc->name, "bayer_", 6);
+}
 
 #define isAnyRGB(x) \
     (           \