[FFmpeg-devel] [PATCH 1/5] libavcodec/jpeg2000.c: Precinct size check removed

[Gautam Ramakrishnan]

On Sat, Jun 27, 2020 at 3:13 PM Michael Niedermayer
<michael at niedermayer.cc> wrote:
>
> On Mon, Jun 22, 2020 at 12:12:04AM +0530, gautamramk at gmail.com wrote:
> > From: Gautam Ramakrishnan <gautamramk at gmail.com>
> >
> > This patch removes a check which throws an error if
> > the log2 precinct width/height is 0. The standard allows
> > the first component to have 0 as the log2 width/height.
> > ---
> >  libavcodec/jpeg2000.c | 3 ---
> >  1 file changed, 3 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;
> > -        }
>
> This checked that log2_prec_width... has been initialized.
> Is there some other check that ensures this is not just 0 from allocation
> which IIUC is not an allowed path in the spec

A check happens only when the COD and COC marker gets read.
The following lines in the get_cox() function in jpeg2000dec.c does it.
if (i)
    if (c->log2_prec_widths[i] == 0 || c->log2_prec_heights[i] == 0) {
        av_log(s->avctx, AV_LOG_ERROR, "PPx %d PPy %d invalid\n",
                   c->log2_prec_widths[i], c->log2_prec_heights[i]);

This check ensures that for all resolution levels except reslevel 0 cannot have
the widths and heights as 0. Do you feel this check is sufficient? Instead of
completely removing the check as in the patch, we could change the condition to
 if ((!reslevel->log2_prec_width || !reslevel->log2_prec_height) && reslevel) {
}
This would at least ensure that the other reslevels are definitely set.



-- 
-------------
Gautam |