AVDADeferredRenderingBackingStrategy::CopySurfaceTextureToPictures leaks fds |
|||||
Issue descriptionBy continually creating and destroying video elements on Android it's possible to get the gpu process to run out of fds. (Can be reproduced with this fiddle http://jsfiddle.net/r7y38zdj/8/) After some debugging, it appears that the code responsible for the leak is CopySurfaceTextureToPictures because the leak no longer occurs if it's not called. I haven't tried to figure which part exactly causes the leak. Ideally we'll delete CopySurfaceTextureToPictures and instead keep the underlying SurfaceTexture alive (see an old CL to attempting this: https://codereview.chromium.org/1682343002/). This should be more robust, faster, and not leak fds.
,
Jun 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e8d35a561423eb3e4520b0ae1fe5a945a62c70af commit e8d35a561423eb3e4520b0ae1fe5a945a62c70af Author: watk <watk@chromium.org> Date: Sat Jun 04 03:14:37 2016 StreamTextureImages can now override a Texture's service id Previously Textures could have an unowned service id, which is used in place of their service id, except that its lifetime is managed externally. The use case this is AVDACodecImages, which use a single SurfaceTexture to back multiple Textures. This change more explicitly ties the unowned service id concept to StreamTextureImages, and makes it possible to have StreamTextureImages own the lifetime of the unowned service id (now called the stream texture service id), by resetting the service id back to its original value if a non-StreamTextureImage is bound. If this were not done and a new image was bound to a level that previously had a StreamTextureImage, the service id would still be the unowned stream texture service id, but the StreamTextureImage would be deleted. That meant it was previously not possible for StreamTextureImages to own the lifetime of the unowned service id without possibly leaving the Texture in an invalid state. Now we can have multiple AVDACodecImages, bound to different Textures, share a SurfaceTexture service id and safely delete the texture when all of the AVDACodecImages are destructed. This CL also adds Texture::SetLevelImageState for setting only the ImageState and not changing the bound image. BUG= 614090 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2014313002 Cr-Commit-Position: refs/heads/master@{#397897} [modify] https://crrev.com/e8d35a561423eb3e4520b0ae1fe5a945a62c70af/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/e8d35a561423eb3e4520b0ae1fe5a945a62c70af/gpu/command_buffer/service/texture_manager.cc [modify] https://crrev.com/e8d35a561423eb3e4520b0ae1fe5a945a62c70af/gpu/command_buffer/service/texture_manager.h [modify] https://crrev.com/e8d35a561423eb3e4520b0ae1fe5a945a62c70af/gpu/command_buffer/service/texture_manager_unittest.cc [modify] https://crrev.com/e8d35a561423eb3e4520b0ae1fe5a945a62c70af/gpu/ipc/service/stream_texture_android.cc [modify] https://crrev.com/e8d35a561423eb3e4520b0ae1fe5a945a62c70af/media/gpu/android_deferred_rendering_backing_strategy.cc [modify] https://crrev.com/e8d35a561423eb3e4520b0ae1fe5a945a62c70af/media/gpu/avda_codec_image.cc
,
Jun 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3e465933721f2732fc422029529b80fc9784e812 commit 3e465933721f2732fc422029529b80fc9784e812 Author: watk <watk@chromium.org> Date: Tue Jun 07 14:17:06 2016 AVDACodecImages keep their backing SurfaceTexture alive Previously, when destructing an AVDA, we had to delete the AVDACodecImages attached to the PictureBuffer textures, and use a fragile method of copying the SurfaceTexture buffer and creating egl images to make sure those PictureBuffer textures didn't become unbacked. Now the AVDACodecImages are kept alive by the Textures they're bound to, and they keep a ref to AVDASharedState which contains the shared SurfaceTexture and its platform texture. The AVDASharedState destructor cleans up the SurfaceTexture and deletes the platform texture. BUG= 533630 , 614090 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2005103004 Cr-Commit-Position: refs/heads/master@{#398293} [modify] https://crrev.com/3e465933721f2732fc422029529b80fc9784e812/media/gpu/android_copying_backing_strategy.cc [modify] https://crrev.com/3e465933721f2732fc422029529b80fc9784e812/media/gpu/android_deferred_rendering_backing_strategy.cc [modify] https://crrev.com/3e465933721f2732fc422029529b80fc9784e812/media/gpu/android_video_decode_accelerator.cc [modify] https://crrev.com/3e465933721f2732fc422029529b80fc9784e812/media/gpu/android_video_decode_accelerator.h [modify] https://crrev.com/3e465933721f2732fc422029529b80fc9784e812/media/gpu/avda_codec_image.cc [modify] https://crrev.com/3e465933721f2732fc422029529b80fc9784e812/media/gpu/avda_codec_image.h [modify] https://crrev.com/3e465933721f2732fc422029529b80fc9784e812/media/gpu/avda_shared_state.cc [modify] https://crrev.com/3e465933721f2732fc422029529b80fc9784e812/media/gpu/avda_shared_state.h [modify] https://crrev.com/3e465933721f2732fc422029529b80fc9784e812/media/gpu/avda_state_provider.h
,
Jun 7 2016
,
Jun 7 2016
Your change meets the bar and is auto-approved for M52 (branch: 2743)
,
Jun 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4a8723b44c79364d23c4a6223971cc0d10ebe574 commit 4a8723b44c79364d23c4a6223971cc0d10ebe574 Author: Chris Watkins <watk@chromium.org> Date: Wed Jun 08 22:29:45 2016 StreamTextureImages can now override a Texture's service id Previously Textures could have an unowned service id, which is used in place of their service id, except that its lifetime is managed externally. The use case this is AVDACodecImages, which use a single SurfaceTexture to back multiple Textures. This change more explicitly ties the unowned service id concept to StreamTextureImages, and makes it possible to have StreamTextureImages own the lifetime of the unowned service id (now called the stream texture service id), by resetting the service id back to its original value if a non-StreamTextureImage is bound. If this were not done and a new image was bound to a level that previously had a StreamTextureImage, the service id would still be the unowned stream texture service id, but the StreamTextureImage would be deleted. That meant it was previously not possible for StreamTextureImages to own the lifetime of the unowned service id without possibly leaving the Texture in an invalid state. Now we can have multiple AVDACodecImages, bound to different Textures, share a SurfaceTexture service id and safely delete the texture when all of the AVDACodecImages are destructed. This CL also adds Texture::SetLevelImageState for setting only the ImageState and not changing the bound image. BUG= 614090 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2014313002 Cr-Commit-Position: refs/heads/master@{#397897} (cherry picked from commit e8d35a561423eb3e4520b0ae1fe5a945a62c70af) Review URL: https://codereview.chromium.org/2051633002 . Cr-Commit-Position: refs/branch-heads/2743@{#286} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/gpu/command_buffer/service/texture_manager.cc [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/gpu/command_buffer/service/texture_manager.h [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/gpu/command_buffer/service/texture_manager_unittest.cc [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/gpu/ipc/service/stream_texture_android.cc [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/media/gpu/android_deferred_rendering_backing_strategy.cc [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/media/gpu/avda_codec_image.cc
,
Jun 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20 commit 89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20 Author: Chris Watkins <watk@chromium.org> Date: Thu Jun 09 00:13:08 2016 AVDACodecImages keep their backing SurfaceTexture alive Previously, when destructing an AVDA, we had to delete the AVDACodecImages attached to the PictureBuffer textures, and use a fragile method of copying the SurfaceTexture buffer and creating egl images to make sure those PictureBuffer textures didn't become unbacked. Now the AVDACodecImages are kept alive by the Textures they're bound to, and they keep a ref to AVDASharedState which contains the shared SurfaceTexture and its platform texture. The AVDASharedState destructor cleans up the SurfaceTexture and deletes the platform texture. BUG= 533630 , 614090 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2005103004 Cr-Commit-Position: refs/heads/master@{#398293} (cherry picked from commit 3e465933721f2732fc422029529b80fc9784e812) Review URL: https://codereview.chromium.org/2049053004 . Cr-Commit-Position: refs/branch-heads/2743@{#290} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/android_copying_backing_strategy.cc [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/android_deferred_rendering_backing_strategy.cc [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/android_video_decode_accelerator.cc [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/android_video_decode_accelerator.h [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/avda_codec_image.cc [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/avda_codec_image.h [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/avda_shared_state.cc [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/avda_shared_state.h [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/avda_state_provider.h
,
Jun 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4a8723b44c79364d23c4a6223971cc0d10ebe574 commit 4a8723b44c79364d23c4a6223971cc0d10ebe574 Author: Chris Watkins <watk@chromium.org> Date: Wed Jun 08 22:29:45 2016 StreamTextureImages can now override a Texture's service id Previously Textures could have an unowned service id, which is used in place of their service id, except that its lifetime is managed externally. The use case this is AVDACodecImages, which use a single SurfaceTexture to back multiple Textures. This change more explicitly ties the unowned service id concept to StreamTextureImages, and makes it possible to have StreamTextureImages own the lifetime of the unowned service id (now called the stream texture service id), by resetting the service id back to its original value if a non-StreamTextureImage is bound. If this were not done and a new image was bound to a level that previously had a StreamTextureImage, the service id would still be the unowned stream texture service id, but the StreamTextureImage would be deleted. That meant it was previously not possible for StreamTextureImages to own the lifetime of the unowned service id without possibly leaving the Texture in an invalid state. Now we can have multiple AVDACodecImages, bound to different Textures, share a SurfaceTexture service id and safely delete the texture when all of the AVDACodecImages are destructed. This CL also adds Texture::SetLevelImageState for setting only the ImageState and not changing the bound image. BUG= 614090 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2014313002 Cr-Commit-Position: refs/heads/master@{#397897} (cherry picked from commit e8d35a561423eb3e4520b0ae1fe5a945a62c70af) Review URL: https://codereview.chromium.org/2051633002 . Cr-Commit-Position: refs/branch-heads/2743@{#286} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/gpu/command_buffer/service/texture_manager.cc [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/gpu/command_buffer/service/texture_manager.h [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/gpu/command_buffer/service/texture_manager_unittest.cc [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/gpu/ipc/service/stream_texture_android.cc [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/media/gpu/android_deferred_rendering_backing_strategy.cc [modify] https://crrev.com/4a8723b44c79364d23c4a6223971cc0d10ebe574/media/gpu/avda_codec_image.cc
,
Jun 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20 commit 89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20 Author: Chris Watkins <watk@chromium.org> Date: Thu Jun 09 00:13:08 2016 AVDACodecImages keep their backing SurfaceTexture alive Previously, when destructing an AVDA, we had to delete the AVDACodecImages attached to the PictureBuffer textures, and use a fragile method of copying the SurfaceTexture buffer and creating egl images to make sure those PictureBuffer textures didn't become unbacked. Now the AVDACodecImages are kept alive by the Textures they're bound to, and they keep a ref to AVDASharedState which contains the shared SurfaceTexture and its platform texture. The AVDASharedState destructor cleans up the SurfaceTexture and deletes the platform texture. BUG= 533630 , 614090 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2005103004 Cr-Commit-Position: refs/heads/master@{#398293} (cherry picked from commit 3e465933721f2732fc422029529b80fc9784e812) Review URL: https://codereview.chromium.org/2049053004 . Cr-Commit-Position: refs/branch-heads/2743@{#290} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/android_copying_backing_strategy.cc [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/android_deferred_rendering_backing_strategy.cc [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/android_video_decode_accelerator.cc [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/android_video_decode_accelerator.h [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/avda_codec_image.cc [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/avda_codec_image.h [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/avda_shared_state.cc [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/avda_shared_state.h [modify] https://crrev.com/89c1f3f09b3fa5fd49e5c07415f5a9e48a29cd20/media/gpu/avda_state_provider.h
,
Jun 15 2016
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dalecur...@chromium.org
, May 23 2016