Skip to content

Commit 175d460

Browse files
scriptorianscriptorian
committed
Add indirection for formatContext to avoid double delete after demuxer forceClose
1 parent 3a04202 commit 175d460

File tree

6 files changed

+94
-41
lines changed

6 files changed

+94
-41
lines changed

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "beamcoder",
3-
"version": "0.6.9",
3+
"version": "0.6.10",
44
"description": "Node.js native bindings to FFmpeg.",
55
"main": "index.js",
66
"types": "index.d.ts",

‎src/demux.cc‎

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,14 @@ void readFrameExecute(napi_env env, void* data) {
234234
readFrameCarrier* c = (readFrameCarrier*) data;
235235
int ret;
236236

237-
ret = av_read_frame(c->format, c->packet);
237+
AVFormatContext* fmtCtx = c->formatRef->fmtCtx;
238+
if (fmtCtx == nullptr) {
239+
c->status = BEAMCODER_ERROR_READ_FRAME;
240+
c->errorMsg = "Format context has been deleted.";
241+
return;
242+
}
243+
244+
ret = av_read_frame(fmtCtx, c->packet);
238245
if (ret == AVERROR_EOF) {
239246
av_packet_free(&c->packet);
240247
} else if (ret < 0) {
@@ -280,7 +287,7 @@ void readFrameComplete(napi_env env, napi_status asyncStatus, void* data) {
280287
}
281288

282289
napi_value readFrame(napi_env env, napi_callback_info info) {
283-
napi_value resourceName, promise, formatJS, formatExt, adaptorExt;
290+
napi_value resourceName, promise, formatJS, formatRefExt, adaptorExt;
284291
readFrameCarrier* c = new readFrameCarrier;
285292

286293
c->status = napi_create_promise(env, &c->_deferred, &promise);
@@ -289,9 +296,9 @@ napi_value readFrame(napi_env env, napi_callback_info info) {
289296
size_t argc = 0;
290297
c->status = napi_get_cb_info(env, info, &argc, nullptr, &formatJS, nullptr);
291298
REJECT_RETURN;
292-
c->status = napi_get_named_property(env, formatJS, "_formatContext", &formatExt);
299+
c->status = napi_get_named_property(env, formatJS, "_formatContextRef", &formatRefExt);
293300
REJECT_RETURN;
294-
c->status = napi_get_value_external(env, formatExt, (void**) &c->format);
301+
c->status = napi_get_value_external(env, formatRefExt, (void**) &c->formatRef);
295302
REJECT_RETURN;
296303

297304
c->status = napi_get_named_property(env, formatJS, "_adaptor", &adaptorExt);
@@ -328,7 +335,14 @@ void seekFrameExecute(napi_env env, void *data) {
328335
seekFrameCarrier* c = (seekFrameCarrier*) data;
329336
int ret;
330337

331-
ret = av_seek_frame(c->format, c->streamIndex, c->timestamp, c->flags);
338+
AVFormatContext* fmtCtx = c->formatRef->fmtCtx;
339+
if (fmtCtx == nullptr) {
340+
c->status = BEAMCODER_ERROR_READ_FRAME;
341+
c->errorMsg = "Format context has been deleted.";
342+
return;
343+
}
344+
345+
ret = av_seek_frame(fmtCtx, c->streamIndex, c->timestamp, c->flags);
332346
// printf("Seek and ye shall %i, streamIndex = %i, timestamp = %i, flags = %i\n",
333347
// ret, c->streamIndex, c->timestamp, c->flags );
334348
if (ret < 0) {
@@ -369,7 +383,7 @@ void seekFrameComplete(napi_env env, napi_status asyncStatus, void *data) {
369383
*/
370384

371385
napi_value seekFrame(napi_env env, napi_callback_info info) {
372-
napi_value resourceName, promise, formatJS, formatExt, value;
386+
napi_value resourceName, promise, formatJS, formatRefExt, value;
373387
napi_valuetype type;
374388
seekFrameCarrier* c = new seekFrameCarrier;
375389
bool isArray, bValue;
@@ -383,9 +397,9 @@ napi_value seekFrame(napi_env env, napi_callback_info info) {
383397

384398
c->status = napi_get_cb_info(env, info, &argc, argv, &formatJS, nullptr);
385399
REJECT_RETURN;
386-
c->status = napi_get_named_property(env, formatJS, "_formatContext", &formatExt);
400+
c->status = napi_get_named_property(env, formatJS, "_formatContextRef", &formatRefExt);
387401
REJECT_RETURN;
388-
c->status = napi_get_value_external(env, formatExt, (void**) &c->format);
402+
c->status = napi_get_value_external(env, formatRefExt, (void**) &c->formatRef);
389403
REJECT_RETURN;
390404

391405
c->status = napi_create_reference(env, formatJS, 1, &c->passthru);
@@ -529,18 +543,40 @@ napi_value seekFrame(napi_env env, napi_callback_info info) {
529543

530544
napi_value forceCloseInput(napi_env env, napi_callback_info info) {
531545
napi_status status;
532-
napi_value result, formatJS, formatExt;
533-
AVFormatContext* format;
546+
napi_value result, formatJS, formatRefExt, adaptorExt;
547+
fmtCtxRef* fmtRef;
548+
AVFormatContext* fc;
549+
Adaptor *adaptor;
550+
int ret;
534551

535552
size_t argc = 0;
536553
status = napi_get_cb_info(env, info, &argc, nullptr, &formatJS, nullptr);
537554
CHECK_STATUS;
538-
status = napi_get_named_property(env, formatJS, "_formatContext", &formatExt);
555+
status = napi_get_named_property(env, formatJS, "_formatContextRef", &formatRefExt);
556+
CHECK_STATUS;
557+
status = napi_get_value_external(env, formatRefExt, (void**) &fmtRef);
539558
CHECK_STATUS;
540-
status = napi_get_value_external(env, formatExt, (void**) &format);
559+
status = napi_get_named_property(env, formatJS, "_adaptor", &adaptorExt);
560+
CHECK_STATUS;
561+
status = napi_get_value_external(env, adaptorExt, (void**) &adaptor);
541562
CHECK_STATUS;
542563

543-
formatContextFinalizer(env, format, format->pb);
564+
if (fmtRef->fmtCtx != nullptr) {
565+
fc = fmtRef->fmtCtx;
566+
if (fc->pb != nullptr) {
567+
if (adaptor)
568+
avio_context_free(&fc->pb);
569+
else {
570+
ret = avio_closep(&fc->pb);
571+
if (ret < 0) {
572+
printf("DEBUG: For url '%s', %s", (fc->url != nullptr) ? fc->url : "unknown",
573+
avErrorMsg("error closing IO: ", ret));
574+
}
575+
}
576+
}
577+
578+
avformat_close_input(&fmtRef->fmtCtx);
579+
}
544580

545581
status = napi_get_undefined(env, &result);
546582
CHECK_STATUS;

‎src/demux.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ struct demuxerCarrier : carrier {
6767
};
6868

6969
struct readFrameCarrier : carrier {
70-
AVFormatContext* format = nullptr;
70+
fmtCtxRef* formatRef = nullptr;
7171
Adaptor *adaptor = nullptr;
7272
AVPacket* packet = av_packet_alloc();
7373
~readFrameCarrier() {
@@ -76,7 +76,7 @@ struct readFrameCarrier : carrier {
7676
};
7777

7878
struct seekFrameCarrier : carrier {
79-
AVFormatContext* format = nullptr;
79+
fmtCtxRef* formatRef = nullptr;
8080
int streamIndex = -1;
8181
int64_t timestamp = 0;
8282
// weird semantic - backward actually means 'find nearest key frame before timestamp'

‎src/format.cc‎

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3494,7 +3494,9 @@ napi_value formatToJSON(napi_env env, napi_callback_info info) {
34943494
napi_status fromAVFormatContext(napi_env env, AVFormatContext* fmtCtx,
34953495
Adaptor *adaptor, napi_value* result) {
34963496
napi_status status;
3497-
napi_value jsFmtCtx, extFmtCtx, extAdaptor, truth, undef;
3497+
napi_value jsFmtCtx, extFmtCtxRef, extFmtCtx, extAdaptor, truth, undef;
3498+
fmtCtxRef* fmtRef = new fmtCtxRef;
3499+
fmtRef->fmtCtx = fmtCtx;
34983500

34993501
bool isMuxer = fmtCtx->oformat != nullptr;
35003502
bool isFormat = !((fmtCtx->oformat == nullptr) ^ (fmtCtx->iformat == nullptr));
@@ -3505,7 +3507,9 @@ napi_status fromAVFormatContext(napi_env env, AVFormatContext* fmtCtx,
35053507
PASS_STATUS;
35063508
status = napi_get_undefined(env, &undef);
35073509
PASS_STATUS;
3508-
status = napi_create_external(env, fmtCtx, formatContextFinalizer, adaptor, &extFmtCtx);
3510+
status = napi_create_external(env, fmtRef, formatContextFinalizer, adaptor, &extFmtCtxRef);
3511+
PASS_STATUS;
3512+
status = napi_create_external(env, fmtCtx, nullptr, adaptor, &extFmtCtx);
35093513
PASS_STATUS;
35103514
status = napi_create_external(env, adaptor, nullptr, nullptr, &extAdaptor);
35113515
PASS_STATUS;
@@ -3686,11 +3690,12 @@ napi_status fromAVFormatContext(napi_env env, AVFormatContext* fmtCtx,
36863690
{ "newStream", nullptr, newStream, nullptr, nullptr, nullptr,
36873691
napi_enumerable, fmtCtx },
36883692
{ "toJSON", nullptr, formatToJSON, nullptr, nullptr, nullptr, napi_default, fmtCtx },
3693+
{ "_formatContextRef", nullptr, nullptr, nullptr, nullptr, extFmtCtxRef, napi_default, nullptr },
36893694
{ "_formatContext", nullptr, nullptr, nullptr, nullptr, extFmtCtx, napi_default, nullptr },
36903695
{ "_adaptor", nullptr, nullptr, nullptr, nullptr, extAdaptor, napi_default, nullptr },
36913696
{ "__streams", nullptr, nullptr, nullptr, nullptr, undef, napi_writable, nullptr }
36923697
};
3693-
status = napi_define_properties(env, jsFmtCtx, 56, desc);
3698+
status = napi_define_properties(env, jsFmtCtx, 57, desc);
36943699
PASS_STATUS;
36953700
} else {
36963701
napi_property_descriptor desc[] = {
@@ -3810,11 +3815,12 @@ napi_status fromAVFormatContext(napi_env env, AVFormatContext* fmtCtx,
38103815
{ "newStream", nullptr, newStream, nullptr, nullptr, nullptr,
38113816
napi_enumerable, fmtCtx },
38123817
{ "toJSON", nullptr, formatToJSON, nullptr, nullptr, nullptr, napi_default, fmtCtx },
3818+
{ "_formatContextRef", nullptr, nullptr, nullptr, nullptr, extFmtCtxRef, napi_default, nullptr },
38133819
{ "_formatContext", nullptr, nullptr, nullptr, nullptr, extFmtCtx, napi_default, nullptr },
38143820
{ "_adaptor", nullptr, nullptr, nullptr, nullptr, extAdaptor, napi_default, nullptr },
38153821
{ "__streams", nullptr, nullptr, nullptr, nullptr, undef, napi_writable, nullptr }
38163822
};
3817-
status = napi_define_properties(env, jsFmtCtx, 57, desc);
3823+
status = napi_define_properties(env, jsFmtCtx, 58, desc);
38183824
PASS_STATUS;
38193825
}
38203826

@@ -3823,17 +3829,13 @@ napi_status fromAVFormatContext(napi_env env, AVFormatContext* fmtCtx,
38233829
}
38243830

38253831
void formatContextFinalizer(napi_env env, void* data, void* hint) {
3826-
AVFormatContext* fc = (AVFormatContext*) data;
3832+
fmtCtxRef* fmtRef = (fmtCtxRef*) data;
3833+
AVFormatContext* fc;
38273834
Adaptor *adaptor = (Adaptor *)hint;
38283835
int ret;
38293836

3830-
if (fc->iformat != nullptr) {
3831-
// The format context we get here is a copy so the close_input call won't clear the JS format context
3832-
// Hence copy the formatContext pointer to null the iformat after close to avoid a double delete
3833-
AVFormatContext* closeFc = fc;
3834-
avformat_close_input(&closeFc);
3835-
fc->iformat = nullptr;
3836-
} else if (fc->oformat != nullptr) {
3837+
if (fmtRef->fmtCtx != nullptr) {
3838+
fc = fmtRef->fmtCtx;
38373839
if (fc->pb != nullptr) {
38383840
if (adaptor)
38393841
avio_context_free(&fc->pb);
@@ -3845,20 +3847,26 @@ void formatContextFinalizer(napi_env env, void* data, void* hint) {
38453847
}
38463848
}
38473849
}
3848-
// FIXME this is segfaulting ... why
3849-
/* if (fc->codec_whitelist != nullptr) {
3850-
av_freep(fc->codec_whitelist);
3851-
}
3852-
if (fc->format_whitelist != nullptr) {
3853-
av_freep(fc->format_whitelist);
3854-
}
3855-
if (fc->protocol_whitelist != nullptr) {
3856-
av_freep(fc->protocol_whitelist);
3850+
3851+
if (fc->iformat != nullptr) {
3852+
avformat_close_input(&fc);
3853+
} else {
3854+
// FIXME this is segfaulting ... why
3855+
/* if (fc->codec_whitelist != nullptr) {
3856+
av_freep(fc->codec_whitelist);
3857+
}
3858+
if (fc->format_whitelist != nullptr) {
3859+
av_freep(fc->format_whitelist);
3860+
}
3861+
if (fc->protocol_whitelist != nullptr) {
3862+
av_freep(fc->protocol_whitelist);
3863+
}
3864+
if (fc->protocol_blacklist != nullptr) {
3865+
av_freep(fc->protocol_blacklist);
3866+
} */
38573867
}
3858-
if (fc->protocol_blacklist != nullptr) {
3859-
av_freep(fc->protocol_blacklist);
3860-
} */
3861-
if (!adaptor) // crashes otherwise...
3868+
3869+
if (adaptor != nullptr) // crashes otherwise...
38623870
avformat_free_context(fc);
38633871
}
38643872
}

‎src/format.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ extern "C" {
3333
#include <libavutil/avstring.h>
3434
}
3535

36+
// Indirection required to avoid double delete after demuxer forceClose
37+
struct fmtCtxRef {
38+
AVFormatContext* fmtCtx = nullptr;
39+
};
40+
3641
napi_value muxers(napi_env env, napi_callback_info info);
3742
napi_value demuxers(napi_env env, napi_callback_info info);
3843
napi_value guessFormat(napi_env env, napi_callback_info info);

‎types/Demuxer.d.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ export interface Demuxer extends Omit<FormatContext,
7272
* @returns a promise that resolves to a Packet when the read has completed
7373
*/
7474
read(): Promise<Packet>
75+
/**
76+
* Abandon the demuxing process and forcibly close the file or stream without waiting for it to finish
77+
*/
78+
forceClose(): undefined
7579
}
7680

7781
/**

0 commit comments

Comments
 (0)