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

fixed frame loss when time_jitter is off #2186

Closed
wants to merge 1 commit into from

Conversation

NodeBoy2
Copy link

@NodeBoy2 NodeBoy2 commented Feb 4, 2021

Description'

Please ensure that the markdown structure is maintained.

  1. SRS Version: 3.0release branch
  2. The log for SRS is as follows:
    Please ensure that the markdown structure is maintained.
shrinking, size=2, removed=2, max=30000ms
dispatch cached gop success. count=49, duration=-1
create consumer, active=1, queue_size=0.00, jitter=30000000
  1. The configuration for SRS is as follows:
    Please ensure that the markdown structure is maintained.
vhost __defaultVhost__ {
    time_jitter     off;
}

Replay
Please ensure that the markdown structure is maintained.

How to replay bug?
Please ensure that the markdown structure is maintained.

How to replay bug?

  1. Configure time_jitter to off.
  2. Start streaming.
  3. After 30 seconds, use ffplay to play the stream. There is a noticeable audio-video desynchronization in the first 2 seconds, with the video playing slower.

Please ensure that the markdown structure is maintained.

Problem

  1. When there is a consumer, SRS will send: metadata -> audio sequence header -> video sequence header -> GOP cache -> ...
  2. The timestamp of the sequence header is usually 0. After disabling time calibration, when putting the GOP cache into SrsMessageQueue, if the duration in the queue has already exceeded 30 seconds, the first frame (usually a video keyframe) will be discarded.

Please ensure that the markdown structure is maintained.

Fix
When calculating the accumulated duration of data in SrsMessageQueue, investigate the sequence header.


TRANS_BY_GPT3

@codecov-io
Copy link

codecov-io commented Feb 4, 2021

Codecov Report

Merging #2186 (b88ff48) into 3.0release (4f01340) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           3.0release    #2186   +/-   ##
===========================================
  Coverage       52.91%   52.91%           
===========================================
  Files              82       82           
  Lines           26693    26693           
===========================================
  Hits            14125    14125           
  Misses          12568    12568           

| Impacted Files | Coverage Δ | |'

'| Impacted Files | Coverage Δ | |
|---|---|---|
| trunk/src/app/srs_app_source.cpp | 0.67% <0.00%> (ø) | |
| trunk/src/service/srs_service_utility.cpp | 77.84% <0.00%> (ø) | |


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f01340...b88ff48. Read the comment docs.

TRANS_BY_GPT3

@winlinvip winlinvip force-pushed the 3.0release branch 28 times, most recently from 3c0fc9b to ccd8c86 Compare August 15, 2021 09:08
@winlinvip winlinvip force-pushed the 3.0release branch 3 times, most recently from 1ca9308 to 8d44b98 Compare August 15, 2021 09:22
@winlinvip winlinvip self-requested a review August 27, 2021 00:11
@winlinvip winlinvip self-assigned this Aug 27, 2021
@winlinvip winlinvip modified the milestone: SRS 4.0 release Aug 27, 2021
@winlinvip
Copy link
Member

@NodeBoy2 Thanks a lot 👍

@winlinvip
Copy link
Member

winlinvip commented Oct 27, 2021

After streaming for a certain period of time, there will be this log when playing:

[2021-10-28 07:22:09.511][Trace][91550][0eq5u805] shrinking, size=2, removed=2, max=30000ms
[2021-10-28 07:22:09.511][Trace][91550][0eq5u805] dispatch cached gop success. count=458, duration=-1

Capturing packets, it was found that there is a SequenceHeader.

image

After reviewing the code, if the data that is shrunk contains a SequenceHeader, it will be retrieved again.

void SrsMessageQueue::shrink()
{
    .............

    // update av_start_time
    av_start_time = av_end_time;
    //push_back secquence header and update timestamp
    if (video_sh) {
        video_sh->timestamp = srsu2ms(av_end_time);
        msgs.push_back(video_sh);
    }
    if (audio_sh) {
        audio_sh->timestamp = srsu2ms(av_end_time);
        msgs.push_back(audio_sh);
    }
    
    if (!_ignore_shrink) {
        srs_trace("shrinking, size=%d, removed=%d, max=%dms", (int)msgs.size(), msgs_size - (int)msgs.size(), srsu2msi(max_queue_size));
    }

The SequenceHeader will be preserved, but the keyframes will indeed be discarded. So in the image above, the first one is the SequenceHeader, and the second one is a P-frame.

The simplest solution is to just check if it is 0, because if other packets are also 0 but not a sequence header, it will still cause issues.

Of course, the best approach is to disable shrink during playback.

TRANS_BY_GPT3

@winlinvip
Copy link
Member

Fixed by ed1c499

@winlinvip winlinvip closed this Oct 27, 2021
@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.

3 participants