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

multiple external-file not supported? #34

Closed
za4410 opened this issue Jul 3, 2017 · 6 comments
Closed

multiple external-file not supported? #34

za4410 opened this issue Jul 3, 2017 · 6 comments

Comments

@za4410
Copy link

za4410 commented Jul 3, 2017

While we can pass multiple external-file arguments to the mpv itself like this:

mpv 1.mov --external-file=2.mov --external-file=3.mov --lavfi-complex="[vid1][vid2][vid3]..."

There is no visible way to do it with this app. I also tried the following:

player["external-file"] = ["2.mov","3.mov"]

which does not throw an error, but won't take the inputs either.

@jaseg
Copy link
Owner

jaseg commented Jul 3, 2017

This is interesting! I did not even know it could do that :D

This actually was not possible before since there was no support for setting node-typed options, which would be required to set any option to a list of file names. I pushed a fix to master. Please try it and tell me what you think. If it works fine I'll code up a few tests and publish a new version on pypi.

@za4410
Copy link
Author

za4410 commented Jul 3, 2017

Beautiful! works great! Thanks!

@jaseg
Copy link
Owner

jaseg commented Jul 3, 2017

Awesome. I have pushed version 0.2.4 adding this change to pypi.

@jaseg jaseg closed this as completed Jul 3, 2017
@McSinyx
Copy link
Contributor

McSinyx commented Jul 4, 2017

The PyPI and Github versions are mismatched:

  • diff mpv.py:

    49c49
    <     backend = CDLL('/home/user/mpv/build/libmpv.so')
    ---
    >     backend = CDLL(sofile) #'/home/user/mpv/build/libmpv.so')

    This causes import failure:

    >>> import mpv
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/home/cnx/.local/lib/python3.5/site-packages/mpv.py", line 49, in <module>
        backend = CDLL('/home/user/mpv/build/libmpv.so')
      File "/usr/lib/python3.5/ctypes/__init__.py", line 351, in __init__
        self._handle = _dlopen(self._name, mode)
    OSError: /home/user/mpv/build/libmpv.so: cannot open shared object file: No such file or directory
  • diff setup.py:

    6c6
    <     version = '0.2.4',
    ---
    >     version = '0.2.3',

BTW about the setup.py, I suggest to use distutils because there isn't any setuptools's feature used (and setuptools is a 3rd party library, so user have to install it to run ./setup.py install). Also setuptools.find_packages is not used and the parenthesis in line 8 are meaningless. As you already used your real name in mpv.py copyright info, can you change line 10? And there is a space before closing square bracket in line 28 (column 50).

Furthermore, do you think distributing using wheel (/.setup.py bdist_wheel instead of ./setup.py sdist) is a nice idea? It's smaller to download and faster to install, and python-mpv is in pure Python.

Edit: setuptools provide Python version checker (e.g. python_requires='>=3.5' as setup argument)

@jaseg
Copy link
Owner

jaseg commented Jul 5, 2017

Argh! Yes, I'm an idiot and accidentially pushed test code to pypi -.- Well, there is a 0.2.5 now up there with that fuckup corrected.

Thanks for the comments wrt. setup.py. I have pushed a fixed setup.py to master.

I'm using my legal name only in the copyright notices since that's legal stuff, but generally everyone only knows me by my alias so I'd like to keep that in the setup.py's author field.

As for setuptools vs. distutils, the doc and this guide say the latter is not supposed to be used directly anymore and everybody has setuptools anyway so I'd rather lean towards setuptools here.

I don't see much of a point of using wheel in addition to setuptools, but if there is a way to publish wheels while maintaining backwards compatibility for people not using wheel I'd be fine with using that. I'll have a closer look into that.

@McSinyx
Copy link
Contributor

McSinyx commented Jul 6, 2017

Thanks a lot for the fix, I have a project uses yours as dependency and fetches from PyPI, so...

BTW as you decided to keep setuptools, I suggest to add Python version checker (e.g. python_requires='>=3.5') to prevent users from accidentally installing library. Also could you join line 13 and 14, because you once said you'll keep hard wrapping on column 120?

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

No branches or pull requests

3 participants