Hi Pierre,

Thanks for suggesting the improvements.
All the suggestions have been reflected in version 8 of the patch set.

Best,
Osamu

> On Aug 6, 2024, at 2:28, Pierre-Anthony Lemieux <p...@sandflow.com> wrote:
> 
> Hi Osamu,
> 
> Below are suggested improvements to error messages.
> 
> There are also a couple of errant trailing white spaces in jpeg2000htdec.c.
> 
> Can you fix them?
> 
> Best,
> 
> -- Pierre
> 
> 
> On Sat, Aug 3, 2024 at 10:27?PM Osamu Watanabe
> <owata...@es.takushoku-u.ac.jp> wrote:
>> 
>> This commit adds support for CAP and CPF markers. Decoding will exit when 
>> encountering Ilegal input codestreams.
>> 
>> The variable `Jpeg2000DecoderContext->HT_MAGB` has been changed to `HT_B` 
>> according to the definition in the spec (ISO/IEC 15444-15).
>> 
>> Signed-off-by: Osamu Watanabe <owata...@es.takushoku-u.ac.jp>
>> ---
>> libavcodec/jpeg2000.h    |   8 +++
>> libavcodec/jpeg2000dec.c | 128 ++++++++++++++++++++++++++++++++++++++-
>> libavcodec/jpeg2000dec.h |   7 +++
>> 3 files changed, 142 insertions(+), 1 deletion(-)
>> 
>> diff --git a/libavcodec/jpeg2000.h b/libavcodec/jpeg2000.h
>> index d004c08f10..4bdc38df7c 100644
>> --- a/libavcodec/jpeg2000.h
>> +++ b/libavcodec/jpeg2000.h
>> @@ -37,12 +37,14 @@
>> 
>> enum Jpeg2000Markers {
>>     JPEG2000_SOC = 0xff4f, // start of codestream
>> +    JPEG2000_CAP = 0xff50, // extended capabilities
>>     JPEG2000_SIZ = 0xff51, // image and tile size
>>     JPEG2000_COD,          // coding style default
>>     JPEG2000_COC,          // coding style component
>>     JPEG2000_TLM = 0xff55, // tile-part length, main header
>>     JPEG2000_PLM = 0xff57, // packet length, main header
>>     JPEG2000_PLT,          // packet length, tile-part header
>> +    JPEG2000_CPF,          // corresponding profile
>>     JPEG2000_QCD = 0xff5c, // quantization default
>>     JPEG2000_QCC,          // quantization component
>>     JPEG2000_RGN,          // region of interest
>> @@ -58,6 +60,12 @@ enum Jpeg2000Markers {
>>     JPEG2000_EOC = 0xffd9, // end of codestream
>> };
>> 
>> +enum JPEG2000_Ccap15_b14_15_params {
>> +    HTJ2K_HTONLY = 0,      // HTONLY, bit 14 and 15 are 0
>> +    HTJ2K_HTDECLARED,      // HTDECLARED, bit 14 = 1 and bit 15 = 0
>> +    HTJ2K_MIXED = 3,       // MIXED, bit 14 and 15 are 1
>> +};
>> +
>> #define JPEG2000_SOP_FIXED_BYTES 0xFF910004
>> #define JPEG2000_SOP_BYTE_LENGTH 6
>> 
>> diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
>> index 091931b1ff..44db131d77 100644
>> --- a/libavcodec/jpeg2000dec.c
>> +++ b/libavcodec/jpeg2000dec.c
>> @@ -408,6 +408,73 @@ static int get_siz(Jpeg2000DecoderContext *s)
>>     s->avctx->bits_per_raw_sample = s->precision;
>>     return 0;
>> }
>> +/* get extended capabilities (CAP) marker segment */
>> +static int get_cap(Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *c)
>> +{
>> +    uint32_t Pcap;
>> +    uint16_t Ccap_i[32] = { 0 };
>> +    uint16_t Ccap_15;
>> +    uint8_t P;
>> +
>> +    if (bytestream2_get_bytes_left(&s->g) < 6) {
>> +        av_log(s->avctx, AV_LOG_ERROR, "Insufficient space for CAP\n");
> 
> av_log(s->avctx, AV_LOG_ERROR, "Underflow while parsing the CAP marker\n");
> 
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +
>> +    Pcap = bytestream2_get_be32u(&s->g);
>> +    s->isHT = (Pcap >> (31 - (15 - 1))) & 1;
>> +    for (int i = 0; i < 32; i++) {
>> +        if ((Pcap >> (31 - i)) & 1)
>> +            Ccap_i[i] = bytestream2_get_be16u(&s->g);
>> +    }
>> +    Ccap_15 = Ccap_i[14];
>> +    if (s->isHT == 1) {
>> +        av_log(s->avctx, AV_LOG_INFO, "This is an HTJ2K codestream.\n");
> 
> av_log(s->avctx, AV_LOG_INFO, "This codestream contains HT codeblocks.\n");
> 
>> +        // Bits 14-15
>> +        switch ((Ccap_15 >> 14) & 0x3) {
>> +            case 0x3:
>> +                s->Ccap15_b14_15 = HTJ2K_MIXED;
>> +                break;
>> +            case 0x1:
>> +                s->Ccap15_b14_15 = HTJ2K_HTDECLARED;
>> +                break;
>> +            case 0x0:
>> +                s->Ccap15_b14_15 = HTJ2K_HTONLY;
>> +                break;
>> +            default:
>> +                av_log(s->avctx, AV_LOG_ERROR, "Unknown CCap value.\n");
>> +                return AVERROR(EINVAL);
>> +                break;
>> +        }
>> +        // Bit 13
>> +        if ((Ccap_15 >> 13) & 1) {
>> +            av_log(s->avctx, AV_LOG_ERROR, "MULTIHT set is not 
>> supported.\n");
>> +            return AVERROR_PATCHWELCOME;
>> +        }
>> +        // Bit 12
>> +        s->Ccap15_b12 = (Ccap_15 >> 12) & 1;
>> +        // Bit 11
>> +        s->Ccap15_b11 = (Ccap_15 >> 11) & 1;
>> +        // Bit 5
>> +        s->Ccap15_b05 = (Ccap_15 >> 5) & 1;
>> +        // Bit 0-4
>> +        P = Ccap_15 & 0x1F;
>> +        if (!P)
>> +            s->HT_B = 8;
>> +        else if (P < 20)
>> +            s->HT_B = P + 8;
>> +        else if (P < 31)
>> +            s->HT_B = 4 * (P - 19) + 27;
>> +        else
>> +            s->HT_B = 74;
>> +
>> +        if (s->HT_B > 31) {
>> +            av_log(s->avctx, AV_LOG_ERROR, "Available internal precision is 
>> exceeded (MAGB> 31).\n");
> 
> av_log(s->avctx, AV_LOG_ERROR, "Codestream exceeds available internal
> precision (B > 31).\n");
> 
>> +            return AVERROR_PATCHWELCOME;
>> +        }
>> +    }
>> +    return 0;
>> +}
>> 
>> /* get common part for COD and COC segments */
>> static int get_cox(Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *c)
>> @@ -802,6 +869,15 @@ static int read_crg(Jpeg2000DecoderContext *s, int n)
>>     bytestream2_skip(&s->g, n - 2);
>>     return 0;
>> }
>> +
>> +static int read_cpf(Jpeg2000DecoderContext *s, int n)
>> +{
>> +    if (bytestream2_get_bytes_left(&s->g) < (n - 2))
>> +        return AVERROR_INVALIDDATA;
>> +    bytestream2_skip(&s->g, n - 2);
>> +    return 0;
>> +}
>> +
>> /* Tile-part lengths: see ISO 15444-1:2002, section A.7.1
>>  * Used to know the number of tile parts and lengths.
>>  * There may be multiple TLMs in the header.
>> @@ -965,6 +1041,14 @@ static int init_tile(Jpeg2000DecoderContext *s, int 
>> tileno)
>>             comp->roi_shift = s->roi_shift[compno];
>>         if (!codsty->init)
>>             return AVERROR_INVALIDDATA;
>> +        if (s->isHT && (!s->Ccap15_b05) && (!codsty->transform)) {
>> +            av_log(s->avctx, AV_LOG_ERROR, "Transformation = 0 (lossy DWT) 
>> is found in HTREV HT set\n");
>> +            return AVERROR_INVALIDDATA;
>> +        }
>> +        if (s->isHT && s->Ccap15_b14_15 != (codsty->cblk_style >> 6) && 
>> s->Ccap15_b14_15 != HTJ2K_HTONLY) {
>> +            av_log(s->avctx, AV_LOG_ERROR, "SPcod/SPcoc value does not 
>> match bit 14-15 values of Ccap15\n");
>> +            return AVERROR_INVALIDDATA;
>> +        }
>>         if (ret = ff_jpeg2000_init_component(comp, codsty, qntsty,
>>                                              s->cbps[compno], s->cdx[compno],
>>                                              s->cdy[compno], s->avctx))
>> @@ -2187,22 +2271,57 @@ static int 
>> jpeg2000_read_main_headers(Jpeg2000DecoderContext *s)
>>             if (!s->tile)
>>                 s->numXtiles = s->numYtiles = 0;
>>             break;
>> +        case JPEG2000_CAP:
>> +            if (!s->ncomponents) {
>> +                av_log(s->avctx, AV_LOG_ERROR, "CAP marker segment shall 
>> come after SIZ\n");
>> +                return AVERROR_INVALIDDATA;
>> +            }
>> +            ret = get_cap(s, codsty);
>> +            break;
>>         case JPEG2000_COC:
>> +            if (s->in_tile_headers == 1 && s->isHT && (!s->Ccap15_b11)) {
>> +                av_log(s->avctx, AV_LOG_ERROR, "COC marker is found in 
>> HOMOGENEOUS HT set\n");
> 
> av_log(s->avctx, AV_LOG_ERROR, "COC marker found in a tile header but
> the codestream belongs to the HOMOGENEOUS set\n");
> 
>> +                return AVERROR_INVALIDDATA;
>> +            }
>>             ret = get_coc(s, codsty, properties);
>>             break;
>>         case JPEG2000_COD:
>> +            if (s->in_tile_headers == 1 && s->isHT && (!s->Ccap15_b11)) {
>> +                av_log(s->avctx, AV_LOG_ERROR, "COD marker is found in 
>> HOMOGENEOUS HT set\n");
> 
> av_log(s->avctx, AV_LOG_ERROR, "COD marker found in a tile header but
> the codestream belongs to the HOMOGENEOUS set\n");
> 
>> +                return AVERROR_INVALIDDATA;
>> +            }
>>             ret = get_cod(s, codsty, properties);
>>             break;
>>         case JPEG2000_RGN:
>> +            if (s->in_tile_headers == 1 && s->isHT && (!s->Ccap15_b11)) {
>> +                av_log(s->avctx, AV_LOG_ERROR, "RGB marker is found in 
>> HOMOGENEOUS HT set\n");
> 
> av_log(s->avctx, AV_LOG_ERROR, "RGN marker found in a tile header but
> the codestream belongs to the HOMOGENEOUS set\n");
> 
>> +                return AVERROR_INVALIDDATA;
>> +            }
>>             ret = get_rgn(s, len);
>> +            if ((!s->Ccap15_b12) && s->isHT) {
>> +                av_log(s->avctx, AV_LOG_ERROR, "RGN marker is found in 
>> RGNFREE HT set\n");
> 
> av_log(s->avctx, AV_LOG_ERROR, "RGN marker found but the codestream
> belongs to the RGNFREE set\n");
> 
>> +                return AVERROR_INVALIDDATA;
>> +            }
>>             break;
>>         case JPEG2000_QCC:
>> +            if (s->in_tile_headers == 1 && s->isHT && (!s->Ccap15_b11)) {
>> +                av_log(s->avctx, AV_LOG_ERROR, "QCC marker is found in 
>> HOMOGENEOUS HT set\n");
> 
> av_log(s->avctx, AV_LOG_ERROR, "QCC marker found in a tile header but
> the codestream belongs to the HOMOGENEOUS set\n");
> 
>> +                return AVERROR_INVALIDDATA;
>> +            }
>>             ret = get_qcc(s, len, qntsty, properties);
>>             break;
>>         case JPEG2000_QCD:
>> +            if (s->in_tile_headers == 1 && s->isHT && (!s->Ccap15_b11)) {
>> +                av_log(s->avctx, AV_LOG_ERROR, "QCD marker is found in 
>> HOMOGENEOUS HT set\n");
> 
> av_log(s->avctx, AV_LOG_ERROR, "QCD marker found in a tile header but
> the codestream belongs to the HOMOGENEOUS set\n");
> 
>> +                return AVERROR_INVALIDDATA;
>> +            }
>>             ret = get_qcd(s, len, qntsty, properties);
>>             break;
>>         case JPEG2000_POC:
>> +            if (s->in_tile_headers == 1 && s->isHT && (!s->Ccap15_b11)) {
>> +                av_log(s->avctx, AV_LOG_ERROR, "POC marker is found in 
>> HOMOGENEOUS HT set\n");
> 
> av_log(s->avctx, AV_LOG_ERROR, "POC marker found in a tile header but
> the codestream belongs to the HOMOGENEOUS set\n");
> 
>> +                return AVERROR_INVALIDDATA;
>> +            }
>>             ret = get_poc(s, len, poc);
>>             break;
>>         case JPEG2000_SOT:
>> @@ -2252,9 +2371,16 @@ static int 
>> jpeg2000_read_main_headers(Jpeg2000DecoderContext *s)
>>                        "Cannot have both PPT and PPM marker.\n");
>>                 return AVERROR_INVALIDDATA;
>>             }
>> -
>> +            if ((!s->Ccap15_b11) && s->isHT) {
>> +                av_log(s->avctx, AV_LOG_ERROR, "PPT marker is found in 
>> HOMOGENEOUS HT set\n");
> 
> av_log(s->avctx, AV_LOG_ERROR, "PPT marker found but the codestream
> belongs to the HOMOGENEOUS set\n");
> 
>> +                return AVERROR_INVALIDDATA;
>> +            }
>>             ret = get_ppt(s, len);
>>             break;
>> +        case JPEG2000_CPF:
>> +            // Corresponding profile marker
>> +            ret = read_cpf(s, len);
>> +            break;
>>         default:
>>             av_log(s->avctx, AV_LOG_ERROR,
>>                    "unsupported marker 0x%.4"PRIX16" at pos 0x%X\n",
>> diff --git a/libavcodec/jpeg2000dec.h b/libavcodec/jpeg2000dec.h
>> index d0ca6e7a79..166cce9eba 100644
>> --- a/libavcodec/jpeg2000dec.h
>> +++ b/libavcodec/jpeg2000dec.h
>> @@ -112,6 +112,13 @@ typedef struct Jpeg2000DecoderContext {
>>     Jpeg2000Tile    *tile;
>>     Jpeg2000DSPContext dsp;
>> 
>> +    uint8_t         isHT; // HTJ2K?
>> +    uint8_t         Ccap15_b14_15; // HTONLY(= 0) or HTDECLARED(= 1) or 
>> MIXED(= 3) ?
>> +    uint8_t         Ccap15_b12; // RGNFREE(= 0) or RGN(= 1)?
>> +    uint8_t         Ccap15_b11; // HOMOGENEOUS(= 0) or HETEROGENEOUS(= 1) ?
>> +    uint8_t         Ccap15_b05; // HTREV(= 0) or HTIRV(= 1) ?
>> +    uint8_t         HT_B; // MAGB value
>> +
>>     /*options parameters*/
>>     int             reduction_factor;
>> } Jpeg2000DecoderContext;
>> --
>> 2.34.1


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to