* [FFmpeg-devel] [PATCH 1/3] lavu/hwcontext_opencl: clear dangling pointers on map failure @ 2022-01-19 13:40 Anton Khirnov 2022-01-19 13:40 ` [FFmpeg-devel] [PATCH 2/3] lavu/hwcontext_vulkan: " Anton Khirnov 2022-01-19 13:40 ` [FFmpeg-devel] [PATCH 3/3] lavu/hwcontext: clarify behavior on av_hwframe_map() failure Anton Khirnov 0 siblings, 2 replies; 6+ messages in thread From: Anton Khirnov @ 2022-01-19 13:40 UTC (permalink / raw) To: ffmpeg-devel --- untested --- libavutil/hwcontext_opencl.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libavutil/hwcontext_opencl.c b/libavutil/hwcontext_opencl.c index 26a3a24593..40ee611943 100644 --- a/libavutil/hwcontext_opencl.c +++ b/libavutil/hwcontext_opencl.c @@ -2146,6 +2146,7 @@ fail: clReleaseMemObject(mapping->frame.planes[p]); } av_free(mapping); + memset(dst->data, 0, sizeof(dst->data)); return err; } @@ -2317,6 +2318,7 @@ fail: if (desc->planes[p]) clReleaseMemObject(desc->planes[p]); av_freep(&desc); + memset(dst->data, 0, sizeof(dst->data)); return err; } @@ -2407,6 +2409,7 @@ fail: 0, NULL, &event); if (cle == CL_SUCCESS) opencl_wait_events(dst_fc, &event, 1); + memset(dst->data, 0, sizeof(dst->data)); return err; } @@ -2562,6 +2565,7 @@ fail: 0, NULL, &event); if (cle == CL_SUCCESS) opencl_wait_events(dst_fc, &event, 1); + memset(dst->data, 0, sizeof(dst->data)); return err; } @@ -2793,6 +2797,7 @@ fail: clReleaseMemObject(mapping->object_buffers[i]); } av_free(mapping); + memset(dst->data, 0, sizeof(dst->data)); return err; } -- 2.33.0 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 6+ messages in thread
* [FFmpeg-devel] [PATCH 2/3] lavu/hwcontext_vulkan: clear dangling pointers on map failure 2022-01-19 13:40 [FFmpeg-devel] [PATCH 1/3] lavu/hwcontext_opencl: clear dangling pointers on map failure Anton Khirnov @ 2022-01-19 13:40 ` Anton Khirnov 2022-01-19 13:40 ` [FFmpeg-devel] [PATCH 3/3] lavu/hwcontext: clarify behavior on av_hwframe_map() failure Anton Khirnov 1 sibling, 0 replies; 6+ messages in thread From: Anton Khirnov @ 2022-01-19 13:40 UTC (permalink / raw) To: ffmpeg-devel --- untested --- libavutil/hwcontext_vulkan.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c index 60a6cf6a91..ae19fc2ab6 100644 --- a/libavutil/hwcontext_vulkan.c +++ b/libavutil/hwcontext_vulkan.c @@ -2927,6 +2927,7 @@ static int vulkan_map_from_drm(AVHWFramesContext *hwfc, AVFrame *dst, fail: vulkan_frame_free(hwfc->device_ctx->hwctx, (uint8_t *)f); + dst->data[0] = NULL; return err; } -- 2.33.0 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 6+ messages in thread
* [FFmpeg-devel] [PATCH 3/3] lavu/hwcontext: clarify behavior on av_hwframe_map() failure 2022-01-19 13:40 [FFmpeg-devel] [PATCH 1/3] lavu/hwcontext_opencl: clear dangling pointers on map failure Anton Khirnov 2022-01-19 13:40 ` [FFmpeg-devel] [PATCH 2/3] lavu/hwcontext_vulkan: " Anton Khirnov @ 2022-01-19 13:40 ` Anton Khirnov 2022-01-19 13:45 ` James Almer 1 sibling, 1 reply; 6+ messages in thread From: Anton Khirnov @ 2022-01-19 13:40 UTC (permalink / raw) To: ffmpeg-devel Clear anything that av_hwframe_map() might have done to the destination frame, but leave caller-provided fields unchanged. --- libavutil/hwcontext.c | 23 +++++++++++++++++++++-- libavutil/hwcontext.h | 4 ++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c index 31c7840dba..ae33da1262 100644 --- a/libavutil/hwcontext.c +++ b/libavutil/hwcontext.c @@ -18,6 +18,7 @@ #include "config.h" +#include "avassert.h" #include "buffer.h" #include "common.h" #include "hwcontext.h" @@ -788,6 +789,8 @@ fail: int av_hwframe_map(AVFrame *dst, const AVFrame *src, int flags) { + AVBufferRef *orig_dst_frames = dst->hw_frames_ctx; + enum AVPixelFormat orig_dst_fmt = dst->format; AVHWFramesContext *src_frames, *dst_frames; HWMapDescriptor *hwmap; int ret; @@ -825,7 +828,7 @@ int av_hwframe_map(AVFrame *dst, const AVFrame *src, int flags) ret = src_frames->internal->hw_type->map_from(src_frames, dst, src, flags); if (ret != AVERROR(ENOSYS)) - return ret; + goto fail; } } @@ -837,11 +840,27 @@ int av_hwframe_map(AVFrame *dst, const AVFrame *src, int flags) ret = dst_frames->internal->hw_type->map_to(dst_frames, dst, src, flags); if (ret != AVERROR(ENOSYS)) - return ret; + goto fail; } } return AVERROR(ENOSYS); + +fail: + // if the caller provided dst frames context, it should be preserved + // by this function + av_assert0(orig_dst_frames == NULL || + orig_dst_frames == dst->hw_frames_ctx); + + // preserve user-provided dst frame fields, but clean + // anything we might have set + dst->hw_frames_ctx = NULL; + av_frame_unref(dst); + + dst->hw_frames_ctx = orig_dst_frames; + dst->format = orig_dst_fmt; + + return ret; } int av_hwframe_ctx_create_derived(AVBufferRef **derived_frame_ctx, diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h index 04d19d89c2..c18b7e1e8b 100644 --- a/libavutil/hwcontext.h +++ b/libavutil/hwcontext.h @@ -571,6 +571,10 @@ enum { * possible with the given arguments and hwframe setup, while other return * values indicate that it failed somehow. * + * On failure, the destination frame will be left blank, except for the + * hw_frames_ctx/format fields thay may have been set by the caller - those will + * be preserved as they were. + * * @param dst Destination frame, to contain the mapping. * @param src Source frame, to be mapped. * @param flags Some combination of AV_HWFRAME_MAP_* flags. -- 2.33.0 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/3] lavu/hwcontext: clarify behavior on av_hwframe_map() failure 2022-01-19 13:40 ` [FFmpeg-devel] [PATCH 3/3] lavu/hwcontext: clarify behavior on av_hwframe_map() failure Anton Khirnov @ 2022-01-19 13:45 ` James Almer 2022-01-19 13:47 ` Anton Khirnov 2022-01-19 14:01 ` [FFmpeg-devel] [PATCH] " Anton Khirnov 0 siblings, 2 replies; 6+ messages in thread From: James Almer @ 2022-01-19 13:45 UTC (permalink / raw) To: ffmpeg-devel On 1/19/2022 10:40 AM, Anton Khirnov wrote: > Clear anything that av_hwframe_map() might have done to the destination > frame, but leave caller-provided fields unchanged. > --- > libavutil/hwcontext.c | 23 +++++++++++++++++++++-- > libavutil/hwcontext.h | 4 ++++ > 2 files changed, 25 insertions(+), 2 deletions(-) > > diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c > index 31c7840dba..ae33da1262 100644 > --- a/libavutil/hwcontext.c > +++ b/libavutil/hwcontext.c > @@ -18,6 +18,7 @@ > > #include "config.h" > > +#include "avassert.h" > #include "buffer.h" > #include "common.h" > #include "hwcontext.h" > @@ -788,6 +789,8 @@ fail: > > int av_hwframe_map(AVFrame *dst, const AVFrame *src, int flags) > { > + AVBufferRef *orig_dst_frames = dst->hw_frames_ctx; > + enum AVPixelFormat orig_dst_fmt = dst->format; > AVHWFramesContext *src_frames, *dst_frames; > HWMapDescriptor *hwmap; > int ret; > @@ -825,7 +828,7 @@ int av_hwframe_map(AVFrame *dst, const AVFrame *src, int flags) > ret = src_frames->internal->hw_type->map_from(src_frames, > dst, src, flags); > if (ret != AVERROR(ENOSYS)) > - return ret; > + goto fail; What if ret is 0? Is the stuff in the fail label meant to run on success? > } > } > > @@ -837,11 +840,27 @@ int av_hwframe_map(AVFrame *dst, const AVFrame *src, int flags) > ret = dst_frames->internal->hw_type->map_to(dst_frames, > dst, src, flags); > if (ret != AVERROR(ENOSYS)) > - return ret; > + goto fail; > } > } > > return AVERROR(ENOSYS); > + > +fail: > + // if the caller provided dst frames context, it should be preserved > + // by this function > + av_assert0(orig_dst_frames == NULL || > + orig_dst_frames == dst->hw_frames_ctx); > + > + // preserve user-provided dst frame fields, but clean > + // anything we might have set > + dst->hw_frames_ctx = NULL; > + av_frame_unref(dst); > + > + dst->hw_frames_ctx = orig_dst_frames; > + dst->format = orig_dst_fmt; > + > + return ret; > } > > int av_hwframe_ctx_create_derived(AVBufferRef **derived_frame_ctx, > diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h > index 04d19d89c2..c18b7e1e8b 100644 > --- a/libavutil/hwcontext.h > +++ b/libavutil/hwcontext.h > @@ -571,6 +571,10 @@ enum { > * possible with the given arguments and hwframe setup, while other return > * values indicate that it failed somehow. > * > + * On failure, the destination frame will be left blank, except for the > + * hw_frames_ctx/format fields thay may have been set by the caller - those will > + * be preserved as they were. > + * > * @param dst Destination frame, to contain the mapping. > * @param src Source frame, to be mapped. > * @param flags Some combination of AV_HWFRAME_MAP_* flags. _______________________________________________ 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". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/3] lavu/hwcontext: clarify behavior on av_hwframe_map() failure 2022-01-19 13:45 ` James Almer @ 2022-01-19 13:47 ` Anton Khirnov 2022-01-19 14:01 ` [FFmpeg-devel] [PATCH] " Anton Khirnov 1 sibling, 0 replies; 6+ messages in thread From: Anton Khirnov @ 2022-01-19 13:47 UTC (permalink / raw) To: FFmpeg development discussions and patches Quoting James Almer (2022-01-19 14:45:12) > > > On 1/19/2022 10:40 AM, Anton Khirnov wrote: > > Clear anything that av_hwframe_map() might have done to the destination > > frame, but leave caller-provided fields unchanged. > > --- > > libavutil/hwcontext.c | 23 +++++++++++++++++++++-- > > libavutil/hwcontext.h | 4 ++++ > > 2 files changed, 25 insertions(+), 2 deletions(-) > > > > diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c > > index 31c7840dba..ae33da1262 100644 > > --- a/libavutil/hwcontext.c > > +++ b/libavutil/hwcontext.c > > @@ -18,6 +18,7 @@ > > > > #include "config.h" > > > > +#include "avassert.h" > > #include "buffer.h" > > #include "common.h" > > #include "hwcontext.h" > > @@ -788,6 +789,8 @@ fail: > > > > int av_hwframe_map(AVFrame *dst, const AVFrame *src, int flags) > > { > > + AVBufferRef *orig_dst_frames = dst->hw_frames_ctx; > > + enum AVPixelFormat orig_dst_fmt = dst->format; > > AVHWFramesContext *src_frames, *dst_frames; > > HWMapDescriptor *hwmap; > > int ret; > > @@ -825,7 +828,7 @@ int av_hwframe_map(AVFrame *dst, const AVFrame *src, int flags) > > ret = src_frames->internal->hw_type->map_from(src_frames, > > dst, src, flags); > > if (ret != AVERROR(ENOSYS)) > > - return ret; > > + goto fail; > > What if ret is 0? Is the stuff in the fail label meant to run on success? Critical brain failure. Will fix. -- Anton Khirnov _______________________________________________ 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". ^ permalink raw reply [flat|nested] 6+ messages in thread
* [FFmpeg-devel] [PATCH] lavu/hwcontext: clarify behavior on av_hwframe_map() failure 2022-01-19 13:45 ` James Almer 2022-01-19 13:47 ` Anton Khirnov @ 2022-01-19 14:01 ` Anton Khirnov 1 sibling, 0 replies; 6+ messages in thread From: Anton Khirnov @ 2022-01-19 14:01 UTC (permalink / raw) To: ffmpeg-devel Clear anything that av_hwframe_map() might have done to the destination frame, but leave caller-provided fields unchanged. --- Interdiff: diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c index ae33da1262..ab9ad3703e 100644 --- a/libavutil/hwcontext.c +++ b/libavutil/hwcontext.c @@ -827,7 +827,9 @@ int av_hwframe_map(AVFrame *dst, const AVFrame *src, int flags) src_frames->internal->hw_type->map_from) { ret = src_frames->internal->hw_type->map_from(src_frames, dst, src, flags); - if (ret != AVERROR(ENOSYS)) + if (ret >= 0) + return ret; + else if (ret != AVERROR(ENOSYS)) goto fail; } } @@ -839,7 +841,9 @@ int av_hwframe_map(AVFrame *dst, const AVFrame *src, int flags) dst_frames->internal->hw_type->map_to) { ret = dst_frames->internal->hw_type->map_to(dst_frames, dst, src, flags); - if (ret != AVERROR(ENOSYS)) + if (ret >= 0) + return ret; + else if (ret != AVERROR(ENOSYS)) goto fail; } } libavutil/hwcontext.c | 27 +++++++++++++++++++++++++-- libavutil/hwcontext.h | 4 ++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c index 31c7840dba..ab9ad3703e 100644 --- a/libavutil/hwcontext.c +++ b/libavutil/hwcontext.c @@ -18,6 +18,7 @@ #include "config.h" +#include "avassert.h" #include "buffer.h" #include "common.h" #include "hwcontext.h" @@ -788,6 +789,8 @@ fail: int av_hwframe_map(AVFrame *dst, const AVFrame *src, int flags) { + AVBufferRef *orig_dst_frames = dst->hw_frames_ctx; + enum AVPixelFormat orig_dst_fmt = dst->format; AVHWFramesContext *src_frames, *dst_frames; HWMapDescriptor *hwmap; int ret; @@ -824,8 +827,10 @@ int av_hwframe_map(AVFrame *dst, const AVFrame *src, int flags) src_frames->internal->hw_type->map_from) { ret = src_frames->internal->hw_type->map_from(src_frames, dst, src, flags); - if (ret != AVERROR(ENOSYS)) + if (ret >= 0) return ret; + else if (ret != AVERROR(ENOSYS)) + goto fail; } } @@ -836,12 +841,30 @@ int av_hwframe_map(AVFrame *dst, const AVFrame *src, int flags) dst_frames->internal->hw_type->map_to) { ret = dst_frames->internal->hw_type->map_to(dst_frames, dst, src, flags); - if (ret != AVERROR(ENOSYS)) + if (ret >= 0) return ret; + else if (ret != AVERROR(ENOSYS)) + goto fail; } } return AVERROR(ENOSYS); + +fail: + // if the caller provided dst frames context, it should be preserved + // by this function + av_assert0(orig_dst_frames == NULL || + orig_dst_frames == dst->hw_frames_ctx); + + // preserve user-provided dst frame fields, but clean + // anything we might have set + dst->hw_frames_ctx = NULL; + av_frame_unref(dst); + + dst->hw_frames_ctx = orig_dst_frames; + dst->format = orig_dst_fmt; + + return ret; } int av_hwframe_ctx_create_derived(AVBufferRef **derived_frame_ctx, diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h index 04d19d89c2..c18b7e1e8b 100644 --- a/libavutil/hwcontext.h +++ b/libavutil/hwcontext.h @@ -571,6 +571,10 @@ enum { * possible with the given arguments and hwframe setup, while other return * values indicate that it failed somehow. * + * On failure, the destination frame will be left blank, except for the + * hw_frames_ctx/format fields thay may have been set by the caller - those will + * be preserved as they were. + * * @param dst Destination frame, to contain the mapping. * @param src Source frame, to be mapped. * @param flags Some combination of AV_HWFRAME_MAP_* flags. -- 2.33.0 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-19 14:01 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-19 13:40 [FFmpeg-devel] [PATCH 1/3] lavu/hwcontext_opencl: clear dangling pointers on map failure Anton Khirnov 2022-01-19 13:40 ` [FFmpeg-devel] [PATCH 2/3] lavu/hwcontext_vulkan: " Anton Khirnov 2022-01-19 13:40 ` [FFmpeg-devel] [PATCH 3/3] lavu/hwcontext: clarify behavior on av_hwframe_map() failure Anton Khirnov 2022-01-19 13:45 ` James Almer 2022-01-19 13:47 ` Anton Khirnov 2022-01-19 14:01 ` [FFmpeg-devel] [PATCH] " Anton Khirnov
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git