[FFmpeg-devel] [PATCH v3 1/3] avutil/hwcontext_videotoolbox: Unset undefined values

[FFmpeg-devel] [PATCH v3 1/3] avutil/hwcontext_videotoolbox: Unset undefined values

[Marvin Scholz]

When mapping AVFrame properties to the CVBuffer attachments, it is
necessary to properly delete undefined attachments, else we can
leave incorrect values in there guessed from VideoToolbox for
example, leading to inconsistent results where the AVFrame and
CVBuffer differ in metadata.

Ref #10884
---
 libavutil/hwcontext_videotoolbox.c | 76 ++++++++++++++++--------------
 1 file changed, 41 insertions(+), 35 deletions(-)

diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c
index 9f82b104c3..0af2ab822f 100644
--- a/libavutil/hwcontext_videotoolbox.c
+++ b/libavutil/hwcontext_videotoolbox.c
@@ -342,8 +342,10 @@ static int vt_pixbuf_set_par(void *log_ctx,
     CFNumberRef num = NULL, den = NULL;
     AVRational avpar = src->sample_aspect_ratio;
 
-    if (avpar.num == 0)
+    if (avpar.num == 0) {
+        CVBufferRemoveAttachment(pixbuf, kCVImageBufferPixelAspectRatioKey);
         return 0;
+    }
 
     av_reduce(&avpar.num, &avpar.den,
                 avpar.num, avpar.den,
@@ -423,7 +425,10 @@ static int vt_pixbuf_set_chromaloc(void *log_ctx,
             kCVImageBufferChromaLocationTopFieldKey,
             loc,
             kCVAttachmentMode_ShouldPropagate);
-    }
+    } else
+        CVBufferRemoveAttachment(
+            pixbuf,
+            kCVImageBufferChromaLocationTopFieldKey);
 
     return 0;
 }
@@ -534,52 +539,53 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
     Float32 gamma = 0;
 
     colormatrix = av_map_videotoolbox_color_matrix_from_av(src->colorspace);
-    if (!colormatrix && src->colorspace != AVCOL_SPC_UNSPECIFIED)
-        av_log(log_ctx, AV_LOG_WARNING, "Color space %s is not supported.\n", av_color_space_name(src->colorspace));
+    if (colormatrix)
+        CVBufferSetAttachment(pixbuf, kCVImageBufferYCbCrMatrixKey,
+            colormatrix, kCVAttachmentMode_ShouldPropagate);
+    else {
+        CVBufferRemoveAttachment(pixbuf, kCVImageBufferYCbCrMatrixKey);
+        if (src->colorspace != AVCOL_SPC_UNSPECIFIED)
+            av_log(log_ctx, AV_LOG_WARNING,
+                "Color space %s is not supported.\n",
+                av_color_space_name(src->colorspace));
+    }
 
     colorpri = av_map_videotoolbox_color_primaries_from_av(src->color_primaries);
-    if (!colorpri && src->color_primaries != AVCOL_PRI_UNSPECIFIED)
-        av_log(log_ctx, AV_LOG_WARNING, "Color primaries %s is not supported.\n", av_color_primaries_name(src->color_primaries));
+    if (colorpri)
+        CVBufferSetAttachment(pixbuf, kCVImageBufferColorPrimariesKey,
+            colorpri, kCVAttachmentMode_ShouldPropagate);
+    else {
+        CVBufferRemoveAttachment(pixbuf, kCVImageBufferColorPrimariesKey);
+        if (src->color_primaries != AVCOL_SPC_UNSPECIFIED)
+            av_log(log_ctx, AV_LOG_WARNING,
+                "Color primaries %s is not supported.\n",
+                av_color_primaries_name(src->color_primaries));
+    }
 
     colortrc = av_map_videotoolbox_color_trc_from_av(src->color_trc);
-    if (!colortrc && src->color_trc != AVCOL_TRC_UNSPECIFIED)
-        av_log(log_ctx, AV_LOG_WARNING, "Color transfer function %s is not supported.\n", av_color_transfer_name(src->color_trc));
+    if (colortrc)
+        CVBufferSetAttachment(pixbuf, kCVImageBufferTransferFunctionKey,
+            colorpri, kCVAttachmentMode_ShouldPropagate);
+    else {
+        CVBufferRemoveAttachment(pixbuf, kCVImageBufferTransferFunctionKey);
+        if (src->color_trc != AVCOL_TRC_UNSPECIFIED)
+            av_log(log_ctx, AV_LOG_WARNING,
+                "Color transfer function %s is not supported.\n",
+                av_color_transfer_name(src->color_trc));
+    }
 
     if (src->color_trc == AVCOL_TRC_GAMMA22)
         gamma = 2.2;
     else if (src->color_trc == AVCOL_TRC_GAMMA28)
         gamma = 2.8;
 
-    if (colormatrix) {
-        CVBufferSetAttachment(
-            pixbuf,
-            kCVImageBufferYCbCrMatrixKey,
-            colormatrix,
-            kCVAttachmentMode_ShouldPropagate);
-    }
-    if (colorpri) {
-        CVBufferSetAttachment(
-            pixbuf,
-            kCVImageBufferColorPrimariesKey,
-            colorpri,
-            kCVAttachmentMode_ShouldPropagate);
-    }
-    if (colortrc) {
-        CVBufferSetAttachment(
-            pixbuf,
-            kCVImageBufferTransferFunctionKey,
-            colortrc,
-            kCVAttachmentMode_ShouldPropagate);
-    }
     if (gamma != 0) {
         CFNumberRef gamma_level = CFNumberCreate(NULL, kCFNumberFloat32Type, &gamma);
-        CVBufferSetAttachment(
-            pixbuf,
-            kCVImageBufferGammaLevelKey,
-            gamma_level,
-            kCVAttachmentMode_ShouldPropagate);
+        CVBufferSetAttachment(pixbuf, kCVImageBufferGammaLevelKey,
+            gamma_level, kCVAttachmentMode_ShouldPropagate);
         CFRelease(gamma_level);
-    }
+    } else
+        CVBufferRemoveAttachment(pixbuf, kCVImageBufferGammaLevelKey);
 
     return 0;
 }

base-commit: fa3b153cb1eba0d68327c716842c9a38f95c1667
-- 
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] [PATCH v3 3/3] avutil/hwcontext_videotoolbox: Set CVBuffer CGColorSpace

[Marvin Scholz]

In addition to the other properties, try to obtain the right
CGColorSpace and set it as well, else it could lead to a CVBuffer
tagged as BT.2020 but with a CGColorSpace indicating BT.709.

Therefore it is essential for consistency to set a colorspace
according to the other values, or if none can be obtained (for example
because the other values are all unspecified) unset it as well.

Fix #10884
---
 libavutil/hwcontext_videotoolbox.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c
index 0af2ab822f..953155ce32 100644
--- a/libavutil/hwcontext_videotoolbox.c
+++ b/libavutil/hwcontext_videotoolbox.c
@@ -535,6 +535,7 @@ 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;
     CFStringRef colormatrix = NULL, colorpri = NULL, colortrc = NULL;
     Float32 gamma = 0;
 
@@ -587,6 +588,21 @@ static int vt_pixbuf_set_colorspace(void *log_ctx,
     } else
         CVBufferRemoveAttachment(pixbuf, kCVImageBufferGammaLevelKey);
 
+    if (__builtin_available(macOS 10.8, iOS 10, *)) {
+        CFDictionaryRef attachments = CVBufferCopyAttachments(pixbuf, kCVAttachmentMode_ShouldPropagate);
+        if (attachments) {
+            colorspace = CVImageBufferCreateColorSpaceFromAttachments(attachments);
+            CFRelease(attachments);
+        }
+    }
+
+    if (colorspace) {
+        CVBufferSetAttachment(pixbuf, kCVImageBufferCGColorSpaceKey,
+            colorspace, kCVAttachmentMode_ShouldPropagate);
+        CFRelease(colorspace);
+    } else
+        CVBufferRemoveAttachment(pixbuf, kCVImageBufferCGColorSpaceKey);
+
     return 0;
 }
 
-- 
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] [PATCH v3 2/3] avutil/hwcontext_videotoolbox: Update documentation

[Marvin Scholz]

The documentation was not clear at all what specifically the
function does, so it was left unspecified if it will unset or
not touch attachments it could not map from the AVFrame.

The documentation of the return  value was wrong as well.
---
 libavutil/hwcontext_videotoolbox.h | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libavutil/hwcontext_videotoolbox.h b/libavutil/hwcontext_videotoolbox.h
index 600e9f2c8d..d35cfbb6c1 100644
--- a/libavutil/hwcontext_videotoolbox.h
+++ b/libavutil/hwcontext_videotoolbox.h
@@ -90,8 +90,15 @@ CFStringRef av_map_videotoolbox_color_primaries_from_av(enum AVColorPrimaries pr
 CFStringRef av_map_videotoolbox_color_trc_from_av(enum AVColorTransferCharacteristic trc);
 
 /**
- * Update a CVPixelBufferRef's metadata to based on an AVFrame.
- * Returns 0 if no known equivalent was found.
+ * Set CVPixelBufferRef's metadata based on an AVFrame.
+ *
+ * Sets/unsets the CVPixelBuffer attachments to match as closely as possible the
+ * AVFrame metadata. To prevent inconsistent attachments, the attachments for properties
+ * that could not be matched or are unspecified in the given AVFrame are unset. So if
+ * any attachments already covered by AVFrame metadata need to be set to a specific
+ * value, this should happen after calling this function.
+ *
+ * Returns < 0 in case of an error.
  */
 int av_vt_pixbuf_set_attachments(void *log_ctx,
                                  CVPixelBufferRef pixbuf, const struct AVFrame *src);
-- 
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".