Closed Bug 1265634 Opened 5 years ago Closed 5 years ago

Remove dependency on MediaDecoderReader from MDSM

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(3 files)

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.
Assignee: nobody → jwwang
Depends on: 1265311, 1265629
Attachment #8742741 - Flags: review?(tkuo) → review+
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 on attachment 8742742 [details]
MozReview Request: Bug 1265634. Part 2 - rename mReaderWrapper for less wording. r=kaku.

https://reviewboard.mozilla.org/r/47443/#review44363
Attachment #8742742 - Flags: review?(tkuo) → review+
Thanks!
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
Bug 1265634. Part 3 - constify some functions. r=jya.
Attachment #8743159 - Flags: review?(jyavenard)
Attachment #8743159 - Flags: review?(jyavenard) → review+
Thanks!
Blocks: 1266027
You need to log in before you can comment on or make changes to this bug.