JPEG XS essence support #48

nishabfhg · 2023-11-10T16:22:54Z

Hi,

Here are my changes to include JPEG XS essence support into bmx. It works pretty much like JPEG 2000. Only PARSE_FRAME_START_SIZE has to be larger in RawEssenceReader. Let me know if I need to modify something.

Thank you.
Best regards,
Nisha

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

philipnbbc · 2023-11-13T09:25:12Z

Thanks! I don't have time available today to review but will try make a start tomorrow or Wednesday. Is there a sample available somewhere online I can use to test?

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

nishabfhg · 2023-11-14T15:33:04Z

Great! I don't know if there are any jxs samples available online. I have added a short RGB test sequence here: https://github.com/nishabfhg/bmx-jxs/tree/main/Blender_FullHD_Testsequence. In raw2bmx, you will need the jxs_rgba command line option in this case.

philipnbbc

I've only managed to review some of it thus far but it is looking good. I'm certain it hasn't been trivial to figure out where things need to go!

There are a couple comments inline. Beyond that you need to convert the tabs to (4) spaces :-) There are also some build warnings which I'll copy into a comment as I don't think you can see the build results in the GitHub Actions output.

I'll try and complete the review this week.

Blender_FullHD_Testsequence/Blender_Sequence_FullHD_1920x1080_8b_444_00000.jxs

apps/raw2bmx/raw2bmx.cpp

deps/libMXF/mxf/mxf_baseline_data_model.h

deps/libMXF/mxf/mxf_header_metadata.c

deps/libMXFpp/libMXF++/metadata/base/JPEGXSSubDescriptorBase.cpp

include/bmx/essence_parser/FilePatternEssenceSource.h

src/common/EssenceType.cpp

deps/libMXF/mxf/mxf_labels_and_keys.h

philipnbbc · 2023-11-20T10:16:47Z

Here is the list of warnings from the Linux build:

/home/runner/work/bmx/bmx/src/essence_parser/FilePatternEssenceSource.cpp: In member function ‘int64_t bmx::FilePatternEssenceSource::ReadFileSize(uint32_t)’:
/home/runner/work/bmx/bmx/src/essence_parser/FilePatternEssenceSource.cpp:208:14: warning: unused variable ‘res’ [-Wunused-variable]
  208 |         bool res = NextFile();
      |              ^~~

In file included from /home/runner/work/bmx/bmx/src/essence_parser/JXSEssenceParser.cpp:33:
/home/runner/work/bmx/bmx/src/essence_parser/JXSEssenceParser.cpp: In member function ‘virtual void bmx::JXSEssenceParser::ParseFrameInfo(const unsigned char*, uint32_t)’:
/home/runner/work/bmx/bmx/include/bmx/essence_parser/JXSEssenceParser.h:49:34: warning: comparison of unsigned expression in ‘< 0’ is always false [-Wtype-limits]
   49 |         #define SUCCESS(v) (((v) < 0) ? 0 : 1)
      |                              ~~~~^~~
/home/runner/work/bmx/bmx/src/essence_parser/JXSEssenceParser.cpp:144:29: note: in expansion of macro ‘SUCCESS’
  144 |         while (p < end_p && SUCCESS(result))
      |                             ^~~~~~~
/home/runner/work/bmx/bmx/include/bmx/essence_parser/JXSEssenceParser.h:50:34: warning: comparison of unsigned expression in ‘< 0’ is always false [-Wtype-limits]
   50 |         #define FAILURE(v) (((v) < 0) ? 1 : 0)
      |                              ~~~~^~~
/home/runner/work/bmx/bmx/src/essence_parser/JXSEssenceParser.cpp:148:21: note: in expansion of macro ‘FAILURE’
  148 |                 if (FAILURE(result))
      |                     ^~~~~~~
/home/runner/work/bmx/bmx/src/essence_parser/JXSEssenceParser.cpp:211:66: warning: comparison is always false due to limited range of data type [-Wtype-limits]
  211 |                                 if (PIH_.Hsl() < 1 || PIH_.Hsl() > 0xffff) // This includes the EOF check
      |                                                       ~~~~~~~~~~~^~~~~~~~
/home/runner/work/bmx/bmx/src/essence_parser/JXSEssenceParser.cpp:154:24: warning: enumeration value ‘MRK_NIL’ not handled in switch [-Wswitch]
  154 |                 switch (NextMarker.m_Type)
      |                        ^
/home/runner/work/bmx/bmx/src/essence_parser/JXSEssenceParser.cpp:154:24: warning: enumeration value ‘MRK_WGT’ not handled in switch [-Wswitch]
/home/runner/work/bmx/bmx/src/essence_parser/JXSEssenceParser.cpp:154:24: warning: enumeration value ‘MRK_COM’ not handled in switch [-Wswitch]
/home/runner/work/bmx/bmx/src/essence_parser/JXSEssenceParser.cpp:154:24: warning: enumeration value ‘MRK_NLT’ not handled in switch [-Wswitch]
/home/runner/work/bmx/bmx/src/essence_parser/JXSEssenceParser.cpp:154:24: warning: enumeration value ‘MRK_CWD’ not handled in switch [-Wswitch]
/home/runner/work/bmx/bmx/src/essence_parser/JXSEssenceParser.cpp:154:24: warning: enumeration value ‘MRK_CTS’ not handled in switch [-Wswitch]
/home/runner/work/bmx/bmx/src/essence_parser/JXSEssenceParser.cpp:154:24: warning: enumeration value ‘MRK_CRG’ not handled in switch [-Wswitch]
/home/runner/work/bmx/bmx/src/essence_parser/JXSEssenceParser.cpp:154:24: warning: enumeration value ‘MRK_CAP’ not handled in switch [-Wswitch]

/home/runner/work/bmx/bmx/apps/bmxtranswrap/bmxtranswrap.cpp: In function ‘int main(int, const char**)’:
/home/runner/work/bmx/bmx/apps/bmxtranswrap/bmxtranswrap.cpp:3967:20: warning: enumeration value ‘JPEGXS_CDCI’ not handled in switch [-Wswitch]
 3967 |             switch (output_essence_type)
      |                    ^
/home/runner/work/bmx/bmx/apps/bmxtranswrap/bmxtranswrap.cpp:3967:20: warning: enumeration value ‘JPEGXS_RGBA’ not handled in switch [-Wswitch]

philipnbbc · 2023-11-22T16:15:40Z

As promised, here is the process for creating regression tests for JXS.

More info on testing can be found here. This process assumes you only have an RGBA example. If you also have a CDCI example file then ignore the steps below that delete that particular part of the tests; you will have 4 test data files instead of 2.

Copy tests/jpeg2000 to tests/jpegxs
Remove the test inputs and data: .j2c .bin and .md5 files
Copy in file 00000 and 00003 from Blender_FullHD_Testsequence/ (i.e. leave a gap which will be filled in using the --fill-pattern-gaps option in the tests)
Modify tests/CMakeLists.txt
- Add a add_subdirectory(jpegxs) after the jpeg2000 inclusion
Modify .gitattributes
- to have only 2 lines for _1 and _2. These test data files will be generated later
Modify tests/jpegxs/CMakeLists.txt
- Remove cdci from the tests list
- Replace jpeg2000 with jpegxs
Modify tests/jpegxs/test_rgba.cmake
- Change the _3 and _4 in names to _1 and _2
- Change the --j2c_rgba arg to --jxs_rgba "${TEST_SOURCE_DIR}/Blender_Sequence_FullHD_1920x1080_8b_444_%d.jxs"
- Remove --aspect-ratio and --frame-layout (which will be parsed from the bitstream) and other args like --transfer-ch as required that don't match the content
Rebuild using cmake --build .
Generate sample files for the test using cmake --build . --target bmx_jpegxs_rgba_data
- the files will be output to the test_samples/jpegxs/ sub-directory in the build directory
- Check that the sample files look ok. The .bin files are the XML output from mxf2raw and the .mxf files are generated by raw2bmx and bmxtranswrap.
Generate the test data (.bin and .md5) files using cmake --build . --target bmx_jpegxs_rgba_data
Run the tests using ctest to check the new JXS tests are run and pass
Remove the Blender_FullHD_Testsequence directory

philipnbbc

I've completed the review and further comments / suggestions can be found inline

apps/raw2bmx/raw2bmx.cpp

include/bmx/essence_parser/JXSEssenceParser.h

src/essence_parser/JXSEssenceParser.cpp

src/writer_helper/JPEGXSWriterHelper.cpp

src/mxf_helper/JPEGXSMXFDescriptorHelper.cpp

nishabfhg · 2023-11-24T12:19:11Z

Hi Philip, thanks for the review! I will try to make all the modifications next week and get back to you.

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

nishabfhg · 2023-12-12T17:00:26Z

Hi Philip,
I have modified the code according to your review points. I also added a regression test for jpegxs cdci. Let me know if there's anything else. (I will only be able to get back to this after the New Year holidays)

philipnbbc · 2023-12-14T12:06:11Z

I've had a quick look through and looks like you covered everything, thanks for the updates! I'll probably take a proper look sometime next week as I'm away from work.

philipnbbc

Thanks for your update!. There are a couple remaining issues to look at (most of the comments are whitespace related).

There are also a couple of merge conflicts that have arisen because of recent updates to main.

The test data is likely to change and you can run cmake --build . --target bmx_jpegxs_rgba_data and cmake --build . --target bmx_jpegxs_cdci_data to update them.

src/essence_parser/JXSEssenceParser.cpp

src/mxf_helper/JPEGXSMXFDescriptorHelper.cpp

apps/raw2bmx/raw2bmx.cpp

apps/bmxtranswrap/bmxtranswrap.cpp

src/essence_parser/JXSEssenceParser.cpp

src/mxf_helper/JPEGXSMXFDescriptorHelper.cpp

nishabfhg · 2024-01-12T16:36:24Z

Hi Philip. I made the fixes. I hope I didn't miss anything. I synced my version with your latest commits, so the merge conflicts should not appear anymore.

philipnbbc · 2024-01-15T12:22:41Z

All done now except for some unexpected changes to some non-jxs test checksums - see below the diff showing what needs to be changed. Do you know what caused this? Just asking as it might be an issue that needs fixing.

If you run cmake --build . -t bmx_test_data and cmake --build . -t libMXF_test_data it should regenerate the checksums as bbelow. cmake --build . -t test should then pass.

Diff showing what needs to be changed back:

diff --git a/deps/libMXF/test/test_indextable.md5 b/deps/libMXF/test/test_indextable.md5
index e781305e..6dc5d47a 100644
--- a/deps/libMXF/test/test_indextable.md5
+++ b/deps/libMXF/test/test_indextable.md5
@@ -1 +1 @@
-d41d8cd98f00b204e9800998ecf8427e
\ No newline at end of file
+5ff6163c7004300edb5e0c6670e547d3
\ No newline at end of file
diff --git a/test/as02/iecdv25.md5s b/test/as02/iecdv25.md5s
index ec6da267..8e831eb3 100644
--- a/test/as02/iecdv25.md5s
+++ b/test/as02/iecdv25.md5s
@@ -1 +1 @@
-abce8e563ec3408a0dbf10ae34dfe30c;ca114c9f2707afde489d6e6066f8833c;e4a2bdff4418802e3d127654ee906dc6;99538bb806a99596b95236ac8a965b52
\ No newline at end of file
+227a7e8a6b34571cfb3a9b70642c0183;4e4605266cb9d916830ea1eef722321d;d8b021e1b8bc3bd3796676b0b5e8f594;16ff89dda611b560eda0739fd90d0214
\ No newline at end of file
diff --git a/test/avid_mxf/mpeg2lg_422p_hl_1080i.md5s b/test/avid_mxf/mpeg2lg_422p_hl_1080i.md5s
index 606895d7..8e1e6d3e 100644
--- a/test/avid_mxf/mpeg2lg_422p_hl_1080i.md5s
+++ b/test/avid_mxf/mpeg2lg_422p_hl_1080i.md5s
@@ -1 +1 @@
-f95eae8d685f5c222c7e3e9fb84b9f9a;419a18c68d4b87dfb10408653a6bc58e;805d6fd84c1ed39585053c49016f52f7
\ No newline at end of file
+f189845c35fbe0a69d36b181d85937f9;0ece8877ab256657df705de6a01a8a48;f323a74f1505abc8bf201eeef07e0089
\ No newline at end of file
diff --git a/test/avid_mxf/mpeg2lg_mp_hl_1920_1080i.md5s b/test/avid_mxf/mpeg2lg_mp_hl_1920_1080i.md5s
index 2b2e3f30..793a0a50 100644
--- a/test/avid_mxf/mpeg2lg_mp_hl_1920_1080i.md5s
+++ b/test/avid_mxf/mpeg2lg_mp_hl_1920_1080i.md5s
@@ -1 +1 @@
-93d9e5755cdcdd13000b71bcad7d55d1;a31f0491c9255156d07ec78ac265311c;f3259501cd9d48ce94f5f1aa0006d697
\ No newline at end of file
+fbd8e5365fbd58c4a4908cd2e78319f7;0ece8877ab256657df705de6a01a8a48;f323a74f1505abc8bf201eeef07e0089
\ No newline at end of file
diff --git a/test/d10_mxf/d10_40_2997.md5 b/test/d10_mxf/d10_40_2997.md5
index 20d6158e..8563ad8d 100644
--- a/test/d10_mxf/d10_40_2997.md5
+++ b/test/d10_mxf/d10_40_2997.md5
@@ -1 +1 @@
-8a256f169a5cbaa6145f95a9d008df0a
\ No newline at end of file
+6e62f6adec11065622203c7e75cc2b38
\ No newline at end of file

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

nishabfhg · 2024-01-15T13:57:08Z

The md5s might have changed when I rebuilt the project using the visual studio solution. I am not sure why this happened.

philipnbbc

All done now as the build succeeds, Thanks for your contribution!

nishabfhg · 2024-01-16T12:52:00Z

That's great! Thanks for the review.

JPEG XS essence support

3077a7e

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

philipnbbc self-requested a review November 13, 2023 09:20

RGB test sequence

9600c72

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

philipnbbc reviewed Nov 20, 2023

View reviewed changes

philipnbbc reviewed Nov 23, 2023

View reviewed changes

Modifications based on the review; Regression tests added

5e6aced

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

philipnbbc reviewed Jan 2, 2024

View reviewed changes

Merged from upstream

765c351

Reverted md5s

20f3bcc

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

philipnbbc self-requested a review January 15, 2024 15:22

philipnbbc approved these changes Jan 15, 2024

View reviewed changes

philipnbbc merged commit c29bc42 into bbc:main Jan 15, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JPEG XS essence support #48

JPEG XS essence support #48

nishabfhg commented Nov 10, 2023

philipnbbc commented Nov 13, 2023

nishabfhg commented Nov 14, 2023

philipnbbc left a comment

philipnbbc commented Nov 20, 2023

philipnbbc commented Nov 22, 2023

philipnbbc left a comment

nishabfhg commented Nov 24, 2023

nishabfhg commented Dec 12, 2023

philipnbbc commented Dec 14, 2023

philipnbbc left a comment

nishabfhg commented Jan 12, 2024

philipnbbc commented Jan 15, 2024 •

edited

Loading

nishabfhg commented Jan 15, 2024 •

edited

Loading

philipnbbc left a comment

nishabfhg commented Jan 16, 2024

JPEG XS essence support #48

JPEG XS essence support #48

Conversation

nishabfhg commented Nov 10, 2023

philipnbbc commented Nov 13, 2023

nishabfhg commented Nov 14, 2023

philipnbbc left a comment

Choose a reason for hiding this comment

philipnbbc commented Nov 20, 2023

philipnbbc commented Nov 22, 2023

philipnbbc left a comment

Choose a reason for hiding this comment

nishabfhg commented Nov 24, 2023

nishabfhg commented Dec 12, 2023

philipnbbc commented Dec 14, 2023

philipnbbc left a comment

Choose a reason for hiding this comment

nishabfhg commented Jan 12, 2024

philipnbbc commented Jan 15, 2024 • edited Loading

nishabfhg commented Jan 15, 2024 • edited Loading

philipnbbc left a comment

Choose a reason for hiding this comment

nishabfhg commented Jan 16, 2024

philipnbbc commented Jan 15, 2024 •

edited

Loading

nishabfhg commented Jan 15, 2024 •

edited

Loading