1272553 - Move all access to mTaskQueue from AppleVTDecoder to the parent class

Status: RESOLVED FIXED

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

Description

[JW Wang [:jwwang] [:jw_wang]]

As bug 1271508.

By separating code about dispatching jobs from code about doing actual decoding jobs, we are able to move all use of mTaskQueue to the parent class (AppleVDADecoder).

This will make it easier for changes to mTaskQueue to get rid of FlushableTaskQueue.

Comment 1

[Jean-Yves Avenard [:jya]]

(In reply to JW Wang [:jwwang] from comment #0)
> As bug 1271508.
> 
> By separating code about dispatching jobs from code about doing actual
> decoding jobs, we are able to move all use of mTaskQueue to the parent class
> (AppleVDADecoder).
> 
> This will make it easier for changes to mTaskQueue to get rid of
> FlushableTaskQueue.

Keep in mind, that with the drop of 10.6 to 10.8, we have no need for the VDA decoder anymore and Apple has obsoleted it in favour of VideoToolbox.

So I'm guessing we will want to remove VDA support in the not so far away timeframe...

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Somehow AppleVDADecoder::OutputFrame() is also called from AppleVTDecoder. We will need to move some code from AppleVDADecoder to AppleVTDecoder if we want to drop AppleVDADecoder.

I believe the refactoring here will help/ease moving the code in the future.

Comment 3

[Jean-Yves Avenard [:jya]]

(In reply to JW Wang [:jwwang] from comment #2)
> Somehow AppleVDADecoder::OutputFrame() is also called from AppleVTDecoder.
> We will need to move some code from AppleVDADecoder to AppleVTDecoder if we
> want to drop AppleVDADecoder.

yes, this is called from the VT decoder callback.
VT and VDA are very similar and the data output by both decoder is identical

> 
> I believe the refactoring here will help/ease moving the code in the future.

that's great to hear

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1272553. Part 1 - move code around to be able to extract common code in P2. r=jya.

Review commit: https://reviewboard.mozilla.org/r/52391/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52391/

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1272553. Part 2 - extract common code to the parent class. r=jya.

Review commit: https://reviewboard.mozilla.org/r/52393/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52393/

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1272553. Part 3 - make mTaskQueue private. r=jya.

Review commit: https://reviewboard.mozilla.org/r/52395/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52395/

Comment 7

[Jean-Yves Avenard [:jya]]

Comment on attachment 8752029 [details]
MozReview Request: Bug 1272553. Part 1 - move code around to be able to extract common code in P2. r=jya.

https://reviewboard.mozilla.org/r/52391/#review49397

::: dom/media/platforms/apple/AppleVTDecoder.cpp:195
(Diff revision 1)
>  nsresult
> -AppleVTDecoder::SubmitFrame(MediaRawData* aSample)
> +AppleVTDecoder::ProcessDecode(MediaRawData* aSample)
>  {
> +#ifdef LOG_MEDIA_SHA1
> +  SHA1Sum hash;
> +  hash.update(aSample->data, aSample->size);

you can get rid of that, ive never used it ever...

Comment 8

[Jean-Yves Avenard [:jya]]

Comment on attachment 8752030 [details]
MozReview Request: Bug 1272553. Part 2 - extract common code to the parent class. r=jya.

https://reviewboard.mozilla.org/r/52393/#review49399

great. should have done this a long time ago...

Comment 9

[Jean-Yves Avenard [:jya]]

Comment on attachment 8752031 [details]
MozReview Request: Bug 1272553. Part 3 - make mTaskQueue private. r=jya.

https://reviewboard.mozilla.org/r/52395/#review49401

::: dom/media/platforms/apple/AppleVDADecoder.h:141
(Diff revision 1)
>    // Cleared on mTaskQueue in ProcessDrain().
>    Atomic<bool> mIsFlushing;
>    ReorderQueue mReorderQueue;
>  
>  private:
> +  RefPtr<FlushableTaskQueue> mTaskQueue;

can make this const too like the other MediaDataDecoder

Comment 10

[JW Wang [:jwwang] [:jw_wang]]

Thanks!

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c2c0d7252dd
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c9ed3793ec2
https://hg.mozilla.org/integration/mozilla-inbound/rev/837ef3fa5ae3

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0c2c0d7252dd
https://hg.mozilla.org/mozilla-central/rev/3c9ed3793ec2
https://hg.mozilla.org/mozilla-central/rev/837ef3fa5ae3