[FFmpeg-devel] [PATCH v2] avcodec/jpeg2000: Add support for High-Throughput JPEG 2000 (HTJ2K) decoding.
Caleb Etemesi
etemesicaleb at gmail.com
Wed Sep 21 11:13:18 EEST 2022
>> Please separate struct renaming into a separate patch
Will do in the next patch
>> Corresponds to µ_start, right?
Not sure what µ_start here relates to, but this is the zero bit plane
information in *B.10.5*
of the jpeg2000 spec (ITU Rec. T.800 (06/2019))
>> Could also be called Lcup and Lref. But fine I suppose.
Will change in the next patch
>> No time to review this atm, hopefully later though
No problem.
>> Why are these tables in here when they're only used by jpeg2000htdec.c?
I wasn't aware of any table placing conventions and my personal preference
is usually to place them
inside header files.
On Tue, Sep 20, 2022 at 4:49 PM Tomas Härdin <tjoppen at acc.umu.se> wrote:
> tor 2022-09-08 klockan 23:49 +0300 skrev etemesicaleb at gmail.com:
> >
> > --- a/libavcodec/j2kenc.c
> > +++ b/libavcodec/j2kenc.c
> > @@ -106,7 +106,7 @@ static const int dwt_norms[2][4][10] = { //
> > [dwt_type][band][rlevel] (multiplied
> > typedef struct {
> > Jpeg2000Component *comp;
> > double *layer_rates;
> > -} Jpeg2000Tile;
> > +} Jpeg2000EncTile;
> >
> > typedef struct {
> > AVClass *class;
> > @@ -131,7 +131,7 @@ typedef struct {
> > Jpeg2000CodingStyle codsty;
> > Jpeg2000QuantStyle qntsty;
> >
> > - Jpeg2000Tile *tile;
> > + Jpeg2000EncTile *tile;
>
> Please separate struct renaming into a separate patch
>
> > typedef struct Jpeg2000Cblk {
> > uint8_t npasses;
> > uint8_t ninclpasses; // number coding of passes included in
> > codestream
> > @@ -181,6 +181,7 @@ typedef struct Jpeg2000Cblk {
> > uint16_t *lengthinc;
> > uint8_t nb_lengthinc;
> > uint8_t lblock;
> > + uint8_t zbp; // Zero bit planes
>
> Corresponds to µ_start, right?
>
> > uint8_t *data;
> > size_t data_allocated;
> > int nb_terminations;
> > @@ -189,6 +190,7 @@ typedef struct Jpeg2000Cblk {
> > Jpeg2000Pass *passes;
> > Jpeg2000Layer *layers;
> > int coord[2][2]; // border coordinates {{x0, x1}, {y0, y1}}
> > + int pass_lengths[2];
>
> Could also be called Lcup and Lref. But fine I suppose.
>
> > } Jpeg2000Cblk; // code block
> >
> > typedef struct Jpeg2000Prec {
> > @@ -227,6 +229,103 @@ typedef struct Jpeg2000Component {
> > uint8_t roi_shift; // ROI scaling value for the component
> > } Jpeg2000Component;
> >
> > +#define JP2_SIG_TYPE 0x6A502020
> > +#define JP2_SIG_VALUE 0x0D0A870A
> > +#define JP2_CODESTREAM 0x6A703263
> > +#define JP2_HEADER 0x6A703268
> > +
> > +#define HAD_COC 0x01
> > +#define HAD_QCC 0x02
> > +
> > +#define MAX_POCS 32
>
> Similarly with code movements. I can see this move adds is_htj2k to
> Jpeg2000DecoderContext. Please separate such things. Makes reviewing
> functional changes much easier.
>
> > -static inline void tile_codeblocks(const Jpeg2000DecoderContext *s,
> > Jpeg2000Tile *tile)
> > +static inline void tile_codeblocks(const Jpeg2000DecoderContext *s,
> > Jpeg2000DecTile *tile)
> > {
> > Jpeg2000T1Context t1;
> >
> > int compno, reslevelno, bandno;
> > + int subbandno;
> >
> > /* Loop on tile components */
> > for (compno = 0; compno < s->ncomponents; compno++) {
> > - Jpeg2000Component *comp = tile->comp + compno;
> > - Jpeg2000CodingStyle *codsty = tile->codsty + compno;
> > + Jpeg2000Component *comp = tile->comp + compno;
> > + Jpeg2000CodingStyle *codsty = tile->codsty + compno;
> > + Jpeg2000QuantStyle *quantsty = tile->qntsty + compno;
>
> Cosmetic and functional changes don't mix
>
> > int coded = 0;
> > + subbandno = 0;
> >
> > t1.stride = (1<<codsty->log2_cblk_width) + 2;
> >
> > @@ -1959,7 +1881,7 @@ static inline void tile_codeblocks(const
> > Jpeg2000DecoderContext *s, Jpeg2000Tile
> > for (reslevelno = 0; reslevelno < codsty->nreslevels2decode;
> > reslevelno++) {
> > Jpeg2000ResLevel *rlevel = comp->reslevel + reslevelno;
> > /* Loop on bands */
> > - for (bandno = 0; bandno < rlevel->nbands; bandno++) {
> > + for (bandno = 0; bandno < rlevel->nbands;
> > bandno++,subbandno++) {
> > int nb_precincts, precno;
> > Jpeg2000Band *band = rlevel->band + bandno;
> > int cblkno = 0, bandpos;
> > @@ -1979,12 +1901,21 @@ static inline void tile_codeblocks(const
> > Jpeg2000DecoderContext *s, Jpeg2000Tile
> > for (cblkno = 0;
> > cblkno < prec->nb_codeblocks_width * prec-
> > >nb_codeblocks_height;
> > cblkno++) {
> > - int x, y;
> > + int x, y, ret, magp;
> > Jpeg2000Cblk *cblk = prec->cblk + cblkno;
> > - int ret = decode_cblk(s, codsty, &t1, cblk,
> > - cblk->coord[0][1] - cblk-
> > >coord[0][0],
> > - cblk->coord[1][1] - cblk-
> > >coord[1][0],
> > - bandpos, comp->roi_shift);
> > + // Annex E (Equation E-2) ISO/IEC 15444-
> > 1:2019
> > + magp = quantsty->expn[subbandno] + quantsty-
> > >nguardbits - 1;
>
> Is expn[] allocated for regular j2k? I'd move this inside the is_htj2k
> block below
>
> > +
> > + if (s->is_htj2k)
> > + ret = decode_htj2k(s, codsty, &t1, cblk,
> > + cblk->coord[0][1] -
> > cblk->coord[0][0],
> > + cblk->coord[1][1] -
> > cblk->coord[1][0],
> > + magp, comp-
> > >roi_shift);
>
> > --- /dev/null
> > +++ b/libavcodec/jpeg2000htdec.c
>
> No time to review this atm, hopefully later though
>
> > --- /dev/null
> > +++ b/libavcodec/jpeg2000htdec.h
> >
> > +/**
> > + * @brief CtxVLC tables, borrowed from openhtj2k (
> > https://github.com/osamu620/OpenHTJ2K) (credits to Osamu Watanabe)
> > + */
> > +static const uint16_t dec_CxtVLC_table1[1024] = {
> > +static const uint16_t dec_CxtVLC_table0[1024] = {
>
> Why are these tables in here when they're only used by jpeg2000htdec.c?
>
> /Tomas
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
More information about the ffmpeg-devel
mailing list