[MPlayer-dev-eng] [PATCH] Dynamic list of EOSD sources

[Nicolas George]

L'octidi 28 fructidor, an CCXVIII, Reimar Döffinger a écrit :
> This loop and the one in eosd_image_first do the same thing
> and so should use the same code.

I think I managed to factor it satisfactorily.

> I'd also suggest writing eosd_image_first as
> images->source = images->first_source;
> images->image  = images->source->images;

Does not work if images->first_source is NULL. But this part is gone anyway.

> Looking at the code without inlining doesn't give you any useful information
> at all.

I was not clear: I disabled the inlining of my_draw_bitmap to see if the
cause of the performance loss was a change in the code. I found that:

- master/my_draw_bitmap-inlined and master/my_draw_bitmap-not_inlined
  have no visible difference of performance;

- dynamic/my_draw_bitmap-inlined and dynamic/my_draw_bitmap-not_inlined
  have no visible difference of performance;

- dynamic/my_draw_bitmap-* is significantly slower than
  master/my_draw_bitmap-*;

- dynamic/my_draw_bitmap-not_inlined and master/my_draw_bitmap-not_inlined
  have exactly the same assembly code.

> Turion is the cheap model, I can't imagine it has more cache.

It probably has less. It may exhibit the "bad" performances on both cases.

> I'd also suggest to try marking the while loop that looks for the next source
> as unlikely (using __builtin_expect).

For my tests, I went further: I replaced eosd_image_next by image->next,
since there was only the ASS source. It did not change anything at all. If I
remember correctly, eosd_image_next amounted to less than 1/100 of the total
time.

> But I think the performance degradation is acceptable if necessary, it
> should be easy enough to optimize this if necessary/wanted.

I believe so too. Furthermore, I think that vf_ass was not especially
optimized: I quickly tried two small ideas (separating the loops for Y, U
and V, and removing a division using some arithmetic) and the speed gain was
greater than the performance loss we are discussing. This belongs in a
separate patch, though.

Attached: updated version:

- Added a eosd_image_remove function, useful for the next patch and probably
  other future sources.

- Code factored between eosd_image_first and eosd_image_next.

- Some more = aligned.

- Bugfix: images->changed was not set.

- VDPAU STILL UNTESTED.

Plus, the slave command overlay_add:

- Updated to the new API. Much simpler than the previous version.

- Added the documentation.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-eosd-01-dynamic-20100915-1438.diff
Type: text/x-diff
Size: 23095 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100915/344c98ee/attachment-0002.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-eosd-02-command-20100915-1438.diff
Type: text/x-diff
Size: 4882 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100915/344c98ee/attachment-0003.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100915/344c98ee/attachment-0001.pgp>