search

gurumukhi / youtube-screenshot

Cross browser addon for youtube video screenshots
https://addons.mozilla.org/en-US/firefox/addon/youtube-screenshot-button/
Mozilla Public License 2.0
41 stars 8 forks source link

Very bad memory leak #59

Open probablytukars opened 2 months ago

probablytukars commented 2 months ago

If you screenshot multiple 4k videos using png (jpeg is also affected, but the leak grows slower) and keep the browser open it is very easy to get 16GB of memory leaked, just take like 20 screenshots and you will already be there. This can make the page file grow to insanely large amounts if only a few screenshots are taken, slowing down your computer immensely.

joggee-fr commented 2 months ago

Good catch @probablytukars. The addon add an hyperlink node for each screenshot to download and even it the node is removed immediately from DOM, it seems the browser does not free up the memory. I can think of two ways to avoid such an issue:

  1. Pass the blob to the background script and use downloads.download(). Furthermore, it will allow to address issue #58. I have given it a quick test and it looks OK.
  2. Re-use the same hyperlink node adding a special identifier to it.

I think I prefer option 1 right now. @gurumukhi, any opinion on this?

gurumukhi commented 1 month ago

Thanks @probablytukars & @joggee-fr. Yes, option 1 looks good to me too. I have merged the PR and have submitted the new version for publishing.

gurumukhi commented 1 month ago

I hope this is fixed now. Closing the issue, feel free to please comment here if you noticed anything else.

gurumukhi commented 1 month ago

Looks like there is some issue with the code changes done here as per https://github.com/gurumukhi/youtube-screenshot/issues/61.