[FFmpeg-devel,v3,5/5] avcodec/v210enc: move the duplicate code to template file

Submitted by lance.lmwang@gmail.com on Sept. 1, 2019, 1:20 p.m.

Details

Message ID 20190901132023.28531-5-lance.lmwang@gmail.com
State New
Headers show

Commit Message

lance.lmwang@gmail.com Sept. 1, 2019, 1:20 p.m.
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavcodec/v210_template.c |  59 +++++++++++++++++++++-
 libavcodec/v210enc.c       | 123 +++++++--------------------------------------
 2 files changed, 75 insertions(+), 107 deletions(-)

Comments

Michael Niedermayer Sept. 16, 2019, 10:21 a.m.
On Sun, Sep 01, 2019 at 09:20:23PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavcodec/v210_template.c |  59 +++++++++++++++++++++-
>  libavcodec/v210enc.c       | 123 +++++++--------------------------------------
>  2 files changed, 75 insertions(+), 107 deletions(-)

[PATCH v3 3/5] avcodec/v210enc: move the duplicate code to template file
[PATCH v3 5/5] avcodec/v210enc: move the duplicate code to template file

why do 2 patches have the same first line in the commit message ?


[...]
lance.lmwang@gmail.com Sept. 16, 2019, 11:17 a.m.
On Mon, Sep 16, 2019 at 12:21:36PM +0200, Michael Niedermayer wrote:
> On Sun, Sep 01, 2019 at 09:20:23PM +0800, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavcodec/v210_template.c |  59 +++++++++++++++++++++-
> >  libavcodec/v210enc.c       | 123 +++++++--------------------------------------
> >  2 files changed, 75 insertions(+), 107 deletions(-)
> 
> [PATCH v3 3/5] avcodec/v210enc: move the duplicate code to template file
> [PATCH v3 5/5] avcodec/v210enc: move the duplicate code to template file
> 
> why do 2 patches have the same first line in the commit message ?
It's for split the patch for review, 3/5 is move v210_planar_pack, 5/5 is moving 
v210enc related function, so the code is different.

> 
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Elect your leaders based on what they did after the last election, not
> based on what they say before an election.
> 



> _______________________________________________
> 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".
Michael Niedermayer Sept. 16, 2019, 11:16 p.m.
On Mon, Sep 16, 2019 at 07:17:52PM +0800, Limin Wang wrote:
> On Mon, Sep 16, 2019 at 12:21:36PM +0200, Michael Niedermayer wrote:
> > On Sun, Sep 01, 2019 at 09:20:23PM +0800, lance.lmwang@gmail.com wrote:
> > > From: Limin Wang <lance.lmwang@gmail.com>
> > > 
> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > ---
> > >  libavcodec/v210_template.c |  59 +++++++++++++++++++++-
> > >  libavcodec/v210enc.c       | 123 +++++++--------------------------------------
> > >  2 files changed, 75 insertions(+), 107 deletions(-)
> > 
> > [PATCH v3 3/5] avcodec/v210enc: move the duplicate code to template file
> > [PATCH v3 5/5] avcodec/v210enc: move the duplicate code to template file
> > 
> > why do 2 patches have the same first line in the commit message ?
> It's for split the patch for review, 3/5 is move v210_planar_pack, 5/5 is moving 
> v210enc related function, so the code is different.

If the same description is good for different changes then the description
is not very good (not specific enough) IMO

thanks

[...]
lance.lmwang@gmail.com Sept. 17, 2019, 1:44 a.m.
On Tue, Sep 17, 2019 at 01:16:36AM +0200, Michael Niedermayer wrote:
> On Mon, Sep 16, 2019 at 07:17:52PM +0800, Limin Wang wrote:
> > On Mon, Sep 16, 2019 at 12:21:36PM +0200, Michael Niedermayer wrote:
> > > On Sun, Sep 01, 2019 at 09:20:23PM +0800, lance.lmwang@gmail.com wrote:
> > > > From: Limin Wang <lance.lmwang@gmail.com>
> > > > 
> > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > > ---
> > > >  libavcodec/v210_template.c |  59 +++++++++++++++++++++-
> > > >  libavcodec/v210enc.c       | 123 +++++++--------------------------------------
> > > >  2 files changed, 75 insertions(+), 107 deletions(-)
> > > 
> > > [PATCH v3 3/5] avcodec/v210enc: move the duplicate code to template file
> > > [PATCH v3 5/5] avcodec/v210enc: move the duplicate code to template file
> > > 
> > > why do 2 patches have the same first line in the commit message ?
> > It's for split the patch for review, 3/5 is move v210_planar_pack, 5/5 is moving 
> > v210enc related function, so the code is different.
> 
> If the same description is good for different changes then the description
> is not very good (not specific enough) IMO

Yes, will avoid to use same description next time.

> 
> thanks
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> In a rich man's house there is no place to spit but his face.
> -- Diogenes of Sinope



> _______________________________________________
> 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".

Patch hide | download patch | download mbox

diff --git a/libavcodec/v210_template.c b/libavcodec/v210_template.c
index 00036ae..a3657d2 100644
--- a/libavcodec/v210_template.c
+++ b/libavcodec/v210_template.c
@@ -32,7 +32,7 @@ 
         dst += 4;                                         \
     } while (0)
 
-static void RENAME(v210_planar_pack)(const TYPE *y,
+static void RENAMEC(v210_planar_pack)(const TYPE *y,
         const TYPE *u, const TYPE *v,
         uint8_t *dst, ptrdiff_t width)
 {
@@ -46,3 +46,60 @@  static void RENAME(v210_planar_pack)(const TYPE *y,
         WRITE_PIXELS(y, v, y, DEPTH);
     }
 }
+
+static void RENAME(v210_enc)(AVCodecContext *avctx,
+        uint8_t *dst, const AVFrame *pic)
+{
+    V210EncContext *s = avctx->priv_data;
+    int aligned_width = ((avctx->width + 47) / 48) * 48;
+    int stride = aligned_width * 8 / 3;
+    int line_padding = stride - ((avctx->width * 8 + 11) / 12) * 4;
+    int h, w;
+    const TYPE *y = (const TYPE *)pic->data[0];
+    const TYPE *u = (const TYPE *)pic->data[1];
+    const TYPE *v = (const TYPE *)pic->data[2];
+    const int sample_size = 6 * s->RENAME(sample_factor);
+    const int sample_w    = avctx->width / sample_size;
+
+    for (h = 0; h < avctx->height; h++) {
+        uint32_t val;
+        w = sample_w * sample_size;
+        s->RENAME(pack_line)(y, u, v, dst, w);
+
+        y += w;
+        u += w >> 1;
+        v += w >> 1;
+        dst += sample_w * 16 * s->RENAME(sample_factor);
+
+        for (; w < avctx->width - 5; w += 6) {
+            WRITE_PIXELS(u, y, v, DEPTH);
+            WRITE_PIXELS(y, u, y, DEPTH);
+            WRITE_PIXELS(v, y, u, DEPTH);
+            WRITE_PIXELS(y, v, y, DEPTH);
+        }
+        if (w < avctx->width - 1) {
+            WRITE_PIXELS(u, y, v, DEPTH);
+
+            val = CLIP(*y++, DEPTH) << (10-DEPTH);
+            if (w == avctx->width - 2) {
+                AV_WL32(dst, val);
+                dst += 4;
+            }
+        }
+        if (w < avctx->width - 3) {
+            val |= (CLIP(*u++, DEPTH) << (20-DEPTH)) | (CLIP(*y++, DEPTH) << (30-DEPTH));
+            AV_WL32(dst, val);
+            dst += 4;
+
+            val = CLIP(*v++, DEPTH) << (10-DEPTH) | (CLIP(*y++, DEPTH) << (20-DEPTH));
+            AV_WL32(dst, val);
+            dst += 4;
+        }
+
+        memset(dst, 0, line_padding);
+        dst += line_padding;
+        y += pic->linesize[0] / BYTES_PER_PIXEL - avctx->width;
+        u += pic->linesize[1] / BYTES_PER_PIXEL - avctx->width / 2;
+        v += pic->linesize[2] / BYTES_PER_PIXEL - avctx->width / 2;
+    }
+}
diff --git a/libavcodec/v210enc.c b/libavcodec/v210enc.c
index d078f0a..1376d88 100644
--- a/libavcodec/v210enc.c
+++ b/libavcodec/v210enc.c
@@ -28,19 +28,27 @@ 
 
 #define TYPE uint8_t
 #define DEPTH 8
-#define RENAME(a) a ## _ ## 8 ##  _c
+#define BYTES_PER_PIXEL 1
+#define RENAMEC(a) a ## _ ## 8 ##  _c
+#define RENAME(a) a ## _ ## 8
 #include "v210_template.c"
-#undef TYPE
 #undef RENAME
+#undef RENAMEC
 #undef DEPTH
+#undef BYTES_PER_PIXEL
+#undef TYPE
 
 #define TYPE uint16_t
 #define DEPTH 10
-#define RENAME(a) a ## _ ## 10 ##  _c
+#define BYTES_PER_PIXEL 2
+#define RENAMEC(a) a ## _ ## 10 ##  _c
+#define RENAME(a) a ## _ ## 10
 #include "v210_template.c"
-#undef TYPE
 #undef RENAME
+#undef RENAMEC
 #undef DEPTH
+#undef BYTES_PER_PIXEL
+#undef TYPE
 
 av_cold void ff_v210enc_init(V210EncContext *s)
 {
@@ -79,12 +87,10 @@  FF_ENABLE_DEPRECATION_WARNINGS
 static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
                         const AVFrame *pic, int *got_packet)
 {
-    V210EncContext *s = avctx->priv_data;
     int aligned_width = ((avctx->width + 47) / 48) * 48;
     int stride = aligned_width * 8 / 3;
-    int line_padding = stride - ((avctx->width * 8 + 11) / 12) * 4;
     AVFrameSideData *side_data;
-    int h, w, ret;
+    int ret;
     uint8_t *dst;
 
     ret = ff_alloc_packet2(avctx, pkt, avctx->height * stride, avctx->height * stride);
@@ -94,105 +100,10 @@  static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
     }
     dst = pkt->data;
 
-    if (pic->format == AV_PIX_FMT_YUV422P10) {
-        const uint16_t *y = (const uint16_t *)pic->data[0];
-        const uint16_t *u = (const uint16_t *)pic->data[1];
-        const uint16_t *v = (const uint16_t *)pic->data[2];
-
-        const int sample_size = 6 * s->sample_factor_10;
-        const int sample_w    = avctx->width / sample_size;
-
-        for (h = 0; h < avctx->height; h++) {
-            uint32_t val;
-            w = sample_w * sample_size;
-            s->pack_line_10(y, u, v, dst, w);
-
-            y += w;
-            u += w >> 1;
-            v += w >> 1;
-            dst += sample_w * 16 * s->sample_factor_10;
-
-            for (; w < avctx->width - 5; w += 6) {
-                WRITE_PIXELS(u, y, v, 10);
-                WRITE_PIXELS(y, u, y, 10);
-                WRITE_PIXELS(v, y, u, 10);
-                WRITE_PIXELS(y, v, y, 10);
-            }
-            if (w < avctx->width - 1) {
-                WRITE_PIXELS(u, y, v, 10);
-
-                val = CLIP(*y++, 10) << (10-10);
-                if (w == avctx->width - 2) {
-                    AV_WL32(dst, val);
-                    dst += 4;
-                }
-            }
-            if (w < avctx->width - 3) {
-                val |= (CLIP(*u++, 10) << (20-10)) | (CLIP(*y++, 10) << (30-10));
-                AV_WL32(dst, val);
-                dst += 4;
-
-                val = CLIP(*v++, 10) << (10-10) | (CLIP(*y++, 10) << (20-10));
-                AV_WL32(dst, val);
-                dst += 4;
-            }
-
-            memset(dst, 0, line_padding);
-            dst += line_padding;
-            y += pic->linesize[0] / 2 - avctx->width;
-            u += pic->linesize[1] / 2 - avctx->width / 2;
-            v += pic->linesize[2] / 2 - avctx->width / 2;
-        }
-    } else if(pic->format == AV_PIX_FMT_YUV422P) {
-        const uint8_t *y = pic->data[0];
-        const uint8_t *u = pic->data[1];
-        const uint8_t *v = pic->data[2];
-
-        const int sample_size = 6 * s->sample_factor_8;
-        const int sample_w    = avctx->width / sample_size;
-
-        for (h = 0; h < avctx->height; h++) {
-            uint32_t val;
-            w = sample_w * sample_size;
-            s->pack_line_8(y, u, v, dst, w);
-
-            y += w;
-            u += w >> 1;
-            v += w >> 1;
-            dst += sample_w * 16 * s->sample_factor_8;
-
-            for (; w < avctx->width - 5; w += 6) {
-                WRITE_PIXELS(u, y, v, 8);
-                WRITE_PIXELS(y, u, y, 8);
-                WRITE_PIXELS(v, y, u, 8);
-                WRITE_PIXELS(y, v, y, 8);
-            }
-            if (w < avctx->width - 1) {
-                WRITE_PIXELS(u, y, v, 8);
-
-                val = CLIP(*y++, 8) << (10-8);
-                if (w == avctx->width - 2) {
-                    AV_WL32(dst, val);
-                    dst += 4;
-                }
-            }
-            if (w < avctx->width - 3) {
-                val |= (CLIP(*u++, 8) << (20-8)) | (CLIP(*y++, 8) << (30-8));
-                AV_WL32(dst, val);
-                dst += 4;
-
-                val = (CLIP(*v++, 8) << (10-8)) | (CLIP(*y++, 8) << (20-8));
-                AV_WL32(dst, val);
-                dst += 4;
-            }
-            memset(dst, 0, line_padding);
-            dst += line_padding;
-
-            y += pic->linesize[0] - avctx->width;
-            u += pic->linesize[1] - avctx->width / 2;
-            v += pic->linesize[2] - avctx->width / 2;
-        }
-    }
+    if (pic->format == AV_PIX_FMT_YUV422P10)
+        v210_enc_10(avctx, dst, pic);
+    else if(pic->format == AV_PIX_FMT_YUV422P)
+        v210_enc_8(avctx, dst, pic);
 
     side_data = av_frame_get_side_data(pic, AV_FRAME_DATA_A53_CC);
     if (side_data && side_data->size) {