| Message ID | 20200330162155.6901-1-gautamramk@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v8,1/3] libavcodec/jpeg2000dec.c: Split function and modify structs for PPM marker support | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
On Mon, Mar 30, 2020 at 09:51:53PM +0530, gautamramk@gmail.com wrote: > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > --- > libavcodec/jpeg2000dec.c | 48 ++++++++++++++++++++++++++++++++++------ > 1 file changed, 41 insertions(+), 7 deletions(-) > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c > index 7103cd6ceb..9d52969821 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; > @@ -935,21 +939,32 @@ static int jpeg2000_decode_packet(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile, > int bandno, cblkno, ret, nb_code_blocks; > int cwsno; > > +// This part decodes the packet header > + > 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; > + // Select appropriate stream to read from > + if (tile->has_ppt) { > + s->g = tile->packed_headers_stream; > + } else { > + 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); > } > > - if (bytestream2_peek_be32(&s->g) == JPEG2000_SOP_FIXED_BYTES) > - bytestream2_skip(&s->g, JPEG2000_SOP_BYTE_LENGTH); > - > if (!(ret = get_bits(s, 1))) { > jpeg2000_flush(s); > + // Save state of stream > + if (tile->has_ppt) > + tile->packed_headers_stream = s->g; > + else > + tile->tile_part[*tp_index].tpg = s->g; > return 0; > } else if (ret < 0) > return ret; > @@ -1056,6 +1071,23 @@ 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)); > } > > + if (tile->has_ppt) > + tile->packed_headers_stream = s->g; > + else > + tile->tile_part[*tp_index].tpg = s->g; > + > +// This part decodes the packet data. > + // Select appropriate stream to read from > + s->g = tile->tile_part[*tp_index].tpg; > + if (tile->has_ppt) { > + 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); > + } > for (bandno = 0; bandno < rlevel->nbands; bandno++) { > Jpeg2000Band *band = rlevel->band + bandno; > Jpeg2000Prec *prec = band->prec + precno; > @@ -1097,6 +1129,8 @@ 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; > } has_ppt is never set in this also it is specific to the new functional code. it and all branches which depend on it being 1 should be in the 2nd patch. The function split should be in the first patch instead Its probably easiest if you first merge the 2 patches and then remove all ppt feature specific code which was added. what remains should be the first patch. the diff from that to the final result would be the 2nd. At least i think that would bring it closer to what the idea is with this split. you / we need to take a look at how this looks after doing it. There may still be obvious things that can be cleaned up ... thx [...]
On Tue, Mar 31, 2020 at 6:10 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Mon, Mar 30, 2020 at 09:51:53PM +0530, gautamramk@gmail.com wrote: > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > --- > > libavcodec/jpeg2000dec.c | 48 ++++++++++++++++++++++++++++++++++------ > > 1 file changed, 41 insertions(+), 7 deletions(-) > > > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c > > index 7103cd6ceb..9d52969821 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; > > @@ -935,21 +939,32 @@ static int jpeg2000_decode_packet(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile, > > int bandno, cblkno, ret, nb_code_blocks; > > int cwsno; > > > > +// This part decodes the packet header > > + > > 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; > > + // Select appropriate stream to read from > > + if (tile->has_ppt) { > > + s->g = tile->packed_headers_stream; > > + } else { > > + 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); > > } > > > > - if (bytestream2_peek_be32(&s->g) == JPEG2000_SOP_FIXED_BYTES) > > - bytestream2_skip(&s->g, JPEG2000_SOP_BYTE_LENGTH); > > - > > if (!(ret = get_bits(s, 1))) { > > jpeg2000_flush(s); > > + // Save state of stream > > + if (tile->has_ppt) > > + tile->packed_headers_stream = s->g; > > + else > > + tile->tile_part[*tp_index].tpg = s->g; > > return 0; > > } else if (ret < 0) > > return ret; > > @@ -1056,6 +1071,23 @@ 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)); > > } > > > > + if (tile->has_ppt) > > + tile->packed_headers_stream = s->g; > > + else > > + tile->tile_part[*tp_index].tpg = s->g; > > + > > +// This part decodes the packet data. > > + // Select appropriate stream to read from > > + s->g = tile->tile_part[*tp_index].tpg; > > + if (tile->has_ppt) { > > + 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); > > + } > > for (bandno = 0; bandno < rlevel->nbands; bandno++) { > > Jpeg2000Band *band = rlevel->band + bandno; > > Jpeg2000Prec *prec = band->prec + precno; > > @@ -1097,6 +1129,8 @@ 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; > > } > > has_ppt is never set in this also it is specific to the new functional code. > it and all branches which depend on it being 1 should be in the 2nd patch. > > The function split should be in the first patch instead > Sure, now its clear. Shall do that > Its probably easiest if you first merge the 2 patches and then remove all > ppt feature specific code which was added. what remains should be the first > patch. the diff from that to the final result would be the 2nd. > At least i think that would bring it closer to what the idea is with this > split. you / we need to take a look at how this looks after doing it. > There may still be obvious things that can be cleaned up ... > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > No snowflake in an avalanche ever feels responsible. -- Voltaire > _______________________________________________ > 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". -- ------------- Gautam |
diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c index 7103cd6ceb..9d52969821 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; @@ -935,21 +939,32 @@ static int jpeg2000_decode_packet(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile, int bandno, cblkno, ret, nb_code_blocks; int cwsno; +// This part decodes the packet header + 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; + // Select appropriate stream to read from + if (tile->has_ppt) { + s->g = tile->packed_headers_stream; + } else { + 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); } - if (bytestream2_peek_be32(&s->g) == JPEG2000_SOP_FIXED_BYTES) - bytestream2_skip(&s->g, JPEG2000_SOP_BYTE_LENGTH); - if (!(ret = get_bits(s, 1))) { jpeg2000_flush(s); + // Save state of stream + if (tile->has_ppt) + tile->packed_headers_stream = s->g; + else + tile->tile_part[*tp_index].tpg = s->g; return 0; } else if (ret < 0) return ret; @@ -1056,6 +1071,23 @@ 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)); } + if (tile->has_ppt) + tile->packed_headers_stream = s->g; + else + tile->tile_part[*tp_index].tpg = s->g; + +// This part decodes the packet data. + // Select appropriate stream to read from + s->g = tile->tile_part[*tp_index].tpg; + if (tile->has_ppt) { + 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); + } for (bandno = 0; bandno < rlevel->nbands; bandno++) { Jpeg2000Band *band = rlevel->band + bandno; Jpeg2000Prec *prec = band->prec + precno; @@ -1097,6 +1129,8 @@ 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; }
From: Gautam Ramakrishnan <gautamramk@gmail.com> --- libavcodec/jpeg2000dec.c | 48 ++++++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 7 deletions(-)