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

SRT: Fix bug for multiple NALUs, when configure OBS in zerolatency. #2440

Merged
merged 11 commits into from
Jun 29, 2021
Merged

SRT: Fix bug for multiple NALUs, when configure OBS in zerolatency. #2440

merged 11 commits into from
Jun 29, 2021

Conversation

runner365
Copy link
Contributor

issue: #2390
the bug happen when obs push srt stream and configue h264 in zerolatency.

@runner365 runner365 linked an issue Jun 25, 2021 that may be closed by this pull request
srs_info("mpegts: demux avc ibp frame size=%d, dts=%d", frame_size, dts);
if ((err = write_h264_ipb_frame(frame, frame_size, dts, pts)) != srs_success) {
srs_info("mpegts: demux avc ibp frame size=%d, dts=%d", avs_ptr->left() + frame_size, dts);
if ((err = write_h264_ipb_frame(avs_ptr->head() - frame_size, avs_ptr->left() + frame_size, dts, pts)) != srs_success) {
Copy link
Collaborator

@xiaozhihong xiaozhihong Jun 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a mistake here, head() - frame_size is a wild pointer?

TRANS_BY_GPT3

Copy link
Collaborator

@xiaozhihong xiaozhihong Jun 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably understand.
Slice multiple frames and NALUs, previously sent one by one.
Now, after parsing the SPS/PPS, the rest is ignored and sent directly merged.

TRANS_BY_GPT3

@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2021

Codecov Report

Merging #2440 (d7d9a85) into develop (ce47d5c) will not change coverage.
The diff coverage is n/a.

❗ Current head d7d9a85 differs from pull request most recent head 48c9fc7. Consider uploading reports for the commit 48c9fc7 to get more accurate results
Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #2440   +/-   ##
========================================
  Coverage    42.77%   42.77%           
========================================
  Files          101      101           
  Lines        35944    35944           
========================================
  Hits         15376    15376           
  Misses       20568    20568           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data'

Translated to English:

Δ = absolute <relative> (impact), ø = not affected, ? = missing data'
Powered by Codecov. Last update ce47d5c...48c9fc7. Read the comment docs.

TRANS_BY_GPT3

@winlinvip winlinvip changed the title Multi nalu srt SRT: Fix bug for multiple NALUs, when configure OBS in zerolatency. Jun 29, 2021
@winlinvip winlinvip merged commit 346cc96 into ossrs:develop Jun 29, 2021
winlinvip pushed a commit that referenced this pull request Jun 29, 2021
…2440)

* solve srt push bugs

* solve h264 mutiple nalus in srt when obs is configured in zerolatency

* optimize error code

* optimize error code

* optimize error code

* add commemnt:we only skip pps/sps frame and send left nalus in srt

* add commemnt:we only skip pps/sps frame and send left nalus in srt

Co-authored-by: shiwei <shiwei05@kuaishou.com>
winlinvip added a commit that referenced this pull request Jun 29, 2021
@zhouxiaojun2008
Copy link
Contributor

zhouxiaojun2008 commented Jul 1, 2021

Shi Dashen, I have some doubts as follows:

This modification is based on the fact that the callback from ts outputs a complete frame of data. Therefore, here we directly send the entire frame of data at once. However, there may be a problem, for example, in the case of zerolatecy, where a frame may have multiple slices. The first slice is 00 00 00 01 xx, and the following slices are also 00 00 01 xx. If we send them all at once, the start code of the first slice is removed, but the start code of the following slices is not removed. This may pose a potential risk. Will there be any issues when this data is sent to SRS? Additionally, I will attach two images.
1
2

My modification is as follows:
`std::vectorstd::string vec_nalus;

// send each frame.
while (!avs_ptr->empty()) {
    char* frame = NULL;
    int frame_size = 0;
    if ((err = _avc_ptr->annexb_demux(avs_ptr, &frame, &frame_size)) != srs_success) {
        return srs_error_wrap(err, "demux annexb");
    }

    // 5bits, 7.3.1 NAL unit syntax,
    // ISO_IEC_14496-10-AVC-2003.pdf, page 44.
    //  7: SPS, 8: PPS, 5: I Frame, 1: P Frame
    SrsAvcNaluType nal_unit_type = (SrsAvcNaluType)(frame[0] & 0x1f);
    //srs_trace("frame size:%d,type:%d", frame_size, nal_unit_type);
    // ignore the nalu type sps(7), pps(8), aud(9)
    switch (nal_unit_type) {
        case SrsAvcNaluTypeSPS: {
            std::string sps;
            if ((err = _avc_ptr->sps_demux(frame, frame_size, sps)) != srs_success) {
                return srs_error_wrap(err, "demux sps");
            }

            if (_h264_sps == sps) {
                continue;
            }
            _h264_sps_changed = true;
            _h264_sps = sps;

            if ((err = write_h264_sps_pps(dts, pts)) != srs_success) {
                return srs_error_wrap(err, "write sps/pps");
            }
            continue;
        }
        case SrsAvcNaluTypePPS: {
            std::string pps;
            if ((err = _avc_ptr->pps_demux(frame, frame_size, pps)) != srs_success) {
                return srs_error_wrap(err, "demux pps");
            }

            if (_h264_pps == pps) {
                continue;
            }
            _h264_pps_changed = true;
            _h264_pps = pps;

            if ((err = write_h264_sps_pps(dts, pts)) != srs_success) {
                return srs_error_wrap(err, "write sps/pps");
            }
            continue;
        }
        case SrsAvcNaluTypeAccessUnitDelimiter:
        case SrsAvcNaluTypeSEI: {
            continue;
        }
        default: {
            // ibp frame

if (!avs_ptr->empty()) { // If it is not empty, it means it is a multi-slice
vec_nalus.push_back(std::string(frame, frame_size));
}
else {
if (vec_nalus.empty()) {
if ((err = write_h264_ipb_frame(frame, frame_size, dts, pts)) != srs_success)
return srs_error_wrap(err, "write h264 ipb frame");
}
else {
vec_nalus.push_back(std::string(frame, frame_size));
if ((err = write_h264_ipb_frame(vec_nalus, dts, pts)) != srs_success)
return srs_error_wrap(err, "write h264 ipb slice frame");
}
_last_live_ts = now_ms();
}
continue;
}
}
}`

TRANS_BY_GPT3

@winlinvip winlinvip added the TransByAI Translated by AI/GPT. label Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TransByAI Translated by AI/GPT.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SRT streaming: A situation where screen flickering always occurs
5 participants