1175656 - [B2G][Gecko] Flame: videos recorded by camera app is not shown in gallery app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Camera, defect)

Description

[Andrew Osmond [:aosmond] (he/him)]

File duplicate of bug 1164200 to track the gecko changes which must land first (otherwise camera recording will be unusable).

Add feature to Gecko to create the poster image for video files on behalf of the application.

Comment 1

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Implement Gecko posters, v1

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Implement Gecko posters, v1.1

Fix a build breakage on mulet (if camera enabled). Tested on aries as well as flame (high and low memory configurations), all seems to be working.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e5ff66eb399b

Comment 3

[Dave Hylands [:dhylands]]

Comment on attachment 8623916 [details] [diff] [review]
Implement Gecko posters, v1.1

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

lgtm - just one question about already_AddRefed

::: dom/camera/CameraControlImpl.cpp
@@ +177,5 @@
>    }
>  }
>  
>  void
> +CameraControlImpl::OnPoster(already_AddRefed<dom::BlobImpl> aBlobImpl)

Shouldn't this be a bare pointer?
https://developer.mozilla.org/en-US/docs/Using_nsCOMPtr/Getting_Started_Guide says "void f( nsCOMPtr<T> ) don't pass an nsCOMPtr by value"

I viewed aleady_AddRefed as a variant of that.

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Implement Gecko posters, v1.2 [carries r=dhylands]

bz, can you review the WebIDL bits?

(In reply to Dave Hylands [:dhylands] from comment #3)
> Comment on attachment 8623916 [details] [diff] [review]
> Implement Gecko posters, v1.1
> 
> Review of attachment 8623916 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> lgtm - just one question about already_AddRefed
> 
> ::: dom/camera/CameraControlImpl.cpp
> @@ +177,5 @@
> >    }
> >  }
> >  
> >  void
> > +CameraControlImpl::OnPoster(already_AddRefed<dom::BlobImpl> aBlobImpl)
> 
> Shouldn't this be a bare pointer?
> https://developer.mozilla.org/en-US/docs/Using_nsCOMPtr/
> Getting_Started_Guide says "void f( nsCOMPtr<T> ) don't pass an nsCOMPtr by
> value"
> 
> I viewed aleady_AddRefed as a variant of that.

Ah, originally I did it think I would avoid a refcount increase but you are right. I should pass the raw pointer because it doesn't save us anything due to the extra layer of indirection (i.e. I can't .forget to the next listener).

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8624561 [details] [diff] [review]
Implement Gecko posters, v1.2 [carries r=dhylands]

r=me on the webidl bit

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

https://hg.mozilla.org/integration/b2g-inbound/rev/e3b6829af189

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e3b6829af189

Comment 8

[Dietrich Ayala (:dietrich)]

In bug 1211032, we found that this change caused a large UI responsiveness regression. It is a blocker for 2.5.