diff mbox

[FFmpeg-devel,v1] swscale/swscale: various cosmetics for the code style

Message ID 20190907155020.2283-1-lance.lmwang@gmail.com
State New
Headers show

Commit Message

Lance Wang Sept. 7, 2019, 3:50 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libswscale/swscale.c | 40 ++++++++++++----------------------------
 1 file changed, 12 insertions(+), 28 deletions(-)

Comments

Andriy Gelman Sept. 7, 2019, 8:20 p.m. UTC | #1 
On Sat, 07. Sep 23:50, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libswscale/swscale.c | 40 ++++++++++++----------------------------
>  1 file changed, 12 insertions(+), 28 deletions(-)
> 
> diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> index 4069550..3e2ebd5 100644
> --- a/libswscale/swscale.c
> +++ b/libswscale/swscale.c
> @@ -271,7 +271,6 @@ static int swscale(SwsContext *c, const uint8_t *src[],
>      int lastInLumBuf = c->lastInLumBuf;
>      int lastInChrBuf = c->lastInChrBuf;
>  
> -
>      int lumStart = 0;
>      int lumEnd = c->descIndex[0];
>      int chrStart = lumEnd;
> @@ -283,22 +282,16 @@ static int swscale(SwsContext *c, const uint8_t *src[],
>      SwsSlice *vout_slice = &c->slice[c->numSlice-1];
>      SwsFilterDescriptor *desc = c->desc;
>  
> -
>      int needAlpha = c->needAlpha;
>  
>      int hasLumHoles = 1;
>      int hasChrHoles = 1;
>  
> -
>      if (isPacked(c->srcFormat)) {
> -        src[0] =
> -        src[1] =
> -        src[2] =
> -        src[3] = src[0];
> -        srcStride[0] =
> -        srcStride[1] =
> -        srcStride[2] =
> -        srcStride[3] = srcStride[0];

> +        for ( int i = 0; i < 4; i++ ) {

extra spaces and I guess you can start i from 1

> +            src[i] = src[0];
> +            srcStride[i] = srcStride[0];
> +        }
>      }
>      srcStride[1] <<= c->vChrDrop;
>      srcStride[2] <<= c->vChrDrop;
> @@ -324,7 +317,7 @@ static int swscale(SwsContext *c, const uint8_t *src[],
>          }
>      }
>  
> -    if (   (uintptr_t)dst[0]&15 || (uintptr_t)dst[1]&15 || (uintptr_t)dst[2]&15
> +    if ((uintptr_t)dst[0]&15 || (uintptr_t)dst[1]&15 || (uintptr_t)dst[2]&15
>          || (uintptr_t)src[0]&15 || (uintptr_t)src[1]&15 || (uintptr_t)src[2]&15
>          || dstStride[0]&15 || dstStride[1]&15 || dstStride[2]&15 || dstStride[3]&15
>          || srcStride[0]&15 || srcStride[1]&15 || srcStride[2]&15 || srcStride[3]&15
> @@ -341,11 +334,9 @@ static int swscale(SwsContext *c, const uint8_t *src[],
>       * will not get executed. This is not really intended but works
>       * currently, so people might do it. */
>      if (srcSliceY == 0) {
> -        lumBufIndex  = -1;
> -        chrBufIndex  = -1;
> +        lumBufIndex  = chrBufIndex  = -1;
>          dstY         = 0;
> -        lastInLumBuf = -1;
> -        lastInChrBuf = -1;
> +        lastInLumBuf = lastInChrBuf = -1;
>      }
>  
>      if (!should_dither) {
> @@ -368,16 +359,14 @@ static int swscale(SwsContext *c, const uint8_t *src[],
>          hout_slice->plane[2].sliceY = lastInChrBuf + 1;
>          hout_slice->plane[3].sliceY = lastInLumBuf + 1;
>  
> -        hout_slice->plane[0].sliceH =
> -        hout_slice->plane[1].sliceH =
> -        hout_slice->plane[2].sliceH =
> -        hout_slice->plane[3].sliceH = 0;
> +        for(int i = 0; i < 4; i++)
> +            hout_slice->plane[i].sliceH = 0;
>          hout_slice->width = dstW;
>      }
>  
>      for (; dstY < dstH; dstY++) {
>          const int chrDstY = dstY >> c->chrDstVSubSample;
> -        int use_mmx_vfilter= c->use_mmx_vfilter;
> +        int use_mmx_vfilter = c->use_mmx_vfilter;
>  
>          // First line needed as input
>          const int firstLumSrcY  = FFMAX(1 - vLumFilterSize, vLumFilterPos[dstY]);
> @@ -396,7 +385,6 @@ static int swscale(SwsContext *c, const uint8_t *src[],
>  
>          // handle holes (FAST_BILINEAR & weird filters)
>          if (firstLumSrcY > lastInLumBuf) {
> -
>              hasLumHoles = lastInLumBuf != firstLumSrcY - 1;
>              if (hasLumHoles) {
>                  hout_slice->plane[0].sliceY = firstLumSrcY;
> @@ -408,7 +396,6 @@ static int swscale(SwsContext *c, const uint8_t *src[],
>              lastInLumBuf = firstLumSrcY - 1;
>          }
>          if (firstChrSrcY > lastInChrBuf) {
> -
>              hasChrHoles = lastInChrBuf != firstChrSrcY - 1;
>              if (hasChrHoles) {
>                  hout_slice->plane[1].sliceY = firstChrSrcY;
> @@ -440,7 +427,6 @@ static int swscale(SwsContext *c, const uint8_t *src[],
>          av_assert0((lastLumSrcY - firstLumSrcY + 1) <= hout_slice->plane[0].available_lines);
>          av_assert0((lastChrSrcY - firstChrSrcY + 1) <= hout_slice->plane[1].available_lines);
>  
> -
>          posY = hout_slice->plane[0].sliceY + hout_slice->plane[0].sliceH;
>          if (posY <= lastLumSrcY && !hasLumHoles) {
>              firstPosY = FFMAX(firstLumSrcY, posY);
> @@ -503,10 +489,8 @@ static int swscale(SwsContext *c, const uint8_t *src[],
>                             yuv2packed1, yuv2packed2, yuv2packedX, yuv2anyX, use_mmx_vfilter);
>          }
>  
> -        {
> -            for (i = vStart; i < vEnd; ++i)
> -                desc[i].process(c, &desc[i], dstY, 1);
> -        }

> +        for (i = vStart; i < vEnd; ++i)

I thought that i++ is preferred to ++i

> +            desc[i].process(c, &desc[i], dstY, 1);
>      }
>      if (isPlanar(dstFormat) && isALPHA(dstFormat) && !needAlpha) {
>          int length = dstW;
> -- 
> 2.6.4
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Andriy
Moritz Barsnick Sept. 8, 2019, 11:53 a.m. UTC | #2 
On Sat, Sep 07, 2019 at 16:20:35 -0400, Andriy Gelman wrote:
> >      if (isPacked(c->srcFormat)) {
> > -        src[0] =
> > -        src[1] =
> > -        src[2] =
> > -        src[3] = src[0];
> > -        srcStride[0] =
> > -        srcStride[1] =
> > -        srcStride[2] =
> > -        srcStride[3] = srcStride[0];
>
> > +        for ( int i = 0; i < 4; i++ ) {
>
> extra spaces and I guess you can start i from 1

I actually wonder what the original point was of assigning src[0] to
src[0].

> > -    if (   (uintptr_t)dst[0]&15 || (uintptr_t)dst[1]&15 || (uintptr_t)dst[2]&15
> > +    if ((uintptr_t)dst[0]&15 || (uintptr_t)dst[1]&15 || (uintptr_t)dst[2]&15
> >          || (uintptr_t)src[0]&15 || (uintptr_t)src[1]&15 || (uintptr_t)src[2]&15

BTW, the original indentation was meant for alignment.

> > +        for(int i = 0; i < 4; i++)

"for (" is the correct style.

Moritz
Lance Wang Sept. 8, 2019, 12:53 p.m. UTC | #3 
On Sun, Sep 08, 2019 at 01:53:12PM +0200, Moritz Barsnick wrote:
> On Sat, Sep 07, 2019 at 16:20:35 -0400, Andriy Gelman wrote:
> > >      if (isPacked(c->srcFormat)) {
> > > -        src[0] =
> > > -        src[1] =
> > > -        src[2] =
> > > -        src[3] = src[0];
> > > -        srcStride[0] =
> > > -        srcStride[1] =
> > > -        srcStride[2] =
> > > -        srcStride[3] = srcStride[0];
> >
> > > +        for ( int i = 0; i < 4; i++ ) {
> >
> > extra spaces and I guess you can start i from 1
> 
> I actually wonder what the original point was of assigning src[0] to
> src[0].
it's cosmetics change, so I want to keep same logic with the old code.
Maybe we can split the patch if the code isn't reasonable.


> 
> > > -    if (   (uintptr_t)dst[0]&15 || (uintptr_t)dst[1]&15 || (uintptr_t)dst[2]&15
> > > +    if ((uintptr_t)dst[0]&15 || (uintptr_t)dst[1]&15 || (uintptr_t)dst[2]&15
> > >          || (uintptr_t)src[0]&15 || (uintptr_t)src[1]&15 || (uintptr_t)src[2]&15
> 
> BTW, the original indentation was meant for alignment.
OK, if for alignment, I'll adjust it.

> 
> > > +        for(int i = 0; i < 4; i++)
> 
> "for (" is the correct style.
OK, maybe I misunderstand. I'll update it.


> 
> Moritz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox

Patch

diff --git a/libswscale/swscale.c b/libswscale/swscale.c
index 4069550..3e2ebd5 100644
--- a/libswscale/swscale.c
+++ b/libswscale/swscale.c
@@ -271,7 +271,6 @@  static int swscale(SwsContext *c, const uint8_t *src[],
     int lastInLumBuf = c->lastInLumBuf;
     int lastInChrBuf = c->lastInChrBuf;
 
-
     int lumStart = 0;
     int lumEnd = c->descIndex[0];
     int chrStart = lumEnd;
@@ -283,22 +282,16 @@  static int swscale(SwsContext *c, const uint8_t *src[],
     SwsSlice *vout_slice = &c->slice[c->numSlice-1];
     SwsFilterDescriptor *desc = c->desc;
 
-
     int needAlpha = c->needAlpha;
 
     int hasLumHoles = 1;
     int hasChrHoles = 1;
 
-
     if (isPacked(c->srcFormat)) {
-        src[0] =
-        src[1] =
-        src[2] =
-        src[3] = src[0];
-        srcStride[0] =
-        srcStride[1] =
-        srcStride[2] =
-        srcStride[3] = srcStride[0];
+        for ( int i = 0; i < 4; i++ ) {
+            src[i] = src[0];
+            srcStride[i] = srcStride[0];
+        }
     }
     srcStride[1] <<= c->vChrDrop;
     srcStride[2] <<= c->vChrDrop;
@@ -324,7 +317,7 @@  static int swscale(SwsContext *c, const uint8_t *src[],
         }
     }
 
-    if (   (uintptr_t)dst[0]&15 || (uintptr_t)dst[1]&15 || (uintptr_t)dst[2]&15
+    if ((uintptr_t)dst[0]&15 || (uintptr_t)dst[1]&15 || (uintptr_t)dst[2]&15
         || (uintptr_t)src[0]&15 || (uintptr_t)src[1]&15 || (uintptr_t)src[2]&15
         || dstStride[0]&15 || dstStride[1]&15 || dstStride[2]&15 || dstStride[3]&15
         || srcStride[0]&15 || srcStride[1]&15 || srcStride[2]&15 || srcStride[3]&15
@@ -341,11 +334,9 @@  static int swscale(SwsContext *c, const uint8_t *src[],
      * will not get executed. This is not really intended but works
      * currently, so people might do it. */
     if (srcSliceY == 0) {
-        lumBufIndex  = -1;
-        chrBufIndex  = -1;
+        lumBufIndex  = chrBufIndex  = -1;
         dstY         = 0;
-        lastInLumBuf = -1;
-        lastInChrBuf = -1;
+        lastInLumBuf = lastInChrBuf = -1;
     }
 
     if (!should_dither) {
@@ -368,16 +359,14 @@  static int swscale(SwsContext *c, const uint8_t *src[],
         hout_slice->plane[2].sliceY = lastInChrBuf + 1;
         hout_slice->plane[3].sliceY = lastInLumBuf + 1;
 
-        hout_slice->plane[0].sliceH =
-        hout_slice->plane[1].sliceH =
-        hout_slice->plane[2].sliceH =
-        hout_slice->plane[3].sliceH = 0;
+        for(int i = 0; i < 4; i++)
+            hout_slice->plane[i].sliceH = 0;
         hout_slice->width = dstW;
     }
 
     for (; dstY < dstH; dstY++) {
         const int chrDstY = dstY >> c->chrDstVSubSample;
-        int use_mmx_vfilter= c->use_mmx_vfilter;
+        int use_mmx_vfilter = c->use_mmx_vfilter;
 
         // First line needed as input
         const int firstLumSrcY  = FFMAX(1 - vLumFilterSize, vLumFilterPos[dstY]);
@@ -396,7 +385,6 @@  static int swscale(SwsContext *c, const uint8_t *src[],
 
         // handle holes (FAST_BILINEAR & weird filters)
         if (firstLumSrcY > lastInLumBuf) {
-
             hasLumHoles = lastInLumBuf != firstLumSrcY - 1;
             if (hasLumHoles) {
                 hout_slice->plane[0].sliceY = firstLumSrcY;
@@ -408,7 +396,6 @@  static int swscale(SwsContext *c, const uint8_t *src[],
             lastInLumBuf = firstLumSrcY - 1;
         }
         if (firstChrSrcY > lastInChrBuf) {
-
             hasChrHoles = lastInChrBuf != firstChrSrcY - 1;
             if (hasChrHoles) {
                 hout_slice->plane[1].sliceY = firstChrSrcY;
@@ -440,7 +427,6 @@  static int swscale(SwsContext *c, const uint8_t *src[],
         av_assert0((lastLumSrcY - firstLumSrcY + 1) <= hout_slice->plane[0].available_lines);
         av_assert0((lastChrSrcY - firstChrSrcY + 1) <= hout_slice->plane[1].available_lines);
 
-
         posY = hout_slice->plane[0].sliceY + hout_slice->plane[0].sliceH;
         if (posY <= lastLumSrcY && !hasLumHoles) {
             firstPosY = FFMAX(firstLumSrcY, posY);
@@ -503,10 +489,8 @@  static int swscale(SwsContext *c, const uint8_t *src[],
                            yuv2packed1, yuv2packed2, yuv2packedX, yuv2anyX, use_mmx_vfilter);
         }
 
-        {
-            for (i = vStart; i < vEnd; ++i)
-                desc[i].process(c, &desc[i], dstY, 1);
-        }
+        for (i = vStart; i < vEnd; ++i)
+            desc[i].process(c, &desc[i], dstY, 1);
     }
     if (isPlanar(dstFormat) && isALPHA(dstFormat) && !needAlpha) {
         int length = dstW;