Last Comment Bug 451674 - Expose camera functionality to web content
: Expose camera functionality to web content
Status: RESOLVED DUPLICATE of bug 692955
: mobile
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: x86 Windows XP
: -- normal with 13 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
: 496791 541842 648704 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-21 22:26 PDT by Stuart Parmenter
Modified: 2012-01-21 15:34 PST (History)
68 users (show)
pavlov: wanted1.9.1+
mozaakash: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP patch inplements support for video4linux2 (7.63 KB, patch)
2008-08-25 08:46 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
WIP v2, works on device (9.80 KB, patch)
2008-08-28 09:24 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
WIP v3, camera tag works (21.19 KB, patch)
2008-08-29 18:10 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
WIP v4, decoders share pipeline (20.76 KB, patch)
2008-09-03 13:44 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
WIP v5, enables gstreamer only for camera (26.02 KB, patch)
2008-09-05 13:24 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
combined patch with gstreamer backend and camera stuff for context (50.16 KB, patch)
2008-09-08 07:27 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
WIP new approach (69.51 KB, patch)
2009-07-25 11:44 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
WIP patch v.2, moz-device:// (69.90 KB, patch)
2009-07-27 12:37 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Review
Configure goop for gstreamer (4.27 KB, patch)
2010-08-05 13:57 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
no flags Details | Diff | Review
Build glue (2.79 KB, patch)
2010-09-06 10:35 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
mitchell.field: review+
Details | Diff | Review
Turn on capture in Fennec on linux (321 bytes, patch)
2010-09-06 10:40 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
blassey.bugs: review+
Details | Diff | Review
GStreamer Capture (31.34 KB, patch)
2010-09-06 11:57 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
no flags Details | Diff | Review
GStreamer Capture (29.34 KB, patch)
2010-09-06 12:05 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
cajbir.bugzilla: review-
Details | Diff | Review
Hook gstreamer capture into the device protocol handler (1.62 KB, patch)
2010-09-06 16:10 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
josh: review+
Details | Diff | Review

Description Stuart Parmenter 2008-08-21 22:26:52 PDT
It would be great if I could have a web page that could take photos using the camera on my laptop or the camera on my phone.
Comment 1 Jonas Sicking (:sicking) 2008-08-22 10:34:48 PDT
So is the goal to only allow this to happen on pages that specifically opt in? Or should we try to allow it on any page with a file upload?
Comment 2 Brad Lassey [:blassey] (use needinfo?) 2008-08-22 11:18:56 PDT
I think this needs to be explicit functionality.  I'm thinking of using the w3c device upload "spec," so you'd have <input type="file" device="camera"/>
Comment 3 Mark Finkle (:mfinkle) (use needinfo?) 2008-08-22 11:40:53 PDT
(In reply to comment #1)
> So is the goal to only allow this to happen on pages that specifically opt in?
> Or should we try to allow it on any page with a file upload?
> 

What kind of opt-in is needed? If the <input type="file" device="camera"/> appears in a page is that good enough? Should the control have a button to actually take the picture (akin to the button to select a file)? Since the user needs to press the button, that is kind of opt-in, right?
Comment 4 Brad Lassey [:blassey] (use needinfo?) 2008-08-22 14:24:19 PDT
So there needs to be "opt-in" for both the web developer and for the user.  From the web developer's point of view, they need to explicitly use device="camera" to get this new UI element.  From the user's point of view, they'll have to explicitly click a "take photo" button to take the photo.  The page will display with a live video preview of the photo, but the at data won't be accessible to the web developer until the "take photo" button is pressed.
Comment 5 Jonas Sicking (:sicking) 2008-08-22 16:51:16 PDT
So there is actually already a feature in HTML4 (and expanded in HTML5) that we can use. Basically saying

<input type=file accept="image/*">

should allow the user to use the camera. Though we should probably also allow the user to attach any previously taken pictures.

Full spec here: http://www.whatwg.org/specs/web-forms/current-work/#accept0

Which also allows things like:
<input type=file accept="application/word-document, application/odf-document">

but I think for our purposes just looking for "image/*" is fine.

Ideal would be if we also allowed things like "image/jpeg" or "image/gif" and encoded into that format when sending. But that seems secondary.
Comment 6 Jesse Ruderman 2008-08-23 16:31:09 PDT
For a single photo, an extension to <input type="file"> could work, but what about continuous video from the user's camera like Flash allows?
Comment 7 Jonas Sicking (:sicking) 2008-08-24 03:49:33 PDT
Well, for a short video sequence

<input type=file accept="video/*">

works fine. We simply upload it as a ogg or mpeg or whatever we support for encoding. For something like streaming video to implement video chatting we need something different.
Comment 8 Brad Lassey [:blassey] (use needinfo?) 2008-08-25 08:46:23 PDT
Created attachment 335366 [details] [diff] [review]
WIP patch inplements support for video4linux2 

This is a very rough patch that depends on the gstreamer decoder for the video tag.  It looks for the protocol "camera://" in the video tag's src attribute.  Currently video from a video4linux2 source is supported. The idea would be to store the camera's settings in prefs.

The "tee" element in the pipeline allows you to stream to multiple sinks, one of which being the content area of the page.  Popping up a second window for debugging is implemented (surrounded by #ifdef DEBUG_VIDEO).  Producing image files or video streams from this is reletively d
Comment 9 Brad Lassey [:blassey] (use needinfo?) 2008-08-28 09:24:55 PDT
Created attachment 335909 [details] [diff] [review]
WIP v2, works on device

there's something wrong with my xbl binding (camera.xml). The video doesn't show up when I use the camera tag, even though the same code in xul works fine.  Also, when using the camera tag, SetRGBData is being called, but still nothing shows up on screen.
Comment 10 Brad Lassey [:blassey] (use needinfo?) 2008-08-29 18:10:58 PDT
Created attachment 336145 [details] [diff] [review]
WIP v3, camera tag works

the issue I was seeing before with the video not showing up when part of an xbl binding was caused by the nsHTMLMediaElement being cloned.  As a result, there were two video decoders trying to access /dev/video0.  The fist one could, which is why I was seeing SetRGBData being called... but the second one couldn't, which was the one being displayed.

The solution I'm using in this patch is to share a reference to the decoder when being cloned.  This probably isn't the ideal solution, and even if it is there are things not being handled by this patch, such as what happens when one media element tries to shut down the decoder that the other is still using.  Another solution would be to shutdown the decoder (freeing the camera device) before cloning.  Right now, Shutdown is a protected function, this would have to be made public. (Also, it seems that the media (and video) element is calling Stop() when it should be calling Shutdown, such as in the deconstructor or in PickMediaElement.  This might be a separate bug) 

To test this patch, just put a <camera> tag into a xul document.  It works on the n800/n810 or a linux box with a video4linux2 camera attached (for example an mbp).
Comment 11 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-09-02 12:06:19 PDT
We don't want to share the decoder, that's going to be incorrect when one <video> element changes state (e.g. does a seek, or changes the playback rate, or just does pause/play/stop).

The source element for the clone is not actually playing, right? Maybe you can avoid opening the device for elements that aren't playing. That would get your demos working and probably be a good idea in general.

However, if there are two pages or even two applications both trying to read from the camera at the same time, there's really no reason why they shouldn't be able to. That sounds like a bug at the GStreamer level or even lower.
Comment 12 Brad Lassey [:blassey] (use needinfo?) 2008-09-03 13:44:10 PDT
Created attachment 336703 [details] [diff] [review]
WIP v4, decoders share pipeline

this wraps the pipeline, camera source and a tee element in a nsCameraSource, then creates a queue for each decoder to link to.
Comment 13 Brad Lassey [:blassey] (use needinfo?) 2008-09-05 13:24:58 PDT
Created attachment 337115 [details] [diff] [review]
WIP v5, enables gstreamer only for camera

Minor change to allow us to turn on the gstreamer backend for the camera source only with the ac flag --enable-gstreamer-camera
Comment 14 Brad Lassey [:blassey] (use needinfo?) 2008-09-06 11:59:21 PDT
roc, does this approach work for you?
Comment 15 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-09-06 23:18:50 PDT
I think it's OK, but you've diffed against something other than trunk so I'm not 100% sure. Chris D, what do you think?

+      NS_NAMED_LITERAL_STRING(ogg_ext, "camera:");
+      if (FindInReadable(ogg_ext, start, end,  
+                         nsCaseInsensitiveStringComparator())) {

ogg_ext is the wrong name here.
Comment 16 Brad Lassey [:blassey] (use needinfo?) 2008-09-08 07:27:23 PDT
Created attachment 337446 [details] [diff] [review]
combined patch with gstreamer backend and  camera stuff for context
Comment 17 Ben Bucksch (:BenB) 2009-03-16 09:44:28 PDT
Taking photos is extremely privacy sensitive and can, when used without the user noticing, represent a surveillance device inside my house. I personally don't feel comfortable seeing this feature at all in Firefox. But, assuming this is accepted:
Given the tricks we've seen with <input type="file"> (ask Jesse Ruderman), please be sure to have explicit consent from the user, in form of a dialog box that cannot possibly be accidentally clicked nor the user tricked into clicking.
Comment 18 Γριφεγ 2009-03-18 10:24:21 PDT
1. Accidental clicks will always happen.
2. Some users will always be tricked.

But, the additional functionality is worth the risks, no?
Comment 19 Brad Lassey [:blassey] (use needinfo?) 2009-07-25 11:44:25 PDT
Created attachment 390662 [details] [diff] [review]
WIP new approach 

This patch implements a protocol handler for camera:// as well as a camera channel and camera input stream. The channel/input stream can output raw rgb or ogg theora video, which can be specified by the url (camera://encoding=ogg). This also implements a raw video "decoder."

With this patch <video src="camera">, <xul:camera> and <input type="camera"> all work in firefox and fennec on the n810 and my mac book pro.
Comment 20 Brad Lassey [:blassey] (use needinfo?) 2009-07-25 11:45:35 PDT
this no longer depends on the gstreamer backend for the video tag
Comment 21 Daniel Veditz [:dveditz] 2009-07-25 20:11:16 PDT
(In reply to comment #19)
> which can be specified by the url (camera://encoding=ogg).

Don't mean to bikeshed here, but scheme+"://" usually indicates a hierarchical URL to many apps which expect to find scheme+"://"+host+path  

In particular, nsStandardURL which you're using interprets things that way, and parsing "encoding=ogg" as a hostname may give you odd results here and there.

I don't know your scheme's syntax, but if you don't need the path parsing functionality nsSimpleURL is lighter weight. (that's what data: and javascript: use).

Is this a scheme you're going to expose to third party apps or expect other apps to implement? If not we probably shouldn't squat on the prime "camera" name and use moz-camera or something. If you're hoping this becomes standard/common functionality then camera: sounds great.
Comment 22 Olli Pettay [:smaug] 2009-07-26 04:18:46 PDT
(In reply to comment #21)
> Is this a scheme you're going to expose to third party apps or expect other
> apps to implement? If not we probably shouldn't squat on the prime "camera"
> name and use moz-camera or something. If you're hoping this becomes
> standard/common functionality then camera: sounds great.

But before it becomes a standard, using moz-camera sounds better.

(Or perhaps something like moz-device://camera?image/jpg)
Comment 23 Brad Lassey [:blassey] (use needinfo?) 2009-07-27 12:37:38 PDT
Created attachment 390870 [details] [diff] [review]
WIP patch v.2, moz-device://

I like the suggestion of moz-device (which I do hope we can eventually standardize), because its forward looking and would allow for capture from other devices like a mic.

Dan, with this new scheme "camera" would be the host, so nsStandardURL seems newly appropriate.  Please correct me if I'm wrong.
Comment 24 Ben Bucksch (:BenB) 2009-07-27 16:12:57 PDT
We could even do a "&authorization=police" parameter, which skips the user confirmation.</sarcasm>

I don't think "camera" should be a host (the host is the user's machine). Dan is right, neither :// nor StandardURL fit here.
Comment 25 Biju 2009-09-07 00:06:11 PDT
(In reply to comment #24)
> I don't think "camera" should be a host (the host is the user's machine).

Then moz-device:camera?type=image/jpg&more_parameters=some_value may be better.
That will facilitate provide future options like
* moz-device:camera?type=image/ogg&frames_per_second=15&size=360x400
* moz-device:microphone?type=image/ogg&rate=10kHz
Comment 26 Sergey «Mithgol the Webmaster» Sokoloff 2009-11-16 01:26:12 PST
> Taking photos is extremely privacy sensitive and can, when used without the
user noticing, represent a surveillance device inside my house.

There are implications of this feature far beyond home surveillance and user's consent.

Imagine some crowd IRL where 10% of people are sharing their video input through Fennec on Maemo on Nokia N900, and anyone can see (online) the street view throught their eyes (like Google Street View, but live), which looks like an extremely great online experience, like nothing ever before.

Now imagine yourself walking in that croud, being constantly monitored (and broadcasted online) by several anonymous volunteers without your prior consent. Suddenly the application doesn't look so great anymore, does it?
Comment 27 Sergey «Mithgol the Webmaster» Sokoloff 2009-11-16 01:29:37 PST
By the way, this bug should block bug 479046, quiaff?
Comment 28 Sergey «Mithgol the Webmaster» Sokoloff 2009-11-16 04:02:31 PST
And while you're at it: some applications (like geographically augmented reality, see bug 528886) does not require any uploading of camera shots or a video stream, they just need a rectangular area on the web page to display the camera input _as_it_is_ (like <img> for Web images, or <video> for Web video streams), and to be able to place objects over the camera frame (with higher z-index, for example), and to change (move, replace) these objects when the camera's geolocation (navigator.geolocation) or tilt (window.onmozorientation) or azimuth changes.
Comment 29 Sergey «Mithgol the Webmaster» Sokoloff 2009-12-13 13:52:18 PST
The Capture API (W3C):

http://dev.w3.org/2009/dap/camera/
Comment 30 Jo Hermans 2010-01-25 04:07:14 PST
*** Bug 541842 has been marked as a duplicate of this bug. ***
Comment 31 Ben Bucksch (:BenB) 2010-01-26 13:42:45 PST
See bug 507749 for implementation.
Comment 32 Kyle Huey [:khuey] (khuey@mozilla.com) 2010-08-05 13:57:54 PDT
Created attachment 463286 [details] [diff] [review]
Configure goop for gstreamer
Comment 33 cajbir (:cajbir) 2010-08-05 17:00:54 PDT
Note that bug 422540 also contains configure goop for gstreamer and has requested review. How different are they?
Comment 34 Kyle Huey [:khuey] (khuey@mozilla.com) 2010-08-05 17:02:18 PDT
Very similar, I wasn't aware that that bug was still alive.
Comment 35 Stuart Parmenter 2010-08-06 15:54:55 PDT
I'd prefer to get this in and not make it depend on that bug, which could take a while to get in.
Comment 36 cajbir (:cajbir) 2010-08-06 22:10:46 PDT
I agree it shouldn't depend on the gstreamer bug. Has licensing@mozilla.org been contacted about the use of gstreamer and the LGPL license?
Comment 37 Lars Gunther 2010-08-07 00:56:10 PDT
Webkit has just gotten a bug for this: https://bugs.webkit.org/show_bug.cgi?id=43572
Comment 38 Kyle Huey [:khuey] (khuey@mozilla.com) 2010-08-14 23:02:38 PDT
*** Bug 496791 has been marked as a duplicate of this bug. ***
Comment 39 Kyle Huey [:khuey] (khuey@mozilla.com) 2010-09-06 10:35:30 PDT
Created attachment 472428 [details] [diff] [review]
Build glue

Lets do this instead.  We add MOZ_CAPTURE that can be set in the app's confvars and then we select the appropriate packages (if any) based on the platform.  When the other bug lands with a gstreamer <video> backend this can just be changed to force MOZ_GSTREAMER to 1 and let that deal with finding the gstreamer CFLAGS and LIBS.
Comment 40 Kyle Huey [:khuey] (khuey@mozilla.com) 2010-09-06 10:40:58 PDT
Created attachment 472429 [details] [diff] [review]
Turn on capture in Fennec on linux
Comment 41 Kyle Huey [:khuey] (khuey@mozilla.com) 2010-09-06 11:57:06 PDT
Created attachment 472442 [details] [diff] [review]
GStreamer Capture
Comment 42 Kyle Huey [:khuey] (khuey@mozilla.com) 2010-09-06 12:05:39 PDT
Created attachment 472443 [details] [diff] [review]
GStreamer Capture

Forgot to qref
Comment 43 Mitchell Field [:Mitch] 2010-09-06 12:43:47 PDT
Comment on attachment 472428 [details] [diff] [review]
Build glue

r=Mitch, when you sort those added headers alphabetically. Thanks.
Comment 44 Brad Lassey [:blassey] (use needinfo?) 2010-09-06 14:56:28 PDT
Comment on attachment 472429 [details] [diff] [review]
Turn on capture in Fennec on linux

>+if test "$OS_ARCH" == "Linux"; then

seems like the rest of the code base uses a single '=' for this test
Comment 45 Kyle Huey [:khuey] (khuey@mozilla.com) 2010-09-06 16:10:00 PDT
Created attachment 472494 [details] [diff] [review]
Hook gstreamer capture into the device protocol handler
Comment 46 Paul Rouget [:paul] 2010-10-13 04:50:28 PDT
No video for me.

As an image, it works:
<img src="moz-device:camera?type=image/png>

But as a video, it's doesn't:
<video src="moz-device:camera?type=video/x-raw-yuv"></video>

With these warnings:

GStreamerInputStream::GStreamerInputStream
GStreamerInputStream::~GStreamerInputStream

(firefox-bin:5085): GLib-GObject-WARNING **: invalid unclassed pointer in cast to `GObject'

(firefox-bin:5085): GLib-GObject-WARNING **: instance with invalid (NULL) class pointer

(firefox-bin:5085): GLib-GObject-CRITICAL **: g_signal_handler_disconnect: assertion `G_TYPE_CHECK_INSTANCE (instance)' failed
Comment 47 Paul Rouget [:paul] 2010-10-13 05:20:20 PDT
In debug mode:

###!!! ASSERTION: Cannot set both a frame limit and a time limit!: 'aParams->frameLimit == 0 || aParams->timeLimit == 0', file /home/paul/dev/buildbox/central/src/netwerk/protocol/device/GStreamerCaptureProvider.cpp, line 522
Comment 48 cajbir (:cajbir) 2010-10-18 15:43:26 PDT
Comment on attachment 472443 [details] [diff] [review]
GStreamer Capture

>+struct nsRawVideo_PRUint24 {

This, and other nsRawVideo stuff, appears to be the same as code in the content/media/raw decoder. It should be moved out into a common file and shared.

>+  operator PRUint32() const
>+  { return value[2] << 16 | value[1] << 8 | value[1]; }

That last value[1] should be value[0]. I've not checked any of the other Raw based stuff, assuming you'll move to sharing the content/media/raw code.
Comment 49 cajbir (:cajbir) 2010-10-18 16:08:46 PDT
Comment on attachment 472443 [details] [diff] [review]
GStreamer Capture

>+                   (const char*)&header,

Use C++ style casts. static_cast, reinterpret_cast, etc, for this and all other C style casts in the patch.

>+  if (!(mConv && mScale && mSink)) {
>+    g_printerr("Failed to create GStreamer pipeline elements");
>+    return NS_ERROR_FAILURE;
>+  }

Use Gecko logging facilities rather than g_printerr.

>+  if (!gst_element_link_filtered(mConv, mScale, NULL))
>+    return NS_ERROR_NOT_IMPLEMENTED;
>+
>+  if (!gst_element_link_filtered(mScale, mSink, sinkcaps))
>+    return NS_ERROR_NOT_IMPLEMENTED;

Do you need to unref the previously created sinkcaps before returning here?

>+  // Create a pad on the bin to receive data.
>+  GstPad* firstPad = gst_element_get_static_pad(mConv, "sink");
>+  gst_element_add_pad(mBin, gst_ghost_pad_new("sink", firstPad));
>+  gst_object_unref(GST_OBJECT(firstPad));

Check return value of get_element_get_static_pad.

>+GStreamerCaptureProvider::GStreamerCaptureProvider()
>+: mPipelineCount(0)
>+{
>+  gst_init(nsnull, nsnull);

The docs for gst_init say:

  "This function should be called before calling any other GLib functions."

How are you ensuring this?

>+  mPipeline = gst_pipeline_new("moz-camera-stream");
>+  mSource = gst_element_factory_make("v4l2src", NULL);
>+  mTee = gst_element_factory_make("tee", NULL);

Check return values of above calls, and all gstreamer calls following that need it.

More comments coming.
Comment 50 cajbir (:cajbir) 2010-10-18 16:24:20 PDT
Comment on attachment 472443 [details] [diff] [review]
GStreamer Capture

>+  // Get a bin containing the stream's pipeline pieces
>+  GstBin* streambin;
>+  rv = stream->GetPipeline(&streambin);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  GstPad* padToBlock;
>+  if (mPipelineCount > 0) {
>+    // We need to block the source
>+    padToBlock = gst_element_get_static_pad(mSource, "src");
>+    if (padToBlock == NULL)
>+      return NS_ERROR_FAILURE;
>+
>+    gst_pad_set_blocked(padToBlock, TRUE);
>+    // Now that everything is holding we can proceed
>+  }

Does streambin need to be released on these early returns and the ones following?

>+  // Ask the tee to make a pad available for us
>+  GstPad* teepad;
>+  teepad = gst_element_get_request_pad(mTee, "src%d");
>+  if (!teepad) {
>+    return NS_ERROR_FAILURE;
>+  }
>+
>+  // Link the tee to the stream's bin
>+  if (!gst_element_link(mTee, GST_ELEMENT(streambin))) {
>+    return NS_ERROR_FAILURE;
>+  }

Does the teepad need unlinking on this early return?

>+
>+  // Pause the pipeline, rip out the stream that's disconnecting
>+  // Then resume the pipeline or shut down as appropriate
>+  gst_element_set_state(GST_ELEMENT(mPipeline), GST_STATE_PAUSED);
>+  gst_element_set_state(GST_ELEMENT(streambin), GST_STATE_NULL);
>+  gst_element_send_event(GST_ELEMENT(streambin),
>+                         gst_event_new_eos());
>+  gst_element_unlink(mTee, GST_ELEMENT(streambin));
>+  gst_bin_remove(GST_BIN(mPipeline), GST_ELEMENT(streambin));

Check return values of those calls that return a value. In particular the set_state ones.

>+  case GST_MESSAGE_ERROR: {
>+    GError* err;
>+    gchar* errmsg;
>+    gst_message_parse_error(aMsg, &err, &errmsg);
>+    g_printerr("ERROR from element %s: %s\n", GST_OBJECT_NAME(aMsg->src),
>+               err->message);
>+    g_printerr("Debugging info: %s\n", (errmsg) ? errmsg : "none");
>+
>+    g_free(errmsg);
>+    g_error_free(err);
>+    gst_element_set_state(mPipeline, GST_STATE_NULL);
>+    break;
>+  }

Use Gecko logging.

>+  GStreamerInputStream(GStreamerCaptureProvider *aCaptureProvider,
>+                       nsACString &aContentType,
>+  		       nsCaptureParams *aParams)

Fix indentation of last parameter.

>+  {
>+    printf("GStreamerInputStream::GStreamerInputStream\n");
>+    mReadQueue = new nsDeque();
>+    mWriteQueue = new nsDeque();
>+  }

Don't use printf.


>+  nsresult GetPipeline(GstBin **aBin);
>+  nsresult Disconnect();

Add comments explaining lifetime requirements of aBin, when these are supposed to be called, are they main thread only, etc.

>+  nsCOMPtr<nsIInputStreamCallback> mCallback;
>+  nsCOMPtr<nsIEventTarget> mCallbackTarget;
>+  nsRefPtr<GStreamerCaptureProvider> mCaptureProvider;
>+  PRUint32 mNotifyThreshold;
>+  PRUint32 mAvailable;
>+  nsCString mContentType;
>+  PRPackedBool mClosed;
>+  PRPackedBool mFinished;
>+  PRPackedBool mHeaderSent;
>+  nsDeque *mReadQueue, *mWriteQueue;
>+  GstElement *mBin;
>+  GstElement *mEnc, *mMuxer, *mConv, *mScale;
>+  GstElement *mSink;
>+  gulong mSignal;
>+  PRUint32 mWidth, mHeight, mFrameLimit;
>+
>+  mozilla::Monitor mMonitor;

What members are protected by the monitor? All of them? Add comment to this effect.
Comment 51 Mats Palmgren (:mats) 2011-04-09 08:17:12 PDT
*** Bug 648704 has been marked as a duplicate of this bug. ***
Comment 52 Christopher Blizzard (:blizzard) 2011-10-11 14:57:22 PDT
Work going on in bug #692955.

*** This bug has been marked as a duplicate of bug 692955 ***
Comment 53 Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]] 2011-10-12 14:47:56 PDT
removing sec-review-needed flag

Note You need to log in before you can comment on or make changes to this bug.