| Message ID | 20200405104328.15484-1-gautamramk@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,v13] libavcodec/jpeg2000dec.c: Add support for PPT marker | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
On Sun, Apr 05, 2020 at 04:13:28PM +0530, gautamramk@gmail.com wrote: > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > This patch adds functional changes to support the > PPT marker. This patch fixes bug ticket #4610. > --- > libavcodec/jpeg2000dec.c | 88 +++++++++++++++++++++++++++++++++++----- > 1 file changed, 77 insertions(+), 11 deletions(-) > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c > index 732d88e6fc..2af3c61c37 100644 > --- a/libavcodec/jpeg2000dec.c > +++ b/libavcodec/jpeg2000dec.c > @@ -83,6 +83,10 @@ typedef struct Jpeg2000Tile { > 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}} > } Jpeg2000Tile; > @@ -855,6 +859,37 @@ static int get_plt(Jpeg2000DecoderContext *s, int n) > return 0; > } > > +static int get_ppt(Jpeg2000DecoderContext *s, int n) > +{ > + Jpeg2000Tile *tile; > + > + if (s->curtileno < 0) > + return AVERROR_INVALIDDATA; > + > + tile = &s->tile[s->curtileno]; > + if (tile->tp_idx != 0) { > + av_log(s->avctx, AV_LOG_ERROR, > + "PPT marker can occur only on first tile part of a tile.\n"); > + return AVERROR_INVALIDDATA; > + } > + > + tile->has_ppt = 1; // this tile has a ppt marker > + bytestream2_get_byte(&s->g); // Zppt is skipped and not used > + if (!tile->packed_headers) > + tile->packed_headers = av_malloc(n - 3); > + else > + tile->packed_headers = av_realloc(tile->packed_headers, > + tile->packed_headers_size + n - 3); why is the special case with av_malloc() needed ? > + if (!tile->packed_headers) > + return AVERROR(ENOMEM); this is looks like a memleak as the original array has been overwritten but not deallocated thx [...]
On Mon, Apr 6, 2020 at 1:16 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Sun, Apr 05, 2020 at 04:13:28PM +0530, gautamramk@gmail.com wrote: > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > This patch adds functional changes to support the > > PPT marker. This patch fixes bug ticket #4610. > > --- > > libavcodec/jpeg2000dec.c | 88 +++++++++++++++++++++++++++++++++++----- > > 1 file changed, 77 insertions(+), 11 deletions(-) > > > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c > > index 732d88e6fc..2af3c61c37 100644 > > --- a/libavcodec/jpeg2000dec.c > > +++ b/libavcodec/jpeg2000dec.c > > @@ -83,6 +83,10 @@ typedef struct Jpeg2000Tile { > > 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}} > > } Jpeg2000Tile; > > @@ -855,6 +859,37 @@ static int get_plt(Jpeg2000DecoderContext *s, int n) > > return 0; > > } > > > > +static int get_ppt(Jpeg2000DecoderContext *s, int n) > > +{ > > + Jpeg2000Tile *tile; > > + > > + if (s->curtileno < 0) > > + return AVERROR_INVALIDDATA; > > + > > + tile = &s->tile[s->curtileno]; > > + if (tile->tp_idx != 0) { > > + av_log(s->avctx, AV_LOG_ERROR, > > + "PPT marker can occur only on first tile part of a tile.\n"); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + tile->has_ppt = 1; // this tile has a ppt marker > > + bytestream2_get_byte(&s->g); // Zppt is skipped and not used > > > > + if (!tile->packed_headers) > > + tile->packed_headers = av_malloc(n - 3); > > + else > > + tile->packed_headers = av_realloc(tile->packed_headers, > > + tile->packed_headers_size + n - 3); > > why is the special case with av_malloc() needed ? > The if else is to check whether this is the first time this tile is being allocated its packed headers. If packed headers does not exist exist, create it. The else condition is just to add a new packed header to it. > > > + if (!tile->packed_headers) > > + return AVERROR(ENOMEM); > > this is looks like a memleak as the original array has been overwritten but > not deallocated This was a check for the malloc failure > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > In fact, the RIAA has been known to suggest that students drop out > of college or go to community college in order to be able to afford > settlements. -- The RIAA > _______________________________________________ > 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".
On Mon, Apr 06, 2020 at 09:23:26AM +0530, Gautam Ramakrishnan wrote: > On Mon, Apr 6, 2020 at 1:16 AM Michael Niedermayer > <michael@niedermayer.cc> wrote: > > > > On Sun, Apr 05, 2020 at 04:13:28PM +0530, gautamramk@gmail.com wrote: > > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > > > This patch adds functional changes to support the > > > PPT marker. This patch fixes bug ticket #4610. > > > --- > > > libavcodec/jpeg2000dec.c | 88 +++++++++++++++++++++++++++++++++++----- > > > 1 file changed, 77 insertions(+), 11 deletions(-) > > > > > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c > > > index 732d88e6fc..2af3c61c37 100644 > > > --- a/libavcodec/jpeg2000dec.c > > > +++ b/libavcodec/jpeg2000dec.c > > > @@ -83,6 +83,10 @@ typedef struct Jpeg2000Tile { > > > 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}} > > > } Jpeg2000Tile; > > > @@ -855,6 +859,37 @@ static int get_plt(Jpeg2000DecoderContext *s, int n) > > > return 0; > > > } > > > > > > +static int get_ppt(Jpeg2000DecoderContext *s, int n) > > > +{ > > > + Jpeg2000Tile *tile; > > > + > > > + if (s->curtileno < 0) > > > + return AVERROR_INVALIDDATA; > > > + > > > + tile = &s->tile[s->curtileno]; > > > + if (tile->tp_idx != 0) { > > > + av_log(s->avctx, AV_LOG_ERROR, > > > + "PPT marker can occur only on first tile part of a tile.\n"); > > > + return AVERROR_INVALIDDATA; > > > + } > > > + > > > + tile->has_ppt = 1; // this tile has a ppt marker > > > + bytestream2_get_byte(&s->g); // Zppt is skipped and not used > > > > > > > + if (!tile->packed_headers) > > > + tile->packed_headers = av_malloc(n - 3); > > > + else > > > + tile->packed_headers = av_realloc(tile->packed_headers, > > > + tile->packed_headers_size + n - 3); > > > > why is the special case with av_malloc() needed ? > > > The if else is to check whether this is the first time this tile is being > allocated its packed headers. If packed headers does not exist > exist, create it. The else condition is just to add a new packed > header to it. what is the problem with av_realloc(tile->packed_headers, tile->packed_headers_size + n - 3); when tile->packed_headers is NULL and tile->packed_headers_size is 0 for the first allocation ? thx [...]
On Mon, Apr 6, 2020 at 5:41 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Mon, Apr 06, 2020 at 09:23:26AM +0530, Gautam Ramakrishnan wrote: > > On Mon, Apr 6, 2020 at 1:16 AM Michael Niedermayer > > <michael@niedermayer.cc> wrote: > > > > > > On Sun, Apr 05, 2020 at 04:13:28PM +0530, gautamramk@gmail.com wrote: > > > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > > > > > This patch adds functional changes to support the > > > > PPT marker. This patch fixes bug ticket #4610. > > > > --- > > > > libavcodec/jpeg2000dec.c | 88 +++++++++++++++++++++++++++++++++++----- > > > > 1 file changed, 77 insertions(+), 11 deletions(-) > > > > > > > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c > > > > index 732d88e6fc..2af3c61c37 100644 > > > > --- a/libavcodec/jpeg2000dec.c > > > > +++ b/libavcodec/jpeg2000dec.c > > > > @@ -83,6 +83,10 @@ typedef struct Jpeg2000Tile { > > > > 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}} > > > > } Jpeg2000Tile; > > > > @@ -855,6 +859,37 @@ static int get_plt(Jpeg2000DecoderContext *s, int n) > > > > return 0; > > > > } > > > > > > > > +static int get_ppt(Jpeg2000DecoderContext *s, int n) > > > > +{ > > > > + Jpeg2000Tile *tile; > > > > + > > > > + if (s->curtileno < 0) > > > > + return AVERROR_INVALIDDATA; > > > > + > > > > + tile = &s->tile[s->curtileno]; > > > > + if (tile->tp_idx != 0) { > > > > + av_log(s->avctx, AV_LOG_ERROR, > > > > + "PPT marker can occur only on first tile part of a tile.\n"); > > > > + return AVERROR_INVALIDDATA; > > > > + } > > > > + > > > > + tile->has_ppt = 1; // this tile has a ppt marker > > > > + bytestream2_get_byte(&s->g); // Zppt is skipped and not used > > > > > > > > > > + if (!tile->packed_headers) > > > > + tile->packed_headers = av_malloc(n - 3); > > > > + else > > > > + tile->packed_headers = av_realloc(tile->packed_headers, > > > > + tile->packed_headers_size + n - 3); > > > > > > why is the special case with av_malloc() needed ? > > > > > The if else is to check whether this is the first time this tile is being > > allocated its packed headers. If packed headers does not exist > > exist, create it. The else condition is just to add a new packed > > header to it. > > what is the problem with > > av_realloc(tile->packed_headers, tile->packed_headers_size + n - 3); > when tile->packed_headers is NULL and tile->packed_headers_size is 0 for the > first allocation ? > Oh!! Thanks for clearing that out. I did not know realloc could be used like that. Was a bit skeptical about writing it that way. I'll test that out and resubmit. > > thx > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > In fact, the RIAA has been known to suggest that students drop out > of college or go to community college in order to be able to afford > settlements. -- The RIAA > _______________________________________________ > 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".
diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c index 732d88e6fc..2af3c61c37 100644 --- a/libavcodec/jpeg2000dec.c +++ b/libavcodec/jpeg2000dec.c @@ -83,6 +83,10 @@ typedef struct Jpeg2000Tile { 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}} } Jpeg2000Tile; @@ -855,6 +859,37 @@ static int get_plt(Jpeg2000DecoderContext *s, int n) return 0; } +static int get_ppt(Jpeg2000DecoderContext *s, int n) +{ + Jpeg2000Tile *tile; + + if (s->curtileno < 0) + return AVERROR_INVALIDDATA; + + tile = &s->tile[s->curtileno]; + if (tile->tp_idx != 0) { + av_log(s->avctx, AV_LOG_ERROR, + "PPT marker can occur only on first tile part of a tile.\n"); + return AVERROR_INVALIDDATA; + } + + tile->has_ppt = 1; // this tile has a ppt marker + bytestream2_get_byte(&s->g); // Zppt is skipped and not used + if (!tile->packed_headers) + tile->packed_headers = av_malloc(n - 3); + else + tile->packed_headers = av_realloc(tile->packed_headers, + tile->packed_headers_size + n - 3); + if (!tile->packed_headers) + return AVERROR(ENOMEM); + memcpy(tile->packed_headers + tile->packed_headers_size, + s->g.buffer, n - 3); + tile->packed_headers_size += n - 3; + bytestream2_skip(&s->g, n - 3); + + return 0; +} + static int init_tile(Jpeg2000DecoderContext *s, int tileno) { int compno; @@ -927,6 +962,19 @@ static int getlblockinc(Jpeg2000DecoderContext *s) return res; } +static inline void select_stream(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile, + int *tp_index) +{ + s->g = tile->tile_part[*tp_index].tpg; + if (bytestream2_get_bytes_left(&s->g) == 0 && s->bit_index == 8) { + if (*tp_index < FF_ARRAY_ELEMS(tile->tile_part) - 1) { + s->g = tile->tile_part[++(*tp_index)].tpg; + } + } + if (bytestream2_peek_be32(&s->g) == JPEG2000_SOP_FIXED_BYTES) + bytestream2_skip(&s->g, JPEG2000_SOP_BYTE_LENGTH); +} + static int jpeg2000_decode_packet(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile, int *tp_index, Jpeg2000CodingStyle *codsty, Jpeg2000ResLevel *rlevel, int precno, @@ -938,19 +986,15 @@ static int jpeg2000_decode_packet(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile, if (layno < rlevel->band[0].prec[precno].decoded_layers) return 0; rlevel->band[0].prec[precno].decoded_layers = layno + 1; - - if (bytestream2_get_bytes_left(&s->g) == 0 && s->bit_index == 8) { - if (*tp_index < FF_ARRAY_ELEMS(tile->tile_part) - 1) { - s->g = tile->tile_part[++(*tp_index)].tpg; - } - } - - if (bytestream2_peek_be32(&s->g) == JPEG2000_SOP_FIXED_BYTES) - bytestream2_skip(&s->g, JPEG2000_SOP_BYTE_LENGTH); + // Select stream to read from + if (tile->has_ppt) + s->g = tile->packed_headers_stream; + else + select_stream(s, tile, tp_index); if (!(ret = get_bits(s, 1))) { jpeg2000_flush(s); - return 0; + goto skip_data; } else if (ret < 0) return ret; @@ -1056,6 +1100,11 @@ static int jpeg2000_decode_packet(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile, av_log(s->avctx, AV_LOG_ERROR, "EPH marker not found. instead %X\n", bytestream2_peek_be32(&s->g)); } + // Save state of stream + if (tile->has_ppt) { + tile->packed_headers_stream = s->g; + select_stream(s, tile, tp_index); + } for (bandno = 0; bandno < rlevel->nbands; bandno++) { Jpeg2000Band *band = rlevel->band + bandno; Jpeg2000Prec *prec = band->prec + precno; @@ -1097,6 +1146,15 @@ static int jpeg2000_decode_packet(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile, av_freep(&cblk->lengthinc); } } + // Save state of stream + tile->tile_part[*tp_index].tpg = s->g; + return 0; + +skip_data: + if (tile->has_ppt) + tile->packed_headers_stream = s->g; + else + tile->tile_part[*tp_index].tpg = s->g; return 0; } @@ -1929,6 +1987,11 @@ static int jpeg2000_read_main_headers(Jpeg2000DecoderContext *s) av_log(s->avctx, AV_LOG_ERROR, "Invalid tpend\n"); return AVERROR_INVALIDDATA; } + + if (tile->has_ppt && tile->tp_idx == 0) { + bytestream2_init(&tile->packed_headers_stream, tile->packed_headers, tile->packed_headers_size); + } + bytestream2_init(&tp->tpg, s->g.buffer, tp->tp_end - s->g.buffer); bytestream2_skip(&s->g, tp->tp_end - s->g.buffer); @@ -1995,6 +2058,10 @@ static int jpeg2000_read_main_headers(Jpeg2000DecoderContext *s) // Packet length, tile-part header ret = get_plt(s, len); break; + case JPEG2000_PPT: + // Packed headers, tile-part header + ret = get_ppt(s, len); + break; default: av_log(s->avctx, AV_LOG_ERROR, "unsupported marker 0x%.4"PRIX16" at pos 0x%X\n", @@ -2024,7 +2091,6 @@ static int jpeg2000_read_bitstream_packets(Jpeg2000DecoderContext *s) if ((ret = init_tile(s, tileno)) < 0) return ret; - s->g = tile->tile_part[0].tpg; if ((ret = jpeg2000_decode_packets(s, tile)) < 0) return ret; }
From: Gautam Ramakrishnan <gautamramk@gmail.com> This patch adds functional changes to support the PPT marker. This patch fixes bug ticket #4610. --- libavcodec/jpeg2000dec.c | 88 +++++++++++++++++++++++++++++++++++----- 1 file changed, 77 insertions(+), 11 deletions(-)