[FFmpeg-devel] [PATCH] avcodec/rl2: set dimensions

[Michael Niedermayer]

On Wed, Jul 24, 2019 at 01:29:26AM +0200, Reimar Döffinger wrote:
> 
> 
> On 22.07.2019, at 23:57, Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > The dimensions are always 320x200 they are hardcoded in the demuxer.
> > Hardcode them instead in the decoder.
> > 
> > Fixes: Timeout (16sec -> 400ms)
> > Fixes: 15574/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RL2_fuzzer-5158614072819712
> > 
> > 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/rl2.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> > 
> > diff --git a/libavcodec/rl2.c b/libavcodec/rl2.c
> > index 6662979c52..2d336a61e5 100644
> > --- a/libavcodec/rl2.c
> > +++ b/libavcodec/rl2.c
> > @@ -134,10 +134,15 @@ static av_cold int rl2_decode_init(AVCodecContext *avctx)
> >     Rl2Context *s = avctx->priv_data;
> >     int back_size;
> >     int i;
> > +    int ret;
> > 
> >     s->avctx       = avctx;
> >     avctx->pix_fmt = AV_PIX_FMT_PAL8;
> > 
> > +    ret = ff_set_dimensions(avctx, 320, 200);
> > +    if (ret < 0)
> > +        return ret;
> 
> I really dislike these completely comment-less patches.
> So it seems this is only based on what our demuxer does.

No, the only specification available to me:
https://wiki.multimedia.cx/index.php/RL2
says this:
"Video Decoding
 The video is always encoded in 320x200 resolution." 

I will add this to the commit message of course in case no other
objections remain

 
> However does the format itself have any inherent size limitations?

yes, it conatins a video_base parameter which is 16bit and indexes into
the width*height array of pixels. suggesting that a size around 320x200
is the intended maximum which would use that parameter.


> What was the cause of the slow decoding? Does this actually fix it, or does it just swipe it "under the carpet"?

The cause of slow decoding was a huge input resolution, precissely 64768x255
that also is far from the worst possible


> If someone ever found a sample with a different solution, how would they know that they shouldn't just remove this again? Without any kind of comment on the point of this call, people might assume it's pointless nonsense.

if some rl2 "2.0" is found which does store a resolution
and if that is reverse engeneered (these being obviously pre-requirements)
then the removial of the hardcoded size is probably not completely unreasonable

Thanks


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190724/0f7b7d55/attachment.sig>