[FFmpeg-devel,v1] lavc/libdavs2.c: optimize frame copy

Submitted by hwren on July 3, 2019, 3:23 a.m.

Details

Message ID 1562124231-18586-1-git-send-email-hwrenx@126.com
State New
Headers show

Commit Message

hwren July 3, 2019, 3:23 a.m.
Signed-off-by: hwrenx <hwrenx@126.com>
---
 libavcodec/libdavs2.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Comments

mypopy@gmail.com July 3, 2019, 6:38 a.m.
On Wed, Jul 3, 2019 at 11:24 AM hwrenx <hwrenx@126.com> wrote:
>
> Signed-off-by: hwrenx <hwrenx@126.com>
> ---
>  libavcodec/libdavs2.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c
> index 218f3ec..15ed3a1 100644
> --- a/libavcodec/libdavs2.c
> +++ b/libavcodec/libdavs2.c
> @@ -62,7 +62,7 @@ static int davs2_dump_frames(AVCodecContext *avctx, davs2_picture_t *pic, int *g
>                               davs2_seq_info_t *headerset, int ret_type, AVFrame *frame)
>  {
>      DAVS2Context *cad    = avctx->priv_data;
> -    int bytes_per_sample = pic->bytes_per_sample;
> +    int bytes_per_sample = pic->bytes_per_sample == 8 ? 1 : 2;
Looks like davs2 use a wrong name for bits per sample, I think need to
fix in davs2 first, it's better than the workaround in the wrapper.
>      int plane = 0;
>      int line  = 0;
>
> @@ -104,6 +104,7 @@ static int davs2_dump_frames(AVCodecContext *avctx, davs2_picture_t *pic, int *g
>
>      for (plane = 0; plane < 3; ++plane) {
>          int size_line = pic->widths[plane] * bytes_per_sample;
> +        void *dst, *src;
>          frame->buf[plane]  = av_buffer_alloc(size_line * pic->lines[plane]);
>
>          if (!frame->buf[plane]){
> @@ -114,10 +115,14 @@ static int davs2_dump_frames(AVCodecContext *avctx, davs2_picture_t *pic, int *g
>          frame->data[plane]     = frame->buf[plane]->data;
>          frame->linesize[plane] = size_line;
>
> -        for (line = 0; line < pic->lines[plane]; ++line)
> -            memcpy(frame->data[plane] + line * size_line,
> -                   pic->planes[plane] + line * pic->strides[plane],
> -                   pic->widths[plane] * bytes_per_sample);
> +        dst = frame->data[plane];
> +        src = pic->planes[plane];
> +
> +        for (line = 0; line < pic->lines[plane]; ++line) {
> +            memcpy(dst, src, size_line);
> +            dst += size_line;
> +            src += pic->strides[plane];
> +        }
>      }
>
>      frame->width     = cad->headerset.width;
> --
> 2.7.4
>
Ruiling Song July 5, 2019, 1:09 a.m.
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of hwrenx

> Sent: Wednesday, July 3, 2019 11:24 AM

> To: ffmpeg-devel@ffmpeg.org

> Subject: [FFmpeg-devel] [PATCH v1] lavc/libdavs2.c: optimize frame copy

> 


I think it's better to use "correct ..." or "fix ..." instead of "optimize" in the title.
Maybe a short commit message here would be useful for reviewer.

> Signed-off-by: hwrenx <hwrenx@126.com>

> ---

>  libavcodec/libdavs2.c | 15 ++++++++++-----

>  1 file changed, 10 insertions(+), 5 deletions(-)

> 

> diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c

> index 218f3ec..15ed3a1 100644

> --- a/libavcodec/libdavs2.c

> +++ b/libavcodec/libdavs2.c

> @@ -62,7 +62,7 @@ static int davs2_dump_frames(AVCodecContext *avctx,

> davs2_picture_t *pic, int *g

>                               davs2_seq_info_t *headerset, int ret_type, AVFrame *frame)

>  {

>      DAVS2Context *cad    = avctx->priv_data;

> -    int bytes_per_sample = pic->bytes_per_sample;

> +    int bytes_per_sample = pic->bytes_per_sample == 8 ? 1 : 2;

>      int plane = 0;

>      int line  = 0;

> 

> @@ -104,6 +104,7 @@ static int davs2_dump_frames(AVCodecContext

> *avctx, davs2_picture_t *pic, int *g

> 

>      for (plane = 0; plane < 3; ++plane) {

>          int size_line = pic->widths[plane] * bytes_per_sample;

> +        void *dst, *src;

>          frame->buf[plane]  = av_buffer_alloc(size_line * pic->lines[plane]);

> 

>          if (!frame->buf[plane]){

> @@ -114,10 +115,14 @@ static int davs2_dump_frames(AVCodecContext

> *avctx, davs2_picture_t *pic, int *g

>          frame->data[plane]     = frame->buf[plane]->data;

>          frame->linesize[plane] = size_line;

> 


Did you observe performance difference with only below lines of change?
If it is just for code cleanup, it's better to split this into separate patch.

Thanks!
Ruiling
> -        for (line = 0; line < pic->lines[plane]; ++line)

> -            memcpy(frame->data[plane] + line * size_line,

> -                   pic->planes[plane] + line * pic->strides[plane],

> -                   pic->widths[plane] * bytes_per_sample);

> +        dst = frame->data[plane];

> +        src = pic->planes[plane];

> +

> +        for (line = 0; line < pic->lines[plane]; ++line) {

> +            memcpy(dst, src, size_line);

> +            dst += size_line;

> +            src += pic->strides[plane];

> +        }

>      }

> 

>      frame->width     = cad->headerset.width;

> --

> 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".
mypopy@gmail.com July 7, 2019, 10:21 a.m.
On Fri, Jul 5, 2019 at 9:09 AM Song, Ruiling <ruiling.song@intel.com> wrote:
>
> > -----Original Message-----
> > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf
> > Of hwrenx
> > Sent: Wednesday, July 3, 2019 11:24 AM
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: [FFmpeg-devel] [PATCH v1] lavc/libdavs2.c: optimize frame copy
> >
>
> I think it's better to use "correct ..." or "fix ..." instead of "optimize" in the title.
> Maybe a short commit message here would be useful for reviewer.
>
> > Signed-off-by: hwrenx <hwrenx@126.com>
> > ---
> >  libavcodec/libdavs2.c | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c
> > index 218f3ec..15ed3a1 100644
> > --- a/libavcodec/libdavs2.c
> > +++ b/libavcodec/libdavs2.c
> > @@ -62,7 +62,7 @@ static int davs2_dump_frames(AVCodecContext *avctx,
> > davs2_picture_t *pic, int *g
> >                               davs2_seq_info_t *headerset, int ret_type, AVFrame *frame)
> >  {
> >      DAVS2Context *cad    = avctx->priv_data;
> > -    int bytes_per_sample = pic->bytes_per_sample;
> > +    int bytes_per_sample = pic->bytes_per_sample == 8 ? 1 : 2;
> >      int plane = 0;
> >      int line  = 0;
> >
> > @@ -104,6 +104,7 @@ static int davs2_dump_frames(AVCodecContext
> > *avctx, davs2_picture_t *pic, int *g
> >
> >      for (plane = 0; plane < 3; ++plane) {
> >          int size_line = pic->widths[plane] * bytes_per_sample;
> > +        void *dst, *src;
> >          frame->buf[plane]  = av_buffer_alloc(size_line * pic->lines[plane]);
> >
> >          if (!frame->buf[plane]){
> > @@ -114,10 +115,14 @@ static int davs2_dump_frames(AVCodecContext
> > *avctx, davs2_picture_t *pic, int *g
> >          frame->data[plane]     = frame->buf[plane]->data;
> >          frame->linesize[plane] = size_line;
> >
>
> Did you observe performance difference with only below lines of change?
> If it is just for code cleanup, it's better to split this into separate patch.
>
After talked with hwrenx offline, I think hwrenx will use the other
way for this type
performance issue like libdav1d.c wrapper, it's will reduce the memory copy from
davs2 decoder to FFmpeg.
> Thanks!
> Ruiling
> > -        for (line = 0; line < pic->lines[plane]; ++line)
> > -            memcpy(frame->data[plane] + line * size_line,
> > -                   pic->planes[plane] + line * pic->strides[plane],
> > -                   pic->widths[plane] * bytes_per_sample);
> > +        dst = frame->data[plane];
> > +        src = pic->planes[plane];
> > +
> > +        for (line = 0; line < pic->lines[plane]; ++line) {
> > +            memcpy(dst, src, size_line);
> > +            dst += size_line;
> > +            src += pic->strides[plane];
> > +        }
> >      }
> >
> >      frame->width     = cad->headerset.width;
> > --
> > 2.7.4

Patch hide | download patch | download mbox

diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c
index 218f3ec..15ed3a1 100644
--- a/libavcodec/libdavs2.c
+++ b/libavcodec/libdavs2.c
@@ -62,7 +62,7 @@  static int davs2_dump_frames(AVCodecContext *avctx, davs2_picture_t *pic, int *g
                              davs2_seq_info_t *headerset, int ret_type, AVFrame *frame)
 {
     DAVS2Context *cad    = avctx->priv_data;
-    int bytes_per_sample = pic->bytes_per_sample;
+    int bytes_per_sample = pic->bytes_per_sample == 8 ? 1 : 2;
     int plane = 0;
     int line  = 0;
 
@@ -104,6 +104,7 @@  static int davs2_dump_frames(AVCodecContext *avctx, davs2_picture_t *pic, int *g
 
     for (plane = 0; plane < 3; ++plane) {
         int size_line = pic->widths[plane] * bytes_per_sample;
+        void *dst, *src;
         frame->buf[plane]  = av_buffer_alloc(size_line * pic->lines[plane]);
 
         if (!frame->buf[plane]){
@@ -114,10 +115,14 @@  static int davs2_dump_frames(AVCodecContext *avctx, davs2_picture_t *pic, int *g
         frame->data[plane]     = frame->buf[plane]->data;
         frame->linesize[plane] = size_line;
 
-        for (line = 0; line < pic->lines[plane]; ++line)
-            memcpy(frame->data[plane] + line * size_line,
-                   pic->planes[plane] + line * pic->strides[plane],
-                   pic->widths[plane] * bytes_per_sample);
+        dst = frame->data[plane];
+        src = pic->planes[plane];
+
+        for (line = 0; line < pic->lines[plane]; ++line) {
+            memcpy(dst, src, size_line);
+            dst += size_line;
+            src += pic->strides[plane];
+        }
     }
 
     frame->width     = cad->headerset.width;