Re: [FFmpeg-devel] [PATCH] avutil/hwcontext_videotoolbox: Allocate attachments dictionary

[Gnattu OC via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

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".