1023771 - Regression: Unable to properly seek in a WEBM video

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Flaviu Cos]

Environment: 
Device: Google Nexus 7 (Android 4.4.2);
Build: Nightly 33.0a1 (2014-06-10);

Steps to reproduce:
1. Load http://people.mozilla.com/~nhirata/html_tp/elephants-dream.webm
2. Try to jump forward in the video.

Expected result:
You can jump forward to any position in the video.

Actual result:
There are only 4 positions where the video can be jumped into.

Notes:
Please check the video: http://youtu.be/OJW3UNwtiF8

Comment 1

[Flaviu Cos]

Regression window:
- Last good build 2014-04-01;
- First bad build 2014-04-02;
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1417d180a1d8&tochange=4941a2ac0786

Comment 2

[Aaron Train [:aaronmt]]

I'd like to guess

9c0afbe41ce8	Chris Pearce — Bug 778077 - Implement HTMLMediaElement.fastSeek(time).

Also Flaviu, are other media formats affected here? http://people.mozilla.org/~atrain/mobile/tests/media.html

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Regression + important impact to user, tracking.

Comment 4

[Flaviu Cos]

(In reply to Aaron Train [:aaronmt] from comment #2)

> Also Flaviu, are other media formats affected here?
> http://people.mozilla.org/~atrain/mobile/tests/media.html

Only the 'Media Test - VP9/WEBM(Video)' is affected.

Comment 5

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

In Bug 778077 I changed the default touch controls to use fastSeek(), which seeks to the keyframe before the seek target when seeking. The alternative is to seek using the previous accurate seek which seeks to the keyframe nearest the seek target, and then decodes up to the seek target.

What behaviour does fennec want here? Which seek do you want to use? Slow and accurate, for fast and inaccurate?

On Firefox OS they're very keen on using fastSeek(), since without it seeking in elephant's dream on low end devices can take up to 30 seconds since it's encoded in a stupid way with keyframes so far apart (for example bug 1022913).

Comment 6

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

Can someone point out the desired behaviour here?

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

Chris, thanks for the explaination. I think this winds up being a product/UX decision.

One question I'd have is there a middle ground where we seek the nearest keyfrane, start decoding to the seek target and set a time out to bail on that after ~2 seconds?

Comment 8

[Karen Rudnitski [:kar]]

It's hard to decide without seeing the difference (and I'm sure Ian would say the same thing). Is it possible to get a look at both behaviours (while Brad's question is also answered, ie is there any middle ground)?

Comment 9

[Ian Barlow (:ibarlow)]

(In reply to Karen Rudnitski [:kar] from comment #8)
> It's hard to decide without seeing the difference (and I'm sure Ian would
> say the same thing). 

Yep totally. If we can't get back to the quality we had before this regression, I'd like to see what our alternative options are here, and compare how they feel.

Comment 10

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

You can just compare what seeking videos feels like in Nightly Fennec vs Beta Fennec.

For example, load http://people.mozilla.org/~cpearce/video/ in Fennec, open some of the videos, seek around, and then compare with the other version.

Also, note that bug 1022913 landed yesterday, and that will probably alleviate some of your concerns here I'd think.

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

NI to Chris for my question in comment 7

NI to Karen and Ian because as Chris says, you can compare behavior between Nightly and Beta

Comment 12

[Karen Rudnitski [:kar]]

I am personally leaning towards 'slow and accurate', although I never like to see the word 'slow' relating to our products. But in the elephants_dream example, it is so painful trying to get to a point in the video and it jumping back to that infernal laugh. Or telephone ring. I found myself getting more frustrated at the lack of granularity vs the time it took to start playing again. (although TBH, I even found myself getting frustrated on beta ;-) ).

Of course, what I *really* want is 'fast and accurate'. But my trade-off would be speed for accuracy (within reason, of course). Current behaviour appears *reasonable* on my N4, and the way I see it, fennec isn't being developed specifically for low-end devices (so thinking about our audience).

Comment 13

[Ian Barlow (:ibarlow)]

I am on a Nexus 5 and frankly, these are both awful. I'm sorry, but I can't in good conscience sign off on either of these. One way or another the seeking should be faster *and* more accurate.

Comment 14

[Karen Rudnitski [:kar]]

Ian - what are your expectations there? I've certainly done some video watching over several services and maybe I just have a higher tolerance for buffering given my netflix, BBC iPlayer and other internet streaming experiences (on phones/ tablets). I agree it's not optimal, but for me, there is a tolerance around buffering. Less tolerance on accuracy.

Comment 15

[Ian Barlow (:ibarlow)]

I might have a higher tolerance for the buffering if it didn't look so bad... right now it's misaligned, covered by other controls http://cl.ly/image/2L3o1g361z2c

I think it's time to prioritize and finish the visual refresh of our video controls in bug 704229 -- and in doing so also take a closer look at what buffering looks like.

Comment 16

[Brad Lassey [:blassey] (use needinfo?)]

So, I think the consensus here is sot stop using fastSeek() on Android. Chris, can you take care of that?

Comment 17

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

Sure, we can do slow-and-accurate seek.

And for the record, I'd say this is a case of garbage in, garbage out - elephant's dream is not encoded in a web-friendly way, its keyframes are too far apart.

Comment 18

[Wesley Johnston (:wesj)]

I think some of the jank I see on videos is because we're seeking/rending frames constantly as you drag. i.e. we seek on every valueChanged event:

http://mxr.mozilla.org/mozilla-central/source/layout/xul/nsSliderFrame.cpp#673
http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/videocontrols.xml#117

It might be worth filing a separate bug to only seek the actual video on touchend or if the users finger stays in one place for a second.

Comment 19

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

Ideally, we could just make the videocontrols only use fastSeek() on B2G.

I'd have thought we could just use a #ifdef MOZ_WIDGET_GONK in videocontrols.xml in its seekToPosition function like so:

#ifdef MOZ_WIDGET_GONK
  this.video.fastSeek(newPosition);
#else
  this.video.currentTime = newPosition;
#endif

But the video controls don't work at all if I do this. Which is weird, as I see a #ifdef XP_MACOSX elsewhere in that file.

wesj/jaws: do either of you two know how to write code to only run on B2G and not other platforms?

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

I remember running in to this problem before. These #ifdefs are not the same ones as in the C++ code, IIRC. You have to update http://mxr.mozilla.org/mozilla-central/source/toolkit/content/moz.build#9 I think to include the GONK define.

Comment 21

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

Normal videos provide a much better user experience using fastseek. The test video is degenerate and needs to be re-encoded.

Comment 22

[Sylvestre Ledru [:Sylvestre]]

Chris, we are going to release beta 6 today. Any chance to see a patch this week or is it a wontfix for 31? Thanks

Comment 23

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

Attachment: Patch: Only use fastSeek() on B2G

Simple #ifdef to only use fastSeek() on B2G, and use slow(er) but more accurate on other platforms.

Comment 24

[Brad Lassey [:blassey] (use needinfo?)]

I assume we want to uplift this to 21, Chris can you request approval?

Comment 25

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/079d12b17a26

Comment 26

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

Sure, I will request uplift once it's green.

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/079d12b17a26

Comment 28

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

Comment on attachment 8449026 [details] [diff] [review]
Patch: Only use fastSeek() on B2G

Approval Request Comment
[Feature/regressing bug #]: bug 778077, Implement HTMLMediaElement.fastSeek()

[User impact if declined]: If we do not uplift this patch, on Firefox for Android, on an unknown proportion of HTML <video>'s that are poorly encoded for web viewing, seeking will be fast, but users only be able to seek to the keyframes in a video. If the video is encoded with few keyframes users will only be able to seek to those position in the media with keyframes. The Fennec product drivers would rather seeking be slower but with more destinations than faster but with fewer destinations on video files that were poorly encoded for the web.

This patch only affects Firefox for Android. B2G retains the faster-but-fewer destinations behaviour, at the request of B2G product team.

[Describe test coverage new/current, TBPL]: Been in Nightly over the weekend. Local testing.

[Risks and why]: Low risk, this reverts behaviour on Fennec to what it was.

[String/UUID change made/needed]: None.

Comment 29

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8449026 [details] [diff] [review]
Patch: Only use fastSeek() on B2G

Taking it for beta 8 since it might affect many users.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b470a4a3e9bf
https://hg.mozilla.org/releases/mozilla-beta/rev/9d671266d0d9