diff mbox

[FFmpeg-devel,v5,2/4] lavc/libxavs2: optimize data access

Message ID 1571059365-3263-2-git-send-email-hwrenx@126.com
State New
Headers show

Commit Message

hwren Oct. 14, 2019, 1:22 p.m. UTC
Optimize data access from multiplication to iteration.

Signed-off-by: hwren <hwrenx@126.com>
---
 libavcodec/libxavs2.c | 45 +++++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 16 deletions(-)

Comments

hwren Nov. 29, 2019, 7:46 a.m. UTC | #1
ping the remaining three patches.


Thanks,
Huiwen Ren






At 2019-10-14 21:22:43, "hwren" <hwrenx@126.com> wrote:
>Optimize data access from multiplication to iteration.
>
>Signed-off-by: hwren <hwrenx@126.com>
>---
> libavcodec/libxavs2.c | 45 +++++++++++++++++++++++++++++----------------
> 1 file changed, 29 insertions(+), 16 deletions(-)
>
>diff --git a/libavcodec/libxavs2.c b/libavcodec/libxavs2.c
>index 0179a1e..b5c07ec 100644
>--- a/libavcodec/libxavs2.c
>+++ b/libavcodec/libxavs2.c
>@@ -132,29 +132,42 @@ static av_cold int xavs2_init(AVCodecContext *avctx)
> 
> static void xavs2_copy_frame_with_shift(xavs2_picture_t *pic, const AVFrame *frame, const int shift_in)
> {
>-    int j, k;
>-    for (k = 0; k < 3; k++) {
>-        int i_stride = pic->img.i_stride[k];
>-        for (j = 0; j < pic->img.i_lines[k]; j++) {
>-            uint16_t *p_plane = (uint16_t *)&pic->img.img_planes[k][j * i_stride];
>-            int i;
>-            uint8_t *p_buffer = frame->data[k] + frame->linesize[k] * j;
>-            memset(p_plane, 0, i_stride);
>-            for (i = 0; i < pic->img.i_width[k]; i++) {
>-                p_plane[i] = p_buffer[i] << shift_in;
>+    uint16_t *p_plane;
>+    uint8_t *p_buffer;
>+    int wIdx;
>+    int hIdx;
>+    int plane;
>+
>+    for (plane = 0; plane < 3; ++plane) {
>+        p_plane  = (uint16_t *)pic->img.img_planes[plane];
>+        p_buffer = frame->data[plane];
>+        for (hIdx = 0; hIdx < pic->img.i_lines[plane]; ++hIdx) {
>+            memset(p_plane, 0, pic->img.i_stride[plane]);
>+            for (wIdx = 0; wIdx < pic->img.i_width[plane]; ++wIdx) {
>+                p_plane[wIdx] = p_buffer[wIdx] << shift_in;
>             }
>+            p_plane += pic->img.i_stride[plane];
>+            p_buffer += frame->linesize[plane];
>         }
>     }
> }
> 
> static void xavs2_copy_frame(xavs2_picture_t *pic, const AVFrame *frame)
> {
>-    int j, k;
>-    for (k = 0; k < 3; k++) {
>-        for (j = 0; j < pic->img.i_lines[k]; j++) {
>-            memcpy( pic->img.img_planes[k] + pic->img.i_stride[k] * j,
>-                    frame->data[k]+frame->linesize[k] * j,
>-                    pic->img.i_width[k] * pic->img.in_sample_size);
>+    uint8_t *p_plane;
>+    uint8_t *p_buffer;
>+    int hIdx;
>+    int plane;
>+    int stride;
>+
>+    for (plane = 0; plane < 3; ++plane) {
>+        p_plane  = pic->img.img_planes[plane];
>+        p_buffer = frame->data[plane];
>+        stride = pic->img.i_width[plane] * pic->img.in_sample_size;
>+        for (hIdx = 0; hIdx < pic->img.i_lines[plane]; ++hIdx) {
>+            memcpy(p_plane, p_buffer, stride);
>+            p_plane += pic->img.i_stride[plane];
>+            p_buffer += frame->linesize[plane];
>         }
>     }
> }
>-- 
>2.7.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".
Michael Niedermayer Nov. 29, 2019, 6:39 p.m. UTC | #2
On Mon, Oct 14, 2019 at 09:22:43PM +0800, hwren wrote:
> Optimize data access from multiplication to iteration.
> 
> Signed-off-by: hwren <hwrenx@126.com>
> ---
>  libavcodec/libxavs2.c | 45 +++++++++++++++++++++++++++++----------------
>  1 file changed, 29 insertions(+), 16 deletions(-)

I think the change is ok but the commit message is hard to understand

thx

[...]
hwren Nov. 30, 2019, 5:21 a.m. UTC | #3
At 2019-11-30 02:39:07, "Michael Niedermayer" <michael@niedermayer.cc> wrote:
>On Mon, Oct 14, 2019 at 09:22:43PM +0800, hwren wrote:
>> Optimize data access from multiplication to iteration.
>> 
>> Signed-off-by: hwren <hwrenx@126.com>
>> ---
>>  libavcodec/libxavs2.c | 45 +++++++++++++++++++++++++++++----------------
>>  1 file changed, 29 insertions(+), 16 deletions(-)
>
>I think the change is ok but the commit message is hard to understand
>

>thx


I'm not sure which way is better to describe this commit. What I did was use iterative addressing instead of multiplicative addressing. So, maybe it's better to use message like Optimize data addressingor like Use iterative addressing instead of multiplicative addressing? I would be appreciate if you could give me some suggestions.


Thanks,
Huiwen Ren


>
>[...]
>-- 
>Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
>Whats the most studid thing your enemy could do ? Blow himself up
>Whats the most studid thing you could do ? Give up your rights and
>freedom because your enemy blew himself up.
>
Michael Niedermayer Nov. 30, 2019, 9:43 p.m. UTC | #4
On Sat, Nov 30, 2019 at 01:21:25PM +0800, hwren wrote:
> 
> 
> 
> 
> 
> 
> 
> 
> 
> At 2019-11-30 02:39:07, "Michael Niedermayer" <michael@niedermayer.cc> wrote:
> >On Mon, Oct 14, 2019 at 09:22:43PM +0800, hwren wrote:
> >> Optimize data access from multiplication to iteration.
> >> 
> >> Signed-off-by: hwren <hwrenx@126.com>
> >> ---
> >>  libavcodec/libxavs2.c | 45 +++++++++++++++++++++++++++++----------------
> >>  1 file changed, 29 insertions(+), 16 deletions(-)
> >
> >I think the change is ok but the commit message is hard to understand
> >
> 
> >thx
> 
> 
> I'm not sure which way is better to describe this commit. What I did was use iterative addressing instead of multiplicative addressing. So, maybe it's better to use message like Optimize data addressingor like Use iterative addressing instead of multiplicative addressing? I would be appreciate if you could give me some suggestions.

you could have one commit whith a commit message like
"Use more descriptive variable names in xavs2_copy_frame*()"

And a 2nd commit with
"Avoid recomputing pointers and instead update them in xavs2_copy_frame*()"


Thanks

[...]
diff mbox

Patch

diff --git a/libavcodec/libxavs2.c b/libavcodec/libxavs2.c
index 0179a1e..b5c07ec 100644
--- a/libavcodec/libxavs2.c
+++ b/libavcodec/libxavs2.c
@@ -132,29 +132,42 @@  static av_cold int xavs2_init(AVCodecContext *avctx)
 
 static void xavs2_copy_frame_with_shift(xavs2_picture_t *pic, const AVFrame *frame, const int shift_in)
 {
-    int j, k;
-    for (k = 0; k < 3; k++) {
-        int i_stride = pic->img.i_stride[k];
-        for (j = 0; j < pic->img.i_lines[k]; j++) {
-            uint16_t *p_plane = (uint16_t *)&pic->img.img_planes[k][j * i_stride];
-            int i;
-            uint8_t *p_buffer = frame->data[k] + frame->linesize[k] * j;
-            memset(p_plane, 0, i_stride);
-            for (i = 0; i < pic->img.i_width[k]; i++) {
-                p_plane[i] = p_buffer[i] << shift_in;
+    uint16_t *p_plane;
+    uint8_t *p_buffer;
+    int wIdx;
+    int hIdx;
+    int plane;
+
+    for (plane = 0; plane < 3; ++plane) {
+        p_plane  = (uint16_t *)pic->img.img_planes[plane];
+        p_buffer = frame->data[plane];
+        for (hIdx = 0; hIdx < pic->img.i_lines[plane]; ++hIdx) {
+            memset(p_plane, 0, pic->img.i_stride[plane]);
+            for (wIdx = 0; wIdx < pic->img.i_width[plane]; ++wIdx) {
+                p_plane[wIdx] = p_buffer[wIdx] << shift_in;
             }
+            p_plane += pic->img.i_stride[plane];
+            p_buffer += frame->linesize[plane];
         }
     }
 }
 
 static void xavs2_copy_frame(xavs2_picture_t *pic, const AVFrame *frame)
 {
-    int j, k;
-    for (k = 0; k < 3; k++) {
-        for (j = 0; j < pic->img.i_lines[k]; j++) {
-            memcpy( pic->img.img_planes[k] + pic->img.i_stride[k] * j,
-                    frame->data[k]+frame->linesize[k] * j,
-                    pic->img.i_width[k] * pic->img.in_sample_size);
+    uint8_t *p_plane;
+    uint8_t *p_buffer;
+    int hIdx;
+    int plane;
+    int stride;
+
+    for (plane = 0; plane < 3; ++plane) {
+        p_plane  = pic->img.img_planes[plane];
+        p_buffer = frame->data[plane];
+        stride = pic->img.i_width[plane] * pic->img.in_sample_size;
+        for (hIdx = 0; hIdx < pic->img.i_lines[plane]; ++hIdx) {
+            memcpy(p_plane, p_buffer, stride);
+            p_plane += pic->img.i_stride[plane];
+            p_buffer += frame->linesize[plane];
         }
     }
 }