Closed Bug 1050031 Opened 6 years ago Closed 5 years ago

Shut down dormant video/audio decoders

Categories

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

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: ajones, Assigned: sotaro)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [MemShrink:P1])

We could get most of the benefits that we need by cutting scope to instead make a media element dormant when any of the following are true:

    The media element is a video element and is in a background tab and has been paused or ended for at least 60s.
    The media element is a video or audio element and has moved into the bfcache.

That drops the stress-test and visibility issues from our use cases. It should be quite easy to do this.

When a video element is dormant on desktop, we should retain the current Image in the VideoFrameContainer, so we can still render it when the tab is brought into the foreground.
Sotaro, can you let us know how much work you think this would be?
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #0)
> When a video element is dormant on desktop, we should retain the current
> Image in the VideoFrameContainer, so we can still render it when the tab is
> brought into the foreground.

On b2g gonk case, the current Image can not be retained when HwCodec is released. Because, OMXCodec requests all decoded buffers to be released before the codec shutdown. I do not know similar restriction exists on other cases like WMF.
Flags: needinfo?(sotaro.ikeda.g)
cpearce, do you know if a restriction exists like in gonk in Comment 2?
Flags: needinfo?(cpearce)
(In reply to Milan Sreckovic [:milan] (PTO 8/11 - 8/15) from comment #1)
> Sotaro, can you let us know how much work you think this would be?

It might be about 3 weeks work. Implementation seems not difficult, but need to be done by dividing it to sub-problems and need to address them one by one. Especially, we need to modify all MediaDecoderReader derived classes to support dormant. The dormant mode is alredy implemented only at MediaOmxReader on gonk. It's implementation is not good enough. It also seems better to update it.
There is no restriction on what you can do with the Images produced by the WMF backends.

The only thing like that is there is a limit to how many DXVA decoders we can create at once. Currently we just keep a running count of how many we keep open at once inside the WMFReader. I think we can just keep doing that; there's no need to create a CodecProxy for WMF at this stage.


(In reply to Sotaro Ikeda [:sotaro] from comment #4)
> Especially, we need to modify all MediaDecoderReader derived classes to
> support dormant.

The top priority are the MP4Reader and the WebMReader, please do them first. Don't bother with the WMFReader or the GStreamerReader, we're going to deprecate those and use MP4Reader instead.
Flags: needinfo?(cpearce)
(In reply to Sotaro Ikeda [:sotaro] from comment #2)
> On b2g gonk case, the current Image can not be retained when HwCodec is
> released. Because, OMXCodec requests all decoded buffers to be released
> before the codec shutdown. I do not know similar restriction exists on other
> cases like WMF.

Could it could be copied into a regular image?
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #6)
> 
> Could it could be copied into a regular image?

It can be copied to GrallocImage. But we need to extend GrallocImage to support specifying android::Graphicbuffer's pixel format.
(In reply to Sotaro Ikeda [:sotaro] from comment #7)
> (In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #6)
> > 
> > Could it could be copied into a regular image?
> 
> It can be copied to GrallocImage. But we need to extend GrallocImage to
> support specifying android::Graphicbuffer's pixel format.

The above is to do copy by same pixel format. If we convert to other color format, extending GrallocImage is not necessary. But copying take time, it could degrade the performance on low end b2g phone.
(In reply to Chris Pearce (:cpearce) from comment #5)
> ...
> 
> (In reply to Sotaro Ikeda [:sotaro] from comment #4)
> > Especially, we need to modify all MediaDecoderReader derived classes to
> > support dormant.
> 
> The top priority are the MP4Reader and the WebMReader, please do them first.
> Don't bother with the WMFReader or the GStreamerReader, we're going to
> deprecate those and use MP4Reader instead.

Chris, somebody else (i.e., not Sotaro) should be able to do this.  Three weeks is a large chunk of time, and it sounds like there are some pointers from Sotaro in here that should enable somebody else to get more progress on this before we go back to him (Sotaro) for more direction.
Assignee: nobody → sotaro.ikeda.g
These bugs are fit and finish issues that might block EME uplift to Aurora.
Blocks: eme-m3
(In reply to Milan Sreckovic [:milan] from comment #9)
> > 
> > The top priority are the MP4Reader and the WebMReader, please do them first.
> > Don't bother with the WMFReader or the GStreamerReader, we're going to
> > deprecate those and use MP4Reader instead.
> 
> Chris, somebody else (i.e., not Sotaro) should be able to do this.  Three
> weeks is a large chunk of time, and it sounds like there are some pointers
> from Sotaro in here that should enable somebody else to get more progress on
> this before we go back to him (Sotaro) for more direction.

Sorry, for my delay. If I do not receive a new big bug, I could start from next week.
Status: NEW → ASSIGNED
Before start implementation. I am catching up the media's current implementation. MediaSource's diagram is created as a part of it.
https://github.com/sotaroikeda/firefox-diagrams/blob/master/media/dom_media_MediaSource_2_2.pdf?raw=true
No longer blocks: eme-m3
I also created some media diagrams(MediaDecoderStateMachine, AudioSink, MP4Decoder) to the following.
https://github.com/sotaroikeda/firefox-diagrams/wiki/Firefox-Diagrams
Depends on: 1107348
Priority: -- → P1
Is this being worked on?
Flags: needinfo?(sotaro.ikeda.g)
Since today, I backed from PTO. I am going to work as high priority.
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Sotaro Ikeda [:sotaro PTO Dec/24-Jan/6] from comment #15)
> Since today, I backed from PTO. I am going to work as high priority.

Excellent. Much appreciated.
Whiteboard: [MEMSHRINK]
Depends on: 1112410
Depends on: 1113527
Depends on: 1121064
Depends on: 1121658
No longer depends on: 1121064
Depends on: 1108728
Depends on: 1110343
Depends on: 1122228
No longer blocks: 1087043
Depends on: 1123452
How much memory is saved by making a decoder dormant? (Trying to prioritize for MemShrink.)
Whiteboard: [MEMSHRINK] → [MemShrink]
We're aiming to ship most of the functionality for MP4 in 36 along with MSE.

I don't know exact numbers, but on Windows our media stack maintains a buffer of 10 decoded frames in RGB format in GPU memory. So for a 1920x1080 video, that's ~8.3MB per frame, plus the 1 frame visible onscreen, so that's about 92MB of GPU memory. The decoder also internally will do some allocation/buffering, and create about 6 threads.

For MSE we also have a memory buffer of about 75MB in memory per video. We plan to make this backed by disk storage, but that probably won't make 36.
Holy smoke!
Whiteboard: [MemShrink] → [MemShrink:P1]
Depends on: 1124844
Depends on: 1125472
No longer depends on: 1107348
Tracking bug; no longer P1
Priority: P1 → P5
Depends on: 1129941
Depends on: 1128357
Depends on: 1133167
No longer depends on: 1133167
Depends on: 1133167
Depends on: 1135304
No longer depends on: 1135304
Depends on: 1135304
Depends on: 1139919
Depends on: 1142527
No longer depends on: 1135304
Depends on: 1147435
Depends on: 1147170
Sotaro, what's the status of this? It looks like it might be mostly or totally fixed? Thank you.
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Nicholas Nethercote [:njn] from comment #21)
> Sotaro, what's the status of this? It looks like it might be mostly or
> totally fixed? Thank you.

The all necessary bugs are already fixed.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: needinfo?(sotaro.ikeda.g)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.