search

maxrd2 / SubtitleComposer

Subtitle Composer - KF5/Qt Video Subtitle Editor
Other
238 stars 29 forks source link

Add isRightToLeft function and some scripts #96

Closed SafaAlfulaij closed 5 years ago

SafaAlfulaij commented 6 years ago

This adds a function to decide if the subtitle line is RTL or not, which makes it possible for scripts to check that and do some post-processing due to the stupidness of ASS engines (libASS and vsFilter) as well as the normal SRT format that doesn't support that, unfortunately. I didn't check other formats supported really.

SafaAlfulaij commented 6 years ago

To be honest, I don't know the deep meanings of const stuff, just took a look on the existing code with some reading of that concept.

Martchus commented 6 years ago

@SafaAlfulaij But you did it right. The const will make the target of the this pointer inside the methods const.

maxrd2 commented 6 years ago

Thank you, it looks good but am wondering if it would be better to add RTL support directly into subtitlecomposer code. That script just adds RTL marks on beginning of each line when QString::isRightToLeft() returns true? Maybe it would be better if I add logic to include those marks immediately when loading/saving subtitles? When/why are those needed?

SafaAlfulaij commented 6 years ago

Well, this is just a workaround for supporting RTL, and I'm not with adding this workaround as part of the app. If so, we'll need more detecting and checking stuff to not add duplicates (not really harmful, but adds a bit of weight to the renderer).

These are needed once the subtitle needs to be displayed in the media player. SRT engine forces LTR with no way to detect RTL lines, so we just force the line to be RTL by that embedded character. SSA engines are two, vsFilter and libASS. vsFilter doesn't support RTL like SRT, and libASS was supporting RTL correctly but dropped the code to be “compatible” with vsFilter and all the hacks and stupid workarounds translators did in the past and are continue doing (like writing the dot at the beginning of the sentence to appear at the end when viewing with the engine).

maxrd2 commented 5 years ago

Have merged this for now. Will look into adding better support for RTL text to the application itself in the future.