xy-VSFilter icon indicating copy to clipboard operation
xy-VSFilter copied to clipboard

XYsubfilter uses the default (Arial) style instead of the file

Open MatiasMovie opened this issue 3 years ago • 17 comments

XYsubfilter uses the default (Arial) style instead of the file Tested with MPC-HC and MPC-BE, subs work fine in MPV player (libass) I think it's problem with "space" on the end in style name

///EDIT/// I changed the video to a random light sampel and added problematic subtitles (Now there is only 27.9 MB) https://gofile.io/d/RqiFs6

MatiasMovie avatar Mar 03 '21 20:03 MatiasMovie

1.2G? Don't you have only the extracted sub file? Or you can test with the current build I've just published (mainly because I made changed to the Avisynth filter), if the problem still occurs.

pinterf avatar Mar 05 '21 09:03 pinterf

Unfortunately, the error occurs only if the subtitles are in the MKV file, external works well I am using version 3.2.0.805

v3.2.0.804 (20210306) still have problem

MatiasMovie avatar Mar 06 '21 00:03 MatiasMovie

XySubFilter and external subtitles: XySubFilter and external

XySubFilter and mkv: XySubFilter and mkv

mpv and mkv: mpv

MatiasMovie avatar Mar 06 '21 01:03 MatiasMovie

Thanks for the report, fixed in https://github.com/pinterf/xy-VSFilter/commit/f71eb969f8b580b43bedc25f4e73d212b8097add Can you build it yourself or I should prepare a test build for you?

pinterf avatar Mar 06 '21 09:03 pinterf

I did test the new sample in mpc with it's internal sub render (I have no xy-VSFilter nor VSFilter in that system) and it's same problem, maybe it's mpc bug after all

realfinder avatar Mar 06 '21 10:03 realfinder

Thanks for the report, fixed in f71eb96 Can you build it yourself or I should prepare a test build for you?

I do not know how to build it, but I can wait for a new version.

MatiasMovie avatar Mar 06 '21 15:03 MatiasMovie

Check this one https://drive.google.com/uc?export=download&id=1yDCrMbaULP3h2yrdNZE3OCoH7g__wP4z

pinterf avatar Mar 06 '21 20:03 pinterf

Now the subtitles do not display Tested in MPC-HC and MPC-BE

MatiasMovie avatar Mar 06 '21 22:03 MatiasMovie

Sorry, then this one should work, I hope. https://drive.google.com/uc?export=download&id=1ZvYtb3l7UeJ0Xz3BgTj9X2x6q_W_O5eR

pinterf avatar Mar 07 '21 07:03 pinterf

now works good with internal subs, the information is probably displays a bad version number image

MatiasMovie avatar Mar 07 '21 08:03 MatiasMovie

I found a problem, now external subtitles have a problem with this style in MPC-HC/BE image

FPS_test_1080p23.976_L4.1.zip

MatiasMovie avatar Mar 07 '21 08:03 MatiasMovie

Great, thanks for checking that

pinterf avatar Mar 07 '21 09:03 pinterf

Please revert f71eb969f8b580b43bedc25f4e73d212b8097add. No other ASS renderer does this, and it actually breaks this file as you can see in the latest comment by OP.

But also, please report and fix bugs upstream in https://github.com/Cyberbeing/xy-VSFilter. We have too many forks as it is.

The problem is in CSubtitleInputPin:

https://github.com/pinterf/xy-VSFilter/blob/13346cb00e7e15a7b5ae0bc78e74017b7fbd1b34/src/subtitles/SubtitleInputPin.cpp#L260-L265

https://github.com/Cyberbeing/xy-VSFilter/blob/fc01a8da5ea6af9091aaab839bc62dc94a90094e/src/subtitles/SubtitleInputPin.cpp#L260-L265

which uses Explode, which trims every field:

https://github.com/pinterf/xy-VSFilter/blob/13346cb00e7e15a7b5ae0bc78e74017b7fbd1b34/src/dsutil/text.h#L35-L38

For what it’s worth, this also affects MPC-HC’s internal subtitle renderer.

astiob avatar Mar 07 '21 14:03 astiob

Thanks astiob, then it wasn't the way to go. As for putting all my changes upstream: if a change is worth to be put there they're gonna cherry pick as it happened in the past. Once I'd do a PR all my changes would appear there in a long row. Unless I change my habit of working into a single branch and make a different branch for each feature/fix. But this project was/is so dead that I really didn't think they would spend any time on with my modifications. Or it breaks the project rules so much that they get incompatible with the original Cyberbeing branch. (E.g. if I make it to require c++17 which is a highly logical thing in 2021)

pinterf avatar Mar 07 '21 16:03 pinterf

Next iteration, please. https://drive.google.com/uc?export=download&id=14RG8OL-YTybEj4BlOPKCaOCEE2Bwkt0b

pinterf avatar Mar 07 '21 18:03 pinterf

Now works well

MatiasMovie avatar Mar 07 '21 19:03 MatiasMovie