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

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(-)

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

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