Skip to content

Commit

Permalink
UniquePtr: Support SrsUniquePtr to replace SrsAutoFree. v6.0.136 (#4109)
Browse files Browse the repository at this point in the history
To manage an object:

```cpp
// Before
MyClass* ptr = new MyClass();
SrsAutoFree(MyClass, ptr);
ptr->do_something();

// Now
SrsUniquePtr<MyClass> ptr(new MyClass());
ptr->do_something();
```

To manage an array of objects:

```cpp
// Before
char* ptr = new char[10];
SrsAutoFreeA(char, ptr);
ptr[0] = 0xf;

// Now
SrsUniquePtr<char[]> ptr(new char[10]);
ptr[0] = 0xf;
```

In fact, SrsUniquePtr is a limited subset of SrsAutoFree, mainly
managing pointers and arrays. SrsUniquePtr is better than SrsAutoFree
because it has the same API to standard unique ptr.

```cpp
SrsUniquePtr<MyClass> ptr(new MyClass());
ptr->do_something();
MyClass* p = ptr.get();
```

SrsAutoFree actually uses a pointer to a pointer, so it can be set to
NULL, allowing the pointer's value to be changed later (this usage is
different from SrsUniquePtr).

```cpp
// OK to free ptr correctly.
MyClass* ptr;
SrsAutoFree(MyClass, ptr);
ptr = new MyClass();

// Crash because ptr is an invalid pointer.
MyClass* ptr;
SrsUniquePtr<MyClass> ptr(ptr);
ptr = new MyClass();
```

Additionally, SrsAutoFreeH can use specific release functions, which
SrsUniquePtr does not support.

---------

Co-authored-by: Jacob Su <suzp1984@gmail.com>
  • Loading branch information
winlinvip and suzp1984 authored Jul 9, 2024
1 parent baf22d0 commit 23d2602
Show file tree
Hide file tree
Showing 72 changed files with 1,721 additions and 1,670 deletions.
1 change: 1 addition & 0 deletions trunk/3rdparty/st-srs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ The branch [srs](https://github.com/ossrs/state-threads/tree/srs) was patched an
- [x] Check capability for backtrack.
- [x] Support set specifics for any thread.
- [x] Support st_destroy to free resources for asan.
- [x] Support free the stack, [#38](https://github.com/ossrs/state-threads/issues/38).
- [ ] System: Support sendmmsg for UDP, [#12](https://github.com/ossrs/state-threads/issues/12).

## GDB Tools
Expand Down
6 changes: 5 additions & 1 deletion trunk/3rdparty/st-srs/stk.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ _st_stack_t *_st_stack_new(int stack_size)
_st_stack_t *ts;
int extra;

/* If cache stack, we try to use stack from the cache list. */
#ifdef MD_CACHE_STACK
for (qp = _st_free_stacks.next; qp != &_st_free_stacks; qp = qp->next) {
ts = _ST_THREAD_STACK_PTR(qp);
Expand All @@ -80,10 +81,13 @@ _st_stack_t *_st_stack_new(int stack_size)
#endif

extra = _st_randomize_stacks ? _ST_PAGE_SIZE : 0;
/* If not cache stack, we will free all stack in the list, which contains the stack to be freed.
* Note that we should never directly free it at _st_stack_free, because it is still be used,
* and will cause crash. */
#ifndef MD_CACHE_STACK
for (qp = _st_free_stacks.next; qp != &_st_free_stacks;) {
ts = _ST_THREAD_STACK_PTR(qp);
// Before qp is freed, move to next one, because the qp will be freed when free the ts.
/* Before qp is freed, move to next one, because the qp will be freed when free the ts. */
qp = qp->next;

ST_REMOVE_LINK(&ts->links);
Expand Down
2 changes: 1 addition & 1 deletion trunk/3rdparty/st-srs/utest/st_utest.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ struct ErrorObject {
};
extern std::ostream& operator<<(std::ostream& out, const ErrorObject* err);
#define ST_ASSERT_ERROR(error, r0, message) if (error) return new ErrorObject(r0, message)
#define ST_COROUTINE_JOIN(trd, r0) ErrorObject* r0 = NULL; SrsAutoFree(ErrorObject, r0); if (trd) st_thread_join(trd, (void**)&r0)
#define ST_COROUTINE_JOIN(trd, r0) ErrorObject* r0 = NULL; if (trd) st_thread_join(trd, (void**)&r0); SrsUniquePtr<ErrorObject> r0_uptr(r0)
#define ST_EXPECT_SUCCESS(r0) EXPECT_TRUE(!r0) << r0
#define ST_EXPECT_FAILED(r0) EXPECT_TRUE(r0) << r0

Expand Down
2 changes: 1 addition & 1 deletion trunk/configure
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ MODULE_ID="CORE"
MODULE_DEPENDS=()
ModuleLibIncs=(${SRS_OBJS})
MODULE_FILES=("srs_core" "srs_core_version" "srs_core_version5" "srs_core_autofree" "srs_core_performance"
"srs_core_time" "srs_core_platform")
"srs_core_time" "srs_core_platform" "srs_core_deprecated")
CORE_INCS="src/core"; MODULE_DIR=${CORE_INCS} . $SRS_WORKDIR/auto/modules.sh
CORE_OBJS="${MODULE_OBJS[@]}"
#
Expand Down
1 change: 1 addition & 0 deletions trunk/doc/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ The changelog for SRS.
<a name="v6-changes"></a>

## SRS 6.0 Changelog
* v6.0, 2024-07-09, Merge [#4109](https://github.com/ossrs/srs/pull/4109): UniquePtr: Support SrsUniquePtr to replace SrsAutoFree. v6.0.136 (#4109)
* v6.0, 2024-07-08, Merge [#4042](https://github.com/ossrs/srs/pull/4042): Refine config directive token parsing. v6.0.135 (#4042)
* v6.0, 2024-07-04, Merge [#4106](https://github.com/ossrs/srs/pull/4106): SmartPtr: Fix SRT source memory leaking. v6.0.134 (#4106)
* v6.0, 2024-06-29, Merge [#4077](https://github.com/ossrs/srs/pull/4077): Fix misspelling error in app config. v6.0.133 (#4077)
Expand Down
4 changes: 2 additions & 2 deletions trunk/ide/srs_clion/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ ProcessorCount(JOBS)
# We should always configure SRS for switching between branches.
IF (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
EXECUTE_PROCESS(
COMMAND ./configure --osx --srt=on --gb28181=on --apm=on --h265=on --utest=on --ffmpeg-opus=off --jobs=${JOBS}
COMMAND ./configure --osx --srt=on --gb28181=on --apm=on --h265=on --hds=on --utest=on --ffmpeg-opus=off --jobs=${JOBS}
WORKING_DIRECTORY ${SRS_DIR} RESULT_VARIABLE ret)
ELSE ()
EXECUTE_PROCESS(
COMMAND ./configure --srt=on --gb28181=on --apm=on --h265=on --utest=on --ffmpeg-opus=off --jobs=${JOBS}
COMMAND ./configure --srt=on --gb28181=on --apm=on --h265=on --hds=on --utest=on --ffmpeg-opus=off --jobs=${JOBS}
WORKING_DIRECTORY ${SRS_DIR} RESULT_VARIABLE ret)
ENDIF ()
if(NOT ret EQUAL 0)
Expand Down
14 changes: 11 additions & 3 deletions trunk/scripts/copy_to_gits.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,26 @@ if [[ ! -f ~/git/srs-bench/go.mod ]]; then
exit -1
fi

if [[ ! -d ~/git/state-threads ]]; then
echo "no state-threads at ~/git"
exit -1
fi

echo "Copy signaling"
cp -R 3rdparty/signaling/* ~/git/signaling/ &&
cp -R 3rdparty/signaling/.gitignore ~/git/signaling/ &&
(cd ~/git/signaling && git st)
(cd ~/git/signaling && git status)

echo "Copy httpx-static"
cp -R 3rdparty/httpx-static/* ~/git/go-oryx/httpx-static/ &&
cp -R 3rdparty/httpx-static/.gitignore ~/git/go-oryx/httpx-static/ &&
(cd ~/git/go-oryx && git st)
(cd ~/git/go-oryx && git status)

echo "Copy srs-bench"
cp -R 3rdparty/srs-bench/* ~/git/srs-bench/ &&
cp -R 3rdparty/srs-bench/.gitignore ~/git/srs-bench/ &&
(cd ~/git/srs-bench && git st)
(cd ~/git/srs-bench && git status)

echo "Copy state-threads"
cp -R 3rdparty/st-srs/* ~/git/state-threads/ &&
(cd ~/git/state-threads && git st)
5 changes: 1 addition & 4 deletions trunk/src/app/srs_app_caster_flv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,7 @@ srs_error_t SrsDynamicHttpConn::proxy(ISrsHttpResponseWriter* w, ISrsHttpMessage

output = o;
srs_trace("flv: proxy %s:%d %s to %s", ip.c_str(), port, r->uri().c_str(), output.c_str());

char* buffer = new char[SRS_HTTP_FLV_STREAM_BUFFER];
SrsAutoFreeA(char, buffer);


ISrsHttpResponseReader* rr = r->body_reader();
SrsHttpFileReader reader(rr);
SrsFlvDecoder dec;
Expand Down
28 changes: 13 additions & 15 deletions trunk/src/app/srs_app_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1157,13 +1157,13 @@ srs_error_t SrsConfDirective::parse_conf(SrsConfigBuffer* buffer, SrsDirectiveCo
srs_assert(!file.empty());
srs_trace("config parse include %s", file.c_str());

SrsConfigBuffer* include_file_buffer = NULL;
SrsAutoFree(SrsConfigBuffer, include_file_buffer);
if ((err = conf->build_buffer(file, &include_file_buffer)) != srs_success) {
SrsConfigBuffer* include_file_buffer_raw = NULL;
if ((err = conf->build_buffer(file, &include_file_buffer_raw)) != srs_success) {
return srs_error_wrap(err, "buffer fullfill %s", file.c_str());
}
SrsUniquePtr<SrsConfigBuffer> include_file_buffer(include_file_buffer_raw);

if ((err = parse_conf(include_file_buffer, SrsDirectiveContextFile, conf)) != srs_success) {
if ((err = parse_conf(include_file_buffer.get(), SrsDirectiveContextFile, conf)) != srs_success) {
return srs_error_wrap(err, "parse include buffer %s", file.c_str());
}
}
Expand Down Expand Up @@ -1628,10 +1628,8 @@ srs_error_t SrsConfig::reload_vhost(SrsConfDirective* old_root)
srs_error_t SrsConfig::reload_conf(SrsConfig* conf)
{
srs_error_t err = srs_success;

SrsConfDirective* old_root = root;
SrsAutoFree(SrsConfDirective, old_root);


SrsUniquePtr<SrsConfDirective> old_root(root);
root = conf->root;
conf->root = NULL;

Expand Down Expand Up @@ -1665,14 +1663,14 @@ srs_error_t SrsConfig::reload_conf(SrsConfig* conf)
}

// Merge config: rtc_server
if ((err = reload_rtc_server(old_root)) != srs_success) {
if ((err = reload_rtc_server(old_root.get())) != srs_success) {
return srs_error_wrap(err, "http steram");;
}

// TODO: FIXME: support reload stream_caster.

// merge config: vhost
if ((err = reload_vhost(old_root)) != srs_success) {
if ((err = reload_vhost(old_root.get())) != srs_success) {
return srs_error_wrap(err, "vhost");;
}

Expand Down Expand Up @@ -2260,13 +2258,13 @@ srs_error_t SrsConfig::parse_file(const char* filename)
return srs_error_new(ERROR_SYSTEM_CONFIG_INVALID, "empty config");
}

SrsConfigBuffer* buffer = NULL;
SrsAutoFree(SrsConfigBuffer, buffer);
if ((err = build_buffer(config_file, &buffer)) != srs_success) {
SrsConfigBuffer* buffer_raw = NULL;
if ((err = build_buffer(config_file, &buffer_raw)) != srs_success) {
return srs_error_wrap(err, "buffer fullfill %s", filename);
}

if ((err = parse_buffer(buffer)) != srs_success) {

SrsUniquePtr<SrsConfigBuffer> buffer(buffer_raw);
if ((err = parse_buffer(buffer.get())) != srs_success) {
return srs_error_wrap(err, "parse buffer %s", filename);
}

Expand Down
9 changes: 4 additions & 5 deletions trunk/src/app/srs_app_conn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -902,19 +902,18 @@ srs_error_t SrsSslConnection::read(void* plaintext, size_t nn_plaintext, ssize_t
if (r0 == -1 && r1 == SSL_ERROR_WANT_READ) {
// TODO: Can we avoid copy?
int nn_cipher = nn_plaintext;
char* cipher = new char[nn_cipher];
SrsAutoFreeA(char, cipher);
SrsUniquePtr<char[]> cipher(new char[nn_cipher]);

// Read the cipher from SSL.
ssize_t nn = 0;
if ((err = transport->read(cipher, nn_cipher, &nn)) != srs_success) {
if ((err = transport->read(cipher.get(), nn_cipher, &nn)) != srs_success) {
return srs_error_wrap(err, "https: read");
}

int r0 = BIO_write(bio_in, cipher, nn);
int r0 = BIO_write(bio_in, cipher.get(), nn);
if (r0 <= 0) {
// TODO: 0 or -1 maybe block, use BIO_should_retry to check.
return srs_error_new(ERROR_HTTPS_READ, "BIO_write r0=%d, cipher=%p, size=%d", r0, cipher, nn);
return srs_error_new(ERROR_HTTPS_READ, "BIO_write r0=%d, cipher=%p, size=%d", r0, cipher.get(), nn);
}
continue;
}
Expand Down
14 changes: 6 additions & 8 deletions trunk/src/app/srs_app_dash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -293,10 +293,9 @@ srs_error_t SrsMpdWriter::write(SrsFormat* format, SrsFragmentWindow* afragments
}
ss << " </Period>" << endl;
ss << "</MPD>" << endl;

SrsFileWriter* fw = new SrsFileWriter();
SrsAutoFree(SrsFileWriter, fw);


SrsUniquePtr<SrsFileWriter> fw(new SrsFileWriter());

string full_path_tmp = full_path + ".tmp";
if ((err = fw->open(full_path_tmp)) != srs_success) {
return srs_error_wrap(err, "Open MPD file=%s failed", full_path_tmp.c_str());
Expand Down Expand Up @@ -651,10 +650,9 @@ srs_error_t SrsDashController::refresh_init_mp4(SrsSharedPtrMessage* msg, SrsFor
} else {
path += "/audio-init.mp4";
}

SrsInitMp4* init_mp4 = new SrsInitMp4();
SrsAutoFree(SrsInitMp4, init_mp4);


SrsUniquePtr<SrsInitMp4> init_mp4(new SrsInitMp4());

init_mp4->set_path(path);

int tid = msg->is_video()? video_track_id : audio_track_id;
Expand Down
67 changes: 30 additions & 37 deletions trunk/src/app/srs_app_dvr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,19 +119,19 @@ srs_error_t SrsDvrSegmenter::write_metadata(SrsSharedPtrMessage* metadata)
srs_error_t SrsDvrSegmenter::write_audio(SrsSharedPtrMessage* shared_audio, SrsFormat* format)
{
srs_error_t err = srs_success;
SrsSharedPtrMessage* audio = shared_audio->copy();
SrsAutoFree(SrsSharedPtrMessage, audio);
if ((err = jitter->correct(audio, jitter_algorithm)) != srs_success) {

// TODO: FIXME: Use SrsSharedPtr instead.
SrsUniquePtr<SrsSharedPtrMessage> audio(shared_audio->copy());

if ((err = jitter->correct(audio.get(), jitter_algorithm)) != srs_success) {
return srs_error_wrap(err, "jitter");
}

if ((err = on_update_duration(audio)) != srs_success) {
if ((err = on_update_duration(audio.get())) != srs_success) {
return srs_error_wrap(err, "update duration");
}

if ((err = encode_audio(audio, format)) != srs_success) {
if ((err = encode_audio(audio.get(), format)) != srs_success) {
return srs_error_wrap(err, "encode audio");
}

Expand All @@ -141,19 +141,19 @@ srs_error_t SrsDvrSegmenter::write_audio(SrsSharedPtrMessage* shared_audio, SrsF
srs_error_t SrsDvrSegmenter::write_video(SrsSharedPtrMessage* shared_video, SrsFormat* format)
{
srs_error_t err = srs_success;
SrsSharedPtrMessage* video = shared_video->copy();
SrsAutoFree(SrsSharedPtrMessage, video);
if ((err = jitter->correct(video, jitter_algorithm)) != srs_success) {

// TODO: FIXME: Use SrsSharedPtr instead.
SrsUniquePtr<SrsSharedPtrMessage> video(shared_video->copy());

if ((err = jitter->correct(video.get(), jitter_algorithm)) != srs_success) {
return srs_error_wrap(err, "jitter");
}

if ((err = encode_video(video, format)) != srs_success) {
if ((err = encode_video(video.get(), format)) != srs_success) {
return srs_error_wrap(err, "encode video");
}

if ((err = on_update_duration(video)) != srs_success) {
if ((err = on_update_duration(video.get())) != srs_success) {
return srs_error_wrap(err, "update duration");
}

Expand Down Expand Up @@ -256,38 +256,34 @@ srs_error_t SrsDvrFlvSegmenter::refresh_metadata()
int64_t cur = fs->tellg();

// buffer to write the size.
char* buf = new char[SrsAmf0Size::number()];
SrsAutoFreeA(char, buf);

SrsBuffer stream(buf, SrsAmf0Size::number());
SrsUniquePtr<char[]> buf(new char[SrsAmf0Size::number()]);
SrsBuffer stream(buf.get(), SrsAmf0Size::number());

// filesize to buf.
SrsAmf0Any* size = SrsAmf0Any::number((double)cur);
SrsAutoFree(SrsAmf0Any, size);

SrsUniquePtr<SrsAmf0Any> size(SrsAmf0Any::number((double)cur));

stream.skip(-1 * stream.pos());
if ((err = size->write(&stream)) != srs_success) {
return srs_error_wrap(err, "write filesize");
}

// update the flesize.
fs->seek2(filesize_offset);
if ((err = fs->write(buf, SrsAmf0Size::number(), NULL)) != srs_success) {
if ((err = fs->write(buf.get(), SrsAmf0Size::number(), NULL)) != srs_success) {
return srs_error_wrap(err, "update filesize");
}

// duration to buf
SrsAmf0Any* dur = SrsAmf0Any::number((double)srsu2ms(fragment->duration()) / 1000.0);
SrsAutoFree(SrsAmf0Any, dur);

SrsUniquePtr<SrsAmf0Any> dur(SrsAmf0Any::number((double)srsu2ms(fragment->duration()) / 1000.0));

stream.skip(-1 * stream.pos());
if ((err = dur->write(&stream)) != srs_success) {
return srs_error_wrap(err, "write duration");
}

// update the duration
fs->seek2(duration_offset);
if ((err = fs->write(buf, SrsAmf0Size::number(), NULL)) != srs_success) {
if ((err = fs->write(buf.get(), SrsAmf0Size::number(), NULL)) != srs_success) {
return srs_error_wrap(err, "update duration");
}

Expand Down Expand Up @@ -332,15 +328,13 @@ srs_error_t SrsDvrFlvSegmenter::encode_metadata(SrsSharedPtrMessage* metadata)
}

SrsBuffer stream(metadata->payload, metadata->size);

SrsAmf0Any* name = SrsAmf0Any::str();
SrsAutoFree(SrsAmf0Any, name);

SrsUniquePtr<SrsAmf0Any> name(SrsAmf0Any::str());
if ((err = name->read(&stream)) != srs_success) {
return srs_error_wrap(err, "read name");
}

SrsAmf0Object* obj = SrsAmf0Any::object();
SrsAutoFree(SrsAmf0Object, obj);

SrsUniquePtr<SrsAmf0Object> obj(SrsAmf0Any::object());
if ((err = obj->read(&stream)) != srs_success) {
return srs_error_wrap(err, "read object");
}
Expand All @@ -355,16 +349,15 @@ srs_error_t SrsDvrFlvSegmenter::encode_metadata(SrsSharedPtrMessage* metadata)
obj->set("duration", SrsAmf0Any::number(0));

int size = name->total_size() + obj->total_size();
char* payload = new char[size];
SrsAutoFreeA(char, payload);

SrsUniquePtr<char[]> payload(new char[size]);

// 11B flv header, 3B object EOF, 8B number value, 1B number flag.
duration_offset = fs->tellg() + size + 11 - SrsAmf0Size::object_eof() - SrsAmf0Size::number();
// 2B string flag, 8B number value, 8B string 'duration', 1B number flag
filesize_offset = duration_offset - SrsAmf0Size::utf8("duration") - SrsAmf0Size::number();

// convert metadata to bytes.
SrsBuffer buf(payload, size);
SrsBuffer buf(payload.get(), size);

if ((err = name->write(&buf)) != srs_success) {
return srs_error_wrap(err, "write name");
Expand All @@ -374,7 +367,7 @@ srs_error_t SrsDvrFlvSegmenter::encode_metadata(SrsSharedPtrMessage* metadata)
}

// to flv file.
if ((err = enc->write_metadata(18, payload, size)) != srs_success) {
if ((err = enc->write_metadata(18, payload.get(), size)) != srs_success) {
return srs_error_wrap(err, "write metadata");
}

Expand Down
Loading

0 comments on commit 23d2602

Please sign in to comment.