Re: [FFmpeg-devel] [PATCH v2] avutil/hwcontext_videotoolbox: Set proper CVBuffer colorspace

[Zhao Zhili <quinkblack@foxmail.com>]

> On May 24, 2024, at 02:41, epirat07@gmail.com wrote:
> 
> 
> On 23 May 2024, at 18:05, Zhao Zhili wrote:
> 
>>> On May 21, 2024, at 09:56, Gnattu OC via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote:
>>> 
>>>> 
>>>> On May 20, 2024, at 09:12, Marvin Scholz <epirat07@gmail.com> wrote:
>>>> 
>>>> Fix #10884
>>>> ---
>>>> libavutil/hwcontext_videotoolbox.c | 54 +++++++++++++++++++++---------
>>>> 1 file changed, 38 insertions(+), 16 deletions(-)
>>>> 
>>>> diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c
>>>> index 9f82b104c3..4a35bfc7ff 100644
>>>> --- a/libavutil/hwcontext_videotoolbox.c
>>>> +++ b/libavutil/hwcontext_videotoolbox.c
>>>> @@ -530,6 +530,8 @@ CFStringRef av_map_videotoolbox_color_trc_from_av(enum AVColorTransferCharacteri
>>>> static int vt_pixbuf_set_colorspace(void *log_ctx,
>>>>                                   CVPixelBufferRef pixbuf, const AVFrame *src)
>>>> {
>>>> +    CGColorSpaceRef colorspace = NULL;
>>>> +    CFMutableDictionaryRef attachments = NULL;
>>>>   CFStringRef colormatrix = NULL, colorpri = NULL, colortrc = NULL;
>>>>   Float32 gamma = 0;
>>>> 
>>>> @@ -550,37 +552,57 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
>>>>   else if (src->color_trc == AVCOL_TRC_GAMMA28)
>>>>       gamma = 2.8;
>>>> 
>>>> +    attachments = CFDictionaryCreateMutable(NULL, 0,
>>>> +                                                 &kCFTypeDictionaryKeyCallBacks,
>>>> +                                                 &kCFTypeDictionaryValueCallBacks);
>>>> +    if (!attachments)
>>>> +        return AVERROR(ENOMEM);
>>>> +
>>>>   if (colormatrix) {
>>>> -        CVBufferSetAttachment(
>>>> -            pixbuf,
>>>> +        CFDictionarySetValue(
>>>> +            attachments,
>>>>           kCVImageBufferYCbCrMatrixKey,
>>>> -            colormatrix,
>>>> -            kCVAttachmentMode_ShouldPropagate);
>>>> +            colormatrix);
>>>>   }
>>>>   if (colorpri) {
>>>> -        CVBufferSetAttachment(
>>>> -            pixbuf,
>>>> +        CFDictionarySetValue(
>>>> +            attachments,
>>>>           kCVImageBufferColorPrimariesKey,
>>>> -            colorpri,
>>>> -            kCVAttachmentMode_ShouldPropagate);
>>>> +            colorpri);
>>>>   }
>>>>   if (colortrc) {
>>>> -        CVBufferSetAttachment(
>>>> -            pixbuf,
>>>> +        CFDictionarySetValue(
>>>> +            attachments,
>>>>           kCVImageBufferTransferFunctionKey,
>>>> -            colortrc,
>>>> -            kCVAttachmentMode_ShouldPropagate);
>>>> +            colortrc);
>>>>   }
>>>>   if (gamma != 0) {
>>>>       CFNumberRef gamma_level = CFNumberCreate(NULL, kCFNumberFloat32Type, &gamma);
>>>> -        CVBufferSetAttachment(
>>>> -            pixbuf,
>>>> +        CFDictionarySetValue(
>>>> +            attachments,
>>>>           kCVImageBufferGammaLevelKey,
>>>> -            gamma_level,
>>>> -            kCVAttachmentMode_ShouldPropagate);
>>>> +            gamma_level);
>>>>       CFRelease(gamma_level);
>>>>   }
>>>> 
>>>> +    if (__builtin_available(macOS 10.8, iOS 10, *))
>>>> +        colorspace = CVImageBufferCreateColorSpaceFromAttachments(attachments);
>>>> +
>>>> +    if (colorspace) {
>>>> +        CFDictionarySetValue(
>>>> +            attachments,
>>>> +            kCVImageBufferCGColorSpaceKey,
>>>> +            colorspace);
>>>> +        CFRelease(colorspace);
>>>> +    } else
>>>> +        av_log(log_ctx, AV_LOG_WARNING, "Unable to set proper colorspace for the CVImageBuffer.\n");
>>> 
>>> This will spam the console on SDR video inputs because they have nothing to be set as the attachment and the colorspace creation will always fail and return nil.
>> 
>> I will just remove the log message and apply the patch this week if no objection.
>> 
> 
> I will do some more tests, if you want apply with the log message removed and I will do a follow up patch
> if I find another solution.

OK, then I can wait. Please notify when it’s ready.

> 
>>> 
>>>> +
>>>> +    CVBufferSetAttachments(
>>>> +        pixbuf,
>>>> +        attachments,
>>>> +        kCVAttachmentMode_ShouldPropagate);
>>>> +    CFRelease(attachments);
>>>> +
>>>>   return 0;
>>>> }
>>>> 
>>>> 
>>>> base-commit: 463c573e6b6489c588bee90124d5cf92db8ccaaa
>>>> -- 
>>>> 2.39.3 (Apple Git-145)
>>>> _______________________________________________
>>>> 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 <mailto:ffmpeg-devel@ffmpeg.org> <mailto:ffmpeg-devel@ffmpeg.org>
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto: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".