[FFmpeg-devel] [PATCH] avcodec/avformat: Store SDP attributes from RTSP stream into AVStream side data.
Connecting to an RTSP stream will now cause the SDP attributes of each media stream to be stored in the codecpar of the relative AVStream. The SDP attributes are stored in the coded_side_data using the (new) type AV_PKT_DATA_SDP_ATTRIBUTES (AVPacketSideDataType enum). Signed-off-by: bpilarz --- libavcodec/packet.h | 7 libavformat/rtsp.c | 96 + 2 files changed, 103 insertions(+) diff --git a/libavcodec/packet.h b/libavcodec/packet.h index 13667ffa36..dabe970dbb 100644 --- a/libavcodec/packet.h +++ b/libavcodec/packet.h @@ -339,6 +339,13 @@ enum AVPacketSideDataType { */ AV_PKT_DATA_FRAME_CROPPING, +/** + * Attributes found in the SDP, associated with the stream. This is a list + * of zero terminated key/value strings. There is no end marker for the + * list, so it is required to rely on the side data size to stop. + */ +AV_PKT_DATA_SDP_ATTRIBUTES, + /** * The number of side data types. * This is not part of the public API/ABI in the sense that it may diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 19b93df839..efe01141c3 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -283,6 +283,90 @@ static int init_satip_stream(AVFormatContext *s) } #endif +static int sdp_add_attribute_to_stream_side_data(AVFormatContext *s, AVStream *st, + const char *attribute_line) +{ +AVPacketSideData *side_data = NULL; +AVDictionary* attributes_dict = NULL; +char *key = NULL, *value = NULL; +int ret = 0; + +/* Get the stream's attributes dictionary. + * If the stream has no attributes dictionary, it will be automatically + * created by av_dict_set. */ +side_data = av_packet_side_data_get(st->codecpar->coded_side_data, +st->codecpar->nb_coded_side_data, +AV_PKT_DATA_SDP_ATTRIBUTES); +if (side_data) { +ret = av_packet_unpack_dictionary(side_data->data, side_data->size, + &attributes_dict); +if (ret) { +av_log(s, AV_LOG_WARNING, + "Unable to unpack SDP attributes dictionary.\n"); +return -1; +} +} + +/* The attribute can either be a value attribute (key:value) or property + * attribute (just the key). + * Look for the ':' separator, and create the 'key' and 'value' + * appropriately. */ +const char *separator = strchr(attribute_line, ':'); +if (separator) { +/* Make a copy of the key and value. */ +key = av_strndup(attribute_line, separator - attribute_line); +value = av_strdup(separator + 1); +} else { +/* Copy the key and create an empty value. */ +key = av_strdup(attribute_line); +value = av_mallocz(1); +} +if (!key || !value) { +av_dict_free(&attributes_dict); +av_free(value); +av_free(key); +return -1; +} + +/* Add the attribute, then pack the dictionary again. */ +ret = av_dict_set(&attributes_dict, key, value, + AV_DICT_DONT_STRDUP_KEY | + AV_DICT_DONT_STRDUP_VAL | + AV_DICT_MULTIKEY); +if (ret) { +av_log(s, AV_LOG_WARNING, + "Unable to add SDL attribute to dictionary.\n"); +av_dict_free(&attributes_dict); +return -1; +} +size_t packed_dict_size = 0u; +uint8_t *packed_dict = av_packet_pack_dictionary(attributes_dict, + &packed_dict_size); + +/* Free the dictionary, which is not needed any longer. */ +av_dict_free(&attributes_dict); + +/* Make sure the dictionary was packet successfully, then add it back to + * the stream's side data. */ +if (!packed_dict) { +av_log(s, AV_LOG_WARNING, + "Unable to pack SDP attributes dictionary.\n"); +return -1; +} +side_data = av_packet_side_data_add(&st->codecpar->coded_side_data, +&st->codecpar->nb_coded_side_data, +AV_PKT_DATA_SDP_ATTRIBUTES, +packed_dict, packed_dict_size, +0); +if (!side_data) { +av_log(s, AV_LOG_WARNING, + "Unable to add SDP attributes side data to stream.\n"); +return -1; +} + +return 0; +} + /* parse the rtpmap description: /[/params>] */ static int sdp_parse_rtpmap(AVFormatContext *s, AVStream *st, RTSPStream *rtsp_st, @@ -570,6 +654,18 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1, sizeof(rtsp_st->control_url)); break; case 'a': +/* add all SDP attributes to the stream's side data */ +if (rt->nb_rtsp_stream
Re: [FFmpeg-devel] videotoolbox increases min target to macOS 12
On 9 Jul 2024, at 8:16, Gnattu OC via ffmpeg-devel wrote: >> On Jul 9, 2024, at 13:47, Helmut K. C. Tessarek wrote: >> >> I'm very sorry to bother the dev list with this, but this is the 2nd time in >> less than a year that the min version of macOS was changed in videotoolbox. >> >> 2023-09-22: macOS 10.13 >> now: macOS 12 >> >> Will ffmpeg soon only compile on the current release of macOS (with >> videtoolbox support)? While I understand that too old OS versions are not >> necessarily the best idea, a lot of people still use older versions. >> Especially when it comes to Intel based static binaries. >> >> I just want to ask what the devs think of this situation. >> I'm not saying that this has to be fixed. (I just removed the videotoolbox >> support from my binaries.) >> >> I'd asked in a forum, but there is none, nor is there another way to ask the >> devs a question. And it is a question to the devs: >> >> Is it feasible that this code raises the minimum depolyment target that >> often, while the rest of ffmpeg just works perfectly fine with lower >> deployment targets? > > Well the code introduced is to fix certain HDR related bugs and it does not > "works perfectly” before, it was just broken. >> >> Cheers, >> K. C. >> >> Here is the error message when compiling ffmpeg for reference: >> >> libavutil/hwcontext_videotoolbox.c:592:39: error: 'CVBufferCopyAttachments' >> is only available on macOS 12.0 or newer >> [-Werror,-Wunguarded-availability-new] >>CFDictionaryRef attachments = CVBufferCopyAttachments(pixbuf, >> kCVAttachmentMode_ShouldPropagate); > > This should be fixed by this patch: > https://patchwork.ffmpeg.org/project/ffmpeg/patch/tencent_386ebee5491311084f93f9136a75c4090...@qq.com/ > > so that it should at least compile on older OS targets. > But the end result is that the colorspace is as wrong as before, an older > version cannot receive the fix to set the correct colorspace. > > For this use case I could see that using the (now deprecated) > `CVBufferGetAttachments` instead on older OS, but I cannot really test that > because I don’t have device running that old version. Yes, I can submit a fix for this soon that makes this work on macOS 10.8+ For now the other patch should be merged to fix the runtime issue. (I am not sure why I did not get any warning about this locally…) >> ^~~ >> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreVideo.framework/Headers/CVBuffer.h:160:59: >> note: 'CVBufferCopyAttachments' has been marked as being introduced in >> macOS 12.0 here, but the deployment target is macOS 10.13.0 >> CV_EXPORT CFDictionaryRef CF_RETURNS_RETAINED CV_NULLABLE >> CVBufferCopyAttachments( CVBufferRef CV_NONNULL buffer, CVAttachmentMode >> attachmentMode ) API_AVAILABLE(macos(12.0), ios(15.0), tvos(15.0), >> watchos(8.0)); >> ^ >> libavutil/hwcontext_videotoolbox.c:592:39: note: enclose >> 'CVBufferCopyAttachments' in a __builtin_available check to silence this >> warning >>CFDictionaryRef attachments = CVBufferCopyAttachments(pixbuf, >> kCVAttachmentMode_ShouldPropagate); >> ^~~ >> 1 error generated. >> >> >> -- >> regards Helmut K. C. Tessarek KeyID 0x172380A011EF4944 >> Key fingerprint = 8A55 70C1 BD85 D34E ADBC 386C 1723 80A0 11EF 4944 >> >> /* >> Thou shalt not follow the NULL pointer for chaos and madness >> await thee at its end. >> */ >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avutil/hwcontext_videotoolbox: Fix version check
On 8 Jul 2024, at 8:30, Zhao Zhili wrote: > From: Zhao Zhili > > --- > libavutil/hwcontext_videotoolbox.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavutil/hwcontext_videotoolbox.c > b/libavutil/hwcontext_videotoolbox.c > index 953155ce32..dd89edfa08 100644 > --- a/libavutil/hwcontext_videotoolbox.c > +++ b/libavutil/hwcontext_videotoolbox.c > @@ -588,7 +588,7 @@ static int vt_pixbuf_set_colorspace(void *log_ctx, > } else > CVBufferRemoveAttachment(pixbuf, kCVImageBufferGammaLevelKey); > > -if (__builtin_available(macOS 10.8, iOS 10, *)) { > +if (__builtin_available(macOS 12.0, iOS 15.0, *)) { > CFDictionaryRef attachments = CVBufferCopyAttachments(pixbuf, > kCVAttachmentMode_ShouldPropagate); > if (attachments) { > colorspace = > CVImageBufferCreateColorSpaceFromAttachments(attachments); > -- > 2.42.0 LGTM. Sorry for breaking this. > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avcodec/avformat: Store SDP attributes from RTSP stream into AVStream side data.
Connecting to an RTSP stream will now cause the SDP attributes of each media stream to be stored in the codecpar of the relative AVStream. The SDP attributes are stored in the coded_side_data using the (new) type AV_PKT_DATA_SDP_ATTRIBUTES (AVPacketSideDataType enum). Signed-off-by: Bernardo Pilarz --- libavcodec/packet.h | 7 libavformat/rtsp.c | 97 + 2 files changed, 104 insertions(+) mode change 100644 => 100755 libavcodec/packet.h mode change 100644 => 100755 libavformat/rtsp.c diff --git a/libavcodec/packet.h b/libavcodec/packet.h old mode 100644 new mode 100755 index 13667ffa36..8cbb1c2a75 --- a/libavcodec/packet.h +++ b/libavcodec/packet.h @@ -339,6 +339,13 @@ enum AVPacketSideDataType { */ AV_PKT_DATA_FRAME_CROPPING, +/** + * Attributes found in the SDP, associated with the stream. This is a + * list of zero terminated key/value strings. There is no end marker for + * the list, so it is required to rely on the side data size to stop. + */ +AV_PKT_DATA_SDP_ATTRIBUTES, + /** * The number of side data types. * This is not part of the public API/ABI in the sense that it may diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c old mode 100644 new mode 100755 index 19b93df839..10594e501a --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -283,6 +283,91 @@ static int init_satip_stream(AVFormatContext *s) } #endif +static int sdp_add_attribute_to_stream_side_data(AVFormatContext *s, + AVStream *st, + const char *p) +{ +AVPacketSideData *side_data = NULL; +AVDictionary* attributes_dict = NULL; +char *key = NULL, *value = NULL; +int ret = 0; + +/* Get the stream's attributes dictionary. + * If the stream has no attributes dictionary, it will be automatically + * created by av_dict_set. */ +side_data = av_packet_side_data_get(st->codecpar->coded_side_data, +st->codecpar->nb_coded_side_data, +AV_PKT_DATA_SDP_ATTRIBUTES); +if (side_data) { +ret = av_packet_unpack_dictionary(side_data->data, side_data->size, + &attributes_dict); +if (ret) { +av_log(s, AV_LOG_WARNING, + "Unable to unpack SDP attributes dictionary.\n"); +return -1; +} +} + +/* The attribute can either be a value attribute (key:value) or + * property attribute (just the key). + * Look for the ':' separator, and create the 'key' and 'value' + * appropriately. */ +const char *separator = strchr(p, ':'); +if (separator) { +/* Make a copy of the key and value. */ +key = av_strndup(p, separator - p); +value = av_strdup(separator + 1); +} else { +/* Copy the key and create an empty value. */ +key = av_strdup(p); +value = av_mallocz(1); +} +if (!key || !value) { +av_dict_free(&attributes_dict); +av_free(value); +av_free(key); +return -1; +} + +/* Add the attribute, then pack the dictionary again. */ +ret = av_dict_set(&attributes_dict, key, value, + AV_DICT_DONT_STRDUP_KEY | + AV_DICT_DONT_STRDUP_VAL | + AV_DICT_MULTIKEY); +if (ret) { +av_log(s, AV_LOG_WARNING, + "Unable to add SDL attribute to dictionary.\n"); +av_dict_free(&attributes_dict); +return -1; +} +size_t packed_dict_size = 0u; +uint8_t *packed_dict = av_packet_pack_dictionary(attributes_dict, + &packed_dict_size); + +/* Free the dictionary, which is not needed any longer. */ +av_dict_free(&attributes_dict); + +/* Make sure the dictionary was packet successfully, then add it + * back to the stream's side data. */ +if (!packed_dict) { +av_log(s, AV_LOG_WARNING, + "Unable to pack SDP attributes dictionary.\n"); +return -1; +} +side_data = av_packet_side_data_add(&st->codecpar->coded_side_data, +&st->codecpar->nb_coded_side_data, +AV_PKT_DATA_SDP_ATTRIBUTES, +packed_dict, packed_dict_size, +0); +if (!side_data) { +av_log(s, AV_LOG_WARNING, + "Unable to add SDP attributes side data to stream.\n"); +return -1; +} + +return 0; +} + /* parse the rtpmap description: /[/] */ static int sdp_parse_rtpmap(AVFormatContext *s, AVStream *st, RTSPStream *rtsp_st, @@ -570,6 +655,18 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1, sizeof(rtsp_st->control_url))
[FFmpeg-devel] [PATCH 1/2] avfilter/vf_scale: fix frame lifetimes
From: Niklas Haas scale_frame() inconsistently handled the lifetime of `in`. Fixes a possible double free and a possible memory leak. The new code always has `scale_frame` take over ownership of the input frame. I first tried writing this code in a way where the calling code retains ownership, but this is nontrivial due to the presence of the no-op short-circuit condition in which the input frame is directly returned. (As an alternative, we could use av_frame_clone() instead, but I wanted to avoid touching the original behavior in this commit) --- libavfilter/vf_scale.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c index 841075193e..a1a322ed9e 100644 --- a/libavfilter/vf_scale.c +++ b/libavfilter/vf_scale.c @@ -869,17 +869,20 @@ static int scale_field(ScaleContext *scale, AVFrame *dst, AVFrame *src, return 0; } -static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out) +/* Takes over ownership of *frame_in, passes ownership of *frame_out to caller */ +static int scale_frame(AVFilterLink *link, AVFrame **frame_in, + AVFrame **frame_out) { AVFilterContext *ctx = link->dst; ScaleContext *scale = ctx->priv; AVFilterLink *outlink = ctx->outputs[0]; -AVFrame *out; +AVFrame *out, *in = *frame_in; const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format); char buf[32]; int ret; int frame_changed; +*frame_in = NULL; *frame_out = NULL; if (in->colorspace == AVCOL_SPC_YCGCO) av_log(link->dst, AV_LOG_WARNING, "Detected unsupported YCgCo colorspace.\n"); @@ -922,11 +925,11 @@ static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out) ret = scale_parse_expr(ctx, NULL, &scale->w_pexpr, "width", scale->w_expr); if (ret < 0) -return ret; +goto err; ret = scale_parse_expr(ctx, NULL, &scale->h_pexpr, "height", scale->h_expr); if (ret < 0) -return ret; +goto err; } if (ctx->filter == &ff_vf_scale2ref) { @@ -957,7 +960,7 @@ FF_ENABLE_DEPRECATION_WARNINGS link->dst->inputs[0]->sample_aspect_ratio.num = in->sample_aspect_ratio.num; if ((ret = config_props(outlink)) < 0) -return ret; +goto err; } scale: @@ -971,10 +974,9 @@ scale: out = ff_get_video_buffer(outlink, outlink->w, outlink->h); if (!out) { -av_frame_free(&in); -return AVERROR(ENOMEM); +ret = AVERROR(ENOMEM); +goto err; } -*frame_out = out; av_frame_copy_props(out, in); out->width = outlink->w; @@ -999,9 +1001,12 @@ scale: ret = sws_scale_frame(scale->sws, out, in); } -av_frame_free(&in); if (ret < 0) -av_frame_free(frame_out); +av_frame_free(&out); +*frame_out = out; + +err: +av_frame_free(&in); return ret; } @@ -1058,7 +1063,7 @@ FF_ENABLE_DEPRECATION_WARNINGS } } -ret = scale_frame(ctx->inputs[0], in, &out); +ret = scale_frame(ctx->inputs[0], &in, &out); if (out) { out->pts = av_rescale_q(fs->pts, fs->time_base, outlink->time_base); return ff_filter_frame(outlink, out); @@ -1077,7 +1082,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) AVFrame *out; int ret; -ret = scale_frame(link, in, &out); +ret = scale_frame(link, &in, &out); if (out) return ff_filter_frame(outlink, out); -- 2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/2] avfilter/vf_scale: test return code of scale_frame()
From: Niklas Haas Instead of testing the returned frame against NULL, test the return code itself, going more in line with the usual behavior of such functions. --- libavfilter/vf_scale.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c index a1a322ed9e..ae7356fd7b 100644 --- a/libavfilter/vf_scale.c +++ b/libavfilter/vf_scale.c @@ -883,7 +883,6 @@ static int scale_frame(AVFilterLink *link, AVFrame **frame_in, int frame_changed; *frame_in = NULL; -*frame_out = NULL; if (in->colorspace == AVCOL_SPC_YCGCO) av_log(link->dst, AV_LOG_WARNING, "Detected unsupported YCgCo colorspace.\n"); @@ -1064,14 +1063,15 @@ FF_ENABLE_DEPRECATION_WARNINGS } ret = scale_frame(ctx->inputs[0], &in, &out); -if (out) { -out->pts = av_rescale_q(fs->pts, fs->time_base, outlink->time_base); -return ff_filter_frame(outlink, out); -} +if (ret < 0) +goto err; + +av_assert0(out); +out->pts = av_rescale_q(fs->pts, fs->time_base, outlink->time_base); +return ff_filter_frame(outlink, out); err: -if (ret < 0) -av_frame_free(&in); +av_frame_free(&in); return ret; } -- 2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avcodec/avformat: Store SDP attributes from RTSP stream into AVStream side data.
Connecting to an RTSP stream will now cause the SDP attributes of each media stream to be stored in the codecpar of the relative AVStream. The SDP attributes are stored in the coded_side_data using the (new) type AV_PKT_DATA_SDP_ATTRIBUTES (AVPacketSideDataType enum). Signed-off-by: bpilarz --- libavcodec/packet.h | 7 libavformat/rtsp.c | 97 + 2 files changed, 104 insertions(+) diff --git a/libavcodec/packet.h b/libavcodec/packet.h index 13667ffa36..3091c3ce56 --- a/libavcodec/packet.h +++ b/libavcodec/packet.h @@ -339,6 +339,13 @@ enum AVPacketSideDataType { */ AV_PKT_DATA_FRAME_CROPPING, +/** + * Attributes found in the SDP, associated with the stream. This is a + * list of zero terminated key/value strings. There is no end marker for + * the list, so it is required to rely on the side data size to stop. + */ +AV_PKT_DATA_SDP_ATTRIBUTES, + /** * The number of side data types. * This is not part of the public API/ABI in the sense that it may diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 19b93df839..10594e501a --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -283,6 +283,91 @@ static int init_satip_stream(AVFormatContext *s) } #endif +static int sdp_add_attribute_to_stream_side_data(AVFormatContext *s, + AVStream *st, + const char *p) +{ +AVPacketSideData *side_data = NULL; +AVDictionary* attributes_dict = NULL; +char *key = NULL, *value = NULL; +int ret = 0; + +/* Get the stream's attributes dictionary. + * If the stream has no attributes dictionary, it will be automatically + * created by av_dict_set. */ +side_data = av_packet_side_data_get(st->codecpar->coded_side_data, +st->codecpar->nb_coded_side_data, +AV_PKT_DATA_SDP_ATTRIBUTES); +if (side_data) { +ret = av_packet_unpack_dictionary(side_data->data, side_data->size, + &attributes_dict); +if (ret) { +av_log(s, AV_LOG_WARNING, + "Unable to unpack SDP attributes dictionary.\n"); +return -1; +} +} + +/* The attribute can either be a value attribute (key:value) or + * property attribute (just the key). + * Look for the ':' separator, and create the 'key' and 'value' + * appropriately. */ +const char *separator = strchr(p, ':'); +if (separator) { +/* Make a copy of the key and value. */ +key = av_strndup(p, separator - p); +value = av_strdup(separator + 1); +} else { +/* Copy the key and create an empty value. */ +key = av_strdup(p); +value = av_mallocz(1); +} +if (!key || !value) { +av_dict_free(&attributes_dict); +av_free(value); +av_free(key); +return -1; +} + +/* Add the attribute, then pack the dictionary again. */ +ret = av_dict_set(&attributes_dict, key, value, + AV_DICT_DONT_STRDUP_KEY | + AV_DICT_DONT_STRDUP_VAL | + AV_DICT_MULTIKEY); +if (ret) { +av_log(s, AV_LOG_WARNING, + "Unable to add SDL attribute to dictionary.\n"); +av_dict_free(&attributes_dict); +return -1; +} +size_t packed_dict_size = 0u; +uint8_t *packed_dict = av_packet_pack_dictionary(attributes_dict, + &packed_dict_size); + +/* Free the dictionary, which is not needed any longer. */ +av_dict_free(&attributes_dict); + +/* Make sure the dictionary was packet successfully, then add it + * back to the stream's side data. */ +if (!packed_dict) { +av_log(s, AV_LOG_WARNING, + "Unable to pack SDP attributes dictionary.\n"); +return -1; +} +side_data = av_packet_side_data_add(&st->codecpar->coded_side_data, +&st->codecpar->nb_coded_side_data, +AV_PKT_DATA_SDP_ATTRIBUTES, +packed_dict, packed_dict_size, +0); +if (!side_data) { +av_log(s, AV_LOG_WARNING, + "Unable to add SDP attributes side data to stream.\n"); +return -1; +} + +return 0; +} + /* parse the rtpmap description: /[/] */ static int sdp_parse_rtpmap(AVFormatContext *s, AVStream *st, RTSPStream *rtsp_st, @@ -570,6 +655,18 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1, sizeof(rtsp_st->control_url)); break; case 'a': +/* add all SDP attributes to the stream's side data */ +if (rt->nb_rtsp_streams > 0) { +rtsp_st = rt->rtsp_st
Re: [FFmpeg-devel] [PATCH 2/2] avutil/executor: Fix stack overflow due to recursive call
On Tue, Jul 9, 2024 at 6:35 AM Timo Rothenpieler wrote: > On 08.07.2024 17:32, Zhao Zhili wrote: > > > > > >> On Jul 8, 2024, at 22:07, Timo Rothenpieler > wrote: > >> > >> On 08.07.2024 09:43, Zhao Zhili wrote: > >>> From: Zhao Zhili > >>> av_executor_execute run the task directly when thread is disabled. > >>> The task can schedule a new task by call av_executor_execute. This > >>> forms an implicit recursive call. This patch removed the recursive > >>> call. > >>> --- > >>> libavutil/executor.c | 5 + > >>> 1 file changed, 5 insertions(+) > >>> diff --git a/libavutil/executor.c b/libavutil/executor.c > >>> index 89058fab2f..c145c51be9 100644 > >>> --- a/libavutil/executor.c > >>> +++ b/libavutil/executor.c > >>> @@ -58,6 +58,7 @@ struct AVExecutor { > >>> int die; > >>> AVTask *tasks; > >>> +int stack_depth; > >>> }; > >>> static AVTask* remove_task(AVTask **prev, AVTask *t) > >>> @@ -207,8 +208,12 @@ void av_executor_execute(AVExecutor *e, AVTask *t) > >>> } > >>> if (!e->thread_count || !HAVE_THREADS) { > >>> +if (e->stack_depth > 0) > >>> +return; > >>> +e->stack_depth++; > >>> // We are running in a single-threaded environment, so we > must handle all tasks ourselves > >>> while (run_one_task(e, e->local_contexts)) > >>> /* nothing */; > >>> +e->stack_depth--; > >> > >> Won't this put the above line into the "nothing" while-loop? > >> Is that intended? If so, the indentation should be adjusted accordingly. > >> If not, the while-loop should gain empty {}. > > > > The comment specify it’s a while loop with empty body. Maybe it’s not > obvious > > in the email client. > > Oh, there is a ; behind the comment. > Completely missed that. Can't say I like it, but yeah, that works. > Google suggests using "while (cond) continue;": https://google.github.io/styleguide/cppguide.html#Formatting_Looping_Branching . However, it's not better than using /* nothing */. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avutil/imgutils: av_image_check_size2() ensure width and height fit in 32bit
on "INT is 64bit" systems they may have been false Signed-off-by: Michael Niedermayer --- libavutil/imgutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c index d2463815637..b738cff37c2 100644 --- a/libavutil/imgutils.c +++ b/libavutil/imgutils.c @@ -298,7 +298,7 @@ int av_image_check_size2(unsigned int w, unsigned int h, int64_t max_pixels, enu stride = 8LL*w; stride += 128*8; -if ((int)w<=0 || (int)h<=0 || stride >= INT_MAX || stride*(uint64_t)(h+128) >= INT_MAX) { +if (w==0 || h==0 || w > INT32_MAX || h > INT32_MAX || stride >= INT_MAX || stride*(uint64_t)(h+128) >= INT_MAX) { av_log(&imgutils, AV_LOG_ERROR, "Picture size %ux%u is invalid\n", w, h); return AVERROR(EINVAL); } -- 2.45.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] avfilter/vf_scale: Cleanup some checks
Fixes: CID1513722 Operands don't affect result Sponsored-by: Sovereign Tech Fund Signed-off-by: Michael Niedermayer --- libavfilter/vf_scale.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c index bf09196e10d..18e9393d6c1 100644 --- a/libavfilter/vf_scale.c +++ b/libavfilter/vf_scale.c @@ -645,10 +645,8 @@ static int config_props(AVFilterLink *outlink) if (ret < 0) goto fail; -if (outlink->w > INT_MAX || -outlink->h > INT_MAX || -(outlink->h * inlink->w) > INT_MAX || -(outlink->w * inlink->h) > INT_MAX) +if ((outlink->h * (int64_t)inlink->w) > INT32_MAX || +(outlink->w * (int64_t)inlink->h) > INT32_MAX) av_log(ctx, AV_LOG_ERROR, "Rescaled value for width or height is too big.\n"); /* TODO: make algorithm configurable */ -- 2.45.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] doc/examples/vaapi_encode: Try to check fwrite() for failure
On Tue, Jul 09, 2024 at 06:14:17AM +, Xiang, Haihao wrote: > On Di, 2024-07-02 at 01:47 +0200, Michael Niedermayer wrote: > > Fixes: CID1604548 Unused value > > > > Sponsored-by: Sovereign Tech Fund > > Signed-off-by: Michael Niedermayer > > --- > > doc/examples/vaapi_encode.c | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/doc/examples/vaapi_encode.c b/doc/examples/vaapi_encode.c > > index d5f472f6dd8..ff3ebb1e2b8 100644 > > --- a/doc/examples/vaapi_encode.c > > +++ b/doc/examples/vaapi_encode.c > > @@ -88,6 +88,10 @@ static int encode_write(AVCodecContext *avctx, AVFrame > > *frame, FILE *fout) > > enc_pkt->stream_index = 0; > > ret = fwrite(enc_pkt->data, enc_pkt->size, 1, fout); > > av_packet_unref(enc_pkt); > > + if (ret != enc_pkt->size) { > > + ret = AVERROR(errno); > > + break; > > + } > > } > > > > end: > > LGTM will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No great genius has ever existed without some touch of madness. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavfi/perlin: Fix out of bounds stack buffer write
On 6 Jul 2024, at 11:26, Stefano Sabatini wrote: > On date Tuesday 2024-07-02 20:38:00 +0200, Marvin Scholz wrote: >> An incorrect calculation in ff_perlin_init causes a write to the >> stack array at index 256, which is out of bounds. >> >> Fixes: CID1608711 >> --- >> libavfilter/perlin.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavfilter/perlin.c b/libavfilter/perlin.c >> index 09bae7ad33..ffad8c1e4e 100644 >> --- a/libavfilter/perlin.c >> +++ b/libavfilter/perlin.c >> @@ -129,7 +129,7 @@ int ff_perlin_init(FFPerlin *perlin, double period, int >> octaves, double persiste >> for (i = 0; i < 256; i++) { >> unsigned int random_idx = av_lfg_get(&lfg) % (256-i); >> uint8_t random_val = random_permutations[random_idx]; >> -random_permutations[random_idx] = random_permutations[256-i]; >> +random_permutations[random_idx] = random_permutations[255-i]; >> >> perlin->permutations[i] = perlin->permutations[i+256] = >> random_val; >> } > > Looks good, thanks. Please push then, I do not have commit access. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] MAINTAINERS: add myself as d3d12va_encode maintainer
On Mon, Jul 08, 2024 at 03:16:28PM +, Tong Wu wrote: > Ping. As the author of d3d12va_encode, I would like to get the access in > order maintain the code and add more new features. will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The day soldiers stop bringing you their problems is the day you have stopped leading them. They have either lost confidence that you can help or concluded you do not care. Either case is a failure of leadership. - Colin Powell signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCHv2] swscale: prevent undefined behaviour in the PUTRGBA macro
On Mon, Jul 08, 2024 at 12:25:17AM -0400, Sean McGovern wrote: > --- > libswscale/yuv2rgb.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libswscale/yuv2rgb.c b/libswscale/yuv2rgb.c > index 977eb3a7dd..ac0b811f61 100644 > --- a/libswscale/yuv2rgb.c > +++ b/libswscale/yuv2rgb.c > @@ -100,9 +100,9 @@ const int *sws_getCoefficients(int colorspace) > > #define PUTRGBA(dst, ysrc, asrc, i, abase) \ > Y = ysrc[2 * i]; \ > -dst[2 * i] = r[Y] + g[Y] + b[Y] + (asrc[2 * i] << abase); \ > +dst[2 * i] = r[Y] + g[Y] + b[Y] + ((int32_t)(asrc[2 * i]) << > abase); \ > Y = ysrc[2 * i + 1]; \ > -dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + (asrc[2 * i + 1] << abase); > +dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + ((int32_t)(asrc[2 * i + 1]) << > abase); can you explain what undefined behavior this does prevent and how ? (in the commit message) thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The greatest way to live with honor in this world is to be what we pretend to be. -- Socrates signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/6] avcodec/vvc/refs: Use unsigned mask
On Mon, Jul 08, 2024 at 09:49:25PM +0800, Nuo Mi wrote: > LGTM. > Thank you, Michael, will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Homeopathy is like voting while filling the ballot out with transparent ink. Sometimes the outcome one wanted occurs. Rarely its worse than filling out a ballot properly. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 8/9] avformat/udp: Fix temporary buffer race
On Mon, Jul 08, 2024 at 07:46:19PM +0200, Marton Balint wrote: > > > On Sun, 9 Jun 2024, Michael Niedermayer wrote: > > > Fixes: CID1551679 Data race condition > > Fixes: CID1551687 Data race condition > > How is this a data race? Concurrent reading and writing is not supported for > UDP as far as I know. maybe coverity tricked me together with memory of a long standing unreproduceable data corruption bug in udp ... thinking there where 2 threads using the same temporary buffer (which would really fit the bug i remembered) feel free to revert! thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Elect your leaders based on what they did after the last election, not based on what they say before an election. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] avfilter/vf_scale: Cleanup some checks
Quoting Michael Niedermayer (2024-07-09 13:37:11) > Fixes: CID1513722 Operands don't affect result > > Sponsored-by: Sovereign Tech Fund > Signed-off-by: Michael Niedermayer > --- > libavfilter/vf_scale.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c > index bf09196e10d..18e9393d6c1 100644 > --- a/libavfilter/vf_scale.c > +++ b/libavfilter/vf_scale.c > @@ -645,10 +645,8 @@ static int config_props(AVFilterLink *outlink) > if (ret < 0) > goto fail; > > -if (outlink->w > INT_MAX || > -outlink->h > INT_MAX || > -(outlink->h * inlink->w) > INT_MAX || > -(outlink->w * inlink->h) > INT_MAX) > +if ((outlink->h * (int64_t)inlink->w) > INT32_MAX || > +(outlink->w * (int64_t)inlink->h) > INT32_MAX) This does not seems cleaner to me. Also, this check overall seems fishy. Why is it here at all and not e.g. in ff_scale_adjust_dimensions()? Why does it not call av_image_check_size()? Why does it only print a warning and not do anything else? -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avutil/imgutils: av_image_check_size2() ensure width and height fit in 32bit
> ensure width and height fit in 32bit why? -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 8/9] avformat/udp: Fix temporary buffer race
On Tue, Jul 09, 2024 at 03:12:52PM +0200, Michael Niedermayer wrote: > On Mon, Jul 08, 2024 at 07:46:19PM +0200, Marton Balint wrote: > > > > > > On Sun, 9 Jun 2024, Michael Niedermayer wrote: > > > > > Fixes: CID1551679 Data race condition > > > Fixes: CID1551687 Data race condition marked the 2 as false positives > > > > How is this a data race? Concurrent reading and writing is not supported for > > UDP as far as I know. > > maybe coverity tricked me together with memory of a long standing > unreproduceable data corruption bug in udp ... thinking there where > 2 threads using the same temporary buffer (which would really fit > the bug i remembered) > > feel free to revert! [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I am the wisest man alive, for I know one thing, and that is that I know nothing. -- Socrates signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avutil/imgutils: av_image_check_size2() ensure width and height fit in 32bit
On Tue, Jul 09, 2024 at 03:17:58PM +0200, Anton Khirnov wrote: > > ensure width and height fit in 32bit > > why? because not everyone wants undefined behavior because not everyone wants security issues because we dont support width and height > 32bit and its easier to check in a central place because the changed codes purpose is to check if the image paramaters are within what we support, and width of 100 billion is not. You can try all encoders with 100billion width. Then try to decode. Iam curious, how many work, how many fail and how they fail how many invalid bitstreams with no warning, how many undefined behaviors, ... Simply building FFmpeg on a platform with 64bit ints doesnt update ISO and ITU standards to allow larger values thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avutil/imgutils: av_image_check_size2() ensure width and height fit in 32bit
On Tue, Jul 09, 2024 at 03:28:10PM +0200, Michael Niedermayer wrote: > On Tue, Jul 09, 2024 at 03:17:58PM +0200, Anton Khirnov wrote: > > > ensure width and height fit in 32bit > > > > why? > > because not everyone wants undefined behavior > because not everyone wants security issues > because we dont support width and height > 32bit and its easier to check in a > central place > because the changed codes purpose is to check if the image paramaters are > within what we support, and width of 100 billion is not. You can try > all encoders with 100billion width. Then try to decode. > Iam curious, how many work, how many fail and how they fail > how many invalid bitstreams with no warning, how many undefined > behaviors, ... > > Simply building FFmpeg on a platform with 64bit ints doesnt update > ISO and ITU standards to allow larger values but theres more :) if we allow 64bit width and height, every check on the pixel number just broke because w*(uint64_t)h just doesnt work anymore. a 64bit int isnt giving us a int128_t. So many checks related to width and height suddenly become more fragile as theres not a simply larger type thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When you are offended at any man's fault, turn to yourself and study your own failings. Then you will forget your anger. -- Epictetus signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/6] avutil/hwcontext_d3d11va: correct sizeof AVD3D11FrameDescriptor
On Mon, Jul 08, 2024 at 09:51:30AM +0200, Steve Lhomme wrote: > Hi Michael, > > All the patches in that patches look good to me. You can apply. will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Opposition brings concord. Out of discord comes the fairest harmony. -- Heraclitus signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] swscale/utils: fix leak on threaded ctx init failure
From: Niklas Haas This count gets incremented after init succeeds, when it should be incremented after *alloc* succeeds. Otherwise, we leak the context on failure. There are no negative consequences of incrementing for allocated-but-not-initialized contexts, as the only functions that reference it will, in the worst case, simply behave as if called on allocated-but-not-initialized contexts, which is in line with expected behavior when sws_init_context() fails. --- libswscale/utils.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libswscale/utils.c b/libswscale/utils.c index 12dba712c1..6ac5b202b0 100644 --- a/libswscale/utils.c +++ b/libswscale/utils.c @@ -2046,6 +2046,7 @@ static int context_init_threaded(SwsContext *c, if (!c->slice_ctx[i]) return AVERROR(ENOMEM); +c->nb_slice_ctx++; c->slice_ctx[i]->parent = c; ret = av_opt_copy((void*)c->slice_ctx[i], (void*)c); @@ -2058,8 +2059,6 @@ static int context_init_threaded(SwsContext *c, if (ret < 0) return ret; -c->nb_slice_ctx++; - if (c->slice_ctx[i]->dither == SWS_DITHER_ED) { av_log(c, AV_LOG_VERBOSE, "Error-diffusion dither is in use, scaling will be single-threaded."); -- 2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avutil/hwcontext_videotoolbox: Fix build with older SDKs
I've accidentally used API not available on the checked version. Additionally check for the SDK to be new enough to even have the CVImageBufferCreateColorSpaceFromAttachments API to not fail the build. --- libavutil/hwcontext_videotoolbox.c | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c index 953155ce32..ab7556936d 100644 --- a/libavutil/hwcontext_videotoolbox.c +++ b/libavutil/hwcontext_videotoolbox.c @@ -588,13 +588,26 @@ static int vt_pixbuf_set_colorspace(void *log_ctx, } else CVBufferRemoveAttachment(pixbuf, kCVImageBufferGammaLevelKey); +#if (TARGET_OS_OSX && __MAC_OS_X_VERSION_MAX_ALLOWED >= 100800) || \ +(TARGET_OS_IOS && __IPHONE_OS_VERSION_MAX_ALLOWED >= 10) if (__builtin_available(macOS 10.8, iOS 10, *)) { -CFDictionaryRef attachments = CVBufferCopyAttachments(pixbuf, kCVAttachmentMode_ShouldPropagate); +CFDictionaryRef attachments = NULL; +if (__builtin_available(macOS 12.0, iOS 15.0, *)) +attachments = CVBufferCopyAttachments(pixbuf, kCVAttachmentMode_ShouldPropagate); +#if (TARGET_OS_OSX && __MAC_OS_X_VERSION_MIN_REQUIRED <= 12) || \ +(TARGET_OS_IOS && __IPHONE_OS_VERSION_MIN_REQUIRED <= 15) +else { +CFDictionaryRef tmp = CVBufferGetAttachments(pixbuf, kCVAttachmentMode_ShouldPropagate); +if (tmp) +attachments = CFDictionaryCreateCopy(NULL, tmp); +} +#endif if (attachments) { colorspace = CVImageBufferCreateColorSpaceFromAttachments(attachments); CFRelease(attachments); } } +#endif if (colorspace) { CVBufferSetAttachment(pixbuf, kCVImageBufferCGColorSpaceKey, base-commit: 9fb8d13d56f20728141fd7070d8a325720727d57 -- 2.39.3 (Apple Git-146) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] avfilter/vf_scale: Cleanup some checks
On Tue, 9 Jul 2024 at 15:17, Anton Khirnov wrote: > > Quoting Michael Niedermayer (2024-07-09 13:37:11) > > Fixes: CID1513722 Operands don't affect result > > > > Sponsored-by: Sovereign Tech Fund > > Signed-off-by: Michael Niedermayer > > --- > > libavfilter/vf_scale.c | 6 ++ > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c > > index bf09196e10d..18e9393d6c1 100644 > > --- a/libavfilter/vf_scale.c > > +++ b/libavfilter/vf_scale.c > > @@ -645,10 +645,8 @@ static int config_props(AVFilterLink *outlink) > > if (ret < 0) > > goto fail; > > > > -if (outlink->w > INT_MAX || > > -outlink->h > INT_MAX || > > -(outlink->h * inlink->w) > INT_MAX || > > -(outlink->w * inlink->h) > INT_MAX) > > +if ((outlink->h * (int64_t)inlink->w) > INT32_MAX || > > +(outlink->w * (int64_t)inlink->h) > INT32_MAX) > > This does not seems cleaner to me. > > Also, this check overall seems fishy. Why is it here at all and not e.g. > in ff_scale_adjust_dimensions()? Why does it not call > av_image_check_size()? Why does it only print a warning and not do > anything else? I agree with Anton here. The checks in vf_scale are iffy at best. For starters, this is `outlink->w > INT_MAX` dead check. As the `w` is int already. And there is already an UB in `scale_eval_dimensions()` which converts double value to int without any checks. I think something like this would make sense to reject big numbers, and ofcourse ff_scale_adjust_dimensions() should be more clever about overflows too. There is also an overflow in swscale, but that's another story. diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c index a1a322ed9e..9483db7564 100644 --- a/libavfilter/vf_scale.c +++ b/libavfilter/vf_scale.c @@ -537,7 +537,7 @@ static int scale_eval_dimensions(AVFilterContext *ctx) const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format); const AVPixFmtDescriptor *out_desc = av_pix_fmt_desc_get(outlink->format); char *expr; - int eval_w, eval_h; + double eval_w, eval_h; int ret; double res; const AVPixFmtDescriptor *main_desc; @@ -588,23 +588,22 @@ static int scale_eval_dimensions(AVFilterContext *ctx) } res = av_expr_eval(scale->w_pexpr, scale->var_values, NULL); - eval_w = scale->var_values[VAR_OUT_W] = scale->var_values[VAR_OW] = (int) res == 0 ? inlink->w : (int) res; + eval_w = scale->var_values[VAR_OUT_W] = scale->var_values[VAR_OW] = res == 0 ? inlink->w : res; res = av_expr_eval(scale->h_pexpr, scale->var_values, NULL); - if (isnan(res)) { - expr = scale->h_expr; + eval_h = scale->var_values[VAR_OUT_H] = scale->var_values[VAR_OH] = res == 0 ? inlink->h : res; + + if (isnan(eval_w) || eval_w < INT_MIN || eval_w > INT_MAX) { + expr = scale->w_expr; ret = AVERROR(EINVAL); goto fail; } - eval_h = scale->var_values[VAR_OUT_H] = scale->var_values[VAR_OH] = (int) res == 0 ? inlink->h : (int) res; - res = av_expr_eval(scale->w_pexpr, scale->var_values, NULL); - if (isnan(res)) { - expr = scale->w_expr; + if (isnan(eval_h) || eval_h < INT_MIN || eval_h > INT_MAX) { + expr = scale->h_expr; ret = AVERROR(EINVAL); goto fail; } - eval_w = scale->var_values[VAR_OUT_W] = scale->var_values[VAR_OW] = (int) res == 0 ? inlink->w : (int) res; scale->w = eval_w; scale->h = eval_h; ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avutil/imgutils: av_image_check_size2() ensure width and height fit in 32bit
Quoting Michael Niedermayer (2024-07-09 15:28:10) > On Tue, Jul 09, 2024 at 03:17:58PM +0200, Anton Khirnov wrote: > > > ensure width and height fit in 32bit > > > > why? > > because not everyone wants undefined behavior > because not everyone wants security issues > because we dont support width and height > 32bit and its easier to check in a > central place > because the changed codes purpose is to check if the image paramaters are > within what we support, and width of 100 billion is not. You can try > all encoders with 100billion width. Then try to decode. > Iam curious, how many work, how many fail and how they fail > how many invalid bitstreams with no warning, how many undefined > behaviors, ... > > Simply building FFmpeg on a platform with 64bit ints doesnt update > ISO and ITU standards to allow larger values Quoting Michael Niedermayer (2020-10-07 16:45:56): > At least in code i wrote and write i consider it a bug if it would > assume sizeof(int/unsigned) == 4 Make up your mind. -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 0/1] fate/png: add mDCv read and write test
Changes since v1: - changed transcode command Leo Izen (1): fate/png: add mDCv read and write test tests/fate/image.mak| 6 ++ tests/ref/fate/png-mdcv | 22 ++ 2 files changed, 28 insertions(+) create mode 100644 tests/ref/fate/png-mdcv -- 2.45.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 1/1] fate/png: add mDCv read and write test
This test confirms that we can write mDCv chunks and read them back via the png decoder. It uses an HEVC conformance sample with this metadata as the base source for the side data in the frames. Signed-off-by: Leo Izen Reported-by: Jan Ekström Reviewed-by: Andreas Rheinhardt --- tests/fate/image.mak| 6 ++ tests/ref/fate/png-mdcv | 22 ++ 2 files changed, 28 insertions(+) create mode 100644 tests/ref/fate/png-mdcv diff --git a/tests/fate/image.mak b/tests/fate/image.mak index 753936ec20..358ab4b765 100644 --- a/tests/fate/image.mak +++ b/tests/fate/image.mak @@ -416,6 +416,12 @@ FATE_PNG_PROBE-$(call ALLYES, LCMS2) += fate-png-icc-parse fate-png-icc-parse: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_frames \ -flags2 icc_profiles $(TARGET_SAMPLES)/png1/lena-int_rgb24.png +FATE_PNG_TRANSCODE-$(call TRANSCODE, PNG HEVC, IMAGE2 HEVC, \ +IMAGE_PNG_PIPE_DEMUXER HEVC_PARSER PNG_DECODER SCALE_FILTER) += fate-png-mdcv +fate-png-mdcv: CMD = transcode hevc $(TARGET_SAMPLES)/hevc/hdr10_plus_h265_sample.hevc image2 \ +"-pix_fmt rgb24 -vf scale -c png" "" \ +"-show_frames -show_entries frame=side_data_list -of flat" + FATE_PNG-$(call DEMDEC, IMAGE2, PNG) += $(FATE_PNG) FATE_PNG_PROBE-$(call DEMDEC, IMAGE2, PNG) += $(FATE_PNG_PROBE) FATE_IMAGE_FRAMECRC += $(FATE_PNG-yes) diff --git a/tests/ref/fate/png-mdcv b/tests/ref/fate/png-mdcv new file mode 100644 index 00..b719152557 --- /dev/null +++ b/tests/ref/fate/png-mdcv @@ -0,0 +1,22 @@ +fc68fe6c8c72343b96d2695f6913995b *tests/data/fate/png-mdcv.image2 +439248 tests/data/fate/png-mdcv.image2 +#tb 0: 1/25 +#media_type 0: video +#codec_id 0: rawvideo +#dimensions 0: 1280x720 +#sar 0: 0/1 +0, 0, 0,1, 2764800, 0x2bfc7b42 +frames.frame.0.side_data_list.side_data.0.side_data_type="Content light level metadata" +frames.frame.0.side_data_list.side_data.0.max_content=1000 +frames.frame.0.side_data_list.side_data.0.max_average=200 +frames.frame.0.side_data_list.side_data.1.side_data_type="Mastering display metadata" +frames.frame.0.side_data_list.side_data.1.red_x="13250/5" +frames.frame.0.side_data_list.side_data.1.red_y="7500/5" +frames.frame.0.side_data_list.side_data.1.green_x="34000/5" +frames.frame.0.side_data_list.side_data.1.green_y="16000/5" +frames.frame.0.side_data_list.side_data.1.blue_x="2/5" +frames.frame.0.side_data_list.side_data.1.blue_y="0/5" +frames.frame.0.side_data_list.side_data.1.white_point_x="15635/5" +frames.frame.0.side_data_list.side_data.1.white_point_y="16450/5" +frames.frame.0.side_data_list.side_data.1.min_luminance="50/1" +frames.frame.0.side_data_list.side_data.1.max_luminance="1000/1" -- 2.45.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avformat/mov: ensure pasp box derived SAR is used if present
It's meant to override any codec specific (but container level) information, but its position is not guaranteed, so apply the values after the entire trak structure has been parsed. Also, replace the ugly roundabout int -> double -> int method to set SAR from existing dimensions while at it. Signed-off-by: James Almer --- libavformat/isom.h | 2 ++ libavformat/mov.c | 16 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/libavformat/isom.h b/libavformat/isom.h index a0498f45e5..5b6125a908 100644 --- a/libavformat/isom.h +++ b/libavformat/isom.h @@ -215,6 +215,8 @@ typedef struct MOVStreamContext { int timecode_track; int width;///< tkhd width int height; ///< tkhd height +int h_spacing;///< pasp hSpacing +int v_spacing;///< pasp vSpacing int dts_shift;///< dts shift when ctts is negative uint32_t palette[256]; int has_palette; diff --git a/libavformat/mov.c b/libavformat/mov.c index ced4b2e6b3..ce95842ce5 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -1287,14 +1287,18 @@ static int mov_read_pasp(MOVContext *c, AVIOContext *pb, MOVAtom atom) const int num = avio_rb32(pb); const int den = avio_rb32(pb); AVStream *st; +MOVStreamContext *sc; if (c->fc->nb_streams < 1) return 0; st = c->fc->streams[c->fc->nb_streams-1]; +sc = st->priv_data; + +av_log(c->fc, AV_LOG_TRACE, "pasp: hSpacing %d, vSpacing %d\n", num, den); if (den != 0) { -av_reduce(&st->sample_aspect_ratio.num, &st->sample_aspect_ratio.den, - num, den, 32767); +sc->h_spacing = num; +sc->v_spacing = den; } return 0; } @@ -5031,11 +5035,15 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom) } if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) { +if (sc->h_spacing && sc->v_spacing) +av_reduce(&st->sample_aspect_ratio.num, &st->sample_aspect_ratio.den, + sc->h_spacing, sc->v_spacing, INT_MAX); if (!st->sample_aspect_ratio.num && st->codecpar->width && st->codecpar->height && sc->height && sc->width && (st->codecpar->width != sc->width || st->codecpar->height != sc->height)) { -st->sample_aspect_ratio = av_d2q(((double)st->codecpar->height * sc->width) / - ((double)st->codecpar->width * sc->height), INT_MAX); +av_reduce(&st->sample_aspect_ratio.num, &st->sample_aspect_ratio.den, + (int64_t)st->codecpar->height * sc->width, + (int64_t)st->codecpar->width * sc->height, INT_MAX); } #if FF_API_R_FRAME_RATE -- 2.45.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCHv3] swscale: prevent undefined behaviour in the PUTRGBA macro
For even small values of 'asrc', shifting them by 24 bits or more will cause arithmetic overflow and be caught by GCC's undefined behaviour sanitizer. Ensure the values do not overflow by up-casting the bracketed expressions involving 'asrc' to int32_t. --- libswscale/yuv2rgb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libswscale/yuv2rgb.c b/libswscale/yuv2rgb.c index 977eb3a7dd..ac0b811f61 100644 --- a/libswscale/yuv2rgb.c +++ b/libswscale/yuv2rgb.c @@ -100,9 +100,9 @@ const int *sws_getCoefficients(int colorspace) #define PUTRGBA(dst, ysrc, asrc, i, abase) \ Y = ysrc[2 * i]; \ -dst[2 * i] = r[Y] + g[Y] + b[Y] + (asrc[2 * i] << abase); \ +dst[2 * i] = r[Y] + g[Y] + b[Y] + ((int32_t)(asrc[2 * i]) << abase); \ Y = ysrc[2 * i + 1]; \ -dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + (asrc[2 * i + 1] << abase); +dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + ((int32_t)(asrc[2 * i + 1]) << abase); #define PUTRGB48(dst, src, asrc, i, abase) \ Y= src[ 2 * i]; \ -- 2.39.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Reset password on Patchwork
Hi Andriy, Any luck with this? I'm still locked out. On Wed, Jun 12, 2024 at 6:59 PM Michael Niedermayer wrote: > > Hi > > adding Andriy to CC, to make sure its not missed > > On Tue, Jun 11, 2024 at 07:45:21PM -0400, Sean McGovern wrote: > > Hi, > > > > Can someone help me reset my password on Patchwork? I've used the 'Forgot > > password ' link several times and never received an email. > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Elect your leaders based on what they did after the last election, not > based on what they say before an election. > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". Thanks in advance, Sean McGovern ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avcodec/h264_mp4toannexb: Prepend SPS/PPS to buffering period SEI
Encoders may emit a buffering period SEI without a corresponding SPS/PPS if the SPS/PPS is carried out-of-band, eg with avcc. During Annex B conversion, this may result in the SPS/PPS being inserted *after* the buffering period SEI but before the IDR NAL. Since the buffering period SEI references the SPS, the SPS/PPS needs to come first. --- Notes: v2: Updated FATE test refs libavcodec/bsf/h264_mp4toannexb.c | 13 + tests/ref/fate/h264-bsf-mp4toannexb| 2 +- tests/ref/fate/h264_mp4toannexb_ticket2991 | 18 +- tests/ref/fate/segment-mp4-to-ts | 12 ++-- 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/libavcodec/bsf/h264_mp4toannexb.c b/libavcodec/bsf/h264_mp4toannexb.c index 92af6a6881..6607f1e91a 100644 --- a/libavcodec/bsf/h264_mp4toannexb.c +++ b/libavcodec/bsf/h264_mp4toannexb.c @@ -363,6 +363,19 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt) if (!new_idr && unit_type == H264_NAL_IDR_SLICE && (buf[1] & 0x80)) new_idr = 1; +/* If this is a buffering period SEI without a corresponding sps/pps + * then prepend any existing sps/pps before the SEI */ +if (unit_type == H264_NAL_SEI && buf[1] == 0 && !sps_seen && !pps_seen) { +if (s->sps_size) { +count_or_copy(&out, &out_size, s->sps, s->sps_size, PS_OUT_OF_BAND, j); +sps_seen = 1; +} +if (s->pps_size) { +count_or_copy(&out, &out_size, s->pps, s->pps_size, PS_OUT_OF_BAND, j); +pps_seen = 1; +} +} + /* prepend only to the first type 5 NAL unit of an IDR picture, if no sps/pps are already present */ if (new_idr && unit_type == H264_NAL_IDR_SLICE && !sps_seen && !pps_seen) { if (s->sps_size) diff --git a/tests/ref/fate/h264-bsf-mp4toannexb b/tests/ref/fate/h264-bsf-mp4toannexb index 2049f39701..81ff568f3d 100644 --- a/tests/ref/fate/h264-bsf-mp4toannexb +++ b/tests/ref/fate/h264-bsf-mp4toannexb @@ -1 +1 @@ -5f04c27cc6ee8625fe2405fb0f7da9a3 +ff2551123909f54c382294baa1bb4364 diff --git a/tests/ref/fate/h264_mp4toannexb_ticket2991 b/tests/ref/fate/h264_mp4toannexb_ticket2991 index f8e3e920d4..9a1fbf2f8c 100644 --- a/tests/ref/fate/h264_mp4toannexb_ticket2991 +++ b/tests/ref/fate/h264_mp4toannexb_ticket2991 @@ -1,4 +1,4 @@ -05d66e60ab22ee004720e0051af0fe74 *tests/data/fate/h264_mp4toannexb_ticket2991.h264 +b6ff5910928ad0b2a7eec481dcc41594 *tests/data/fate/h264_mp4toannexb_ticket2991.h264 1985815 tests/data/fate/h264_mp4toannexb_ticket2991.h264 #extradata 0: 47, 0x3a590d55 #tb 0: 1/120 @@ -6,7 +6,7 @@ #codec_id 0: h264 #dimensions 0: 1280x720 #sar 0: 3/4 -0, 0, 0,40040,37126, 0xb020184c +0, 0, 0,40040,37126, 0x515c184c 0, 40040, 40040,40040, 6920, 0x8512361a, F=0x0 0, 80081, 80081,40040, 7550, 0x1bc56ed4, F=0x0 0, 120121, 120121,40040, 8752, 0xb8c6f0a1, F=0x0 @@ -21,7 +21,7 @@ 0, 480485, 480485,40040,11234, 0x83cbd9fd, F=0x0 0, 520525, 520525,40040,17616, 0xfdf95104, F=0x0 0, 560566, 560566,40040,10689, 0x9633d32b, F=0x0 -0, 600606, 600606,40040,45291, 0x543c2cf6 +0, 600606, 600606,40040,45291, 0xa8292cf6 0, 640646, 640646,40040,20837, 0x051abfab, F=0x0 0, 680687, 680687,40040,21418, 0xe2a59d70, F=0x0 0, 720727, 720727,40040,15643, 0x15cf2cec, F=0x0 @@ -36,7 +36,7 @@ 0,1081091,1081091,40040,13130, 0xcbb6bb8e, F=0x0 0,1121131,1121131,40040,16180, 0x5d188a7a, F=0x0 0,1161172,1161172,40040,14961, 0x9ff2f463, F=0x0 -0,1201212,1201212,40040,54296, 0xe6ec30ed +0,1201212,1201212,40040,54296, 0x3ae830ed 0,1241252,1241252,40040,11500, 0x8c4852c9, F=0x0 0,1281293,1281293,40040,12065, 0xfb7954c3, F=0x0 0,1321333,1321333,40040,12532, 0xf0a935d3, F=0x0 @@ -51,7 +51,7 @@ 0,1681697,1681697,40040,13250, 0xfed0deb8, F=0x0 0,1721737,1721737,40040,13360, 0xbf92d476, F=0x0 0,1761778,1761778,40040,11749, 0x3041eaf1, F=0x0 -0,1801818,1801818,40040,23997, 0xdbe6d5c4 +0,1801818,1801818,40040,23997, 0x2fe2d5c4 0,1841858,1841858,40040,16065, 0xe8f715b7, F=0x0 0,1881899,1881899,40040,16441, 0x0a4e060f, F=0x0 0,1921939,1921939,40040,17395, 0xa8edecc2, F=0x0 @@ -66,7 +66,7 @@ 0,2282303,2282303,40040,13748, 0xed26aeb4, F=0x0 0,2322343,2322343,40040,15092, 0x3c983538, F=0x0 0,2362384,2362384,40040,14636, 0x9b278a6c, F=0x0 -0,2402424,24024
[FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: set keep_aspect to true for the crop filter
The input sample aspect ratio applies to the pre-cropping dimensions, so update it. Signed-off-by: James Almer --- fftools/ffmpeg_filter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c index 097bd2ed48..0d85c30aba 100644 --- a/fftools/ffmpeg_filter.c +++ b/fftools/ffmpeg_filter.c @@ -1703,7 +1703,7 @@ static int configure_input_video_filter(FilterGraph *fg, AVFilterGraph *graph, if ((ifp->opts.flags & IFILTER_FLAG_CROP)) { char crop_buf[64]; -snprintf(crop_buf, sizeof(crop_buf), "w=iw-%d-%d:h=ih-%d-%d:x=%d:y=%d", +snprintf(crop_buf, sizeof(crop_buf), "w=iw-%d-%d:h=ih-%d-%d:x=%d:y=%d:keep_aspect=1", ifp->opts.crop_left, ifp->opts.crop_right, ifp->opts.crop_top, ifp->opts.crop_bottom, ifp->opts.crop_left, ifp->opts.crop_top); -- 2.45.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] videotoolbox increases min target to macOS 12
Thanks for the reply. On 2024-07-09 02:16, Gnattu OC via ffmpeg-devel wrote: Well the code introduced is to fix certain HDR related bugs and it does not "works perfectly” before, it was just broken. I was rather talking about being able to compile it. I can compile ffmpeg without videotoolbox for older deployment targets without any issues. It is great to fix videotoolbox, but is it required to use functions/methods that are only available in newer OS? All the other libraries and ffmpeg itself can fix bugs without using functions that are not available on "older" systems. If you say it isn't possible, then all is good. I am not asking anyone to make the impossible possible. I am just trying to understaand why videotoolbox is raising the minimum deploymewnt target all the time, while this is not necessary for anything else. Don't get me wrong, I am compiling ffmpeg with around 50 3rd party libraries. None of them requires me to raise the deployment target. so that it should at least compile on older OS targets. But the end result is that the colorspace is as wrong as before, an older version cannot receive the fix to set the correct colorspace. For this use case I could see that using the (now deprecated) `CVBufferGetAttachments` instead on older OS, but I cannot really test that because I don’t have device running that old version. FYI. I am also compiling on the most recent macOS release but I set the deployment target via env var: export MACOSX_DEPLOYMENT_TARGET=10.13 -- regards Helmut K. C. Tessarek KeyID 0x172380A011EF4944 Key fingerprint = 8A55 70C1 BD85 D34E ADBC 386C 1723 80A0 11EF 4944 /* Thou shalt not follow the NULL pointer for chaos and madness await thee at its end. */ OpenPGP_signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] videotoolbox increases min target to macOS 12
On 9 Jul 2024, at 21:44, Helmut K. C. Tessarek wrote: > Thanks for the reply. > > On 2024-07-09 02:16, Gnattu OC via ffmpeg-devel wrote: >> Well the code introduced is to fix certain HDR related bugs and it does not >> "works perfectly” before, it was just broken. > > I was rather talking about being able to compile it. I can compile ffmpeg > without videotoolbox for older deployment targets without any issues. > > It is great to fix videotoolbox, but is it required to use functions/methods > that are only available in newer OS? All the other libraries and ffmpeg > itself can fix bugs without using functions that are not available on "older" > systems. > > If you say it isn't possible, then all is good. I am not asking anyone to > make the impossible possible. > > I am just trying to understaand why videotoolbox is raising the minimum > deploymewnt target all the time, while this is not necessary for anything > else. Don't get me wrong, I am compiling ffmpeg with around 50 3rd party > libraries. None of them requires me to raise the deployment target. Videotoolbox should not have raised the deployment target and I did not notice any change recently that would have done it other than mine accidentally. If you encounter other issues with it, once my fix is merged, do not hesitate to report them here. > >> so that it should at least compile on older OS targets. >> But the end result is that the colorspace is as wrong as before, an older >> version cannot receive the fix to set the correct colorspace. >> >> For this use case I could see that using the (now deprecated) >> `CVBufferGetAttachments` instead on older OS, but I cannot really test that >> because I don’t have device running that old version. > > FYI. I am also compiling on the most recent macOS release but I set the > deployment target via env var: > > export MACOSX_DEPLOYMENT_TARGET=10.13 > > > -- > regards Helmut K. C. Tessarek KeyID 0x172380A011EF4944 > Key fingerprint = 8A55 70C1 BD85 D34E ADBC 386C 1723 80A0 11EF 4944 > > /* >Thou shalt not follow the NULL pointer for chaos and madness >await thee at its end. > */ > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] videotoolbox increases min target to macOS 12
On 2024-07-09 16:04, epira...@gmail.com wrote: Videotoolbox should not have raised the deployment target and I did not notice any change recently that would have done it other than mine accidentally. If you encounter other issues with it, once my fix is merged, do not hesitate to report them here. Thanks, will do. OpenPGP_signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCHv3] swscale: prevent undefined behaviour in the PUTRGBA macro
On 7/9/24 2:34 PM, Sean McGovern wrote: For even small values of 'asrc', shifting them by 24 bits or more will cause arithmetic overflow and be caught by GCC's undefined behaviour sanitizer. Ensure the values do not overflow by up-casting the bracketed expressions involving 'asrc' to int32_t. --- libswscale/yuv2rgb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libswscale/yuv2rgb.c b/libswscale/yuv2rgb.c index 977eb3a7dd..ac0b811f61 100644 --- a/libswscale/yuv2rgb.c +++ b/libswscale/yuv2rgb.c @@ -100,9 +100,9 @@ const int *sws_getCoefficients(int colorspace) #define PUTRGBA(dst, ysrc, asrc, i, abase) \ Y = ysrc[2 * i]; \ -dst[2 * i] = r[Y] + g[Y] + b[Y] + (asrc[2 * i] << abase); \ +dst[2 * i] = r[Y] + g[Y] + b[Y] + ((int32_t)(asrc[2 * i]) << abase); \ Y = ysrc[2 * i + 1]; \ -dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + (asrc[2 * i + 1] << abase); +dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + ((int32_t)(asrc[2 * i + 1]) << abase); #define PUTRGB48(dst, src, asrc, i, abase) \ Y= src[ 2 * i]; \ Are these able to be negative? If not, it may be wise to cast to uint32_t instead as left shifting a negative integer is UB. - Leo Izen (Traneptora) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avfilter/vf_crop: prevent integer overflows when calculating SAR
Signed-off-by: James Almer --- libavfilter/vf_crop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavfilter/vf_crop.c b/libavfilter/vf_crop.c index 6361209941..d4966323f5 100644 --- a/libavfilter/vf_crop.c +++ b/libavfilter/vf_crop.c @@ -206,7 +206,7 @@ static int config_input(AVFilterLink *link) AVRational dar = av_mul_q(link->sample_aspect_ratio, (AVRational){ link->w, link->h }); av_reduce(&s->out_sar.num, &s->out_sar.den, - dar.num * s->h, dar.den * s->w, INT_MAX); + (int64_t)dar.num * s->h, (int64_t)dar.den * s->w, INT_MAX); } else s->out_sar = link->sample_aspect_ratio; -- 2.45.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCHv4] swscale: prevent undefined behaviour in the PUTRGBA macro
For even small values of 'asrc[x]', shifting them by 24 bits or more will cause arithmetic overflow and be caught by GCC's undefined behaviour sanitizer. Ensure the values do not overflow by up-casting the bracketed expressions involving 'asrc' to uint32_t. --- libswscale/yuv2rgb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libswscale/yuv2rgb.c b/libswscale/yuv2rgb.c index 977eb3a7dd..cfbc54abd0 100644 --- a/libswscale/yuv2rgb.c +++ b/libswscale/yuv2rgb.c @@ -100,9 +100,9 @@ const int *sws_getCoefficients(int colorspace) #define PUTRGBA(dst, ysrc, asrc, i, abase) \ Y = ysrc[2 * i]; \ -dst[2 * i] = r[Y] + g[Y] + b[Y] + (asrc[2 * i] << abase); \ +dst[2 * i] = r[Y] + g[Y] + b[Y] + ((uint32_t)(asrc[2 * i]) << abase); \ Y = ysrc[2 * i + 1]; \ -dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + (asrc[2 * i + 1] << abase); +dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + ((uint32_t)(asrc[2 * i + 1]) << abase); #define PUTRGB48(dst, src, asrc, i, abase) \ Y= src[ 2 * i]; \ -- 2.39.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] avfilter/vf_scale: Cleanup some checks
On Tue, Jul 09, 2024 at 04:46:59PM +0200, Kacper Michajlow wrote: > On Tue, 9 Jul 2024 at 15:17, Anton Khirnov wrote: > > > > Quoting Michael Niedermayer (2024-07-09 13:37:11) > > > Fixes: CID1513722 Operands don't affect result > > > > > > Sponsored-by: Sovereign Tech Fund > > > Signed-off-by: Michael Niedermayer > > > --- > > > libavfilter/vf_scale.c | 6 ++ > > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > > > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c > > > index bf09196e10d..18e9393d6c1 100644 > > > --- a/libavfilter/vf_scale.c > > > +++ b/libavfilter/vf_scale.c > > > @@ -645,10 +645,8 @@ static int config_props(AVFilterLink *outlink) > > > if (ret < 0) > > > goto fail; > > > > > > -if (outlink->w > INT_MAX || > > > -outlink->h > INT_MAX || > > > -(outlink->h * inlink->w) > INT_MAX || > > > -(outlink->w * inlink->h) > INT_MAX) > > > +if ((outlink->h * (int64_t)inlink->w) > INT32_MAX || > > > +(outlink->w * (int64_t)inlink->h) > INT32_MAX) > > > > This does not seems cleaner to me. > > > > Also, this check overall seems fishy. Why is it here at all and not e.g. > > in ff_scale_adjust_dimensions()? Why does it not call > > av_image_check_size()? Why does it only print a warning and not do > > anything else? I intend to post a better version, but iam a little overworked ATM so not sure if someone else will do it first. > > I agree with Anton here. The checks in vf_scale are iffy at best. > For starters, this is `outlink->w > INT_MAX` dead check. As the `w` is > int already. that was removed by my patch for that reason. The issue btw originated by code factorization that replaced int64 by int IIRC > And there is already an UB in `scale_eval_dimensions()` > which converts double value to int without any checks. i try to work on one issue at a time ... > > I think something like this would make sense to reject big numbers, > and ofcourse ff_scale_adjust_dimensions() should be more clever about > overflows too. There is also an overflow in swscale, but that's > another story. > > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c > index a1a322ed9e..9483db7564 100644 > --- a/libavfilter/vf_scale.c > +++ b/libavfilter/vf_scale.c > @@ -537,7 +537,7 @@ static int scale_eval_dimensions(AVFilterContext *ctx) > const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format); > const AVPixFmtDescriptor *out_desc = av_pix_fmt_desc_get(outlink->format); > char *expr; > - int eval_w, eval_h; > + double eval_w, eval_h; > int ret; > double res; > const AVPixFmtDescriptor *main_desc; not a valid patch, that wont apply, its also too messed up formating wise to review also we generally want to minimize double/float so any switch from int to double generally feels "wrong" thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are too smart to engage in politics are punished by being governed by those who are dumber. -- Plato signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avutil/imgutils: av_image_check_size2() ensure width and height fit in 32bit
On Tue, Jul 09, 2024 at 05:14:37PM +0200, Anton Khirnov wrote: > Quoting Michael Niedermayer (2024-07-09 15:28:10) > > On Tue, Jul 09, 2024 at 03:17:58PM +0200, Anton Khirnov wrote: > > > > ensure width and height fit in 32bit > > > > > > why? > > > > because not everyone wants undefined behavior > > because not everyone wants security issues > > because we dont support width and height > 32bit and its easier to check in > > a central place > > because the changed codes purpose is to check if the image paramaters are > > within what we support, and width of 100 billion is not. You can try > > all encoders with 100billion width. Then try to decode. > > Iam curious, how many work, how many fail and how they fail > > how many invalid bitstreams with no warning, how many undefined > > behaviors, ... > > > > Simply building FFmpeg on a platform with 64bit ints doesnt update > > ISO and ITU standards to allow larger values > > Quoting Michael Niedermayer (2020-10-07 16:45:56): > > At least in code i wrote and write i consider it a bug if it would > > assume sizeof(int/unsigned) == 4 > > Make up your mind. Where do you see a contradiction ? 2020: assuming sizeof(int/unsigned) == 4 is a bug 2024: we do not support more than 32bit width and height, nor is that supported by the majority of codec bitsterams and formats -> We thus should in a central place check that instead of generating undefined behavior and security issues What i suggest IS actually fixing a "sizeof(int/unsigned) == 4" bug If someone wants to make the codebase work with 64bit width and height, this should not be limited to "int is 64bit" systems that would be a very seriously broken design and also very illogic. Also your terse replies feel a bit rude thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No great genius has ever existed without some touch of madness. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 1/2] hwcontext_vulkan: add a new mechanism to expose used queue families
The issue with the old mechanism is that we had to introduce new API each time we needed a new queue family, and all the queue families were functionally fixed to a given purpose. Nvidia's GPUs are able to handle video encoding and compute on the same queue, which results in a speedup when pre-processing is required. Also, this enables us to expose optical flow queues for frame interpolation. --- libavutil/hwcontext_vulkan.c | 57 libavutil/hwcontext_vulkan.h | 25 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c index da377aa1a4..af60ab29d2 100644 --- a/libavutil/hwcontext_vulkan.c +++ b/libavutil/hwcontext_vulkan.c @@ -1428,7 +1428,8 @@ static int vulkan_device_init(AVHWDeviceContext *ctx) VulkanDevicePriv *p = ctx->hwctx; AVVulkanDeviceContext *hwctx = &p->p; FFVulkanFunctions *vk = &p->vkctx.vkfn; -VkQueueFamilyProperties *qf; +VkQueueFamilyProperties2 *qf; +VkQueueFamilyVideoPropertiesKHR *qf_vid; int graph_index, comp_index, tx_index, enc_index, dec_index; /* Set device extension flags */ @@ -1474,11 +1475,27 @@ static int vulkan_device_init(AVHWDeviceContext *ctx) return AVERROR_EXTERNAL; } -qf = av_malloc_array(qf_num, sizeof(VkQueueFamilyProperties)); +qf = av_malloc_array(qf_num, sizeof(VkQueueFamilyProperties2)); if (!qf) return AVERROR(ENOMEM); -vk->GetPhysicalDeviceQueueFamilyProperties(hwctx->phys_dev, &qf_num, qf); +qf_vid = av_malloc_array(qf_num, sizeof(VkQueueFamilyVideoPropertiesKHR)); +if (!qf_vid) { +av_free(qf); +return AVERROR(ENOMEM); +} + +for (uint32_t i = 0; i < qf_num; i++) { +qf_vid[i] = (VkQueueFamilyVideoPropertiesKHR) { +.sType = VK_STRUCTURE_TYPE_QUEUE_FAMILY_VIDEO_PROPERTIES_KHR, +}; +qf[i] = (VkQueueFamilyProperties2) { +.sType = VK_STRUCTURE_TYPE_QUEUE_FAMILY_PROPERTIES_2, +.pNext = &qf_vid[i], +}; +} + +vk->GetPhysicalDeviceQueueFamilyProperties2(hwctx->phys_dev, &qf_num, qf); p->qf_mutex = av_calloc(qf_num, sizeof(*p->qf_mutex)); if (!p->qf_mutex) { @@ -1488,12 +1505,12 @@ static int vulkan_device_init(AVHWDeviceContext *ctx) p->nb_tot_qfs = qf_num; for (uint32_t i = 0; i < qf_num; i++) { -p->qf_mutex[i] = av_calloc(qf[i].queueCount, sizeof(**p->qf_mutex)); +p->qf_mutex[i] = av_calloc(qf[i].queueFamilyProperties.queueCount, sizeof(**p->qf_mutex)); if (!p->qf_mutex[i]) { av_free(qf); return AVERROR(ENOMEM); } -for (uint32_t j = 0; j < qf[i].queueCount; j++) { +for (uint32_t j = 0; j < qf[i].queueFamilyProperties.queueCount; j++) { err = pthread_mutex_init(&p->qf_mutex[i][j], NULL); if (err != 0) { av_log(ctx, AV_LOG_ERROR, "pthread_mutex_init failed : %s\n", @@ -1550,6 +1567,36 @@ static int vulkan_device_init(AVHWDeviceContext *ctx) #undef CHECK_QUEUE +/* Update the new queue family fields. If non-zero already, + * it means API users have set it. */ +if (!hwctx->nb_qf) { +#define ADD_QUEUE(ctx_qf, qc, flag)\ +do { \ +if (ctx_qf != -1) {\ +hwctx->qf[hwctx->nb_qf++] = (AVVulkanDeviceQueueFamily) { \ +.idx = ctx_qf, \ +.num = qc, \ +.flags = flag, \ +}; \ +} \ +} while (0) + +ADD_QUEUE(hwctx->queue_family_index, hwctx->nb_graphics_queues, VK_QUEUE_GRAPHICS_BIT); +ADD_QUEUE(hwctx->queue_family_comp_index, hwctx->nb_comp_queues, VK_QUEUE_COMPUTE_BIT); +ADD_QUEUE(hwctx->queue_family_tx_index, hwctx->nb_tx_queues, VK_QUEUE_TRANSFER_BIT); +ADD_QUEUE(hwctx->queue_family_decode_index, hwctx->nb_decode_queues, VK_QUEUE_VIDEO_DECODE_BIT_KHR); +ADD_QUEUE(hwctx->queue_family_encode_index, hwctx->nb_encode_queues, VK_QUEUE_VIDEO_ENCODE_BIT_KHR); +#undef ADD_QUEUE +} + +for (int i = 0; i < hwctx->nb_qf; i++) { +if (!hwctx->qf[i].video_caps && +hwctx->qf[i].flags & (VK_QUEUE_VIDEO_DECODE_BIT_KHR | + VK_QUEUE_VIDEO_ENCODE_BIT_KHR)) { +hwctx->qf[i].video_caps = qf_vid[hwctx->qf[i].idx].videoCodecOperations; +} +} + if (!hwctx->lock_queue) hwctx->lock_queue = lock_queue; if (!hwctx->unlock_queue) diff --git a/libavutil/hwcontext_vulkan.h b/libavutil/hwcontext_vulkan.h index cbbd2390c1..394af46649
[FFmpeg-devel] [PATCH v2 2/2] vulkan: use the new queue family mechanism
--- libavutil/vulkan.c | 68 ++ libavutil/vulkan.h | 2 +- 2 files changed, 21 insertions(+), 49 deletions(-) diff --git a/libavutil/vulkan.c b/libavutil/vulkan.c index e0208c5a7c..d98e863711 100644 --- a/libavutil/vulkan.c +++ b/libavutil/vulkan.c @@ -189,37 +189,14 @@ int ff_vk_load_props(FFVulkanContext *s) static int vk_qf_get_index(FFVulkanContext *s, VkQueueFlagBits dev_family, int *nb) { -int ret, num; - -switch (dev_family) { -case VK_QUEUE_GRAPHICS_BIT: -ret = s->hwctx->queue_family_index; -num = s->hwctx->nb_graphics_queues; -break; -case VK_QUEUE_COMPUTE_BIT: -ret = s->hwctx->queue_family_comp_index; -num = s->hwctx->nb_comp_queues; -break; -case VK_QUEUE_TRANSFER_BIT: -ret = s->hwctx->queue_family_tx_index; -num = s->hwctx->nb_tx_queues; -break; -case VK_QUEUE_VIDEO_ENCODE_BIT_KHR: -ret = s->hwctx->queue_family_encode_index; -num = s->hwctx->nb_encode_queues; -break; -case VK_QUEUE_VIDEO_DECODE_BIT_KHR: -ret = s->hwctx->queue_family_decode_index; -num = s->hwctx->nb_decode_queues; -break; -default: -av_assert0(0); /* Should never happen */ +for (int i = 0; i < s->hwctx->nb_qf; i++) { +if (s->hwctx->qf[i].flags & dev_family) { +*nb = s->hwctx->qf[i].num; +return s->hwctx->qf[i].idx; +} } -if (nb) -*nb = num; - -return ret; +av_assert0(0); /* Should never happen */ } int ff_vk_qf_init(FFVulkanContext *s, FFVkQueueFamilyCtx *qf, @@ -229,25 +206,20 @@ int ff_vk_qf_init(FFVulkanContext *s, FFVkQueueFamilyCtx *qf, if (!s->nb_qfs) { s->nb_qfs = 0; -/* Simply fills in all unique queues into s->qfs */ -if (s->hwctx->queue_family_index >= 0) -s->qfs[s->nb_qfs++] = s->hwctx->queue_family_index; -if (!s->nb_qfs || s->qfs[0] != s->hwctx->queue_family_tx_index) -s->qfs[s->nb_qfs++] = s->hwctx->queue_family_tx_index; -if (!s->nb_qfs || (s->qfs[0] != s->hwctx->queue_family_comp_index && - s->qfs[1] != s->hwctx->queue_family_comp_index)) -s->qfs[s->nb_qfs++] = s->hwctx->queue_family_comp_index; -if (s->hwctx->queue_family_decode_index >= 0 && - (s->qfs[0] != s->hwctx->queue_family_decode_index && - s->qfs[1] != s->hwctx->queue_family_decode_index && - s->qfs[2] != s->hwctx->queue_family_decode_index)) -s->qfs[s->nb_qfs++] = s->hwctx->queue_family_decode_index; -if (s->hwctx->queue_family_encode_index >= 0 && - (s->qfs[0] != s->hwctx->queue_family_encode_index && - s->qfs[1] != s->hwctx->queue_family_encode_index && - s->qfs[2] != s->hwctx->queue_family_encode_index && - s->qfs[3] != s->hwctx->queue_family_encode_index)) -s->qfs[s->nb_qfs++] = s->hwctx->queue_family_encode_index; +for (int i = 0; i < s->hwctx->nb_qf; i++) { +/* Skip duplicates */ +int skip = 0; +for (int j = 0; j < s->nb_qfs; j++) { +if (s->qfs[j] == s->hwctx->qf[i].idx) { +skip = 1; +break; +} +} +if (skip) +continue; + +s->qfs[s->nb_qfs++] = s->hwctx->qf[i].idx; +} } return (qf->queue_family = vk_qf_get_index(s, dev_family, &qf->nb_queues)); diff --git a/libavutil/vulkan.h b/libavutil/vulkan.h index 15d954fcb8..bedadedde6 100644 --- a/libavutil/vulkan.h +++ b/libavutil/vulkan.h @@ -257,7 +257,7 @@ typedef struct FFVulkanContext { AVHWFramesContext *frames; AVVulkanFramesContext *hwfc; -uint32_t qfs[5]; +uint32_t qfs[16]; intnb_qfs; /* Properties */ -- 2.45.1.288.g0e0cd299f1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] hwcontext_vulkan: add a new mechanism to expose used queue families
On 09/07/2024 08:57, Anton Khirnov wrote: Quoting Lynne via ffmpeg-devel (2024-07-09 03:07:12) @@ -151,6 +162,17 @@ typedef struct AVVulkanDeviceContext { * Similar to lock_queue(), unlocks a queue. Must only be called after locking. */ void (*unlock_queue)(struct AVHWDeviceContext *ctx, uint32_t queue_family, uint32_t index); + +/** + * Queue families used. Must be preferentially ordered. List may contain + * duplicates, as long as their capability flags do not match. + * + * For compatibility reasons, all the enabled queue families listed above + * (queue_family_(tx/comp/encode/decode)_index) must also be included in + * this list until they're removed after deprecation. + */ +AVVulkanDeviceQueueFamily qf[16]; Why 16? And are we really really sure sizeof(AVVulkanDeviceQueueFamily) should be a part of the ABI? 16 is just an arbitrary limit. I don't expect to need more than this ever, but if we do, its not something that we can't wait until a bump occurs. I can increase it to 32 if you're concerned about it. There are 6 total queue family types, and 6 more currently supported encode and decode operations for each queue -> 12. I'd like to avoid making this not a part of the ABI, particularly as its a context that users should be able to easily set themselves. OpenPGP_0xA2FEA5F03F034464.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 1/2] hwcontext_vulkan: add a new mechanism to expose used queue families
The issue with the old mechanism is that we had to introduce new API each time we needed a new queue family, and all the queue families were functionally fixed to a given purpose. Nvidia's GPUs are able to handle video encoding and compute on the same queue, which results in a speedup when pre-processing is required. Also, this enables us to expose optical flow queues for frame interpolation. --- libavutil/hwcontext_vulkan.c | 85 libavutil/hwcontext_vulkan.h | 25 +++ 2 files changed, 93 insertions(+), 17 deletions(-) diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c index da377aa1a4..33d856ddd3 100644 --- a/libavutil/hwcontext_vulkan.c +++ b/libavutil/hwcontext_vulkan.c @@ -1423,12 +1423,13 @@ static void unlock_queue(AVHWDeviceContext *ctx, uint32_t queue_family, uint32_t static int vulkan_device_init(AVHWDeviceContext *ctx) { -int err; +int err = 0; uint32_t qf_num; VulkanDevicePriv *p = ctx->hwctx; AVVulkanDeviceContext *hwctx = &p->p; FFVulkanFunctions *vk = &p->vkctx.vkfn; -VkQueueFamilyProperties *qf; +VkQueueFamilyProperties2 *qf; +VkQueueFamilyVideoPropertiesKHR *qf_vid; int graph_index, comp_index, tx_index, enc_index, dec_index; /* Set device extension flags */ @@ -1474,38 +1475,53 @@ static int vulkan_device_init(AVHWDeviceContext *ctx) return AVERROR_EXTERNAL; } -qf = av_malloc_array(qf_num, sizeof(VkQueueFamilyProperties)); +qf = av_malloc_array(qf_num, sizeof(VkQueueFamilyProperties2)); if (!qf) return AVERROR(ENOMEM); -vk->GetPhysicalDeviceQueueFamilyProperties(hwctx->phys_dev, &qf_num, qf); +qf_vid = av_malloc_array(qf_num, sizeof(VkQueueFamilyVideoPropertiesKHR)); +if (!qf_vid) { +av_free(qf); +return AVERROR(ENOMEM); +} + +for (uint32_t i = 0; i < qf_num; i++) { +qf_vid[i] = (VkQueueFamilyVideoPropertiesKHR) { +.sType = VK_STRUCTURE_TYPE_QUEUE_FAMILY_VIDEO_PROPERTIES_KHR, +}; +qf[i] = (VkQueueFamilyProperties2) { +.sType = VK_STRUCTURE_TYPE_QUEUE_FAMILY_PROPERTIES_2, +.pNext = &qf_vid[i], +}; +} + +vk->GetPhysicalDeviceQueueFamilyProperties2(hwctx->phys_dev, &qf_num, qf); p->qf_mutex = av_calloc(qf_num, sizeof(*p->qf_mutex)); if (!p->qf_mutex) { -av_free(qf); -return AVERROR(ENOMEM); +err = AVERROR(ENOMEM); +goto end; } p->nb_tot_qfs = qf_num; for (uint32_t i = 0; i < qf_num; i++) { -p->qf_mutex[i] = av_calloc(qf[i].queueCount, sizeof(**p->qf_mutex)); +p->qf_mutex[i] = av_calloc(qf[i].queueFamilyProperties.queueCount, + sizeof(**p->qf_mutex)); if (!p->qf_mutex[i]) { -av_free(qf); -return AVERROR(ENOMEM); +err = AVERROR(ENOMEM); +goto end; } -for (uint32_t j = 0; j < qf[i].queueCount; j++) { +for (uint32_t j = 0; j < qf[i].queueFamilyProperties.queueCount; j++) { err = pthread_mutex_init(&p->qf_mutex[i][j], NULL); if (err != 0) { av_log(ctx, AV_LOG_ERROR, "pthread_mutex_init failed : %s\n", av_err2str(err)); -av_free(qf); -return AVERROR(err); +err = AVERROR(err); +goto end; } } } -av_free(qf); - graph_index = hwctx->nb_graphics_queues ? hwctx->queue_family_index : -1; comp_index = hwctx->nb_comp_queues ? hwctx->queue_family_comp_index : -1; tx_index= hwctx->nb_tx_queues ? hwctx->queue_family_tx_index : -1; @@ -1517,13 +1533,15 @@ static int vulkan_device_init(AVHWDeviceContext *ctx) if (ctx_qf < 0 && required) { \ av_log(ctx, AV_LOG_ERROR, "%s queue family is required, but marked as missing" \ " in the context!\n", type); \ -return AVERROR(EINVAL); \ +err = AVERROR(EINVAL); \ +goto end; \ } else if (fidx < 0 || ctx_qf < 0) { \ break; \ } else if (ctx_qf >= qf_num) { \ av_log(ctx, AV_LOG_ERROR, "Invalid %s family index %i (device has %i families)!\n", \ type, ctx_qf, qf_num); \ -return AVERROR(EINVAL);
Re: [FFmpeg-devel] [PATCH v3 3/3] lavc/vaapi_hevc: Don't require exact profiles
On Wed, 2024-06-05 at 20:59 +0100, Mark Thompson wrote: > Rather than turning the constraint flags into a single profile and > then > searching for that profile (and failing if it doesn't match any > profile > exactly), instead search all supported profiles and use the first one > which supports the given set of constraint flags. > --- > libavcodec/vaapi_decode.c | 45 +++--- > libavcodec/vaapi_hevc.c | 99 ++--- > -- > libavcodec/vaapi_hevc.h | 4 +- > 3 files changed, 87 insertions(+), 61 deletions(-) Test good for me. Thanks. -- Fei > > diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c > index 7c91d50f7b..c9d8add69f 100644 > --- a/libavcodec/vaapi_decode.c > +++ b/libavcodec/vaapi_decode.c > @@ -393,7 +393,9 @@ static const struct { > enum AVCodecID codec_id; > int codec_profile; > VAProfile va_profile; > - VAProfile (*profile_parser)(AVCodecContext *avctx); > + VAProfile (*match_profile)(AVCodecContext *avctx, > + const VAProfile *profile_list, > + int profile_count); > } vaapi_profile_map[] = { > #define MAP(c, p, v, ...) { AV_CODEC_ID_ ## c, AV_PROFILE_ ## p, > VAProfile ## v, __VA_ARGS__ } > MAP(MPEG2VIDEO, MPEG2_SIMPLE, MPEG2Simple ), > @@ -420,9 +422,9 @@ static const struct { > #endif > #if VA_CHECK_VERSION(1, 2, 0) && CONFIG_HEVC_VAAPI_HWACCEL > MAP(HEVC, HEVC_REXT, None, > - ff_vaapi_parse_hevc_rext_scc_profile ), > + ff_vaapi_hevc_match_rext_scc_profile ), > MAP(HEVC, HEVC_SCC, None, > - ff_vaapi_parse_hevc_rext_scc_profile ), > + ff_vaapi_hevc_match_rext_scc_profile ), > #endif > MAP(MJPEG, MJPEG_HUFFMAN_BASELINE_DCT, > JPEGBaseline), > @@ -505,22 +507,33 @@ static int > vaapi_decode_make_config(AVCodecContext *avctx, > vaapi_profile_map[i].codec_profile == > AV_PROFILE_UNKNOWN) > profile_match = 1; > > - va_profile = vaapi_profile_map[i].profile_parser ? > - vaapi_profile_map[i].profile_parser(avctx) : > - vaapi_profile_map[i].va_profile; > codec_profile = vaapi_profile_map[i].codec_profile; > - > - for (j = 0; j < profile_count; j++) { > - if (va_profile == profile_list[j]) { > - exact_match = profile_match; > + if (vaapi_profile_map[i].match_profile) { > + va_profile = > + vaapi_profile_map[i].match_profile(avctx, > profile_list, > + profile_count); > + if (va_profile != VAProfileNone) { > + matched_va_profile = va_profile; > + matched_ff_profile = codec_profile; > + exact_match = 1; > break; > } > - } > - if (j < profile_count) { > - matched_va_profile = va_profile; > - matched_ff_profile = codec_profile; > - if (exact_match) > - break; > + } else { > + va_profile = vaapi_profile_map[i].va_profile; > + > + for (j = 0; j < profile_count; j++) { > + if (va_profile == profile_list[j]) { > + exact_match = profile_match; > + break; > + } > + } > + > + if (j < profile_count) { > + matched_va_profile = va_profile; > + matched_ff_profile = codec_profile; > + if (exact_match) > + break; > + } > } > } > av_freep(&profile_list); > diff --git a/libavcodec/vaapi_hevc.c b/libavcodec/vaapi_hevc.c > index 6164a1d223..716a4a4b43 100644 > --- a/libavcodec/vaapi_hevc.c > +++ b/libavcodec/vaapi_hevc.c > @@ -589,63 +589,74 @@ static int ptl_convert(const PTLCommon > *general_ptl, H265RawProfileTierLevel *h2 > } > > /* > - * Find exact va_profile for HEVC Range Extension and Screen Content > Coding Extension > + * Find compatible va_profile for HEVC Range Extension and Screen > + * Content Coding Extension profiles. > */ > -VAProfile ff_vaapi_parse_hevc_rext_scc_profile(AVCodecContext > *avctx) > +VAProfile ff_vaapi_hevc_match_rext_scc_profile(AVCodecContext > *avctx, > + const VAProfile > *profile_list, > + int profile_count) > { > const HEVCContext *h = avctx->priv_data; > const HEVCSPS *sps = h->ps.sps; > const PTL *ptl = &sps->ptl; > const PTLCommon *general_ptl = &ptl->general_ptl; > - const H265ProfileDescriptor *profile; > H265RawProfileTierLevel h265_raw_ptl = {0}; > > + static const struct { > + enum H265Profile profile; > + VAProfile va_profile; > + } map[] = { > +#if VA_CHECK_VERSION(1, 2, 0) > +
Re: [FFmpeg-devel] [PATCH v3 2/2] lavc/hevcdec: Update slice index before hwaccel decode slice
> -Original Message- > From: Anton Khirnov > Sent: Tuesday, June 25, 2024 3:45 PM > To: FFmpeg development discussions and patches > Cc: Wang, Fei W > Subject: Re: [FFmpeg-devel] [PATCH v3 2/2] lavc/hevcdec: Update slice index > before hwaccel decode slice > > Quoting fei.w.wang-at-intel@ffmpeg.org (2024-06-24 08:23:31) > > From: Fei Wang > > > > Otherwise, slice index will never update for hwaccel decode, and slice > > RPL will be always overlap into first one which use slice index to > > construct. > > > > Fixes hwaccel decoding after 47d34ba7fbb81 > > > > Signed-off-by: Fei Wang > > ok Will push the patchset in 3 days if no more objection. -- Fei > > -- > 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-requ...@ffmpeg.org with subject "unsubscribe".