[FFmpeg-devel] [PATCH 2/2] libavcodec/jpeg2000dec.c: ROI marker support

[Michael Niedermayer]

On Wed, Apr 22, 2020 at 08:44:10AM +0530, Gautam Ramakrishnan wrote:
> On Wed, Apr 22, 2020 at 3:04 AM Michael Niedermayer
> <michael at niedermayer.cc> wrote:
> >
> > On Tue, Apr 21, 2020 at 01:07:39AM +0530, gautamramk at gmail.com wrote:
> > > From: Gautam Ramakrishnan <gautamramk at gmail.com>
> > >
> > > This patch adds support for decoding images
> > > with a Region of Interest. Allows decoding
> > > samples such as p0_03.j2k. This patch should
> > > fix ticket #4681.
> > > ---
> > >  libavcodec/jpeg2000.h    |  1 +
> > >  libavcodec/jpeg2000dec.c | 57 +++++++++++++++++++++++++++++++++++-----
> > >  2 files changed, 51 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/libavcodec/jpeg2000.h b/libavcodec/jpeg2000.h
> > > index 7b78c0193e..0f82716981 100644
> > > --- a/libavcodec/jpeg2000.h
> > > +++ b/libavcodec/jpeg2000.h
> > > @@ -210,6 +210,7 @@ typedef struct Jpeg2000Component {
> > >      int *i_data;
> > >      int coord[2][2];   // border coordinates {{x0, x1}, {y0, y1}} -- can be reduced with lowres option
> > >      int coord_o[2][2]; // border coordinates {{x0, x1}, {y0, y1}} -- original values from jpeg2000 headers
> > > +    uint8_t roi_shift; // ROI scaling value for the component
> > >  } Jpeg2000Component;
> > >
> > >  /* misc tools */
> > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> > > index 5a7d9e7882..da19345ee7 100644
> > > --- a/libavcodec/jpeg2000dec.c
> > > +++ b/libavcodec/jpeg2000dec.c
> > > @@ -117,6 +117,7 @@ typedef struct Jpeg2000DecoderContext {
> > >      Jpeg2000CodingStyle codsty[4];
> > >      Jpeg2000QuantStyle  qntsty[4];
> > >      Jpeg2000POC         poc;
> >
> > > +    uint8_t             roi_shift[4];
> > >
> > >      int             bit_index;
> > >
> > > @@ -598,6 +599,29 @@ static int get_coc(Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *c,
> > >      return 0;
> > >  }
> > >
> > > +static int get_rgn(Jpeg2000DecoderContext *s, int n)
> > > +{
> > > +    uint16_t compno;
> > > +    compno = (s->ncomponents < 257)? bytestream2_get_byte(&s->g):
> > > +                                     bytestream2_get_be16u(&s->g);
> > > +    if (bytestream2_get_byte(&s->g)) {
> > > +        av_log(s->avctx, AV_LOG_ERROR, "Invalid RGN header.\n");
> > > +        return AVERROR_INVALIDDATA; // SRgn field value is 0
> > > +    }
> > > +    // SPrgn field
> > > +    if (compno < s->ncomponents) {
> > > +        if (s->curtileno == -1)
> > > +            s->roi_shift[compno] = bytestream2_get_byte(&s->g);
> >
> > theres a check for s->ncomponents < 257 implying that if this check is not a
> > dead check ncomponents can be bigger and then compno is just checked by that
> > before being used to index into a 4 entry array (roi_shift)
> >
> > something in here is not entirely correct
> For our decoder, it is a dead check as we are supposed to return an error
> before this marker reaches if the number of components is greater than 4.
> However, I have still kept this check keeping in mind support for more than
> 4 components in the future.

ok, please add a comment in the code
(this dead check can be detected probably by automated tools and
 without a comment people would otherwise "fix" it)

 [...]
 
thx

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200422/2ca1a558/attachment.sig>