Closed Bug 1053016 Opened 6 years ago Closed 6 years ago

Remove libomxpluginfroyo.so from Android build

Categories

(Core :: Audio/Video, defect)

All
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: m_kato, Assigned: rnewman, Mentored)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 2 obsolete files)

By Bug 1017242, Froyo is no longer supported.  But libomxpluginfroyo.so is in tree.  It is unnecessary.
Blocks: fatfennec
Summary: Remove libomxpluginfroyo.so from android build → Remove libomxpluginfroyo.so from Android build
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.
Mentor: nalexander, blassey.bugs
Flags: needinfo?(blassey.bugs)
Whiteboard: [lang=c++][good second bug]
(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?
Flags: needinfo?(blassey.bugs)
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
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.
Attachment #8485344 - Flags: review?(blassey.bugs)
Confirmed that libomxpluginfroyo.so is no longer packaged into the APK with these changes. And we save a surprising amount of space.
Comment on attachment 8485346 [details] [diff] [review]
Part 2: remove Froyo conditional from AndroidMediaPluginHost.cpp. v2

This change slipped into Part 3. Fixing.
Attachment #8485346 - Attachment description: Part 2: remove Froyo conditional from AndroidMediaPluginHost.cpp → Part 2: remove Froyo conditional from AndroidMediaPluginHost.cpp. v2
Attachment #8485346 - Flags: review?(blassey.bugs)
Attachment #8485343 - Attachment is obsolete: true
Attachment #8485343 - Flags: review?(blassey.bugs)
Attachment #8485344 - Attachment is obsolete: true
Attachment #8485344 - Flags: review?(blassey.bugs)
Comment on attachment 8485347 [details] [diff] [review]
Part 3: eliminate omx-plugin Froyo version. v2

Without embedded Part 2!
Attachment #8485347 - Attachment description: Part 3: eliminate omx-plugin Froyo version → Part 3: eliminate omx-plugin Froyo version. v2
Attachment #8485347 - Flags: review?(blassey.bugs)
Comment on attachment 8485342 [details] [diff] [review]
Part 1: remove libomxpluginfroyo.so from Android package. v1

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

wfm.
Attachment #8485342 - Flags: review?(nalexander) → review+
Attachment #8485346 - Flags: review?(blassey.bugs) → review+
Attachment #8485347 - Flags: review?(cajbir.bugzilla)
Attachment #8485347 - Flags: review?(blassey.bugs)
Attachment #8485347 - Flags: feedback+
Figured I'd get the first two patches out of my queue while waiting for review on the third.

(And thanks, Brad!)
Whiteboard: [lang=c++][good second bug] → [leave open until part 3 lands]
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!
Attachment #8485347 - Flags: review?(cajbir.bugzilla) → review+
Whiteboard: [leave open until part 3 lands]
Duplicate of this bug: 1053278
You need to log in before you can comment on or make changes to this bug.