[FFmpeg-devel] [PATCH v2] avcodec/jpeg2000: Add support for High-Throughput JPEG 2000 (HTJ2K) decoding.
Caleb Etemesi
etemesicaleb at gmail.com
Sat Sep 10 12:13:51 EEST 2022
Ideally, what I wanted to achieve was following best practices of what I
saw,
most of the error reporting inside jpeg2000dec.c would log errors using
s->avctx, in case of things like corrupt bitstream errors, I did want also
the htj2k code to emulate this, and the only reason its present in
jpeg2000_decode_ctx_vlc was for it to report in case I read out of bounds,
(it was to replace the assert0(index < 1024))
That's the only reason I need a Jpeg2000DecoderContext in jpeg2000htdec.c
I later learnt you can pass NULL to av_log, and used that to report invalid
bit reads e.g at
https://github.com/etemesi254/FFmpeg/blob/87de24efb472c1cf60d920c433d710cf95a64ea5/libavcodec/jpeg2000htdec.c#L189-L191
since I saw the call stack was too deep to justify me passing around
Jpeg2000DecoderContext everywhere.
So since I'll be making changes to address these issues, what are your
thoughts?
For the circular reference, I forgot about inclusion guards, sorry about
that.
Regards,
Caleb
On Sat, Sep 10, 2022 at 12:34 AM Andreas Rheinhardt <
andreas.rheinhardt at outlook.com> wrote:
> Caleb Etemesi:
> >> This is unneeded for the encoder
> >> maybe a jpeg2000dec.h would be better for this
> >> also code moving should be in a seperate patch
> >> from functional changes
> >
> >
> > This will be a circular dependence, since jpeg2000htdec.h needs the
> decoder
> > context, and jpeg2000.c needs jpeg2000htdec.c.
> >
>
> It seems like the only use of the decoder context in jpeg2000htdec.h is
> in jpeg2000_decode_ctx_vlc() where the Jpeg2000DecoderContext is not
> even used at all. And I fail to see the circular dependency even if
> jpeg2000htdec.h needed the decoder context: after all, you are not
> adding an inclusion of jpeg2000htdec.h to jpeg2000.h, so jpeg2000dec.h
> will also not include jpeg2000htdec.h.
>
> >
> > On Fri, 9 Sept 2022, 19:52 Caleb Etemesi, <etemesicaleb at gmail.com>
> wrote:
> >
> >> Will address in the next iteration
> >>
> >> On Fri, 9 Sept 2022, 18:46 Michael Niedermayer, <michael at niedermayer.cc
> >
> >> wrote:
> >>
> >>> On Thu, Sep 08, 2022 at 11:49:53PM +0300, etemesicaleb at gmail.com
> wrote:
> >>>> From: caleb <etemesicaleb at gmail.com>
> >>>>
> >>>> Rebased this patch on master branch
> >>>> ---
> >>>> libavcodec/Makefile | 2 +-
> >>>> libavcodec/j2kenc.c | 26 +-
> >>>> libavcodec/jpeg2000.h | 103 ++-
> >>>> libavcodec/jpeg2000dec.c | 193 ++----
> >>>> libavcodec/jpeg2000htdec.c | 1212
> ++++++++++++++++++++++++++++++++++++
> >>>> libavcodec/jpeg2000htdec.h | 210 +++++++
> >>>> 6 files changed, 1599 insertions(+), 147 deletions(-)
> >>>> create mode 100644 libavcodec/jpeg2000htdec.c
> >>>> create mode 100644 libavcodec/jpeg2000htdec.h
> >>>>
> >>>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> >>>> index 945908e3b8..ecf5c47cad 100644
> >>>> --- a/libavcodec/Makefile
> >>>> +++ b/libavcodec/Makefile
> >>>> @@ -450,7 +450,7 @@ OBJS-$(CONFIG_JACOSUB_DECODER) +=
> >>> jacosubdec.o ass.o
> >>>> OBJS-$(CONFIG_JPEG2000_ENCODER) += j2kenc.o mqcenc.o mqc.o
> >>> jpeg2000.o \
> >>>> jpeg2000dwt.o
> >>>> OBJS-$(CONFIG_JPEG2000_DECODER) += jpeg2000dec.o jpeg2000.o
> >>> jpeg2000dsp.o \
> >>>> - jpeg2000dwt.o mqcdec.o
> mqc.o
> >>>> + jpeg2000dwt.o mqcdec.o
> mqc.o
> >>> jpeg2000htdec.o
> >>>> OBJS-$(CONFIG_JPEGLS_DECODER) += jpeglsdec.o jpegls.o
> >>>> OBJS-$(CONFIG_JPEGLS_ENCODER) += jpeglsenc.o jpegls.o
> >>>> OBJS-$(CONFIG_JV_DECODER) += jvdec.o
> >>>> diff --git a/libavcodec/j2kenc.c b/libavcodec/j2kenc.c
> >>>> index e883d5deb7..233d75e96d 100644
> >>>> --- 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;
> >>>> int layer_rates[100];
> >>>> uint8_t compression_rate_enc; ///< Is compression done using
> >>> compression ratio?
> >>>>
> >>>> @@ -427,7 +427,7 @@ static void compute_rates(Jpeg2000EncoderContext*
> s)
> >>>> int layno, compno;
> >>>> for (i = 0; i < s->numYtiles; i++) {
> >>>> for (j = 0; j < s->numXtiles; j++) {
> >>>> - Jpeg2000Tile *tile = &s->tile[s->numXtiles * i + j];
> >>>> + Jpeg2000EncTile *tile = &s->tile[s->numXtiles * i + j];
> >>>> for (compno = 0; compno < s->ncomponents; compno++) {
> >>>> int tilew = tile->comp[compno].coord[0][1] -
> >>> tile->comp[compno].coord[0][0];
> >>>> int tileh = tile->comp[compno].coord[1][1] -
> >>> tile->comp[compno].coord[1][0];
> >>>> @@ -460,12 +460,12 @@ static int init_tiles(Jpeg2000EncoderContext *s)
> >>>> s->numXtiles = ff_jpeg2000_ceildiv(s->width, s->tile_width);
> >>>> s->numYtiles = ff_jpeg2000_ceildiv(s->height, s->tile_height);
> >>>>
> >>>> - s->tile = av_calloc(s->numXtiles, s->numYtiles *
> >>> sizeof(Jpeg2000Tile));
> >>>> + s->tile = av_calloc(s->numXtiles, s->numYtiles *
> >>> sizeof(Jpeg2000EncTile));
> >>>> if (!s->tile)
> >>>> return AVERROR(ENOMEM);
> >>>> for (tileno = 0, tiley = 0; tiley < s->numYtiles; tiley++)
> >>>> for (tilex = 0; tilex < s->numXtiles; tilex++, tileno++){
> >>>> - Jpeg2000Tile *tile = s->tile + tileno;
> >>>> + Jpeg2000EncTile *tile = s->tile + tileno;
> >>>>
> >>>> tile->comp = av_calloc(s->ncomponents,
> >>> sizeof(*tile->comp));
> >>>> if (!tile->comp)
> >>>> @@ -509,7 +509,7 @@ static int init_tiles(Jpeg2000EncoderContext *s)
> >>>> int tileno, compno, i, y, x;
> >>> \
> >>>> const PIXEL *line;
> >>> \
> >>>> for (tileno = 0; tileno < s->numXtiles * s->numYtiles;
> >>> tileno++){ \
> >>>> - Jpeg2000Tile *tile = s->tile + tileno;
> >>> \
> >>>> + Jpeg2000EncTile *tile = s->tile + tileno;
> >>> \
> >>>> if (s->planar){
> >>> \
> >>>> for (compno = 0; compno < s->ncomponents; compno++){
> >>> \
> >>>> Jpeg2000Component *comp = tile->comp + compno;
> >>> \
> >>>> @@ -701,7 +701,7 @@ static void encode_clnpass(Jpeg2000T1Context *t1,
> >>> int width, int height, int ban
> >>>> }
> >>>> }
> >>>>
> >>>> -static void encode_cblk(Jpeg2000EncoderContext *s, Jpeg2000T1Context
> >>> *t1, Jpeg2000Cblk *cblk, Jpeg2000Tile *tile,
> >>>> +static void encode_cblk(Jpeg2000EncoderContext *s, Jpeg2000T1Context
> >>> *t1, Jpeg2000Cblk *cblk, Jpeg2000EncTile *tile,
> >>>> int width, int height, int bandpos, int lev)
> >>>> {
> >>>> int pass_t = 2, passno, x, y, max=0, nmsedec, bpno;
> >>>> @@ -935,7 +935,7 @@ static int encode_packet(Jpeg2000EncoderContext
> *s,
> >>> Jpeg2000ResLevel *rlevel, in
> >>>> return 0;
> >>>> }
> >>>>
> >>>> -static int encode_packets(Jpeg2000EncoderContext *s, Jpeg2000Tile
> >>> *tile, int tileno, int nlayers)
> >>>> +static int encode_packets(Jpeg2000EncoderContext *s, Jpeg2000EncTile
> >>> *tile, int tileno, int nlayers)
> >>>> {
> >>>> int compno, reslevelno, layno, ret;
> >>>> Jpeg2000CodingStyle *codsty = &s->codsty;
> >>>> @@ -1181,7 +1181,7 @@ static int encode_packets(Jpeg2000EncoderContext
> >>> *s, Jpeg2000Tile *tile, int til
> >>>> return 0;
> >>>> }
> >>>>
> >>>> -static void makelayer(Jpeg2000EncoderContext *s, int layno, double
> >>> thresh, Jpeg2000Tile* tile, int final)
> >>>> +static void makelayer(Jpeg2000EncoderContext *s, int layno, double
> >>> thresh, Jpeg2000EncTile* tile, int final)
> >>>> {
> >>>> int compno, resno, bandno, precno, cblkno;
> >>>> int passno;
> >>>> @@ -1264,7 +1264,7 @@ static void makelayer(Jpeg2000EncoderContext *s,
> >>> int layno, double thresh, Jpeg2
> >>>> }
> >>>> }
> >>>>
> >>>> -static void makelayers(Jpeg2000EncoderContext *s, Jpeg2000Tile *tile)
> >>>> +static void makelayers(Jpeg2000EncoderContext *s, Jpeg2000EncTile
> >>> *tile)
> >>>> {
> >>>> int precno, compno, reslevelno, bandno, cblkno, lev, passno,
> layno;
> >>>> int i;
> >>>> @@ -1365,7 +1365,7 @@ static int getcut(Jpeg2000Cblk *cblk, int64_t
> >>> lambda, int dwt_norm)
> >>>> return res;
> >>>> }
> >>>>
> >>>> -static void truncpasses(Jpeg2000EncoderContext *s, Jpeg2000Tile
> *tile)
> >>>> +static void truncpasses(Jpeg2000EncoderContext *s, Jpeg2000EncTile
> >>> *tile)
> >>>> {
> >>>> int precno, compno, reslevelno, bandno, cblkno, lev;
> >>>> Jpeg2000CodingStyle *codsty = &s->codsty;
> >>>> @@ -1399,7 +1399,7 @@ static void truncpasses(Jpeg2000EncoderContext
> >>> *s, Jpeg2000Tile *tile)
> >>>> }
> >>>> }
> >>>>
> >>>> -static int encode_tile(Jpeg2000EncoderContext *s, Jpeg2000Tile *tile,
> >>> int tileno)
> >>>> +static int encode_tile(Jpeg2000EncoderContext *s, Jpeg2000EncTile
> >>> *tile, int tileno)
> >>>> {
> >>>> int compno, reslevelno, bandno, ret;
> >>>> Jpeg2000T1Context t1;
> >>>> @@ -1514,7 +1514,7 @@ static void reinit(Jpeg2000EncoderContext *s)
> >>>> {
> >>>> int tileno, compno;
> >>>> for (tileno = 0; tileno < s->numXtiles * s->numYtiles; tileno++){
> >>>> - Jpeg2000Tile *tile = s->tile + tileno;
> >>>> + Jpeg2000EncTile *tile = s->tile + tileno;
> >>>> for (compno = 0; compno < s->ncomponents; compno++)
> >>>> ff_jpeg2000_reinit(tile->comp + compno, &s->codsty);
> >>>> }
> >>>
> >>> Renaming Jpeg2000Tile could be in a seperate patch
> >>>
> >>>
> >>>> diff --git a/libavcodec/jpeg2000.h b/libavcodec/jpeg2000.h
> >>>> index e5ecb4cbf9..a5dd693392 100644
> >>>> --- a/libavcodec/jpeg2000.h
> >>>> +++ b/libavcodec/jpeg2000.h
> >>>> @@ -33,8 +33,9 @@
> >>>>
> >>>> #include "avcodec.h"
> >>>> #include "mqc.h"
> >>>> +#include "bytestream.h"
> >>>> #include "jpeg2000dwt.h"
> >>>> -
> >>>> +#include "jpeg2000dsp.h"
> >>>> enum Jpeg2000Markers {
> >>>> JPEG2000_SOC = 0xff4f, // start of codestream
> >>>> JPEG2000_SIZ = 0xff51, // image and tile size
> >>>
> >>>> @@ -171,7 +172,6 @@ typedef struct Jpeg2000Layer {
> >>>> double disto;
> >>>> int cum_passes;
> >>>> } Jpeg2000Layer;
> >>>> -
> >>>> typedef struct Jpeg2000Cblk {
> >>>> uint8_t npasses;
> >>>> uint8_t ninclpasses; // number coding of passes included in
> >>> codestream
> >>>
> >>> unintended, i assume
> >>>
> >>>
> >>>> @@ -181,6 +181,7 @@ typedef struct Jpeg2000Cblk {
> >>>> uint16_t *lengthinc;
> >>>> uint8_t nb_lengthinc;
> >>>> uint8_t lblock;
> >>>> + uint8_t zbp; // Zero bit planes
> >>>> 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];
> >>>> } Jpeg2000Cblk; // code block
> >>>
> >>> Please use doxygen compatible comments so it all appears on for example
> >>> https://ffmpeg.org/doxygen/trunk/structJpeg2000Cblk.html
> >>>
> >>>>
> >>>> 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
> >>>> +
> >>>> +typedef struct Jpeg2000POCEntry {
> >>>> + uint16_t LYEpoc;
> >>>> + uint16_t CSpoc;
> >>>> + uint16_t CEpoc;
> >>>> + uint8_t RSpoc;
> >>>> + uint8_t REpoc;
> >>>> + uint8_t Ppoc;
> >>>> +} Jpeg2000POCEntry;
> >>>> +
> >>>> +typedef struct Jpeg2000POC {
> >>>> + Jpeg2000POCEntry poc[MAX_POCS];
> >>>> + int nb_poc;
> >>>> + int is_default;
> >>>> +} Jpeg2000POC;
> >>>> +
> >>>> +typedef struct Jpeg2000TilePart {
> >>>> + uint8_t tile_index; // Tile index who refers the
> >>> tile-part
> >>>> + const uint8_t *tp_end;
> >>>> + GetByteContext header_tpg; // bit stream of header if
> PPM
> >>> header is used
> >>>> + GetByteContext tpg; // bit stream in tile-part
> >>>> +} Jpeg2000TilePart;
> >>>> +
> >>>> +/* RMK: For JPEG2000 DCINEMA 3 tile-parts in a tile
> >>>> + * one per component, so tile_part elements have a size of 3 */
> >>>> +typedef struct Jpeg2000Tile {
> >>> ^^^^^^^^^^^^
> >>>> + Jpeg2000Component *comp;
> >>>> + uint8_t properties[4];
> >>>> + Jpeg2000CodingStyle codsty[4];
> >>>> + Jpeg2000QuantStyle qntsty[4];
> >>>> + Jpeg2000POC poc;
> >>>> + Jpeg2000TilePart tile_part[32];
> >>>> + uint8_t has_ppt; // whether this tile
> >>> has a ppt marker
> >>>> + uint8_t *packed_headers; // contains packed
> >>> headers. Used only along with PPT marker
> >>>> + int packed_headers_size; // size in bytes of
> >>> the packed headers
> >>>> + GetByteContext packed_headers_stream; // byte context
> >>> corresponding to packed headers
> >>>> + uint16_t tp_idx; // Tile-part index
> >>>> + int coord[2][2]; // border
> coordinates
> >>> {{x0, x1}, {y0, y1}}
> >>>> +} Jpeg2000DecTile;
> >>> ^^^^^^^^^^^^^^^
> >>> This difference appears unintended
> >>>
> >>>
> >>>> +
> >>>> +typedef struct Jpeg2000DecoderContext {
> >>>
> >>> This is unneeded for the encoder
> >>> maybe a jpeg2000dec.h would be better for this
> >>> also code moving should be in a seperate patch from functional changes
> >>>
> >>> [...]
> >>>> +#endif /* AVCODEC_JPEG2000HTDEC_H */
> >>>
> >>>> \ No newline at end of file
> >>>
> >>> needs newline
> >>>
> >>> new decoder not reviewed, feel free to wait for someone to review it
> >>> before reposting
> >>>
> >>> thx
> >>>
> >>>
> >>> [...]
> >>> --
> >>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >>>
> >>> Those who would give up essential Liberty, to purchase a little
> >>> temporary Safety, deserve neither Liberty nor Safety -- Benjamin
> Franklin
> _______________________________________________
> 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