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

Manually taking screenshot (Ctrl+Shift+V) is not working when watching local videos on 'killergerbah.github.io/asbplayer' (despite having the extension installed) #238

Closed
Y-LBG opened this issue May 3, 2023 · 9 comments
Labels
enhancement New feature or request

Comments

@Y-LBG
Copy link

Y-LBG commented May 3, 2023

Hi @killergerbah,

I'm not sure if that's a bug, or a not-yet-implemented feature.

This is regarding the capability to manually take a screenshot, rather than hoping the image fitting the beginning of the subtitle will be good enough. Quite an amazing feature proposed by the asbplayer extension, since the proper image describing the action usually tends to arrive just a few seconds later.
However, while it seems to work fine on streamed videos (I checked on Youtube), this is not working when watching local videos on 'killergerbah.github.io/asbplayer'.

I do have the extension installed, so I'd have expected this "extension only" feature would be available but...

I also noticed that the 'Screenshot capture delay' feature proposed in the extension isn't working on 'killergerbah.github.io/asbplayer' either (which I'm guessing is directly tied to the previously mentionned issue)

Cheers

@killergerbah
Copy link
Owner

@Y-LBG That's correct - screenshot delay and "manually take screenshot" features are not implemented on the site side. One reason for this is that any cards mined from the site will use the current frame for the screenshot, so in some sense "manually take screenshot" as a separate command isn't really necessary for the site. However I realize that this is inconsistent w/ the extension mining flow and in some cases the extension flow would be preferable. Since you're bringing this problem up now it may make more sense to make both flows more similar.

@Y-LBG
Copy link
Author

Y-LBG commented May 6, 2023

Interesting.

I was actually thiking about that as a potential enhancement : Using the current frame for the screenshot.
I do believe this is indeed the best from a UX perspective (rather than having to manually edit it again with Ctrl+Shift+V afterwards)

But you tell me this should already the case...
Yet, at least as per my tests, it doesn't seem to be working that way.
Even on the website, when I hit Ctrl+Shit+X or Ctrl+Shit+U, it uses the frame corresponding to the beginning of the subtitle for the screenshot.
There might be a bug there.

As for the enhancement to make both flows (website and extension) similar...
Only my opinion here, but I'd definitley say the best would be a mix of both worlds:

  • Default: Use the current frame (rather than the frame corresponding to the beginning of the subtitle, which very rarely fits)
  • Possibility: Ctrl+Shit+V to overwrite the above default behaviour (i.e. use whatever frame you want, even if before/after the subtitle/audio)

Cheers.

@killergerbah
Copy link
Owner

@Y-LBG You're right, it looks I broke the "current frame" behavior at some point. I've pushed the fix for that; let me know what you think.

I think your suggestion for Ctrl+Shift+V makes sense. If anything it will make the UX more consistent and less confusing between the site and the extension, and it's a convenient way to open the last card.

@killergerbah killergerbah added the enhancement New feature or request label May 6, 2023
@Y-LBG
Copy link
Author

Y-LBG commented May 7, 2023

The fix for the "current frame" behaviour works like a charm... But only on my local build 😝
On 'https://killergerbah.github.io/asbplayer/', I get an 'MP3 encoding failed: undefined' error...
Might just be a deployment issue, since it's working perfecty when I build the code myself and tets it on 'localhost:3000/asbplayer/'.

As for the enhancement... Agreed ^^

@killergerbah
Copy link
Owner

@Y-LBG Interesting I'm not able to reproduce that error on https://killergerbah.github.io/asbplayer/
What specifically are you doing to trigger it? And if possible could you paste a log here as well?

@Y-LBG
Copy link
Author

Y-LBG commented May 7, 2023

Can't reproduce anymore either...
Was getting a 404 on the mp3-converter.js
Might have just been a cache issue on my client.
気にしないで;」

@killergerbah
Copy link
Owner

I see, that makes sense

@killergerbah
Copy link
Owner

This is implemented in the localization branch.

@Y-LBG
Copy link
Author

Y-LBG commented Jun 2, 2023

Nice =D
Looking forward to that ;]

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

No branches or pull requests

2 participants