performance is very poor for videos from giantbomb.com play

RESOLVED INCOMPLETE

Status

()

Core
Audio/Video: Playback
P5
normal
RESOLVED INCOMPLETE
4 years ago
2 years ago

People

(Reporter: Piotr Mitas, Assigned: eflores)

Tracking

({regressionwindow-wanted, reproducible})

Trunk
ARM
Android
regressionwindow-wanted, reproducible
Points:
---

Firefox Tracking Flags

(firefox30 affected, fennec+)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/32.0.1700.107 Safari/537.36

Steps to reproduce:

Go to giantbomb.com and try to play any video.


Actual results:

A spinning "loading" wheel appears and doesn't go away. The sound from the video plays.

http://www.youtube.com/watch?v=eKA_LTJucSM


Expected results:

The video should play.
This is fixed in Firefox 29 via bug 866080.

This site was originally reported back in bug 884280. It worked for me once bug 866080 landed.

Can you try out Nightly or Aurora for Android and report back if that works for you?
Status: UNCONFIRMED → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(yabolus)
Resolution: --- → DUPLICATE
Duplicate of bug: 866080
(Reporter)

Comment 2

4 years ago
I tried Aurora, it's not fixed. In fact, it's even worse. The video still doesn't play and now the audio stutters horribly.
Status: RESOLVED → UNCONFIRMED
Flags: needinfo?(yabolus)
Resolution: DUPLICATE → ---
Yeah, I see that. Seems to be a new issue.
Status: UNCONFIRMED → NEW
Component: General → Video/Audio
Ever confirmed: true
Product: Firefox for Android → Core
Version: Firefox 27 → Trunk
Edwin, can you reproduce? 

Video playback stutters like crazy on my Nexus 5 (Android 4.4.3) on Nightly (02/11), I only hear audio and the spinner on the player continues to spin.
status-firefox30: --- → affected
Flags: needinfo?(edwin)
Keywords: reproducible
OS: Linux → Android
Hardware: x86_64 → ARM
The site is using http://www.videojs.com/ for their player; oddly the demo on their page works perfectly for me.
tracking-fennec: --- → ?
It doesn't play smoothly here either - testing with the attachment from bug 884280:
https://bugzilla.mozilla.org/attachment.cgi?id=832851

When it plays only sound (stuttering) with a spinning "loading" animation, it seems to be a problem on our end: the VIDEO element in the DOM has sensible width/height, and the animation looks just like the native <video> element's loading state. I'll presume for now that it's a result of the poor decoding performance? I've started looking into it, but probably won't get further today.
Tracking 30 for now, let's get a regression window to see if we need to track an earlier release. Please renom if so.
Assignee: nobody → edwin
tracking-fennec: ? → 30+
Keywords: regressionwindow-wanted
Looking at it now.
Status: NEW → ASSIGNED
I don't have a Nexus 5 handy. Do you have a logcat?
Flags: needinfo?(edwin) → needinfo?(aaron.train)
Created attachment 8377586 [details]
nightly-log.log

Oddly enough this works for me all of a sudden (Nightly 02/18) and Aurora (02/18).

Reporter, can you still reproduce?

I used the front-page video on http://giantbomb.com.

Edwin, here's a log still
Flags: needinfo?(aaron.train) → needinfo?(yabolus)
(Reporter)

Comment 11

4 years ago
Tried it on current aurora, still stutters, video keeps appearing and disappearing.

http://www.youtube.com/watch?v=7qlMBhih-wE
Flags: needinfo?(yabolus)
I get the same problems when testing videos on http://play.brightcove.com/ in Nightly on Android (HTC One phone). Those videos work just fine in stock Android browser on this phone, stutter a bit in Firefox release and beta, and play only stuttering sound with a "loading" graphic in Nightly..
I'm not sure about the stuttering, but this build might fix the video. Could you try it out?

http://people.mozilla.org/~eflores/apk/fennec-30.0a1.en-US.android-arm.apk
Flags: needinfo?(yabolus)
Flags: needinfo?(hsteen)
(Reporter)

Comment 14

4 years ago
Tried it, the videos don't play at all on this one. There's just a black rectangle where there should be the video.
Flags: needinfo?(yabolus)
Could you go to about:support on your phone and post the output, please?
Flags: needinfo?(yabolus)
(Reporter)

Comment 16

4 years ago
Application Basics
------------------

Name: Fennec
Version: 30.0a1
User Agent: Mozilla/5.0 (Android; Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Crash Reports for the Last 3 Days
---------------------------------

All Crash Reports

Extensions
----------

Important Modified Preferences
------------------------------

browser.cache.disk.capacity: 204800
browser.cache.disk.smart_size.first_run: false
browser.cache.disk.smart_size.use_old_max: false
browser.cache.disk.smart_size_cached_value: 204800
browser.startup.homepage_override.mstone: 30.0a1
extensions.lastAppVersion: 30.0a1

Graphics
--------

Adapter Description: Model: Moto G, Product: cm_falcon, Manufacturer: motorola, Hardware: qcom, OpenGL: Qualcomm -- Adreno (TM) 305 -- OpenGL ES 3.0 V@66.0 AU@ (CL@)
Device ID: Adreno (TM) 305
Driver Version: OpenGL ES 3.0 V@66.0 AU@ (CL@)
GPU Accelerated Windows: 1/1 OpenGL (OMTC)
Vendor ID: Qualcomm
WebGL Renderer: Qualcomm -- Adreno (TM) 305
windowLayerManagerRemote: true
AzureCanvasBackend: skia
AzureContentBackend: cairo
AzureFallbackCanvasBackend: none
AzureSkiaAccelerated: 1

JavaScript
----------

Incremental GC: true

Accessibility
-------------

Activated: false
Prevent Accessibility: 0

Library Versions
----------------

NSPR
Expected minimum version: 4.10.4 Beta
Version in use: 4.10.4 Beta

NSS
Expected minimum version: 3.16 Basic ECC Beta
Version in use: 3.16 Basic ECC Beta

NSSSMIME
Expected minimum version: 3.16 Basic ECC Beta
Version in use: 3.16 Basic ECC Beta

NSSSSL
Expected minimum version: 3.16 Basic ECC Beta
Version in use: 3.16 Basic ECC Beta

NSSUTIL
Expected minimum version: 3.16 Beta
Version in use: 3.16 Beta
Flags: needinfo?(yabolus)
Same here. 

Application Basics
------------------

Name: Fennec
Version: 30.0a1
User Agent: Mozilla/5.0 (Android; Mobile; rv:30.0) Gecko/30.0 Firefox/30.0
---------------------------------

All Crash Reports

Extensions
----------

Important Modified Preferences
------------------------------

browser.cache.disk.capacity: 204800
browser.cache.disk.smart_size.first_run: false
browser.cache.disk.smart_size.use_old_max: false
browser.cache.disk.smart_size_cached_value: 112640
browser.startup.homepage_override.mstone: 30.0a1
extensions.lastAppVersion: 30.0a1
network.cookie.prefsMigrated: true

Graphics
--------

Adapter Description: Model: HTC One X, Product: endeavoru, Manufacturer: HTC, Hardware: endeavoru, OpenGL: NVIDIA Corporation -- NVIDIA Tegra 3 -- OpenGL ES 2.0 14.01002
Device ID: NVIDIA Tegra 3
Driver Version: OpenGL ES 2.0 14.01002
GPU Accelerated Windows: 1/1 OpenGL (OMTC)
Vendor ID: NVIDIA Corporation
WebGL Renderer: NVIDIA Corporation -- NVIDIA Tegra 3
windowLayerManagerRemote: true
AzureCanvasBackend: skia
AzureContentBackend: cairo
AzureFallbackCanvasBackend: none
AzureSkiaAccelerated: 1

JavaScript
----------

Incremental GC: true

Accessibility
-------------

Activated: false
Prevent Accessibility: 0

Library Versions
----------------

NSPR
Expected minimum version: 4.10.4 Beta
Version in use: 4.10.4 Beta

NSS
Expected minimum version: 3.16 Basic ECC Beta
Version in use: 3.16 Basic ECC Beta

NSSSMIME
Expected minimum version: 3.16 Basic ECC Beta
Version in use: 3.16 Basic ECC Beta

NSSSSL
Expected minimum version: 3.16 Basic ECC Beta
Version in use: 3.16 Basic ECC Beta

NSSUTIL
Expected minimum version: 3.16 Beta
Version in use: 3.16 Beta
Flags: needinfo?(hsteen)
Aaron, any update?
Flags: needinfo?(aaron.train)
I'm working on a patch to reduce how often we have to perform software colour conversions. It should make decoding a _lot_ faster and get rid of (or at least defer) most of our Android video crashes.
Flags: needinfo?(aaron.train)
Awesome!
Edwin - Any progress?
Mike, there was an assertion that the site is sticking an image above the video and not removing it (poor-man's poster). Can you have a look?
Flags: needinfo?(miket)
redirecting to Hallvord since he's already engaged on this bug
Flags: needinfo?(miket) → needinfo?(hsteen)
Anthony - Comment 19 is interesting and exciting. Any way to move it forward?
Flags: needinfo?(ajones)
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #22)
> there was an assertion that the site is sticking an image above the
> video and not removing it (poor-man's poster). Can you have a look?

I don't see this issue, no. What I see:
1) Firefox on Android: the video starts loading, but the performance is very poor. It stutters sometimes, hangs and just plays back the voices at other times.

2) Firefox OS (or Firefox on Android with a Firefox OS User-Agent string): black screen, nothing plays or appears. Probably due to User-Agent sniffing. Given the performance issues if we do get the video I haven't made analysing this a priority..
Flags: needinfo?(hsteen)
tracking-fennec: 30+ → +
Summary: Videos from giantbomb.com play only sound → performance is very poor for videos from giantbomb.com play
Created attachment 8455064 [details] [diff] [review]
970847-2.patch

This patch adds some frame skipping logic to the omx plugin, and makes the media plugin bitrate estimation more accurate (previously the bitrate was estimated from the MediaResource stream postiion, and was therefore often totally wrong due to the MediaResourceServer serving up lots of data).

Gralloc-related performance work is in bug 1026305, which applies on top of this patch.
Attachment #8455064 - Flags: review?(cajbir.bugzilla)
Flags: needinfo?(ajones)
Comment on attachment 8455064 [details] [diff] [review]
970847-2.patch

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

::: content/media/plugins/MediaPluginDecoder.cpp
@@ +32,5 @@
> +  if (!reliable || mCurrentTime < 0) {
> +    return -1;
> +  }
> +
> +  return mCurrentTime * bytesPerSecond;

You might want to make this std::min<int64_t>(mCurrentTime*bytesPerSecond, GetResource()->Length());

Comment 28

4 years ago
Comment on attachment 8455064 [details] [diff] [review]
970847-2.patch

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

Please provide a comment somewhere that explains in english how the bitrate estimation works and how it was improved. Ditto with the frame skipping.

::: content/media/MediaDecoder.h
@@ +335,5 @@
>    // Return the time position in the video stream being
>    // played measured in seconds.
>    virtual double GetCurrentTime();
>  
> +  // Return the position of the cursor of our MediaResource.

What is a cursor? Is this term used anywhere else in the media code? Maybe explain exactly what it's returning (read byte offset?).

::: content/media/plugins/MPAPI.h
@@ +148,5 @@
>    void (*GetVideoParameters)(Decoder *aDecoder, int32_t *aWidth, int32_t *aHeight);
>    void (*GetAudioParameters)(Decoder *aDecoder, int32_t *aNumChannels, int32_t *aSampleRate);
>    bool (*HasVideo)(Decoder *aDecoder);
>    bool (*HasAudio)(Decoder *aDecoder);
> +  bool (*ReadVideo)(Decoder *aDecoder, VideoFrame *aFrame, int64_t aSeekTimeUs, int64_t aCatchupTimeUs, BufferCallback *aBufferCallback);

Add a comment explaining the parameters. Especially what 'aCatchupTimeUs' is.

::: media/omx-plugin/OmxPlugin.cpp
@@ +354,5 @@
> +    return ColorFormatSupportPreferred;
> +  }
> +
> +  if (IsColorFormatConvertible(aColorFormat)) {
> +    LOG("Colour format %#x supported by Android ColorConverter.", aColorFormat);

What are the color format changes here for? I don't see a description in the patch about color format and it seems unrelated to bitrate estimation and frame rate changes.

@@ +963,5 @@
>  };
>  
> +status_t OmxDecoder::SkipVideoToTime(int64_t aTimeUs)
> +{
> +  while (true) {

How are we avoiding an infinite loop here if server sends video with a timestamp that is always before aTimeUs?

@@ +978,5 @@
> +    }
> +
> +    ReleaseVideoBuffer();
> +
> +    if (aTimeUs < timeUs) {

Should this be <= if we are skipping to the time rather than past it?

@@ +1006,3 @@
>  
> +  } else if (aCatchupTimeUs != -1) {
> +    if (!IsColorFormatNative((OMX_COLOR_FORMATTYPE)mVideoColorFormat)) {

What does the color format have to do with the skipping logic? Might want a comment here explaining.

@@ +1010,5 @@
> +      err = SkipVideoToTime(aCatchupTimeUs + SKIP_US);
> +    } else {
> +      options.setSeekTo(aCatchupTimeUs + 1000000);
> +    }
> +  }

Should there be a catch-all else here? If aSeekTime == -1 and aCatchupTimeUs == -1? Is that possible?
Attachment #8455064 - Flags: review?(cajbir.bugzilla) → review-
filter on [mass-p5]
Priority: -- → P5
Component: Audio/Video → Audio/Video: Playback
I can't videos to play on their website at all on my HTC M8 running Aurora43.
I just tested and videos seem to work for me on my OncPlus One on Nightly 44, although I have to wait like 10 seconds for the ad to start playing (I thought the page was busted and went to look at logcat, but then it turned on). After the ad, it seems OK to me.
Piotr, can you still reproduce the problem on a newer version?
Flags: needinfo?(yabolus)
Closing this out due to lack of ability to reproduce with current builds. Piotr, if you happen to return to this bug and the problem still reproduces for you, please feel free to reopen the bug.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago2 years ago
Flags: needinfo?(yabolus)
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.