diff mbox series

[FFmpeg-devel,1/6] avformat/jpegxl_anim_dec: Perform operations in a different order

Message ID 20230618215021.3044-1-michael@niedermayer.cc
State Accepted
Commit 9fc141f32d52099a6bfeba6e4e7ee2ce7fadc833
Headers show
Series [FFmpeg-devel,1/6] avformat/jpegxl_anim_dec: Perform operations in a different order | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Michael Niedermayer June 18, 2023, 9:50 p.m. UTC
Fixes: OOM
Fixes: 59802/clusterfuzz-testcase-minimized-ffmpeg_dem_JPEGXL_ANIM_fuzzer-5681765466112000

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/jpegxl_anim_dec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Lynne June 18, 2023, 9:55 p.m. UTC | #1
Jun 18, 2023, 23:50 by michael@niedermayer.cc:

> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  doc/developer.texi | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index a7da2ce2d5..0c2f2cd7d1 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -805,7 +805,10 @@ Lines with similar content should be aligned vertically when doing so
>  improves readability.
>  
>  @item
> -Consider adding a regression test for your code.
> +Consider adding a regression test for your code. All new modules
> +should be covered by tests. That includes demuxers, muxers, decoders, encoders
> +filters, bitstream filters, parsers. If its not possible to do that, add
> +an explanation why to your patchset, its ok to not test if theres a reason.
>

Could you add assembly code to this as well?
checkasm is super useful, but is currently lacking quite
a few tests.
Leo Izen June 18, 2023, 9:56 p.m. UTC | #2
On 6/18/23 17:50, Michael Niedermayer wrote:
> Fixes: OOM
> Fixes: 59802/clusterfuzz-testcase-minimized-ffmpeg_dem_JPEGXL_ANIM_fuzzer-5681765466112000
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavformat/jpegxl_anim_dec.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/jpegxl_anim_dec.c b/libavformat/jpegxl_anim_dec.c
> index 6ea6c46d8f..c62b596f76 100644
> --- a/libavformat/jpegxl_anim_dec.c
> +++ b/libavformat/jpegxl_anim_dec.c
> @@ -227,7 +227,7 @@ static int jpegxl_anim_read_packet(AVFormatContext *s, AVPacket *pkt)
>       if (ctx->initial && size < ctx->initial->size)
>           size = ctx->initial->size;
>   
> -    if ((ret = av_new_packet(pkt, size) < 0))
> +    if ((ret = av_new_packet(pkt, size)) < 0)
>           return ret;
>   
>       if (ctx->initial) {

Pushed the first patch, I maintain this code and it LGTM. I left others 
pending.

- Leo Izen
Anton Khirnov June 19, 2023, 4:08 p.m. UTC | #3
Quoting Lynne (2023-06-18 23:55:56)
> Jun 18, 2023, 23:50 by michael@niedermayer.cc:
> 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  doc/developer.texi | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index a7da2ce2d5..0c2f2cd7d1 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -805,7 +805,10 @@ Lines with similar content should be aligned vertically when doing so
> >  improves readability.
> >  
> >  @item
> > -Consider adding a regression test for your code.
> > +Consider adding a regression test for your code. All new modules
> > +should be covered by tests. That includes demuxers, muxers, decoders, encoders
> > +filters, bitstream filters, parsers. If its not possible to do that, add
> > +an explanation why to your patchset, its ok to not test if theres a reason.
> >
> 
> Could you add assembly code to this as well?
> checkasm is super useful, but is currently lacking quite
> a few tests.

The SIMD/DSP section already says new asm should have tests.
I would be in favor of making that into 'must' (unless very good reason
otherwise).
diff mbox series

Patch

diff --git a/libavformat/jpegxl_anim_dec.c b/libavformat/jpegxl_anim_dec.c
index 6ea6c46d8f..c62b596f76 100644
--- a/libavformat/jpegxl_anim_dec.c
+++ b/libavformat/jpegxl_anim_dec.c
@@ -227,7 +227,7 @@  static int jpegxl_anim_read_packet(AVFormatContext *s, AVPacket *pkt)
     if (ctx->initial && size < ctx->initial->size)
         size = ctx->initial->size;
 
-    if ((ret = av_new_packet(pkt, size) < 0))
+    if ((ret = av_new_packet(pkt, size)) < 0)
         return ret;
 
     if (ctx->initial) {