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

[Gautam Ramakrishnan]

On Tue, Mar 31, 2020 at 6:10 AM Michael Niedermayer
<michael at niedermayer.cc> wrote:
>
> 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
>
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 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".



--
-------------
Gautam |