feat(encoder): Added support for mastering display color volume and c… #168

dkozinski · 2025-11-17T14:52:41Z

…ontent light level metadata

…ontent light level metadata Signed-off-by: Dawid Kozinski <[email protected]>

kpchoi · 2025-11-17T21:50:10Z

I guess you're trying to support HDR information, which is a part of metadata not encoding part.
Check this syntaxes;
https://www.ietf.org/archive/id/draft-lim-apv-09.html#section-8

You need to create metadata related MDCV and CLL, after that you can add it to frame data.
OpenAPV APIs provides how to create metadata and add it.
I believe that this PR needs to be closed.

dkozinski · 2025-11-18T10:44:00Z

Yes, I am indeed trying to support HDR information (There is a request from library users to add the ability to pass metadata to the APV stream: GitHub Issue #167).

The liboapv library includes an API for adding metadata, and the header file oapv.h provides an interface for the metadata container.

I used the function OAPV_EXPORT int oapvm_set(oapvm_t mid, int group_id, int type, void *data, int size) to add metadata:

Mastering display color volume metadata (oapv_md_mdcv) and
Content light level information (oapv_md_cll).

This function requires passing the metadata type (type definitions are in the public header oapv.h, in our case OAPV_METADATA_MDCV and OAPV_METADATA_CLL), along with a pointer to the data and its size.

The file oapv_metadata.h already contains definitions for the appropriate structures for MDCV and CLL:

/* Mastering display color volume metadata */
typedef struct oapv_md_mdcv oapv_md_mdcv_t;
struct oapv_md_mdcv {
    u16 primary_chromaticity_x[3];  /* u(16) */
    u16 primary_chromaticity_y[3];  /* u(16) */
    u16 white_point_chromaticity_x; /* u(16) */
    u16 white_point_chromaticity_y; /* u(16) */
    u32 max_mastering_luminance;    /* u(32) */
    u32 min_mastering_luminance;    /* u(32) */
};

/* Content light level information */
typedef struct oapv_md_cll oapv_md_cll_t;
struct oapv_md_cll {
    u16 max_cll;  /* u(16) */
    u16 max_fall; /* u(16) */
};

However, oapv_metadata.h is not a public header, and these types are not accessible to library users.

In PR#168, I added the appropriate fields (those from the structures oapv_md_mdcv and oapv_md_cll defined in oapv_metadata.h) to the oapv_param structure.
This was done to allow storing metadata (Mastering display color volume and Content light level information) provided by the user and then setting it using oapvm_set(oapvm_t mid, int group_id, int type, void *data, int size).

What can we do?

Instead of using oapv_params to store user-provided metadata, we can add a structure definition for metadata in oapv.h, e.g., oapv_metadata, to collect metadata passed by the user.
Make the following structures public and embed them in oapv_metadata:

/* Recommendation ITU-T T.35 metadata */
typedef struct oapv_md_t35 oapv_md_t35_t;
struct oapv_md_t35 {
    u32 itu_t_t35_country_code;           /* b(8) */
    u32 itu_t_t35_country_code_extension; /* b(8) */
    u8 *itu_t_t35_payload;
};

/* Mastering display color volume metadata */
typedef struct oapv_md_mdcv oapv_md_mdcv_t;
struct oapv_md_mdcv {
    u16 primary_chromaticity_x[3];  /* u(16) */
    u16 primary_chromaticity_y[3];  /* u(16) */
    u16 white_point_chromaticity_x; /* u(16) */
    u16 white_point_chromaticity_y; /* u(16) */
    u32 max_mastering_luminance;    /* u(32) */
    u32 min_mastering_luminance;    /* u(32) */
};

/* Content light level information */
typedef struct oapv_md_cll oapv_md_cll_t;
struct oapv_md_cll {
    u16 max_cll;  /* u(16) */
    u16 max_fall; /* u(16) */
};

Add a corresponding function oapvm_metadata_parse() analogous to oapve_param_parse() but designed to handle metadata.

What's your thoughts?

kpchoi · 2025-11-18T18:34:25Z

I am thinking that the metadata embedding doesn't need to have xxx_parse() function.
So, I'd like to suggest to not to use oapv_params command line arguments for passing this information.
It could be supported separated arguments if you can.

- Responsibility for HDR metadata preparation moved from the library to the application side - Used the liboapv metadata container API to pass the prepared metadata to the library Signed-off-by: Dawid Kozinski <[email protected]>

kpchoi · 2025-11-19T23:45:05Z

Still, the encoder of OpenAPV needs to support commandline argument for setting max-cll and mdcv?
This PR has the changes, but it seems to need many refactoring to support it properly.

dkozinski · 2025-11-20T08:36:36Z

Currently there are no changes in the encoder implementation regarding support for HDR-related metadata; it does not include handling for master-display and max-cll parameters via the command line.

PR #168 only adds support for the master-display and max-cll parameters to the reference application (request from library users to add the ability to pass metadata to the APV stream: GitHub issue #167).

This is solely a change in the reference application to support HDR, not in the encoder.

The user can pass mastering display color volume metadata and content light level information metadata to the reference application. Then the reference application passes this data to the metadata container using the public liboapv API (oapvm_set_all()).

kpchoi · 2025-11-21T16:28:41Z

@dkozinski
The metadata is a binary reprsentation, which is different from C-structure.
So, I can say it is not possible to store C-structured data to bitstream directly.
Futhermore, the metadata syntaxes use big endian, but mostly computing system supports little endian.
And, sometimes, C-compiler tries to change the postion of the structure's members during its optimzation process.
I could prepare code patch for resoloving this issue with my idea soon.

Signed-off-by: Dawid Kozinski <[email protected]>

- Detected platform endianness (Big/Little Endian) in CMake configuration - Added conditional compilation for endianness-specific byte order conversion in oapv_app_enc.c - Introduced helper macros (bswap16, bswap32, bswap64) and endian conversion functions (be2ne, le2ne, ne2be, ne2le) in oapv_app_util.h - Updated metadata serialization functions to use new byte order conversion utilities for consistent cross-platform behavior Signed-off-by: Dawid Kozinski <[email protected]>

dkozinski · 2025-11-25T21:51:58Z

@dkozinski The metadata is a binary reprsentation, which is different from C-structure. So, I can say it is not possible to store C-structured data to bitstream directly. Futhermore, the metadata syntaxes use big endian, but mostly computing system supports little endian. And, sometimes, C-compiler tries to change the postion of the structure's members during its optimzation process. I could prepare code patch for resoloving this issue with my idea soon.

@kpchoi
I've added metadata serialization with network byte order support.

kpchoi · 2025-11-28T00:59:46Z

@dkozinski The metadata is a binary reprsentation, which is different from C-structure. So, I can say it is not possible to store C-structured data to bitstream directly. Futhermore, the metadata syntaxes use big endian, but mostly computing system supports little endian. And, sometimes, C-compiler tries to change the postion of the structure's members during its optimzation process. I could prepare code patch for resoloving this issue with my idea soon.

@kpchoi I've added metadata serialization with network byte order support.

I think your patch is not the usual way of video codec area even though the results are similar.
I will rase code patch as soon as possible.

Signed-off-by: [email protected] <[email protected]>

kpchoi · 2025-11-29T07:17:12Z

@dkozinski Please check the latest commits from me.
I think that testing of metadata embedding with your test set is neccessary.

dkozinski · 2025-11-30T23:23:12Z

@kpchoi I’ll check your latest changes and run the tests as soon as possible this morning

Signed-off-by: [email protected] <[email protected]>

src/oapv_metadata.c

dkozinski · 2025-12-01T09:03:30Z

src/oapv_metadata.c

+    }
+    for(i = 0; i < 3; i++) {
+        t = mdcv->primary_chromaticity_y[i];
+        oapv_assert_rv(t >=0 && t < (2^16), OAPV_ERR_INVALID_ARGUMENT);


(2^16) =>(1<<16)

Mistake, It is changed to 0xFFFF

src/oapv_metadata.c

dkozinski · 2025-12-01T09:03:45Z

src/oapv_metadata.c

+        oapv_bsw_write(&bs, t, 16);
+    }
+    t = mdcv->white_point_chromaticity_x;
+    oapv_assert_rv(t >=0 && t < (2^16), OAPV_ERR_INVALID_ARGUMENT);


(2^16) =>(1<<16)

Mistake, It is changed to 0xFFFF

dkozinski · 2025-12-01T09:03:58Z

src/oapv_metadata.c

+    oapv_bsw_write(&bs, t, 16);
+
+    t = mdcv->white_point_chromaticity_y;
+    oapv_assert_rv(t >=0 && t < (2^16), OAPV_ERR_INVALID_ARGUMENT);


(2^16) =>(1<<16)

Mistake, It is changed to 0xFFFF

dkozinski · 2025-12-01T09:05:23Z

src/oapv_metadata.c

+    oapv_assert_rv(t >=0 && t < (2^16), OAPV_ERR_INVALID_ARGUMENT);
+    oapv_bsw_write(&bs, t, 16);
+
+    oapv_assert_rv(mdcv->max_mastering_luminance < (2^32), OAPV_ERR_INVALID_ARGUMENT);


The result of 2^32 using the bitwise XOR operator (^) in C is 2 XOR 32 = 34, not the intended exponentiation result of 4,294,967,296

Please change (2^32) to (1ULL << 32)

Mistake, It is changed to 0xFFFFFFFF

dkozinski · 2025-12-01T09:05:36Z

src/oapv_metadata.c

+    tu32 = mdcv->max_mastering_luminance;
+    oapv_bsw_write(&bs, tu32, 32);
+
+    oapv_assert_rv(mdcv->min_mastering_luminance < (2^32), OAPV_ERR_INVALID_ARGUMENT);


(2^32) to (1ULL << 32)

Mistake, It is changed to 0xFFFFFFFF

dkozinski · 2025-12-01T09:06:01Z

src/oapv_metadata.c

+    oapv_bsw_init(&bs, data, 4, NULL); // CLL payload has 4 bytes
+
+    t = cll->max_cll;
+    oapv_assert_rv(t >=0 && t < (2^16), OAPV_ERR_INVALID_ARGUMENT);


(2^16) =>(1<<16)

Mistake, It is changed to 0xFFFF

dkozinski · 2025-12-01T09:06:12Z

src/oapv_metadata.c

+    oapv_bsw_write(&bs, t, 16);
+
+    t = cll->max_fall;
+    oapv_assert_rv(t >=0 && t < (2^16), OAPV_ERR_INVALID_ARGUMENT);


(2^16) =>(1<<16)

Mistake, It is changed to 0xFFFF

src/oapv_metadata.c

dkozinski · 2025-12-01T10:37:50Z

app/oapv_app_enc.c

+
+static int update_metadata(args_var_t *vars, oapvm_t mid)
+{
+    int ret, size;


ret is not initialized at the start of the function, so if everything succeeds, the function returns an undefined value. Please initialize ret to a default value (0 for success).

ret is still not initialized causes the function to return an invalid value on success.

static int update_metadata(args_var_t *vars, oapvm_t mid) { int ret = 0, size;

Signed-off-by: [email protected] <[email protected]>

kpchoi · 2025-12-02T02:51:37Z

@dkozinski
Could you test it again?

dkozinski

some changes are still needed

dkozinski · 2025-12-02T07:23:14Z

app/oapv_app_enc.c

+
+static int update_metadata(args_var_t *vars, oapvm_t mid)
+{
+    int ret, size;


ret is still not initialized causes the function to return an invalid value on success.

static int update_metadata(args_var_t *vars, oapvm_t mid) { int ret = 0, size;

src/oapv_metadata.c

app/oapv_app_enc.c

kpchoi · 2025-12-03T06:24:20Z

@dkozinski plesae test the current code again.
And, if you need to modify in application code, you could commit your changes.

- Swapped the order of primary chromaticity coordinates in 'parse_master_display' function to ensure correct assignment - Refactor writing primary chromaticity x and y coordinates in 'oapvm_write_mdcv' function to ensure correct order Signed-off-by: Dawid Kozinski <[email protected]>

dkozinski

commit 9108e20 Looks good

feat(encoder): Added support for mastering display color volume and c…

7e9d0f7

…ontent light level metadata Signed-off-by: Dawid Kozinski <[email protected]>

dkozinski requested a review from dariusz-f November 17, 2025 14:52

dkozinski added the wip Work in progress label Nov 17, 2025

dariusz-f requested a review from kpchoi November 18, 2025 10:45

Refactor: Move HDR metadata preparation out of library

2ad0759

- Responsibility for HDR metadata preparation moved from the library to the application side - Used the liboapv metadata container API to pass the prepared metadata to the library Signed-off-by: Dawid Kozinski <[email protected]>

dkozinski added 2 commits November 25, 2025 10:53

Add metadata serialization with network byte order support

5b79915

Signed-off-by: Dawid Kozinski <[email protected]>

kpchoi added 3 commits November 29, 2025 16:00

refactored code for clarificatin

e3114e9

Signed-off-by: [email protected] <[email protected]>

removed unused code

7254eeb

Signed-off-by: [email protected] <[email protected]>

implemented read function for MDCV and CLL

803e75e

Signed-off-by: [email protected] <[email protected]>

set the payload size

5a56d72

Signed-off-by: [email protected] <[email protected]>

dkozinski commented Dec 1, 2025

View reviewed changes

kpchoi added 2 commits December 2, 2025 11:34

fixed wrong constant value

b731485

Signed-off-by: [email protected] <[email protected]>

fixed bug

9108e20

Signed-off-by: [email protected] <[email protected]>

dkozinski commented Dec 2, 2025

View reviewed changes

dkozinski commented Dec 3, 2025

View reviewed changes

dkozinski removed the wip Work in progress label Dec 3, 2025

dariusz-f approved these changes Dec 3, 2025

View reviewed changes

kpchoi merged commit 720c3b7 into main Dec 4, 2025
7 checks passed

kpchoi mentioned this pull request Dec 4, 2025

New Feature: Add support for HDR master-display and max-cll to OAPV #167

Open

feat(encoder): Added support for mastering display color volume and c… #168

feat(encoder): Added support for mastering display color volume and c… #168

Conversation

dkozinski commented Nov 17, 2025

Uh oh!

kpchoi commented Nov 17, 2025

Uh oh!

dkozinski commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kpchoi commented Nov 18, 2025

Uh oh!

kpchoi commented Nov 19, 2025

Uh oh!

dkozinski commented Nov 20, 2025

Uh oh!

kpchoi commented Nov 21, 2025

Uh oh!

dkozinski commented Nov 25, 2025

Uh oh!

kpchoi commented Nov 28, 2025

Uh oh!

kpchoi commented Nov 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkozinski commented Nov 30, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkozinski Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kpchoi commented Dec 2, 2025

Uh oh!

dkozinski left a comment

Choose a reason for hiding this comment

Uh oh!

dkozinski Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kpchoi commented Dec 3, 2025

Uh oh!

dkozinski commented Nov 18, 2025 •

edited

Loading

kpchoi commented Nov 29, 2025 •

edited

Loading

dkozinski Dec 2, 2025 •

edited

Loading

dkozinski Dec 2, 2025 •

edited

Loading

dkozinski left a comment •

edited

Loading