Draft: Adapt bindings to the LibVLC 4.0 rework
Adapt libvlcpp bindings to the LibVLC 4.0 rework (vlc!3503)
Overview of changes:
-
Callback event handlers now have their own class. Users can create an instance of this class by passing the mandatory callbacks as arguments, while optional callbacks can be assigned through the class methods. This provides flexibility for users to assign optional event handlers as needed.
-
The
EventManager
has been removed as part of the rework. All events are now handled directly by their respective binding'sCallbacks
class. -
New
Thumbnailer
andParser
classes have been introduced for the bindings of their respective APIs. -
All API calls have been updated in accordance with the rework.
TODO:
-
A wrapper function is still pending implementation. This function will be used during callback assignment, accepting raw libvlc struct objects from LibVLC and creating corresponding libvlcpp wrapper objects before exposing them to the user.
Example: In theon_update
callback withinlibvlc_media_player_watch_time_cbs
, the goal is to accept alibvlc_media_player_time_point_t*
object from LibVLC, create aMediaPlayer::TimePoint
object, and expose it to the user, allowing further use of its methods for thelibvlc_media_player_time_point_*
APIs. -
The LIBVLC_VERSION
checks need to be removed. As discussed, the rework will not maintain backward compatibility with LibVLC 3.0 or earlier, due primarily to the removal ofEventManager
. -
Documentation for MediaPlayerCallbacks
needs to be added. -
Tests using the updated API bindings need to be written.
Note: I'll complete the last three tasks after the code has been reviewed and finalised.
Merge request reports
Activity
2079 int interpolate( int64_t system_now_us, int64_t *out_ts_us, double *out_pos ) 2080 { 2081 return libvlc_media_player_time_point_interpolate( *this, system_now_us, 2082 out_ts_us, out_pos ); 2083 } 2084 2085 /** 2086 * Get the date of the next interval 2087 * 2088 * \param system_now_us same system date used by interpolate() 2089 * \param interpolated_ts_us ts returned by interpolate() 2090 * \param next_interval_us next interval, in us 2091 * \return the absolute system date, in us, of the next interval 2092 * \version LibVLC 4.0.0 or later 2093 */ 2094 int64_t getNextDate(int64_t system_now_us, int64_t interpolated_ts_us, changed this line in version 2 of the diff
- Resolved by Ayush Dey
- Resolved by Ayush Dey
2156 */ 2157 template <typename OnSeek> 2158 void setOnSeek(OnSeek&& onSeek) 2159 { 2160 static_assert(signature_match_or_nullptr<OnSeek, ExpectedOnSeek>::value, 2161 "Mismatched on seek callback prototype"); 2162 cbs.on_seek = CallbackWrapper<(unsigned int)CallbackIdx::TimeSeek, 2163 void(*)(void*, const libvlc_media_player_time_point_t*)>:: 2164 wrap(*m_callbacks, std::forward<OnSeek>(onSeek)); 2165 } 2166 2167 /** Returns a void pointer to the array of callbacks. */ 2168 void* getCallbacksPtr() 2169 { 2170 return m_callbacks.get(); 2171 } - Resolved by Ayush Dey
- Resolved by Ayush Dey
- Resolved by Ayush Dey
- vlcpp/Thumbnailer.hpp 0 → 100644
142 */ 143 void destroy() 144 { 145 libvlc_thumbnailer_request_destroy( *this ); 146 } 147 148 /** 149 * Set position for the thumbnailer request 150 * 151 * \param pos The position at which the thumbnail should be generated 152 * \param fast_seek true for fast seek, false for precise seek 153 */ 154 void setPosition( double pos, bool fast_seek ) 155 { 156 libvlc_thumbnailer_request_set_position( *this, pos, fast_seek ); 157 } - Comment on lines +154 to +157
@tguillem I'm not exactly sure about that but it seems that requests should be modified like that only before queueing? If that's the case, Maybe a request builder that only gives access to a unique_ptr would better represent the lifetime of the request objects.
Edited by Alaric Senat Can we use a bool flag
isQueued
in the Request class? We could set this flag when the request is being queued and check it in the setter methods to determine whether the request has already been queued before proceeding with the setter operation.While unique_ptr can be used, I can't understand how ownership can be transferred back in the callback (since the callback uses the same request object)
But the result callback uses the same request as an identifier, so I don't know it its work here. We can adapt the C API if needed.
It feels like the callback gives back the request object only for the target media to be available. We could just give access only to the media reference in the libvlc callback?
I spent some time trying to have a satisfying ownership based solution for this issue. The thumbnailer is supposed to own the request except in case of cancellation, which make the wrapper awkward to write here.
If we keep the libvlc API as-is, I suggest consuming the request in
queue()
and returning a special object containing a weak reference onrequest_t
that would crash if called after the event completion (just like the C API)...class QueuedRequest { public: QueuedRequest(libvlc_thumbnailer_request_t&); void cancel() {/* ... destroy here */ } private: libvlc_thumbnailer_request_t &weak; }; QueuedRequest queue( Request&& req );
@tguillem To be fair, the
QueuedRequest
or an opaque handle dedicated to cancellation on the request object could be implemented in libvlc. It makes the request only cancelable and would avoid users trying to modify it while being processed. Making it an actual safe to use weak reference, failing or doing nothing when the request is completed.Edited by Alaric SenatSo, we are talking about the following:
LIBVLC_API libvlc_thumbnailer_request_id_t libvlc_thumbnailer_queue( libvlc_thumbnailer_t *thumbnailer, libvlc_thumbnailer_request_t *req );
libvlc_thumbnailer_queue
take ownership of the request and return an opaque handle used only to cancel it ?For the implementation, libvlc_thumbnailer_queue will return the same pointer to req (but the user doesn't have to know it). The request will either be destroyed internally by the ended callback, or if the user cancel it.
The thing is, I don't know how you could protect from accidental double frees from the user by returning the same pointer.
The scenario causing a double free right now is:- user queues a request
- the request complete fast and gets destroyed automatically
- user accidentally calls destroy on the dangling request handle
If you think that it just shouldn't happen, I'm fine with having the same pointer as the handler.
Edit: I just remembered that the request is refcounted, so my point above might be not possible by design.
Edited by Alaric SenatAfter some discussion with @chub and @asenat I will try the following:
LIBVLC_API unsigned long libvlc_thumbnailer_queue( libvlc_thumbnailer_t *thumbnailer, libvlc_thumbnailer_request_t *req );
libvlc_thumbnailer_queue
takes ownership oflibvlc_thumbnailer_request_t
, it returns a unique integer incremented for each request, that can be used to cancel a request.
added 27 commits
- 5c09cbdd...2a1132b0 - 17 earlier commits
- c4ad49c9 - MediaPlayer: use a class MediaPlayerCallbacks for callback handling
- 661f9f73 - structures: accept const libvlc_chapter_description_t* in ChapterDescription constructor
- 41739144 - MediaPlayer: expose structs from on_chapter_selection_changed callback
- cb72dd55 - MediaListPlayer: remove MediaListPlayerEventManager class
- 49f906ae - Media: remove MediaEventManager class
- 3efbccca - EventManager: remove class EventManager
- e20dfa14 - MediaPlayer: add on_media_attachments_added callback
- 376831b6 - MediaTrack: rename libvlc_media_track_hold to retain
- c576d906 - RendererDiscoverer: rename libvlc_renderer_item_hold to retain
- 74189740 - test: adapt examples and add thumbnailer test
Toggle commit list- Resolved by Ayush Dey
- Resolved by Thomas Guillem
- Resolved by Ayush Dey
One question:
Should the C++ callbacks/class be made static like the libvlc C is doing ? Is that possible or wanted ?
mentioned in merge request vlc!6183 (merged)
added 28 commits
- 74189740...82c7d442 - 18 earlier commits
- 0e85e58d - MediaPlayer: use a class MediaPlayerCallbacks for callback handling
- 92ef7437 - structures: accept const libvlc_chapter_description_t* in ChapterDescription constructor
- f2e364a7 - MediaPlayer: expose structs from on_chapter_selection_changed callback
- 5a1e1463 - MediaListPlayer: remove MediaListPlayerEventManager class
- a8e605f9 - Media: remove MediaEventManager class
- 8389288b - EventManager: remove class EventManager
- 56ab1bb3 - MediaPlayer: add on_media_attachments_added callback
- b15e1eff - MediaTrack: rename libvlc_media_track_hold to retain
- f462b7f6 - RendererDiscoverer: rename libvlc_renderer_item_hold to retain
- b5889806 - test: adapt examples and add thumbnailer test
Toggle commit list