Re: [FFmpeg-devel] [PATCH] MXF - Add jpeg2000 subdescriptor - Sponsored by INA

["Tomas Härdin" <git@haerdin.se>]

tor 2023-06-01 klockan 17:19 +0200 skrev Cédric Le Barz:
> Attach to this mail, is my new patch for adding jpeg2000 sub-
> descriptor 
> in MXF file  taking into account remarks from FFmpeg community
> (remarks 
> from Pierre-Anthony above as well as this from Tomas), i.e. :
> 
> 1 - When there is a mismatch in components number, this is now
> process 
> as an error.
> 
> 2 - When defining and using the J2K descriptor items, I now refer to
> the 
> symbol name from the SMPTE registers (X0siz, XT0siz...)
> 
> 3 - As suggested, I make use of  jpeg2000_read_main_headers() in 
> libavcodec. But, I let the parsing function in mxfenc.c file as all 
> other parsing functions are in this file (mxf_parse_mpeg2_frame, 
> mxf_parse_h264_frame...). The use of jpeg2000_read_main_headers() in 
> mxfenc implies some minor modifications in jpeg2000 files :
> 
> * rename of Jpeg2000Tile structure to J2kTile in j2kenc.c (to avoid 
> redefinition)
> 
> * move some structure declarations from jpeg2000dec.c to jpeg2000.h
> 
> * make jpeg2000_read_main_headers() function "public"

For this you need to prefix the name with ff_ and bump libavcodec's
minor version number

> +static int mxf_parse_jpeg2000_frame(AVFormatContext *s, AVStream
> *st, AVPacket *pkt)
> +{
> +    MXFContext *mxf = s->priv_data;
> +    MXFStreamContext *sc = st->priv_data;
> +    int component_count = av_pix_fmt_count_planes(st->codecpar-
> >format);
> +    Jpeg2000DecoderContext jpeg2000ctx;

This makes sizeof(Jpeg2000DecoderContext) part of the public API which
is a big no-no. Consider making the fields part of a different smaller
struct instead that is also included in Jpeg2000DecoderContext. The
safest option is to have a function that allocates that struct. The
reason for this is because we might want to read other things from the
headers at a later date. Another possibility is to pass a bunch of
pointers to ints that the parsed values get written to. If at a later
date we need to parse more fields then we can introduce
ff_jpeg2000_read_main_headers_2() and so on. The latter seems easier
API stability wise, if a bit tedious with the number of function
arguments.

/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".