MediaArea / MediaInfoLib

Convenient unified display of the most relevant technical and tag data for video and audio files.
https://mediaarea.net/MediaInfo
BSD 2-Clause "Simplified" License
636 stars 176 forks source link

Fix bug in IAB channel code mapping #2116

Closed cjee21 closed 1 month ago

cjee21 commented 1 month ago

As mentioned at the end of https://github.com/MediaArea/MediaInfoLib/issues/2105#issuecomment-2381399096, Cppcheck found an array access that is out of bounds:

Id: arrayIndexOutOfBoundsCond
CWE: 788
Either the condition 'Code>=0x80' is redundant or the array 'Iab_Channel_Values[34]' is accessed at index 104, which is out of bounds.

https://github.com/MediaArea/MediaInfoLib/blob/dbed0279c05911f50f9ad715d187666e38f8764c/Source/MediaInfo/Audio/File_Iab.cpp#L117-L118

Upon further inspection and looking at the IAB specs, I concluded that there is indeed a bug. Since I do not have an IAB test file to test, I wrote a test based on pages 31 and 32 of https://pub.smpte.org/doc/st2098-2/20220525-pub/st2098-2-2022.pdf to test it and confirmed that this PR is needed. The fixed version is also validated with Visual Studio Code Analysis and Cppcheck.

Test Code:

Test.cpp

```c++ #include #include #include typedef unsigned int int32u; const char* Iab_Channel(int32u Code) { static const char* Iab_Channel_Values[] = { "L", "Lc", "C", "Rc", "R", "Lss", "Ls", "Lb", "Rb", "Rss", "Rs", "Tsl", "Tsr", "LFE", "Left Height", "Right Height", "Center Height", "Left Surround Height", "Right Surround Height", "Left Side Surround Height", "Right Side Surround Height", "Left Rear Surround Height", "Right Rear Surround Height", "Tc", //0x18-0x7F reserved "Tfl", "Tfr", "Tbl", "Tbr", "Tsl", "Tsr", "LFE1", "LFE2", "Lw", "Rw", }; if (Code < 0x18) return Iab_Channel_Values[Code]; if (Code >= 0x80 && Code < sizeof(Iab_Channel_Values) / sizeof(const char*) + 0x68) return Iab_Channel_Values[Code - 0x68]; return ""; } const char* Iab_Channel_OLD(int32u Code) { static const char* Iab_Channel_Values[] = { "L", "Lc", "C", "Rc", "R", "Lss", "Ls", "Lb", "Rb", "Rss", "Rs", "Tsl", "Tsr", "LFE", "Left Height", "Right Height", "Center Height", "Left Surround Height", "Right Surround Height", "Left Side Surround Height", "Right Side Surround Height", "Left Rear Surround Height", "Right Rear Surround Height", "Tc", //0x18-0x7F reserved "Tfl", "Tfr", "Tbl", "Tbr", "Tsl", "Tsr", "LFE1", "LFE2", "Lw", "Rw", }; if (Code < 0x18) return Iab_Channel_Values[Code]; if (Code >= 0x80 && Code < sizeof(Iab_Channel_Values) / sizeof(const char*) - 0x18) return Iab_Channel_Values[Code - 0x18]; return ""; } struct ChannelIDMap { unsigned int ChannelID; const char* Channel_Name; }; int main() { // ChannelID and DestinationChannelID Codes // From Pages 31 and 32 of SMPTE ST 2098-2:2022 ChannelIDMap Iab_Channel_Values_Map[] = { //0x0-0x17 SMPTE ST 428-12:2013 and SMPTE ST 2098-5:2018 Channel Name {0x00, "L"}, {0x01, "Lc"}, {0x02, "C"}, {0x03, "Rc"}, {0x04, "R"}, {0x05, "Lss"}, {0x06, "Ls"}, {0x07, "Lb"}, {0x08, "Rb"}, {0x09, "Rss"}, {0x0A, "Rs"}, {0x0B, "Tsl"}, {0x0C, "Tsr"}, {0x0D, "LFE"}, {0x0E, "Left Height"}, {0x0F, "Right Height"}, {0x10, "Center Height"}, {0x11, "Left Surround Height"}, {0x12, "Right Surround Height"}, {0x13, "Left Side Surround Height"}, {0x14, "Right Side Surround Height"}, {0x15, "Left Rear Surround Height"}, {0x16, "Right Rear Surround Height"}, {0x17, "Tc"}, //0x18-0x7F Reserved for D-Cinema //0x80-0x89 ITU-R BS.2051-2 Channel Name / System {0x80, "Tfl" }, {0x81, "Tfr"}, {0x82, "Tbl"}, {0x83, "Tbr"}, {0x84, "Tsl"}, {0x85, "Tsr"}, {0x86, "LFE1"}, {0x87, "LFE2"}, {0x88, "Lw"}, {0x89, "Rw"} }; // Test fixed Iab_Channel_Values_Map() std::cout << "\nTesting Iab_Channel_Values_Map()\n"; for (unsigned int i = 0x0; i < sizeof(Iab_Channel_Values_Map) / sizeof(ChannelIDMap); ++i) { std::cout << "Testing " << std::hex << Iab_Channel_Values_Map[i].ChannelID << " - " << Iab_Channel_Values_Map[i].Channel_Name << " "; assert(Iab_Channel(Iab_Channel_Values_Map[i].ChannelID) == Iab_Channel_Values_Map[i].Channel_Name); std::cout << "PASS\n"; } // Test previous Iab_Channel_Values_Map() std::cout << "\nTesting OLD Iab_Channel_Values_Map()\n"; for (unsigned int i = 0x0; i < sizeof(Iab_Channel_Values_Map) / sizeof(ChannelIDMap); ++i) { std::cout << "Testing 0x" << std::hex << Iab_Channel_Values_Map[i].ChannelID << " - " << Iab_Channel_Values_Map[i].Channel_Name << " "; assert(Iab_Channel_OLD(Iab_Channel_Values_Map[i].ChannelID) == Iab_Channel_Values_Map[i].Channel_Name); std::cout << "PASS\n"; } return 0; } ```

Results:

Test Output

``` Testing Iab_Channel_Values_Map() Testing 0x0 - L PASS Testing 0x1 - Lc PASS Testing 0x2 - C PASS Testing 0x3 - Rc PASS Testing 0x4 - R PASS Testing 0x5 - Lss PASS Testing 0x6 - Ls PASS Testing 0x7 - Lb PASS Testing 0x8 - Rb PASS Testing 0x9 - Rss PASS Testing 0xa - Rs PASS Testing 0xb - Tsl PASS Testing 0xc - Tsr PASS Testing 0xd - LFE PASS Testing 0xe - Left Height PASS Testing 0xf - Right Height PASS Testing 0x10 - Center Height PASS Testing 0x11 - Left Surround Height PASS Testing 0x12 - Right Surround Height PASS Testing 0x13 - Left Side Surround Height PASS Testing 0x14 - Right Side Surround Height PASS Testing 0x15 - Left Rear Surround Height PASS Testing 0x16 - Right Rear Surround Height PASS Testing 0x17 - Tc PASS Testing 0x80 - Tfl PASS Testing 0x81 - Tfr PASS Testing 0x82 - Tbl PASS Testing 0x83 - Tbr PASS Testing 0x84 - Tsl PASS Testing 0x85 - Tsr PASS Testing 0x86 - LFE1 PASS Testing 0x87 - LFE2 PASS Testing 0x88 - Lw PASS Testing 0x89 - Rw PASS Testing OLD Iab_Channel_Values_Map() Testing 0x0 - L PASS Testing 0x1 - Lc PASS Testing 0x2 - C PASS Testing 0x3 - Rc PASS Testing 0x4 - R PASS Testing 0x5 - Lss PASS Testing 0x6 - Ls PASS Testing 0x7 - Lb PASS Testing 0x8 - Rb PASS Testing 0x9 - Rss PASS Testing 0xa - Rs PASS Testing 0xb - Tsl PASS Testing 0xc - Tsr PASS Testing 0xd - LFE PASS Testing 0xe - Left Height PASS Testing 0xf - Right Height PASS Testing 0x10 - Center Height PASS Testing 0x11 - Left Surround Height PASS Testing 0x12 - Right Surround Height PASS Testing 0x13 - Left Side Surround Height PASS Testing 0x14 - Right Side Surround Height PASS Testing 0x15 - Left Rear Surround Height PASS Testing 0x16 - Right Rear Surround Height PASS Testing 0x17 - Tc PASS Testing 0x80 - Tfl Assertion failed: Iab_Channel_OLD(Iab_Channel_Values_Map[i].ChannelID) == Iab_Channel_Values_Map[i].Channel_Name, file Test.cpp, line 165 ```

JeromeMartinez commented 1 month ago

Right!