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

James Almer jamrial at gmail.com
Sat Jan 5 00:58:44 EET 2019


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 at 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.


More information about the ffmpeg-devel mailing list