diff mbox series

[FFmpeg-devel,RFC] libavcodec/jpeg2000: Make corrections jpeg2000 decoder

Message ID 20200618182520.5382-1-gautamramk@gmail.com
State New
Headers show
Series [FFmpeg-devel,RFC] libavcodec/jpeg2000: Make corrections jpeg2000 decoder
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Gautam Ramakrishnan June 18, 2020, 6:25 p.m. UTC
From: Gautam Ramakrishnan <gautamramk@gmail.com>

This is with reference to my previous email on the mailing list
with subject: "query on pixel formats".
I wish to cleanup some errors in the decoder code. These changes
would allow the samples p1_01.j2k and p1_07.j2k to be decoded.
However, I am facing issues with pixel format selection and have
currently forced the pixel formats to demonstrate the changes made.
Would be grateful if anyone could suggest modifications to the pix
format selection.
---
 libavcodec/jpeg2000.c    |  3 ---
 libavcodec/jpeg2000dec.c | 53 ++++++++++++++++++++++++++--------------
 2 files changed, 34 insertions(+), 22 deletions(-)

Comments

Carl Eugen Hoyos June 18, 2020, 8:03 p.m. UTC | #1
Am Do., 18. Juni 2020 um 21:50 Uhr schrieb <gautamramk@gmail.com>:
>
> From: Gautam Ramakrishnan <gautamramk@gmail.com>
>
> This is with reference to my previous email on the mailing list
> with subject: "query on pixel formats".
> I wish to cleanup some errors in the decoder code. These changes
> would allow the samples p1_01.j2k and p1_07.j2k to be decoded.
> However, I am facing issues with pixel format selection and have
> currently forced the pixel formats to demonstrate the changes made.
> Would be grateful if anyone could suggest modifications to the pix
> format selection.

The patch looks to me as if it should be split but maybe I
misunderstand and the changes are strongly related?

> ---
>  libavcodec/jpeg2000.c    |  3 ---
>  libavcodec/jpeg2000dec.c | 53 ++++++++++++++++++++++++++--------------
>  2 files changed, 34 insertions(+), 22 deletions(-)
>
> diff --git a/libavcodec/jpeg2000.c b/libavcodec/jpeg2000.c
> index 73206d17f3..1aca31ffa4 100644
> --- a/libavcodec/jpeg2000.c
> +++ b/libavcodec/jpeg2000.c
> @@ -509,9 +509,6 @@ int ff_jpeg2000_init_component(Jpeg2000Component *comp,
>          // update precincts size: 2^n value
>          reslevel->log2_prec_width  = codsty->log2_prec_widths[reslevelno];
>          reslevel->log2_prec_height = codsty->log2_prec_heights[reslevelno];

> -        if (!reslevel->log2_prec_width || !reslevel->log2_prec_height) {
> -            return AVERROR_INVALIDDATA;
> -        }

Why exactly is this a good idea?

>
>          /* Number of bands for each resolution level */
>          if (reslevelno == 0)
> diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> index ab36009a2d..ae63c68ca8 100644
> --- a/libavcodec/jpeg2000dec.c
> +++ b/libavcodec/jpeg2000dec.c
> @@ -269,6 +269,7 @@ static int get_siz(Jpeg2000DecoderContext *s)
>      const enum AVPixelFormat *possible_fmts = NULL;
>      int possible_fmts_nb = 0;
>      int ret;
> +    int dimx, dimy;
>
>      if (bytestream2_get_bytes_left(&s->g) < 36) {
>          av_log(s->avctx, AV_LOG_ERROR, "Insufficient space for SIZ\n");
> @@ -286,10 +287,6 @@ static int get_siz(Jpeg2000DecoderContext *s)
>      s->tile_offset_y  = bytestream2_get_be32u(&s->g); // YT0Siz
>      ncomponents       = bytestream2_get_be16u(&s->g); // CSiz

> -    if (s->image_offset_x || s->image_offset_y) {
> -        avpriv_request_sample(s->avctx, "Support for image offsets");
> -        return AVERROR_PATCHWELCOME;
> -    }

I would have expected this change to be part of a patch "support
image offsets".

>      if (av_image_check_size2(s->width, s->height, s->avctx->max_pixels, AV_PIX_FMT_NONE, 0, s->avctx)) {
>          avpriv_request_sample(s->avctx, "Large Dimensions");
>          return AVERROR_PATCHWELCOME;
> @@ -371,11 +368,18 @@ static int get_siz(Jpeg2000DecoderContext *s)
>      }
>
>      /* compute image size with reduction factor */
> -    ret = ff_set_dimensions(s->avctx,
> -            ff_jpeg2000_ceildivpow2(s->width  - s->image_offset_x,
> -                                               s->reduction_factor),
> -            ff_jpeg2000_ceildivpow2(s->height - s->image_offset_y,
> -                                               s->reduction_factor));
> +    dimx = ff_jpeg2000_ceildivpow2(s->width  - s->image_offset_x,
> +                                               s->reduction_factor);
> +    dimy = ff_jpeg2000_ceildivpow2(s->height - s->image_offset_y,
> +                                               s->reduction_factor);
> +    dimx = ff_jpeg2000_ceildiv(dimx, s->cdx[0]);
> +    dimy = ff_jpeg2000_ceildiv(dimy, s->cdy[0]);
> +    for (i = 1; i < s->ncomponents; i++) {
> +        dimx = FFMAX(dimx, ff_jpeg2000_ceildiv(dimx, s->cdx[i]));
> +        dimy = FFMAX(dimy, ff_jpeg2000_ceildiv(dimy, s->cdy[i]));
> +    }
> +
> +    ret = ff_set_dimensions(s->avctx, dimx, dimy);
>      if (ret < 0)
>          return ret;
>
> @@ -427,6 +431,13 @@ static int get_siz(Jpeg2000DecoderContext *s)
>                  s->cdef[3] = 3;
>                  i = 0;
>              }

> +        } else if (ncomponents == 2) {
> +            s->avctx->pix_fmt = AV_PIX_FMT_YA8;
> +            i = 0;

I am not convinced that this is a good idea:
Why is this not detected, what is the sample and most important:
What happens if the two components have different subsampling?

> +        } else if (ncomponents == 1) {
> +            s->avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> +            i = 0;
> +            s->cdef[0] = 0;

This may be ok but why is it not detected in the existing code?

I cannot comment on the other changes.

Carl Eugen
Gautam Ramakrishnan June 19, 2020, 2:40 a.m. UTC | #2
On Fri, Jun 19, 2020 at 1:34 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> Am Do., 18. Juni 2020 um 21:50 Uhr schrieb <gautamramk@gmail.com>:
> >
> > From: Gautam Ramakrishnan <gautamramk@gmail.com>
> >
> > This is with reference to my previous email on the mailing list
> > with subject: "query on pixel formats".
> > I wish to cleanup some errors in the decoder code. These changes
> > would allow the samples p1_01.j2k and p1_07.j2k to be decoded.
> > However, I am facing issues with pixel format selection and have
> > currently forced the pixel formats to demonstrate the changes made.
> > Would be grateful if anyone could suggest modifications to the pix
> > format selection.
>
> The patch looks to me as if it should be split but maybe I
> misunderstand and the changes are strongly related?
>
Yes, it will have to split. However, I just wanted to get a few
comments first regarding the
changes. Especially with respect to the colour format.
> > ---
> >  libavcodec/jpeg2000.c    |  3 ---
> >  libavcodec/jpeg2000dec.c | 53 ++++++++++++++++++++++++++--------------
> >  2 files changed, 34 insertions(+), 22 deletions(-)
> >
> > diff --git a/libavcodec/jpeg2000.c b/libavcodec/jpeg2000.c
> > index 73206d17f3..1aca31ffa4 100644
> > --- a/libavcodec/jpeg2000.c
> > +++ b/libavcodec/jpeg2000.c
> > @@ -509,9 +509,6 @@ int ff_jpeg2000_init_component(Jpeg2000Component *comp,
> >          // update precincts size: 2^n value
> >          reslevel->log2_prec_width  = codsty->log2_prec_widths[reslevelno];
> >          reslevel->log2_prec_height = codsty->log2_prec_heights[reslevelno];
>
> > -        if (!reslevel->log2_prec_width || !reslevel->log2_prec_height) {
> > -            return AVERROR_INVALIDDATA;
> > -        }
>
> Why exactly is this a good idea?
Firstly, even if log2_prec_width or log2_prec_height are 0, we will
not face any division by 0
issues as it is an exponent. 2^0 would give us 1. This check prevents
decoding of p1_07.j2k
and I believe it is wrong. This condition is handled in the COD marker
itself and permits the
first component to have these values as 0. This condition could be
changed to check if the
values are positive.
>
> >
> >          /* Number of bands for each resolution level */
> >          if (reslevelno == 0)
> > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> > index ab36009a2d..ae63c68ca8 100644
> > --- a/libavcodec/jpeg2000dec.c
> > +++ b/libavcodec/jpeg2000dec.c
> > @@ -269,6 +269,7 @@ static int get_siz(Jpeg2000DecoderContext *s)
> >      const enum AVPixelFormat *possible_fmts = NULL;
> >      int possible_fmts_nb = 0;
> >      int ret;
> > +    int dimx, dimy;
> >
> >      if (bytestream2_get_bytes_left(&s->g) < 36) {
> >          av_log(s->avctx, AV_LOG_ERROR, "Insufficient space for SIZ\n");
> > @@ -286,10 +287,6 @@ static int get_siz(Jpeg2000DecoderContext *s)
> >      s->tile_offset_y  = bytestream2_get_be32u(&s->g); // YT0Siz
> >      ncomponents       = bytestream2_get_be16u(&s->g); // CSiz
>
> > -    if (s->image_offset_x || s->image_offset_y) {
> > -        avpriv_request_sample(s->avctx, "Support for image offsets");
> > -        return AVERROR_PATCHWELCOME;
> > -    }
>
> I would have expected this change to be part of a patch "support
> image offsets".
Yes, I shall split the patches. However, purely in terms of adding
support for image offsets, this is the only change required.
All other changes look like other errors in the code.
>
> >      if (av_image_check_size2(s->width, s->height, s->avctx->max_pixels, AV_PIX_FMT_NONE, 0, s->avctx)) {
> >          avpriv_request_sample(s->avctx, "Large Dimensions");
> >          return AVERROR_PATCHWELCOME;
> > @@ -371,11 +368,18 @@ static int get_siz(Jpeg2000DecoderContext *s)
> >      }
> >
> >      /* compute image size with reduction factor */
> > -    ret = ff_set_dimensions(s->avctx,
> > -            ff_jpeg2000_ceildivpow2(s->width  - s->image_offset_x,
> > -                                               s->reduction_factor),
> > -            ff_jpeg2000_ceildivpow2(s->height - s->image_offset_y,
> > -                                               s->reduction_factor));
> > +    dimx = ff_jpeg2000_ceildivpow2(s->width  - s->image_offset_x,
> > +                                               s->reduction_factor);
> > +    dimy = ff_jpeg2000_ceildivpow2(s->height - s->image_offset_y,
> > +                                               s->reduction_factor);
> > +    dimx = ff_jpeg2000_ceildiv(dimx, s->cdx[0]);
> > +    dimy = ff_jpeg2000_ceildiv(dimy, s->cdy[0]);
> > +    for (i = 1; i < s->ncomponents; i++) {
> > +        dimx = FFMAX(dimx, ff_jpeg2000_ceildiv(dimx, s->cdx[i]));
> > +        dimy = FFMAX(dimy, ff_jpeg2000_ceildiv(dimy, s->cdy[i]));
> > +    }
> > +
> > +    ret = ff_set_dimensions(s->avctx, dimx, dimy);
> >      if (ret < 0)
> >          return ret;
> >
> > @@ -427,6 +431,13 @@ static int get_siz(Jpeg2000DecoderContext *s)
> >                  s->cdef[3] = 3;
> >                  i = 0;
> >              }
>
> > +        } else if (ncomponents == 2) {
> > +            s->avctx->pix_fmt = AV_PIX_FMT_YA8;
> > +            i = 0;
>
> I am not convinced that this is a good idea:
> Why is this not detected, what is the sample and most important:
> What happens if the two components have different subsampling?
In this case, the image p1_07.j2k has different sample separations for the
two different layers. If it's fair to assume that sample separation
and subsampling
are the same, the rest of the code accounts for the difference in subsampling.
>
> > +        } else if (ncomponents == 1) {
> > +            s->avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> > +            i = 0;
> > +            s->cdef[0] = 0;
>
> This may be ok but why is it not detected in the existing code?
This check was for the sample p1_01.j2k. It has only 1 layer. However,
The sample separation x = 2 and sample separation y = 1. I feel this could
be the reason it is not getting recognized. I guess the pix_fmt_match()
needs some modification, but I do not fully understand pixel formats.
Any reference I could use would be really helpful.
>
> I cannot comment on the other changes.
I do not intend this patch to get merged, I wanted to review the
changes first. Hopefully I could get some help with the colour
formats.
>
> Carl Eugen
> _______________________________________________
> 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 June 19, 2020, 7:59 p.m. UTC | #3
On Thu, Jun 18, 2020 at 11:55:20PM +0530, gautamramk@gmail.com wrote:
> From: Gautam Ramakrishnan <gautamramk@gmail.com>
> 
> This is with reference to my previous email on the mailing list
> with subject: "query on pixel formats".
> I wish to cleanup some errors in the decoder code. These changes
> would allow the samples p1_01.j2k and p1_07.j2k to be decoded.
> However, I am facing issues with pixel format selection and have
> currently forced the pixel formats to demonstrate the changes made.
> Would be grateful if anyone could suggest modifications to the pix
> format selection.
> ---
>  libavcodec/jpeg2000.c    |  3 ---
>  libavcodec/jpeg2000dec.c | 53 ++++++++++++++++++++++++++--------------
>  2 files changed, 34 insertions(+), 22 deletions(-)

Its probably a good idea to run this through some fuzzer when checks are
removed, something really simple like zzuf is probably better than nothing

thx

[...]
Gautam Ramakrishnan June 20, 2020, 4:18 a.m. UTC | #4
On Sat, Jun 20, 2020 at 1:29 AM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Thu, Jun 18, 2020 at 11:55:20PM +0530, gautamramk@gmail.com wrote:
> > From: Gautam Ramakrishnan <gautamramk@gmail.com>
> >
> > This is with reference to my previous email on the mailing list
> > with subject: "query on pixel formats".
> > I wish to cleanup some errors in the decoder code. These changes
> > would allow the samples p1_01.j2k and p1_07.j2k to be decoded.
> > However, I am facing issues with pixel format selection and have
> > currently forced the pixel formats to demonstrate the changes made.
> > Would be grateful if anyone could suggest modifications to the pix
> > format selection.
> > ---
> >  libavcodec/jpeg2000.c    |  3 ---
> >  libavcodec/jpeg2000dec.c | 53 ++++++++++++++++++++++++++--------------
> >  2 files changed, 34 insertions(+), 22 deletions(-)
>
> Its probably a good idea to run this through some fuzzer when checks are
> removed, something really simple like zzuf is probably better than nothing
>
Yes, I shall do that.
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I know you won't believe me, but the highest form of Human Excellence is
> to question oneself and others. -- Socrates
> _______________________________________________
> 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".



--
-------------
Gautam |
Gautam Ramakrishnan June 21, 2020, 5:25 p.m. UTC | #5
On Sat, Jun 20, 2020 at 9:48 AM Gautam Ramakrishnan
<gautamramk@gmail.com> wrote:
>
> On Sat, Jun 20, 2020 at 1:29 AM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > On Thu, Jun 18, 2020 at 11:55:20PM +0530, gautamramk@gmail.com wrote:
> > > From: Gautam Ramakrishnan <gautamramk@gmail.com>
> > >
> > > This is with reference to my previous email on the mailing list
> > > with subject: "query on pixel formats".
> > > I wish to cleanup some errors in the decoder code. These changes
> > > would allow the samples p1_01.j2k and p1_07.j2k to be decoded.
> > > However, I am facing issues with pixel format selection and have
> > > currently forced the pixel formats to demonstrate the changes made.
> > > Would be grateful if anyone could suggest modifications to the pix
> > > format selection.
> > > ---
> > >  libavcodec/jpeg2000.c    |  3 ---
> > >  libavcodec/jpeg2000dec.c | 53 ++++++++++++++++++++++++++--------------
> > >  2 files changed, 34 insertions(+), 22 deletions(-)
> >
> > Its probably a good idea to run this through some fuzzer when checks are
> > removed, something really simple like zzuf is probably better than nothing
> >
> Yes, I shall do that.
> > thx
> >
> > [...]
> > --
> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > I know you won't believe me, but the highest form of Human Excellence is
> > to question oneself and others. -- Socrates
> > _______________________________________________
> > 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".
>
>
>
> --
> -------------
> Gautam |

When trying to perform fuzz testing, what kind of configuration would
you recommend using.
For example, I am taking some of the reference files and setting the
error rate to 0.01. Is that sufficient?
Carl Eugen Hoyos June 21, 2020, 5:52 p.m. UTC | #6
Am So., 21. Juni 2020 um 19:51 Uhr schrieb Gautam Ramakrishnan
<gautamramk@gmail.com>:

> When trying to perform fuzz testing, what kind of configuration would
> you recommend using.
> For example, I am taking some of the reference files and setting the
> error rate to 0.01. Is that sufficient?

You could test with different error rates.

Carl Eugen
Michael Niedermayer June 22, 2020, 7:22 p.m. UTC | #7
On Sun, Jun 21, 2020 at 10:55:24PM +0530, Gautam Ramakrishnan wrote:
> On Sat, Jun 20, 2020 at 9:48 AM Gautam Ramakrishnan
> <gautamramk@gmail.com> wrote:
> >
> > On Sat, Jun 20, 2020 at 1:29 AM Michael Niedermayer
> > <michael@niedermayer.cc> wrote:
> > >
> > > On Thu, Jun 18, 2020 at 11:55:20PM +0530, gautamramk@gmail.com wrote:
> > > > From: Gautam Ramakrishnan <gautamramk@gmail.com>
> > > >
> > > > This is with reference to my previous email on the mailing list
> > > > with subject: "query on pixel formats".
> > > > I wish to cleanup some errors in the decoder code. These changes
> > > > would allow the samples p1_01.j2k and p1_07.j2k to be decoded.
> > > > However, I am facing issues with pixel format selection and have
> > > > currently forced the pixel formats to demonstrate the changes made.
> > > > Would be grateful if anyone could suggest modifications to the pix
> > > > format selection.
> > > > ---
> > > >  libavcodec/jpeg2000.c    |  3 ---
> > > >  libavcodec/jpeg2000dec.c | 53 ++++++++++++++++++++++++++--------------
> > > >  2 files changed, 34 insertions(+), 22 deletions(-)
> > >
> > > Its probably a good idea to run this through some fuzzer when checks are
> > > removed, something really simple like zzuf is probably better than nothing
> > >
> > Yes, I shall do that.
> > > thx
> > >
> > > [...]
> > > --
> > > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> > >
> > > I know you won't believe me, but the highest form of Human Excellence is
> > > to question oneself and others. -- Socrates
> > > _______________________________________________
> > > 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".
> >
> >
> >
> > --
> > -------------
> > Gautam |
> 
> When trying to perform fuzz testing, what kind of configuration would
> you recommend using.
> For example, I am taking some of the reference files and setting the
> error rate to 0.01. Is that sufficient?

i tried zzuf with defaults before writing my suggestion and it spotted
segfaults rather quickly, but i dont know if these segfaults where
related to these changes

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/jpeg2000.c b/libavcodec/jpeg2000.c
index 73206d17f3..1aca31ffa4 100644
--- a/libavcodec/jpeg2000.c
+++ b/libavcodec/jpeg2000.c
@@ -509,9 +509,6 @@  int ff_jpeg2000_init_component(Jpeg2000Component *comp,
         // update precincts size: 2^n value
         reslevel->log2_prec_width  = codsty->log2_prec_widths[reslevelno];
         reslevel->log2_prec_height = codsty->log2_prec_heights[reslevelno];
-        if (!reslevel->log2_prec_width || !reslevel->log2_prec_height) {
-            return AVERROR_INVALIDDATA;
-        }
 
         /* Number of bands for each resolution level */
         if (reslevelno == 0)
diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index ab36009a2d..ae63c68ca8 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -269,6 +269,7 @@  static int get_siz(Jpeg2000DecoderContext *s)
     const enum AVPixelFormat *possible_fmts = NULL;
     int possible_fmts_nb = 0;
     int ret;
+    int dimx, dimy;
 
     if (bytestream2_get_bytes_left(&s->g) < 36) {
         av_log(s->avctx, AV_LOG_ERROR, "Insufficient space for SIZ\n");
@@ -286,10 +287,6 @@  static int get_siz(Jpeg2000DecoderContext *s)
     s->tile_offset_y  = bytestream2_get_be32u(&s->g); // YT0Siz
     ncomponents       = bytestream2_get_be16u(&s->g); // CSiz
 
-    if (s->image_offset_x || s->image_offset_y) {
-        avpriv_request_sample(s->avctx, "Support for image offsets");
-        return AVERROR_PATCHWELCOME;
-    }
     if (av_image_check_size2(s->width, s->height, s->avctx->max_pixels, AV_PIX_FMT_NONE, 0, s->avctx)) {
         avpriv_request_sample(s->avctx, "Large Dimensions");
         return AVERROR_PATCHWELCOME;
@@ -371,11 +368,18 @@  static int get_siz(Jpeg2000DecoderContext *s)
     }
 
     /* compute image size with reduction factor */
-    ret = ff_set_dimensions(s->avctx,
-            ff_jpeg2000_ceildivpow2(s->width  - s->image_offset_x,
-                                               s->reduction_factor),
-            ff_jpeg2000_ceildivpow2(s->height - s->image_offset_y,
-                                               s->reduction_factor));
+    dimx = ff_jpeg2000_ceildivpow2(s->width  - s->image_offset_x,
+                                               s->reduction_factor);
+    dimy = ff_jpeg2000_ceildivpow2(s->height - s->image_offset_y,
+                                               s->reduction_factor);
+    dimx = ff_jpeg2000_ceildiv(dimx, s->cdx[0]);
+    dimy = ff_jpeg2000_ceildiv(dimy, s->cdy[0]);
+    for (i = 1; i < s->ncomponents; i++) {
+        dimx = FFMAX(dimx, ff_jpeg2000_ceildiv(dimx, s->cdx[i]));
+        dimy = FFMAX(dimy, ff_jpeg2000_ceildiv(dimy, s->cdy[i]));
+    }
+
+    ret = ff_set_dimensions(s->avctx, dimx, dimy);
     if (ret < 0)
         return ret;
 
@@ -427,6 +431,13 @@  static int get_siz(Jpeg2000DecoderContext *s)
                 s->cdef[3] = 3;
                 i = 0;
             }
+        } else if (ncomponents == 2) {
+            s->avctx->pix_fmt = AV_PIX_FMT_YA8;
+            i = 0;
+        } else if (ncomponents == 1) {
+            s->avctx->pix_fmt = AV_PIX_FMT_GRAY8;
+            i = 0;
+            s->cdef[0] = 0;
         }
     }
 
@@ -969,12 +980,11 @@  static int init_tile(Jpeg2000DecoderContext *s, int tileno)
         comp->coord_o[0][1] = tile->coord[0][1];
         comp->coord_o[1][0] = tile->coord[1][0];
         comp->coord_o[1][1] = tile->coord[1][1];
-        if (compno) {
-            comp->coord_o[0][0] /= s->cdx[compno];
-            comp->coord_o[0][1] /= s->cdx[compno];
-            comp->coord_o[1][0] /= s->cdy[compno];
-            comp->coord_o[1][1] /= s->cdy[compno];
-        }
+
+        comp->coord_o[0][0] = ff_jpeg2000_ceildiv(comp->coord_o[0][0], s->cdx[compno]);
+        comp->coord_o[0][1] = ff_jpeg2000_ceildiv(comp->coord_o[0][1], s->cdx[compno]);
+        comp->coord_o[1][0] = ff_jpeg2000_ceildiv(comp->coord_o[1][0], s->cdy[compno]);
+        comp->coord_o[1][1] = ff_jpeg2000_ceildiv(comp->coord_o[1][1], s->cdy[compno]);
 
         comp->coord[0][0] = ff_jpeg2000_ceildivpow2(comp->coord_o[0][0], s->reduction_factor);
         comp->coord[0][1] = ff_jpeg2000_ceildivpow2(comp->coord_o[0][1], s->reduction_factor);
@@ -1927,18 +1937,23 @@  static inline void tile_codeblocks(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile
             float *datap     = comp->f_data;                                                      \
             int32_t *i_datap = comp->i_data;                                                      \
             int cbps         = s->cbps[compno];                                                   \
-            int w            = tile->comp[compno].coord[0][1] - s->image_offset_x;                \
+            int w            = tile->comp[compno].coord[0][1] -                                   \
+                               ff_jpeg2000_ceildiv(s->image_offset_x, s->cdx[compno]);            \
+            int h            = tile->comp[compno].coord[1][1] -                                   \
+                               ff_jpeg2000_ceildiv(s->image_offset_y, s->cdy[compno]);            \
             int plane        = 0;                                                                 \
                                                                                                   \
             if (planar)                                                                           \
                 plane = s->cdef[compno] ? s->cdef[compno]-1 : (s->ncomponents-1);                 \
                                                                                                   \
-            y    = tile->comp[compno].coord[1][0] - s->image_offset_y / s->cdy[compno];           \
+            y    = tile->comp[compno].coord[1][0] -                                               \
+                   ff_jpeg2000_ceildiv(s->image_offset_y, s->cdy[compno]);                        \
             line = (PIXEL *)picture->data[plane] + y * (picture->linesize[plane] / sizeof(PIXEL));\
-            for (; y < tile->comp[compno].coord[1][1] - s->image_offset_y; y++) {                 \
+            for (; y < h; y++) {                                                                  \
                 PIXEL *dst;                                                                       \
                                                                                                   \
-                x   = tile->comp[compno].coord[0][0] - s->image_offset_x / s->cdx[compno];        \
+                x   = tile->comp[compno].coord[0][0] -                                            \
+                      ff_jpeg2000_ceildiv(s->image_offset_x, s->cdx[compno]);                     \
                 dst = line + x * pixelsize + compno*!planar;                                      \
                                                                                                   \
                 if (codsty->transform == FF_DWT97) {                                              \