diff mbox

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

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

Commit Message

Michael Niedermayer Jan. 4, 2019, 10:07 p.m. UTC
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. UTC | #1
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.
diff mbox

Patch

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];