[FFmpeg-devel] [PATCH 1/2] avcodec/mpegvideo_enc: Check FLV1 resolution limits

[Michael Niedermayer]

Hi

On Sun, Jan 12, 2025 at 12:31:53PM +0100, Alexander Strasser via ffmpeg-devel wrote:
> On 2025-01-08 05:13 +0100, Michael Niedermayer wrote:
> > Found-by: Elias Myllymäki <elias.myllymaki04p at gmail.com>
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/mpegvideo_enc.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> > index c5f20c2d855..09df2f52a6c 100644
> > --- a/libavcodec/mpegvideo_enc.c
> > +++ b/libavcodec/mpegvideo_enc.c
> > @@ -567,6 +567,12 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
> >          av_log(avctx, AV_LOG_ERROR, "H.263 does not support resolutions above 2048x1152\n");
> >          return AVERROR(EINVAL);
> >      }
> > +    if (s->codec_id == AV_CODEC_ID_FLV1 &&
> > +        (avctx->width  > 65535 ||
> > +         avctx->height > 65535 )) {
> > +        av_log(avctx, AV_LOG_ERROR, "FLV does not support resolutions above 16bit\n");
> > +        return AVERROR(EINVAL);
> > +    }
> >      if ((s->codec_id == AV_CODEC_ID_H263  ||
> >           s->codec_id == AV_CODEC_ID_H263P ||
> >           s->codec_id == AV_CODEC_ID_RV20) &&
> > --
> 
> LGTM

will apply


> 
> Just out of curiosity, did this just produce broken video streams
> before this patch?

it produced broken streams but there was also a crash later in a filter
which i was not able to reproduce.


> 
> Something else only a little related to this patch, but somehow
> to this one and all that came before. As this is in ff_mpv_encode_init
> and there are dozens of checks like this and the whole function
> is around 600 lines, it would be good to refactor this into a
> more readable and potentially stronger form.

yes

ideally the whole block based DCT + MC code should be refactored
into a more modular encoder architecture.
Long ago i wanted to do that but noone was willing to help back then

Nowadays alot of factoring has been done but the code has not become
more generic. In fact its more hardcoded to 8x8 than before i think

Thing is if you change blocks to 4x4 the overall flow is still the
same. It would be nice if the whole was less rigid and could handle
a wider range of codecs. And if things where nicely modularized so
that would actually also look clean and pretty,

For now, one could try to factor the checks, though its a bit tricky
due to inheritance between codecs. (there is a lot of shared checks
as many of the propriatary codecs where based on open codecs ...

So factorizing all this cleanly would have to likely also follow
this sort of inheritance of ideas

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

z(9) = an object that transcends all computable functions describable
in finite terms. - ChatGPT in 2024
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250113/4cc6f528/attachment.sig>