Re: [FFmpeg-devel] [PATCH v2] avcodec/jpeg2000: Add support for High-Throughput JPEG 2000 (HTJ2K) decoding.

["Tomas Härdin" <tjoppen@acc.umu.se>]

tor 2022-09-08 klockan 23:49 +0300 skrev etemesicaleb@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@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".