1214997 - Use MozPromise in MediaCodecProxy and OMXCodecProxy

Status: RESOLVED FIXED

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

Description

[Sotaro Ikeda [:sotaro]]

It seems nice to use MozPromise in MediaSystemResourceClient.

Comment 1

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #0)
> It seems nice to use MozPromise in MediaSystemResourceClient.

bug's scope was changed as to use MozPromise in MediaCodecProxy and OMXCodecProxy.

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Use MozPromise in MediaCodecProxy and OMXCodecProxy

Comment 3

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Use MozPromise in MediaCodecProxy and OMXCodecProxy

Rebased.

Comment 4

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Use MozPromise in MediaCodecProxy and OMXCodecProxy

Correct patch.

Comment 5

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

Comment on attachment 8684735 [details] [diff] [review]
patch - Use MozPromise in MediaCodecProxy and OMXCodecProxy

Review of attachment 8684735 [details] [diff] [review]:
-----------------------------------------------------------------

It looks good to me!

::: dom/media/omx/MediaCodecProxy.cpp
@@ +115,5 @@
>    }
>    return false;
>  }
>  
> +RefPtr<MediaCodecProxy::CodecPromise> 

remove trailing space.

::: dom/media/omx/MediaCodecReader.h
@@ +172,5 @@
>    MozPromiseRequestHolder<MediaResourcePromise> mMediaResourceRequest;
>    MozPromiseHolder<MediaResourcePromise> mMediaResourcePromise;
>  
> +  MozPromiseRequestHolder<android::MediaCodecProxy::CodecPromise> mVideoCodecRequest;
> +

Now we have mMediaResourceRequest, mMediaResourcePromise, and mVideoCodecRequest to handle the request for video codec use. It looks a little complicated and there should be a simpler way to do it. However, we are going to remove MediaCodecReader in bug 1198576, so it would be no need to simplify this codes.

Comment 6

[Sotaro Ikeda [:sotaro]]

(In reply to Blake Wu [:bwu][:blakewu] from comment #5)
> 
> Now we have mMediaResourceRequest, mMediaResourcePromise, and
> mVideoCodecRequest to handle the request for video codec use. It looks a
> little complicated and there should be a simpler way to do it. However, we
> are going to remove MediaCodecReader in bug 1198576, so it would be no need
> to simplify this codes.

Thanks for the review. I almost did the the above suggestion, but stopped to do it because of the above reason.

Comment 7

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Use MozPromise in MediaCodecProxy and OMXCodecProxy

Apply the comment. Carry "r=bwu".

Comment 8

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6239c9a44c6f

Comment 9

[Pulsebot]

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

Comment 10

[Carsten Book [:Tomcat]]

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