Commit 13057ab6 authored by Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen

Allow callbacks to be shared between instances.

This fixes an issue where having a callback set on a specific object
would make an application crash. For instance, having 2 VLC::Instance
objects and calling logSet on one would cause the wrapper
libvlc_instance_t to invoke this callback. If the instance on which
logSet was called gets destroyed, the object wrapping the callback will
be destroyed as well, thus causing the next log callback invocation to
crash.
All callbacks are now shared among objects that wrap the same underlying
libvlc_xxx_t object.
In addition to that, we now ensure the wrapped vlc object gets destroyed
before our callbacks wrappers.
parent 1e25a15a
......@@ -34,7 +34,7 @@
namespace VLC
{
class Instance : public Internal<libvlc_instance_t>, private CallbackOwner<2>
class Instance : protected CallbackOwner<2>, public Internal<libvlc_instance_t>
{
private:
enum class CallbackIdx : unsigned int
......@@ -118,8 +118,8 @@ public:
{
static_assert(signature_match_or_nullptr<ExitCb, void()>::value, "Mismatched exit callback" );
libvlc_set_exit_handler( *this,
CallbackWrapper<(unsigned int)CallbackIdx::Exit, void(*)(void*)>::wrap( this, std::forward<ExitCb>( exitCb ) ),
static_cast<CallbackOwner<2>*>( this ) );
CallbackWrapper<(unsigned int)CallbackIdx::Exit, void(*)(void*)>::wrap( *m_callbacks, std::forward<ExitCb>( exitCb ) ),
m_callbacks.get() );
}
/**
......@@ -208,8 +208,8 @@ public:
logCb(level, ctx, std::string{ buff });
#endif
};
libvlc_log_set(*this, CallbackWrapper<(unsigned int)CallbackIdx::Log, libvlc_log_cb>::wrap(this, std::move(wrapper)),
static_cast<CallbackOwner<2>*>(this));
libvlc_log_set(*this, CallbackWrapper<(unsigned int)CallbackIdx::Log, libvlc_log_cb>::wrap( *m_callbacks, std::move(wrapper)),
m_callbacks.get() );
}
/**
......@@ -324,7 +324,6 @@ public:
res.emplace_back( p );
return res;
}
};
} // namespace VLC
......
......@@ -66,10 +66,10 @@ class Internal
Internal( InternalPtr obj, Releaser releaser )
: m_obj{ obj, releaser }
{
if ( obj == nullptr )
throw std::runtime_error("Wrapping a NULL instance");
m_obj.reset( obj, releaser );
}
Internal(Releaser releaser)
......
......@@ -36,7 +36,7 @@ class MediaEventManager;
class Instance;
class MediaList;
class Media : public Internal<libvlc_media_t>, private CallbackOwner<4>
class Media : protected CallbackOwner<4>, public Internal<libvlc_media_t>
{
private:
enum class CallbackIdx : unsigned int
......@@ -256,17 +256,17 @@ public:
auto ptr = libvlc_media_new_callbacks( instance,
imem::CallbackWrapper<(unsigned int)CallbackIdx::Open, libvlc_media_open_cb>::
wrap<imem::GuessBoxingStrategy<OpenCb, imem::BoxingStrategy::Setup>::Strategy>(
this, std::forward<OpenCb>( openCb ) ),
*m_callbacks, std::forward<OpenCb>( openCb ) ),
imem::CallbackWrapper<(unsigned int)CallbackIdx::Read, libvlc_media_read_cb>::
wrap<imem::GuessBoxingStrategy<OpenCb, imem::BoxingStrategy::Unbox>::Strategy>(
this, std::forward<ReadCb>( readCb ) ),
*m_callbacks, std::forward<ReadCb>( readCb ) ),
imem::CallbackWrapper<(unsigned int)CallbackIdx::Seek, libvlc_media_seek_cb>::
wrap<imem::GuessBoxingStrategy<OpenCb, imem::BoxingStrategy::Unbox>::Strategy>(
this, std::forward<SeekCb>( seekCb ) ),
*m_callbacks, std::forward<SeekCb>( seekCb ) ),
imem::CallbackWrapper<(unsigned int)CallbackIdx::Close, libvlc_media_close_cb>::
wrap<imem::GuessBoxingStrategy<OpenCb, imem::BoxingStrategy::Cleanup>::Strategy>(
this, std::forward<CloseCb>( closeCb ) ),
static_cast<CallbackOwner<NbEvents>*>( this )
*m_callbacks, std::forward<CloseCb>( closeCb ) ),
m_callbacks.get()
);
if ( ptr == nullptr )
throw std::runtime_error( "Failed to create media" );
......
......@@ -44,7 +44,7 @@ class TrackDescription;
///
/// \brief The MediaPlayer class exposes libvlc_media_player_t functionnalities
///
class MediaPlayer : public Internal<libvlc_media_player_t>, private CallbackOwner<13>
class MediaPlayer : private CallbackOwner<13>, public Internal<libvlc_media_player_t>
{
private:
enum class CallbackIdx : unsigned int
......@@ -81,7 +81,7 @@ public:
* \param p_libvlc_instance the libvlc instance in which the Media
* Player should be created.
*/
MediaPlayer(Instance& instance )
MediaPlayer( Instance& instance )
: Internal{ libvlc_media_player_new( instance ), libvlc_media_player_release }
{
}
......@@ -98,7 +98,7 @@ public:
{
}
/**
/**
* Create an empty VLC MediaPlayer instance.
*
* Calling any method on such an instance is undefined.
......@@ -692,14 +692,14 @@ public:
static_assert(signature_match_or_nullptr<DrainCb, void()>::value, "Mismatched drain callback prototype");
libvlc_audio_set_callbacks( *this,
CallbackWrapper<(unsigned int)CallbackIdx::AudioPlay, libvlc_audio_play_cb>::wrap( this, std::forward<PlayCb>( play ) ),
CallbackWrapper<(unsigned int)CallbackIdx::AudioPause, libvlc_audio_pause_cb>::wrap( this, std::forward<PauseCb>( pause ) ),
CallbackWrapper<(unsigned int)CallbackIdx::AudioResume, libvlc_audio_resume_cb>::wrap( this, std::forward<ResumeCb>( resume ) ),
CallbackWrapper<(unsigned int)CallbackIdx::AudioFlush, libvlc_audio_flush_cb>::wrap( this, std::forward<FlushCb>( flush ) ),
CallbackWrapper<(unsigned int)CallbackIdx::AudioDrain, libvlc_audio_drain_cb>::wrap( this, std::forward<DrainCb>( drain ) ),
CallbackWrapper<(unsigned int)CallbackIdx::AudioPlay, libvlc_audio_play_cb>::wrap( *m_callbacks, std::forward<PlayCb>( play ) ),
CallbackWrapper<(unsigned int)CallbackIdx::AudioPause, libvlc_audio_pause_cb>::wrap( *m_callbacks, std::forward<PauseCb>( pause ) ),
CallbackWrapper<(unsigned int)CallbackIdx::AudioResume, libvlc_audio_resume_cb>::wrap( *m_callbacks, std::forward<ResumeCb>( resume ) ),
CallbackWrapper<(unsigned int)CallbackIdx::AudioFlush, libvlc_audio_flush_cb>::wrap( *m_callbacks, std::forward<FlushCb>( flush ) ),
CallbackWrapper<(unsigned int)CallbackIdx::AudioDrain, libvlc_audio_drain_cb>::wrap( *m_callbacks, std::forward<DrainCb>( drain ) ),
// We will receive the pointer as a void*, we need to offset the value *now*, otherwise we'd get
// a shifted value, resulting in an invalid callback array.
static_cast<CallbackOwner<13>*>( this ) );
m_callbacks.get() );
}
/**
......@@ -1034,12 +1034,12 @@ public:
static_assert(signature_match_or_nullptr<DisplayCb, void(void*)>::value, "Mismatched lock callback signature");
libvlc_video_set_callbacks(*this,
CallbackWrapper<(unsigned int)CallbackIdx::VideoLock, libvlc_video_lock_cb>::wrap( this, std::forward<LockCb>( lock ) ),
CallbackWrapper<(unsigned int)CallbackIdx::VideoUnlock, libvlc_video_unlock_cb>::wrap( this, std::forward<UnlockCb>( unlock ) ),
CallbackWrapper<(unsigned int)CallbackIdx::VideoDisplay, libvlc_video_display_cb>::wrap( this, std::forward<DisplayCb>( display ) ),
CallbackWrapper<(unsigned int)CallbackIdx::VideoLock, libvlc_video_lock_cb>::wrap( *m_callbacks, std::forward<LockCb>( lock ) ),
CallbackWrapper<(unsigned int)CallbackIdx::VideoUnlock, libvlc_video_unlock_cb>::wrap( *m_callbacks, std::forward<UnlockCb>( unlock ) ),
CallbackWrapper<(unsigned int)CallbackIdx::VideoDisplay, libvlc_video_display_cb>::wrap( *m_callbacks, std::forward<DisplayCb>( display ) ),
// We will receive the pointer as a void*, we need to offset the value *now*, otherwise we'd get
// a shifted value, resulting in an empty callback array.
static_cast<CallbackOwner<13>*>( this ) );
m_callbacks.get() );
}
/**
......@@ -1088,8 +1088,8 @@ public:
static_assert(signature_match_or_nullptr<CleanupCb, void()>::value, "Unmatched prototype for cleanup callback");
libvlc_video_set_format_callbacks(*this,
CallbackWrapper<(unsigned int)CallbackIdx::VideoFormat, libvlc_video_format_cb>::wrap( static_cast<CallbackOwner<13>*>( this ), std::forward<FormatCb>( setup ) ),
CallbackWrapper<(unsigned int)CallbackIdx::VideoCleanup, libvlc_video_cleanup_cb>::wrap( this, std::forward<CleanupCb>( cleanup ) ) );
CallbackWrapper<(unsigned int)CallbackIdx::VideoFormat, libvlc_video_format_cb>::wrap( *m_callbacks, std::forward<FormatCb>( setup ) ),
CallbackWrapper<(unsigned int)CallbackIdx::VideoCleanup, libvlc_video_cleanup_cb>::wrap( *m_callbacks, std::forward<CleanupCb>( cleanup ) ) );
}
/**
......
......@@ -108,37 +108,48 @@ namespace VLC
Func func;
};
template <int NbEvent>
struct CallbackOwner
template <size_t NbEvent>
using CallbackArray = std::array<std::shared_ptr<CallbackHandlerBase>, NbEvent>;
///
/// Utility class that contains a shared pointer to a callback array.
/// We use a shared_ptr to allow multiple instances to share the same callback array
/// This must be inherited before the Internal<T> type, to ensure it gets deleted
/// after the wrapped libvlc object.
///
template <size_t NbEvent>
class CallbackOwner
{
std::array<std::shared_ptr<CallbackHandlerBase>, NbEvent> callbacks;
protected:
CallbackOwner() = default;
CallbackOwner()
: m_callbacks( std::make_shared<CallbackArray<NbEvent>>() )
{
}
std::shared_ptr<CallbackArray<NbEvent>> m_callbacks;
};
template <int, typename>
template <size_t, typename>
struct FromOpaque;
template <int NbEvents>
template <size_t NbEvents>
struct FromOpaque<NbEvents, void*>
{
static CallbackOwner<NbEvents>* get(void* opaque)
static CallbackArray<NbEvents>& get( void* opaque )
{
return reinterpret_cast<CallbackOwner<NbEvents>*>( opaque );
return *reinterpret_cast<CallbackArray<NbEvents>*>( opaque );
}
};
template <int NbEvents>
template <size_t NbEvents>
struct FromOpaque<NbEvents, void**>
{
static CallbackOwner<NbEvents>* get(void** opaque)
static CallbackArray<NbEvents>& get(void** opaque)
{
return reinterpret_cast<CallbackOwner<NbEvents>*>( *opaque );
return *reinterpret_cast<CallbackArray<NbEvents>*>( *opaque );
}
};
template <int Idx, typename... Args>
template <size_t Idx, typename... Args>
struct CallbackWrapper;
// We assume that any callback will take a void*/void** opaque as its first parameter.
......@@ -147,19 +158,19 @@ namespace VLC
// parameters.
// Using partial specialization also allows us to get the list of the expected
// callback parameters automatically, rather than having to specify them.
template <int Idx, typename Ret, typename Opaque, typename... Args>
template <size_t Idx, typename Ret, typename Opaque, typename... Args>
struct CallbackWrapper<Idx, Ret(*)(Opaque, Args...)>
{
using Wrapped = Ret(*)(Opaque, Args...);
template <int NbEvents, typename Func>
static Wrapped wrap(CallbackOwner<NbEvents>* owner, Func&& func)
template <size_t NbEvents, typename Func>
static Wrapped wrap(CallbackArray<NbEvents>& callbacks, Func&& func)
{
owner->callbacks[Idx] = std::make_shared<CallbackHandler<Func>>( std::forward<Func>( func ) );
callbacks[Idx] = std::make_shared<CallbackHandler<Func>>( std::forward<Func>( func ) );
return [](Opaque opaque, Args... args) -> Ret {
auto self = FromOpaque<NbEvents, Opaque>::get( opaque );
assert(self->callbacks[Idx] != nullptr);
auto cbHandler = static_cast<CallbackHandler<Func>*>( self->callbacks[Idx].get() );
auto& callbacks = FromOpaque<NbEvents, Opaque>::get( opaque );
assert(callbacks[Idx] != nullptr);
auto cbHandler = static_cast<CallbackHandler<Func>*>( callbacks[Idx].get() );
return cbHandler->func( std::forward<Args>(args)... );
};
}
......@@ -169,8 +180,8 @@ namespace VLC
// since Func is a template type, which roughly has to satisfy the "Callable" concept,
// it could be an instance of a function object, which doesn't compare nicely against nullptr.
// Using the specialization at build time is easier and performs better.
template <int NbEvents>
static std::nullptr_t wrap(CallbackOwner<NbEvents>*, std::nullptr_t)
template <size_t NbEvents>
static std::nullptr_t wrap(CallbackArray<NbEvents>&, std::nullptr_t)
{
return nullptr;
}
......@@ -202,7 +213,7 @@ namespace VLC
// Instead of always receiving the initialy provided void* opaque, the open
// callback is responsible for creating another opaque value, that will be
// passed to read/seek/close.
// Since we use the opaque value to pass a CallbackOwner instance, we need
// Since we use the opaque value to pass a CallbackArray instance, we need
// a way to keep the user's opaque value, and replace it by our own.
// The Opaque type is a small boxing helper, that will be used for this purpose.
// In case the OpenCallback is a nullptr, there is no need for boxing, since
......@@ -211,7 +222,7 @@ namespace VLC
template <int NbEvent>
struct Opaque
{
CallbackOwner<NbEvent>* owner;
CallbackArray<NbEvent>* callbacks;
void* userOpaque;
};
......@@ -221,7 +232,7 @@ namespace VLC
NoBoxing,
/// Used to create the Opaque wrapper and setup pointers
Setup,
/// Unbox CallbackOwner/user callback pointers
/// Unbox CallbackArray/user callback pointers
Unbox,
/// Releases the Opaque, created during Setup
Cleanup,
......@@ -239,7 +250,7 @@ namespace VLC
};
// In case the user provides a nullptr open callback, there's nothing
// to box, as we get the original opaque (which is our CallbackOwner*)
// to box, as we get the original opaque (which is our CallbackArray*)
template <BoxingStrategy Strategy_>
struct GuessBoxingStrategy<std::nullptr_t, Strategy_>
{
......@@ -250,12 +261,12 @@ namespace VLC
#endif
};
template <int NbEvents, BoxingStrategy Strategy>
template <size_t NbEvents, BoxingStrategy Strategy>
struct BoxOpaque;
// This specialization is our base case. It unboxes an Opaque pointer
// to a CallbackOwner, of a user provided opaque.
template <int NbEvents>
// to a CallbackArray, of a user provided opaque.
template <size_t NbEvents>
struct BoxOpaque<NbEvents, BoxingStrategy::Unbox>
{
template <typename... Args>
......@@ -263,9 +274,7 @@ namespace VLC
// Assume the cast operator will be used when calling the callback.
// In this case, decay the boxed type to the user provided value
operator void*() { return m_ptr->userOpaque; }
// Assume the dereferencing operator to be used by our wrappers. In
// this case, we only care about our types
CallbackOwner<NbEvents>* operator ->() { return m_ptr->owner; }
CallbackArray<NbEvents>& callbacks() { return *(m_ptr->callbacks); }
Opaque<NbEvents>* m_ptr;
};
......@@ -277,7 +286,7 @@ namespace VLC
// This makes us receive all parameters so we can extract the void** param.
// As a drawback, all other overloads have to receive those parameters, and
// ignore them.
template <int NbEvents>
template <size_t NbEvents>
struct BoxOpaque<NbEvents, BoxingStrategy::Setup>
: public BoxOpaque<NbEvents, BoxingStrategy::Unbox>
{
......@@ -287,7 +296,7 @@ namespace VLC
: Base( new Opaque<NbEvents> )
, m_userOpaque( userOpaque )
{
Base::m_ptr->owner = reinterpret_cast<CallbackOwner<NbEvents>*>( ptr );
Base::m_ptr->callbacks = reinterpret_cast<CallbackArray<NbEvents>*>( ptr );
}
operator void*() { return Base::m_ptr; }
......@@ -304,7 +313,7 @@ namespace VLC
// This is a special case which is only used to delete the Opaque type,
// created by the BoxingStrategy::Setup specialization
template <int NbEvents>
template <size_t NbEvents>
struct BoxOpaque<NbEvents, BoxingStrategy::Cleanup>
: public BoxOpaque<NbEvents, BoxingStrategy::Unbox>
{
......@@ -317,18 +326,18 @@ namespace VLC
// When no boxing is required, enfore the user provided callback as a nullptr.
// Otherwise, we assume there is no Opaque wrapper, and therefore the
// pointer we receive already is our CallbackOwner instance.
template <int NbEvents>
template <size_t NbEvents>
struct BoxOpaque<NbEvents, BoxingStrategy::NoBoxing>
{
template <typename... Args>
BoxOpaque(void* ptr, Args...) : m_ptr( reinterpret_cast<CallbackOwner<NbEvents>*>( ptr ) ) {}
BoxOpaque(void* ptr, Args...) : m_ptr( reinterpret_cast<CallbackArray<NbEvents>*>( ptr ) ) {}
operator void*() { return nullptr; }
CallbackOwner<NbEvents>* operator->(){ return m_ptr; }
CallbackArray<NbEvents>& callbacks() { return *m_ptr; }
CallbackOwner<NbEvents>* m_ptr;
CallbackArray<NbEvents>* m_ptr;
};
template <int Idx, typename... Args>
template <size_t Idx, typename... Args>
struct CallbackWrapper;
// Reimplement the general case CallbackWrapper for imem specific purpose.
......@@ -346,25 +355,25 @@ namespace VLC
// with lambdas, as we would lose the ability to detect when nullptr is provided,
// so we'd have VLC call a no-op function, while the nullptr wrap() specialization
// takes care of this.
template <int Idx, typename Ret, typename... Args>
template <size_t Idx, typename Ret, typename... Args>
struct CallbackWrapper<Idx, Ret(*)(void*, Args...)>
{
using Wrapped = Ret(*)(void*, Args...);
template <BoxingStrategy Strategy, int NbEvents, typename Func>
static Wrapped wrap(CallbackOwner<NbEvents>* owner, Func&& func)
template <BoxingStrategy Strategy, size_t NbEvents, typename Func>
static Wrapped wrap(CallbackArray<NbEvents>& callbacks, Func&& func)
{
owner->callbacks[Idx] = std::make_shared<CallbackHandler<Func>>( std::forward<Func>( func ) );
callbacks[Idx] = std::make_shared<CallbackHandler<Func>>( std::forward<Func>( func ) );
return [](void* opaque, Args... args) -> Ret {
auto boxed = BoxOpaque<NbEvents, Strategy>( opaque, std::forward<Args>( args )... );
assert(boxed->callbacks[Idx] != nullptr );
auto cbHandler = static_cast<CallbackHandler<Func>*>( boxed->callbacks[Idx].get() );
assert(boxed.callbacks()[Idx] != nullptr );
auto cbHandler = static_cast<CallbackHandler<Func>*>( boxed.callbacks()[Idx].get() );
return cbHandler->func( boxed, std::forward<Args>(args)... );
};
}
template <BoxingStrategy Strategy, int NbEvents>
static std::nullptr_t wrap(CallbackOwner<NbEvents>*, std::nullptr_t)
template <BoxingStrategy Strategy, size_t NbEvents>
static std::nullptr_t wrap(CallbackArray<NbEvents>&, std::nullptr_t)
{
return nullptr;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment