Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in IAB channel code mapping #2116

Merged
merged 1 commit into from
Sep 30, 2024
Merged

Conversation

cjee21
Copy link
Contributor

@cjee21 cjee21 commented Sep 30, 2024

As mentioned at the end of #2105 (comment), 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.

if (Code>=0x80 && Code<sizeof(Iab_Channel_Values)/sizeof(const char*)-0x18)
return Iab_Channel_Values[Code-0x18];

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

#include <string>
#include <iostream>
#include <cassert>

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
Copy link
Member

Right!

@JeromeMartinez JeromeMartinez merged commit f24a17b into MediaArea:master Sep 30, 2024
9 checks passed
@cjee21 cjee21 deleted the IAB branch September 30, 2024 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants