982695 - Thumbnailing videos on Tarako is slow and induces crashes

Status: RESOLVED INVALID

(Core :: Audio/Video: Playback, defect, P3)

Description

[(no longer active)]

We are going to need to do a Tarako specific video thumbnailing API, I'll fill out the details later today, this is a placeholder for now.

Assigning to overholt to find an owner.

Comment 1

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

OK, I had a look into this. Thumbnailing is causing memory pressure on the Tarako, and causing us to crash.

We often crash while thumbnailing, and usually crash if we're playing an MP3 with Music App and then load the Video App.

Note: you can delete the old thumbnails with this shell command:
adb shell rm -R data/local/storage/persistent/20+f+app+++video.gaiamobile.org/

We don't need a dedicated thumbnailing API to fix this. We can instead:

1. [Bug 984698] Change the video stack to only preroll (decode) 1 video frame and no audio samples when the video element's "preload" attribute is set to "metadata" provided the video is not playing. We can do this on B2G (and maybe Fennec too) only. We should also not preroll after a seek when we're preload=metadata and not playing. This means the Video App will have reduced memory usage while thumbnailing, and general web pages which contain <video preload="metadata"> will have lower memory usage.

2. [Bug 778077] Implement HTMLMediaElement.fastSeek(), and change the Video App to use this instead of normal "frame accurate" seek. Provided we do 1. above, we should not be doing audio decode while seeking in a preroll=metadata video, and we should only decode 1 video frame (the keyframe closest to the seek target) when we seek, so the seek should be fast and thumbnailing should thus be speed up.

3. [Bug 631058] Reduce the amount of decoded audio data we preroll when we're playing a file which doesn't have a video stream. (This is not really related to thumbnailing, but it's very easy way to reduce memory usage). Currently we try to keep 1 second of audio decoded ahead of the current playback position when playing a media file. This is so that if there's a slow video decode we won't run out of audio to play, since we still decode audio and video on the same thread (I'm adding infrastructure to fix that in bug 979104). For 48kHz stereo audio this requires about 200kB of memory. When we're not playing video, we don't need to worry about slow video decodes, so we can probably reduce the amount of audio we preroll here and save some memory.

4. [Bug 976273] Add memory-pressure observers to destroy unneeded decoders, and a panic mode which just tears down all decoders. We should be doing this anyway, since it's pretty trivial to crash even the browser on desktop by creating video elements in a loop.

I'm not convinced that we should rush out a thumbnailing API right now for the initial Tarako release. It won't be a big perf win, since the Video App *must* load unthumbnailed media in a regular <video> element anyway in order to retrieve the media's metadata, and if we do 1. and 2. above our perf and memory use should be equivalanet, and doing 1. and 2. above is less work and are a good idea to do anyway.

We should still do the Thumbnailing API in the long term however, as it will enable Gecko to select a better keyframe for the thumbnail, rather than the Video App having to seek and hope we get a good representative thumbnail frame. But maybe it makes sense to put the thumbnail API on the video element instead of having a stand alone API that accepts a URL, since the Video App needs to load the file in a <video> element anyway to retrieve its other metadata?

Comment 2

[(no longer active)]

Thanks for working on this, Chris!

The last time that we discussed this on dev-webapi, Sotaro (IIRC) said that the video element usage would be unacceptable because it needs to do things such as decode audio, grab the lock for the video hardware decoder, etc.  Does this completely address those concerns?

Comment 3

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

Yes, number 1 above will greatly reduce the amount of audio we decode, from 1s to one packet, however big that is. So we will still be doing some decoding, but not much. With my WIP patches for the above, thumbnail time on the tarako drops from about 2.2s to 0.2s, so the above pretty much mitigates the issues here.

Comment 4

[Sotaro Ikeda [:sotaro]]

Yes! As cpearce already answered, it address the concerns.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Awesome, this sounds like a great plan for now!

Comment 6

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

(In reply to Chris Pearce (:cpearce) from comment #1)

> 1. [Bug 984698] Change the video stack to only preroll (decode) 1 video
> frame and no audio samples when the video element's "preload" attribute is
> set to "metadata" provided the video is not playing. We can do this on B2G
> (and maybe Fennec too) only. We should also not preroll after a seek when
> we're preload=metadata and not playing. This means the Video App will have
> reduced memory usage while thumbnailing, and general web pages which contain
> <video preload="metadata"> will have lower memory usage.

Without decoding audio samples, would that introduce some latency on audio playing if user wants to play instead of thumbnailing?

Comment 7

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

(In reply to Blake Wu [:bwu] from comment #6)
> (In reply to Chris Pearce (:cpearce) from comment #1)
> Without decoding audio samples, would that introduce some latency on audio
> playing if user wants to play instead of thumbnailing?

I tested and did not notice any problems. We still decode one audio packet (so that we can determine the actual start time of the media) so we will have a small amount of audio to play when playback starts.

Comment 8

[Sotaro Ikeda [:sotaro]]

One problem to using video tag as thumbnail generation is that there is no way to prioritize hw codec usages between playback and thumbnail generation. In current b2g, it is managed by video app.

Comment 9

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #8)
> One problem to using video tag as thumbnail generation is that there is no
> way to prioritize hw codec usages between playback and thumbnail generation.
> In current b2g, it is managed by video app.

Ideally, it should be automatically managed by gecko side.

Comment 10

[(no longer active)]

(In reply to Sotaro Ikeda [:sotaro] from comment #9)
> (In reply to Sotaro Ikeda [:sotaro] from comment #8)
> > One problem to using video tag as thumbnail generation is that there is no
> > way to prioritize hw codec usages between playback and thumbnail generation.
> > In current b2g, it is managed by video app.
> 
> Ideally, it should be automatically managed by gecko side.

Filed bug 986645 for that.

Comment 11

[Joe Cheng [:jcheng] (please needinfo)]

triage: let's not block tarako with this bug, if we have solution ready, we may uplift. thanks

Comment 12

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

I am not able to reproduce this issue on the current 1.3T build.

1.3T Environmental Variables:
Device: Tarako 1.3T MOZ
BuildID: 20140502014001
Gaia: a8e0ff550de08e58e4bf75af3cecf175b9b71e70
Gecko: 71790bf476cb
Version: 28.1
Firmware Version: sp6821a-Gonk-4.0-4-29

Even thumbnailing 100 images at once did not cause a crash.

Comment 13

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I'm not sure we want to close this just yet.  I'll leave it up to Ehsan and the other dev to see if they can improve some of the performance here.

Comment 14

[(no longer active)]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #13)
> I'm not sure we want to close this just yet.  I'll leave it up to Ehsan and
> the other dev to see if they can improve some of the performance here.

Well, this is sort of a metabug anyway, it doesn't really make sense to keep it open indefinitely.