507554 - Update in-tree libtheora to 1.1.0 (Thusnelda)

Status: RESOLVED FIXED

(Core :: Audio/Video, enhancement)

Description

[cajbir (:cajbir)]

The Theora Thusnelda version of the decoder has improvements to the decoding. It's faster and more efficient. We should look at moving to using this version of the decoder rather than the one we are using at the moment which is from the standard Theora svn repository.

Comment 1

[cajbir (:cajbir)]

Attachment: r16363 of theora-thusnelda svn

This will need testing to see if it breaks any videos. I've only tried building on x86 32-bit Linux.

Comment 2

[cajbir (:cajbir)]

Attachment: Update for missing theoraenc.h file

Tested build on tryserver and it highlighted a missing file. This patch adds it.

Comment 3

[Matthew Gregan [:kinetik]]

I tested BillysBrowser480.ogg (30s video) with sync disabled on Mac.  In a debug build, this patch is about 32% slower.  In an opt build, it's about 10% slower.

Comment 4

[Matthew Gregan [:kinetik]]

What's weird is that dump_video shows current Thusnelda SVN being about 5-10% faster than Theora 1.0 using the same video.

That's using the dump_video.c from Thusnelda SVN with both library versions, since dump_video.c was replaced with a rewrite recently (and the old and new versions seem to perform differently).

Comment 5

[cajbir (:cajbir)]

Have I missed a configure flag? Has the setting to turn assembler optimisations on changed maybe?

Comment 6

[Matthew Gregan [:kinetik]]

Yep.  OC_X86ASM was renamed to OC_X86_ASM.

Comment 7

[Matthew Gregan [:kinetik]]

Updating that define in media/libtheora/lib/Makefile.in gives us a 5% improvement over baseline in an opt build and 12% in debug (same test as comment #3).

Comment 8

[Matthew Gregan [:kinetik]]

Attachment: update to libtheora 1.1.0

Update to libtheora 1.1.0 release version.  Also rearrange the layout of the files to match the rearrangement that occurred upstream.  Fixes the OC_X86ASM typo.  Rebased and reapplied the WinCE build fixes.

The bug498770.patch fix is not applied, and won't apply easily because the file it applies to no longer exists.  The rewritten version of the functions it patches also use the EBX register without saving and restoring them, so I suspect we may run into the same crashes when built with FPO on Windows.

Comment 9

[Matthew Gregan [:kinetik]]

Revisiting my numbers from comment 7, on my current system a debug build is 10% faster and an optimized build is 4.5% faster using BillysBrowser480.ogg with video tab hidden (so, no painting cost included).

Comment 10

[Matthew Gregan [:kinetik]]

I haven't tested an FPO build on Windows yet (supplied the wrong command line to build), but a regular optimized build is pretty broken.  Videos play, but the decoded image is corrupt.  I'm seeing lots of "magic pink" and when playing from the start of a video, it looks garbage is used as the keyframe.

Comment 11

[Matthew Gregan [:kinetik]]

A build with assembly optimizations disabled confirms that the asm is broken on Windows in the 1.1.0 release.  Still haven't tested the FPO/EBX issue, but suspect that will be a problem too.

Comment 12

[Ralph Giles (:rillian)]

Attachment: proposed fix

Matthew, can you please try this patch, which replaces uses of ebx with edi or esi. This is in upstream svn as c16578.

Comment 13

[Matthew Gregan [:kinetik]]

A non-PGO/FPO build with that patch is still showing the corrupted decoded frames issue.  Investigating...

Comment 14

[Matthew Gregan [:kinetik]]

README_MOZILLA is out of date in my patch, must fix.

Comment 15

[Matthew Gregan [:kinetik]]

Thought I had narrowed it down to oc_state_frag_recon_mmx by binary searching the vtable init in mmxstate.c, but oc_state_frag_recon_mmx actually calls a bunch of the MMX routines directly without going via the vtable.

Running a debug build to see if disabling optimization changes the behaviour.

Comment 16

[Matthew Gregan [:kinetik]]

Attachment: screenshot

Looks like the Windows problem is optimized-build only.  Reproduced in MSVC 2005 Pro SP1 and MSVC 2005 Express SP1, regular optimized build (no PGO).

Comment 17

[Matthew Gregan [:kinetik]]

Attachment: win32 asm fix

Comment 18

[Matthew Gregan [:kinetik]]

That fix has been upstreamed, so I'll resync the sources with Theora SVN.

Fix explanation:  in the original code, |movd ECX,p| intends to move the value of p into ECX.  movd moves quadwords in and out of MMX registers, so the instruction ends up encoded as |movd MM1, p| (ECX and MM1 share the same encoding), leaving ECX containing whatever happened to be present earlier.   The fix is to use the correct zero-extending word mov instruction (movzx) to move p into ECX.

It's not a problem with optimization turned off because the compiler happens to compute |p| in ECX, so it has the correct value by luck without having to load it by hand.  In a release build, the compiler computes |p| using EAX and ECX contains the result of some other computation.

Comment 19

[Matthew Gregan [:kinetik]]

Attachment: update to libtheora 1.1.0 + SVN

Updates to SVN r16584 (includes my fix, Cristian's EBX fix, and the WinCE define renames).  This removes all local patches against upstream.  Updated README_MOZILLA to reflect new reality.

Tryservers have been fed this tasty new snack.

Comment 20

[Matthew Gregan [:kinetik]]

Builds are turning up here, for anyone who wants to do early testing: https://build.mozilla.org/tryserver-builds/[e-mail]-try-3e1d0cea3fe9/

Comment 21

[Matthew Gregan [:kinetik]]

Tests passed on one unit test machine for each platform, and timed out for
another.  The timeouts look like two or three unrelated problems.

The tree I pushed to tryserver was about 200 revs behind current trunk, so I've
repushed patch with the latest trunk.

Comment 22

[Matthew Gregan [:kinetik]]

New builds will be at:
http://build.mozilla.org/tryserver-builds/[e-mail]-try-319f2c67029c/

Comment 23

[Matthew Gregan [:kinetik]]

Comment on attachment 403059 [details] [diff] [review]
update to libtheora 1.1.0 + SVN

Only got one unit test box per platform this cycle.  Linux and Mac were green.  Windows timed out due to bug 518534 (unrelated).  So we've got 2x green cycles on Mac and Linux and 1x green cycle on Windows (plus 1x orange cycle on Linux and Mac and 2x orange cycles on Windows).

Comment 24

[Matthew Gregan [:kinetik]]

http://hg.mozilla.org/mozilla-central/rev/04a5d978b06e

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We need this on the 1.9.2 branch to fix bug

Comment 26

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 403059 [details] [diff] [review]
update to libtheora 1.1.0 + SVN

Let's take this on branch to fix bug 515882.

Comment 27

[Matthew Gregan [:kinetik]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/432ee8396816

Comment 28

[Daniel Veditz [:dveditz]]

I believe the decision on this one for the 1.9.1 branch was "wontfix" since it's a feature upgrade rather than simply fixes.