Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing media_new_location definition #261

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Bi0T1N
Copy link

@Bi0T1N Bi0T1N commented Sep 30, 2023

Looks like it was added manually but only to the 3.0 file. However, it's mentioned in every definition of media_new.

@oaubert
Copy link
Owner

oaubert commented Oct 1, 2023

Thanks for the patch. In fact, the situation is a bit different. The signature for libvlc_media_new_location changed in the dev version: it was previously (in 2.2 and 3.0) libvlc_media_new_location(libvlc_instance_t *p_instance, const char * psz_mrl ) with a libvlc_instance as first parameter. It was then properly automatically wrapped in the pythonic conversion to a vlc.Instance method. In the dev version the first parameter (libvlc_instance_t) has been removed, which makes it not anymore a default vlc.Instance method. The signature is now libvlc_media_new_location(const char * psz_mrl);
The question is now: should we try to maintain backward compatibility (which required having a vlc.Instance just to instanciate a media), or should we track the API principle of decoupling what can be decoupled.

@Bi0T1N
Copy link
Author

Bi0T1N commented Oct 3, 2023

Alright but you still need to create an Instance before you can do anything useful:

Before it can do anything useful, LibVLC must be initialized.

You can create one (or more) instance(s) of LibVLC in a given process, with libvlc_new() and destroy them with libvlc_release().

So I assume that creating a media via libvlc_media_new_location and trying to play it will result in an error.
Therefore calling it from vlc.Instance is probably the better pythonic approach, no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants