From bc8783869e3254a3a37c7b3b770c640a5c09f936 Mon Sep 17 00:00:00 2001 From: Bart van Strien Date: Sun, 19 Feb 2017 16:22:00 +0100 Subject: [PATCH 1/2] Improved seeking support, especially for short video files Support inter-page seeking, in case there are multiple packets per page. Also support seeking in single-page files. --- changes.txt | 1 + src/modules/video/theora/VideoStream.cpp | 67 ++++++++++++++++++------ 2 files changed, 53 insertions(+), 15 deletions(-) diff --git a/changes.txt b/changes.txt index e410f4ee7..849ef40b8 100644 --- a/changes.txt +++ b/changes.txt @@ -10,6 +10,7 @@ Released: N/A * Fixed BezierCurves to error instead of hanging in some situations. * Improved command line argument handling. + * Improved seeking support, especially for short video files. * Updated the default error handler to allow copying the error to the clipboard when the user decides to do so. * Updated love.filesystem.setRequirePath to support multiple template '?' characters in each path. diff --git a/src/modules/video/theora/VideoStream.cpp b/src/modules/video/theora/VideoStream.cpp index 4d3d3354a..16714793a 100644 --- a/src/modules/video/theora/VideoStream.cpp +++ b/src/modules/video/theora/VideoStream.cpp @@ -270,7 +270,10 @@ void VideoStream::rewind() void VideoStream::seekDecoder(double target) { - if (target < 0.01) + const double rewindThreshold = 0.01; + const double seekThreshold = 0.0001; + + if (target < rewindThreshold) { rewind(); return; @@ -279,7 +282,7 @@ void VideoStream::seekDecoder(double target) double low = 0; double high = file->getSize(); - while (high-low > 0.0001) + while (high-low > rewindThreshold) { // Determine our next binary search position double pos = (high+low)/2; @@ -289,23 +292,57 @@ void VideoStream::seekDecoder(double target) ogg_sync_reset(&sync); ogg_sync_pageseek(&sync, &page); - // Read a packet + // Read a page readPacket(false); if (eos) - return; - - // Determine if this is the right place - double curTime = th_granule_time(decoder, packet.granulepos); - double nextTime = th_granule_time(decoder, packet.granulepos+1); - - if (curTime == -1) - continue; // Invalid granule position (magic?) - else if (curTime <= target && nextTime > target) - break; // the current frame should be displaying right now - else if (curTime > target) + { + // EOS, so we're definitely past our target (or the target is past + // the end) high = pos; - else + eos = false; + + // And a workaround for single-page files: + if (high < rewindThreshold) + rewind(); + else + continue; + } + + // Now search all packets in this page + int result = -1; + for (int i = 0; i < ogg_page_packets(&page); ++i) + { + if (i > 0) + readPacket(true); + + // Determine if this is the right place + double curTime = th_granule_time(decoder, packet.granulepos); + double nextTime = th_granule_time(decoder, packet.granulepos+1); + + if (curTime == -1) + continue; // Invalid granule position (magic?) + else if (curTime <= target && nextTime > target) + { + // the current frame should be displaying right now + result = 0; + break; + } + else if (curTime > target) + { + // No need to check the other packets, they're all past + // this one + result = 1; + break; + } + } + + // The sign of result determines the direction + if (result == 0) + break; + else if (result < 0) low = pos; + else + high = pos; } // Now update theora and our decoder on this new position of ours From 6b7172bd40a0401d778973da3b242c6d0b95e578 Mon Sep 17 00:00:00 2001 From: raidho36 Date: Fri, 14 Apr 2017 05:53:36 +0300 Subject: [PATCH 2/2] Fix Issue #1273 Added Lua index registry clear routine to `destroy` method of classes that use it. If this is left to GC, the abandoned userdata would stay in memory for one full extra cycle. Prettified the code slightly. Clarified some comments. Refactored `Fixture` class `udata` name. Removed Reference class include from `Shape` (it doesn't use it). --- src/common/Reference.cpp | 2 +- src/common/runtime.cpp | 2 +- src/modules/physics/box2d/Body.cpp | 16 +++++++++--- src/modules/physics/box2d/Body.h | 2 +- src/modules/physics/box2d/Fixture.cpp | 37 +++++++++++++++++++-------- src/modules/physics/box2d/Fixture.h | 2 +- src/modules/physics/box2d/Joint.cpp | 23 ++++++++++++++--- src/modules/physics/box2d/Shape.h | 1 - src/modules/physics/box2d/World.cpp | 8 ++++++ 9 files changed, 71 insertions(+), 22 deletions(-) diff --git a/src/common/Reference.cpp b/src/common/Reference.cpp index 6fa34e077..6a50d7e98 100644 --- a/src/common/Reference.cpp +++ b/src/common/Reference.cpp @@ -45,7 +45,7 @@ Reference::~Reference() void Reference::ref(lua_State *L) { - unref(); // Just to be safe. + unref(); // Previously created reference needs to be cleared pinnedL = luax_getpinnedthread(L); luax_insist(L, LUA_REGISTRYINDEX, REFERENCE_TABLE_NAME); lua_insert(L, -2); // Move reference table behind value. diff --git a/src/common/runtime.cpp b/src/common/runtime.cpp index f6ecdebf3..2c505fef0 100644 --- a/src/common/runtime.cpp +++ b/src/common/runtime.cpp @@ -83,7 +83,7 @@ Reference *luax_refif(lua_State *L, int type) // Create a reference only if the test succeeds. if (lua_type(L, -1) == type) r = new Reference(L); - else // Pop the value even if it fails (but also if it succeeds). + else // Pop the value manually if it fails (done by Reference if it succeeds). lua_pop(L, 1); return r; diff --git a/src/modules/physics/box2d/Body.cpp b/src/modules/physics/box2d/Body.cpp index c9dad71c1..e3b256e16 100644 --- a/src/modules/physics/box2d/Body.cpp +++ b/src/modules/physics/box2d/Body.cpp @@ -67,8 +67,12 @@ Body::Body(b2Body *b) Body::~Body() { - if (udata != nullptr) + if (!udata) + return; + + if (udata->ref) delete udata->ref; + delete udata; } @@ -521,6 +525,10 @@ void Body::destroy() Memoizer::remove(body); body = NULL; + // Remove userdata reference to avoid it sticking around after GC + if (udata && udata->ref) + udata->ref->unref(); + // Box2D body destroyed. Release its reference to the love Body. this->release(); } @@ -535,8 +543,10 @@ int Body::setUserData(lua_State *L) body->SetUserData((void *) udata); } - delete udata->ref; - udata->ref = new Reference(L); + if(!udata->ref) + udata->ref = new Reference(); + + udata->ref->ref(L); return 0; } diff --git a/src/modules/physics/box2d/Body.h b/src/modules/physics/box2d/Body.h index 8803eff11..567c53974 100644 --- a/src/modules/physics/box2d/Body.h +++ b/src/modules/physics/box2d/Body.h @@ -69,7 +69,7 @@ public: friend class Shape; friend class Fixture; - // The Box2D body. (Should not be public?) + // Public because joints et al ask for b2body b2Body *body; /** diff --git a/src/modules/physics/box2d/Fixture.cpp b/src/modules/physics/box2d/Fixture.cpp index 7f0b31e28..89668de82 100644 --- a/src/modules/physics/box2d/Fixture.cpp +++ b/src/modules/physics/box2d/Fixture.cpp @@ -41,11 +41,11 @@ Fixture::Fixture(Body *body, Shape *shape, float density) : body(body) , fixture(nullptr) { - data = new fixtureudata(); - data->ref = nullptr; + udata = new fixtureudata(); + udata->ref = nullptr; b2FixtureDef def; def.shape = shape->shape; - def.userData = (void *)data; + def.userData = (void *)udata; def.density = density; fixture = body->body->CreateFixture(&def); this->retain(); @@ -55,7 +55,7 @@ Fixture::Fixture(Body *body, Shape *shape, float density) Fixture::Fixture(b2Fixture *f) : fixture(f) { - data = (fixtureudata *)f->GetUserData(); + udata = (fixtureudata *)f->GetUserData(); body = (Body *)Memoizer::find(f->GetBody()); if (!body) body = new Body(f->GetBody()); @@ -65,10 +65,13 @@ Fixture::Fixture(b2Fixture *f) Fixture::~Fixture() { - if (data != nullptr) - delete data->ref; + if (!udata) + return; - delete data; + if (udata->ref) + delete udata->ref; + + delete udata; } Shape::Type Fixture::getType() const @@ -239,16 +242,24 @@ int Fixture::setUserData(lua_State *L) { love::luax_assert_argc(L, 1, 1); - delete data->ref; - data->ref = new Reference(L); + if (udata == nullptr) + { + udata = new fixtureudata(); + fixture->SetUserData((void *) udata); + } + + if(!udata->ref) + udata->ref = new Reference(); + + udata->ref->ref(L); return 0; } int Fixture::getUserData(lua_State *L) { - if (data->ref != nullptr) - data->ref->push(L); + if (udata->ref != nullptr) + udata->ref->push(L); else lua_pushnil(L); @@ -321,6 +332,10 @@ void Fixture::destroy(bool implicit) Memoizer::remove(fixture); fixture = nullptr; + // Remove userdata reference to avoid it sticking around after GC + if (udata && udata->ref) + udata->ref->unref(); + // Box2D fixture destroyed. Release its reference to the love Fixture. this->release(); } diff --git a/src/modules/physics/box2d/Fixture.h b/src/modules/physics/box2d/Fixture.h index 278befa00..fa001901f 100644 --- a/src/modules/physics/box2d/Fixture.h +++ b/src/modules/physics/box2d/Fixture.h @@ -211,7 +211,7 @@ public: protected: Body *body; - fixtureudata *data; + fixtureudata *udata; b2Fixture *fixture; }; diff --git a/src/modules/physics/box2d/Joint.cpp b/src/modules/physics/box2d/Joint.cpp index b5e5fdcff..6a3727bb9 100644 --- a/src/modules/physics/box2d/Joint.cpp +++ b/src/modules/physics/box2d/Joint.cpp @@ -61,8 +61,12 @@ Joint::Joint(Body *body1, Body *body2) Joint::~Joint() { - if (udata != nullptr) + if (!udata) + return; + + if (udata->ref) delete udata->ref; + delete udata; } @@ -175,6 +179,11 @@ void Joint::destroyJoint(bool implicit) world->world->DestroyJoint(joint); Memoizer::remove(joint); joint = NULL; + + // Remove userdata reference to avoid it sticking around after GC + if (udata && udata->ref) + udata->ref->unref(); + // Release the reference of the Box2D joint. this->release(); } @@ -193,8 +202,16 @@ int Joint::setUserData(lua_State *L) { love::luax_assert_argc(L, 1, 1); - delete udata->ref; - udata->ref = new Reference(L); + if (udata == nullptr) + { + udata = new jointudata(); + joint->SetUserData((void *) udata); + } + + if(!udata->ref) + udata->ref = new Reference(); + + udata->ref->ref(L); return 0; } diff --git a/src/modules/physics/box2d/Shape.h b/src/modules/physics/box2d/Shape.h index c5a3db249..5a187cbd0 100644 --- a/src/modules/physics/box2d/Shape.h +++ b/src/modules/physics/box2d/Shape.h @@ -24,7 +24,6 @@ // LOVE #include "physics/Shape.h" #include "physics/box2d/Body.h" -#include "common/Reference.h" // Box2D #include diff --git a/src/modules/physics/box2d/World.cpp b/src/modules/physics/box2d/World.cpp index 92ff802d4..8993cf39e 100644 --- a/src/modules/physics/box2d/World.cpp +++ b/src/modules/physics/box2d/World.cpp @@ -577,6 +577,14 @@ void World::destroy() world->DestroyBody(groundBody); Memoizer::remove(world); + + // Remove userdata reference to avoid it sticking around after GC + if (begin.ref) begin.ref->unref(); + if (end.ref) end.ref->unref(); + if (presolve.ref) presolve.ref->unref(); + if (postsolve.ref) postsolve.ref->unref(); + if (filter.ref) filter.ref->unref(); + delete world; world = nullptr; }