[FFmpeg-devel] [PATCH 1/2] avcodec/jpeg2000dec: Check for reduction factor and image offset

[FFmpeg-devel] [PATCH 1/2] avcodec/jpeg2000dec: Check for reduction factor and image offset

[Michael Niedermayer]

This combination is not working (it writes out of array)

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/jpeg2000dec.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index 0e95cca64e..d6f2a5938e 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -238,6 +238,11 @@ static int get_siz(Jpeg2000DecoderContext *s)
         return AVERROR_INVALIDDATA;
     }
 
+    if (s->reduction_factor && (s->image_offset_x || s->image_offset_y) ){
+        av_log(s->avctx, AV_LOG_ERROR, "reduction factor with image offsets is not fully implemented");
+        return AVERROR_PATCHWELCOME;
+    }
+
     s->ncomponents = ncomponents;
 
     if (s->tile_width <= 0 || s->tile_height <= 0) {
-- 
2.17.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/2] avcodec/jpeg2000dec: jpeg2000 has its own lowres option

[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

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(-)

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();
 
-- 
2.17.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/jpeg2000dec: jpeg2000 has its own lowres option

[Tomas Härdin]

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
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/jpeg2000dec: jpeg2000 has its own lowres option

[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.


> /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".
>
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/jpeg2000dec: jpeg2000 has its own lowres option

[Tomas Härdin]

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
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/jpeg2000dec: jpeg2000 has its own lowres option

[James Almer]

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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/jpeg2000dec: jpeg2000 has its own lowres option

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1204 bytes --]

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

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

Never trust a computer, one day, it may think you are the virus. -- Compn

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/jpeg2000dec: Check for reduction factor and image offset

[Tomas Härdin]

lör 2023-06-10 klockan 20:31 +0200 skrev Michael Niedermayer:
> This combination is not working (it writes out of array)

Looks OK for now

/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".

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/jpeg2000dec: Check for reduction factor and image offset

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 505 bytes --]

On Sat, Jun 24, 2023 at 12:50:27PM +0200, Tomas Härdin wrote:
> lör 2023-06-10 klockan 20:31 +0200 skrev Michael Niedermayer:
> > This combination is not working (it writes out of array)
> 
> Looks OK for now

will apply

thx

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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".