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

[epirat07@gmail.com]

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