Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: media source object URL revocation #5214

Merged
merged 1 commit into from
May 5, 2023
Merged

fix: media source object URL revocation #5214

merged 1 commit into from
May 5, 2023

Conversation

zangue
Copy link
Contributor

@zangue zangue commented May 5, 2023

createMediaSource() sets this.url_ for later revocation when the media source opens. However, createMediaSource() is called before the url_ property is declared and initialised in the constructor. As result, this.url_ actually get declared an initialised with the object URL in createMediaSource() and is later overwritten with an empty string in the constructor. This compromises the revocation logic as we end up always passing an empty string to URL.revokeObjectURL().

To fix this, we make sure this.url_ is declared and assigned default value in the constructor before any other class methods are called.

@zangue
fix: media source object URL revocation
2e4eb3f 
`createMediaSource()` sets `this.url_` for later revocation when the media
source opens. However, `createMediaSource()` is called before the `url_`
property is declared and initialised in the constructor. As result, `this.url_`
actually get declared an initialised with the object URL in
`createMediaSource()` and is later overwritten with an empty string in the
constructor. This compromises the revocation logic as we end up always passing
an empty string to `URL.revokeObjectURL()`.

To fix this, we make sure `this.url_` is declared and assigned default value in
the constructor before any other class methods are called.
@github-actions
Copy link
Contributor

github-actions bot commented May 5, 2023

Incremental code coverage: 100.00%

@avelad avelad added type: bug Something isn't working correctly priority: P2 Smaller impact or easy workaround labels May 5, 2023
@avelad avelad added this to the v4.4 milestone May 5, 2023
@avelad avelad merged commit fecb11a into shaka-project:main May 5, 2023
@github-actions github-actions bot mentioned this pull request May 5, 2023
Merged
joeyparrish pushed a commit that referenced this pull request May 30, 2023
@zangue @joeyparrish
fix: media source object URL revocation (#5214)
80ce378 
`createMediaSource()` sets `this.url_` for later revocation when the
media source opens. However, `createMediaSource()` is called before the
`url_` property is declared and initialised in the constructor. As
result, `this.url_` actually get declared an initialised with the object
URL in `createMediaSource()` and is later overwritten with an empty
string in the constructor. This compromises the revocation logic as we
end up always passing an empty string to `URL.revokeObjectURL()`.

To fix this, we make sure `this.url_` is declared and assigned default
value in the constructor before any other class methods are called.
@github-actions github-actions bot mentioned this pull request May 30, 2023
Merged
joeyparrish pushed a commit that referenced this pull request May 30, 2023
@zangue @joeyparrish
fix: media source object URL revocation (#5214)
1a89daa 
`createMediaSource()` sets `this.url_` for later revocation when the
media source opens. However, `createMediaSource()` is called before the
`url_` property is declared and initialised in the constructor. As
result, `this.url_` actually get declared an initialised with the object
URL in `createMediaSource()` and is later overwritten with an empty
string in the constructor. This compromises the revocation logic as we
end up always passing an empty string to `URL.revokeObjectURL()`.

To fix this, we make sure `this.url_` is declared and assigned default
value in the constructor before any other class methods are called.
@github-actions github-actions bot mentioned this pull request May 30, 2023
Merged
JulianDomingo pushed a commit that referenced this pull request Jun 20, 2023
@github-actions
chore(v4.2.x): release 4.2.11 (#5254)
76e84d5 
🤖 I have created a release *beep* *boop*
---


##
[4.2.11](v4.2.10...v4.2.11)
(2023-06-20)


### Bug Fixes

* **Demo:** Fix deployment of codem-isoboxer in the Demo
([#5257](#5257))
([23d48d4](23d48d4))
* **Demo:** Fix error link width to avoid overlap with close button
([#5309](#5309))
([a6f980d](a6f980d))
* Fix error when network status changes on src= playbacks
([#5305](#5305))
([ce354ba](ce354ba))
* **HLS:** Avoid "Possible encoding problem detected!" when is a preload
reference
([#5332](#5332))
([763ae6a](763ae6a))
* **HLS:** Avoid HLS resync when there is a gap in the stream
([#5284](#5284))
([256cf20](256cf20))
* **HLS:** Avoid variable substitution if no variables
([#5269](#5269))
([b549b60](b549b60))
* **HLS:** Parse EXT-X-PART-INF as media playlist tag
([#5311](#5311))
([d78c080](d78c080))
* **HLS:** Skip EXT-X-PRELOAD-HINT without full byterange info
([#5294](#5294))
([e462711](e462711))
* media source object URL revocation
([#5214](#5214))
([80ce378](80ce378))
* Ship to NPM without node version restrictions
([#5253](#5253))
([41c1ace](41c1ace)),
closes
[#5243](#5243)
* unnecessary parsing of in-band pssh when pssh is in the manifest
([#5198](#5198))
([889cc68](889cc68))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
JulianDomingo pushed a commit that referenced this pull request Jun 21, 2023
@github-actions
chore(v4.3.x): release 4.3.7 (#5255)
203231d 
🤖 I have created a release *beep* *boop*
---


##
[4.3.7](v4.3.6...v4.3.7)
(2023-06-21)


### Bug Fixes

* CEA 608 captions not work with H.265 video streams
([#5252](#5252))
([b08bb41](b08bb41)),
closes
[#5251](#5251)
* **Demo:** Fix deployment of codem-isoboxer in the Demo
([#5257](#5257))
([7e2903a](7e2903a))
* **demo:** Fix deployment of v4.3.x on appspot
([ccf5e2e](ccf5e2e))
* **Demo:** Fix error link width to avoid overlap with close button
([#5309](#5309))
([f575dab](f575dab))
* Fix error when network status changes on src= playbacks
([#5305](#5305))
([cf683f5](cf683f5))
* **HLS:** Avoid "Possible encoding problem detected!" when is a preload
reference
([#5332](#5332))
([9ce8cc0](9ce8cc0))
* **HLS:** Avoid HLS resync when there is a gap in the stream
([#5284](#5284))
([679dbae](679dbae))
* **HLS:** Avoid variable substitution if no variables
([#5269](#5269))
([49afa92](49afa92))
* **HLS:** Fix HLS seekRange for live streams
([#5263](#5263))
([03df9cb](03df9cb))
* **HLS:** Fix seekRange for EVENT playlist not using
EXT-X-PLAYLIST-TYPE
([#5220](#5220))
([562831b](562831b))
* **HLS:** Parse EXT-X-PART-INF as media playlist tag
([#5311](#5311))
([f6210ee](f6210ee))
* **HLS:** Skip EXT-X-PRELOAD-HINT without full byterange info
([#5294](#5294))
([9e193e2](9e193e2))
* media source object URL revocation
([#5214](#5214))
([1a89daa](1a89daa))
* Ship to NPM without node version restrictions
([#5253](#5253))
([ca096a8](ca096a8)),
closes
[#5243](#5243)
* unnecessary parsing of in-band pssh when pssh is in the manifest
([#5198](#5198))
([8d6494d](8d6494d))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@avelad avelad avelad approved these changes

@littlespex littlespex littlespex approved these changes

@joeyparrish joeyparrish Awaiting requested review from joeyparrish

@theodab theodab Awaiting requested review from theodab

Assignees
No one assigned
Labels
priority: P2 Smaller impact or easy workaround status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Milestone
v4.4
Development

Successfully merging this pull request may close these issues.
3 participants
@zangue @littlespex @avelad