| Message ID | 20230610183109.24802-2-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | c012d1f2bb8735f2b17ce88cd8181d2ffc989b02 |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/jpeg2000dec: Check for reduction factor and image offset | expand |
| 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 |
lör 2023-06-10 klockan 20:31 +0200 skrev Michael Niedermayer: > jpeg2000 overrides the global lowres variable with a lowres field > called reduction_factor > ffmpeg -lowres X causes the reduction_factor to be set > ffplay -lowres X causes both lowres and the reduction_factor to be > set > ossfuss sets only lowres Why does jpeg2000dec have its own -lowres? Does lavc reset it somewhere to where the decoder can't make use of the avctx-level setting? /Tomas
On Sat, Jun 24, 2023 at 12:53 PM Tomas Härdin <git@haerdin.se> wrote: > lör 2023-06-10 klockan 20:31 +0200 skrev Michael Niedermayer: > > jpeg2000 overrides the global lowres variable with a lowres field > > called reduction_factor > > ffmpeg -lowres X causes the reduction_factor to be set > > ffplay -lowres X causes both lowres and the reduction_factor to be > > set > > ossfuss sets only lowres > > Why does jpeg2000dec have its own -lowres? Does lavc reset it somewhere > to where the decoder can't make use of the avctx-level setting? > > Libav removed lowres from avctx, but same was not done here. IMHO lowres is codec-specific thing and only codecs supporting it should have private option to change it. > /Tomas > _______________________________________________ > 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". >
lör 2023-06-24 klockan 19:54 +0200 skrev Paul B Mahol: > On Sat, Jun 24, 2023 at 12:53 PM Tomas Härdin <git@haerdin.se> wrote: > > > lör 2023-06-10 klockan 20:31 +0200 skrev Michael Niedermayer: > > > jpeg2000 overrides the global lowres variable with a lowres field > > > called reduction_factor > > > ffmpeg -lowres X causes the reduction_factor to be set > > > ffplay -lowres X causes both lowres and the reduction_factor to > > > be > > > set > > > ossfuss sets only lowres > > > > Why does jpeg2000dec have its own -lowres? Does lavc reset it > > somewhere > > to where the decoder can't make use of the avctx-level setting? > > > > > Libav removed lowres from avctx, but same was not done here. > > IMHO lowres is codec-specific thing and only codecs supporting it > should > have private option to change it. Enough codecs support lowres that marking support for it in codec caps may be worthwhile. Something to consider for the future. /Tomas
On 6/24/2023 5:46 PM, Tomas Härdin wrote: > lör 2023-06-24 klockan 19:54 +0200 skrev Paul B Mahol: >> On Sat, Jun 24, 2023 at 12:53 PM Tomas Härdin <git@haerdin.se> wrote: >> >>> lör 2023-06-10 klockan 20:31 +0200 skrev Michael Niedermayer: >>>> jpeg2000 overrides the global lowres variable with a lowres field >>>> called reduction_factor >>>> ffmpeg -lowres X causes the reduction_factor to be set >>>> ffplay -lowres X causes both lowres and the reduction_factor to >>>> be >>>> set >>>> ossfuss sets only lowres >>> >>> Why does jpeg2000dec have its own -lowres? Does lavc reset it >>> somewhere >>> to where the decoder can't make use of the avctx-level setting? >>> >>> >> Libav removed lowres from avctx, but same was not done here. >> >> IMHO lowres is codec-specific thing and only codecs supporting it >> should >> have private option to change it. > > Enough codecs support lowres that marking support for it in codec caps > may be worthwhile. Something to consider for the future. There's AVCodec.max_lowres, which when not zero it means the decoder supports lowres. > > /Tomas > _______________________________________________ > 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".
On Sat, Jun 10, 2023 at 08:31:09PM +0200, Michael Niedermayer wrote: > jpeg2000 overrides the global lowres variable with a lowres field called reduction_factor > ffmpeg -lowres X causes the reduction_factor to be set > ffplay -lowres X causes both lowres and the reduction_factor to be set > ossfuss sets only lowres > > only the ffmpeg variant works. This patch tries to make the other 2 work. > > Alternative we could just error out if things are inconsistent. > More complex restructuring should be limited to the master branch > to keep this reasonably easy to backport > > Fixes: out of array access > Fixes: 59672/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/jpeg2000dec.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) if there are no objections then i plan to apply this in a few days [...]
diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c index d6f2a5938e..dc484344ab 100644 --- a/libavcodec/jpeg2000dec.c +++ b/libavcodec/jpeg2000dec.c @@ -308,7 +308,7 @@ static int get_siz(Jpeg2000DecoderContext *s) dimy = FFMAX(dimy, ff_jpeg2000_ceildiv(o_dimy, s->cdy[i])); } - ret = ff_set_dimensions(s->avctx, dimx, dimy); + ret = ff_set_dimensions(s->avctx, dimx << s->avctx->lowres, dimy << s->avctx->lowres); if (ret < 0) return ret; @@ -2426,6 +2426,14 @@ static av_cold int jpeg2000_decode_init(AVCodecContext *avctx) { Jpeg2000DecoderContext *s = avctx->priv_data; + if (avctx->lowres) + av_log(avctx, AV_LOG_WARNING, "lowres is overriden by reduction_factor but set anyway\n"); + if (!s->reduction_factor && avctx->lowres < JPEG2000_MAX_RESLEVELS) { + s->reduction_factor = avctx->lowres; + } + if (avctx->lowres != s->reduction_factor && avctx->lowres) + return AVERROR(EINVAL); + ff_jpeg2000dsp_init(&s->dsp); ff_jpeg2000_init_tier1_luts();
jpeg2000 overrides the global lowres variable with a lowres field called reduction_factor ffmpeg -lowres X causes the reduction_factor to be set ffplay -lowres X causes both lowres and the reduction_factor to be set ossfuss sets only lowres only the ffmpeg variant works. This patch tries to make the other 2 work. Alternative we could just error out if things are inconsistent. More complex restructuring should be limited to the master branch to keep this reasonably easy to backport Fixes: out of array access Fixes: 59672/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/jpeg2000dec.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)