[FFmpeg-devel] [PATCH] [MXF] - Add jpeg2000 subdescriptor in MXF file (V2).

[Tomas Härdin]

sön 2024-10-27 klockan 21:06 -0700 skrev Pierre-Anthony Lemieux:
> av_pix_fmt_count_planes() in
> 
> On Thu, Dec 21, 2023 at 3:11 AM Tomas Härdin <git at haerdin.se> wrote:
> > 
> > ons 2023-12-20 klockan 18:04 +0100 skrev Cédric Le Barz:
> > > Le 19/12/2023 à 14:36, Tomas Härdin a écrit :
> > > > > +    for ( comp = 0; comp < component_count; comp++ ) {
> > > > > +        avio_write(pb, &sc->j2k_info.j2k_comp_desc[3*comp] ,
> > > > > 3);
> > > > > +    }
> > > > Looks like this could be simplified to just
> > > > 
> > > >    avio_write(pb, sc->j2k_info.j2k_comp_desc,
> > > > 3*component_count);
> > > > 
> > > > > +    if (j2k_ncomponents != component_count) {
> > > > > +        av_log(s, AV_LOG_ERROR, "Incoherence about
> > > > > components
> > > > > image
> > > > > number.\n");
> > > > > +    }
> > > > I again feel this should be a hard error
> > > > 
> > > > > +    for (comp = 0; comp < j2k_ncomponents; comp++) {
> > > > > +        sc->j2k_info.j2k_comp_desc[comp*j2k_ncomponents] =
> > > > > bytestream2_get_byteu(&g);   // Bitdepth for each component
> > > > > +        sc->j2k_info.j2k_comp_desc[comp*j2k_ncomponents+1] =
> > > > > bytestream2_get_byteu(&g); // Horizontal sampling for each
> > > > > component
> > > > > +        sc->j2k_info.j2k_comp_desc[comp*j2k_ncomponents+2] =
> > > > > bytestream2_get_byteu(&g); // Vertical sampling for each
> > > > > component
> > > > > +    }
> > > > Could be simplified to a single avio_read()
> > > > 
> > > > /Tomas
> > > > _______________________________________________
> > > > ffmpeg-devel mailing list
> > > > ffmpeg-devel at ffmpeg.org
> > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > > 
> > > > To unsubscribe, visit link above, or email
> > > > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> > > 
> > > Here is the new version of the patch taken into account your
> > > remarks.
> > 
> > Looks OK. Does this also work with RGB(A)? Seems to not be
> > hardcoded
> > for YUV at least. Higher bitdepths would also be nice, I've been
> > working with lossless RGB48 J2K. Doesn't necessarily need to hold
> > up
> > this patch, just curious.
> 
> av_pix_fmt_count_planes() in mxf_write_jpeg2000_subdesc() will not
> work for RGB(A) since the number of planes will not be equal to the
> number of J2K components.
> 
> See https://trac.ffmpeg.org/ticket/11267

Good catch. I'll send a small patch for it.

I see mjpegdec also uses av_pix_fmt_count_planes() for counting
components..

/Tomas