Oleg Oshmyan [Sun, 5 Feb 2017 02:24:54 +0000 (04:24 +0200)]
Travis CI: run Coverity Scan on every master build
We never remember to push to the coverity_scan branch, so currently
Coverity Scan never runs. Our master builds are not very frequent,
so we should be able to afford running Coverity Scan on every build.
Since https://blog.travis-ci.com/2016-10-04-osx-73-default-image-live/,
this libtool comes preinstalled on Travis CI, thus the hack is no longer needed.
Homebrew bug report possibly relevant to the original problem:
https://github.com/Homebrew/legacy-homebrew/issues/43874
Oleg Oshmyan [Sun, 5 Feb 2017 01:44:34 +0000 (03:44 +0200)]
Travis CI: don't require Fontconfig binaries
Only the library is needed.
In fact, `apt-get install fontconfig` didn't even install the library at
all. Luckily, the package we actually want is preinstalled on Travis CI.
We could continue to rely on this fact and completely remove Fontconfig
from the install list, but it's clearer and possibly more future-proof
to explicitly list it there.
Oleg Oshmyan [Sun, 5 Feb 2017 00:05:20 +0000 (02:05 +0200)]
Travis CI: disable Fontconfig on OS X
Homebrew generates the Fontconfig cache when installing Fontconfig,
which delays the build by several minutes. Disable the Fontconfig
font provider on OS X to avoid this.
Oleg Oshmyan [Mon, 30 Jan 2017 22:11:49 +0000 (00:11 +0200)]
Reduce precision of border width in outline cache keys
The value used to generate outline cache values is 26.6, so there
is no point in storing the more precise 16.16 in the cache key.
Indeed, this can only reduce the efficiency of the cache
and provide an extra opportunity for overflow.
Oleg Oshmyan [Mon, 30 Jan 2017 21:45:43 +0000 (23:45 +0200)]
Reflect border_scale in outline cache keys
border_scale can change, e. g. when ass_render_frame is called twice with
the same renderer but different tracks. Glyphs with equal \bord tag values
but different border_scale values produce different border outlines and
hence should be distinguished in outline cache keys. To this end, store
scaled border widths (which are really used when generating the outlines)
in cache keys instead of \bord tag values.
Dr.Smile [Mon, 30 Jan 2017 23:47:58 +0000 (02:47 +0300)]
render: remove redundant has_clips
has_clips was a workaround for the case where a new image reused
the same memory address as another image used in the previous frame.
In case of such reuse, comparison by pointer address failed
to distinguish the different images in ass_detect_change().
After commit dd06ca30ea79ce50116a43cc5521d4eaf60a017e,
images in the previous frame are no longer freed before
the comparison with current frame. Thus no such reuse can occur,
and the workaround is redundant.
wm4 [Fri, 13 Jan 2017 08:19:23 +0000 (09:19 +0100)]
render_api: do not discard old images on reconfiguration
I noticed that when resizing the mpv window while playback is ongoing
and with subtitles, that subtitles could sometimes get "stuck" on the
screen. The stuck subtitle would remain until the next subtitle event,
or until seeking to a position that has subtitles again.
It turned out that this was a libass change detection bug. The following
steps should reproduce the problem:
1. call ass_render_frame() with a time that has subtitles
2. call ass_set_frame_size() with a different size
3. call ass_render_frame() with a time that has no subtitles
The previous call will return with *detect_change==0.
To make this worse, libass will deallocate image data before the next
ass_render_frame() or ass_renderer_done(), which violates the API and
could possibly make some API users crash. (That the user can rely on
this is not documented though.)
There are two possible solutions:
1. Set a flag in ass_reconfigure(), that makes the next
ass_render_frame() call always return *detect_change==2.
2. Do not discard the previous subtitles (images_root), so change
detection can work reliably.
This commit implements 2. - I prefer this in part because it doesn't
clobber the previously returned image list before the next
ass_render_frame() call. (As pointed out above, this might be unexpected
behavior to the API user.)
This is a regression and was possibly broken by commit dd06ca and later.
I did not check whether it actually behaved sanely before that change,
but it probably did to a degree.
wm4 [Wed, 11 Jan 2017 06:10:13 +0000 (07:10 +0100)]
render: clip BorderStyle=4 against screen
ASS_Images returned by libass are guaranteed to be clipped. Not doing
this will cause invalid memory accesses in applications which try to use
this guarantee.
Oleg Oshmyan [Tue, 3 Jan 2017 19:20:20 +0000 (21:20 +0200)]
Bump ABI version and release 0.13.6
sizeof(ASS_Style) is actually part of the ABI, so adding the Justify field
in commit e54c123d5a08b6212533ddcced2cb1a50fa3d2b2 broke the ABI even
though we tried to avoid it by placing the field at the end of the struct.
Oleg Oshmyan [Wed, 28 Dec 2016 19:14:21 +0000 (21:14 +0200)]
Fix buffer overread in parse_tag when end points to a space
When parse_tag is invoked recursively to handle the animated tags inside
a \t tag, the `end` argument is taken from the `end` field of a struct arg
in the enclosing parse_tag. When struct arg is filled by push_arg, this
field is always right-trimmed using rskip_spaces. Ultimately, the inner
parse_tag invokation sees its `end` argument point not to the ')' or '}'
of the \t as it expects but rather to the spaces preceding the ')' or '}'.
At this point, when parse_tag calls skip_spaces, which is ignorant of the
end pointer, it happily skips over the spaces preceding the ')', moving the
pointer past `end`. Subsequent `pointer != end` comparisons in parse_tag
fail (as in fact `pointer > end`), and parse_tag thinks it is still inside
the substring to be parsed.
This is harmless in many cases, but given either of the following inputs,
parse_tag reads past the end of the actual buffer that stores the string:
{\t(\ }
{\t(\ )(}
After this commit, parse_tag knows that `end` can point to a sequence of
spaces and avoids calling skip_spaces on `end`, thus avoiding the overread.
Discovered by OSS-Fuzz.
Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=194.
Oleg Oshmyan [Fri, 4 Nov 2016 14:27:44 +0000 (16:27 +0200)]
ass_strtod: correctly convert large negative exponents
Avoid overflow in dblExp that prevents subnormal numbers from being
generated (or small normal numbers if `double` supports many more
negative exponents than positive): if `10**abs(exp)` would overflow
and we actually want a negative exponent, switch to using precomputed
negative powers of 10 rather than positive.
Also avoid underflow for numbers with a large negative exponent where
the exponent alone underflows but the significand has enough digits to
cancel this out, e. g. in `10e-324` with IEEE 754 double.
Oleg Oshmyan [Sun, 30 Oct 2016 00:26:00 +0000 (03:26 +0300)]
ass_strtod: skip leading zeros in mantissa
ass_strtod reads at most 18 leading digits of the mantissa.
This previously included zeros, even though they are not significant
digits, e. g. 0.000000000000000001e18 was converted to 0.0.
After this commit, leading zeros before and after the decimal point
will be skipped, so the above number will be correctly converted to 1.0.
Dan Oscarsson [Wed, 5 Oct 2016 11:52:47 +0000 (13:52 +0200)]
Add text justification
Subtitle recommendations often include that multi line
subtitles should be left justified as this is easier for
the eyes. This is also the standard used by several television
companies.
This add the possibility to define how subtitles are to
be justified, independently of where they are aligned.
The most common way could be to set justify to left, and have
alignment to center. But you can, for example, have alignment
to left and justify to center, giving subtitles to the left but
justifed on the center (instead of normal left justified).
Using justify right and alignment of center, might be good
choice for Arabic.
If justify is not defined, all works like before.
If justify is defined, subtitles are aligned as defined
by alignment and justified as defined by justify.
ASS is not extended by this, justify can only be defined
by setting Justify to wanted justification.
Grigori Goronzy [Tue, 4 Oct 2016 19:25:41 +0000 (21:25 +0200)]
Fix line wrapping mode 0/3 bugs
This fixes two separate bugs:
a) Don't move a linebreak into the first symbol. This results in a empty
line at the front, which does not help to equalize line lengths at all.
Instead, merge line with the second one.
b) When moving a linebreak into a symbol that already is a break, the
number of lines must be decremented. Otherwise, uninitialized memory
is possibly used for later layout operations.
Found by fuzzer test case
id:000085,sig:11,src:003377+003350,op:splice,rep:8.
This might also affect and hopefully fix libass#229.
Grigori Goronzy [Tue, 4 Oct 2016 00:35:26 +0000 (02:35 +0200)]
shaper: fix reallocation
Update the variable that tracks the allocated size. This potentially
improves performance and avoid some side effects, which lead to
undefined behavior in some cases.
Fixes fuzzer test case id:000051,sig:11,sync:fuzzer3,src:004221.
At some point in the past Travis and homebrew colluded to break the
preinstalled libtool on travis MacOS instances. Forcing brew to
reinstall libtool seems to be the common solution that several other
projects on github have used.
font load from dir: use MSGL_INFO instead of MSGL_WARN
This is a normal course of action and should not generate a warning,
especially for applications which use libass and might notify the user
on such "warnings", while in fact it should be info or even verbose.
Advantages over the old algorithm consist of the following.
* There are no glitches due to full cache clearing.
Items are arranged into linked list ordered by time of last use.
Only the oldest items get deleted at the clearing event.
* Each item now keeps track of number of references.
Referenced cache values are immune to clearing.
* Reduced amount of total cache memory for the same performance.
* Reduced number of memory allocations per cache item.
wm4 [Thu, 31 Dec 2015 16:26:46 +0000 (17:26 +0100)]
ass: add ass_set_check_readorder() API function
Not all API users will keep the event list on seeking. This also gives
the opportunity to API users to handle severely broken files with
duplicate ReadOrder entries. (It is not known whether this is really
needed, however VSFilter does not deduplicate using the ReadOrder
field.)
wm4 [Thu, 31 Dec 2015 15:28:00 +0000 (16:28 +0100)]
ass: declare mixing ass_flush_events() and ass_process_chunk() allowed
This was always the intention, but the wording could be read as if this
is not allowed. There was a bug that broke ass_flush_events() too, which
gives all the more reason to clarify this.
wm4 [Thu, 31 Dec 2015 15:23:44 +0000 (16:23 +0100)]
Changelog: create entry for the next version
It's always such a bother to write the changelog on release. And in
fact, there is no reason why the changelog should be written on release.
It's easier to edit it as we commit fixes or features.
I'm adding the changelog entries for the past two commits, and in the
future, we should add entries as we commit bug fixes or new features.
Oleg Oshmyan [Wed, 4 Nov 2015 13:25:47 +0000 (15:25 +0200)]
fontselect: replace is_postscript flag with check_postscript function
DirectWrite does not provide fast access to the is_postscript flag,
requiring each font to be loaded before its format can be determined.
Eagerly doing this for every installed font can be quite slow,
on the order of seconds.
To improve performance, ask the font provider for this information
only when it is actually needed, i.e. when one of the font's full
names or its PostScript name matches a requested font name and we
need to know whether to accept this match.
The return value of check_postscript is not cached in this commit.
This makes repeated calls slower than accessing is_postscript was.
This should not be a problem, but if it is, the value can be cached
(or precomputed) by font providers in their font private data.
This commit also potentially increases the memory usage of some
font providers by retaining data structures needed to implement
check_postscript in their font private data. This should not be
a problem either, but if it is, the value of check_postscript
can be precomputed by all providers other than DirectWrite.
Oleg Oshmyan [Thu, 29 Oct 2015 23:33:30 +0000 (01:33 +0200)]
directwrite: improve error handling
* Check malloc and calloc return values.
* Abort if a name can't be fetched, rather than supply a NULL
string to fontselect causing it to crash.
* Make sure to free all allocated memory.
* Always check FAILED(hr) before using the value of any output
argument returned by DirectWrite, because it is not clear whether
they are guaranteed to have correct values in case of error.
Oleg Oshmyan [Thu, 29 Oct 2015 23:33:30 +0000 (01:33 +0200)]
directwrite: slightly clean up the code
* Metrics are not used, so don't fetch them.
* All variables except meta are always explicitly set before use,
so don't initialize them.
* Declare variables where they are used.
* Use int loop variables when the loop bound is int.
* Prefer post-increment to pre-increment.
Oleg Oshmyan [Thu, 29 Oct 2015 23:13:59 +0000 (01:13 +0200)]
directwrite: split out the inner loop of scan_fonts as a separate function
This has the side effect that the ASS_FontProviderMetaData instance is now
cleared for every font rather than only once at the start of the search,
which fixes some use-after-free scenarios and prevents the creation of
chimeric fonts using names left over from other fonts processed earlier.
This lays the groundwork for further code simplification and error handling
improvements within this function, which will come in a separate commit.
This commit is transparent to `git blame -w` except for return statements.
Oleg Oshmyan [Wed, 28 Oct 2015 22:19:20 +0000 (00:19 +0200)]
fontselect: silence warnings about discarding const
Fixing this properly involves constifying ASS_FontProviderMetaData
and refactoring code that allocates and frees strings stored in it.
This seems easy on the surface but turns out to be nontrivial when
you actually try to do it. This may still be done at a later date,
but for now, just add explicit casts.
Oleg Oshmyan [Thu, 22 Oct 2015 23:20:19 +0000 (02:20 +0300)]
fontselect: don't trim font names
This matches the behavior of GDI and hence VSFilter.
Note that \fn arguments are trimmed during parsing.
However, none of the names inside fonts should be trimmed,
and @-prefixed fonts should keep whitespace following the @,
both of which this commit addresses.
Remove strdup_trimmed because it is no longer used. Also remove
the declaration of a function that was deleted a few months ago.
Oleg Oshmyan [Thu, 22 Oct 2015 22:55:06 +0000 (01:55 +0300)]
fontselect: don't find fonts with PostScript outlines by full name
Related to commit e00691e8096cc69e5651480155ebc61d9e079290:
it turns out that GDI (and hence VSFilter) does not check full names of
fonts that have PostScript outlines when searching for a font by name.
To summarize the resulting behavior:
* Fonts with PostScript outlines can be found by family name
and by PostScript name.
* Fonts without PostScript outlines can be found by family name
and by full name.
Oleg Oshmyan [Mon, 19 Oct 2015 13:28:33 +0000 (16:28 +0300)]
fontselect: read PostScript names for memory fonts
Currently this affects only the verbose output in
ass_font_select, but it will become more useful when we
start matching against PostScript names in the future.
Oleg Oshmyan [Wed, 14 Oct 2015 18:45:31 +0000 (21:45 +0300)]
configure: don't add unnecessary libraries to PKG_LIBS_PRIVATE
Library checks can succeed if the needed functions exist in libc
and don't need any extra linker flags. Avoid adding unnecessary
flags (which break static linking against libass) in this case.
wm4 [Mon, 12 Oct 2015 19:56:44 +0000 (21:56 +0200)]
ass: use a bitmap for checking duplicate events
The loop in check_duplicate_event() essentially makes event processing
with ass_process_chunk() O(n^2). Using a bitmap instead of a loop brings
it back to O(n).
This could be interpreted as an API change: since the event list is
freely modifieable by the API user through ASS_Track public fields,
libass can't know if the internal bitmap went out of sync with the
public event list. We just redefine it so that calling
ass_process_chunk() means the API user agrees not to manipulate the
event list otherwise.