[FFmpeg-devel] [PATCH v10 1/3] libavcodec/jpeg2000dec.c: Add support for PPT marker

[Michael Niedermayer]

On Fri, Apr 03, 2020 at 11:57:56PM +0530, gautamramk at gmail.com wrote:
> From: Gautam Ramakrishnan <gautamramk at gmail.com>
> 
> This patch adds functional changes to support the
> PPT marker.
> ---
>  libavcodec/jpeg2000dec.c | 88 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 79 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> index 7103cd6ceb..02905b9e49 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,39 @@ 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
> +/*    Zppt = */ bytestream2_get_byte(&s->g); // Zppt is skipped and not used
> +    if (!tile->packed_headers) {
> +        tile->packed_headers = av_malloc_array(n - 3, 1);

av_malloc() or av_realloc seems to make more sense here, the *1 isnt usefull


> +        memcpy(tile->packed_headers, s->g.buffer, n - 3);

missing check for malloc failure


> +        tile->packed_headers_size = n - 3;
> +    } else {
> +        tile->packed_headers = av_realloc_array(tile->packed_headers,
> +                                                tile->packed_headers_size + n - 3,
> +                                                1);
> +        memcpy(tile->packed_headers + tile->packed_headers_size,
> +               s->g.buffer, n - 3);
> +        tile->packed_headers_size += n - 3;
> +    }

isnt packed_headers_size 0 before the initial allocation ?
if so the if and else can be simplified



> +    bytestream2_skip(&s->g, n - 3);
> +
> +    return 0;
> +}
> +
>  static int init_tile(Jpeg2000DecoderContext *s, int tileno)
>  {
>      int compno;
> @@ -938,19 +975,23 @@ 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;
> +    // Select 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);
> -        return 0;
> +        goto skip_data;
>      } else if (ret < 0)
>          return ret;
>  
> @@ -1056,6 +1097,18 @@ 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;
> +        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);

This looks like duplicated code, maybe this is cleaner if its factored into a seperate function

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- 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/20200404/5e468733/attachment.sig>