waddou / libass

Automatically exported from code.google.com/p/libass
1 stars 0 forks source link

API cleanup #96

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
Motivation for this request:

The libass API evolved from an internal MPlayer API. It hasn't aged well. It is 
full of cruft and isn't well documented and usage is often not intuitive.

Description of the feature or improvement:

Here's a collection of ideas for an improved API:

- Confusing cruft, like the second argument to ass_set_aspect_ratio or the 
'update' parameter to ass_set_fonts, should be removed.

- The 'const' keyword should be used whenever possible.

- ASS_Library is quite useless and confusing. Some state is global (e.g. fonts, 
logging), but other state is part of ASS_Renderer. Multithreading or handling 
multiple renderer instances at the same time can be a huge mess. ASS_Library 
functionality should be incorporated into ASS_Renderer.

- The font management APIs need an overhaul. Especially ass_set_fonts is just 
crap. I'm not quite sure how it should be improved, yet.

- libass internals and the public API shouldn't use the same set of structs for 
storing track and style data. More abstraction is needed. A getter/setter type 
API is probably suitable.

Original issue reported on code.google.com by g...@chown.ath.cx on 20 Mar 2013 at 1:03

GoogleCodeExporter commented 8 years ago

Original comment by g...@chown.ath.cx on 20 Mar 2013 at 1:11

GoogleCodeExporter commented 8 years ago
I never really understood ASS_Library. mplayer2 and mpv keep the ASS_Library 
around just as long as the ASS_Renderer. However, do take note of this: 
http://repo.or.cz/w/mplayer.git/blob/dd066519f6266d562e8575bdc0a411590b573aef:/m
player.c#l4559 (the ASS_Renderer is destroyed and recreated on file change, due 
to an unspecified libass bug).

I'd like to append to this list:
- Margin handling is not obvious - maybe this can be improved?
- ass_read_styles() should probably go. Unfortunately, it's different from 
ass_set_style_overrides()
- ass_process_force_style is not obvious.
- Functions like ass_free_event() are extremely awkward (look what they do: it 
frees the event without removing it, even though you pass an event index)
- The documentation could be improved. Sometimes functions have doxygen both in 
the headers and the .c files, and the one in the .c file is more extensive 
(like ass_render_frame).

I have some other gripes with libass, but maybe these matter only with more 
obscure use-cases:
- The fact that ass_render_frame() returns a "volatile" list is not very 
useful. For example, considering you want to use a multithreading to render the 
resulting ASS_Image lists, and perhaps render them on different threads. You 
can't just call ass_render_frame() twice, and distribute the results to the 
threads. You can't even create two renderers, because ASS_Track.render_priv 
contains mysterious state, and the two renderers will mess it up. Maybe the 
user can get more control about when the cache is emptied?
- What if you want to render several tracks at once? You have the previous 
problem. Even if you use multiple ASS_Renderers, you'll waste memory by loading 
fonts twice etc.
- change detection sucks too.
- It would also be nice if the image list could maybe be segmented per event. 
This would make converting to RGBA simpler, which I bet most programs want. The 
current ASS_Image format is a bit obscure, somewhat hard to implement a GPU 
renderer for (look what mplayer2/mpv do: packing the images into a texture, 
creating vertex lists, etc.), and in the end it's not even very efficient. I 
suspect transfering all this bitmap data costs a lot CPU->GPU memory bandwidth, 
where a single RGBA texture would be smaller. Or actually, provide alternate 
APIs with allow a) outputting a list of RGBA images, and b) outputting a single 
frame-sized RGBA texture with a list of bounding boxes that are not transparent.

On the other hand, I think API changes should try to stay as compatible as 
possible, even if it's painful. At least an easy migration path should be 
provided (it's the worst for programs which try to stay compatible with both 
old and new versions).

Last but not least, messing with the subtitle data structures 
(styles/events/tracks) is quite useful if you're displaying something that's 
not necessarily ASS subtitles. mplayer(2) does a good bit of that for 
displaying SRT subtitles, and mpv goes further and uses it for OSD rendering.

I think most of these changes could be done gradually by (1) introducing fixed 
API and deprecating the old functions and (2) removing the deprecated APIs.

Also see #60 for reactions and questions of someone who just came in contact 
with libass.

Original comment by nfxjfg@googlemail.com on 20 Mar 2013 at 2:04

GoogleCodeExporter commented 8 years ago
So how would you manage logging for ASS_Track?

Also, I would prefer if there's some way to share fonts between ASS_Renderers, 
as well as cache state (to reduce memory usage when two renderers use the same 
fonts anyway). It appears this is the only reason why mplayer2/mpv go out of 
their way to use single global ASS_Library and ASS_Renderer objects, which also 
makes their architecture worse.

Original comment by nfxjfg@googlemail.com on 22 Jun 2013 at 4:54