Repository: ossrs/srs
File: trunk/src/app/srs_app_rtc_api.cpp (C++)

Code before:

    if ((err = check_remote_sdp(ruc.remote_sdp_)) != srs_success) {
        return srs_error_wrap(err, "remote sdp check failed");
    }

    SrsSdp local_sdp;

    // Config for SDP and session.
    local_sdp.session_config_.dtls_role = _srs_config->get_rtc_dtls_role(ruc.req_->vhost);
    local_sdp.session_config_.dtls_version = _srs_config->get_rtc_dtls_version(ruc.req_->vhost);

    // Whether enabled.
    bool server_enabled = _srs_config->get_rtc_server_enabled();
    bool rtc_enabled = _srs_config->get_rtc_enabled(ruc.req_->vhost);
    if (server_enabled && !rtc_enabled) {
        srs_warn("RTC disabled in vhost %s", ruc.req_->vhost.c_str());
    }
    if (!server_enabled || !rtc_enabled) {
        return srs_error_new(ERROR_RTC_DISABLED, "Disabled server=%d, rtc=%d, vhost=%s",
            server_enabled, rtc_enabled, ruc.req_->vhost.c_str());
    }

    // For RTMP to RTC, fail if disabled and RTMP is active, see https://github.com/ossrs/srs/issues/2728
    if (!_srs_config->get_rtc_from_rtmp(ruc.req_->vhost)) {
        // FIX ERR: when config is { rtmp_to_rtc off; rtc_to_rtmp on; }, playing rtc will be rejected if publishing rtc stream.
        if (!_srs_config->get_rtc_to_rtmp(ruc.req_->vhost)) {
            SrsLiveSource* rtmp = _srs_sources->fetch(ruc.req_);
            if (rtmp && !rtmp->inactive()) {
                return srs_error_new(ERROR_RTC_DISABLED, "Disabled rtmp_to_rtc of %s, see #2728", ruc.req_->vhost.c_str());
            }
        }
    }

    // TODO: FIXME: When server enabled, but vhost disabled, should report error.
    SrsRtcConnection* session = NULL;
    if ((err = server_->create_session(&ruc, local_sdp, &session)) != srs_success) {
        return srs_error_wrap(err, "create session, dtls=%u, srtp=%u, eip=%s", ruc.dtls_, ruc.srtp_, eip.c_str());
    }

    ostringstream os;
    if ((err = local_sdp.encode(os)) != srs_success) {
        return srs_error_wrap(err, "encode sdp");
    }

    string local_sdp_str = os.str();
    // Filter the \r\n to \\r\\n for JSON.
    string local_sdp_escaped = srs_string_replace(local_sdp_str.c_str(), "\r\n", "\\r\\n");

    res->set("code", SrsJsonAny::integer(ERROR_SUCCESS));
    res->set("server", SrsJsonAny::str(SrsStatistic::instance()->server_id().c_str()));


Review comment (bug):
This place is not just about checking the configuration, this logic is incorrect. For example, there will still be issues in the following situation:

1. Configure `rtmp2rtc off; rtc2rtmp on;`, which means not enabling RTMP to RTC conversion, but enabling RTC to RTMP conversion.
2. Push RTMP stream, but do not push RTC stream.
3. Play RTC stream, here it will display a black screen instead of prompting a configuration error.

The core issue here is how to detect this type of error.

1. Configure as `rtmp2rtc off`, which means not enabling stream conversion.
2. Push RTMP stream and play RTC stream. At this point, there won't be any RTC stream (no RTC stream pushed, no RTMP to RTC stream conversion). It must fail at this point, otherwise, further investigation is required to determine why there is a black screen.

The issue with the original logic is not caused by the configuration, but rather due to overlooking a special case: "If there is an RTC stream, there is no need to check the rtmp2rtc switch." Because if there is an RTC stream, it means there is already an RTC stream available, so there is no need to worry about any switches. The RTC stream should be playable in this case.

If there is an RTC stream, it doesn't mean that enabling `rtc2rtmp` implies the presence of an RTC stream. Instead, it should directly check whether there is an RTC stream being pushed and obtain the stream status from the RTC source.

It should be changed to this:

'It should be changed to this: 

```
bool is_rtc_stream_active = xxx; // Check if RTC has a stream
if (!is_rtc_stream_active && !_srs_config->get_rtc_from_rtmp(ruc.req_->vhost)) {
// Check if there is an RTMP stream and rtmp2rtc is disabled, in this case an error should be reported, the current logic is correct.
```

`TRANS_BY_GPT3`

Code after:

    if ((err = check_remote_sdp(ruc.remote_sdp_)) != srs_success) {
        return srs_error_wrap(err, "remote sdp check failed");
    }

    SrsSdp local_sdp;

    // Config for SDP and session.
    local_sdp.session_config_.dtls_role = _srs_config->get_rtc_dtls_role(ruc.req_->vhost);
    local_sdp.session_config_.dtls_version = _srs_config->get_rtc_dtls_version(ruc.req_->vhost);

    // Whether enabled.
    bool server_enabled = _srs_config->get_rtc_server_enabled();
    bool rtc_enabled = _srs_config->get_rtc_enabled(ruc.req_->vhost);
    if (server_enabled && !rtc_enabled) {
        srs_warn("RTC disabled in vhost %s", ruc.req_->vhost.c_str());
    }
    if (!server_enabled || !rtc_enabled) {
        return srs_error_new(ERROR_RTC_DISABLED, "Disabled server=%d, rtc=%d, vhost=%s",
            server_enabled, rtc_enabled, ruc.req_->vhost.c_str());
    }

    // Whether RTC stream is active.
    bool is_rtc_stream_active = false;
    if (true) {
        SrsRtcSource* source = _srs_rtc_sources->fetch(ruc.req_);
        is_rtc_stream_active = (source && !source->can_publish());
    }

    // For RTMP to RTC, fail if disabled and RTMP is active, see https://github.com/ossrs/srs/issues/2728
    if (!is_rtc_stream_active && !_srs_config->get_rtc_from_rtmp(ruc.req_->vhost)) {
        SrsLiveSource* rtmp = _srs_sources->fetch(ruc.req_);
        if (rtmp && !rtmp->inactive()) {
            return srs_error_new(ERROR_RTC_DISABLED, "Disabled rtmp_to_rtc of %s, see #2728", ruc.req_->vhost.c_str());
        }
    }

    // TODO: FIXME: When server enabled, but vhost disabled, should report error.
    SrsRtcConnection* session = NULL;
    if ((err = server_->create_session(&ruc, local_sdp, &session)) != srs_success) {
        return srs_error_wrap(err, "create session, dtls=%u, srtp=%u, eip=%s", ruc.dtls_, ruc.srtp_, eip.c_str());
    }

    ostringstream os;
    if ((err = local_sdp.encode(os)) != srs_success) {
        return srs_error_wrap(err, "encode sdp");
    }

    string local_sdp_str = os.str();
    // Filter the \r\n to \\r\\n for JSON.
    string local_sdp_escaped = srs_string_replace(local_sdp_str.c_str(), "\r\n", "\\r\\n");