fxmarty / rikai-mpv

A port of Rikaichamp Japanese dictionary and parser into mpv video player
MIT License
33 stars 3 forks source link

subtitles not displayed #1

Closed TomRomeo closed 3 years ago

TomRomeo commented 3 years ago

Hey there! I really loved the idea of this plugin and wanted to try it out. Sadly, the subtitles of my video vanish when I start rikai with F5. They only reappear when quitting rikai with F7. When inspecting the logs afterwards I found following error:

Traceback (most recent call last): 0.000 Dropped: 6
  File "/home/osaka/.config/mpv/scripts/rikai-mpv/subtitles_popup_graphics.py", line 635, in <module>
    form = ParentFrame(config)
  File "/home/osaka/.config/mpv/scripts/rikai-mpv/subtitles_popup_graphics.py", line 460, in __init__
    self.subtext = TextWidget(parent=self)
  File "/home/osaka/.config/mpv/scripts/rikai-mpv/subtitles_popup_graphics.py", line 204, in __init__
    self.popup = Popup(self)
  File "/home/osaka/.config/mpv/scripts/rikai-mpv/subtitles_popup_graphics.py", line 120, in __init__
    self.focusProxy().installEventFilter(self)
AttributeError: 'NoneType' object has no attribute 'installEventFilter'

Do you by some chance know where this error is coming from and how I can fix it?

The complete log for rikai-mpv:

[front] Starting rikai-mpv...
[rikai-mpv] Starting rikai-mpv in python...
Launching node.js backend...
Connecting from Python...
Checking for leftover socket.A-V:  0.000
Removing leftover socket.
Creating server.
python <---> node.js socket connected.
Connection acknowledged.
node client connected.
Traceback (most recent call last): 0.000 Dropped: 6
  File "/home/osaka/.config/mpv/scripts/rikai-mpv/subtitles_popup_graphics.py", line 635, in <module>
    form = ParentFrame(config)
  File "/home/osaka/.config/mpv/scripts/rikai-mpv/subtitles_popup_graphics.py", line 460, in __init__
    self.subtext = TextWidget(parent=self)
  File "/home/osaka/.config/mpv/scripts/rikai-mpv/subtitles_popup_graphics.py", line 204, in __init__
    self.popup = Popup(self)
  File "/home/osaka/.config/mpv/scripts/rikai-mpv/subtitles_popup_graphics.py", line 120, in __init__
    self.focusProxy().installEventFilter(self)
AttributeError: 'NoneType' object has no attribute 'installEventFilter'
AV: 00:01:13 / 00:22:59 (5%) A-V:  0.000 Dropped: 6
[input] No key binding found for key 'F6'.
AV: 00:01:16 / 00:22:59 (6%) A-V:  0.000 Dropped: 6
[front] Quitting rikai-mpv...
[front] Finished exiting.
AV: 00:01:18 / 00:22:59 (6%) A-V:  0.000 Dropped: 6
[front] Not running.

Regards

fxmarty commented 3 years ago

Thanks a lot for your bug report!

You actually managed to already make it work quite far if Python and node.js manage to communicate ;-)

Could you give me more information on how you installed PyQt5 and PyQtWebEngine? If possible, try to reinstall them as detailed here https://stackoverflow.com/a/54947671/4370080 .

Edit: to explain to you what happens when you click f5: the subtitles from mpv are replaced by my own implementation, however this error makes basically everything crash and so nothing is shown as long as you don't exit.

Edit: Ok, I am able to reproduce the error and will look into it.

fxmarty commented 3 years ago

Alright, I pushed some modifications which should hopefully fix things up. Could you give it a try?

The thing is that I was using a conda environment, which has PyQt5 version 5.9.2 ( https://anaconda.org/anaconda/pyqt ), while most recent version are >=5.10. It seems like there was a non-standard behavior with PyQt5 <5.10 that allowed this line of code: https://linux.debian.bugs.rc.narkive.com/9hCRNf71/bug-906256-bug-905379-anki-hangs-after-nonetype-object-has-no-attribute-installeventfilter#post2

TomRomeo commented 3 years ago

Hello again, Yes, these changes seem to have fixed it! I can see the subtitles and interact with them now :D. Thanks for the help! Though the subtitles are displayed in the center of the screen now and I can't seem to move them with the typical mpv bindings (r, t). Do you know how to move them to the bottom of the screen? image

Regards

fxmarty commented 3 years ago

Oh, I did not know of these mpv bindings! It would be nice if I add them, I will have a look some time.

For the moment I did a very quick fix, could you try to tweak this option ( https://github.com/fxmarty/rikai-mpv/blob/main/rikai_config.py#L19 ) to see if you manage to bring it back down?

Actually I am a bit surprised it is so much in the middle. What is your screen resolution? If the above do not help (e.g., you need to put negative values for bottom_spacing_pixels), I would rate as high the chances that the screen resolution detection is failing: https://github.com/fxmarty/rikai-mpv/blob/main/subtitles_popup_graphics.py#L688 . It would then help if you give me your screen resolution and add some print(config.screen_width, config.screen_height) and see if it matches.

Edit: The line where the y value for the box (semi-transparent) is defined here (hence my surprise): https://github.com/fxmarty/rikai-mpv/blob/main/subtitles_popup_graphics.py#L581

Edit2: It also looks like you have some blurry effect on the semi-transparent box. This is not intended and I don't have this behavior. I would guess it is a window manager property, as I basically create a window for the subtitles. I have to see how to override it.

TomRomeo commented 3 years ago

Hey there, sorry for the late response,

Alright so I actually found out why the textheight was so messed up for me: One of my monitors is positioned lower than the other ones which is why i manually positioned the screen in the nvidia-settings so that my mouse cursor would align when moving it to the other screen. Idk why but disabling this height difference gets rid of the problem.

image

anyways, changing the bottom_spacing_pixels property worked great for me in order to get rid of this height difference.

We can solve this issue however you feel like is the right way. For me, setting this bottom_spacing_pixels property is totally fine. On the other hand we could also change the meaning of the property, making it a fixed height. If the value isn't set, it will just calculate the height like it does rn.

Another thing that I noticed is that the subtitles are always displayed on my left-most screen. I saw that you pinned the messages of interSubs discussing that issue but the maintainer only pointed to a solution that was solved by editing specific lines of code (which I could not easily apply to your code)

I tried to create a patch that would make multiple monitor setup work and it is working great for the subtitles but the rikai box is still displayed on the left-most screen. It would be great if you could help me with that. Here is the patch that I started writing:

multi_monitor_patch.txt

Best regards

fxmarty commented 3 years ago

Thank you for the details on your multi screen settings! I will be able next week to get hands on a second screen so that I can try out with it and hopefully come up with a nice and not-too-hacky solution for this.

Just to clarify, what does it is working great for the subtitles but the rikai box is still displayed on the left-most screen means? Does it mean that you manage to have the subtitles displayed on any screen you want with your edits, but that the semi-transparent background is stuck on the left screen (while mpv is in full-screen on an other screen)?

Also, I did not get what you mean by On the other hand we could also change the meaning of the property, making it a fixed height. If the value isn't set, it will just calculate the height like it does rn.. Right now the way it works is to set a number of pixels you want to have at the bottom below subtitles. This method (at least without proper screen selection) breaks at least in your case where screens are not positioned at the same height. Are you suggesting something else?

TomRomeo commented 3 years ago

Heyho,

so what I meant was that my patch does align the subtitles on the correct screen when specified. When you hover over a word however, the blue box with the translations still appears on the left-most screen. I think this is due to one snippet in your code where you move the box x value back if the width is bigger than the screen_width. This ofc makes sense in a single monitor setup but positiones the box on the wrong monitor with multiple monitors. (I will add a screenshot amd the mentioned code snippets later when I am back at home)

I should mention that my solution is basically only changing the X- coordinate calculation for the subtitle box by adding the screen_width multiplied by a number. This only works if all of your screens have the same width. e.g.: You specify the screen_num as 1 in the config file. When the x value for the subtitles is calculated, it calculates the center of the screen - the subtitle width like before but then adds screen_width*screen_num to the x value, moving it to another screen. This ofc also has to be set manually, making it more like a "hacky" solution. When I am back home I can also look up if there is a way to get the screen x coordinate through mpv, maybe that would be the best.

As for the bottom_spacing_pixels: I didn't quite get how the property was applied in your code, it seemed to me like it was a value that was substracted from the calculated height. My suggestion was to change the behaviour of this property: When it is not set, just calculate the subtitle height like before, if it is set to a value, set the height of the subtitles to that value. But the way it works right now is working for me too and since I am pretty sure that my setup is not the norm, we don't need to do that.

I am sorry, English isn't my mother tongue. I hope it was understandable :)

Regards

fxmarty commented 3 years ago

Thanks for the clarification, it makes perfect sense now!

For the bottom_spacing_pixels: it was already here before, just that before my quick patch it was not set as an option but you had a hard-coded - 100 in the positioning (previous version): https://github.com/fxmarty/rikai-mpv/blob/6b469d05ca5a86045e1021edd92b4bcffede3298/subtitles_popup_graphics.py#L580

As for the problem with the popup on the left-most screen, I would guess it is because you would need to change the x position of the popup as well.

Edit: I think this is due to one snippet in your code where you move the box x value back if the width is bigger than the screen_width --> this is also probably a problem, nice catch!

Indeed you snippet would break if screens have different width. I think there will be issues also if somebody has two screens one above the other (now, the issue will not be with width but height). That is why I want to have a look and see if in Qt there is better support for the multi screen case. But I will wait for having a second screen so that I can test.

Indeed I was thinking the options screen ( https://mpv.io/manual/master/#options-screen ) and geometry ( https://mpv.io/manual/master/#options-geometry ) from mpv could maybe be retrieved and used, it would really be the cleanest. An other simple solution is to have a shortcut to cycle through screens (to set the subtitles/popup on the right one on the fly), and an option with the default screen. Subideal, but it would work.

fxmarty commented 3 years ago

Hello @TomRomeo , in the end I won't be able to get a hand on a second screen this week, but hopefully next week will do. Sorry for the delay!

TomRomeo commented 3 years ago

That's alright, it's not like this is a gamebreaker for me. I will just use mpv on my first screen in the meantime :)

TomRomeo commented 3 years ago

Hey, how is is going?

Did you manage to get your hands on a second monitor? :)

fxmarty commented 3 years ago

Hello, sorry for the long delay!

There was quarantine for two weeks there so I could not, but now this week I am back at work so have access to more screens. Let me have a look tomorrow / Friday!

By the way, I made a post on Reddit to advertise a bit the repo, and somebody pointed out a similar solution exists for mpv, using yomichan: https://github.com/ripose-jp/Memento . Maybe you want to have a look too, depending on your needs!

fxmarty commented 3 years ago

Hello, hope you are well. I did a quick fix today. It should hopefully work, but I would be extremely grateful if you have the time to test it on your side to make sure it is portable enough. You can see the commit here: https://github.com/fxmarty/rikai-mpv/commit/cd9e8c153afd044de9072e9d03ea35a3e37689a5 . Thank you!

TomRomeo commented 3 years ago

Hello, sorry I didn't see the message for some reason.

I know about the Yomichan plugin but I liked this one more.

I just tried the commit and it seems to work really well! The subtitles and the rikai box get displayed in the correct window!

Thank you very much for this great plugin!

fxmarty commented 3 years ago

That's great news, glad it works for you!

Let me close the issue for now :)