Jellyfin’s dropAllAnimations #134
Conversation
To aid readability by making it easy to see all class-local variables and their types and visibility.
TheOneric
force-pushed
the
jf_drop_animations
branch
from
e23b54c to
64feebc
Compare
| case '{': | ||
| // Escaping the opening curly bracket to not start an override block is | ||
| // a VSFilter incompatiple libass extension. But we only use libass, so... | ||
| if (!block_start && (p == event->Text || *(p-1) != '\\')) |
There was a problem hiding this comment.
Shouldn't the escaping be parsed with the boolean flag?
For something like bla-bla \\{\pos(100,200)}.
libass doesn't seem to escape the backslash either: 1\\N2 gets a new line.
There was a problem hiding this comment.
Sorry, I don't understand what your suggesting with a “boolean flag”. Can you elaborate?
Also there aren't any backslashes being escaped being checked, but backslashes escaping curly brackets (working only in libass).
There was a problem hiding this comment.
I meant that the condition only checks the previous backslash, but there can be another one before it. In this case something like if (*p == '\\') escaped = !escaped; is required. But since libass (SSA/ASS standard) has no escaping logic, the current implementation is fine.
There was a problem hiding this comment.
Great, then it seems this is ready to be merged.
This prepares the dropAnimations and renderAhead features from jellyfin, though the functions are not yet used. The implementation in this commit completly replaces the faulty code from jellyfin, which was added in the misleadingly named "Less zealous logging" commit jellyfin@6980deb The old version was not detecting valid ASS tags like '\t (\frz50' and did some other checks not backed by ASS' behaviour. Rewrite it from scratch to not miss any animated tags. The previous version made an attempt to also detect and not report noop animation tags like '\move(1,1,1,1)'; the new version does not as correctly parsing the tag and its values is simply more effort than its worth and likely to result in false negatives. Noop animation tags should be sufficiently rare anyway.
Original implementation by JustAMan, mostly in the following three commits sans unrelated changes. Backported by Oneric. jellyfin@d3bc472 jellyfin@a66e797 jellyfin@971a997 The commits have also been adjusted to apply on top of current upstream master, to use the animations detection from the previous commit instead of the faulty original logic and the scanned_events variable was added. Also added documentation which was missing in the original commits. Co-authored-by: Oneric <oneric@oneric.stub>
TheOneric
force-pushed
the
jf_drop_animations
branch
from
64feebc to
5b42203
Compare
Adds the corrected core animation detection and deletion code for
dropAllAnimationsandrenderAheadand backportsdropAnimationsfrom jellyfin.@dmitrylyzo Could you take a look at this to check if I missed to backport any relevant fixes for
dropAllAnimations? In particular there’s 08f72cd which refers to “lite mode” in its title, but iiuc the actual changes are all in therenderAheadportion.