986381 - Frame drop observed for high resolution .webm video clips

Status: RESOLVED FIXED

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

Description

[Pooja]

Attachment: 720p_2Mbps_30fps.WebM video test clip

Steps to reproduce:
1. Copy any high resolution .webm clip (1st attachment) to sdcard
2. Open video app and play same clip.

Expected behavior:
No frame drop should observed also video and audio should be in sync.

Actual behavior:
Frame drop observed. Video stuck at some point and audio continues to play.

Also take a look on 2nd attached recorded clip of same issue. 
Please find below logs which show the high frame corrupted and dropped count.
-------
I/GeckoDump( 1613): Video statistics start
I/GeckoDump( 1613): Dimensions: 1280x720
I/GeckoDump( 1613): Complete duration: 02:33
I/GeckoDump( 1613): Start time: 00:00
I/GeckoDump( 1613): End time: 00:34
I/GeckoDump( 1613): Total frames: 1042
I/GeckoDump( 1613): Decoded frames: 534
I/GeckoDump( 1613): Corrupted frames: 508
I/GeckoDump( 1613): Rendered frames: 514
I/GeckoDump( 1613): Dropped frames: 528
I/GeckoDump( 1613): Average rendered FPS: 14.83
I/GeckoDump( 1613): Video statistics end
---------------------

Comment 1

[Pooja]

Attachment: Frame drop observed in high resolution 720p_2Mbps_30fps.WebM video clip

Comment 2

[Jason Smith [:jsmith]]

QA Wanted here to see if this happens on 1.3.

Comment 3

[Matthew Vaughan]

This issue reproduces on the 03/20/14 1.3 build.

Device: Buri v1.3 MOZ RIL
BuildID: 20140320004000
Gaia: 35a51cad2b8cd63c62152bc42c242ce67f27ffea
Gecko: ed31cfc75f93
Version: 28.0
Firmware Version: V1.2-device.cfg

Additionally, this issue reproduces on 1.1.

Comment 4

[Jason Smith [:jsmith]]

mvines - Why is this a blocker for 1.4 if it's already present on 1.1 & 1.3?

Comment 5

[Michael Vines [:m1] [:evilmachines]]

We measure ourselves against other OSes running on the same chipset, not older versions of FxOS running on older chipsets.  The bar is constantly being raised with every release and one day (hopefully soon!) we'll reach parity with the other players.

Comment 6

[Preeti Raghunath(:Preeti)]

Product,

This seems to be a new feature ask. Can you please weigh in if we should take this for 1.4?

Comment 7

[Preeti Raghunath(:Preeti)]

Neither of these are blocking bugs IMO -- doesn't meet our criteria.

However, these are fine bugs to work on as they are being identified.  We *need* to move away from marking all things we want to work on as blocking.  

Chris

Inder,
This is a candidate for unblocking.

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

Are you testing on a device with VP8 hardware decoding? It seems unlikely to me that we'll be able to to HD VP8 without hardware accelerated video decoding.

Comment 9

[Michael Vines [:m1] [:evilmachines]]

We do have a VP8 hardware decoder available, I suspect that Gecko is not using it.   This may stem back to v1.0 days when we couldn't use the VP8 hardware decoder due to pmem limitations.   This is no longer the case on JB and KK, so perhaps we need to look at using the VP8 OMX decoder when !ICS.

Moving to 1.4+ because on this class of device it is perfectly reasonable for a user to expect smooth high-resolution VP8 playback, especially when other codecs are smooth at the same resolution.  However this could be considered a performance optimization and not a "feature" so adjusting the blocking bug accordingly to give more time here.

Comment 10

[Inder]

Clearing ni, Michael provided info.

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

OK, we need to change DecoderTraits.cpp to use the OMX decoder/reader when the OMX backend supports HWAccel WebM, and fallback to the existing WebMReader when the MediaOMXReader does not support HWAccel WebM.

Comment 12

[Ivan Tsay (:ITsay)]

Hi Eric,

Could you help to find an assignee to handle this blocker? Thanks.

Comment 13

[Jason Smith [:jsmith]]

(In reply to Ivan Tsay (:ITsay) from comment #12)
> Hi Eric,
> 
> Could you help to find an assignee to handle this blocker? Thanks.

Product already weighed in that this isn't a blocker. QC needs to raise their concerns directly with our product team here if they would like to consider this for 1.4.

(In reply to Michael Vines [:m1] [:evilmachines] (on PTO until 4/6) from comment #9)
> Moving to 1.4+ because on this class of device it is perfectly reasonable
> for a user to expect smooth high-resolution VP8 playback, especially when
> other codecs are smooth at the same resolution.  However this could be
> considered a performance optimization and not a "feature" so adjusting the
> blocking bug accordingly to give more time here.

I think that makes a compelling argument to consider this as something we should look into, but that isn't a case to block on the bug. A blocker is something we would absolutely hold the release on against our triage criteria here - https://wiki.mozilla.org/B2G/Triage#Issues_that_Should_Block. In this bug's case, this isn't a performance regression, as this problem has been present in past releases. So in this case we need to explain why the performance optimization is required for 1.4 - is there a partner app desiring high resolution webm support? Is a critical market in 1.4 requiring this? These are example of questions need to answer here to justify blocking here. What I see here is an argument that we should consider implementing this performance enhancement, but not really a critical need why this is required for 1.4. Given this & the fact that product already claimed this wasn't a blocker, I'm moving this to the backlog. If you still disagree, then I'd suggest following up with our product team to discuss this further.

Comment 14

[Inder]

> product already claimed this wasn't a blocker, I'm moving this to the backlog
Noming it again for 1.4. 
Sandip, can you please provide your comments here.

Comment 15

[Jason Smith [:jsmith]]

required for madai, so blocking

Comment 16

[Milan Sreckovic [:milan] (needinfo for best results)]

Since this is back to 1.4+, repeating the comment 12 request for assignment.

Comment 17

[Eric Chou [:ericchou] [:echou]]

(In reply to Milan Sreckovic [:milan] from comment #16)
> Since this is back to 1.4+, repeating the comment 12 request for assignment.

ok. Blake will look into this.

Comment 18

[Blake Wu [:bwu][:blakewu]]

I am going to change the default decoder of webm files to be stagefright's (OmxDecoder) which will check media_codecs.xml (located at /system/etc/) to use HW decoder or Google's SW decoder (not currently-used decoder).

Comment 19

[Inder]

Blake -- is it going to look for the decoder in the order listed in the file?
This solution looks good to me.
Adding Diego to get their take on it.

Comment 20

[Preeti Raghunath(:Preeti)]

drawing attention to the date of expected fix: no later than 4/21.

Comment 21

[Diego Wilson [:diego]]

(In reply to Inder from comment #19)
> Blake -- is it going to look for the decoder in the order listed in the file?
> This solution looks good to me.
> Adding Diego to get their take on it.

SGTM

Comment 22

[Michael Vines [:m1] [:evilmachines]]

We might just want to pick the OMX HW decoder if possible and then fall back to the Gecko SW decoder rather than the OMX SW decoder.

Comment 23

[Blake Wu [:bwu][:blakewu]]

(In reply to Inder from comment #19)
> Blake -- is it going to look for the decoder in the order listed in the file?
Yes. I will not change the existing behavior which already exists in OMXCodec.cpp(http://androidxref.com/4.3_r2.1/xref/frameworks/av/media/libstagefright/OMXCodec.cpp#182). 

The only files I will change is DecoderTraits.cpp (add webm mime type to OMX decoder supported list)and OmxDecoder.cpp (set the flags to be 0 to not use HW codec only, if there is no HW codec, fall back to SW codec). The patch will be attached today.

Comment 24

[Blake Wu [:bwu][:blakewu]]

(In reply to Michael Vines [:m1] [:evilmachines] from comment #22)
> We might just want to pick the OMX HW decoder if possible and then fall back
> to the Gecko SW decoder rather than the OMX SW decoder.
May I know the concern you prefer not to use OMX SW decoder?

Comment 25

[Chris Pearce [:cpearce (Not reading bugmail)]]

It would be good if we can use the existing WebMReader when we need to fallback to SW decoding of WebM instead of using OMX's SW decoder. We've been been using WebMReader in desktop Firefox for years, so has been tested more.

Comment 26

[Blake Wu [:bwu][:blakewu]]

On B2G, partners may expect to have a consistent behavior/capability with Android version. 
For OMX SW decoder, it should be tested by many Android users.

Comment 27

[Michael Vines [:m1] [:evilmachines]]

(In reply to Blake Wu [:bwu] from comment #26)
> On B2G, partners may expect to have a consistent behavior/capability with
> Android version. 

Which partners have this expectation?  

It's better to use the existing Gecko SW decoder for v1.4 for a couple reasons:
1) Less risk.  The OMX SW decoder may be better and it may be worse.  We don't really know.
2) Consistency between Firefox OS/Desktop, as well as between FxOSes on different devices.
3) Less blobs for Mozilla.  The OMX SW decoder may or may not be something you have source to.

Note that on some older devices the OMX HW decoder cannot support the same max resolution as the SW decoder (Gecko or OMX) due to the use of pmem for decoding (Buri is one such device).  This bug has the possibility of breaking higher-resolution webm content on those devices.  One possible way to avoid this side-effect is to only enable the OMX HW codec for a JB or greater gonk as those devices are all ICS based.

Comment 28

[Blake Wu [:bwu][:blakewu]]

(In reply to Michael Vines [:m1] [:evilmachines] from comment #27)
> (In reply to Blake Wu [:bwu] from comment #26)
> > On B2G, partners may expect to have a consistent behavior/capability with
> > Android version. 
> 
> Which partners have this expectation?  
Phone makers. I think they may compare the Android's version and FFOS's. 
> It's better to use the existing Gecko SW decoder for v1.4 for a couple
> reasons:
> 1) Less risk.  The OMX SW decoder may be better and it may be worse.  We
> don't really know.
> 2) Consistency between Firefox OS/Desktop, as well as between FxOSes on
> different devices.
> 3) Less blobs for Mozilla.  The OMX SW decoder may or may not be something
> you have source to.
Agree. Just thought OMX SW decoder may be optimized for mobile device. 
> 
> Note that on some older devices the OMX HW decoder cannot support the same
> max resolution as the SW decoder (Gecko or OMX) due to the use of pmem for
> decoding (Buri is one such device).  This bug has the possibility of
> breaking higher-resolution webm content on those devices.  One possible way
> to avoid this side-effect is to only enable the OMX HW codec for a JB or
> greater gonk as those devices are all ICS based.
Fair enough and also agree these concerns.

Comment 29

[Sotaro Ikeda [:sotaro]]

I also think it is not a good idea to use androis' sw codec from the following point of view.
- To keep consistent behavior between firefox browsers.
- Andorid SW codec is not tested on b2g
- We did not configure for android's SW codec(like decoded buffer number)

Comment 30

[Sotaro Ikeda [:sotaro]]

In future, b2g's media platform should be more platform independent except hw codec. platform dependent sw codec needs to be prevented.

Comment 31

[Blake Wu [:bwu][:blakewu]]

BTW, I found in the media_codecs.xml of Nexus4 (https://www.codeaurora.org/cgit/quic/la/device/lge/mako/tree/media_codecs.xml?h=LNX.LA.3.2.1.5#n64) webm is not decoded by QCT HW decoder. After checking the pull-out the media_codecs.xml from Nexus5 with Android version, it doesn't use HW decoder either. 
Is there any concern not to use HW decoder in these two devices?

Comment 32

[Michael Vines [:m1] [:evilmachines]]

Not really a concern for the N5 since that's just a Mozilla reference device.  Do you have a Flame or a QRD8x26 device?   If not, we can certainly test out a WIP patch over here.

Comment 33

[Blake Wu [:bwu][:blakewu]]

Attachment: Enable webm OMX decoder

Hi Michael,
you can use this patch to have a try to see if HW decoder can be used. 
I am trying to modify the media_codecs.xml on my Nexus4 and QRD/Flame to see if HW decoder can be accessed.

Comment 34

[Michael Vines [:m1] [:evilmachines]]

Pooja, could you please try out this WIP patch.

Comment 35

[Blake Wu [:bwu][:blakewu]]

VP8 HW decoder works well on my QRD with no-change media_codecs.xml which has a HW vp8 decoder in the list. 
Compared to SW decoder, I can observe the playback is more smooth with HW decoder. 
But there is no rule to know which device can support vp8 HW decoder (Nexus4 cannot support), unless we query the media_codecs.xml.

Comment 36

[cajbir (:cajbir)]

What happens if the VP8 HW decoder encounters a WebM file that uses VP9? Will it fallback to our internal VP9 support?

Comment 37

[Blake Wu [:bwu][:blakewu]]

(In reply to cajbir (:cajbir) from comment #36)
> What happens if the VP8 HW decoder encounters a WebM file that uses VP9?
> Will it fallback to our internal VP9 support?
Yes. I expect to do that as well. Besides, some platform may support VP9 HW decoder in the near future.
So will check if there is any HW decoder supported or not. If not, fallback to our internal decoder.

Comment 38

[Blake Wu [:bwu][:blakewu]]

Hi All, 
Currently in Gecko,IIRC we don't have a fallback mechanism to use other decoder when some decoder is not capable of decoding some profile/type well since demuxer is created after newing decoder. For a long-term solution, I think we should change the current design to create a demuxer first and then choose a proper decoder. 
 
For a short-term solution, I am going to write/use a lightweight webm file parser to get the codec type(VP8 or VP9) before newing decoder in DecoderTraits::InstantiateDecoder(). If OMX has a HW decoder, use it. Otherwise, use internal decoder. 
Agree? Other ideas?

Comment 39

[Pooja]

With fix high resolution Webm clip is playing very well.

Ideally dropped frame count should not exceed 1% of total frames. 
But  with this fix its currently 2% .Is it  acceptable ?

---LOGs-----
I/GeckoDump( 1498): Video statistics start
I/GeckoDump( 1498): Dimensions: 1920x1088
I/GeckoDump( 1498): Complete duration: 00:10
I/GeckoDump( 1498): Start time: 00:00
I/GeckoDump( 1498): End time: 00:00
I/GeckoDump( 1498): Total frames: 732
I/GeckoDump( 1498): Decoded frames: 731
I/GeckoDump( 1498): Corrupted frames: 1
I/GeckoDump( 1498): Rendered frames: 718
I/GeckoDump( 1498): Dropped frames: 14
I/GeckoDump( 1498): Average rendered FPS: 23.09
I/GeckoDump( 1498): Video statistics end
---LOGS------------

Comment 40

[cajbir (:cajbir)]

(In reply to Blake Wu [:bwu] from comment #38)
> For a short-term solution, I am going to write/use a lightweight webm file
> parser to get the codec type(VP8 or VP9) before newing decoder in
> DecoderTraits::InstantiateDecoder(). If OMX has a HW decoder, use it.
> Otherwise, use internal decoder. 
> Agree? Other ideas?

Instead of using your own parser, maybe use libnestegg which is what we use elsewhere for parsing WebM. Or could the mime type sniffer be extended? I'm wary of having half a dozen slightly different lightweight WebM parsers in the tree (sniffer, libnestegg, MSE, WebRTC, etc).

Comment 41

[Paul Adenot (:padenot)]

The mime type sniffer uses libnestegg for matching WebM files. It's actually pretty straightforward to use or adapt for your usecases, I'm sure kinetik or me can provide info.

Comment 42

[Sotaro Ikeda [:sotaro]]

Comment about attachment 8407232 [details] [diff] [review]. We limit hw decoder for video decoding from the following reasons. We should keep that. SW video codec can not provide enough quality as a product.
- Can not provide enough decoding performance
- android H.264 decoder support only baseline profile.
- It could cause oom problem. It actually happened in the past.

Comment 43

[Sotaro Ikeda [:sotaro]]

I think that if hw video codec is enabled on a format, sw vide codec should not be used for the the format on the product. Otherwise, we can not ensure the product quality.

Comment 44

[Sotaro Ikeda [:sotaro]]

(In reply to Blake Wu [:bwu][:blakewu] from comment #38)
> For a short-term solution, I am going to write/use a lightweight webm file
> parser to get the codec type(VP8 or VP9) before newing decoder in
> DecoderTraits::InstantiateDecoder().

If there is a good enough parser on open source, it is better to use it on all platforms.

Comment 45

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #43)
> I think that if hw video codec is enabled on a format, sw vide codec should
> not be used for the the format on the product. Otherwise, we can not ensure
> the product quality.

From the above point, OMXDecoder does not use SW video decoder on b2g.

Comment 46

[Michael Vines [:m1] [:evilmachines]]

(In reply to Pooja from comment #39)
>
> Ideally dropped frame count should not exceed 1% of total frames. 
> But  with this fix its currently 2% .Is it  acceptable ?

Before: >50% dropped frames
After: ~2% dropped frames

Yeah, that'll do for v1.4 :)
We can squeeze out that last 1% in v2.0

Comment 47

[Chris Pearce [:cpearce (Not reading bugmail)]]

I'd guess that to reduce that 2% drop more we'd need to change our rendering pipeline so that the compositor decides which frame needs to be displayed on every vertical refresh, rather than us setting a timer and updating the current frame that way. This would be a good change to make, but it's not a simple change...

Comment 48

[Sotaro Ikeda [:sotaro]]

(In reply to Chris Pearce (:cpearce) from comment #47)
> I'd guess that to reduce that 2% drop more we'd need to change our rendering
> pipeline so that the compositor decides which frame needs to be displayed on
> every vertical refresh, rather than us setting a timer and updating the
> current frame that way. This would be a good change to make, but it's not a
> simple change...

One difficulty is OMXCodec's gralloc buffer is very scarce. If we hold more buffer on Compositor, OMXCodec fall into buffer starvation.

Comment 49

[Blake Wu [:bwu][:blakewu]]

Thanks for Padenot and Kinetik for the information they provided on irc. 

I am trying to use nestegg sniffer in a newly-added callback used in nsBaseChannel. But hit a problem that there is no simple way to bring the codec id back to HTMLMediaElement(OnStartRequest()) and need your suggestions. The following is my ideas. 

1. Create a new callback function(CallWebmSniffer) as CallUnknownTypeSniffer does to sniff the codec id in http://dxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsBaseChannel.cpp#705
------- 
 if (NS_SUCCEEDED(mStatus) && shouldSniff) {
    mPump->PeekStream(CallUnknownTypeSniffer, static_cast<nsIChannel*>(this));
  }
  
  /*newly-added*/
  mPump->PeekStream(CallWebmSniffer, static_cast<nsIChannel*>(this));//new  
  
  // Now, the general type sniffers. Skip this if we have none.
  if (mLoadFlags & LOAD_CALL_CONTENT_SNIFFERS)
    mPump->PeekStream(CallTypeSniffers, static_cast<nsIChannel*>(this));
----
2. Inside CallWebmSniffer(), I can use nestegg functions to get codec id. But there is no interfaces from nsIChannel to set/store this sniffed information and then get it in HTMLMediaElement, like SetContentType/GetContentType. 

Do you suggest to add new interface (setContentSubType) or there is another way to bring codec id back to HTMLMediaElement for us to better know to create a proper decoder?

Comment 50

[Paul Adenot (:padenot)]

I'm not sure we want to sniff everything that flows through necko, here. We could simply run the logic sniffing logic again when we are about to instantiate a webm decoder.

Comment 51

[Matthew Gregan [:kinetik]]

Yeah, what Paul said.  I *think* you want to use an nsIStreamListenerTee to siphon off a copy of the incoming data to run the nestegg sniffer (extended to report codecs) against, after which you can shut the tee down and initialize the appropriate decoder.  It might be worth discussing your options with a Necko expert before going ahead, as this isn't really my area.

Comment 52

[Blake Wu [:bwu][:blakewu]]

(In reply to Paul Adenot (:padenot) from comment #50)
> I'm not sure we want to sniff everything that flows through necko, here. We
> could simply run the logic sniffing logic again when we are about to
> instantiate a webm decoder.
Could you share me some hints to do this? I have no idea to directly read (from channel?) the media data to do sniffing in the HTMLMediaElement or DecoderTraits. Currently the way I think to reading and sniffing data is from Necko like current getContentType.

Comment 53

[Paul Adenot (:padenot)]

(answered on irc)

Comment 54

[Blake Wu [:bwu][:blakewu]]

Hi Patrick,

May I have your help on reading stream/data via Necko? 
Is it possible to read data via the "channel" @http://dxr.mozilla.org/mozilla-central/source/content/html/content/src/HTMLMediaElement.cpp#340?  
I need to read the data and do some sniffing to get codec ID, not only getting mime type. 
Do you have other suggestions, like comment 51?

Comment 55

[Preeti Raghunath(:Preeti)]

Blake

Can your fix for HW decoder be uplifted and we can take care of the SW piece within another week?

Comment 56

[Blake Wu [:bwu][:blakewu]]

(In reply to Preeti Raghunath(:Preeti) from comment #55)
> Blake
> 
> Can your fix for HW decoder be uplifted and we can take care of the SW piece
> within another week?
OK. I just created a new bug (bug 1000671) for the fallback mechanism. Will enable HW codec first and take enough time on fallback mechanism.

Comment 57

[Blake Wu [:bwu][:blakewu]]

Comment on attachment 8407232 [details] [diff] [review]
Enable webm OMX decoder

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

Use HW decoder if available, otherwise fallback to omx SW decoder.

Comment 58

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8407232 [details] [diff] [review]
Enable webm OMX decoder

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

Software video codec can not ensure h.264's playback capability and quality.

Comment 59

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #58)
> Comment on attachment 8407232 [details] [diff] [review]
> Enable webm OMX decoder
> 
> Review of attachment 8407232 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Software video codec can not ensure h.264's playback capability and quality.

video codec's acquire/release happens asynchronously between video tags. Even when each applications use only one video tag, fallback to sw video codec could happen. It actually happened in the past on b2g.

Comment 60

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8407232 [details] [diff] [review]
Enable webm OMX decoder

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

Additional comment.

::: content/media/omx/OmxDecoder.cpp
@@ +503,5 @@
>  
>      // Experience with OMX codecs is that only the HW decoders are
>      // worth bothering with, at least on the platforms where this code
>      // is currently used, and for formats this code is currently used
> +    // for (h.264).  If we don't get a hardware decoder, just fall back to 

spece on the end of line.

@@ +504,5 @@
>      // Experience with OMX codecs is that only the HW decoders are
>      // worth bothering with, at least on the platforms where this code
>      // is currently used, and for formats this code is currently used
> +    // for (h.264).  If we don't get a hardware decoder, just fall back to 
> +    // other SW decoders specified in media_codecs.xml.

OMXCodec does not check media_codecs.xml. Did you expect MediaCodec?

Comment 61

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #59)
> > -----------------------------------------------------------------
> > 
> > Software video codec can not ensure h.264's playback capability and quality.
> 
> video codec's acquire/release happens asynchronously between video tags.
> Even when each applications use only one video tag, fallback to sw video
> codec could happen. It actually happened in the past on b2g.

Sorry, fix the comment. OmxDecoder allocates OMXDecoder via OMXCodecProxy. OMXCodecProxy serializes the video decoder allocation requests. So, the patch should not degrade the H.264 video.

Comment 62

[Blake Wu [:bwu][:blakewu]]

(In reply to Sotaro Ikeda [:sotaro] from comment #60)
> OMXCodec does not check media_codecs.xml. Did you expect MediaCodec?
IIRC, OMXCodec check media_codecs.xml in findMatchingCodecs() @http://androidxref.com/4.3_r2.1/xref/frameworks/av/media/libstagefright/OMXCodec.cpp#189
Read file @ http://androidxref.com/4.3_r2.1/xref/frameworks/av/media/libstagefright/MediaCodecList.cpp#39

Comment 63

[Sotaro Ikeda [:sotaro]]

attachment 8407232 [details] [diff] [review] actually does not change using hardware codec for H.264 case on b2g. And instance of OMXDecoder as video decoder is limited to one.

If we use OMXCodec for webm video playback, this insatance limitation is also applied to webm video. It change how webm video could be playback. This change seems very big for b2g v1.4 at this time. We need to get a consensus about change.

Comment 64

[Sotaro Ikeda [:sotaro]]

(In reply to Blake Wu [:bwu][:blakewu] from comment #62)
> (In reply to Sotaro Ikeda [:sotaro] from comment #60)
> > OMXCodec does not check media_codecs.xml. Did you expect MediaCodec?
> IIRC, OMXCodec check media_codecs.xml in findMatchingCodecs()
> @http://androidxref.com/4.3_r2.1/xref/frameworks/av/media/libstagefright/
> OMXCodec.cpp#189
> Read file @
> http://androidxref.com/4.3_r2.1/xref/frameworks/av/media/libstagefright/
> MediaCodecList.cpp#39

Ok, thanks.

Comment 65

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8407232 [details] [diff] [review]
Enable webm OMX decoder

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

I change to review+ if Comment 63 becomes clear.

Comment 66

[Sotaro Ikeda [:sotaro]]

(In reply to Blake Wu [:bwu][:blakewu] from comment #62)
> (In reply to Sotaro Ikeda [:sotaro] from comment #60)
> > OMXCodec does not check media_codecs.xml. Did you expect MediaCodec?
> IIRC, OMXCodec check media_codecs.xml in findMatchingCodecs()
> @http://androidxref.com/4.3_r2.1/xref/frameworks/av/media/libstagefright/
> OMXCodec.cpp#189
> Read file @
> http://androidxref.com/4.3_r2.1/xref/frameworks/av/media/libstagefright/
> MediaCodecList.cpp#39

It might be better that media_codecs.xml is used by OMXCodec since JB. ICS does not use it.

Comment 67

[Sotaro Ikeda [:sotaro]]

> It might be better that media_codecs.xml is used by OMXCodec since JB. ICS
> does not use it.

Correction:
It might better to explain that media_codecs.xml is used by OMXCodec since JB.

Comment 68

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8407232 [details] [diff] [review]
Enable webm OMX decoder

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

::: content/media/DecoderTraits.cpp
@@ +214,5 @@
>    "video/mp4",
>    "video/3gpp",
>    "video/quicktime",
> +  "video/webm",
> +  "audio/webm",

You also need to change DecoderTraits::CanHandleMediaType(). You need to add code to the IsOmxSupportedType() block that sets codecList to the WebM codecs supported by the OMX decoder if IsWebMType(nsDependentCString(aMIMEType)) is true.

Comment 69

[Sotaro Ikeda [:sotaro]]

By the way, iirc, webm's parser had some defects in past android. I am not sure about current android. It seems necessary to do a lot of testing of webm after enabling it.

Comment 70

[Bruce Sun [:brsun]]

Is it good to refine the naming of MediaResourceManagerClient::HW_VIDEO_DECODER since OMXCodecProxy doesn't limit to allocate hardware-only video decoders any more?
 - http://dxr.mozilla.org/mozilla-central/source/content/media/omx/OMXCodecProxy.cpp#129

Comment 71

[Blake Wu [:bwu][:blakewu]]

(In reply to Sotaro Ikeda [:sotaro] from comment #63)
> attachment 8407232 [details] [diff] [review] actually does not change using
> hardware codec for H.264 case on b2g. And instance of OMXDecoder as video
> decoder is limited to one.
> 
> If we use OMXCodec for webm video playback, this insatance limitation is
> also applied to webm video. It change how webm video could be playback. This
> change seems very big for b2g v1.4 at this time. We need to get a consensus
> about change.
If I understand correctly, you mean webm's behavior (instance limitation) will be like current h.264, right?

Comment 72

[Sotaro Ikeda [:sotaro]]

(In reply to Bruce Sun [:brsun] from comment #70)
> Is it good to refine the naming of
> MediaResourceManagerClient::HW_VIDEO_DECODER since OMXCodecProxy doesn't
> limit to allocate hardware-only video decoders any more?
>  -
> http://dxr.mozilla.org/mozilla-central/source/content/media/omx/
> OMXCodecProxy.cpp#129

Yeah, it seems better to change the name to more generic one.

Comment 73

[Sotaro Ikeda [:sotaro]]

(In reply to Blake Wu [:bwu][:blakewu] from comment #71)
> (In reply to Sotaro Ikeda [:sotaro] from comment #63)
> > attachment 8407232 [details] [diff] [review] actually does not change using
> > hardware codec for H.264 case on b2g. And instance of OMXDecoder as video
> > decoder is limited to one.
> > 
> > If we use OMXCodec for webm video playback, this insatance limitation is
> > also applied to webm video. It change how webm video could be playback. This
> > change seems very big for b2g v1.4 at this time. We need to get a consensus
> > about change.
> If I understand correctly, you mean webm's behavior (instance limitation)
> will be like current h.264, right?

Yes. It seems better for b2g. Almost all b2g phone have a very tight memory/cpu resource limitations. But it might affect to some existing testing or use cases.

Comment 74

[Blake Wu [:bwu][:blakewu]]

Attachment: Enable webm OMX decoder with CanHandleMediaType

1. Add a OMX webm codeclist for CanHandleMediaType
2. Add description for media_codecs.xml supported since JB

Comment 75

[Blake Wu [:bwu][:blakewu]]

Comment on attachment 8412292 [details]
Enable webm OMX decoder with CanHandleMediaType

Obsolete this and re-submit a new one.

Comment 76

[Blake Wu [:bwu][:blakewu]]

Attachment: Enable webm OMX decoder with CanHandleMediaType

Comment 77

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8412337 [details] [diff] [review]
Enable webm OMX decoder with CanHandleMediaType

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

I think we should have a pref to enable this. The pref should be off by default, and only enabled where we think we need it.

::: content/media/DecoderTraits.cpp
@@ +246,5 @@
>    nullptr
>  };
> +
> +static char const *const gOMXWebMCodecs[4] = {
> +  "vorbis",	

Don't add trailing whitespace, on the "vorbis" and "vp8" lines.

@@ +409,5 @@
>      result = CANPLAY_MAYBE;
>      if (nsDependentCString(aMIMEType).EqualsASCII("audio/mpeg")) {
>        codecList = gMpegAudioCodecs;
> +    } else if (nsDependentCString(aMIMEType).EqualsASCII("audio/webm")||
> +              nsDependentCString(aMIMEType).EqualsASCII("video/webm")) {

} else if (nsDependentCString(aMIMEType).EqualsASCII("audio/webm") ||
		           nsDependentCString(aMIMEType).EqualsASCII("video/webm")) {


i.e. space between ')' and "||", and line up conditions in if statements.

Comment 78

[Michael Vines [:m1] [:evilmachines]]

How about the pref off for ICS based devices but enabled for KK based devices?  We'd always want to use this for KK (until we don't, and then we can file a new bug for that)

Comment 79

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8412337 [details] [diff] [review]
Enable webm OMX decoder with CanHandleMediaType

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

Review only OmxDecoder part. Looks good. Comment 78 seems reasonable comment. We do not need to change ICS gonk's capability in this version.

::: content/media/omx/OmxDecoder.cpp
@@ +504,5 @@
>      // Experience with OMX codecs is that only the HW decoders are
>      // worth bothering with, at least on the platforms where this code
>      // is currently used, and for formats this code is currently used
> +    // for (h.264).  So if we don't get a hardware decoder, just fallback
> +    // to sw decoders which are listed in MediaCodec.xml since JB 

Remove space

@@ +505,5 @@
>      // worth bothering with, at least on the platforms where this code
>      // is currently used, and for formats this code is currently used
> +    // for (h.264).  So if we don't get a hardware decoder, just fallback
> +    // to sw decoders which are listed in MediaCodec.xml since JB 
> +    int flags = 0;//kHardwareCodecsOnly;

It seems better to remove commented out "kHardwareCodecsOnly".

Comment 80

[Blake Wu [:bwu][:blakewu]]

Thanks for all your feedbacks! 
Also agree to add a pref to be more flexible to enable on some version or device. 

The following is what I am going to do. 
1. Add a pref and enable it on by default KK as comment 77, 78, and 79. 
2. Modify the current test case to check vp9 and opus according the pref.
3. Remove some unnecessary whitespace and comment and line up conditions as comment 77 and 79. 
Please let me if there is anything missed.

Comment 81

[Preeti Raghunath(:Preeti)]

(In reply to Blake Wu [:bwu][:blakewu] from comment #80)
> Thanks for all your feedbacks! 
> Also agree to add a pref to be more flexible to enable on some version or
> device. 
> 
> The following is what I am going to do. 
> 1. Add a pref and enable it on by default KK as comment 77, 78, and 79. 
> 2. Modify the current test case to check vp9 and opus according the pref.
> 3. Remove some unnecessary whitespace and comment and line up conditions as
> comment 77 and 79. 
> Please let me if there is anything missed.

Blake,

Are you creating another patch which addresses all of the points in comment #80?

Comment 82

[Blake Wu [:bwu][:blakewu]]

Attachment: enable-omx-webm-decoder-jb-kk

I enable omx webm decoder on JB as well since currently Flame has not had KK yet but it has webm HW decoder supported.

Comment 83

[Blake Wu [:bwu][:blakewu]]

(In reply to Preeti Raghunath(:Preeti) from comment #81)
> Blake,
> 
> Are you creating another patch which addresses all of the points in comment
> #80?

Yes. But since Flame which supports webm hw decoder still uses JB, I enable omx webm decoder on JB as well. 
For the test case, I am modifying it and submit another patch later.

Comment 84

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8414171 [details] [diff] [review]
enable-omx-webm-decoder-jb-kk

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

r=cpearce for DecoderTraits.cpp.

sotaro needs to review OmxDecoder.cpp

You need a build peer (like :glandium) to review configure.in.

::: configure.in
@@ +259,5 @@
>          AC_SUBST(MOZ_OMX_DECODER)
>          MOZ_OMX_ENCODER=1
>          AC_SUBST(MOZ_OMX_ENCODER)
>          AC_DEFINE(MOZ_OMX_ENCODER)
> +	MOZ_OMX_WEBM_DECODER=1

You need to get review from a build peer on this. Like Mike Hommey (:glandium).

::: content/media/DecoderTraits.cpp
@@ +206,5 @@
>  }
>  #endif
>  
>  #ifdef MOZ_OMX_DECODER
> +#ifdef MOZ_OMX_WEBM_DECODER

I think you can make this:

ifdef MOZ_OMX_DECODER
static const char* const gOmxTypes[] = {
  "audio/mpeg",
  "audio/mp4",
  "audio/amr",
  "video/mp4",
  "video/3gpp",
  "video/quicktime",
#ifdef MOZ_OMX_WEBM_DECODER
  "video/webm",
  "audio/webm",
#endif
  nullptr
};

@@ +422,5 @@
>    if (IsOmxSupportedType(nsDependentCString(aMIMEType))) {
>      result = CANPLAY_MAYBE;
>      if (nsDependentCString(aMIMEType).EqualsASCII("audio/mpeg")) {
>        codecList = gMpegAudioCodecs;
> +#ifdef MOZ_OMX_WEBM_DECODER

Please also make the MOZ_WEBM block inside CanHandleMediaType defined out if MOZ_OMX_WEBM_DECODER is defined, i.e.:

#if defined(MOZ_WEBM) && !defined(MOZ_OMX_WEBM_DECODER)
  if (IsWebMType(nsDependentCString(aMIMEType))) {
    codecList = gWebMCodecs;
    result = CANPLAY_MAYBE;
  }
#endif

Comment 85

[Blake Wu [:bwu][:blakewu]]

(In reply to Chris Pearce (:cpearce) from comment #84)
> Comment on attachment 8414171 [details] [diff] [review]
> enable-omx-webm-decoder-jb-kk
> 
> Review of attachment 8414171 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=cpearce for DecoderTraits.cpp.
> 
> sotaro needs to review OmxDecoder.cpp
> 
> You need a build peer (like :glandium) to review configure.in.
Will do! 
> ::: configure.in
> @@ +259,5 @@
> >          AC_SUBST(MOZ_OMX_DECODER)
> >          MOZ_OMX_ENCODER=1
> >          AC_SUBST(MOZ_OMX_ENCODER)
> >          AC_DEFINE(MOZ_OMX_ENCODER)
> > +	MOZ_OMX_WEBM_DECODER=1
> 
> You need to get review from a build peer on this. Like Mike Hommey
> (:glandium).
> 
> ::: content/media/DecoderTraits.cpp
> @@ +206,5 @@
> >  }
> >  #endif
> >  
> >  #ifdef MOZ_OMX_DECODER
> > +#ifdef MOZ_OMX_WEBM_DECODER
> 
> I think you can make this:
Yes. It is much simpler. 
> ifdef MOZ_OMX_DECODER
> static const char* const gOmxTypes[] = {
>   "audio/mpeg",
>   "audio/mp4",
>   "audio/amr",
>   "video/mp4",
>   "video/3gpp",
>   "video/quicktime",
> #ifdef MOZ_OMX_WEBM_DECODER
>   "video/webm",
>   "audio/webm",
> #endif
>   nullptr
> };
> 
> @@ +422,5 @@
> >    if (IsOmxSupportedType(nsDependentCString(aMIMEType))) {
> >      result = CANPLAY_MAYBE;
> >      if (nsDependentCString(aMIMEType).EqualsASCII("audio/mpeg")) {
> >        codecList = gMpegAudioCodecs;
> > +#ifdef MOZ_OMX_WEBM_DECODER
> 
> Please also make the MOZ_WEBM block inside CanHandleMediaType defined out if
> MOZ_OMX_WEBM_DECODER is defined, i.e.:
OK. To be mutually exclusive with internal webm decoder  
> #if defined(MOZ_WEBM) && !defined(MOZ_OMX_WEBM_DECODER)
>   if (IsWebMType(nsDependentCString(aMIMEType))) {
>     codecList = gWebMCodecs;
>     result = CANPLAY_MAYBE;
>   }
> #endif

Comment 86

[Blake Wu [:bwu][:blakewu]]

Attachment: Add a define for omx webm decoder on JB and KK

Add a define of MOZ_OMX_WEBM_DECODER for JB and KK only.
DecoderTraits and OmxDecoder check it to enable OMX WebM decoder.

Comment 87

[Blake Wu [:bwu][:blakewu]]

Attachment: add-omx-webm-decoder-jb-kk

Changes in DecoderTraits.cpp 
1. Check MOZ_OMX_WEBM_DECODER to see if it is required to use omx webm decoder or not.
2. Re-summit with some changes mentioned in comment 88.

Comment 88

[Blake Wu [:bwu][:blakewu]]

Attachment: add-omx-webm-decoder-jb-kk-2

Changes in OmxDecoder.cpp. 
Allow to use OMX SW decoder if there is no HW decoder supported.

Comment 89

[Blake Wu [:bwu][:blakewu]]

(In reply to Blake Wu [:bwu][:blakewu] from comment #87)
> Created attachment 8414362 [details] [diff] [review]
> add-omx-webm-decoder-jb-kk
> 
> Changes in DecoderTraits.cpp 
> 1. Check MOZ_OMX_WEBM_DECODER to see if it is required to use omx webm
> decoder or not.
opps! some typos. Corrected it as below. 
2. Re-submit with some changes mentioned in comment 84.

Comment 90

[Blake Wu [:bwu][:blakewu]]

Attachment: Modify the test case of can_play_webm_type

Test case change in can_play_type_webm.js. 
Don't check vp9 and opus when OMX webm decoder enabled in JB and KK (Android Version > 15).

Comment 91

[Mike Hommey [:glandium]]

Comment on attachment 8414358 [details] [diff] [review]
Add a define for omx webm decoder on JB and KK

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

::: configure.in
@@ +259,5 @@
>          AC_SUBST(MOZ_OMX_DECODER)
>          MOZ_OMX_ENCODER=1
>          AC_SUBST(MOZ_OMX_ENCODER)
>          AC_DEFINE(MOZ_OMX_ENCODER)
> +	MOZ_OMX_WEBM_DECODER=1

This line is not needed.

@@ +260,5 @@
>          MOZ_OMX_ENCODER=1
>          AC_SUBST(MOZ_OMX_ENCODER)
>          AC_DEFINE(MOZ_OMX_ENCODER)
> +	MOZ_OMX_WEBM_DECODER=1
> +	AC_DEFINE(MOZ_OMX_WEBM_DECODER)

Do you need that set on all the tree?

Comment 92

[Blake Wu [:bwu][:blakewu]]

(In reply to Mike Hommey [:glandium] from comment #91)
> Comment on attachment 8414358 [details] [diff] [review]
> Add a define for omx webm decoder on JB and KK
> 
> Review of attachment 8414358 [details] [diff] [review]:
> -----------------------------------------------------------------
> Do you need that set on all the tree?
Thanks for your hint. 
It is no need to add this in configure.in. 
I will add it to the corresponding moz.build.

Comment 93

[Blake Wu [:bwu][:blakewu]]

Attachment: Add OMX WebM Decoder Define in moz.builds

Add MOZ_OMX_WEBM_DECODER define in moz.builds in the gecko/content/media and gecko/contenet/media/omx to check if it is required to enable omx webm decoder or not.

Comment 94

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8414407 [details] [diff] [review]
Modify the test case of can_play_webm_type

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

::: content/media/test/can_play_type_webm.js
@@ +19,5 @@
> +    var audio = ['vorbis'];
> +  } else {
> +    var video = ['vp8', 'vp8.0', 'vp9', 'vp9.0'];
> +    var audio = ['vorbis', 'opus'];
> +  }

Put spaces around operators, like '+' and '>', and on the outside of parenthesis ('(' and ')') in "if" statements.

So that should be:

  if (androidVer > 15) {
    var video = ['vp8', 'vp8.0'];
    var audio = ['vorbis'];
  } else {
    var video = ['vp8', 'vp8.0', 'vp9', 'vp9.0'];
    var audio = ['vorbis', 'opus'];
  }

Comment 95

[Blake Wu [:bwu][:blakewu]]

Attachment: Modify the test case of can_play_type_webm v2

Update the attachment 8414407 [details] [diff] [review] for the followings. 
1. Add the OS check for B2G 
2. Put spaces around operators

Comment 96

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8417260 [details] [diff] [review]
Modify the test case of can_play_type_webm v2

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

::: content/media/test/can_play_type_webm.js
@@ +6,5 @@
>    // WebM types
>    check("video/webm", "maybe");
>    check("audio/webm", "maybe");
>  
> +  // Enable OMX webm decoder after Android version 16 as MOZ_OMX_WEBM_DECODER

You comment here says that you are enabling OMX WebM decoder. But this code does not do that. The code detects whether the OMX WebM decoder is enabled.

The comment needs to be changed so it's not misleading.

@@ +13,5 @@
> +  var androidVer = SpecialPowers.Cc['@mozilla.org/system-info;1']
> +                                  .getService(SpecialPowers.Ci.nsIPropertyBag2)
> +                                  .getProperty('version');
> +  info("android version:"+androidVer);
> +  if (navigator.userAgent.indexOf("Mobile") != -1 && androidVer > 15) {

Won't this disable VP9 and Opus for Android builds? We should keep testing that.

Comment 97

[Blake Wu [:bwu][:blakewu]]

(In reply to Chris Pearce (:cpearce) from comment #96)
> Comment on attachment 8417260 [details] [diff] [review]
> Modify the test case of can_play_type_webm v2
> 
> @@ +13,5 @@
> > +  var androidVer = SpecialPowers.Cc['@mozilla.org/system-info;1']
> > +                                  .getService(SpecialPowers.Ci.nsIPropertyBag2)
> > +                                  .getProperty('version');
> > +  info("android version:"+androidVer);
> > +  if (navigator.userAgent.indexOf("Mobile") != -1 && androidVer > 15) {
> 
> Won't this disable VP9 and Opus for Android builds? We should keep testing
> that.
AFAIK, the string, "Mobile", is for B2G. This check just keeps that only B2G will be affected. Others will keep the same as usual.

Comment 98

[Blake Wu [:bwu][:blakewu]]

Attachment: Modify-test-case-can_play_webm_type-v3

Per comment 96, modify the comment to be more clear and add one more check to make sure this change is only for B2G.

Comment 99

[Blake Wu [:bwu][:blakewu]]

The results of test cases on TBPL. 
https://tbpl.mozilla.org/?tree=Try&rev=b07ad47535eb

Comment 100

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8418453 [details] [diff] [review]
Modify-test-case-can_play_webm_type-v3

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

::: content/media/test/can_play_type_webm.js
@@ +6,5 @@
>    // WebM types
>    check("video/webm", "maybe");
>    check("audio/webm", "maybe");
>  
> +  // OMX webm decoder is enabled after Android version 16 as MOZ_OMX_WEBM_DECODER

This comment says what is happening. The code also says what's happening. So comments that just say what is happening don't add much information (unless the code is very complex and hard to understand). Good comments will say *why* the code is doing what it's doing.

So make this comment:

// OMX webm decoder is enabled on B2G after Android version 16 (JellyBean) so that we can use hardware acceleration to speed up VP8 video decoding. OMX does not support VP9 or Opus, so we don't test for that when decoding WebM using OMX here.

@@ +15,5 @@
> +                                  .getProperty('version');
> +  info("android version:"+androidVer);
> +  //Check for FxOS case
> +  if (navigator.userAgent.indexOf("Mobile") != -1 && 
> +      navigator.userAgent.indexOf("Android") == -1 && androidVer > 15) {	  

Remove trailing space on these two lines.

Comment 101

[Blake Wu [:bwu][:blakewu]]

Attachment: Modify test case can_play_webm_type v4

Per comment 100, make comments to be more clear :) and remove some unnecessary space.

Comment 102

[Blake Wu [:bwu][:blakewu]]

Attachment: Add-OMX-WebM-Decoder-Def.patch

Add reviewer to patch.

Comment 103

[Blake Wu [:bwu][:blakewu]]

Attachment: Add-omx-webm-decoder-jb-kk.patch

Add reviewer to patch.

Comment 104

[Blake Wu [:bwu][:blakewu]]

Attachment: Add-omx-webm-decoder-jb-kk2.patch

Add reviewer to patch.

Comment 105

[Blake Wu [:bwu][:blakewu]]

Attachment: Modify-test-case-can_play_webm_type.patch

Add reviewer to patch.

Comment 106

[Blake Wu [:bwu][:blakewu]]

Test results on TBPL: https://tbpl.mozilla.org/?tree=Try&rev=38a2ee17a3ef
Check-in needed:
 Add-OMX-WebM-Decoder-Def.patch: r=glandium
 Add-omx-webm-decoder-jb-kk.patch: r=cpearce
 Add-omx-webm-decoder-jb-kk2.patch: r=sotaro
 Modify-test-case-can_play_webm_type.patch: r=cpearce

Comment 107

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6d8b0bf673c5
https://hg.mozilla.org/integration/mozilla-inbound/rev/49831d474b17
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2a9fef81547
https://hg.mozilla.org/integration/mozilla-inbound/rev/86616594f0c9

Comment 110

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6d8b0bf673c5
https://hg.mozilla.org/mozilla-central/rev/49831d474b17
https://hg.mozilla.org/mozilla-central/rev/a2a9fef81547
https://hg.mozilla.org/mozilla-central/rev/86616594f0c9

Comment 111

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/34b821bbc7ba
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/b7f5d141c237
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/3ecdbb73c057
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/e96076a30b09

Comment 112

[Robert Mead [:RobertM]]

Test case is valid because test pass requires you to play a .webm video.

https://moztrap.mozilla.org/manage/case/8582/

Comment 113

[KTucker [:KTucker][Inactive 3/4/2016]]

Test case updated in moztrap to include paying attention to the framerate when playing .webm video:

https://moztrap.mozilla.org/manage/case/8582/