search

migaku-official / migaku-mpv

Add-on script for mpv to learn languages together with the Migaku browser extension.
GNU General Public License v3.0
15 stars 4 forks source link

[Implements #9] Default to using ffmpeg for audio and screenshots, fallback to mpv if ffmpeg fails #11

Closed christofferaakre closed 2 years ago

christofferaakre commented 2 years ago

Not sure if you guys accept pull requests, but I had a go at implementing #9. Sorry for the messy diff. Let me know if there is anything I should change before you can merge this! I belive this fixes #9

RicBent commented 2 years ago

Hey there! Thanks for the contribution.

Actually this was already somewhat implemented in https://github.com/migaku-official/migaku-mpv/tree/ffmpeg-export

The main reason this was not yet merged is that migaku mpv actually allows creating cards from web resources like YouTube and Viki. Switching to ffmpeg breaks that use case. What might work however is first attempting to export the media with ffmpeg and then use mpv as a fallback on failure.

christofferaakre commented 2 years ago

Hey there! Thanks for the contribution.

Actually this was already somewhat implemented in https://github.com/migaku-official/migaku-mpv/tree/ffmpeg-export

The main reason this was not yet merged is that migaku mpv actually allows creating cards from web resources like YouTube and Viki. Switching to ffmpeg breaks that use case. What might work however is first attempting to export the media with ffmpeg and then use mpv as a fallback on failure.

I see. The code I wrote does actually fallback to mpv if ffmpeg fails. @RicBent

RicBent commented 2 years ago

Oh I'm actually blind. Sorry for that.

Will check it out and merge it asap.

christofferaakre commented 2 years ago

@RicBent just doing from migaku_mpv import find_executable in ankiexport.py doesn't work since they are not in the same directory, and I don't really want to mess with the directory strucutre. I see there is an __init__.py file in utils, but I am not quite sure what the package structure is supposed to be like.

RicBent commented 2 years ago

The best solution probably would be handling it the same way as mpv_executable as that file was meant as something independent that could be reused in other projects later down the line. References for how mpv_executable is set:

https://github.com/migaku-official/migaku-mpv/blob/063fe424754af72350d2ad6935c5bd57d35a1d3d/migaku_mpv.py#L490 https://github.com/migaku-official/migaku-mpv/blob/063fe424754af72350d2ad6935c5bd57d35a1d3d/utils/ankiexport.py#L12

christofferaakre commented 2 years ago

@RicBent I think this latest commit https://github.com/migaku-official/migaku-mpv/pull/11/commits/914c52a3f5bc7b30d67d05c1c1602d8706d2bca1 does what you asked

RicBent commented 2 years ago

Thanks a lot! Merging~

christofferaakre commented 2 years ago

Great! Thanks for being patient with me