On Fri, Mar 31, 2023 at 08:29:40AM -0700, pal@sandflow.com wrote:
> From: caleb <etemesicaleb@gmail.com>
[...]
> @@ -1113,8 +1117,29 @@ static int jpeg2000_decode_packet(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile,
>                      }
>                  }
>  
> -                if ((ret = get_bits(s, av_log2(newpasses1) + cblk->lblock)) < 0)
> -                    return ret;
> +                if (newpasses > 1 && s->is_htj2k) {
> +                    // Retrieve pass lengths for each pass
> +                    int href_passes =  (cblk->npasses + newpasses - 1) % 3;
> +                    int segment_passes = newpasses - href_passes;
> +                    int pass_bound = 2;
> +                    int eb = 0;
> +                    int extra_bit = newpasses > 2 ? 1 : 0;
> +                    while (pass_bound <=segment_passes) {
> +                        eb++;
> +                        pass_bound +=pass_bound;
> +                    }

something with av_log2() should be able to do this simpler


[...]
> +/**
> + * Drops bits from lower bits in the bit buffer. buf contains the bit buffers.
> + * nbits is the number of bits to remove.
> + */
> +av_always_inline
> +static void jpeg2000_bitbuf_drop_bits_lsb(StateVars *buf, uint8_t nbits)
> +{
> +    if (buf->bits_left < nbits) {

> +        av_log(NULL, AV_LOG_ERROR, "Invalid bit read of %d, bits in buffer are %d\n", nbits, buf->bits_left);
> +        av_assert0(0);


[...]
> +int
> +ff_jpeg2000_decode_htj2k(const Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *codsty, Jpeg2000T1Context *t1, Jpeg2000Cblk *cblk,
> +                         int width, int height, int magp, uint8_t roi_shift)
> +{
> +    uint8_t p0 = 0;             // Number of placeholder passes
> +    uint32_t Lcup;              // Length of HT cleanup segment
> +    uint32_t Lref;              // Length of Refinement segment
> +    uint32_t Scup;              // HT cleanup segment suffix length
> +    uint32_t Pcup;              // HT cleanup segment prefix length
> +
> +    uint8_t S_blk;              // Number of skipped magnitude bitplanes
> +    uint8_t pLSB;
> +
> +    uint8_t *Dcup;              // Byte of an HT cleanup segment
> +    uint8_t *Dref;              // Byte of an HT refinement segment
> +
> +    int z_blk;                  // Number of ht coding pass
> +
> +    uint8_t empty_passes;
> +
> +    StateVars mag_sgn;          // Magnitude and Sign
> +    StateVars mel;              // Adaptive run-length coding
> +    StateVars vlc;              // Variable Length coding
> +    StateVars sig_prop;         // Significance propagation
> +
> +    MelDecoderState mel_state;
> +
> +    int ret;
> +
> +    /* Temporary buffers */
> +    int32_t *sample_buf;
> +    uint8_t *block_states;
> +
> +    int32_t n, val;             // Post-processing
> +
> +    int32_t M_b = magp;
> +    av_assert0(width <= 1024U && height <= 1024U);
> +    av_assert0(width * height <= 4096);
> +    av_assert0(width * height > 0);

Has this decoder been tested with some fuzzer ?
I see a bunch of asserts in it and i dont immedeatly see what would prevent them from
triggering

thx

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

"I am not trying to be anyone's saviour, I'm trying to think about the
 future and not be sad" - Elon Musk