[FFmpeg-devel] [PATCH v8 1/3] libavcodec/jpeg2000dec.c: Split function and modify structs for PPM marker support

Michael Niedermayer michael at niedermayer.cc
Tue Mar 31 03:40:06 EEST 2020


On Mon, Mar 30, 2020 at 09:51:53PM +0530, gautamramk at gmail.com wrote:
> From: Gautam Ramakrishnan <gautamramk at 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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200331/afbdf022/attachment.sig>


More information about the ffmpeg-devel mailing list