diff mbox series

[FFmpeg-devel,v4,1/3] avcodec/jpeg2000dec: Add support for CAP and CPF markers

Message ID 20240624133734.3032409-1-owatanab@es.takushoku-u.ac.jp
State New
Headers show
Series [FFmpeg-devel,v4,1/3] avcodec/jpeg2000dec: Add support for CAP and CPF markers | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Osamu Watanabe June 24, 2024, 1:37 p.m. UTC
This commit adds support for CAP and CPF markers.

The previous version
(v3, https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=12246)
was wrongly separated. I have fixed the way to separation.

The changes are essentially the same as v2
(https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=12199).
The suggested modifications have been made according to the discussion
on this mailing list.

Signed-off-by: Osamu Watanabe <owatanab@es.takushoku-u.ac.jp>
---
 libavcodec/jpeg2000.h    |   8 +++
 libavcodec/jpeg2000dec.c | 112 ++++++++++++++++++++++++++++++++++++++-
 libavcodec/jpeg2000dec.h |   7 +++
 3 files changed, 126 insertions(+), 1 deletion(-)

Comments

Pierre-Anthony Lemieux July 12, 2024, 4:57 a.m. UTC | #1
I checked that the patch results in FFMPEG decoding without error the
additional conformance codestreams at [1].

[1] https://ffmpeg.org/pipermail/ffmpeg-devel/2024-June/329727.html

@Osamu, did you check if the decoded images fall within the tolerances
specified in 15444-4?

There is an errant space character at the end of four lines for some
reason. I can fix that before merging.

Any other comments? If not, I plan to merge sometime this week, and
plan to then submit a patch to add the conformance codestream to FATE.

On Mon, Jun 24, 2024 at 6:37 AM Osamu Watanabe
<owatanab@es.takushoku-u.ac.jp> wrote:
>
> This commit adds support for CAP and CPF markers.
>
> The previous version
> (v3, https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=12246)
> was wrongly separated. I have fixed the way to separation.
>
> The changes are essentially the same as v2
> (https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=12199).
> The suggested modifications have been made according to the discussion
> on this mailing list.
>
> Signed-off-by: Osamu Watanabe <owatanab@es.takushoku-u.ac.jp>
> ---
>  libavcodec/jpeg2000.h    |   8 +++
>  libavcodec/jpeg2000dec.c | 112 ++++++++++++++++++++++++++++++++++++++-
>  libavcodec/jpeg2000dec.h |   7 +++
>  3 files changed, 126 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..d1046661c4 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");
> +        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");
> +        // 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_MAGB = 8;
> +        else if (P < 20)
> +            s->HT_MAGB = P + 8;
> +        else if (P < 31)
> +            s->HT_MAGB = 4 * (P - 19) + 27;
> +        else
> +            s->HT_MAGB = 74;
> +
> +        if (s->HT_MAGB > 31) {
> +                av_log(s->avctx, AV_LOG_ERROR, "Available internal precision is exceeded (MAGB> 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,43 @@ 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_WARNING, "COC marker is found in HOMOGENEOUS HT set\n");
>              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_WARNING, "COD marker is found in HOMOGENEOUS HT set\n");
>              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_WARNING, "RGB marker is found in HOMOGENEOUS HT set\n");
>              ret = get_rgn(s, len);
> +            if ((!s->Ccap15_b12) && s->isHT)
> +                av_log(s->avctx, AV_LOG_WARNING, "RGN marker is found in RGNFREE HT set\n");
>              break;
>          case JPEG2000_QCC:
> +            if (s->in_tile_headers == 1 && s->isHT && (!s->Ccap15_b11))
> +                av_log(s->avctx, AV_LOG_WARNING, "QCC marker is found in HOMOGENEOUS HT set\n");
>              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_WARNING, "QCD marker is found in HOMOGENEOUS HT set\n");
>              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_WARNING, "POC marker is found in HOMOGENEOUS HT set\n");
>              ret = get_poc(s, len, poc);
>              break;
>          case JPEG2000_SOT:
> @@ -2252,9 +2357,14 @@ 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_WARNING, "PPT marker is found in HOMOGENEOUS HT set\n");
>              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..326a572722 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_MAGB; // MAGB value
> +
>      /*options parameters*/
>      int             reduction_factor;
>  } Jpeg2000DecoderContext;
> --
> 2.34.1
>
Tomas Härdin July 12, 2024, 5:27 a.m. UTC | #2
> +            if (s->in_tile_headers == 1 && s->isHT && (!s-
> >Ccap15_b11))
> +                av_log(s->avctx, AV_LOG_WARNING, "COD marker is
> found in HOMOGENEOUS HT set\n");

How bad is this and the other markers being present in this case?
Should we perhaps error out?

/Tomas
Pierre-Anthony Lemieux July 12, 2024, 7:51 p.m. UTC | #3
On Thu, Jul 11, 2024 at 10:28 PM Tomas Härdin <git@haerdin.se> wrote:
>
> > +            if (s->in_tile_headers == 1 && s->isHT && (!s-
> > >Ccap15_b11))
> > +                av_log(s->avctx, AV_LOG_WARNING, "COD marker is
> > found in HOMOGENEOUS HT set\n");
>
> How bad is this and the other markers being present in this case?

At the very least, it means that signaling is inconsistent within the
codestream since the standard states that:
"""
The HOMOGENEOUS set is the set of HTJ2K codestreams where:
• none of the functional marker segments, e.g., COD, COC, RGN, QCD,
QCC, and POC, are present in any
tile-part header; and
• no PPT marker segment is present.
"""

The point of signalling that a codestream is "HOMOGENEOUS" is to allow
decoders to configure themselves solely based on information retrieved
entirely from the main header.

Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS to short-circuit
configuration, incorrect HOMOGENEOUS signalling will likely not impact
FFMPEG.

This condition may impact downstream decoders and might signal
something deeply wrong with the codestream.

In any case, maybe the case ought to be clarified to something along
the lines of: "Non-conformant codestream: a COD marker is present in a
tile-part header even though the codestream is marked as HOMOGENEOUS."

> Should we perhaps error out?
>
> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Osamu Watanabe July 13, 2024, 10:31 a.m. UTC | #4
> @Osamu, did you check if the decoded images fall within the tolerances
> specified in 15444-4?

Yes, I did. It has been confirmed that the decoder passes all the test cases defined in 15444-4.
The errors (PAE and MSE) in the reconstructed images are within the allowable tolerances.



> On Jul 12, 2024, at 13:57, Pierre-Anthony Lemieux <pal@sandflow.com> wrote:
> 
> I checked that the patch results in FFMPEG decoding without error the
> additional conformance codestreams at [1].
> 
> [1] https://ffmpeg.org/pipermail/ffmpeg-devel/2024-June/329727.html
> 
> @Osamu, did you check if the decoded images fall within the tolerances
> specified in 15444-4?
> 
> There is an errant space character at the end of four lines for some
> reason. I can fix that before merging.
> 
> Any other comments? If not, I plan to merge sometime this week, and
> plan to then submit a patch to add the conformance codestream to FATE.
> 
> On Mon, Jun 24, 2024 at 6:37?AM Osamu Watanabe
> <owatanab@es.takushoku-u.ac.jp> wrote:
>> 
>> This commit adds support for CAP and CPF markers.
>> 
>> The previous version
>> (v3, https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=12246)
>> was wrongly separated. I have fixed the way to separation.
>> 
>> The changes are essentially the same as v2
>> (https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=12199).
>> The suggested modifications have been made according to the discussion
>> on this mailing list.
>> 
>> Signed-off-by: Osamu Watanabe <owatanab@es.takushoku-u.ac.jp>
>> ---
>> libavcodec/jpeg2000.h    |   8 +++
>> libavcodec/jpeg2000dec.c | 112 ++++++++++++++++++++++++++++++++++++++-
>> libavcodec/jpeg2000dec.h |   7 +++
>> 3 files changed, 126 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..d1046661c4 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");
>> +        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");
>> +        // 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_MAGB = 8;
>> +        else if (P < 20)
>> +            s->HT_MAGB = P + 8;
>> +        else if (P < 31)
>> +            s->HT_MAGB = 4 * (P - 19) + 27;
>> +        else
>> +            s->HT_MAGB = 74;
>> +
>> +        if (s->HT_MAGB > 31) {
>> +                av_log(s->avctx, AV_LOG_ERROR, "Available internal precision is exceeded (MAGB> 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,43 @@ 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_WARNING, "COC marker is found in HOMOGENEOUS HT set\n");
>>             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_WARNING, "COD marker is found in HOMOGENEOUS HT set\n");
>>             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_WARNING, "RGB marker is found in HOMOGENEOUS HT set\n");
>>             ret = get_rgn(s, len);
>> +            if ((!s->Ccap15_b12) && s->isHT)
>> +                av_log(s->avctx, AV_LOG_WARNING, "RGN marker is found in RGNFREE HT set\n");
>>             break;
>>         case JPEG2000_QCC:
>> +            if (s->in_tile_headers == 1 && s->isHT && (!s->Ccap15_b11))
>> +                av_log(s->avctx, AV_LOG_WARNING, "QCC marker is found in HOMOGENEOUS HT set\n");
>>             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_WARNING, "QCD marker is found in HOMOGENEOUS HT set\n");
>>             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_WARNING, "POC marker is found in HOMOGENEOUS HT set\n");
>>             ret = get_poc(s, len, poc);
>>             break;
>>         case JPEG2000_SOT:
>> @@ -2252,9 +2357,14 @@ 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_WARNING, "PPT marker is found in HOMOGENEOUS HT set\n");
>>             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..326a572722 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_MAGB; // 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-request@ffmpeg.org with subject "unsubscribe".
Tomas Härdin July 15, 2024, 1:32 p.m. UTC | #5
fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony Lemieux:
> On Thu, Jul 11, 2024 at 10:28 PM Tomas Härdin <git@haerdin.se> wrote:
> > 
> > > +            if (s->in_tile_headers == 1 && s->isHT && (!s-
> > > > Ccap15_b11))
> > > +                av_log(s->avctx, AV_LOG_WARNING, "COD marker is
> > > found in HOMOGENEOUS HT set\n");
> > 
> > How bad is this and the other markers being present in this case?
> 
> At the very least, it means that signaling is inconsistent within the
> codestream since the standard states that:
> """
> The HOMOGENEOUS set is the set of HTJ2K codestreams where:
> • none of the functional marker segments, e.g., COD, COC, RGN, QCD,
> QCC, and POC, are present in any
> tile-part header; and
> • no PPT marker segment is present.
> """
> 
> The point of signalling that a codestream is "HOMOGENEOUS" is to
> allow
> decoders to configure themselves solely based on information
> retrieved
> entirely from the main header.
> 
> Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS to short-
> circuit
> configuration, incorrect HOMOGENEOUS signalling will likely not
> impact
> FFMPEG.

It could happen that information in tile headers contradict information
in the main header, right? In such a case it sounds like we can't be
sure which decode is the correct one.

Much like with broken MXF files I think we should error out because of
the potential ambiguity, to discourage broken encoders from
proliferating. Else we'll have to deal with them perpetually

/Tomas
Pierre-Anthony Lemieux July 18, 2024, 2:10 p.m. UTC | #6
On Mon, Jul 15, 2024 at 10:33 PM Tomas Härdin <git@haerdin.se> wrote:
>
> fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony Lemieux:
> > On Thu, Jul 11, 2024 at 10:28 PM Tomas Härdin <git@haerdin.se> wrote:
> > >
> > > > +            if (s->in_tile_headers == 1 && s->isHT && (!s-
> > > > > Ccap15_b11))
> > > > +                av_log(s->avctx, AV_LOG_WARNING, "COD marker is
> > > > found in HOMOGENEOUS HT set\n");
> > >
> > > How bad is this and the other markers being present in this case?
> >
> > At the very least, it means that signaling is inconsistent within the
> > codestream since the standard states that:
> > """
> > The HOMOGENEOUS set is the set of HTJ2K codestreams where:
> > • none of the functional marker segments, e.g., COD, COC, RGN, QCD,
> > QCC, and POC, are present in any
> > tile-part header; and
> > • no PPT marker segment is present.
> > """
> >
> > The point of signalling that a codestream is "HOMOGENEOUS" is to
> > allow
> > decoders to configure themselves solely based on information
> > retrieved
> > entirely from the main header.
> >
> > Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS to short-
> > circuit
> > configuration, incorrect HOMOGENEOUS signalling will likely not
> > impact
> > FFMPEG.
>
> It could happen that information in tile headers contradict information
> in the main header, right? In such a case it sounds like we can't be
> sure which decode is the correct one.

Per the spec, the decoder uses the COD information in tile-parts over
the COD information in the header.

The issue here is that a decoder, upon seeing HOMOGENEOUS, simply does
not bother with looking for COD information in tile-parts, thereby
missing critical information.

>
> Much like with broken MXF files I think we should error out because of
> the potential ambiguity, to discourage broken encoders from
> proliferating. Else we'll have to deal with them perpetually

I agree. I also think that, in this particular case, exiting decoding
seems extreme since the error does not cause an inconsistent state
within the FFMPEG decoder. An application could always choose to
interpret all FFMPEG warnings as fatal errors.

Any other  opinions?

>
> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Tomas Härdin July 20, 2024, 8:12 a.m. UTC | #7
tor 2024-07-18 klockan 23:10 +0900 skrev Pierre-Anthony Lemieux:
> On Mon, Jul 15, 2024 at 10:33 PM Tomas Härdin <git@haerdin.se> wrote:
> > 
> > fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony Lemieux:
> > > On Thu, Jul 11, 2024 at 10:28 PM Tomas Härdin <git@haerdin.se>
> > > wrote:
> > > > 
> > > > > +            if (s->in_tile_headers == 1 && s->isHT && (!s-
> > > > > > Ccap15_b11))
> > > > > +                av_log(s->avctx, AV_LOG_WARNING, "COD marker
> > > > > is
> > > > > found in HOMOGENEOUS HT set\n");
> > > > 
> > > > How bad is this and the other markers being present in this
> > > > case?
> > > 
> > > At the very least, it means that signaling is inconsistent within
> > > the
> > > codestream since the standard states that:
> > > """
> > > The HOMOGENEOUS set is the set of HTJ2K codestreams where:
> > > • none of the functional marker segments, e.g., COD, COC, RGN,
> > > QCD,
> > > QCC, and POC, are present in any
> > > tile-part header; and
> > > • no PPT marker segment is present.
> > > """
> > > 
> > > The point of signalling that a codestream is "HOMOGENEOUS" is to
> > > allow
> > > decoders to configure themselves solely based on information
> > > retrieved
> > > entirely from the main header.
> > > 
> > > Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS to short-
> > > circuit
> > > configuration, incorrect HOMOGENEOUS signalling will likely not
> > > impact
> > > FFMPEG.
> > 
> > It could happen that information in tile headers contradict
> > information
> > in the main header, right? In such a case it sounds like we can't
> > be
> > sure which decode is the correct one.
> 
> Per the spec, the decoder uses the COD information in tile-parts over
> the COD information in the header.
> 
> The issue here is that a decoder, upon seeing HOMOGENEOUS, simply
> does
> not bother with looking for COD information in tile-parts, thereby
> missing critical information.

So it is actually perfectly legal? Then it seems this patch is wrong

> > Much like with broken MXF files I think we should error out because
> > of
> > the potential ambiguity, to discourage broken encoders from
> > proliferating. Else we'll have to deal with them perpetually
> 
> I agree. I also think that, in this particular case, exiting decoding
> seems extreme since the error does not cause an inconsistent state
> within the FFMPEG decoder. An application could always choose to
> interpret all FFMPEG warnings as fatal errors.

Well yeah, users can choose to handle errors any way they see fit. We
could return AVERROR_INVALIDDATA but also set *got_frame to signal
"this frame is probably broken but here you go anyway"

/Tomas
Pierre-Anthony Lemieux July 21, 2024, 5:07 a.m. UTC | #8
On Sat, Jul 20, 2024 at 5:12 PM Tomas Härdin <git@haerdin.se> wrote:
>
> tor 2024-07-18 klockan 23:10 +0900 skrev Pierre-Anthony Lemieux:
> > On Mon, Jul 15, 2024 at 10:33 PM Tomas Härdin <git@haerdin.se> wrote:
> > >
> > > fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony Lemieux:
> > > > On Thu, Jul 11, 2024 at 10:28 PM Tomas Härdin <git@haerdin.se>
> > > > wrote:
> > > > >
> > > > > > +            if (s->in_tile_headers == 1 && s->isHT && (!s-
> > > > > > > Ccap15_b11))
> > > > > > +                av_log(s->avctx, AV_LOG_WARNING, "COD marker
> > > > > > is
> > > > > > found in HOMOGENEOUS HT set\n");
> > > > >
> > > > > How bad is this and the other markers being present in this
> > > > > case?
> > > >
> > > > At the very least, it means that signaling is inconsistent within
> > > > the
> > > > codestream since the standard states that:
> > > > """
> > > > The HOMOGENEOUS set is the set of HTJ2K codestreams where:
> > > > • none of the functional marker segments, e.g., COD, COC, RGN,
> > > > QCD,
> > > > QCC, and POC, are present in any
> > > > tile-part header; and
> > > > • no PPT marker segment is present.
> > > > """
> > > >
> > > > The point of signalling that a codestream is "HOMOGENEOUS" is to
> > > > allow
> > > > decoders to configure themselves solely based on information
> > > > retrieved
> > > > entirely from the main header.
> > > >
> > > > Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS to short-
> > > > circuit
> > > > configuration, incorrect HOMOGENEOUS signalling will likely not
> > > > impact
> > > > FFMPEG.
> > >
> > > It could happen that information in tile headers contradict
> > > information
> > > in the main header, right? In such a case it sounds like we can't
> > > be
> > > sure which decode is the correct one.
> >
> > Per the spec, the decoder uses the COD information in tile-parts over
> > the COD information in the header.
> >
> > The issue here is that a decoder, upon seeing HOMOGENEOUS, simply
> > does
> > not bother with looking for COD information in tile-parts, thereby
> > missing critical information.
>
> So it is actually perfectly legal? Then it seems this patch is wrong

What is not "illegal": the HOMOGENEOUS flag being equal to true *and*
having COD marker segments in tile-parts.

This is what the patch detects.

FFMPEG can decode such illegal codestream. Other decoders might not.

The question is: what should FFMPEG do? Should FFMPEG exit or warn and continue.

>
> > > Much like with broken MXF files I think we should error out because
> > > of
> > > the potential ambiguity, to discourage broken encoders from
> > > proliferating. Else we'll have to deal with them perpetually
> >
> > I agree. I also think that, in this particular case, exiting decoding
> > seems extreme since the error does not cause an inconsistent state
> > within the FFMPEG decoder. An application could always choose to
> > interpret all FFMPEG warnings as fatal errors.
>
> Well yeah, users can choose to handle errors any way they see fit. We
> could return AVERROR_INVALIDDATA but also set *got_frame to signal
> "this frame is probably broken but here you go anyway"
>
> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Tomas Härdin July 25, 2024, 9:16 a.m. UTC | #9
sön 2024-07-21 klockan 14:07 +0900 skrev Pierre-Anthony Lemieux:
> On Sat, Jul 20, 2024 at 5:12 PM Tomas Härdin <git@haerdin.se> wrote:
> > 
> > tor 2024-07-18 klockan 23:10 +0900 skrev Pierre-Anthony Lemieux:
> > > On Mon, Jul 15, 2024 at 10:33 PM Tomas Härdin <git@haerdin.se>
> > > wrote:
> > > > 
> > > > fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony
> > > > Lemieux:
> > > > > On Thu, Jul 11, 2024 at 10:28 PM Tomas Härdin
> > > > > <git@haerdin.se>
> > > > > wrote:
> > > > > > 
> > > > > > > +            if (s->in_tile_headers == 1 && s->isHT &&
> > > > > > > (!s-
> > > > > > > > Ccap15_b11))
> > > > > > > +                av_log(s->avctx, AV_LOG_WARNING, "COD
> > > > > > > marker
> > > > > > > is
> > > > > > > found in HOMOGENEOUS HT set\n");
> > > > > > 
> > > > > > How bad is this and the other markers being present in this
> > > > > > case?
> > > > > 
> > > > > At the very least, it means that signaling is inconsistent
> > > > > within
> > > > > the
> > > > > codestream since the standard states that:
> > > > > """
> > > > > The HOMOGENEOUS set is the set of HTJ2K codestreams where:
> > > > > • none of the functional marker segments, e.g., COD, COC,
> > > > > RGN,
> > > > > QCD,
> > > > > QCC, and POC, are present in any
> > > > > tile-part header; and
> > > > > • no PPT marker segment is present.
> > > > > """
> > > > > 
> > > > > The point of signalling that a codestream is "HOMOGENEOUS" is
> > > > > to
> > > > > allow
> > > > > decoders to configure themselves solely based on information
> > > > > retrieved
> > > > > entirely from the main header.
> > > > > 
> > > > > Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS to
> > > > > short-
> > > > > circuit
> > > > > configuration, incorrect HOMOGENEOUS signalling will likely
> > > > > not
> > > > > impact
> > > > > FFMPEG.
> > > > 
> > > > It could happen that information in tile headers contradict
> > > > information
> > > > in the main header, right? In such a case it sounds like we
> > > > can't
> > > > be
> > > > sure which decode is the correct one.
> > > 
> > > Per the spec, the decoder uses the COD information in tile-parts
> > > over
> > > the COD information in the header.
> > > 
> > > The issue here is that a decoder, upon seeing HOMOGENEOUS, simply
> > > does
> > > not bother with looking for COD information in tile-parts,
> > > thereby
> > > missing critical information.
> > 
> > So it is actually perfectly legal? Then it seems this patch is
> > wrong
> 
> What is not "illegal": the HOMOGENEOUS flag being equal to true *and*
> having COD marker segments in tile-parts.
> 
> This is what the patch detects.
> 
> FFMPEG can decode such illegal codestream. Other decoders might not.
> 
> The question is: what should FFMPEG do? Should FFMPEG exit or warn
> and continue.

If the spec allows it but it's perhaps unadviced then warning about it
seems reasonable

/Tomas
Pierre-Anthony Lemieux July 26, 2024, 12:06 a.m. UTC | #10
On Thu, Jul 25, 2024 at 2:17 AM Tomas Härdin <git@haerdin.se> wrote:
>
> sön 2024-07-21 klockan 14:07 +0900 skrev Pierre-Anthony Lemieux:
> > On Sat, Jul 20, 2024 at 5:12 PM Tomas Härdin <git@haerdin.se> wrote:
> > >
> > > tor 2024-07-18 klockan 23:10 +0900 skrev Pierre-Anthony Lemieux:
> > > > On Mon, Jul 15, 2024 at 10:33 PM Tomas Härdin <git@haerdin.se>
> > > > wrote:
> > > > >
> > > > > fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony
> > > > > Lemieux:
> > > > > > On Thu, Jul 11, 2024 at 10:28 PM Tomas Härdin
> > > > > > <git@haerdin.se>
> > > > > > wrote:
> > > > > > >
> > > > > > > > +            if (s->in_tile_headers == 1 && s->isHT &&
> > > > > > > > (!s-
> > > > > > > > > Ccap15_b11))
> > > > > > > > +                av_log(s->avctx, AV_LOG_WARNING, "COD
> > > > > > > > marker
> > > > > > > > is
> > > > > > > > found in HOMOGENEOUS HT set\n");
> > > > > > >
> > > > > > > How bad is this and the other markers being present in this
> > > > > > > case?
> > > > > >
> > > > > > At the very least, it means that signaling is inconsistent
> > > > > > within
> > > > > > the
> > > > > > codestream since the standard states that:
> > > > > > """
> > > > > > The HOMOGENEOUS set is the set of HTJ2K codestreams where:
> > > > > > • none of the functional marker segments, e.g., COD, COC,
> > > > > > RGN,
> > > > > > QCD,
> > > > > > QCC, and POC, are present in any
> > > > > > tile-part header; and
> > > > > > • no PPT marker segment is present.
> > > > > > """
> > > > > >
> > > > > > The point of signalling that a codestream is "HOMOGENEOUS" is
> > > > > > to
> > > > > > allow
> > > > > > decoders to configure themselves solely based on information
> > > > > > retrieved
> > > > > > entirely from the main header.
> > > > > >
> > > > > > Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS to
> > > > > > short-
> > > > > > circuit
> > > > > > configuration, incorrect HOMOGENEOUS signalling will likely
> > > > > > not
> > > > > > impact
> > > > > > FFMPEG.
> > > > >
> > > > > It could happen that information in tile headers contradict
> > > > > information
> > > > > in the main header, right? In such a case it sounds like we
> > > > > can't
> > > > > be
> > > > > sure which decode is the correct one.
> > > >
> > > > Per the spec, the decoder uses the COD information in tile-parts
> > > > over
> > > > the COD information in the header.
> > > >
> > > > The issue here is that a decoder, upon seeing HOMOGENEOUS, simply
> > > > does
> > > > not bother with looking for COD information in tile-parts,
> > > > thereby
> > > > missing critical information.
> > >
> > > So it is actually perfectly legal? Then it seems this patch is
> > > wrong
> >
> > What is not "illegal": the HOMOGENEOUS flag being equal to true *and*
> > having COD marker segments in tile-parts.
> >
> > This is what the patch detects.
> >
> > FFMPEG can decode such illegal codestream. Other decoders might not.
> >
> > The question is: what should FFMPEG do? Should FFMPEG exit or warn
> > and continue.
>
> If the spec allows it but it's perhaps unadviced then warning about it
> seems reasonable

(I totally messed up my double negative. Repeat below. Sorry for the confusion.)

What is "illegal": the HOMOGENEOUS flag being equal to true *and*
having COD marker segments in tile-parts.

This is what the patch detects.

FFMPEG can decode such illegal codestream. Other decoders might not.

The question is: what should FFMPEG do? Should FFMPEG exit or warn and continue.

>
> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Tomas Härdin July 26, 2024, 8:04 a.m. UTC | #11
tor 2024-07-25 klockan 17:06 -0700 skrev Pierre-Anthony Lemieux:
> On Thu, Jul 25, 2024 at 2:17 AM Tomas Härdin <git@haerdin.se> wrote:
> > 
> > sön 2024-07-21 klockan 14:07 +0900 skrev Pierre-Anthony Lemieux:
> > > On Sat, Jul 20, 2024 at 5:12 PM Tomas Härdin <git@haerdin.se>
> > > wrote:
> > > > 
> > > > tor 2024-07-18 klockan 23:10 +0900 skrev Pierre-Anthony
> > > > Lemieux:
> > > > > On Mon, Jul 15, 2024 at 10:33 PM Tomas Härdin
> > > > > <git@haerdin.se>
> > > > > wrote:
> > > > > > 
> > > > > > fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony
> > > > > > Lemieux:
> > > > > > > On Thu, Jul 11, 2024 at 10:28 PM Tomas Härdin
> > > > > > > <git@haerdin.se>
> > > > > > > wrote:
> > > > > > > > 
> > > > > > > > > +            if (s->in_tile_headers == 1 && s->isHT
> > > > > > > > > &&
> > > > > > > > > (!s-
> > > > > > > > > > Ccap15_b11))
> > > > > > > > > +                av_log(s->avctx, AV_LOG_WARNING,
> > > > > > > > > "COD
> > > > > > > > > marker
> > > > > > > > > is
> > > > > > > > > found in HOMOGENEOUS HT set\n");
> > > > > > > > 
> > > > > > > > How bad is this and the other markers being present in
> > > > > > > > this
> > > > > > > > case?
> > > > > > > 
> > > > > > > At the very least, it means that signaling is
> > > > > > > inconsistent
> > > > > > > within
> > > > > > > the
> > > > > > > codestream since the standard states that:
> > > > > > > """
> > > > > > > The HOMOGENEOUS set is the set of HTJ2K codestreams
> > > > > > > where:
> > > > > > > • none of the functional marker segments, e.g., COD, COC,
> > > > > > > RGN,
> > > > > > > QCD,
> > > > > > > QCC, and POC, are present in any
> > > > > > > tile-part header; and
> > > > > > > • no PPT marker segment is present.
> > > > > > > """
> > > > > > > 
> > > > > > > The point of signalling that a codestream is
> > > > > > > "HOMOGENEOUS" is
> > > > > > > to
> > > > > > > allow
> > > > > > > decoders to configure themselves solely based on
> > > > > > > information
> > > > > > > retrieved
> > > > > > > entirely from the main header.
> > > > > > > 
> > > > > > > Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS to
> > > > > > > short-
> > > > > > > circuit
> > > > > > > configuration, incorrect HOMOGENEOUS signalling will
> > > > > > > likely
> > > > > > > not
> > > > > > > impact
> > > > > > > FFMPEG.
> > > > > > 
> > > > > > It could happen that information in tile headers contradict
> > > > > > information
> > > > > > in the main header, right? In such a case it sounds like we
> > > > > > can't
> > > > > > be
> > > > > > sure which decode is the correct one.
> > > > > 
> > > > > Per the spec, the decoder uses the COD information in tile-
> > > > > parts
> > > > > over
> > > > > the COD information in the header.
> > > > > 
> > > > > The issue here is that a decoder, upon seeing HOMOGENEOUS,
> > > > > simply
> > > > > does
> > > > > not bother with looking for COD information in tile-parts,
> > > > > thereby
> > > > > missing critical information.
> > > > 
> > > > So it is actually perfectly legal? Then it seems this patch is
> > > > wrong
> > > 
> > > What is not "illegal": the HOMOGENEOUS flag being equal to true
> > > *and*
> > > having COD marker segments in tile-parts.
> > > 
> > > This is what the patch detects.
> > > 
> > > FFMPEG can decode such illegal codestream. Other decoders might
> > > not.
> > > 
> > > The question is: what should FFMPEG do? Should FFMPEG exit or
> > > warn
> > > and continue.
> > 
> > If the spec allows it but it's perhaps unadviced then warning about
> > it
> > seems reasonable
> 
> (I totally messed up my double negative. Repeat below. Sorry for the
> confusion.)
> 
> What is "illegal": the HOMOGENEOUS flag being equal to true *and*
> having COD marker segments in tile-parts.
> 
> This is what the patch detects.
> 
> FFMPEG can decode such illegal codestream. Other decoders might not.
> 
> The question is: what should FFMPEG do? Should FFMPEG exit or warn
> and continue.

Since it's illegal then we should complain and bail out. Users that
*really* want it to keep going can change the code.

/Tomas
Pierre-Anthony Lemieux July 26, 2024, 9:11 p.m. UTC | #12
On Fri, Jul 26, 2024 at 1:04 AM Tomas Härdin <git@haerdin.se> wrote:
>
> tor 2024-07-25 klockan 17:06 -0700 skrev Pierre-Anthony Lemieux:
> > On Thu, Jul 25, 2024 at 2:17 AM Tomas Härdin <git@haerdin.se> wrote:
> > >
> > > sön 2024-07-21 klockan 14:07 +0900 skrev Pierre-Anthony Lemieux:
> > > > On Sat, Jul 20, 2024 at 5:12 PM Tomas Härdin <git@haerdin.se>
> > > > wrote:
> > > > >
> > > > > tor 2024-07-18 klockan 23:10 +0900 skrev Pierre-Anthony
> > > > > Lemieux:
> > > > > > On Mon, Jul 15, 2024 at 10:33 PM Tomas Härdin
> > > > > > <git@haerdin.se>
> > > > > > wrote:
> > > > > > >
> > > > > > > fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony
> > > > > > > Lemieux:
> > > > > > > > On Thu, Jul 11, 2024 at 10:28 PM Tomas Härdin
> > > > > > > > <git@haerdin.se>
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > +            if (s->in_tile_headers == 1 && s->isHT
> > > > > > > > > > &&
> > > > > > > > > > (!s-
> > > > > > > > > > > Ccap15_b11))
> > > > > > > > > > +                av_log(s->avctx, AV_LOG_WARNING,
> > > > > > > > > > "COD
> > > > > > > > > > marker
> > > > > > > > > > is
> > > > > > > > > > found in HOMOGENEOUS HT set\n");
> > > > > > > > >
> > > > > > > > > How bad is this and the other markers being present in
> > > > > > > > > this
> > > > > > > > > case?
> > > > > > > >
> > > > > > > > At the very least, it means that signaling is
> > > > > > > > inconsistent
> > > > > > > > within
> > > > > > > > the
> > > > > > > > codestream since the standard states that:
> > > > > > > > """
> > > > > > > > The HOMOGENEOUS set is the set of HTJ2K codestreams
> > > > > > > > where:
> > > > > > > > • none of the functional marker segments, e.g., COD, COC,
> > > > > > > > RGN,
> > > > > > > > QCD,
> > > > > > > > QCC, and POC, are present in any
> > > > > > > > tile-part header; and
> > > > > > > > • no PPT marker segment is present.
> > > > > > > > """
> > > > > > > >
> > > > > > > > The point of signalling that a codestream is
> > > > > > > > "HOMOGENEOUS" is
> > > > > > > > to
> > > > > > > > allow
> > > > > > > > decoders to configure themselves solely based on
> > > > > > > > information
> > > > > > > > retrieved
> > > > > > > > entirely from the main header.
> > > > > > > >
> > > > > > > > Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS to
> > > > > > > > short-
> > > > > > > > circuit
> > > > > > > > configuration, incorrect HOMOGENEOUS signalling will
> > > > > > > > likely
> > > > > > > > not
> > > > > > > > impact
> > > > > > > > FFMPEG.
> > > > > > >
> > > > > > > It could happen that information in tile headers contradict
> > > > > > > information
> > > > > > > in the main header, right? In such a case it sounds like we
> > > > > > > can't
> > > > > > > be
> > > > > > > sure which decode is the correct one.
> > > > > >
> > > > > > Per the spec, the decoder uses the COD information in tile-
> > > > > > parts
> > > > > > over
> > > > > > the COD information in the header.
> > > > > >
> > > > > > The issue here is that a decoder, upon seeing HOMOGENEOUS,
> > > > > > simply
> > > > > > does
> > > > > > not bother with looking for COD information in tile-parts,
> > > > > > thereby
> > > > > > missing critical information.
> > > > >
> > > > > So it is actually perfectly legal? Then it seems this patch is
> > > > > wrong
> > > >
> > > > What is not "illegal": the HOMOGENEOUS flag being equal to true
> > > > *and*
> > > > having COD marker segments in tile-parts.
> > > >
> > > > This is what the patch detects.
> > > >
> > > > FFMPEG can decode such illegal codestream. Other decoders might
> > > > not.
> > > >
> > > > The question is: what should FFMPEG do? Should FFMPEG exit or
> > > > warn
> > > > and continue.
> > >
> > > If the spec allows it but it's perhaps unadviced then warning about
> > > it
> > > seems reasonable
> >
> > (I totally messed up my double negative. Repeat below. Sorry for the
> > confusion.)
> >
> > What is "illegal": the HOMOGENEOUS flag being equal to true *and*
> > having COD marker segments in tile-parts.
> >
> > This is what the patch detects.
> >
> > FFMPEG can decode such illegal codestream. Other decoders might not.
> >
> > The question is: what should FFMPEG do? Should FFMPEG exit or warn
> > and continue.
>
> Since it's illegal then we should complain and bail out. Users that
> *really* want it to keep going can change the code.

@Osamu Watanabe Can you update the patch to make FFMPEG exit when
encountering an illegal codestream, even if FFMPEG can theoretically
decode it?

>
> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Michael Niedermayer July 26, 2024, 9:29 p.m. UTC | #13
On Thu, Jul 25, 2024 at 05:06:04PM -0700, Pierre-Anthony Lemieux wrote:
> On Thu, Jul 25, 2024 at 2:17 AM Tomas Härdin <git@haerdin.se> wrote:
> >
> > sön 2024-07-21 klockan 14:07 +0900 skrev Pierre-Anthony Lemieux:
> > > On Sat, Jul 20, 2024 at 5:12 PM Tomas Härdin <git@haerdin.se> wrote:
> > > >
> > > > tor 2024-07-18 klockan 23:10 +0900 skrev Pierre-Anthony Lemieux:
> > > > > On Mon, Jul 15, 2024 at 10:33 PM Tomas Härdin <git@haerdin.se>
> > > > > wrote:
> > > > > >
> > > > > > fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony
> > > > > > Lemieux:
> > > > > > > On Thu, Jul 11, 2024 at 10:28 PM Tomas Härdin
> > > > > > > <git@haerdin.se>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > > +            if (s->in_tile_headers == 1 && s->isHT &&
> > > > > > > > > (!s-
> > > > > > > > > > Ccap15_b11))
> > > > > > > > > +                av_log(s->avctx, AV_LOG_WARNING, "COD
> > > > > > > > > marker
> > > > > > > > > is
> > > > > > > > > found in HOMOGENEOUS HT set\n");
> > > > > > > >
> > > > > > > > How bad is this and the other markers being present in this
> > > > > > > > case?
> > > > > > >
> > > > > > > At the very least, it means that signaling is inconsistent
> > > > > > > within
> > > > > > > the
> > > > > > > codestream since the standard states that:
> > > > > > > """
> > > > > > > The HOMOGENEOUS set is the set of HTJ2K codestreams where:
> > > > > > > • none of the functional marker segments, e.g., COD, COC,
> > > > > > > RGN,
> > > > > > > QCD,
> > > > > > > QCC, and POC, are present in any
> > > > > > > tile-part header; and
> > > > > > > • no PPT marker segment is present.
> > > > > > > """
> > > > > > >
> > > > > > > The point of signalling that a codestream is "HOMOGENEOUS" is
> > > > > > > to
> > > > > > > allow
> > > > > > > decoders to configure themselves solely based on information
> > > > > > > retrieved
> > > > > > > entirely from the main header.
> > > > > > >
> > > > > > > Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS to
> > > > > > > short-
> > > > > > > circuit
> > > > > > > configuration, incorrect HOMOGENEOUS signalling will likely
> > > > > > > not
> > > > > > > impact
> > > > > > > FFMPEG.
> > > > > >
> > > > > > It could happen that information in tile headers contradict
> > > > > > information
> > > > > > in the main header, right? In such a case it sounds like we
> > > > > > can't
> > > > > > be
> > > > > > sure which decode is the correct one.
> > > > >
> > > > > Per the spec, the decoder uses the COD information in tile-parts
> > > > > over
> > > > > the COD information in the header.
> > > > >
> > > > > The issue here is that a decoder, upon seeing HOMOGENEOUS, simply
> > > > > does
> > > > > not bother with looking for COD information in tile-parts,
> > > > > thereby
> > > > > missing critical information.
> > > >
> > > > So it is actually perfectly legal? Then it seems this patch is
> > > > wrong
> > >
> > > What is not "illegal": the HOMOGENEOUS flag being equal to true *and*
> > > having COD marker segments in tile-parts.
> > >
> > > This is what the patch detects.
> > >
> > > FFMPEG can decode such illegal codestream. Other decoders might not.
> > >
> > > The question is: what should FFMPEG do? Should FFMPEG exit or warn
> > > and continue.
> >
> > If the spec allows it but it's perhaps unadviced then warning about it
> > seems reasonable
> 
> (I totally messed up my double negative. Repeat below. Sorry for the confusion.)
> 
> What is "illegal": the HOMOGENEOUS flag being equal to true *and*
> having COD marker segments in tile-parts.
> 
> This is what the patch detects.
> 
> FFMPEG can decode such illegal codestream. Other decoders might not.
> 
> The question is: what should FFMPEG do? Should FFMPEG exit or warn and continue.

Does such a codestream actually exist ? I mean is this just a hypothetical case
or something existing ?

thx

[...]
Pierre-Anthony Lemieux July 26, 2024, 9:32 p.m. UTC | #14
On Fri, Jul 26, 2024 at 2:29 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Thu, Jul 25, 2024 at 05:06:04PM -0700, Pierre-Anthony Lemieux wrote:
> > On Thu, Jul 25, 2024 at 2:17 AM Tomas Härdin <git@haerdin.se> wrote:
> > >
> > > sön 2024-07-21 klockan 14:07 +0900 skrev Pierre-Anthony Lemieux:
> > > > On Sat, Jul 20, 2024 at 5:12 PM Tomas Härdin <git@haerdin.se> wrote:
> > > > >
> > > > > tor 2024-07-18 klockan 23:10 +0900 skrev Pierre-Anthony Lemieux:
> > > > > > On Mon, Jul 15, 2024 at 10:33 PM Tomas Härdin <git@haerdin.se>
> > > > > > wrote:
> > > > > > >
> > > > > > > fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony
> > > > > > > Lemieux:
> > > > > > > > On Thu, Jul 11, 2024 at 10:28 PM Tomas Härdin
> > > > > > > > <git@haerdin.se>
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > +            if (s->in_tile_headers == 1 && s->isHT &&
> > > > > > > > > > (!s-
> > > > > > > > > > > Ccap15_b11))
> > > > > > > > > > +                av_log(s->avctx, AV_LOG_WARNING, "COD
> > > > > > > > > > marker
> > > > > > > > > > is
> > > > > > > > > > found in HOMOGENEOUS HT set\n");
> > > > > > > > >
> > > > > > > > > How bad is this and the other markers being present in this
> > > > > > > > > case?
> > > > > > > >
> > > > > > > > At the very least, it means that signaling is inconsistent
> > > > > > > > within
> > > > > > > > the
> > > > > > > > codestream since the standard states that:
> > > > > > > > """
> > > > > > > > The HOMOGENEOUS set is the set of HTJ2K codestreams where:
> > > > > > > > • none of the functional marker segments, e.g., COD, COC,
> > > > > > > > RGN,
> > > > > > > > QCD,
> > > > > > > > QCC, and POC, are present in any
> > > > > > > > tile-part header; and
> > > > > > > > • no PPT marker segment is present.
> > > > > > > > """
> > > > > > > >
> > > > > > > > The point of signalling that a codestream is "HOMOGENEOUS" is
> > > > > > > > to
> > > > > > > > allow
> > > > > > > > decoders to configure themselves solely based on information
> > > > > > > > retrieved
> > > > > > > > entirely from the main header.
> > > > > > > >
> > > > > > > > Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS to
> > > > > > > > short-
> > > > > > > > circuit
> > > > > > > > configuration, incorrect HOMOGENEOUS signalling will likely
> > > > > > > > not
> > > > > > > > impact
> > > > > > > > FFMPEG.
> > > > > > >
> > > > > > > It could happen that information in tile headers contradict
> > > > > > > information
> > > > > > > in the main header, right? In such a case it sounds like we
> > > > > > > can't
> > > > > > > be
> > > > > > > sure which decode is the correct one.
> > > > > >
> > > > > > Per the spec, the decoder uses the COD information in tile-parts
> > > > > > over
> > > > > > the COD information in the header.
> > > > > >
> > > > > > The issue here is that a decoder, upon seeing HOMOGENEOUS, simply
> > > > > > does
> > > > > > not bother with looking for COD information in tile-parts,
> > > > > > thereby
> > > > > > missing critical information.
> > > > >
> > > > > So it is actually perfectly legal? Then it seems this patch is
> > > > > wrong
> > > >
> > > > What is not "illegal": the HOMOGENEOUS flag being equal to true *and*
> > > > having COD marker segments in tile-parts.
> > > >
> > > > This is what the patch detects.
> > > >
> > > > FFMPEG can decode such illegal codestream. Other decoders might not.
> > > >
> > > > The question is: what should FFMPEG do? Should FFMPEG exit or warn
> > > > and continue.
> > >
> > > If the spec allows it but it's perhaps unadviced then warning about it
> > > seems reasonable
> >
> > (I totally messed up my double negative. Repeat below. Sorry for the confusion.)
> >
> > What is "illegal": the HOMOGENEOUS flag being equal to true *and*
> > having COD marker segments in tile-parts.
> >
> > This is what the patch detects.
> >
> > FFMPEG can decode such illegal codestream. Other decoders might not.
> >
> > The question is: what should FFMPEG do? Should FFMPEG exit or warn and continue.
>
> Does such a codestream actually exist ? I mean is this just a hypothetical case
> or something existing ?

Such a codestream exists: a codestream with the HOMOGENEOUS flag equal
to true *and* COD marker segments in tile-parts is "illegal" per the
specification. Such a codestream may cause certain decoders (esp.
hardware decoders) to fail because, based on the value of the
HOMOGENEOUS flag, they will not expect COD marker segments in
tile-parts. FFMPEG will not fail, because it does not use the value of
the HOMOGENEOUS flag to configure itself.

>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Rewriting code that is poorly written but fully understood is good.
> Rewriting code that one doesnt understand is a sign that one is less smart
> than the original author, trying to rewrite it will not make it better.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Osamu Watanabe July 29, 2024, 7:46 a.m. UTC | #15
> @Osamu Watanabe Can you update the patch to make FFMPEG exit when
> encountering an illegal codestream, even if FFMPEG can theoretically
> decode it?

I will update the patch in a few days.


> On Jul 27, 2024, at 6:11, Pierre-Anthony Lemieux <pal@sandflow.com> wrote:
> 
> On Fri, Jul 26, 2024 at 1:04?AM Tomas Hardin <git@haerdin.se> wrote:
>> 
>> tor 2024-07-25 klockan 17:06 -0700 skrev Pierre-Anthony Lemieux:
>>> On Thu, Jul 25, 2024 at 2:17?AM Tomas Hardin <git@haerdin.se> wrote:
>>>> 
>>>> son 2024-07-21 klockan 14:07 +0900 skrev Pierre-Anthony Lemieux:
>>>>> On Sat, Jul 20, 2024 at 5:12?PM Tomas Hardin <git@haerdin.se>
>>>>> wrote:
>>>>>> 
>>>>>> tor 2024-07-18 klockan 23:10 +0900 skrev Pierre-Anthony
>>>>>> Lemieux:
>>>>>>> On Mon, Jul 15, 2024 at 10:33?PM Tomas Hardin
>>>>>>> <git@haerdin.se>
>>>>>>> wrote:
>>>>>>>> 
>>>>>>>> fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony
>>>>>>>> Lemieux:
>>>>>>>>> On Thu, Jul 11, 2024 at 10:28?PM Tomas Hardin
>>>>>>>>> <git@haerdin.se>
>>>>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>>> +            if (s->in_tile_headers == 1 && s->isHT
>>>>>>>>>>> &&
>>>>>>>>>>> (!s-
>>>>>>>>>>>> Ccap15_b11))
>>>>>>>>>>> +                av_log(s->avctx, AV_LOG_WARNING,
>>>>>>>>>>> "COD
>>>>>>>>>>> marker
>>>>>>>>>>> is
>>>>>>>>>>> found in HOMOGENEOUS HT set\n");
>>>>>>>>>> 
>>>>>>>>>> How bad is this and the other markers being present in
>>>>>>>>>> this
>>>>>>>>>> case?
>>>>>>>>> 
>>>>>>>>> At the very least, it means that signaling is
>>>>>>>>> inconsistent
>>>>>>>>> within
>>>>>>>>> the
>>>>>>>>> codestream since the standard states that:
>>>>>>>>> """
>>>>>>>>> The HOMOGENEOUS set is the set of HTJ2K codestreams
>>>>>>>>> where:
>>>>>>>>> ? none of the functional marker segments, e.g., COD, COC,
>>>>>>>>> RGN,
>>>>>>>>> QCD,
>>>>>>>>> QCC, and POC, are present in any
>>>>>>>>> tile-part header; and
>>>>>>>>> ? no PPT marker segment is present.
>>>>>>>>> """
>>>>>>>>> 
>>>>>>>>> The point of signalling that a codestream is
>>>>>>>>> "HOMOGENEOUS" is
>>>>>>>>> to
>>>>>>>>> allow
>>>>>>>>> decoders to configure themselves solely based on
>>>>>>>>> information
>>>>>>>>> retrieved
>>>>>>>>> entirely from the main header.
>>>>>>>>> 
>>>>>>>>> Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS to
>>>>>>>>> short-
>>>>>>>>> circuit
>>>>>>>>> configuration, incorrect HOMOGENEOUS signalling will
>>>>>>>>> likely
>>>>>>>>> not
>>>>>>>>> impact
>>>>>>>>> FFMPEG.
>>>>>>>> 
>>>>>>>> It could happen that information in tile headers contradict
>>>>>>>> information
>>>>>>>> in the main header, right? In such a case it sounds like we
>>>>>>>> can't
>>>>>>>> be
>>>>>>>> sure which decode is the correct one.
>>>>>>> 
>>>>>>> Per the spec, the decoder uses the COD information in tile-
>>>>>>> parts
>>>>>>> over
>>>>>>> the COD information in the header.
>>>>>>> 
>>>>>>> The issue here is that a decoder, upon seeing HOMOGENEOUS,
>>>>>>> simply
>>>>>>> does
>>>>>>> not bother with looking for COD information in tile-parts,
>>>>>>> thereby
>>>>>>> missing critical information.
>>>>>> 
>>>>>> So it is actually perfectly legal? Then it seems this patch is
>>>>>> wrong
>>>>> 
>>>>> What is not "illegal": the HOMOGENEOUS flag being equal to true
>>>>> *and*
>>>>> having COD marker segments in tile-parts.
>>>>> 
>>>>> This is what the patch detects.
>>>>> 
>>>>> FFMPEG can decode such illegal codestream. Other decoders might
>>>>> not.
>>>>> 
>>>>> The question is: what should FFMPEG do? Should FFMPEG exit or
>>>>> warn
>>>>> and continue.
>>>> 
>>>> If the spec allows it but it's perhaps unadviced then warning about
>>>> it
>>>> seems reasonable
>>> 
>>> (I totally messed up my double negative. Repeat below. Sorry for the
>>> confusion.)
>>> 
>>> What is "illegal": the HOMOGENEOUS flag being equal to true *and*
>>> having COD marker segments in tile-parts.
>>> 
>>> This is what the patch detects.
>>> 
>>> FFMPEG can decode such illegal codestream. Other decoders might not.
>>> 
>>> The question is: what should FFMPEG do? Should FFMPEG exit or warn
>>> and continue.
>> 
>> Since it's illegal then we should complain and bail out. Users that
>> *really* want it to keep going can change the code.
> 
> @Osamu Watanabe Can you update the patch to make FFMPEG exit when
> encountering an illegal codestream, even if FFMPEG can theoretically
> decode it?
> 
>> 
>> /Tomas
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Tomas Härdin July 30, 2024, 8:22 p.m. UTC | #16
fre 2024-07-26 klockan 23:29 +0200 skrev Michael Niedermayer:
> On Thu, Jul 25, 2024 at 05:06:04PM -0700, Pierre-Anthony Lemieux
> wrote:
> > On Thu, Jul 25, 2024 at 2:17 AM Tomas Härdin <git@haerdin.se>
> > wrote:
> > > 
> > > sön 2024-07-21 klockan 14:07 +0900 skrev Pierre-Anthony Lemieux:
> > > > On Sat, Jul 20, 2024 at 5:12 PM Tomas Härdin <git@haerdin.se>
> > > > wrote:
> > > > > 
> > > > > tor 2024-07-18 klockan 23:10 +0900 skrev Pierre-Anthony
> > > > > Lemieux:
> > > > > > On Mon, Jul 15, 2024 at 10:33 PM Tomas Härdin
> > > > > > <git@haerdin.se>
> > > > > > wrote:
> > > > > > > 
> > > > > > > fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony
> > > > > > > Lemieux:
> > > > > > > > On Thu, Jul 11, 2024 at 10:28 PM Tomas Härdin
> > > > > > > > <git@haerdin.se>
> > > > > > > > wrote:
> > > > > > > > > 
> > > > > > > > > > +            if (s->in_tile_headers == 1 && s->isHT
> > > > > > > > > > &&
> > > > > > > > > > (!s-
> > > > > > > > > > > Ccap15_b11))
> > > > > > > > > > +                av_log(s->avctx, AV_LOG_WARNING,
> > > > > > > > > > "COD
> > > > > > > > > > marker
> > > > > > > > > > is
> > > > > > > > > > found in HOMOGENEOUS HT set\n");
> > > > > > > > > 
> > > > > > > > > How bad is this and the other markers being present
> > > > > > > > > in this
> > > > > > > > > case?
> > > > > > > > 
> > > > > > > > At the very least, it means that signaling is
> > > > > > > > inconsistent
> > > > > > > > within
> > > > > > > > the
> > > > > > > > codestream since the standard states that:
> > > > > > > > """
> > > > > > > > The HOMOGENEOUS set is the set of HTJ2K codestreams
> > > > > > > > where:
> > > > > > > > • none of the functional marker segments, e.g., COD,
> > > > > > > > COC,
> > > > > > > > RGN,
> > > > > > > > QCD,
> > > > > > > > QCC, and POC, are present in any
> > > > > > > > tile-part header; and
> > > > > > > > • no PPT marker segment is present.
> > > > > > > > """
> > > > > > > > 
> > > > > > > > The point of signalling that a codestream is
> > > > > > > > "HOMOGENEOUS" is
> > > > > > > > to
> > > > > > > > allow
> > > > > > > > decoders to configure themselves solely based on
> > > > > > > > information
> > > > > > > > retrieved
> > > > > > > > entirely from the main header.
> > > > > > > > 
> > > > > > > > Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS
> > > > > > > > to
> > > > > > > > short-
> > > > > > > > circuit
> > > > > > > > configuration, incorrect HOMOGENEOUS signalling will
> > > > > > > > likely
> > > > > > > > not
> > > > > > > > impact
> > > > > > > > FFMPEG.
> > > > > > > 
> > > > > > > It could happen that information in tile headers
> > > > > > > contradict
> > > > > > > information
> > > > > > > in the main header, right? In such a case it sounds like
> > > > > > > we
> > > > > > > can't
> > > > > > > be
> > > > > > > sure which decode is the correct one.
> > > > > > 
> > > > > > Per the spec, the decoder uses the COD information in tile-
> > > > > > parts
> > > > > > over
> > > > > > the COD information in the header.
> > > > > > 
> > > > > > The issue here is that a decoder, upon seeing HOMOGENEOUS,
> > > > > > simply
> > > > > > does
> > > > > > not bother with looking for COD information in tile-parts,
> > > > > > thereby
> > > > > > missing critical information.
> > > > > 
> > > > > So it is actually perfectly legal? Then it seems this patch
> > > > > is
> > > > > wrong
> > > > 
> > > > What is not "illegal": the HOMOGENEOUS flag being equal to true
> > > > *and*
> > > > having COD marker segments in tile-parts.
> > > > 
> > > > This is what the patch detects.
> > > > 
> > > > FFMPEG can decode such illegal codestream. Other decoders might
> > > > not.
> > > > 
> > > > The question is: what should FFMPEG do? Should FFMPEG exit or
> > > > warn
> > > > and continue.
> > > 
> > > If the spec allows it but it's perhaps unadviced then warning
> > > about it
> > > seems reasonable
> > 
> > (I totally messed up my double negative. Repeat below. Sorry for
> > the confusion.)
> > 
> > What is "illegal": the HOMOGENEOUS flag being equal to true *and*
> > having COD marker segments in tile-parts.
> > 
> > This is what the patch detects.
> > 
> > FFMPEG can decode such illegal codestream. Other decoders might
> > not.
> > 
> > The question is: what should FFMPEG do? Should FFMPEG exit or warn
> > and continue.
> 
> Does such a codestream actually exist ? I mean is this just a
> hypothetical case
> or something existing ?

This is more to stem the stream before anything happens because we were
lax with parsing

/Tomas
Michael Niedermayer July 30, 2024, 9:39 p.m. UTC | #17
On Tue, Jul 30, 2024 at 10:22:37PM +0200, Tomas Härdin wrote:
> fre 2024-07-26 klockan 23:29 +0200 skrev Michael Niedermayer:
> > On Thu, Jul 25, 2024 at 05:06:04PM -0700, Pierre-Anthony Lemieux
> > wrote:
> > > On Thu, Jul 25, 2024 at 2:17 AM Tomas Härdin <git@haerdin.se>
> > > wrote:
> > > > 
> > > > sön 2024-07-21 klockan 14:07 +0900 skrev Pierre-Anthony Lemieux:
> > > > > On Sat, Jul 20, 2024 at 5:12 PM Tomas Härdin <git@haerdin.se>
> > > > > wrote:
> > > > > > 
> > > > > > tor 2024-07-18 klockan 23:10 +0900 skrev Pierre-Anthony
> > > > > > Lemieux:
> > > > > > > On Mon, Jul 15, 2024 at 10:33 PM Tomas Härdin
> > > > > > > <git@haerdin.se>
> > > > > > > wrote:
> > > > > > > > 
> > > > > > > > fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony
> > > > > > > > Lemieux:
> > > > > > > > > On Thu, Jul 11, 2024 at 10:28 PM Tomas Härdin
> > > > > > > > > <git@haerdin.se>
> > > > > > > > > wrote:
> > > > > > > > > > 
> > > > > > > > > > > +            if (s->in_tile_headers == 1 && s->isHT
> > > > > > > > > > > &&
> > > > > > > > > > > (!s-
> > > > > > > > > > > > Ccap15_b11))
> > > > > > > > > > > +                av_log(s->avctx, AV_LOG_WARNING,
> > > > > > > > > > > "COD
> > > > > > > > > > > marker
> > > > > > > > > > > is
> > > > > > > > > > > found in HOMOGENEOUS HT set\n");
> > > > > > > > > > 
> > > > > > > > > > How bad is this and the other markers being present
> > > > > > > > > > in this
> > > > > > > > > > case?
> > > > > > > > > 
> > > > > > > > > At the very least, it means that signaling is
> > > > > > > > > inconsistent
> > > > > > > > > within
> > > > > > > > > the
> > > > > > > > > codestream since the standard states that:
> > > > > > > > > """
> > > > > > > > > The HOMOGENEOUS set is the set of HTJ2K codestreams
> > > > > > > > > where:
> > > > > > > > > • none of the functional marker segments, e.g., COD,
> > > > > > > > > COC,
> > > > > > > > > RGN,
> > > > > > > > > QCD,
> > > > > > > > > QCC, and POC, are present in any
> > > > > > > > > tile-part header; and
> > > > > > > > > • no PPT marker segment is present.
> > > > > > > > > """
> > > > > > > > > 
> > > > > > > > > The point of signalling that a codestream is
> > > > > > > > > "HOMOGENEOUS" is
> > > > > > > > > to
> > > > > > > > > allow
> > > > > > > > > decoders to configure themselves solely based on
> > > > > > > > > information
> > > > > > > > > retrieved
> > > > > > > > > entirely from the main header.
> > > > > > > > > 
> > > > > > > > > Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS
> > > > > > > > > to
> > > > > > > > > short-
> > > > > > > > > circuit
> > > > > > > > > configuration, incorrect HOMOGENEOUS signalling will
> > > > > > > > > likely
> > > > > > > > > not
> > > > > > > > > impact
> > > > > > > > > FFMPEG.
> > > > > > > > 
> > > > > > > > It could happen that information in tile headers
> > > > > > > > contradict
> > > > > > > > information
> > > > > > > > in the main header, right? In such a case it sounds like
> > > > > > > > we
> > > > > > > > can't
> > > > > > > > be
> > > > > > > > sure which decode is the correct one.
> > > > > > > 
> > > > > > > Per the spec, the decoder uses the COD information in tile-
> > > > > > > parts
> > > > > > > over
> > > > > > > the COD information in the header.
> > > > > > > 
> > > > > > > The issue here is that a decoder, upon seeing HOMOGENEOUS,
> > > > > > > simply
> > > > > > > does
> > > > > > > not bother with looking for COD information in tile-parts,
> > > > > > > thereby
> > > > > > > missing critical information.
> > > > > > 
> > > > > > So it is actually perfectly legal? Then it seems this patch
> > > > > > is
> > > > > > wrong
> > > > > 
> > > > > What is not "illegal": the HOMOGENEOUS flag being equal to true
> > > > > *and*
> > > > > having COD marker segments in tile-parts.
> > > > > 
> > > > > This is what the patch detects.
> > > > > 
> > > > > FFMPEG can decode such illegal codestream. Other decoders might
> > > > > not.
> > > > > 
> > > > > The question is: what should FFMPEG do? Should FFMPEG exit or
> > > > > warn
> > > > > and continue.
> > > > 
> > > > If the spec allows it but it's perhaps unadviced then warning
> > > > about it
> > > > seems reasonable
> > > 
> > > (I totally messed up my double negative. Repeat below. Sorry for
> > > the confusion.)
> > > 
> > > What is "illegal": the HOMOGENEOUS flag being equal to true *and*
> > > having COD marker segments in tile-parts.
> > > 
> > > This is what the patch detects.
> > > 
> > > FFMPEG can decode such illegal codestream. Other decoders might
> > > not.
> > > 
> > > The question is: what should FFMPEG do? Should FFMPEG exit or warn
> > > and continue.
> > 
> > Does such a codestream actually exist ? I mean is this just a
> > hypothetical case
> > or something existing ?
> 
> This is more to stem the stream before anything happens because we were
> lax with parsing

If such files dont exist then being picky makes sense

thx

[...]
Osamu Watanabe Aug. 1, 2024, 1:58 a.m. UTC | #18
According to the suggestions for v4, I have posted the patch set v5.

It is confirmed that the decoder with v5 passes all the conformance testing defined in ISO/IEC 15444-4. 

Best,
Osamu


> On Jul 31, 2024, at 6:39, Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
> On Tue, Jul 30, 2024 at 10:22:37PM +0200, Tomas Hardin wrote:
>> fre 2024-07-26 klockan 23:29 +0200 skrev Michael Niedermayer:
>>> On Thu, Jul 25, 2024 at 05:06:04PM -0700, Pierre-Anthony Lemieux
>>> wrote:
>>>> On Thu, Jul 25, 2024 at 2:17?AM Tomas Hardin <git@haerdin.se>
>>>> wrote:
>>>>> 
>>>>> son 2024-07-21 klockan 14:07 +0900 skrev Pierre-Anthony Lemieux:
>>>>>> On Sat, Jul 20, 2024 at 5:12?PM Tomas Hardin <git@haerdin.se>
>>>>>> wrote:
>>>>>>> 
>>>>>>> tor 2024-07-18 klockan 23:10 +0900 skrev Pierre-Anthony
>>>>>>> Lemieux:
>>>>>>>> On Mon, Jul 15, 2024 at 10:33?PM Tomas Hardin
>>>>>>>> <git@haerdin.se>
>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>> fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony
>>>>>>>>> Lemieux:
>>>>>>>>>> On Thu, Jul 11, 2024 at 10:28?PM Tomas Hardin
>>>>>>>>>> <git@haerdin.se>
>>>>>>>>>> wrote:
>>>>>>>>>>> 
>>>>>>>>>>>> +            if (s->in_tile_headers == 1 && s->isHT
>>>>>>>>>>>> &&
>>>>>>>>>>>> (!s-
>>>>>>>>>>>>> Ccap15_b11))
>>>>>>>>>>>> +                av_log(s->avctx, AV_LOG_WARNING,
>>>>>>>>>>>> "COD
>>>>>>>>>>>> marker
>>>>>>>>>>>> is
>>>>>>>>>>>> found in HOMOGENEOUS HT set\n");
>>>>>>>>>>> 
>>>>>>>>>>> How bad is this and the other markers being present
>>>>>>>>>>> in this
>>>>>>>>>>> case?
>>>>>>>>>> 
>>>>>>>>>> At the very least, it means that signaling is
>>>>>>>>>> inconsistent
>>>>>>>>>> within
>>>>>>>>>> the
>>>>>>>>>> codestream since the standard states that:
>>>>>>>>>> """
>>>>>>>>>> The HOMOGENEOUS set is the set of HTJ2K codestreams
>>>>>>>>>> where:
>>>>>>>>>> ? none of the functional marker segments, e.g., COD,
>>>>>>>>>> COC,
>>>>>>>>>> RGN,
>>>>>>>>>> QCD,
>>>>>>>>>> QCC, and POC, are present in any
>>>>>>>>>> tile-part header; and
>>>>>>>>>> ? no PPT marker segment is present.
>>>>>>>>>> """
>>>>>>>>>> 
>>>>>>>>>> The point of signalling that a codestream is
>>>>>>>>>> "HOMOGENEOUS" is
>>>>>>>>>> to
>>>>>>>>>> allow
>>>>>>>>>> decoders to configure themselves solely based on
>>>>>>>>>> information
>>>>>>>>>> retrieved
>>>>>>>>>> entirely from the main header.
>>>>>>>>>> 
>>>>>>>>>> Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS
>>>>>>>>>> to
>>>>>>>>>> short-
>>>>>>>>>> circuit
>>>>>>>>>> configuration, incorrect HOMOGENEOUS signalling will
>>>>>>>>>> likely
>>>>>>>>>> not
>>>>>>>>>> impact
>>>>>>>>>> FFMPEG.
>>>>>>>>> 
>>>>>>>>> It could happen that information in tile headers
>>>>>>>>> contradict
>>>>>>>>> information
>>>>>>>>> in the main header, right? In such a case it sounds like
>>>>>>>>> we
>>>>>>>>> can't
>>>>>>>>> be
>>>>>>>>> sure which decode is the correct one.
>>>>>>>> 
>>>>>>>> Per the spec, the decoder uses the COD information in tile-
>>>>>>>> parts
>>>>>>>> over
>>>>>>>> the COD information in the header.
>>>>>>>> 
>>>>>>>> The issue here is that a decoder, upon seeing HOMOGENEOUS,
>>>>>>>> simply
>>>>>>>> does
>>>>>>>> not bother with looking for COD information in tile-parts,
>>>>>>>> thereby
>>>>>>>> missing critical information.
>>>>>>> 
>>>>>>> So it is actually perfectly legal? Then it seems this patch
>>>>>>> is
>>>>>>> wrong
>>>>>> 
>>>>>> What is not "illegal": the HOMOGENEOUS flag being equal to true
>>>>>> *and*
>>>>>> having COD marker segments in tile-parts.
>>>>>> 
>>>>>> This is what the patch detects.
>>>>>> 
>>>>>> FFMPEG can decode such illegal codestream. Other decoders might
>>>>>> not.
>>>>>> 
>>>>>> The question is: what should FFMPEG do? Should FFMPEG exit or
>>>>>> warn
>>>>>> and continue.
>>>>> 
>>>>> If the spec allows it but it's perhaps unadviced then warning
>>>>> about it
>>>>> seems reasonable
>>>> 
>>>> (I totally messed up my double negative. Repeat below. Sorry for
>>>> the confusion.)
>>>> 
>>>> What is "illegal": the HOMOGENEOUS flag being equal to true *and*
>>>> having COD marker segments in tile-parts.
>>>> 
>>>> This is what the patch detects.
>>>> 
>>>> FFMPEG can decode such illegal codestream. Other decoders might
>>>> not.
>>>> 
>>>> The question is: what should FFMPEG do? Should FFMPEG exit or warn
>>>> and continue.
>>> 
>>> Does such a codestream actually exist ? I mean is this just a
>>> hypothetical case
>>> or something existing ?
>> 
>> This is more to stem the stream before anything happens because we were
>> lax with parsing
> 
> If such files dont exist then being picky makes sense
> 
> thx
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> You can kill me, but you cannot change the truth.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Osamu Watanabe Aug. 1, 2024, 2:40 a.m. UTC | #19
I found that the subject for this v5 set of patches was wrong.
I have posted the v6 with the correction.

> On Aug 1, 2024, at 10:58, WATANABE Osamu <owatanab@es.takushoku-u.ac.jp> wrote:
> 
> According to the suggestions for v4, I have posted the patch set v5.
> 
> It is confirmed that the decoder with v5 passes all the conformance testing defined in ISO/IEC 15444-4. 
> 
> Best,
> Osamu
> 
> 
>> On Jul 31, 2024, at 6:39, Michael Niedermayer <michael@niedermayer.cc> wrote:
>> 
>> On Tue, Jul 30, 2024 at 10:22:37PM +0200, Tomas Hardin wrote:
>>> fre 2024-07-26 klockan 23:29 +0200 skrev Michael Niedermayer:
>>>> On Thu, Jul 25, 2024 at 05:06:04PM -0700, Pierre-Anthony Lemieux
>>>> wrote:
>>>>> On Thu, Jul 25, 2024 at 2:17?AM Tomas Hardin <git@haerdin.se>
>>>>> wrote:
>>>>>> 
>>>>>> son 2024-07-21 klockan 14:07 +0900 skrev Pierre-Anthony Lemieux:
>>>>>>> On Sat, Jul 20, 2024 at 5:12?PM Tomas Hardin <git@haerdin.se>
>>>>>>> wrote:
>>>>>>>> 
>>>>>>>> tor 2024-07-18 klockan 23:10 +0900 skrev Pierre-Anthony
>>>>>>>> Lemieux:
>>>>>>>>> On Mon, Jul 15, 2024 at 10:33?PM Tomas Hardin
>>>>>>>>> <git@haerdin.se>
>>>>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>> fre 2024-07-12 klockan 12:51 -0700 skrev Pierre-Anthony
>>>>>>>>>> Lemieux:
>>>>>>>>>>> On Thu, Jul 11, 2024 at 10:28?PM Tomas Hardin
>>>>>>>>>>> <git@haerdin.se>
>>>>>>>>>>> wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>>> +            if (s->in_tile_headers == 1 && s->isHT
>>>>>>>>>>>>> &&
>>>>>>>>>>>>> (!s-
>>>>>>>>>>>>>> Ccap15_b11))
>>>>>>>>>>>>> +                av_log(s->avctx, AV_LOG_WARNING,
>>>>>>>>>>>>> "COD
>>>>>>>>>>>>> marker
>>>>>>>>>>>>> is
>>>>>>>>>>>>> found in HOMOGENEOUS HT set\n");
>>>>>>>>>>>> 
>>>>>>>>>>>> How bad is this and the other markers being present
>>>>>>>>>>>> in this
>>>>>>>>>>>> case?
>>>>>>>>>>> 
>>>>>>>>>>> At the very least, it means that signaling is
>>>>>>>>>>> inconsistent
>>>>>>>>>>> within
>>>>>>>>>>> the
>>>>>>>>>>> codestream since the standard states that:
>>>>>>>>>>> """
>>>>>>>>>>> The HOMOGENEOUS set is the set of HTJ2K codestreams
>>>>>>>>>>> where:
>>>>>>>>>>> ? none of the functional marker segments, e.g., COD,
>>>>>>>>>>> COC,
>>>>>>>>>>> RGN,
>>>>>>>>>>> QCD,
>>>>>>>>>>> QCC, and POC, are present in any
>>>>>>>>>>> tile-part header; and
>>>>>>>>>>> ? no PPT marker segment is present.
>>>>>>>>>>> """
>>>>>>>>>>> 
>>>>>>>>>>> The point of signalling that a codestream is
>>>>>>>>>>> "HOMOGENEOUS" is
>>>>>>>>>>> to
>>>>>>>>>>> allow
>>>>>>>>>>> decoders to configure themselves solely based on
>>>>>>>>>>> information
>>>>>>>>>>> retrieved
>>>>>>>>>>> entirely from the main header.
>>>>>>>>>>> 
>>>>>>>>>>> Since, AFAIK, FFMPEG does not rely on the HOMOGENEOUS
>>>>>>>>>>> to
>>>>>>>>>>> short-
>>>>>>>>>>> circuit
>>>>>>>>>>> configuration, incorrect HOMOGENEOUS signalling will
>>>>>>>>>>> likely
>>>>>>>>>>> not
>>>>>>>>>>> impact
>>>>>>>>>>> FFMPEG.
>>>>>>>>>> 
>>>>>>>>>> It could happen that information in tile headers
>>>>>>>>>> contradict
>>>>>>>>>> information
>>>>>>>>>> in the main header, right? In such a case it sounds like
>>>>>>>>>> we
>>>>>>>>>> can't
>>>>>>>>>> be
>>>>>>>>>> sure which decode is the correct one.
>>>>>>>>> 
>>>>>>>>> Per the spec, the decoder uses the COD information in tile-
>>>>>>>>> parts
>>>>>>>>> over
>>>>>>>>> the COD information in the header.
>>>>>>>>> 
>>>>>>>>> The issue here is that a decoder, upon seeing HOMOGENEOUS,
>>>>>>>>> simply
>>>>>>>>> does
>>>>>>>>> not bother with looking for COD information in tile-parts,
>>>>>>>>> thereby
>>>>>>>>> missing critical information.
>>>>>>>> 
>>>>>>>> So it is actually perfectly legal? Then it seems this patch
>>>>>>>> is
>>>>>>>> wrong
>>>>>>> 
>>>>>>> What is not "illegal": the HOMOGENEOUS flag being equal to true
>>>>>>> *and*
>>>>>>> having COD marker segments in tile-parts.
>>>>>>> 
>>>>>>> This is what the patch detects.
>>>>>>> 
>>>>>>> FFMPEG can decode such illegal codestream. Other decoders might
>>>>>>> not.
>>>>>>> 
>>>>>>> The question is: what should FFMPEG do? Should FFMPEG exit or
>>>>>>> warn
>>>>>>> and continue.
>>>>>> 
>>>>>> If the spec allows it but it's perhaps unadviced then warning
>>>>>> about it
>>>>>> seems reasonable
>>>>> 
>>>>> (I totally messed up my double negative. Repeat below. Sorry for
>>>>> the confusion.)
>>>>> 
>>>>> What is "illegal": the HOMOGENEOUS flag being equal to true *and*
>>>>> having COD marker segments in tile-parts.
>>>>> 
>>>>> This is what the patch detects.
>>>>> 
>>>>> FFMPEG can decode such illegal codestream. Other decoders might
>>>>> not.
>>>>> 
>>>>> The question is: what should FFMPEG do? Should FFMPEG exit or warn
>>>>> and continue.
>>>> 
>>>> Does such a codestream actually exist ? I mean is this just a
>>>> hypothetical case
>>>> or something existing ?
>>> 
>>> This is more to stem the stream before anything happens because we were
>>> lax with parsing
>> 
>> If such files dont exist then being picky makes sense
>> 
>> thx
>> 
>> [...]
>> -- 
>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>> 
>> You can kill me, but you cannot change the truth.
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

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..d1046661c4 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");
+        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");
+        // 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_MAGB = 8;
+        else if (P < 20)
+            s->HT_MAGB = P + 8;
+        else if (P < 31)
+            s->HT_MAGB = 4 * (P - 19) + 27;
+        else
+            s->HT_MAGB = 74;
+
+        if (s->HT_MAGB > 31) {
+                av_log(s->avctx, AV_LOG_ERROR, "Available internal precision is exceeded (MAGB> 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,43 @@  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_WARNING, "COC marker is found in HOMOGENEOUS HT set\n");
             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_WARNING, "COD marker is found in HOMOGENEOUS HT set\n");
             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_WARNING, "RGB marker is found in HOMOGENEOUS HT set\n");
             ret = get_rgn(s, len);
+            if ((!s->Ccap15_b12) && s->isHT)
+                av_log(s->avctx, AV_LOG_WARNING, "RGN marker is found in RGNFREE HT set\n");
             break;
         case JPEG2000_QCC:
+            if (s->in_tile_headers == 1 && s->isHT && (!s->Ccap15_b11))
+                av_log(s->avctx, AV_LOG_WARNING, "QCC marker is found in HOMOGENEOUS HT set\n");
             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_WARNING, "QCD marker is found in HOMOGENEOUS HT set\n");
             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_WARNING, "POC marker is found in HOMOGENEOUS HT set\n");
             ret = get_poc(s, len, poc);
             break;
         case JPEG2000_SOT:
@@ -2252,9 +2357,14 @@  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_WARNING, "PPT marker is found in HOMOGENEOUS HT set\n");
             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..326a572722 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_MAGB; // MAGB value
+
     /*options parameters*/
     int             reduction_factor;
 } Jpeg2000DecoderContext;