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 RBSP issue, where 0x03 should be removed. v5.0.178 v6.0.75 #3597

Merged
merged 12 commits into from
Sep 9, 2023

Conversation

feixintianxia
Copy link
Contributor

@feixintianxia feixintianxia commented Jun 21, 2023

ISO_IEC_14496-10-AVC-2012.pdf, page 65
7.4.1.1 Encapsulation of an SODB within an RBSP (informative)

... 00 00 03 xx, the 03 byte should be drop where xx represents any 2 bit pattern: 00, 01, 10, or 11.


Co-authored-by: john hondaxiao@tencent.com
Co-authored-by: chundonglinlin chundonglinlin@163.com

@winlinvip winlinvip added the EnglishNative This issue is conveyed exclusively in English. label Jul 29, 2023
@xiaozhihong
Copy link
Collaborator

xiaozhihong commented Aug 8, 2023

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
Copy link
Contributor Author

feixintianxia commented Aug 19, 2023

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;
}

@winlinvip
Copy link
Member

winlinvip commented Aug 20, 2023

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

@winlinvip
Copy link
Member

winlinvip commented Aug 20, 2023

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

@xiaozhihong
Copy link
Collaborator

xiaozhihong commented Aug 21, 2023

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

@winlinvip winlinvip changed the title fix rbsp drop 0x03 Fix RBSP stream parsing bug, should drop 03. Sep 8, 2023
@winlinvip winlinvip changed the title Fix RBSP stream parsing bug, should drop 03. Fix RBSP stream parsing bug, should drop 0x03. Sep 8, 2023
@winlinvip winlinvip changed the title Fix RBSP stream parsing bug, should drop 0x03. Fix RBSP stream parsing bug, should drop 0x03. v5.0.178 v6.0.75 Sep 8, 2023
@winlinvip winlinvip added the RefinedByAI Refined by AI/GPT. label Sep 8, 2023
@winlinvip winlinvip changed the title Fix RBSP stream parsing bug, should drop 0x03. v5.0.178 v6.0.75 Fix the RBSP stream parsing issue, where 0x03 should be removed. v5.0.178 v6.0.75 Sep 8, 2023
@winlinvip winlinvip changed the title Fix the RBSP stream parsing issue, where 0x03 should be removed. v5.0.178 v6.0.75 Fix RBSP parsing issue, where 0x03 should be removed. v5.0.178 v6.0.75 Sep 8, 2023
@winlinvip winlinvip changed the title Fix RBSP parsing issue, where 0x03 should be removed. v5.0.178 v6.0.75 Fix RBSP issue, where 0x03 should be removed. v5.0.178 v6.0.75 Sep 8, 2023
@winlinvip
Copy link
Member

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

@winlinvip
Copy link
Member

winlinvip commented Sep 8, 2023

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

@winlinvip winlinvip changed the title Fix RBSP issue, where 0x03 should be removed. v5.0.178 v6.0.75 Fix RBSP issue, where 0x03 should be removed. Sep 8, 2023
@winlinvip winlinvip merged commit a671f1a into ossrs:develop Sep 9, 2023
17 checks passed
@winlinvip winlinvip changed the title Fix RBSP issue, where 0x03 should be removed. Fix RBSP issue, where 0x03 should be removed. v5.0.178 v6.0.75 Sep 9, 2023
winlinvip added a commit that referenced this pull request Sep 9, 2023
ISO_IEC_14496-10-AVC-2012.pdf, page 65
7.4.1.1 Encapsulation of an SODB within an RBSP (informative)

... 00 00 03 xx, the 03 byte should be drop where xx represents any 2
bit pattern: 00, 01, 10, or 11.

---------

Co-authored-by: john <hondaxiao@tencent.com>
Co-authored-by: chundonglinlin <chundonglinlin@163.com>
Co-authored-by: winlin <winlin@vip.126.com>
winlinvip added a commit that referenced this pull request Sep 9, 2023
ISO_IEC_14496-10-AVC-2012.pdf, page 65
7.4.1.1 Encapsulation of an SODB within an RBSP (informative)

... 00 00 03 xx, the 03 byte should be drop where xx represents any 2
bit pattern: 00, 01, 10, or 11.

---------

Co-authored-by: john <hondaxiao@tencent.com>
Co-authored-by: chundonglinlin <chundonglinlin@163.com>
Co-authored-by: winlin <winlin@vip.126.com>
duiniuluantanqin pushed a commit to duiniuluantanqin/srs that referenced this pull request Sep 14, 2023
ISO_IEC_14496-10-AVC-2012.pdf, page 65
7.4.1.1 Encapsulation of an SODB within an RBSP (informative)

... 00 00 03 xx, the 03 byte should be drop where xx represents any 2
bit pattern: 00, 01, 10, or 11.

---------

Co-authored-by: john <hondaxiao@tencent.com>
Co-authored-by: chundonglinlin <chundonglinlin@163.com>
Co-authored-by: winlin <winlin@vip.126.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EnglishNative This issue is conveyed exclusively in English. RefinedByAI Refined by AI/GPT.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants