1412205 - MediaCacheStream::Read() should update mStreamOffset before releasing the monitor

Status: RESOLVED FIXED

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

Description

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

MediaCache::Update() depends on mStreamOffset to make decision whether to read streams or not. It is import to update mStreamOffset before releasing the monitor so Update() won't make wrong decisions based on the stale value.

Comment 1

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

Attachment: Bug 1412205 - MediaCacheStream::Read() should update mStreamOffset before releasing the monitor.

MediaCache::Update() depends on mStreamOffset to make decision whether to
read streams or not. It is important to update mStreamOffset before releasing
the monitor so Update() won't make wrong decisions based on the stale value.

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

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8924108 [details]
Bug 1412205 - MediaCacheStream::Read() should update mStreamOffset before releasing the monitor.

https://reviewboard.mozilla.org/r/195336/#review200404

::: dom/media/MediaCache.cpp:2505
(Diff revision 1)
>          count = bytes;
>          break;
>        }
>  
> +      if (mStreamOffset != streamOffset) {
> +        // mStreamOffset is changed while we drop the lock. We need to run

The first sentence confused me. Do you mean "Update mStreamOffset before we drop the lock"?

Comment 3

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

Comment on attachment 8924108 [details]
Bug 1412205 - MediaCacheStream::Read() should update mStreamOffset before releasing the monitor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/195336/diff/1-2/

Comment 4

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

Thanks! The comment is fixed.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/010c85dae2c7
MediaCacheStream::Read() should update mStreamOffset before releasing the monitor. r=gerald

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/010c85dae2c7