[FFmpeg-devel] avcodec/utils: Optimize ff_color_frame()

Submitted by Michael Niedermayer on Jan. 4, 2019, 10:07 p.m.

Details

Message ID 20190104220731.15069-1-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer Jan. 4, 2019, 10:07 p.m.
Fixes: Timeout
Fixes: 10709/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-5630617975259136

Before change: Executed clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-5630617975259136 in 28285 ms
After change:  Executed clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-5630617975259136 in  9395 ms

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/utils.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

James Almer Jan. 4, 2019, 10:58 p.m.
On 1/4/2019 7:07 PM, Michael Niedermayer wrote:
> Fixes: Timeout
> Fixes: 10709/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-5630617975259136
> 
> Before change: Executed clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-5630617975259136 in 28285 ms
> After change:  Executed clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-5630617975259136 in  9395 ms
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/utils.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index d519b16092..963924a9ca 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -423,8 +423,12 @@ void ff_color_frame(AVFrame *frame, const int c[4])
>          int height = is_chroma ? AV_CEIL_RSHIFT(frame->height, desc->log2_chroma_h) : frame->height;
>          for (y = 0; y < height; y++) {
>              if (desc->comp[0].depth >= 9) {
> -                for (x = 0; x<bytes; x++)
> -                    ((uint16_t*)dst)[x] = c[p];
> +                if (!y) {
> +                    for (x = 0; x<bytes; x++)
> +                        ((uint16_t*)dst)[x] = c[p];
> +                } else {
> +                    memcpy(dst, frame->data[p], 2*bytes);

I think it would be cleaner if you instead rewrite the loop. Something like:

>     for (p = 0; p<desc->nb_components; p++) {
>         uint8_t *dst = frame->data[p];
>         int is_chroma = p == 1 || p == 2;
>         int hbd    = desc->comp[0].depth >= 9;
>         int bytes  = is_chroma ? AV_CEIL_RSHIFT(frame->width,  desc->log2_chroma_w) : frame->width;
>         int height = is_chroma ? AV_CEIL_RSHIFT(frame->height, desc->log2_chroma_h) : frame->height;
>         if (hbd) {
>             for (x = 0; x < bytes; x++)
>                 ((uint16_t*)dst)[x] = c[p];
>         } else
>             memset(dst, c[p], bytes);
>         bytes <<= hbd;
>         for (y = 1; y < height; y++) {
>             memcpy(dst, frame->data[p], bytes);
>             dst += frame->linesize[p];
>         }
>     }

Not sure if replacing the memset with memcpy for the 8bit case is faster
or not, but the removal of the branch should help in any case.

Patch hide | download patch | download mbox

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index d519b16092..963924a9ca 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -423,8 +423,12 @@  void ff_color_frame(AVFrame *frame, const int c[4])
         int height = is_chroma ? AV_CEIL_RSHIFT(frame->height, desc->log2_chroma_h) : frame->height;
         for (y = 0; y < height; y++) {
             if (desc->comp[0].depth >= 9) {
-                for (x = 0; x<bytes; x++)
-                    ((uint16_t*)dst)[x] = c[p];
+                if (!y) {
+                    for (x = 0; x<bytes; x++)
+                        ((uint16_t*)dst)[x] = c[p];
+                } else {
+                    memcpy(dst, frame->data[p], 2*bytes);
+                }
             }else
                 memset(dst, c[p], bytes);
             dst += frame->linesize[p];