Thank you for the PR. This PR is an enhancement PR.
I'm not sure if we need to handle preventing competition codes in this way, because only 0x00 0x00 0x00, 0x00 0x00 0x01, 0x00, 0x00, 0x02, 0x00, 0x00, 0x03 will add competition codes. So your condition if (tmp > 3) { may always be true.

TRANS_BY_GPT3

Thank you for your reply. I referred to the following code.

https://github.com/mirror/x264/blob/master/common/bitstream.c

static uint8_t *nal_escape_c( uint8_t *dst, uint8_t *src, uint8_t *end )
{
    if( src < end ) *dst++ = *src++;
    if( src < end ) *dst++ = *src++;
    while( src < end )
    {
        if( src[0] <= 0x03 && !dst[-2] && !dst[-1] )      // this 
            *dst++ = 0x03;
        *dst++ = *src++;
    }
    return dst;
}

There indeed is this logic, that is, if the original data in the video is 00 00 01, this would conflict with the prefix of annexb, so it should be specially encoded.

TRANS_BY_GPT4

Please add the corresponding utest cases. For the various scenarios mentioned in the specifications, test cases need to be added for verification and testing.

TRANS_BY_GPT4

Thank you for the PR. This PR is an enhancement PR. I'm not sure if we need to handle preventing competition codes in this way, because only 0x00 0x00 0x00, 0x00 0x00 0x01, 0x00, 0x00, 0x02, 0x00, 0x00, 0x03 will add competition codes. So your condition if (tmp > 3) { may always be true.

TRANS_BY_GPT3

@feixintianxia I revisited the code logic, it seems I misunderstood it before, this modification should be fine. Could you please help to add some unit tests? Thanks.

TRANS_BY_GPT4

Nice work! Thank you all @feixintianxia @xiaozhihong @chundonglinlin ! 👍

I added some utests and found that the boundary conditions were not handled correctly, the following case will fail:

VOID TEST(KernelCodecTest, VideoFormatRbspEdge)
{
    if (true) {
        vector<uint8_t> nalu = {0x00, 0x00, 0x03};
        vector<uint8_t> expect = {0x00, 0x00, 0x03};

        vector<uint8_t> rbsp(nalu.size());
        SrsBuffer b((char*)nalu.data(), nalu.size());
        int nb_rbsp = srs_rbsp_remove_emulation_bytes(&b, rbsp);

        ASSERT_EQ(nb_rbsp, (int)expect.size());
        EXPECT_TRUE(srs_bytes_equals(rbsp.data(), expect.data(), nb_rbsp));
    }

Fixed by 5e6784c

TRANS_BY_GPT4

Thank you for your reply. I referred to the following code.

https://github.com/mirror/x264/blob/master/common/bitstream.c

static uint8_t *nal_escape_c( uint8_t *dst, uint8_t *src, uint8_t *end )
{
    if( src < end ) *dst++ = *src++;
    if( src < end ) *dst++ = *src++;
    while( src < end )
    {
        if( src[0] <= 0x03 && !dst[-2] && !dst[-1] )      // this 
            *dst++ = 0x03;
        *dst++ = *src++;
    }
    return dst;
}

I came across the code this pr made changes on, and I don't think it is accurate. Specifically, according to the x264 code referred here, wouldn't the emulated bytes of rbsp 0x00 0x00 0x03 0x04 be 0x00 0x00 0x03 0x03 0x04? which makes the table from the code comment incorrect:

// |---------------------|----------------------------|
// |      rbsp           |  nalu with emulation bytes |
// |---------------------|----------------------------|
// | 0x00 0x00 0x00      |     0x00 0x00 0x03 0x00    |
// | 0x00 0x00 0x01      |     0x00 0x00 0x03 0x01    |
// | 0x00 0x00 0x02      |     0x00 0x00 0x03 0x02    |
// | 0x00 0x00 0x03      |     0x00 0x00 0x03 0x03    |
// | 0x00 0x00 0x03 0x04 |     0x00 0x00 0x03 0x04    |
// |---------------------|----------------------------|

should be:

// |---------------------|---------------------------------|
// |      rbsp           |  nalu with emulation bytes      |
// |---------------------|---------------------------------|
// | 0x00 0x00 0x00      |     0x00 0x00 0x03 0x00         |
// | 0x00 0x00 0x01      |     0x00 0x00 0x03 0x01         |
// | 0x00 0x00 0x02      |     0x00 0x00 0x03 0x02         |
// | 0x00 0x00 0x03      |     0x00 0x00 0x03 0x03         |
// | 0x00 0x00 0x03 0x04 |     0x00 0x00 0x03 0x03 0x04    |
//                                              ^
// |---------------------|---------------------------------|

and if I understand it right, the emulation removing subroutine would also be incorrect regarding bytes encoded by the x264 encoder ?

cc @feixintianxia @xiaozhihong @winlinvip @chundonglinlin @gaod