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

2023-09-22 Thread Cédric Le Barz


Le 04/06/2023 à 20:24, Tomas Härdin a écrit :

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-requ...@ffmpeg.org with subject "unsubscribe".


Hi,

I'm facing a problem to get the jpeg2000 information I need to build the 
mxf file.


All the functions to extract jpeg2000 information exist : they are 
located in libavcodec side and used Jpeg2000DecoderContext, which is not 
obviously public and therefore unknown from libavformat side.


Is there a way to get a pointer on the Jpeg2000DecoderContext (at least 
a void*) from libavcodec side ?


Thanks for your help.

Cédric



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

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


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

2023-06-04 Thread Tomas Härdin
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-requ...@ffmpeg.org with subject "unsubscribe".


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

2023-06-01 Thread Cédric Le Barz


Le 09/05/2023 à 16:28, Pierre-Anthony Lemieux a écrit :

Couple of follow-up comments.

- "mxf_parse_jpeg2000_frame" could be moved to one of jpeg2000 source
files, to keep J2K parsing code together. Maybe there is a way to
reuse jpeg2000_read_main_headers() at jpeg2000dec.c?

- when defining the J2K descriptor items, please refer to the symbol
name from the SMPTE registers, it make following/debugging the code a
lot easier:

{ 0x8405,
{0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x05,0x00,0x0
0,0x00}},
/* Vertical offset from the origin of the reference grid to the left
side of the image area */

becomes

{ 0x8405,
{0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x05,0x00,0x0
0,0x00}},
/* YOsiz: vertical offset from the origin of the reference grid to the
left side of the image area */
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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



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"


Regards,


Cédric
--- Begin Message ---
Signed-off-by: Cedric Le Barz 
---
 ffmpeg/libavcodec/j2kenc.c  |  28 +++---
 ffmpeg/libavcodec/jpeg2000.h|  90 +
 ffmpeg/libavcodec/jpeg2000dec.c |  89 +
 ffmpeg/libavformat/mxf.h|   1 +
 ffmpeg/libavformat/mxfenc.c | 169 +++-
 5 files changed, 273 insertions(+), 104 deletions(-)

diff --git a/ffmpeg/libavcodec/j2kenc.c b/ffmpeg/libavcodec/j2kenc.c
index 6406f90..f5178d7 100644
--- a/ffmpeg/libavcodec/j2kenc.c
+++ b/ffmpeg/libavcodec/j2kenc.c
@@ -106,7 +106,7 @@ static const int dwt_norms[2][4][10] = { // 
[dwt_type][band][rlevel] (multiplied
 typedef struct {
Jpeg2000Component *comp;
double *layer_rates;
-} Jpeg2000Tile;
+} J2kTile;
 
 typedef struct {
 AVClass *class;
@@ -131,7 +131,7 @@ typedef struct {
 Jpeg2000CodingStyle codsty;
 Jpeg2000QuantStyle  qntsty;
 
-Jpeg2000Tile *tile;
+J2kTile *tile;
 int layer_rates[100];
 uint8_t compression_rate_enc; ///< Is compression done using compression 
ratio?
 
@@ -171,7 +171,7 @@ static void dump(Jpeg2000EncoderContext *s, FILE *fd)
 s->width, s->height, s->tile_width, s->tile_height,
 s->numXtiles, s->numYtiles, s->ncomponents);
 for (tileno = 0; tileno < s->numXtiles * s->numYtiles; tileno++){
-Jpeg2000Tile *tile = s->tile + tileno;
+J2kTile *tile = s->tile + tileno;
 nspaces(fd, 2);
 fprintf(fd, "tile %d:\n", tileno);
 for(compno = 0; compno < s->ncomponents; compno++){
@@ -427,7 +427,7 @@ static void compute_rates(Jpeg2000EncoderContext* s)
 int layno, compno;
 for (i = 0; i < s->numYtiles; i++) {
 for (j = 0; j < s->numXtiles; j++) {
-Jpeg2000Tile *tile = >tile[s->numXtiles * i + j];
+J2kTile *tile = >tile[s->numXtiles * i + j];
 for (compno = 0; compno < s->ncomponents; compno++) {
 int tilew = tile->comp[compno].coord[0][1] - 
tile->comp[compno].coord[0][0];
 int tileh = tile->comp[compno].coord[1][1] - 
tile->comp[compno].coord[1][0];
@@ -460,12 +460,12 @@ static int init_tiles(Jpeg2000EncoderContext *s)
 s->numXtiles = ff_jpeg2000_ceildiv(s->width, s->tile_width);
 s->numYtiles = ff_jpeg2000_ceildiv(s->height, s->tile_height);
 
-s->tile = av_calloc(s->numXtiles, s->numYtiles * sizeof(Jpeg2000Tile));
+s->tile = av_calloc(s->numXtiles, s->numYtiles * sizeof(J2kTile));
 if (!s->tile)
 return AVERROR(ENOMEM);
 for (tileno = 0, tiley = 0; tiley < s->numYtiles; tiley++)
 for (tilex = 0; tilex < s->numXtiles; tilex++, tileno++){
-Jpeg2000Tile *tile = s->tile + tileno;
+J2kTile *tile = s->tile + tileno;
 
 tile->comp = av_calloc(s->ncomponents, sizeof(*tile->comp));
 if (!tile->comp)
@@ -509,7 +509,7 @@ static int init_tiles(Jpeg2000EncoderContext *s)
 

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

2023-05-09 Thread Pierre-Anthony Lemieux
Couple of follow-up comments.

- "mxf_parse_jpeg2000_frame" could be moved to one of jpeg2000 source
files, to keep J2K parsing code together. Maybe there is a way to
reuse jpeg2000_read_main_headers() at jpeg2000dec.c?

- when defining the J2K descriptor items, please refer to the symbol
name from the SMPTE registers, it make following/debugging the code a
lot easier:

{ 0x8405, 
{0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x05,0x00,0x00,0x00}},
/* Vertical offset from the origin of the reference grid to the left
side of the image area */

becomes

{ 0x8405, 
{0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x05,0x00,0x00,0x00}},
/* YOsiz: vertical offset from the origin of the reference grid to the
left side of the image area */
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


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

2023-05-09 Thread Tomas Härdin
> +if (j2k_ncomponents != component_count) {
> +av_log(s, AV_LOG_ERROR, "Incoherence about components image
> number.\n");
> +}

I still think you should error out here, since mismatched component
count is indicative of broken internal logic

/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-requ...@ffmpeg.org with subject "unsubscribe".


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

2023-05-02 Thread Cédric Le Barz


Le 27/04/2023 à 14:45, Tomas Hardin a écrit :

  static inline uint16_t rescale_mastering_chroma(AVRational q)
  {
@@ -1260,7 +1305,6 @@ static int64_t
mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
  f1 *= 2;
  }
  
-

Stray deleted line


+/* Image components number */
+mxf_write_local_tag(s, 2, 0x840A);
+avio_wb16(pb, component_count);
+   /* Array of picture components where each component comprises 3

Looks like you missed a space there (:


+/* Extract usefull size infromation from the SIZ marker */
+if (bytestream2_get_be16u() != JPEG2000_SIZ) {
+av_log(s, AV_LOG_ERROR, "SIZ marker not present\n");
+return 0;
+}
+bytestream2_skip(, 2); // Skip Lsiz
+sc->j2k_cap = bytestream2_get_be16u();
+sc->j2k_xsiz = bytestream2_get_be32u();
+sc->j2k_ysiz = bytestream2_get_be32u();
+sc->j2k_x0siz = bytestream2_get_be32u();
+sc->j2k_y0siz = bytestream2_get_be32u();
+sc->j2k_xtsiz = bytestream2_get_be32u();
+sc->j2k_ytsiz = bytestream2_get_be32u();
+sc->j2k_xt0siz = bytestream2_get_be32u();
+sc->j2k_yt0siz = bytestream2_get_be32u();
+j2k_ncomponents = bytestream2_get_be16u();
+if (j2k_ncomponents != component_count) {
+av_log(s, AV_LOG_WARNING, "Incoherence about components
image number.\n");

Erroring out here seems more appropriate.

/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-requ...@ffmpeg.org with subject "unsubscribe".


I've attached to this mail the new patch taking into account the 3 
remarks above.


Regards

Cédric
--- Begin Message ---
Signed-off-by: Cedric Le Barz 
---
 ffmpeg/libavformat/mxf.h|   1 +
 ffmpeg/libavformat/mxfenc.c | 169 +++-
 2 files changed, 168 insertions(+), 2 deletions(-)

diff --git a/ffmpeg/libavformat/mxf.h b/ffmpeg/libavformat/mxf.h
index 2561605..7dd1681 100644
--- a/ffmpeg/libavformat/mxf.h
+++ b/ffmpeg/libavformat/mxf.h
@@ -55,6 +55,7 @@ enum MXFMetadataSetType {
 SoundfieldGroupLabelSubDescriptor,
 GroupOfSoundfieldGroupsLabelSubDescriptor,
 FFV1SubDescriptor,
+JPEG2000SubDescriptor,
 };
 
 enum MXFFrameLayout {
diff --git a/ffmpeg/libavformat/mxfenc.c b/ffmpeg/libavformat/mxfenc.c
index a29d678..909682a 100644
--- a/ffmpeg/libavformat/mxfenc.c
+++ b/ffmpeg/libavformat/mxfenc.c
@@ -48,8 +48,10 @@
 #include "libavutil/pixdesc.h"
 #include "libavutil/time_internal.h"
 #include "libavcodec/avcodec.h"
+#include "libavcodec/bytestream.h"
 #include "libavcodec/golomb.h"
 #include "libavcodec/h264.h"
+#include "libavcodec/jpeg2000.h"
 #include "libavcodec/packet_internal.h"
 #include "libavcodec/startcode.h"
 #include "avformat.h"
@@ -102,6 +104,16 @@ typedef struct MXFStreamContext {
 int b_picture_count; ///< maximum number of consecutive b pictures, 
used in mpeg-2 descriptor
 int low_delay;   ///< low delay, used in mpeg-2 descriptor
 int avc_intra;
+uint16_t j2k_cap;///< j2k required decoder capabilities
+uint32_t j2k_xsiz;   ///< j2k widht of the reference grid
+uint32_t j2k_ysiz;   ///< j2k height of the reference grid
+uint32_t j2k_x0siz;  ///< j2k horizontal offset from the origin of the 
reference grid to the left side of the image
+uint32_t j2k_y0siz;  ///< j2k vertical offset from the origin of the 
reference grid to the left side of the image
+uint32_t j2k_xtsiz;  ///< j2k width of one reference tile with respect 
to the reference grid
+uint32_t j2k_ytsiz;  ///< j2k height of one reference tile with 
respect to the reference grid
+uint32_t j2k_xt0siz; ///< j2k horizontal offset from the origin of the 
reference grid to the left side of the first tile
+uint32_t j2k_yt0siz; ///< j2k vertical offset from the origin of the 
reference grid to the left side of the first tile
+uint8_t  j2k_comp_desc[12]; ///< j2k components descriptor
 } MXFStreamContext;
 
 typedef struct MXFContainerEssenceEntry {
@@ -390,6 +402,20 @@ static const MXFLocalTagPair mxf_local_tag_batch[] = {
 { 0x8302, FF_MXF_MasteringDisplayWhitePointChromaticity },
 { 0x8303, FF_MXF_MasteringDisplayMaximumLuminance },
 { 0x8304, FF_MXF_MasteringDisplayMinimumLuminance },
+// ff_mxf_jpeg2000_local_tags
+{ 0x8400, 
{0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x09,0x06,0x01,0x01,0x04,0x06,0x10,0x00,0x00}},
 /* Sub Descriptors / Opt Ordered array of strong references to sub descriptor 
sets */
+{ 0x8401, 
{0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x01,0x00,0x00,0x00}},
 /* An enumerated value that defines the decoder capabilities */
+{ 0x8402, 
{0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x02,0x00,0x00,0x00}},
 /* Width of the reference grid */
+{ 0x8403, 

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

2023-04-27 Thread Tomas Härdin
>  static inline uint16_t rescale_mastering_chroma(AVRational q)
>  {
> @@ -1260,7 +1305,6 @@ static int64_t
> mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
>  f1 *= 2;
>  }
>  
> -

Stray deleted line

> +/* Image components number */
> +mxf_write_local_tag(s, 2, 0x840A);
> +avio_wb16(pb, component_count);
> +   /* Array of picture components where each component comprises 3

Looks like you missed a space there (:

> +/* Extract usefull size infromation from the SIZ marker */
> +if (bytestream2_get_be16u() != JPEG2000_SIZ) {
> +av_log(s, AV_LOG_ERROR, "SIZ marker not present\n");
> +return 0;
> +}
> +bytestream2_skip(, 2); // Skip Lsiz
> +sc->j2k_cap = bytestream2_get_be16u();
> +sc->j2k_xsiz = bytestream2_get_be32u();
> +sc->j2k_ysiz = bytestream2_get_be32u();
> +sc->j2k_x0siz = bytestream2_get_be32u();
> +sc->j2k_y0siz = bytestream2_get_be32u();
> +sc->j2k_xtsiz = bytestream2_get_be32u();
> +sc->j2k_ytsiz = bytestream2_get_be32u();
> +sc->j2k_xt0siz = bytestream2_get_be32u();
> +sc->j2k_yt0siz = bytestream2_get_be32u();
> +j2k_ncomponents = bytestream2_get_be16u();
> +if (j2k_ncomponents != component_count) {
> +av_log(s, AV_LOG_WARNING, "Incoherence about components
> image number.\n");

Erroring out here seems more appropriate.

/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-requ...@ffmpeg.org with subject "unsubscribe".


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

2023-04-25 Thread Cédric Le Barz


Le 05/04/2023 à 15:53, Tomas Härdin a écrit :

ons 2023-04-05 klockan 15:05 +0200 skrev Cédric Le Barz:

Le 03/04/2023 à 17:14, Michael Niedermayer a écrit :

On Mon, Apr 03, 2023 at 10:08:25AM +0200, Cédric Le Barz wrote:

Hi,

I've attached the patch to this mail, in order to solve newlines

insertion

issue.

Please make sure each patch also updates the fate tests so
make fate
doesnt fail

I've attached to this mail the new patch. Fate test issue is fixed.


Please avoid top posting.

I was actually about to suggest merging these two patches but I see you
read my mind :)


@@ -1131,9 +1164,9 @@ static const UID mxf_aes3_descriptor_key  =
{ 0x06,0x0E,0x2B,0x34,0x02,0x53,
  static const UID mxf_cdci_descriptor_key  = {
0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0D,0x01,0x01,0x01,0x01,0x01
,0x28,0x00 };
  static const UID mxf_rgba_descriptor_key  = {
0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0D,0x01,0x01,0x01,0x01,0x01
,0x29,0x00 };
  static const UID mxf_generic_sound_descriptor_key = {
0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0D,0x01,0x01,0x01,0x01,0x01
,0x42,0x00 };
-

Stray line deletion


+mxf_write_local_tag(s, 2, 0x8401);
+avio_wb16(pb, 0x);
+mxf_write_local_tag(s, 4, 0x8402);
+avio_wb32(pb, st->codecpar->width);
+mxf_write_local_tag(s, 4, 0x8403);
+avio_wb32(pb, st->codecpar->height);
+mxf_write_local_tag(s, 4, 0x8404);
+avio_wb32(pb, 0);
+mxf_write_local_tag(s, 4, 0x8405);
+avio_wb32(pb, 0);
+mxf_write_local_tag(s, 4, 0x8406);
+avio_wb32(pb, st->codecpar->width);
+mxf_write_local_tag(s, 4, 0x8407);
+avio_wb32(pb, st->codecpar->height);
+mxf_write_local_tag(s, 4, 0x8408);
+avio_wb32(pb, 0);
+mxf_write_local_tag(s, 4, 0x8409);
+avio_wb32(pb, 0);
+mxf_write_local_tag(s, 2, 0x840A);
+avio_wb16(pb, component_count);

A comment on each of these explaining what they are would be nice.


+{
+char _desc [3][3]= {  {0x09,0x01,0x01} , {0x09,0x02,0x01} ,
{0x09,0x02,0x01} };
+int comp = 0;
+for ( comp = 0; comp< component_count ;comp++ ) {
+avio_write(pb, _desc[comp%3] , 3);
+}
+}

Maybe just a style nit but you could move the char desc[] into the loop
body, int comp to the start of the function and then you can remove the
extra {} around this. Also you could make desc static const.


+{
+char _layout[16] = {  'Y' , '\n', 'U' , '\n', 'V' , '\n',
'F' , 0x02,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00 };
+avio_write(pb, _layout , 16);
+}

Again there is the issue of RGB(A)

/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-requ...@ffmpeg.org with subject "unsubscribe".



Please consider this new patch taking into account remarks. For the 
moment, I remove the RGB(A) / YUV code part as it is an optional feature 
for the JPEG2000 subdescriptor.


Regards,

Cédric
--- Begin Message ---
Signed-off-by: Cedric Le Barz 
---
 ffmpeg/libavformat/mxf.h|   1 +
 ffmpeg/libavformat/mxfenc.c | 169 +++-
 2 files changed, 167 insertions(+), 3 deletions(-)

diff --git a/ffmpeg/libavformat/mxf.h b/ffmpeg/libavformat/mxf.h
index 2561605..7dd1681 100644
--- a/ffmpeg/libavformat/mxf.h
+++ b/ffmpeg/libavformat/mxf.h
@@ -55,6 +55,7 @@ enum MXFMetadataSetType {
 SoundfieldGroupLabelSubDescriptor,
 GroupOfSoundfieldGroupsLabelSubDescriptor,
 FFV1SubDescriptor,
+JPEG2000SubDescriptor,
 };
 
 enum MXFFrameLayout {
diff --git a/ffmpeg/libavformat/mxfenc.c b/ffmpeg/libavformat/mxfenc.c
index a29d678..7065a7d 100644
--- a/ffmpeg/libavformat/mxfenc.c
+++ b/ffmpeg/libavformat/mxfenc.c
@@ -48,8 +48,10 @@
 #include "libavutil/pixdesc.h"
 #include "libavutil/time_internal.h"
 #include "libavcodec/avcodec.h"
+#include "libavcodec/bytestream.h"
 #include "libavcodec/golomb.h"
 #include "libavcodec/h264.h"
+#include "libavcodec/jpeg2000.h"
 #include "libavcodec/packet_internal.h"
 #include "libavcodec/startcode.h"
 #include "avformat.h"
@@ -102,6 +104,16 @@ typedef struct MXFStreamContext {
 int b_picture_count; ///< maximum number of consecutive b pictures, 
used in mpeg-2 descriptor
 int low_delay;   ///< low delay, used in mpeg-2 descriptor
 int avc_intra;
+uint16_t j2k_cap;///< j2k required decoder capabilities
+uint32_t j2k_xsiz;   ///< j2k widht of the reference grid
+uint32_t j2k_ysiz;   ///< j2k height of the reference grid
+uint32_t j2k_x0siz;  ///< j2k horizontal offset from the origin of the 
reference grid to the left side of the image
+uint32_t j2k_y0siz;  ///< j2k vertical offset from the origin of the 
reference grid to the left side of the image
+uint32_t j2k_xtsiz;  ///< j2k width of one reference tile with respect 
to the reference grid
+

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

2023-04-22 Thread Pierre-Anthony Lemieux
On Wed, Mar 29, 2023 at 1:54 PM Cédric Le Barz  wrote:
>
> Add jpeg2000 subdescriptor in MXF file.
>
> Signed-off-by: Cedric Le Barz 
> ---
>   ffmpeg/libavformat/mxf.h|  1 +
>   ffmpeg/libavformat/mxfenc.c | 74 -
>   2 files changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/ffmpeg/libavformat/mxf.h b/ffmpeg/libavformat/mxf.h
> index 2561605..7dd1681 100644
> --- a/ffmpeg/libavformat/mxf.h
> +++ b/ffmpeg/libavformat/mxf.h
> @@ -55,6 +55,7 @@ enum MXFMetadataSetType {
>   SoundfieldGroupLabelSubDescriptor,
>   GroupOfSoundfieldGroupsLabelSubDescriptor,
>   FFV1SubDescriptor,
> +JPEG2000SubDescriptor,
>   };
>enum MXFFrameLayout {
> diff --git a/ffmpeg/libavformat/mxfenc.c b/ffmpeg/libavformat/mxfenc.c
> index a29d678..3bdf90a 100644
> --- a/ffmpeg/libavformat/mxfenc.c
> +++ b/ffmpeg/libavformat/mxfenc.c
> @@ -390,6 +390,20 @@ static const MXFLocalTagPair mxf_local_tag_batch[] = {
>   { 0x8302, FF_MXF_MasteringDisplayWhitePointChromaticity },
>   { 0x8303, FF_MXF_MasteringDisplayMaximumLuminance },
>   { 0x8304, FF_MXF_MasteringDisplayMinimumLuminance },
> +// ff_mxf_jpeg2000_local_tags
> +{ 0x8400,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x09,0x06,0x01,0x01,0x04,0x06,0x10,0x00,0x00}},
> /* Sub Descriptors / Opt Ordered array of strong references to sub
> descriptor sets */
> +{ 0x8401,
> {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x01,0x00,0x00,0x00}},
> /* 2 bytes : An enumerated value that defines the decoder capabilities.  */

Please add to the comment the symbol and type of the attribute as
specified in the SMPTE registers [1]  -- it makes it easier to
review/debug.

In the case above, it would be Riz (UInt16).

[1] https://registry.smpte-ra.org/view/published/elements_by_group_view.html

> +{ 0x8402,
> {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x02,0x00,0x00,0x00}},
> /* 4 bytes : Width of the reference grid */
> +{ 0x8403,
> {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x03,0x00,0x00,0x00}},
> /* 4 bytes : Height of the reference grid */
> +{ 0x8404,
> {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x04,0x00,0x00,0x00}},
> /* 4 bytes : Horizontal offset from the origin of the reference grid to
> the left side of the image area */
> +{ 0x8405,
> {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x05,0x00,0x00,0x00}},
> /* 4 bytes : Vertical offset from the origin of the reference grid to
> the left side of the image area */
> +{ 0x8406,
> {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x06,0x00,0x00,0x00}},
> /* 4 bytes : Width of one reference tile with respect to the reference
> grid, */
> +{ 0x8407,
> {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x07,0x00,0x00,0x00}},
> /* 4 bytes : Height of one reference tile with respect to the reference
> grid, */
> +{ 0x8408,
> {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x08,0x00,0x00,0x00}},
> /* 4 bytes : Horizontal offset from the origin of the reference grid to
> the left side of the first tile */
> +{ 0x8409,
> {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x09,0x00,0x00,0x00}},
> /* 4 bytes : Vertical offset from the origin of the reference grid to
> the left side of the first tile */
> +{ 0x840A,
> {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x0A,0x00,0x00,0x00}},
> /* 2 bytes : The number of components in the picture */
> +{ 0x840B,
> {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x0B,0x00,0x00,0x00}},
> /* 8+3n bytes : Array of picture components where each component
> comprises 3 bytes named Ssizi, XRSizi, YRSizi  The array of 3-byte
> groups is preceded by the array header comprising a 4-byte value of the
> number of components followed by a 4-byte value of 3. */
> +{ 0x840C,
> {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x0E,0x00,0x00,0x00}},
> /* The nature and order of the image components in the compressed domain
> as carried in the J2C codestream.. */
>   };
>#define MXF_NUM_TAGS FF_ARRAY_ELEMS(mxf_local_tag_batch)
> @@ -1095,8 +1109,8 @@ static const UID mxf_wav_descriptor_key   = {
> 0x06,0x0E,0x2B,0x34,0x02,0x53,
>   static const UID mxf_aes3_descriptor_key  = {
> 0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x47,0x00
> };
>   static const UID mxf_cdci_descriptor_key  = {
> 0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0D,0x01,0x01,0x01,0x01,0x01,0x28,0x00
> };
>   static const UID mxf_generic_sound_descriptor_key = {
> 0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0D,0x01,0x01,0x01,0x01,0x01,0x42,0x00
> };
> -
>   static const UID mxf_avc_subdescriptor_key = {
> 0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x6E,0x00
> };
> +static const UID mxf_jpeg2000_subdescriptor_key   = {
> 0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0D,0x01,0x01,0x01,0x01,0x01,0x5A,00
> };
>