| Message ID | 20190807025037.29902-1-pkoshevoy@gmail.com |
|---|---|
| State | Accepted |
| Commit | 22a14ee753f372bf0ae4bd1e743670f353f7a17a |
| Headers | show |
On Tue, Aug 6, 2019 at 8:50 PM Pavel Koshevoy <pkoshevoy@gmail.com> wrote: > > vtctx->cached_hw_frames_ctx is unref'd in videotoolbox_uninit, > but videotoolbox_hevc used ff_videotoolbox_uninit which > doesn't unref cache_hw_frames_ctx. > --- > libavcodec/videotoolbox.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c > index c718e82cc5..acaeef77dd 100644 > --- a/libavcodec/videotoolbox.c > +++ b/libavcodec/videotoolbox.c > @@ -1143,7 +1143,7 @@ const AVHWAccel ff_hevc_videotoolbox_hwaccel = { > .end_frame = videotoolbox_hevc_end_frame, > .frame_params = videotoolbox_frame_params, > .init = videotoolbox_common_init, > - .uninit = ff_videotoolbox_uninit, > + .uninit = videotoolbox_uninit, > .priv_data_size = sizeof(VTContext), > }; > > -- > 2.16.4 > Ping. It's a 1-line leak fix ... is there any reason this should not be applied? Thank you, Pavel.
On Tue, Aug 6, 2019 at 8:50 PM Pavel Koshevoy <pkoshevoy@gmail.com> wrote: > > vtctx->cached_hw_frames_ctx is unref'd in videotoolbox_uninit, > but videotoolbox_hevc used ff_videotoolbox_uninit which > doesn't unref cache_hw_frames_ctx. > --- > libavcodec/videotoolbox.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c > index c718e82cc5..acaeef77dd 100644 > --- a/libavcodec/videotoolbox.c > +++ b/libavcodec/videotoolbox.c > @@ -1143,7 +1143,7 @@ const AVHWAccel ff_hevc_videotoolbox_hwaccel = { > .end_frame = videotoolbox_hevc_end_frame, > .frame_params = videotoolbox_frame_params, > .init = videotoolbox_common_init, > - .uninit = ff_videotoolbox_uninit, > + .uninit = videotoolbox_uninit, > .priv_data_size = sizeof(VTContext), > }; > > -- > 2.16.4 > I'd like to see this merged. Would anyone be upset if I pushed it? When I ran into this issue testing on a Tesla P4 I observed that each hw frames ctx leak would cost ~120MB GPU memory. Once all GPU memory is exhausted nvdec and nvenc would fail, and this happens pretty quickly when processing 4s video fragments from unrelated sources/timelines. Rick Kern and Aman Gupta are listed as maintainers for videotoolbox*, but I don't know how to contact them. Pavel.
On Fri, Aug 23, 2019 at 8:12 AM Pavel Koshevoy <pkoshevoy@gmail.com> wrote: > > On Tue, Aug 6, 2019 at 8:50 PM Pavel Koshevoy <pkoshevoy@gmail.com> wrote: > > > > vtctx->cached_hw_frames_ctx is unref'd in videotoolbox_uninit, > > but videotoolbox_hevc used ff_videotoolbox_uninit which > > doesn't unref cache_hw_frames_ctx. > > --- > > libavcodec/videotoolbox.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c > > index c718e82cc5..acaeef77dd 100644 > > --- a/libavcodec/videotoolbox.c > > +++ b/libavcodec/videotoolbox.c > > @@ -1143,7 +1143,7 @@ const AVHWAccel ff_hevc_videotoolbox_hwaccel = { > > .end_frame = videotoolbox_hevc_end_frame, > > .frame_params = videotoolbox_frame_params, > > .init = videotoolbox_common_init, > > - .uninit = ff_videotoolbox_uninit, > > + .uninit = videotoolbox_uninit, > > .priv_data_size = sizeof(VTContext), > > }; > > > > -- > > 2.16.4 > > > > > I'd like to see this merged. Would anyone be upset if I pushed it? > > When I ran into this issue testing on a Tesla P4 I observed that each > hw frames ctx leak would cost ~120MB GPU memory. Once all GPU memory > is exhausted nvdec and nvenc would fail, and this happens pretty > quickly when processing 4s video fragments from unrelated > sources/timelines. sorry, this was half-awake nonsense ... the cuda hwframes memleak was external to ffmpeg and unrelated to this videotoolbox fix. > > Rick Kern and Aman Gupta are listed as maintainers for videotoolbox*, > but I don't know how to contact them. > > Pavel.
On Wed, Aug 14, 2019 at 3:53 AM Pavel Koshevoy <pkoshevoy@gmail.com> wrote: > On Tue, Aug 6, 2019 at 8:50 PM Pavel Koshevoy <pkoshevoy@gmail.com> wrote: > > > > vtctx->cached_hw_frames_ctx is unref'd in videotoolbox_uninit, > > but videotoolbox_hevc used ff_videotoolbox_uninit which > > doesn't unref cache_hw_frames_ctx. > > --- > > libavcodec/videotoolbox.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c > > index c718e82cc5..acaeef77dd 100644 > > --- a/libavcodec/videotoolbox.c > > +++ b/libavcodec/videotoolbox.c > > @@ -1143,7 +1143,7 @@ const AVHWAccel ff_hevc_videotoolbox_hwaccel = { > > .end_frame = videotoolbox_hevc_end_frame, > > .frame_params = videotoolbox_frame_params, > > .init = videotoolbox_common_init, > > - .uninit = ff_videotoolbox_uninit, > > + .uninit = videotoolbox_uninit, > > .priv_data_size = sizeof(VTContext), > > }; > > > > -- > > 2.16.4 > > > > > Ping. It's a 1-line leak fix ... is there any reason this should not > be applied? > LGTM. Feel free to push it. > > Thank you, > Pavel. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 8/23/19 3:46 PM, Aman Gupta wrote: > On Wed, Aug 14, 2019 at 3:53 AM Pavel Koshevoy <pkoshevoy@gmail.com> wrote: > >> On Tue, Aug 6, 2019 at 8:50 PM Pavel Koshevoy <pkoshevoy@gmail.com> wrote: >>> vtctx->cached_hw_frames_ctx is unref'd in videotoolbox_uninit, >>> but videotoolbox_hevc used ff_videotoolbox_uninit which >>> doesn't unref cache_hw_frames_ctx. >>> --- >>> libavcodec/videotoolbox.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c >>> index c718e82cc5..acaeef77dd 100644 >>> --- a/libavcodec/videotoolbox.c >>> +++ b/libavcodec/videotoolbox.c >>> @@ -1143,7 +1143,7 @@ const AVHWAccel ff_hevc_videotoolbox_hwaccel = { >>> .end_frame = videotoolbox_hevc_end_frame, >>> .frame_params = videotoolbox_frame_params, >>> .init = videotoolbox_common_init, >>> - .uninit = ff_videotoolbox_uninit, >>> + .uninit = videotoolbox_uninit, >>> .priv_data_size = sizeof(VTContext), >>> }; >>> >>> -- >>> 2.16.4 >>> >> >> Ping. It's a 1-line leak fix ... is there any reason this should not >> be applied? >> > LGTM. Feel free to push it. > applied and pushed, Thank you, Pavel.
diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c index c718e82cc5..acaeef77dd 100644 --- a/libavcodec/videotoolbox.c +++ b/libavcodec/videotoolbox.c @@ -1143,7 +1143,7 @@ const AVHWAccel ff_hevc_videotoolbox_hwaccel = { .end_frame = videotoolbox_hevc_end_frame, .frame_params = videotoolbox_frame_params, .init = videotoolbox_common_init, - .uninit = ff_videotoolbox_uninit, + .uninit = videotoolbox_uninit, .priv_data_size = sizeof(VTContext), };