906929 - Improve performance of Stagefright internal http server

Status: RESOLVED WONTFIX

(Core :: Audio/Video: Playback, defect)

Description

[cajbir (:cajbir)]

The stagefright internal http server introduced in bug 860599 is not optimal for performance. It uses a blocking/threaded structure with unneeded copying of data. This should be improved as recommended by Honza Bambas un bug 860599 comment 34.

Comment 1

[Brian Carr]

I would be happy to take a look if no one has fixed this yet.  Just let me know what files I'll need to look at and I'll get started.

Comment 2

[cajbir (:cajbir)]

That would be great! The existing http server code is in:

content/media/plugins/MediaResourceServer.h
content/media/plugins/MediaResourceServer.cpp

The server is instantiated in:

content/media/plugins/MediaPluginHost.cpp

It is accessed from:

media/omx/OmxPlugin.cpp (OmxDecoder::Init)

The main code will bein the MediaResourceServer files.

Comment 3

[Brian Carr]

Alright, I've taken a look at the bug you linked in your first comment.  I'm completely new to this codebase so I don't really know what a lot of these classes do yet.  From what I gathered from Honza's post, the current implementation is inefficient because of how many times it unnecessarily copies code.  It looks like he has identified the copy to a pipe here http://hg.mozilla.org/mozilla-central/annotate/dbd7d55d64ff/netwerk/base/src/nsSocketTransport2.cpp#l1772 in the routine for opening an input stream (when OPEN_BLOCKING is passed as a parameter) as a copy that could be eliminated.  It looks to me like the code for opening an unbuffered input stream in the same function does avoid this copy because it doesn't use the pipe, however, wouldn't we run into concurrency issues with that approach or am I completely confused?  

I'm also quite confused about what this suggestion by Honza means: "You may implement nsIInputStream that wraps the MediaResource.  This way you can use stream copier (via WriteSegments to the socket output stream)."

Comment 4

[Josh Matthews [:jdm]]

nsIInputStream is an interface defined here: http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsIInputStream.idl . It allows reading from some arbitrary source in a controlled, efficient way. There are various examples of classes that implement it, usually named nsSomethingStream: http://mxr.mozilla.org/mozilla-central/source/xpcom/io/

Comment 5

[Brian Carr]

Ok first of all sorry I disappeared for a few days, between classes, job interviews and a midterm I was just absolutely swamped.

Anyways, I've read through those links Josh posted and several other interfaces involved and I think I'm starting to get at least some idea of what's going on here.  I looks like what I need to do to start is make a new class that implements nsIInputStream, and wraps the MediaResource class so that we have a clean, controlled way of reading from the MediaResource and can use stream copier?

Comment 6

[B. Dahse]

This sounds like something I would like to work on - of course, provided that it is still needed?

Comment 7

[Mike Hoye [:mhoye]]

I'm not sure - Josh, do you have any insight here?

Comment 8

[B. Dahse]

Certainly something has changed since comment 2 because I am no longer able to locate the code.

Comment 9

[Josh Matthews [:jdm]]

Looks like the files form comment 2 are in dom/media/android/ now: https://dxr.mozilla.org/mozilla-central/source/dom/media/android/AndroidMediaResourceServer.h

Comment 10

[B. Dahse]

Thanks Josh. Looking briefly at the code, I assume this is still relevant?

Comment 11

[Josh Matthews [:jdm]]

Maybe Anthony can answer that question, since he oversees the media team.

Comment 12

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

This only affects older android versions. We're not going try to fix anything here.

Comment 13

[B. Dahse]

Fair enough, I will find something else.