1220558 - Remove aLength and aOffset from arguments of MediaDecoderReader::DispatchNotifyDataArrived()

Status: RESOLVED FIXED

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

Description

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

Since bug 1218157 is landded, the arguments are unused anymore.

Comment 1

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

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2266e4c11ae8

Comment 2

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

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

Comment 3

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

Attachment: MozReview Request: Bug 1220558. Part 1 - remove unused arguments from MediaDecoderReader::DispatchNotifyDataArrived() and its callees/callers. r=jya.

Bug 1220558. Part 1 - remove unused arguments from MediaDecoderReader::DispatchNotifyDataArrived() and its callees/callers. r=jya.

Comment 4

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

Attachment: MozReview Request: Bug 1220558. Part 2 - remove unused members. r=jya.

Bug 1220558. Part 2 - remove unused members. r=jya.

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8682301 [details]
MozReview Request: Bug 1220558. Part 1 - remove unused arguments from MediaDecoderReader::DispatchNotifyDataArrived() and its callees/callers. r=jya.

https://reviewboard.mozilla.org/r/24053/#review21737

Can be even more aggressive in the cleanup. And remove everything related to throttling.
I would much prefer for the MEdiaResource not to fire NotifyDataArrived as often as it does (every 32 kB)

::: dom/media/AbstractMediaDecoder.h:114
(Diff revision 1)
> -  virtual void NotifyDataArrived(uint32_t aLength, int64_t aOffset,
> +  virtual void NotifyDataArrived(bool aThrottleUpdates) = 0;

I would even remove ThrottleUpdates and all the machineray around throttling.

The various NotifyDataArrivedInternal implementation now only ever parse new data ; so throttling is unecessary

::: dom/media/MediaDecoderReader.h:236
(Diff revision 1)
>        aThrottleUpdates ? &MediaDecoderReader::ThrottledNotifyDataArrived :

so this can go and only dispatch MediaDecoderReader::NotifyDataArrived

::: dom/media/MediaDecoderReader.cpp:186
(Diff revision 1)
> -MediaDecoderReader::ThrottledNotifyDataArrived(const Interval<int64_t>& aInterval)
> +MediaDecoderReader::ThrottledNotifyDataArrived()

remove

Comment 6

[Jean-Yves Avenard [:jya]]

Comment on attachment 8682302 [details]
MozReview Request: Bug 1220558. Part 2 - remove unused members. r=jya.

https://reviewboard.mozilla.org/r/24055/#review21739

::: dom/media/MediaDecoderReader.h:366
(Diff revision 1)
>    MozPromiseRequestHolder<MediaTimerPromise> mThrottledNotify;

Assuming all of this will also go in the previous patch

Comment 7

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

(In reply to Jean-Yves Avenard [:jya] from comment #5)
> Can be even more aggressive in the cleanup. And remove everything related to
> throttling.
> I would much prefer for the MEdiaResource not to fire NotifyDataArrived as
> often as it does (every 32 kB)

I will do that in a new bug because the rationale for removing throttling isn't as trivia as removing first 2 arguments in this bug.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef5ac56a15d2
https://hg.mozilla.org/integration/mozilla-inbound/rev/b509735f38af

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ef5ac56a15d2
https://hg.mozilla.org/mozilla-central/rev/b509735f38af