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

[Tomas Härdin]

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 at 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 at 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