srs icon indicating copy to clipboard operation
srs copied to clipboard

WebRTC: Should put lost packet in receive queue

Open crudbot opened this issue 2 years ago • 1 comments

Describe the bug

srs_error_t SrsRtcRecvTrack::on_nack(SrsRtpPacket** ppkt)
{
    srs_error_t err = srs_success;

    SrsRtpPacket* pkt = *ppkt;
    uint16_t seq = pkt->header.get_sequence();
    SrsRtpNackInfo* nack_info = nack_receiver_->find(seq);
    if (nack_info) {
        // seq had been received.
        nack_receiver_->remove(seq);
        return err;
    }

    // insert check nack list
    uint16_t nack_first = 0, nack_last = 0;
    if (!rtp_queue_->update(seq, nack_first, nack_last)) {
        srs_warn("NACK: too old seq %u, range [%u, %u]", seq, rtp_queue_->begin, rtp_queue_->end);
    }
    if (srs_rtp_seq_distance(nack_first, nack_last) > 0) {
        srs_trace("NACK: update seq=%u, nack range [%u, %u]", seq, nack_first, nack_last);
        nack_receiver_->insert(nack_first, nack_last);
        nack_receiver_->check_queue_size();
    }

    // insert into video_queue and audio_queue
    // We directly use the pkt, never copy it, so we should set the pkt to NULL.
    if (nack_no_copy_) {
        rtp_queue_->set(seq, pkt);
        *ppkt = NULL;
    } else {
        rtp_queue_->set(seq, pkt->copy());
    }

    return err;
}

Version dev 5.0release Expected behavior when we receive a packet in nack status, we should also put it into rtp_queue_ maybe as follows

srs_error_t SrsRtcRecvTrack::on_nack(SrsRtpPacket** ppkt)
{
    srs_error_t err = srs_success;

    SrsRtpPacket* pkt = *ppkt;
    uint16_t seq = pkt->header.get_sequence();

do {
    SrsRtpNackInfo* nack_info = nack_receiver_->find(seq);

    if (nack_info) {
        // seq had been received.
        nack_receiver_->remove(seq);
        break;
    }
    // insert check nack list
    uint16_t nack_first = 0, nack_last = 0;
    if (!rtp_queue_->update(seq, nack_first, nack_last)) {
        srs_warn("NACK: too old seq %u, range [%u, %u]", seq, rtp_queue_->begin, rtp_queue_->end);
    }
    if (srs_rtp_seq_distance(nack_first, nack_last) > 0) {
        srs_trace("NACK: update seq=%u, nack range [%u, %u]", seq, nack_first, nack_last);
        nack_receiver_->insert(nack_first, nack_last);
        nack_receiver_->check_queue_size();
    }
} while(0);

    // insert into video_queue and audio_queue
    // We directly use the pkt, never copy it, so we should set the pkt to NULL.
    if (nack_no_copy_) {
        rtp_queue_->set(seq, pkt);
        *ppkt = NULL;
    } else {
        rtp_queue_->set(seq, pkt->copy());
    }

    return err;
}

crudbot avatar Sep 30 '23 01:09 crudbot

Welcome anyone to submit a pullrequest directly, noting to cover the changes by utest and describing the detail backgroud.

winlinvip avatar Mar 26 '24 00:03 winlinvip