ronantakizawa/codereview-bench huggingface dataset:
Repository: ossrs/srs
File: trunk/src/app/srs_app_source.cpp (C++)

Code before:
    // notify the handler.
    srs_assert(handler);
    SrsStatistic* stat = SrsStatistic::instance();
    stat->on_stream_close(req);

    handler->on_unpublish(this, req);

    if (bridge_) {
        bridge_->on_unpublish();
        srs_freep(bridge_);
    }

    // no consumer, stream is die.
    if (consumers.empty()) {
        die_at = srs_get_system_time();
    }
}

srs_error_t SrsLiveSource::create_consumer(SrsLiveConsumer*& consumer)
{
    srs_error_t err = srs_success;
    
    consumer = new SrsLiveConsumer(this);
    consumers.push_back(consumer);
#ifdef SRS_PERF_KICKOFF_AS_NO_ONE_WATCHING
    no_one_wating_at = 0;
#endif

    // for edge, when play edge stream, check the state
    if (_srs_config->get_vhost_is_edge(req->vhost)) {
        // notice edge to start for the first client.
        if ((err = play_edge->on_client_play()) != srs_success) {
            return srs_error_wrap(err, "play edge");
        }
    }
    
    return err;
}

srs_error_t SrsLiveSource::consumer_dumps(SrsLiveConsumer* consumer, bool ds, bool dm, bool dg)
{
    srs_error_t err = srs_success;

    srs_utime_t queue_size = _srs_config->get_queue_length(req->vhost);
    consumer->set_queue_size(queue_size);

    // if atc, update the sequence header to gop cache time.
    if (atc && !gop_cache->empty()) {
        if (meta->data()) {
            meta->data()->timestamp = srsu2ms(gop_cache->start_time());
        }

Review comment (suggestion):
The variable name `idle_at` expresses the time point when the Source starts becoming idle.

`TRANS_BY_GPT3`

Code after:
    stat->on_stream_publish(req, _source_id.c_str());

    // When no players, the publisher is idle now.
    if (consumers.empty()) {
        publisher_idle_at_ = srs_get_system_time();
    }
    
    return err;
}

void SrsLiveSource::on_unpublish()
{
    // ignore when already unpublished.
    if (_can_publish) {
        return;
    }
    
    // Notify the hub about the unpublish event.
    hub->on_unpublish();
    
    // only clear the gop cache,
    // donot clear the sequence header, for it maybe not changed,
    // when drop dup sequence header, drop the metadata also.
    gop_cache->clear();

    // Reset the metadata cache, to make VLC happy when disable/enable stream.
    // @see https://github.com/ossrs/srs/issues/1630#issuecomment-597979448
    meta->update_previous_vsh();
    meta->update_previous_ash();

    srs_trace("cleanup when unpublish");
    
    _can_publish = true;
    if (!_source_id.empty()) {
        _pre_source_id = _source_id;
    }
    _source_id = SrsContextId();

    // notify the handler.
    srs_assert(handler);
    SrsStatistic* stat = SrsStatistic::instance();
    stat->on_stream_close(req);

    handler->on_unpublish(this, req);

    if (bridge_) {
        bridge_->on_unpublish();
        srs_freep(bridge_);
    }

    // no consumer, stream is die.
Repository: ossrs/srs
File: trunk/src/app/srs_app_config.cpp (C++)

Code before:
    SRS_OVERWRITE_BY_ENV_MILLISECONDS("srs.vhost.publish.normal_timeout"); // SRS_VHOST_PUBLISH_NORMAL_TIMEOUT

    // the timeout for publish recv.
    // we must use more smaller timeout, for the recv never know the status
    // of underlayer socket.
    static srs_utime_t DEFAULT = 5 * SRS_UTIME_SECONDS;
    
    SrsConfDirective* conf = get_vhost(vhost);
    if (!conf) {
        return DEFAULT;
    }
    
    conf = conf->get("publish");
    if (!conf) {
        return DEFAULT;
    }
    
    conf = conf->get("normal_timeout");
    if (!conf || conf->arg0().empty()) {
        return DEFAULT;
    }
    
    return (srs_utime_t)(::atoi(conf->arg0().c_str()) * SRS_UTIME_MILLISECONDS);
}

srs_utime_t SrsConfig::get_publish_kickoff_timeout(std::string vhost)
{
    // the timeout to kickoff publish as no one watching,
    // the default value is 3 minutes.
    static srs_utime_t DEFAULT = 180 * SRS_UTIME_SECONDS;
    
    SrsConfDirective* conf = get_vhost(vhost);
    if (!conf) {
        return DEFAULT;
    }
    
    conf = conf->get("publish");
    if (!conf) {
        return DEFAULT;
    }

    conf = conf->get("kickoff_for_idle");
    if (!conf || conf->arg0().empty()) {
        return DEFAULT;
    }
    
    return (srs_utime_t)(::atoi(conf->arg0().c_str()) * SRS_UTIME_MILLISECONDS);
}

int SrsConfig::get_global_chunk_size()
{

Review comment (suggestion):
The function name should also be changed accordingly so that it can be searched for.

```
get_publish_kickoff_for_idle
```

`TRANS_BY_GPT3`

Code after:
        return DEFAULT;
    }
    
    return (srs_utime_t)(::atoi(conf->arg0().c_str()) * SRS_UTIME_MILLISECONDS);
}

srs_utime_t SrsConfig::get_publish_normal_timeout(string vhost)
{
    SRS_OVERWRITE_BY_ENV_MILLISECONDS("srs.vhost.publish.normal_timeout"); // SRS_VHOST_PUBLISH_NORMAL_TIMEOUT

    // the timeout for publish recv.
    // we must use more smaller timeout, for the recv never know the status
    // of underlayer socket.
    static srs_utime_t DEFAULT = 5 * SRS_UTIME_SECONDS;
    
    SrsConfDirective* conf = get_vhost(vhost);
    if (!conf) {
        return DEFAULT;
    }
    
    conf = conf->get("publish");
    if (!conf) {
        return DEFAULT;
    }
    
    conf = conf->get("normal_timeout");
    if (!conf || conf->arg0().empty()) {
        return DEFAULT;
    }
    
    return (srs_utime_t)(::atoi(conf->arg0().c_str()) * SRS_UTIME_MILLISECONDS);
}

srs_utime_t SrsConfig::get_publish_kickoff_for_idle(std::string vhost)
{
    return get_publish_kickoff_for_idle(get_vhost(vhost));
}

srs_utime_t SrsConfig::get_publish_kickoff_for_idle(SrsConfDirective* vhost)
{
    SRS_OVERWRITE_BY_ENV_FLOAT_SECONDS("srs.vhost.publish.kickoff_for_idle"); // SRS_VHOST_PUBLISH_KICKOFF_FOR_IDLE

    static srs_utime_t DEFAULT = 0 * SRS_UTIME_SECONDS;
    
    SrsConfDirective* conf = vhost;
    if (!conf) {
        return DEFAULT;
    }
    
    conf = conf->get("publish");
    if (!conf) {
Repository: ossrs/srs
File: trunk/src/app/srs_app_source.cpp (C++)

Code before:

    // notify the handler.
    srs_assert(handler);
    SrsStatistic* stat = SrsStatistic::instance();
    stat->on_stream_close(req);

    handler->on_unpublish(this, req);

    if (bridge_) {
        bridge_->on_unpublish();
        srs_freep(bridge_);
    }

    // no consumer, stream is die.
    if (consumers.empty()) {
        die_at = srs_get_system_time();
    }
}

srs_error_t SrsLiveSource::create_consumer(SrsLiveConsumer*& consumer)
{
    srs_error_t err = srs_success;
    
    consumer = new SrsLiveConsumer(this);
    consumers.push_back(consumer);
    idle_at = 0;

    // for edge, when play edge stream, check the state
    if (_srs_config->get_vhost_is_edge(req->vhost)) {
        // notice edge to start for the first client.
        if ((err = play_edge->on_client_play()) != srs_success) {
            return srs_error_wrap(err, "play edge");
        }
    }
    
    return err;
}

srs_error_t SrsLiveSource::consumer_dumps(SrsLiveConsumer* consumer, bool ds, bool dm, bool dg)
{
    srs_error_t err = srs_success;

    srs_utime_t queue_size = _srs_config->get_queue_length(req->vhost);
    consumer->set_queue_size(queue_size);

    // if atc, update the sequence header to gop cache time.
    if (atc && !gop_cache->empty()) {
        if (meta->data()) {
            meta->data()->timestamp = srsu2ms(gop_cache->start_time());
        }
        if (meta->vsh()) {

Review comment (suggestion):
If `forward` is configured, but it is not a `consumer`, should it also be considered?
```config
vhost __defaultVhost__ {
    forward 127.0.0.1:19350;
    publish {
        kickoff_for_idle     60000; # default is 60s, 0 means disable
    }
}
```

`TRANS_BY_GPT3`

Code after:
    }
    _source_id = SrsContextId();

    // notify the handler.
    srs_assert(handler);
    SrsStatistic* stat = SrsStatistic::instance();
    stat->on_stream_close(req);

    handler->on_unpublish(this, req);

    if (bridge_) {
        bridge_->on_unpublish();
        srs_freep(bridge_);
    }

    // no consumer, stream is die.
    if (consumers.empty()) {
        stream_die_at_ = srs_get_system_time();
    }
}

srs_error_t SrsLiveSource::create_consumer(SrsLiveConsumer*& consumer)
{
    srs_error_t err = srs_success;
    
    consumer = new SrsLiveConsumer(this);
    consumers.push_back(consumer);

    // There should be one consumer, so reset the timeout.
    stream_die_at_ = 0;
    publisher_idle_at_ = 0;

    // for edge, when play edge stream, check the state
    if (_srs_config->get_vhost_is_edge(req->vhost)) {
        // notice edge to start for the first client.
        if ((err = play_edge->on_client_play()) != srs_success) {
            return srs_error_wrap(err, "play edge");
        }
    }
    
    return err;
}

srs_error_t SrsLiveSource::consumer_dumps(SrsLiveConsumer* consumer, bool ds, bool dm, bool dg)
{
    srs_error_t err = srs_success;

    srs_utime_t queue_size = _srs_config->get_queue_length(req->vhost);
    consumer->set_queue_size(queue_size);

    // if atc, update the sequence header to gop cache time.
Repository: ossrs/srs
File: trunk/src/app/srs_app_hls.cpp (C++)

Code before:

    return err;
}

srs_error_t SrsHlsMuxer::restore_stream()
{
    srs_error_t err = srs_success;

    // exist the m3u8 file.
    if (!srs_path_exists(m3u8)) {
        return err;
    }

    srs_trace("hls: restore stream m3u8=%s, m3u8_url=%s, hls_path=%s",
            m3u8.c_str(), m3u8_url.c_str(), hls_path.c_str());

    // read m3u8
    SrsFileReader fr;
    if ((err = fr.open(m3u8)) != srs_success) {
        return srs_error_wrap(err, "open file");
    }

    int nb_fbuf = fr.filesize();
    char* fbuf = new char[nb_fbuf];
    SrsAutoFreeA(char, fbuf);
    if ((err = fr.read(fbuf, nb_fbuf, NULL)) != srs_success) {
        return srs_error_wrap(err, "read data");
    }

    // parse
    std::string body(fbuf, nb_fbuf);
    if (body.empty()) {
        return srs_error_wrap(err, "read empty m3u8");
    }

    srs_utime_t max_target_duration = 0;
    bool discon = false;

    std::string ptl;
    while (!body.empty()) {
        size_t pos = string::npos;

        std::string line;
        if ((pos = body.find("\n")) != string::npos) {
            line = body.substr(0, pos);
            body = body.substr(pos + 1);
        } else {
            line = body;
            body = "";
        }


Review comment (suggestion):
I seem to remember there is an auxiliary function like `srs read all`, which can read all the content of a certain Reader until EOF.

It's a bit like the `ioutil.ReadAll(io.Reader)` function in Go.

`TRANS_BY_GPT4`

Code after:
    latest_acodec_ = SrsAudioCodecIdForbidden;
    latest_vcodec_ = SrsVideoCodecIdForbidden;
    video_track_id_ = 0;
    audio_track_id_ = 0;
    init_mp4_ready_ = false;
    video_dts_ = 0;

    memset(key_, 0, 16);
    memset(iv_, 0, 16);
}

SrsHlsFmp4Muxer::~SrsHlsFmp4Muxer()
{
    srs_freep(segments_);
    srs_freep(current_);
    srs_freep(req_);
    srs_freep(async_);
    srs_freep(writer_);
}

void SrsHlsFmp4Muxer::dispose()
{
    srs_error_t err = srs_success;

    segments_->dispose();

    if (current_) {
        if ((err = current_->unlink_tmpfile()) != srs_success) {
            srs_warn("Unlink tmp ts failed %s", srs_error_desc(err).c_str());
            srs_freep(err);
        }
        srs_freep(current_);
    }

    if (unlink(m3u8_.c_str()) < 0) {
        srs_warn("dispose unlink path failed. file=%s", m3u8_.c_str());
    }

    srs_trace("gracefully dispose hls %s", req_ ? req_->get_stream_url().c_str() : "");
}

int SrsHlsFmp4Muxer::sequence_no()
{
    return sequence_no_;
}

std::string SrsHlsFmp4Muxer::m4s_url()
{
    return current_ ? current_->uri : "";
}
Repository: ossrs/srs
File: trunk/src/app/srs_app_config.cpp (C++)

Code before:

    return SRS_CONF_PERFER_FALSE(conf->arg0());
}

bool SrsConfig::get_exporter_enabled()
{
    SRS_OVERWRITE_BY_ENV_BOOL("SRS_EXPORTER_ENABLED");

    static bool DEFAULT = false;

    SrsConfDirective* conf = root->get("exporter");
    if (!conf) {
        return DEFAULT;
    }

    conf = conf->get("enabled");
    if (!conf) {
        return DEFAULT;
    }

    return SRS_CONF_PERFER_FALSE(conf->arg0());
}

int SrsConfig::get_exporter_listen()
{
    SRS_OVERWRITE_BY_ENV_INT("SRS_EXPORTER_LISTEN");

    static int DEFAULT = 9972;

    SrsConfDirective* conf = root->get("exporter");
    if (!conf) {
        return DEFAULT;
    }

    conf = conf->get("listen");
    if (!conf) {
        return DEFAULT;
    }

    return ::atoi(conf->arg0().c_str());
}

string SrsConfig::get_exporter_label()
{
    SRS_OVERWRITE_BY_ENV_STRING("SRS_EXPORTER_LABEL");

    static string DEFAULT = "";

    SrsConfDirective* conf = root->get("exporter");
    if (!conf) {
        return DEFAULT;

Review comment (suggestion):
If it doesn't match `full.conf`, treat the configuration file and comments as code, just like that.

`TRANS_BY_GPT3`

Code after:

    return SRS_CONF_PERFER_FALSE(conf->arg0());
}

bool SrsConfig::get_exporter_enabled()
{
    SRS_OVERWRITE_BY_ENV_BOOL("SRS_EXPORTER_ENABLED");

    static bool DEFAULT = false;

    SrsConfDirective* conf = root->get("exporter");
    if (!conf) {
        return DEFAULT;
    }

    conf = conf->get("enabled");
    if (!conf) {
        return DEFAULT;
    }

    return SRS_CONF_PERFER_FALSE(conf->arg0());
}

string SrsConfig::get_exporter_listen()
{
    SRS_OVERWRITE_BY_ENV_STRING("SRS_EXPORTER_LISTEN");

    static string DEFAULT = "9972";

    SrsConfDirective* conf = root->get("exporter");
    if (!conf) {
        return DEFAULT;
    }

    conf = conf->get("listen");
    if (!conf) {
        return DEFAULT;
    }

    return conf->arg0();
}

string SrsConfig::get_exporter_label()
{
    SRS_OVERWRITE_BY_ENV_STRING("SRS_EXPORTER_LABEL");

    static string DEFAULT = "";

    SrsConfDirective* conf = root->get("exporter");
    if (!conf) {
        return DEFAULT;
Repository: ossrs/srs
File: trunk/src/app/srs_app_http_api.cpp (C++)

Code before:
       << nstreams
       << "\n";

    // Current number of online clients.
    ss << "# HELP srs_clients The number of SRS concurrent clients.\n"
       << "# TYPE srs_clients gauge\n"
       << "srs_clients "
       << nclients
       << "\n";

    // The total of clients connections.
    ss << "# HELP srs_clients_total The total counts of SRS clients.\n"
       << "# TYPE srs_clients_total counter\n"
       << "srs_clients_total "
       << total_nclients
       << "\n";

    // The total of clients errors.
    ss << "# HELP srs_clients_errs_total The total errors of SRS clients.\n"
       << "# TYPE srs_clients_errs_total counter\n"
       << "srs_clients_errs_total "
       << nerrs
       << "\n";

    w->header()->set_content_type("text/plain; charset=utf-8");
    return w->write((char*)ss.str().c_str(), (int)ss.str().length());
}


Review comment (suggestion):
This modification method is to apply a patch, but it is not the most appropriate approach. The function that should be modified is:

```
srs_error_t srs_api_response_json(ISrsHttpResponseWriter* w, string data)
{
    srs_error_t err = srs_success;
    
    SrsHttpHeader* h = w->header();
    
    h->set_content_length(data.length());
    h->set_content_type("application/json");
```

Check here, if the Content-Type has already been set, then do not set it again. This way, in the API, we can add a single setting and continue calling this function, supporting JSONP.

`TRANS_BY_GPT3`

Code after:
       << "# TYPE srs_streams gauge\n"
       << "srs_streams "
       << nstreams
       << "\n";

    // Current number of online clients.
    ss << "# HELP srs_clients The number of SRS concurrent clients.\n"
       << "# TYPE srs_clients gauge\n"
       << "srs_clients "
       << nclients
       << "\n";

    // The total of clients connections.
    ss << "# HELP srs_clients_total The total counts of SRS clients.\n"
       << "# TYPE srs_clients_total counter\n"
       << "srs_clients_total "
       << total_nclients
       << "\n";

    // The total of clients errors.
    ss << "# HELP srs_clients_errs_total The total errors of SRS clients.\n"
       << "# TYPE srs_clients_errs_total counter\n"
       << "srs_clients_errs_total "
       << nerrs
       << "\n";

    w->header()->set_content_type("text/plain; charset=utf-8");

    return srs_api_response(w, r, ss.str());
}
Repository: ossrs/srs
File: trunk/src/app/srs_app_source.cpp (C++)

Code before:
}

SrsGopCache::~SrsGopCache()
{
    clear();
}

void SrsGopCache::dispose()
{
    clear();
}

void SrsGopCache::set(bool v)
{
    enable_gop_cache = v;
    
    if (!v) {
        clear();
        return;
    }
}


void SrsGopCache::set_max_frames(int v)
{
    gop_cache_max_frames_ = v;

    if (cached_video_count > gop_cache_max_frames_){
        srs_warn("too much frames in the gop cache");
        clear();
    }
}

bool SrsGopCache::enabled()
{
    return enable_gop_cache;
}

srs_error_t SrsGopCache::cache(SrsSharedPtrMessage* shared_msg)
{
    srs_error_t err = srs_success;
    
    if (!enable_gop_cache) {
        return err;
    }
    
    // the gop cache know when to gop it.
    SrsSharedPtrMessage* msg = shared_msg;
    
    // got video, update the video count if acceptable
    if (msg->is_video()) {

Review comment (suggestion):
This function should not call clear because it is a set function, and it is generally expected to only set variables without dropping frames.

After setting, the next incoming frame will trigger the frame dropping logic, but it is not necessary to do it here.

`TRANS_BY_GPT3`

Code after:
}

SrsGopCache::~SrsGopCache()
{
    clear();
}

void SrsGopCache::dispose()
{
    clear();
}

void SrsGopCache::set(bool v)
{
    enable_gop_cache = v;
    
    if (!v) {
        clear();
        return;
    }
}

void SrsGopCache::set_gop_cache_max_frames(int v)
{
    gop_cache_max_frames_ = v;
}

bool SrsGopCache::enabled()
{
    return enable_gop_cache;
}

srs_error_t SrsGopCache::cache(SrsSharedPtrMessage* shared_msg)
{
    srs_error_t err = srs_success;
    
    if (!enable_gop_cache) {
        return err;
    }
    
    // the gop cache know when to gop it.
    SrsSharedPtrMessage* msg = shared_msg;
    
    // got video, update the video count if acceptable
    if (msg->is_video()) {
        // drop video when not h.264
        if (!SrsFlvVideo::h264(msg->payload, msg->size)) {
            return err;
        }
        
        cached_video_count++;
Repository: ossrs/srs
File: trunk/src/app/srs_app_source.cpp (C++)

Code before:
    SrsSharedPtrMessage* msg = shared_msg;
    
    // got video, update the video count if acceptable
    if (msg->is_video()) {
        // drop video when not h.264
        if (!SrsFlvVideo::h264(msg->payload, msg->size)) {
            return err;
        }
        
        cached_video_count++;
        audio_after_last_video_count = 0;
    }
    
    // no acceptable video or pure audio, disable the cache.
    if (pure_audio()) {
        return err;
    }
    
    // ok, gop cache enabled, and got an audio.
    if (msg->is_audio()) {
        audio_after_last_video_count++;
    }
    
    // clear gop cache when pure audio count overflow
    if (audio_after_last_video_count > SRS_PURE_AUDIO_GUESS_COUNT) {
        srs_warn("clear gop cache for guess pure audio overflow");
        clear();
        return err;
    }
    
    if (cached_video_count > gop_cache_max_frames_){
        srs_warn("too much frames in the gop cache");
        clear();
    }

    // clear gop cache when got key frame
    if (msg->is_video() && SrsFlvVideo::keyframe(msg->payload, msg->size)) {
        clear();
        
        // curent msg is video frame, so we set to 1.
        cached_video_count = 1;
    }
    
    // cache the frame.
    gop_cache.push_back(msg->copy());
    
    return err;
}

void SrsGopCache::clear()
{

Review comment (performance):
It would be better to drop frames after this.

```cpp
if (msg->is_video() && SrsFlvVideo::keyframe(msg->payload, msg->size)) {}

if (cached_video_count > gop_cache_max_frames_){}
```

Priority should be given to keyframes, followed by a fallback strategy for video frames.

`TRANS_BY_GPT3`

Code after:
            return err;
        }
        
        cached_video_count++;
        audio_after_last_video_count = 0;
    }
    
    // no acceptable video or pure audio, disable the cache.
    if (pure_audio()) {
        return err;
    }
    
    // ok, gop cache enabled, and got an audio.
    if (msg->is_audio()) {
        audio_after_last_video_count++;
    }
    
    // clear gop cache when pure audio count overflow
    if (audio_after_last_video_count > SRS_PURE_AUDIO_GUESS_COUNT) {
        srs_warn("clear gop cache for guess pure audio overflow");
        clear();
        return err;
    }
    
    // clear gop cache when got key frame
    if (msg->is_video() && SrsFlvVideo::keyframe(msg->payload, msg->size)) {
        clear();
        
        // curent msg is video frame, so we set to 1.
        cached_video_count = 1;
    }

    // cache the frame.
    gop_cache.push_back(msg->copy());

    // Clear gop cache if exceed the max frames.
    if (gop_cache.size() > (size_t)gop_cache_max_frames_) {
        srs_warn("Gop cache exceed max frames=%d, total=%d, videos=%d, aalvc=%d",
            gop_cache_max_frames_, (int)gop_cache.size(), cached_video_count, audio_after_last_video_count);
        clear();
    }

    return err;
}

void SrsGopCache::clear()
{
    std::vector<SrsSharedPtrMessage*>::iterator it;
    for (it = gop_cache.begin(); it != gop_cache.end(); ++it) {
        SrsSharedPtrMessage* msg = *it;
        srs_freep(msg);