* [FFmpeg-devel] [PATCH] avutil/hwcontext_videotoolbox: Allocate attachments dictionary
@ 2024-08-01 22:25 gnattu via ffmpeg-devel
2024-08-01 22:47 ` epirat07
0 siblings, 1 reply; 3+ messages in thread
From: gnattu via ffmpeg-devel @ 2024-08-01 22:25 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: gnattu
From: Gnattu OC <gnattuoc@me.com>
Allocate a dedicated attachments dictionary instead of trying to get
one from the pixel buffer. The attachments got from the pixel buffer
confuses the CVImageBufferCreateColorSpaceFromAttachments method and
will make it to output a wrong colorspace that causes problem like
#10884.
Signed-off-by: gnattu <gnattuoc@me.com>
---
libavutil/hwcontext_videotoolbox.c | 35 ++++++++++++++++++------------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c
index 1794459943..dda6ada1af 100644
--- a/libavutil/hwcontext_videotoolbox.c
+++ b/libavutil/hwcontext_videotoolbox.c
@@ -569,11 +569,19 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
CGColorSpaceRef colorspace = NULL;
CFStringRef colormatrix = NULL, colorpri = NULL, colortrc = NULL;
Float32 gamma = 0;
+ CFMutableDictionaryRef attachments = NULL;
+ attachments = CFDictionaryCreateMutable(NULL, 0,
+ &kCFTypeDictionaryKeyCallBacks,
+ &kCFTypeDictionaryValueCallBacks);
+ if (!attachments)
+ return AVERROR(ENOMEM);
colormatrix = av_map_videotoolbox_color_matrix_from_av(src->colorspace);
- if (colormatrix)
+ if (colormatrix) {
+ CFDictionarySetValue(attachments, kCVImageBufferYCbCrMatrixKey, colormatrix);
CVBufferSetAttachment(pixbuf, kCVImageBufferYCbCrMatrixKey,
- colormatrix, kCVAttachmentMode_ShouldPropagate);
+ colormatrix, kCVAttachmentMode_ShouldPropagate);
+ }
else {
CVBufferRemoveAttachment(pixbuf, kCVImageBufferYCbCrMatrixKey);
if (src->colorspace != AVCOL_SPC_UNSPECIFIED)
@@ -583,9 +591,11 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
}
colorpri = av_map_videotoolbox_color_primaries_from_av(src->color_primaries);
- if (colorpri)
+ if (colorpri) {
+ CFDictionarySetValue(attachments, kCVImageBufferColorPrimariesKey, colorpri);
CVBufferSetAttachment(pixbuf, kCVImageBufferColorPrimariesKey,
- colorpri, kCVAttachmentMode_ShouldPropagate);
+ colorpri, kCVAttachmentMode_ShouldPropagate);
+ }
else {
CVBufferRemoveAttachment(pixbuf, kCVImageBufferColorPrimariesKey);
if (src->color_primaries != AVCOL_SPC_UNSPECIFIED)
@@ -595,9 +605,11 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
}
colortrc = av_map_videotoolbox_color_trc_from_av(src->color_trc);
- if (colortrc)
+ if (colortrc) {
+ CFDictionarySetValue(attachments, kCVImageBufferTransferFunctionKey, colortrc);
CVBufferSetAttachment(pixbuf, kCVImageBufferTransferFunctionKey,
- colorpri, kCVAttachmentMode_ShouldPropagate);
+ colorpri, kCVAttachmentMode_ShouldPropagate);
+ }
else {
CVBufferRemoveAttachment(pixbuf, kCVImageBufferTransferFunctionKey);
if (src->color_trc != AVCOL_TRC_UNSPECIFIED)
@@ -622,17 +634,12 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
#if (TARGET_OS_OSX && __MAC_OS_X_VERSION_MAX_ALLOWED >= 100800) || \
(TARGET_OS_IOS && __IPHONE_OS_VERSION_MAX_ALLOWED >= 100000)
if (__builtin_available(macOS 10.8, iOS 10, *)) {
- CFDictionaryRef attachments =
- vt_cv_buffer_copy_attachments(pixbuf, kCVAttachmentMode_ShouldPropagate);
-
- if (attachments) {
- colorspace =
- CVImageBufferCreateColorSpaceFromAttachments(attachments);
- CFRelease(attachments);
- }
+ colorspace = CVImageBufferCreateColorSpaceFromAttachments(attachments);
}
#endif
+ CFRelease(attachments);
+
// Done outside the above preprocessor code and if's so that
// in any case a wrong kCVImageBufferCGColorSpaceKey is removed
// if the above code is not used or fails.
--
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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avutil/hwcontext_videotoolbox: Allocate attachments dictionary
2024-08-01 22:25 [FFmpeg-devel] [PATCH] avutil/hwcontext_videotoolbox: Allocate attachments dictionary gnattu via ffmpeg-devel
@ 2024-08-01 22:47 ` epirat07
2024-08-01 23:33 ` Gnattu OC via ffmpeg-devel
0 siblings, 1 reply; 3+ messages in thread
From: epirat07 @ 2024-08-01 22:47 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: gnattu
On 2 Aug 2024, at 0:25, gnattu via ffmpeg-devel wrote:
> From: Gnattu OC <gnattuoc@me.com>
>
> Allocate a dedicated attachments dictionary instead of trying to get
> one from the pixel buffer. The attachments got from the pixel buffer
> confuses the CVImageBufferCreateColorSpaceFromAttachments method and
> will make it to output a wrong colorspace that causes problem like
> #10884.
Thanks for the patch. Looks fine in principle given this was essentially
how I did it before.
Did you check if maybe removing the kCVImageBufferCGColorSpaceKey by doing
CVBufferRemoveAttachment(pixbuf, kCVImageBufferCGColorSpaceKey); before
calling CVImageBufferCreateColorSpaceFromAttachments() is enough to make
it work with the attachments obtained from the pixel buffer?
>
> Signed-off-by: gnattu <gnattuoc@me.com>
> ---
> libavutil/hwcontext_videotoolbox.c | 35 ++++++++++++++++++------------
> 1 file changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c
> index 1794459943..dda6ada1af 100644
> --- a/libavutil/hwcontext_videotoolbox.c
> +++ b/libavutil/hwcontext_videotoolbox.c
> @@ -569,11 +569,19 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
> CGColorSpaceRef colorspace = NULL;
> CFStringRef colormatrix = NULL, colorpri = NULL, colortrc = NULL;
> Float32 gamma = 0;
> + CFMutableDictionaryRef attachments = NULL;
> + attachments = CFDictionaryCreateMutable(NULL, 0,
> + &kCFTypeDictionaryKeyCallBacks,
> + &kCFTypeDictionaryValueCallBacks);
> + if (!attachments)
> + return AVERROR(ENOMEM);
>
> colormatrix = av_map_videotoolbox_color_matrix_from_av(src->colorspace);
> - if (colormatrix)
> + if (colormatrix) {
> + CFDictionarySetValue(attachments, kCVImageBufferYCbCrMatrixKey, colormatrix);
> CVBufferSetAttachment(pixbuf, kCVImageBufferYCbCrMatrixKey,
> - colormatrix, kCVAttachmentMode_ShouldPropagate);
> + colormatrix, kCVAttachmentMode_ShouldPropagate);
> + }
> else {
> CVBufferRemoveAttachment(pixbuf, kCVImageBufferYCbCrMatrixKey);
> if (src->colorspace != AVCOL_SPC_UNSPECIFIED)
> @@ -583,9 +591,11 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
> }
>
> colorpri = av_map_videotoolbox_color_primaries_from_av(src->color_primaries);
> - if (colorpri)
> + if (colorpri) {
> + CFDictionarySetValue(attachments, kCVImageBufferColorPrimariesKey, colorpri);
> CVBufferSetAttachment(pixbuf, kCVImageBufferColorPrimariesKey,
> - colorpri, kCVAttachmentMode_ShouldPropagate);
> + colorpri, kCVAttachmentMode_ShouldPropagate);
> + }
> else {
> CVBufferRemoveAttachment(pixbuf, kCVImageBufferColorPrimariesKey);
> if (src->color_primaries != AVCOL_SPC_UNSPECIFIED)
> @@ -595,9 +605,11 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
> }
>
> colortrc = av_map_videotoolbox_color_trc_from_av(src->color_trc);
> - if (colortrc)
> + if (colortrc) {
> + CFDictionarySetValue(attachments, kCVImageBufferTransferFunctionKey, colortrc);
> CVBufferSetAttachment(pixbuf, kCVImageBufferTransferFunctionKey,
> - colorpri, kCVAttachmentMode_ShouldPropagate);
> + colorpri, kCVAttachmentMode_ShouldPropagate);
> + }
> else {
> CVBufferRemoveAttachment(pixbuf, kCVImageBufferTransferFunctionKey);
> if (src->color_trc != AVCOL_TRC_UNSPECIFIED)
> @@ -622,17 +634,12 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
> #if (TARGET_OS_OSX && __MAC_OS_X_VERSION_MAX_ALLOWED >= 100800) || \
> (TARGET_OS_IOS && __IPHONE_OS_VERSION_MAX_ALLOWED >= 100000)
> if (__builtin_available(macOS 10.8, iOS 10, *)) {
> - CFDictionaryRef attachments =
> - vt_cv_buffer_copy_attachments(pixbuf, kCVAttachmentMode_ShouldPropagate);
> -
> - if (attachments) {
> - colorspace =
> - CVImageBufferCreateColorSpaceFromAttachments(attachments);
> - CFRelease(attachments);
> - }
> + colorspace = CVImageBufferCreateColorSpaceFromAttachments(attachments);
> }
> #endif
>
> + CFRelease(attachments);
> +
> // Done outside the above preprocessor code and if's so that
> // in any case a wrong kCVImageBufferCGColorSpaceKey is removed
> // if the above code is not used or fails.
> --
> 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-request@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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avutil/hwcontext_videotoolbox: Allocate attachments dictionary
2024-08-01 22:47 ` epirat07
@ 2024-08-01 23:33 ` Gnattu OC via ffmpeg-devel
0 siblings, 0 replies; 3+ messages in thread
From: Gnattu OC via ffmpeg-devel @ 2024-08-01 23:33 UTC (permalink / raw)
To: epirat07; +Cc: Gnattu OC, FFmpeg development discussions and patches
Replaced by: https://ffmpeg.org//pipermail/ffmpeg-devel/2024-August/331962.html
> On Aug 2, 2024, at 06:47, epirat07@gmail.com wrote:
>
>
>
> On 2 Aug 2024, at 0:25, gnattu via ffmpeg-devel wrote:
>
>> From: Gnattu OC <gnattuoc@me.com>
>>
>> Allocate a dedicated attachments dictionary instead of trying to get
>> one from the pixel buffer. The attachments got from the pixel buffer
>> confuses the CVImageBufferCreateColorSpaceFromAttachments method and
>> will make it to output a wrong colorspace that causes problem like
>> #10884.
>
> Thanks for the patch. Looks fine in principle given this was essentially
> how I did it before.
>
> Did you check if maybe removing the kCVImageBufferCGColorSpaceKey by doing
> CVBufferRemoveAttachment(pixbuf, kCVImageBufferCGColorSpaceKey); before
> calling CVImageBufferCreateColorSpaceFromAttachments() is enough to make
> it work with the attachments obtained from the pixel buffer?
>
>>
>> Signed-off-by: gnattu <gnattuoc@me.com>
>> ---
>> libavutil/hwcontext_videotoolbox.c | 35 ++++++++++++++++++------------
>> 1 file changed, 21 insertions(+), 14 deletions(-)
>>
>> diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c
>> index 1794459943..dda6ada1af 100644
>> --- a/libavutil/hwcontext_videotoolbox.c
>> +++ b/libavutil/hwcontext_videotoolbox.c
>> @@ -569,11 +569,19 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
>> CGColorSpaceRef colorspace = NULL;
>> CFStringRef colormatrix = NULL, colorpri = NULL, colortrc = NULL;
>> Float32 gamma = 0;
>> + CFMutableDictionaryRef attachments = NULL;
>> + attachments = CFDictionaryCreateMutable(NULL, 0,
>> + &kCFTypeDictionaryKeyCallBacks,
>> + &kCFTypeDictionaryValueCallBacks);
>> + if (!attachments)
>> + return AVERROR(ENOMEM);
>>
>> colormatrix = av_map_videotoolbox_color_matrix_from_av(src->colorspace);
>> - if (colormatrix)
>> + if (colormatrix) {
>> + CFDictionarySetValue(attachments, kCVImageBufferYCbCrMatrixKey, colormatrix);
>> CVBufferSetAttachment(pixbuf, kCVImageBufferYCbCrMatrixKey,
>> - colormatrix, kCVAttachmentMode_ShouldPropagate);
>> + colormatrix, kCVAttachmentMode_ShouldPropagate);
>> + }
>> else {
>> CVBufferRemoveAttachment(pixbuf, kCVImageBufferYCbCrMatrixKey);
>> if (src->colorspace != AVCOL_SPC_UNSPECIFIED)
>> @@ -583,9 +591,11 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
>> }
>>
>> colorpri = av_map_videotoolbox_color_primaries_from_av(src->color_primaries);
>> - if (colorpri)
>> + if (colorpri) {
>> + CFDictionarySetValue(attachments, kCVImageBufferColorPrimariesKey, colorpri);
>> CVBufferSetAttachment(pixbuf, kCVImageBufferColorPrimariesKey,
>> - colorpri, kCVAttachmentMode_ShouldPropagate);
>> + colorpri, kCVAttachmentMode_ShouldPropagate);
>> + }
>> else {
>> CVBufferRemoveAttachment(pixbuf, kCVImageBufferColorPrimariesKey);
>> if (src->color_primaries != AVCOL_SPC_UNSPECIFIED)
>> @@ -595,9 +605,11 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
>> }
>>
>> colortrc = av_map_videotoolbox_color_trc_from_av(src->color_trc);
>> - if (colortrc)
>> + if (colortrc) {
>> + CFDictionarySetValue(attachments, kCVImageBufferTransferFunctionKey, colortrc);
>> CVBufferSetAttachment(pixbuf, kCVImageBufferTransferFunctionKey,
>> - colorpri, kCVAttachmentMode_ShouldPropagate);
>> + colorpri, kCVAttachmentMode_ShouldPropagate);
>> + }
>> else {
>> CVBufferRemoveAttachment(pixbuf, kCVImageBufferTransferFunctionKey);
>> if (src->color_trc != AVCOL_TRC_UNSPECIFIED)
>> @@ -622,17 +634,12 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
>> #if (TARGET_OS_OSX && __MAC_OS_X_VERSION_MAX_ALLOWED >= 100800) || \
>> (TARGET_OS_IOS && __IPHONE_OS_VERSION_MAX_ALLOWED >= 100000)
>> if (__builtin_available(macOS 10.8, iOS 10, *)) {
>> - CFDictionaryRef attachments =
>> - vt_cv_buffer_copy_attachments(pixbuf, kCVAttachmentMode_ShouldPropagate);
>> -
>> - if (attachments) {
>> - colorspace =
>> - CVImageBufferCreateColorSpaceFromAttachments(attachments);
>> - CFRelease(attachments);
>> - }
>> + colorspace = CVImageBufferCreateColorSpaceFromAttachments(attachments);
>> }
>> #endif
>>
>> + CFRelease(attachments);
>> +
>> // Done outside the above preprocessor code and if's so that
>> // in any case a wrong kCVImageBufferCGColorSpaceKey is removed
>> // if the above code is not used or fails.
>> --
>> 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-request@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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-08-01 23:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-01 22:25 [FFmpeg-devel] [PATCH] avutil/hwcontext_videotoolbox: Allocate attachments dictionary gnattu via ffmpeg-devel
2024-08-01 22:47 ` epirat07
2024-08-01 23:33 ` Gnattu OC via ffmpeg-devel
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git