1053016 - Remove libomxpluginfroyo.so from Android build

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Makoto Kato [:m_kato]]

By Bug 1017242, Froyo is no longer supported.  But libomxpluginfroyo.so is in tree.  It is unnecessary.

Comment 1

[Nick Alexander :nalexander [he/him]]

There are a few parts to this ticket:

1) remove the case for the Froyo version from the C++ code around http://mxr.mozilla.org/mozilla-central/source/content/media/android/AndroidMediaPluginHost.cpp#196

2) remove the Froyo version from DIST_FILES around http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#405

That removes the plugin from the APK.  I'd be happy to make culling media/omx-plugin/froyo/ be a separate ticket, perhaps mentored by blassey or snorp, as Brad deems appropriate.

To test, rebuild and repackage everything before manually opening the APK file to verify that the Froyo plugin is not in the resulting package.

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Nick Alexander :nalexander from comment #1)
> There are a few parts to this ticket:
> 
> 1) remove the case for the Froyo version from the C++ code around
> http://mxr.mozilla.org/mozilla-central/source/content/media/android/
> AndroidMediaPluginHost.cpp#196
> 
> 2) remove the Froyo version from DIST_FILES around
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/
> packager.mk#405
> 
> That removes the plugin from the APK.  I'd be happy to make culling
> media/omx-plugin/froyo/ be a separate ticket, perhaps mentored by blassey or
> snorp, as Brad deems appropriate.
> 
> To test, rebuild and repackage everything before manually opening the APK
> file to verify that the Froyo plugin is not in the resulting package.

What info do you need?

Comment 3

[Richard Newman [:rnewman]]

Attachment: Part 1: remove libomxpluginfroyo.so from Android package. v1

Comment 4

[Richard Newman [:rnewman]]

Attachment: Part 2: remove Froyo conditional from AndroidMediaPluginHost.cpp. v1

Comment 5

[Richard Newman [:rnewman]]

Attachment: Part 3: eliminate omx-plugin Froyo version. v1

Brad, please redirect review as appropriate.

This is a pretty sledgehammer approach, but it builds. It doesn't completely remove all Froyo code (see Bug 1041882), but it's a pretty good start.

Comment 6

[Richard Newman [:rnewman]]

Confirmed that libomxpluginfroyo.so is no longer packaged into the APK with these changes. And we save a surprising amount of space.

Comment 7

[Richard Newman [:rnewman]]

Attachment: Part 2: remove Froyo conditional from AndroidMediaPluginHost.cpp. v2

Comment 8

[Richard Newman [:rnewman]]

Comment on attachment 8485346 [details] [diff] [review]
Part 2: remove Froyo conditional from AndroidMediaPluginHost.cpp. v2

This change slipped into Part 3. Fixing.

Comment 9

[Richard Newman [:rnewman]]

Attachment: Part 3: eliminate omx-plugin Froyo version. v2

Comment 10

[Richard Newman [:rnewman]]

Comment on attachment 8485347 [details] [diff] [review]
Part 3: eliminate omx-plugin Froyo version. v2

Without embedded Part 2!

Comment 11

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8485342 [details] [diff] [review]
Part 1: remove libomxpluginfroyo.so from Android package. v1

Review of attachment 8485342 [details] [diff] [review]:
-----------------------------------------------------------------

wfm.

Comment 12

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/314e44e93c71
https://hg.mozilla.org/integration/fx-team/rev/498a3f688a3d

Comment 13

[Richard Newman [:rnewman]]

Figured I'd get the first two patches out of my queue while waiting for review on the third.

(And thanks, Brad!)

Comment 14

[Nigel Babu [:nigelb]]

Backed out in https://hg.mozilla.org/integration/fx-team/rev/5487e2fdacf6 for Android build bustage

Comment 15

[Richard Newman [:rnewman]]

Ah, a hunk slipped into Part 3:

 ifdef MOZ_OMX_PLUGIN
 DIST_FILES += libomxplugin.so libomxplugingb.so libomxplugingb235.so \
-              libomxpluginhc.so libomxpluginfroyo.so libomxpluginkk.so
+              libomxpluginhc.so libomxpluginkk.so
 endif

That'll do it.

Guess I'll wait to land all three!

Comment 16

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/98f9426b777f
https://hg.mozilla.org/integration/fx-team/rev/541261861115
https://hg.mozilla.org/integration/fx-team/rev/bb7be324d2b5

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/98f9426b777f
https://hg.mozilla.org/mozilla-central/rev/541261861115
https://hg.mozilla.org/mozilla-central/rev/bb7be324d2b5