| Message ID | HE1PR0301MB21544D0BFBC271E203CB0ED68F769@HE1PR0301MB2154.eurprd03.prod.outlook.com |
|---|---|
| State | Accepted |
| Commit | d393c45051ddaf6146e7e29ec2ea97035a727529 |
| Headers | show |
| Series | [FFmpeg-devel,1/9] avcodec/encode: Fix check for allowed LJPEG pixel formats | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On Tue, Apr 06, 2021 at 11:55:09PM +0200, Andreas Rheinhardt wrote: > The (deprecated) field AVCodecContext.mpeg_quant has no range > restriction; MpegEncContext.mpeg_quant is restricted to 0..1. > If the former is set, the latter is overwritten with it without > checking the range. This can trigger an av_assert2() with the MPEG-4 > encoder when writing said field. > > Fix this by just setting MpegEncContext.mpeg_quant to 1 if > AVCodecContext.mpeg_quant is set. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > This can not be triggered if one only sets options via the dictionary in > avcodec_open2(); one needs to set it directly in the AVCodecContext or > use the private class of AVCodecContext. I tend toward checking mpeg_quant to be valid in init somewhere but this here is fine too or in addition thx [...]
Michael Niedermayer: > On Tue, Apr 06, 2021 at 11:55:09PM +0200, Andreas Rheinhardt wrote: >> The (deprecated) field AVCodecContext.mpeg_quant has no range >> restriction; MpegEncContext.mpeg_quant is restricted to 0..1. >> If the former is set, the latter is overwritten with it without >> checking the range. This can trigger an av_assert2() with the MPEG-4 >> encoder when writing said field. >> >> Fix this by just setting MpegEncContext.mpeg_quant to 1 if >> AVCodecContext.mpeg_quant is set. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> This can not be triggered if one only sets options via the dictionary in >> avcodec_open2(); one needs to set it directly in the AVCodecContext or >> use the private class of AVCodecContext. > > I tend toward checking mpeg_quant to be valid in init somewhere but this > here is fine too or in addition > Do you want to error out if AVCodecContext.mpeg_quant is outside of 0..1? Or do you want another check for MpegEncContext.mpeg_quant? The option for the latter already uses the proper range, so I don't think we need a check for that. - Andreas
On Wed, Apr 07, 2021 at 11:07:54PM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > On Tue, Apr 06, 2021 at 11:55:09PM +0200, Andreas Rheinhardt wrote: > >> The (deprecated) field AVCodecContext.mpeg_quant has no range > >> restriction; MpegEncContext.mpeg_quant is restricted to 0..1. > >> If the former is set, the latter is overwritten with it without > >> checking the range. This can trigger an av_assert2() with the MPEG-4 > >> encoder when writing said field. > >> > >> Fix this by just setting MpegEncContext.mpeg_quant to 1 if > >> AVCodecContext.mpeg_quant is set. > >> > >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > >> --- > >> This can not be triggered if one only sets options via the dictionary in > >> avcodec_open2(); one needs to set it directly in the AVCodecContext or > >> use the private class of AVCodecContext. > > > > I tend toward checking mpeg_quant to be valid in init somewhere but this > > here is fine too or in addition > > > Do you want to error out if AVCodecContext.mpeg_quant is outside of > 0..1? that was the idea, i did not test this though > Or do you want another check for MpegEncContext.mpeg_quant? The > option for the latter already uses the proper range, so I don't think we > need a check for that. > > - Andreas > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Michael Niedermayer: > On Wed, Apr 07, 2021 at 11:07:54PM +0200, Andreas Rheinhardt wrote: >> Michael Niedermayer: >>> On Tue, Apr 06, 2021 at 11:55:09PM +0200, Andreas Rheinhardt wrote: >>>> The (deprecated) field AVCodecContext.mpeg_quant has no range >>>> restriction; MpegEncContext.mpeg_quant is restricted to 0..1. >>>> If the former is set, the latter is overwritten with it without >>>> checking the range. This can trigger an av_assert2() with the MPEG-4 >>>> encoder when writing said field. >>>> >>>> Fix this by just setting MpegEncContext.mpeg_quant to 1 if >>>> AVCodecContext.mpeg_quant is set. >>>> >>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >>>> --- >>>> This can not be triggered if one only sets options via the dictionary in >>>> avcodec_open2(); one needs to set it directly in the AVCodecContext or >>>> use the private class of AVCodecContext. >>> >>> I tend toward checking mpeg_quant to be valid in init somewhere but this >>> here is fine too or in addition >>> >> Do you want to error out if AVCodecContext.mpeg_quant is outside of >> 0..1? > > that was the idea, i did not test this though > I think it's not worth it for an option that will be removed on the next bump (AVCodecContext.mpeg_quant is deprecated). > >> Or do you want another check for MpegEncContext.mpeg_quant? The >> option for the latter already uses the proper range, so I don't think we >> need a check for that. >> >> - Andreas
On Thu, Apr 08, 2021 at 12:17:04AM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > On Wed, Apr 07, 2021 at 11:07:54PM +0200, Andreas Rheinhardt wrote: > >> Michael Niedermayer: > >>> On Tue, Apr 06, 2021 at 11:55:09PM +0200, Andreas Rheinhardt wrote: > >>>> The (deprecated) field AVCodecContext.mpeg_quant has no range > >>>> restriction; MpegEncContext.mpeg_quant is restricted to 0..1. > >>>> If the former is set, the latter is overwritten with it without > >>>> checking the range. This can trigger an av_assert2() with the MPEG-4 > >>>> encoder when writing said field. > >>>> > >>>> Fix this by just setting MpegEncContext.mpeg_quant to 1 if > >>>> AVCodecContext.mpeg_quant is set. > >>>> > >>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > >>>> --- > >>>> This can not be triggered if one only sets options via the dictionary in > >>>> avcodec_open2(); one needs to set it directly in the AVCodecContext or > >>>> use the private class of AVCodecContext. > >>> > >>> I tend toward checking mpeg_quant to be valid in init somewhere but this > >>> here is fine too or in addition > >>> > >> Do you want to error out if AVCodecContext.mpeg_quant is outside of > >> 0..1? > > > > that was the idea, i did not test this though > > > > I think it's not worth it for an option that will be removed on the next > bump (AVCodecContext.mpeg_quant is deprecated). ok then forget about the idea [...]
diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c index d1f2460409..0a1d0db86d 100644 --- a/libavcodec/mpegvideo_enc.c +++ b/libavcodec/mpegvideo_enc.c @@ -564,7 +564,7 @@ FF_ENABLE_DEPRECATION_WARNINGS #if FF_API_PRIVATE_OPT FF_DISABLE_DEPRECATION_WARNINGS if (avctx->mpeg_quant) - s->mpeg_quant = avctx->mpeg_quant; + s->mpeg_quant = 1; FF_ENABLE_DEPRECATION_WARNINGS #endif
The (deprecated) field AVCodecContext.mpeg_quant has no range restriction; MpegEncContext.mpeg_quant is restricted to 0..1. If the former is set, the latter is overwritten with it without checking the range. This can trigger an av_assert2() with the MPEG-4 encoder when writing said field. Fix this by just setting MpegEncContext.mpeg_quant to 1 if AVCodecContext.mpeg_quant is set. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- This can not be triggered if one only sets options via the dictionary in avcodec_open2(); one needs to set it directly in the AVCodecContext or use the private class of AVCodecContext. libavcodec/mpegvideo_enc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)