ripose-jp / Memento

An mpv-based video player for studying Japanese
https://ripose-jp.github.io/Memento/
GNU General Public License v2.0
488 stars 22 forks source link

Ability to edit the current subtitle entry #65

Closed Calvin-Xu closed 2 years ago

Calvin-Xu commented 2 years ago

First of all, just want to say a big thank you. This project has really come a long way and has become smoother and smoother to use than I ever imagined in the beginning (measured by how often I need to leave Memento in the middle of watching to make something work). With apologies for all the yak shaving I contributed to, I wish a happy 2022 for everyone.

Here's a feature that I think would still be more useful: allow the current subtitle displayed by SubtitleWidget to be edited. There are a number of use cases for this, one being fixing auto-generated subs mentioned in https://github.com/ripose-jp/Memento/issues/61. The other is when you have extracted bitmap subs from BD/DVDs in the form of SUP or VobSub (SUB/IDX) files. With OCR tools, it is possible to convert these to SRT with the correct timing information. The user can load both the bitmap sub and the SRT in Memento, and use the SRT to lookup & add to Anki. There are just a few problems that prevent this from being done smoothly at the moment, which I'll open as separate issues:

  1. The text from OCR is often wrong. To be fair, the timing information allows manually searching up a word in the bitmap sub with Ctrl-R and adding the card to Anki with the correct audio-context. But it might still be nicer if the user can edit the entry to correct it (with the original bitmap sub as reference). Currently, double clicking on the SubtitleWidget copies the current line. I wonder if this can be changed to enable editing instead. Ctrl-C copies the current context and should be able to replace the functionality, although there seems to be a problem at the moment: https://github.com/ripose-jp/Memento/issues/67.
  2. It might be nice to enable the SubtitleWidget to use the secondary subtitle. This is a problem I found specifically with using bitmap subs. Since the SRT from OCR can be wrong and sometimes gibberish, it is desirable to hide it in mpv, and only show it on pause in Memento's SubtitleWidget during editing and lookup. However, mpv does not allow the primary sub to be hidden while the secondary sub is visible, though the opposite is supported. So the ideal arrangement is to have the original bitmap sub as the primary sub and the SRT as secondary. This issue is opened separately here: https://github.com/ripose-jp/Memento/issues/66.
ripose-jp commented 2 years ago

Rather than discussing this in an unrelated issue I still don't like that compromise. The way I see it, UI elements that people generally assume to be static should be static. I can possibly create a popup window that acts as a sort of advanced add that will let you manually edit some markers like {cloze-*}, {sentence}, etc. before the card is added to Anki. Let me know if that will work instead.

Calvin-Xu commented 2 years ago

I would still argue that double click to edit certain displayed values is a fairly widely adopted UX, but I am certainly not particular about being able to do it in the SubtitleWidget. I think editing the entry can solve a lot of manual work compared to the proposed popup window. As an example, suppose you encounter this line that is first but not least garbled up from OCR:

、ベイー8全くもってまどろこっしい真似をしているわけた河*#ヵ火野ぁ

Suppose you want to look up まどろっこしい which becomes more troublesome. Most dictionaries are going to tell you "まどろこしいに同じ。" Then manually looking up まどろこしい yields "「まだるっこい」の口頭語的表現。" Searching for まだるっこい finally yields the entries with definitions and you add it. To clean up the card either before or after it is added to Anki, you would need to do the following:

If instead the current line can be edited, you can just first of all fix the line to 全くもってまどろっこしい真似をしているわけだ. Additionally, as it turns out that まだるっこい is the actual useful term to look up, you can just change まどろっこしい in the line to まだるっこい and then look up, and only having to change {sentence} and {cloze-body} back later (this might be further streamlined if https://github.com/ripose-jp/Memento/issues/80 can lookup in place). Everything is populated correctly in this case, and it can take much less work.

So I do think being able to edit is a better solution overall. Is the issue is with making SubtitleWidget editable, I think it is fine too to have a popup with duplicate text that can be edited, etc., like Yomichan’s search page that is prepopulated with the text of the current subtitle.

ksesong commented 2 years ago

I think that for clarity subtitles should remain read-only. Giving the ability to edit the entry would induce other issues, such as "I have wrongly edited a subtitle, how to cancel?", "If I edit a subtitle, does it save it to the .srt file?", "Should we show edited subtitles differently in the subtitle list?"...

I would propose instead some sort of a "subtitle scratchpad/draft/sandbox", separate from read-only subtitles, as a temporary tier of subtitles. This sandbox would be fully editable, from timecodes to text. When the sandbox is enabled, all read-only subtitles are hidden (one can also give it a different style/colour, so that there's no confusion). You can't edit subtitles (as there are read-only), but you would be able to have a "Open subtitle in Sandbox" function, where you would be able to use the subtitle data as a starting point.

Calvin-Xu commented 2 years ago

I suppose a popup text editor/subtitle editor in Memento that modifies a copy and can hot reload works too. Would be nice if it scrolls to the current entry or remembers last scroll position.

Calvin-Xu commented 2 years ago

The way I see it, UI elements that people generally assume to be static should be static.

To be honest, with the improvements to Memento that have come along, it is no longer so bad to just go edit the subtitle file and then import it back to Memento. The main pain points date back to before the subtitle file parser and audio-context were added and when things were finicky if you mess with it during playback.

I suppose it still has some value for the specific "transcribe bitmap subtitles" use case I described. But if you think this should not be implemented given the overall design choices, I'd be willing to close this issue.

ripose-jp commented 2 years ago

There are a couple of reasons I'm sour on this feature.

I understand that OCR can produce poor results, but there probably aren't that many people that have that in their workflow. If this was a common problem I might want to find a way to make it work, but right now I don't think enough people are using OCR subs.

Calvin-Xu commented 2 years ago

I quite understand. In that case, I'll close this issue. If OCR subs ever comes up again I guess this can be referenced.