1265634 - Remove dependency on MediaDecoderReader from MDSM

Status: RESOLVED FIXED

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

Description

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

MediaDecoderReaderWrapper provides all the functions that MDSM needs. It also dispatches tasks to the task queue of the reader to simplify the coding of MDSM.

Comment 1

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

Attachment: MozReview Request: Bug 1265634. Part 1 - add more proxy functions to MediaDecoderReaderWrapper and remove unused members from MDSM. r=kaku.

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

Comment 2

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

Attachment: MozReview Request: Bug 1265634. Part 2 - rename mReaderWrapper for less wording. r=kaku.

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

Comment 3

[Tzuhao Kuo [:kaku]]

Comment on attachment 8742741 [details]
MozReview Request: Bug 1265634. Part 1 - add more proxy functions to MediaDecoderReaderWrapper and remove unused members from MDSM. r=kaku.

https://reviewboard.mozilla.org/r/47441/#review44359

::: dom/media/MediaDecoderStateMachine.h:247
(Diff revision 1)
>  
> -  size_t SizeOfVideoQueue() {
> +  size_t SizeOfVideoQueue();
> -    return mReader->SizeOfVideoQueueInBytes();
> -  }
>  
> -  size_t SizeOfAudioQueue() {
> +  size_t SizeOfAudioQueue();

Can we make these two method const? The MediaDecoderReaderWrapper::SizeOfAudioQueueInBytes() and MediaDecoderReaderWrapper::SizeOfVideoQueueInBytes() are already const methods and it seems that these two methods are not going to modify anything.

Comment 4

[Tzuhao Kuo [:kaku]]

Comment on attachment 8742742 [details]
MozReview Request: Bug 1265634. Part 2 - rename mReaderWrapper for less wording. r=kaku.

https://reviewboard.mozilla.org/r/47443/#review44363

Comment 5

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

Thanks!

Comment 6

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

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=50a218e683cb

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/913714b39c4c
https://hg.mozilla.org/integration/mozilla-inbound/rev/9161ee282b67

Comment 8

[Jean-Yves Avenard [:jya]]

https://reviewboard.mozilla.org/r/47441/#review44413

there's a few members that could be constified...

::: dom/media/MediaDecoderStateMachine.h:225
(Diff revision 1)
>  
>    TimedMetadataEventSource& TimedMetadataEvent() {
>      return mMetadataManager.TimedMetadataEvent();
>    }
>  
> -  MediaEventSource<void>& OnMediaNotSeekable() {
> +  MediaEventSource<void>& OnMediaNotSeekable();

const?

::: dom/media/MediaDecoderStateMachine.h:245
(Diff revision 1)
>    OnSeekingStart() { return mOnSeekingStart; }
>  
>    // Immutable after construction - may be called on any thread.
>    bool IsRealTime() const { return mRealTime; }
>  
> -  size_t SizeOfVideoQueue() {
> +  size_t SizeOfVideoQueue();

could be made const

::: dom/media/MediaDecoderStateMachine.h:1042
(Diff revision 1)
>  
>    // Used to distiguish whether the audio is producing sound.
>    Canonical<bool> mIsAudioDataAudible;
>  
>  public:
> -  AbstractCanonical<media::TimeIntervals>* CanonicalBuffered() {
> +  AbstractCanonical<media::TimeIntervals>* CanonicalBuffered();

const too

Comment 9

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

Attachment: 1265634_part3_constify_functions.patch

Bug 1265634. Part 3 - constify some functions. r=jya.

Comment 10

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

Thanks!

Comment 11

[Pulsebot]

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

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/913714b39c4c
https://hg.mozilla.org/mozilla-central/rev/9161ee282b67
https://hg.mozilla.org/mozilla-central/rev/f2d195c7313f