1222919 - Make GonkDecoderManager::ProcessFlush() virtual rather than Flush().

Status: RESOLVED FIXED

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

Description

[John Lin [:jhlin][:jolin]]

Flush() is called by reader and runs on reader thread. To customize flush behavior subclasses should implement ProcessFlush(), which runs on decoder looper thread.

Comment 1

[John Lin [:jhlin][:jolin]]

Attachment: Make ProcessFlush() virtual.

Comment 2

[John Lin [:jhlin][:jolin]]

Comment on attachment 8684795 [details] [diff] [review]
Make ProcessFlush() virtual.

GonkAudioDecoderManager and GonkVideoDecoderManager overrides Flush() without protecting their data members. Overriding ProcessFlush() instead seems better IMHO.

Comment 3

[John Lin [:jhlin][:jolin]]

Attachment: Make ProcessFlush() virtual. r=jya

Comment 4

[John Lin [:jhlin][:jolin]]

Comment on attachment 8685245 [details] [diff] [review]
Make ProcessFlush() virtual. r=jya

Carry r+ from jya.

Comment 5

[John Lin [:jhlin][:jolin]]

Try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eaa74da920cb

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cc01ca9f3dff

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cc01ca9f3dff

Comment 8

[John Lin [:jhlin][:jolin]]

Comment on attachment 8685245 [details] [diff] [review]
Make ProcessFlush() virtual. r=jya

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 1222919
User impact if declined: occasional video app crashes if seek continuously
Testing completed: video app doesn't crash
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch:

Comment 10

[Mahendra Potharaju [:mahe]]

Comment on attachment 8685245 [details] [diff] [review]
Make ProcessFlush() virtual. r=jya

Approved for 2.5 uplift. 

Thanks

Comment 11

[Carsten Book [:Tomcat]]

this has problems on uplift:

warning: conflicts during merge.
merging dom/media/platforms/gonk/GonkMediaDataDecoder.cpp incomplete! (edit conflicts, then use 'hg resolve --mark')
merging dom/media/platforms/gonk/GonkMediaDataDecoder.h
merging dom/media/platforms/gonk/GonkVideoDecoderManager.h
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)

could you take a look? thanks!

Comment 12

[John Lin [:jhlin][:jolin]]

(In reply to Carsten Book [:Tomcat] from comment #11)
> this has problems on uplift:
> 
> warning: conflicts during merge.
> merging dom/media/platforms/gonk/GonkMediaDataDecoder.cpp incomplete! (edit
> conflicts, then use 'hg resolve --mark')
> merging dom/media/platforms/gonk/GonkMediaDataDecoder.h
> merging dom/media/platforms/gonk/GonkVideoDecoderManager.h
> abort: unresolved conflicts, can't continue
> (use hg resolve and hg graft --continue)
> 
> could you take a look? thanks!

Sorry, looks like this patch depends on the one in bug 1216895. I'll mark that as approval‑mozilla‑b2g44.

Comment 13

[Carsten Book [:Tomcat]]

mahe ping for the uplift approval requests for 1216895

Comment 14

[Mahendra Potharaju [:mahe]]

Marked this one and dependent bug for 2.5 uplift. 

Thanks

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/69af64b4b84b