[FFmpeg-devel] [PATCH 1/2] lavc/mediacodecenc: Probe supported pixel formats

[FFmpeg-devel] [PATCH 1/2] lavc/mediacodecenc: Probe supported pixel formats

[Tomas Härdin]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: 0001-lavc-mediacodecenc-Probe-supported-pixel-formats.patch --]
[-- Type: text/x-patch, Size: 6063 bytes --]

From eb6d090967b8ed7ea0ee0651a1f557633fa23517 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
Date: Thu, 22 Dec 2022 13:29:58 +0100
Subject: [PATCH 1/2] lavc/mediacodecenc: Probe supported pixel formats

For each entry in color_formats[] an encoder is configured and opened.
If this succeeds then the corresponding pixel format is added to probed_pix_fmts[].

This patch has been released by Epic Games' legal department.
---
 libavcodec/mediacodecenc.c | 76 ++++++++++++++++++++++++++++++++++----
 1 file changed, 68 insertions(+), 8 deletions(-)

diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
index 4c1809093c..fd90d41625 100644
--- a/libavcodec/mediacodecenc.c
+++ b/libavcodec/mediacodecenc.c
@@ -2,6 +2,7 @@
  * Android MediaCodec encoders
  *
  * Copyright (c) 2022 Zhao Zhili <zhilizhao@tencent.com>
+ * Modifications by Epic Games, Inc., 2022.
  *
  * This file is part of FFmpeg.
  *
@@ -89,12 +90,8 @@ static const struct {
     { COLOR_FormatSurface,              AV_PIX_FMT_MEDIACODEC },
 };
 
-static const enum AVPixelFormat avc_pix_fmts[] = {
-    AV_PIX_FMT_MEDIACODEC,
-    AV_PIX_FMT_YUV420P,
-    AV_PIX_FMT_NV12,
-    AV_PIX_FMT_NONE
-};
+// filled in by mediacodec_init_static_data()
+static enum AVPixelFormat probed_pix_fmts[FF_ARRAY_ELEMS(color_formats)+1];
 
 static void mediacodec_output_format(AVCodecContext *avctx)
 {
@@ -534,6 +531,69 @@ static av_cold int mediacodec_close(AVCodecContext *avctx)
     return 0;
 }
 
+static av_cold void mediacodec_init_static_data(FFCodec *ffcodec)
+{
+    const char *codec_mime = ffcodec->p.id == AV_CODEC_ID_H264 ? "video/avc" : "video/hevc";
+    FFAMediaCodec *codec;
+    int num_pix_fmts = 0;
+    int use_ndk_codec = !av_jni_get_java_vm(NULL);
+
+    if (!(codec = ff_AMediaCodec_createEncoderByType(codec_mime, use_ndk_codec))) {
+        av_log(NULL, AV_LOG_ERROR, "Failed to create encoder for type %s\n", codec_mime);
+        return;
+    }
+
+    for (int i = 0; i < FF_ARRAY_ELEMS(color_formats); i++) {
+        if (color_formats[i].pix_fmt == AV_PIX_FMT_MEDIACODEC) {
+            // assumme AV_PIX_FMT_MEDIACODEC always works
+            // we don't have a context at this point with which to test it
+            probed_pix_fmts[num_pix_fmts++] = color_formats[i].pix_fmt;
+        } else {
+            FFAMediaFormat *format;
+            int ret;
+
+            if (!(format = ff_AMediaFormat_new(use_ndk_codec))) {
+                av_log(NULL, AV_LOG_ERROR, "Failed to create media format\n");
+                ff_AMediaCodec_delete(codec);
+                continue;
+            }
+
+            ff_AMediaFormat_setString(format, "mime", codec_mime);
+            ff_AMediaFormat_setInt32(format, "width", 1280);
+            ff_AMediaFormat_setInt32(format, "height", 720);
+            ff_AMediaFormat_setInt32(format, "color-format", color_formats[i].color_format);
+            ff_AMediaFormat_setInt32(format, "bitrate", 1000000);
+            ff_AMediaFormat_setInt32(format, "bitrate-mode", BITRATE_MODE_VBR);
+            ff_AMediaFormat_setInt32(format, "frame-rate", 30);
+            ff_AMediaFormat_setInt32(format, "i-frame-interval", 1);
+
+            // no need to set profile, level or number of B-frames it seems
+            ret = ff_AMediaCodec_getConfigureFlagEncode(codec);
+            ret = ff_AMediaCodec_configure(codec, format, NULL, NULL, ret);
+            if (ret) {
+                av_log(NULL, AV_LOG_ERROR, "MediaCodec configure failed, %s\n", av_err2str(ret));
+                goto bailout;
+            }
+
+            ret = ff_AMediaCodec_start(codec);
+            if (ret) {
+                av_log(NULL, AV_LOG_ERROR, "MediaCodec failed to start, %s\n", av_err2str(ret));
+                goto bailout;
+            }
+            ff_AMediaCodec_stop(codec);
+
+            probed_pix_fmts[num_pix_fmts++] = color_formats[i].pix_fmt;
+        bailout:
+            // format is never NULL here
+            ff_AMediaFormat_delete(format);
+        }
+    }
+
+    probed_pix_fmts[num_pix_fmts] = AV_PIX_FMT_NONE;
+    ffcodec->p.pix_fmts = probed_pix_fmts;
+    ff_AMediaCodec_delete(codec);
+}
+
 static const AVCodecHWConfigInternal *const mediacodec_hw_configs[] = {
     &(const AVCodecHWConfigInternal) {
         .public          = {
@@ -579,7 +639,7 @@ static const AVClass name ## _mediacodec_class = {  \
 
 #define DECLARE_MEDIACODEC_ENCODER(short_name, long_name, codec_id)     \
 MEDIACODEC_ENCODER_CLASS(short_name)                                    \
-const FFCodec ff_ ## short_name ## _mediacodec_encoder = {              \
+FFCodec ff_ ## short_name ## _mediacodec_encoder = {              \
     .p.name           = #short_name "_mediacodec",                      \
     CODEC_LONG_NAME(long_name " Android MediaCodec encoder"),           \
     .p.type           = AVMEDIA_TYPE_VIDEO,                             \
@@ -587,7 +647,6 @@ const FFCodec ff_ ## short_name ## _mediacodec_encoder = {              \
     .p.capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY           \
                         | AV_CODEC_CAP_HARDWARE,                        \
     .priv_data_size   = sizeof(MediaCodecEncContext),                   \
-    .p.pix_fmts       = avc_pix_fmts,                                   \
     .init             = mediacodec_init,                                \
     FF_CODEC_RECEIVE_PACKET_CB(mediacodec_encode),                      \
     .close            = mediacodec_close,                               \
@@ -595,6 +654,7 @@ const FFCodec ff_ ## short_name ## _mediacodec_encoder = {              \
     .caps_internal    = FF_CODEC_CAP_INIT_CLEANUP,                      \
     .p.wrapper_name = "mediacodec",                                     \
     .hw_configs     = mediacodec_hw_configs,                            \
+    .init_static_data = mediacodec_init_static_data,                    \
 };                                                                      \
 
 #if CONFIG_H264_MEDIACODEC_ENCODER
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 2/2] lavc/mediacodecenc: Probe actually supported color formats using JNI

[Tomas Härdin]

[-- Attachment #1: Type: text/plain, Size: 70 bytes --]

This should be faster than opening the encoder several times.

/Tomas

[-- Attachment #2: 0002-lavc-mediacodecenc-Probe-actually-supported-color-fo.patch --]
[-- Type: text/x-patch, Size: 8990 bytes --]

From 44c347f5a2b6160abd8fc09a0255ee21a7ba6ee4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
Date: Wed, 4 Jan 2023 16:27:26 +0100
Subject: [PATCH 2/2] lavc/mediacodecenc: Probe actually supported color
 formats using JNI

Fall back on trying to open the encoder if JNI doesn't work.

This patch has been released by Epic Games' legal department.
---
 libavcodec/mediacodec_wrapper.c | 93 +++++++++++++++++++++++++++++++++
 libavcodec/mediacodec_wrapper.h | 15 ++++++
 libavcodec/mediacodecenc.c      | 27 +++++++++-
 3 files changed, 134 insertions(+), 1 deletion(-)

diff --git a/libavcodec/mediacodec_wrapper.c b/libavcodec/mediacodec_wrapper.c
index 4d6e9487b8..ac77672013 100644
--- a/libavcodec/mediacodec_wrapper.c
+++ b/libavcodec/mediacodec_wrapper.c
@@ -2,6 +2,7 @@
  * Android MediaCodec Wrapper
  *
  * Copyright (c) 2015-2016 Matthieu Bouron <matthieu.bouron stupeflix.com>
+ * Modifications by Epic Games, Inc., 2023.
  *
  * This file is part of FFmpeg.
  *
@@ -176,6 +177,7 @@ struct JNIAMediaCodecFields {
     jmethodID release_id;
 
     jmethodID get_output_format_id;
+    jmethodID get_codec_info_id;
 
     jmethodID dequeue_input_buffer_id;
     jmethodID queue_input_buffer_id;
@@ -228,6 +230,7 @@ static const struct FFJniField jni_amediacodec_mapping[] = {
         { "android/media/MediaCodec", "release", "()V", FF_JNI_METHOD, offsetof(struct JNIAMediaCodecFields, release_id), 1 },
 
         { "android/media/MediaCodec", "getOutputFormat", "()Landroid/media/MediaFormat;", FF_JNI_METHOD, offsetof(struct JNIAMediaCodecFields, get_output_format_id), 1 },
+        { "android/media/MediaCodec", "getCodecInfo", "()Landroid/media/MediaCodecInfo;", FF_JNI_METHOD, offsetof(struct JNIAMediaCodecFields, get_codec_info_id), 1 },
 
         { "android/media/MediaCodec", "dequeueInputBuffer", "(J)I", FF_JNI_METHOD, offsetof(struct JNIAMediaCodecFields, dequeue_input_buffer_id), 1 },
         { "android/media/MediaCodec", "queueInputBuffer", "(IIIJI)V", FF_JNI_METHOD, offsetof(struct JNIAMediaCodecFields, queue_input_buffer_id), 1 },
@@ -604,6 +607,96 @@ done:
     return name;
 }
 
+int ff_AMediaCodec_color_formats_intersect(FFAMediaCodec *codec, const char *mime,
+                                           const int *in_formats, int nin_formats,
+                                           int *out_formats, void *log_ctx)
+{
+    FFAMediaCodecJni *jni_codec = (FFAMediaCodecJni*)codec;
+    jobject info = NULL;
+    jobject capabilities = NULL;
+    JNIEnv *env = NULL;
+    jintArray color_formats = NULL;
+    int color_count, ret, *elems;
+    jboolean is_copy;
+    jstring jmime = NULL;
+    struct JNIAMediaCodecFields jfields = { 0 };
+    struct JNIAMediaCodecListFields jfields2 = { 0 };
+
+    // make sure we have a JVM
+    JNI_GET_ENV_OR_RETURN(env, log_ctx, -1);
+
+    // grab mappings
+    if ((ret = ff_jni_init_jfields(env, &jfields, jni_amediacodec_mapping, 0, log_ctx)) < 0) {
+        goto done;
+    }
+
+    if ((ret = ff_jni_init_jfields(env, &jfields2, jni_amediacodeclist_mapping, 0, log_ctx)) < 0) {
+        goto done;
+    }
+
+    // codec.getCodecInfo().getCapabilitiesForType(mime).colorFormats
+    ret = -1;
+    info = (*env)->CallObjectMethod(env, jni_codec->object, jfields.get_codec_info_id);
+    if (ff_jni_exception_check(env, 1, log_ctx) < 0) {
+        goto done;
+    }
+
+    // convert const char* to java.lang.String
+    jmime = ff_jni_utf_chars_to_jstring(env, mime, log_ctx);
+    if (!jmime) {
+        goto done;
+    }
+
+    capabilities = (*env)->CallObjectMethod(env, info, jfields2.get_codec_capabilities_id, jmime);
+    if (ff_jni_exception_check(env, 1, log_ctx) < 0) {
+        goto done;
+    }
+
+    color_formats = (*env)->GetObjectField(env, capabilities, jfields2.color_formats_id);
+    if (ff_jni_exception_check(env, 1, log_ctx) < 0) {
+        goto done;
+    }
+
+    color_count = (*env)->GetArrayLength(env, color_formats);
+    elems = (*env)->GetIntArrayElements(env, color_formats, &is_copy);
+    ret = 0;
+
+    // out_formats = intersect(in_formats, elems)
+    for (int i = 0; i < nin_formats; i++) {
+        for (int j = 0; j < color_count; j++) {
+            if (elems[j] == in_formats[i]) {
+                out_formats[ret++] = in_formats[i];
+                break;
+            }
+        }
+    }
+
+    (*env)->ReleaseIntArrayElements(env, color_formats, elems, JNI_ABORT);
+
+done:
+    // clean up
+    if (jmime) {
+        (*env)->DeleteLocalRef(env, jmime);
+    }
+
+    if (color_formats) {
+        (*env)->DeleteLocalRef(env, color_formats);
+    }
+
+    if (capabilities) {
+        (*env)->DeleteLocalRef(env, capabilities);
+    }
+
+    if (info) {
+        (*env)->DeleteLocalRef(env, info);
+    }
+
+    ff_jni_reset_jfields(env, &jfields, jni_amediacodec_mapping, 0, log_ctx);
+    ff_jni_reset_jfields(env, &jfields2, jni_amediacodeclist_mapping, 0, log_ctx);
+
+    return ret;
+}
+
 static FFAMediaFormat *mediaformat_jni_new(void)
 {
     JNIEnv *env = NULL;
diff --git a/libavcodec/mediacodec_wrapper.h b/libavcodec/mediacodec_wrapper.h
index 1b81e6db84..f0427f3287 100644
--- a/libavcodec/mediacodec_wrapper.h
+++ b/libavcodec/mediacodec_wrapper.h
@@ -2,6 +2,7 @@
  * Android MediaCodec Wrapper
  *
  * Copyright (c) 2015-2016 Matthieu Bouron <matthieu.bouron stupeflix.com>
+ * Modifications by Epic Games, Inc., 2023.
  *
  * This file is part of FFmpeg.
  *
@@ -230,6 +231,20 @@ FFAMediaCodec* ff_AMediaCodec_createCodecByName(const char *name, int ndk);
 FFAMediaCodec* ff_AMediaCodec_createDecoderByType(const char *mime_type, int ndk);
 FFAMediaCodec* ff_AMediaCodec_createEncoderByType(const char *mime_type, int ndk);
 
+/**
+ * Intersects the given list of color formats with the formats actually supported by the device.
+ * @param[in]  codec        The codec to query
+ * @param[in]  mime         MIME format. Typically "video/avc" or "video/hevc"
+ * @param[in]  in_formats   Array of input color formats
+ * @param[in]  nin_formats  Number of elements in in_formats
+ * @param[out] out_formats  Computed intersection of in_formats and supported formats
+ * @param[in]  log_ctx      Logging context
+ * @return Size of out_formats or negative in case of error
+ */
+int ff_AMediaCodec_color_formats_intersect(FFAMediaCodec *codec, const char *mime,
+                                           const int *in_formats, int nin_formats,
+                                           int *out_formats, void *log_ctx);
+
 static inline int ff_AMediaCodec_configure(FFAMediaCodec *codec,
                                            const FFAMediaFormat *format,
                                            FFANativeWindow *surface,
diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
index fd90d41625..2185e0b5ab 100644
--- a/libavcodec/mediacodecenc.c
+++ b/libavcodec/mediacodecenc.c
@@ -90,6 +90,12 @@ static const struct {
     { COLOR_FormatSurface,              AV_PIX_FMT_MEDIACODEC },
 };
 
+static const int in_formats[] = {
+    COLOR_FormatYUV420Planar,
+    COLOR_FormatYUV420SemiPlanar,
+    COLOR_FormatSurface,
+};
+
 // filled in by mediacodec_init_static_data()
 static enum AVPixelFormat probed_pix_fmts[FF_ARRAY_ELEMS(color_formats)+1];
 
@@ -535,7 +541,7 @@ static av_cold void mediacodec_init_static_data(FFCodec *ffcodec)
 {
     const char *codec_mime = ffcodec->p.id == AV_CODEC_ID_H264 ? "video/avc" : "video/hevc";
     FFAMediaCodec *codec;
-    int num_pix_fmts = 0;
+    int num_pix_fmts, out_formats[FF_ARRAY_ELEMS(in_formats)];
     int use_ndk_codec = !av_jni_get_java_vm(NULL);
 
     if (!(codec = ff_AMediaCodec_createEncoderByType(codec_mime, use_ndk_codec))) {
@@ -543,6 +549,24 @@ static av_cold void mediacodec_init_static_data(FFCodec *ffcodec)
         return;
     }
 
+    // attempt to query color formats using JNI
+    // fall back on NDK-compatible method below if this fails
+    if ((num_pix_fmts = ff_AMediaCodec_color_formats_intersect(
+            codec, codec_mime, in_formats, FF_ARRAY_ELEMS(in_formats),
+            out_formats, NULL)) >= 0) {
+        // translate color formats to pixel formats
+        for (int i = 0; i < num_pix_fmts; i++) {
+            for (int j = 0; j < FF_ARRAY_ELEMS(color_formats); j++) {
+                if (out_formats[i] == color_formats[j].color_format) {
+                    probed_pix_fmts[i] = color_formats[j].pix_fmt;
+                    break;
+                }
+            }
+        }
+        goto done;
+    }
+
+    num_pix_fmts = 0;
     for (int i = 0; i < FF_ARRAY_ELEMS(color_formats); i++) {
         if (color_formats[i].pix_fmt == AV_PIX_FMT_MEDIACODEC) {
             // assumme AV_PIX_FMT_MEDIACODEC always works
@@ -589,6 +613,7 @@ static av_cold void mediacodec_init_static_data(FFCodec *ffcodec)
         }
     }
 
+done:
     probed_pix_fmts[num_pix_fmts] = AV_PIX_FMT_NONE;
     ffcodec->p.pix_fmts = probed_pix_fmts;
     ff_AMediaCodec_delete(codec);
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

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

Re: [FFmpeg-devel] [PATCH 1/2] lavc/mediacodecenc: Probe supported pixel formats

[Zhao Zhili]

On Fri, 2023-01-06 at 16:42 +0100, Tomas Härdin wrote:


> For each entry in color_formats[] an encoder is configured and opened.
> If this succeeds then the corresponding pixel format is added to probed_pix_fmts[].
> 
> This patch has been released by Epic Games' legal department.
> ---
>  libavcodec/mediacodecenc.c | 76 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 68 insertions(+), 8 deletions(-)
> 
> diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
> index 4c1809093c..fd90d41625 100644
> --- a/libavcodec/mediacodecenc.c
> +++ b/libavcodec/mediacodecenc.c
> @@ -2,6 +2,7 @@
>   * Android MediaCodec encoders
>   *
>   * Copyright (c) 2022 Zhao Zhili <zhilizhao@tencent.com>
> + * Modifications by Epic Games, Inc., 2022.
>   *
>   * This file is part of FFmpeg.
>   *
> @@ -89,12 +90,8 @@ static const struct {
>      { COLOR_FormatSurface,              AV_PIX_FMT_MEDIACODEC },
>  };
>  
> -static const enum AVPixelFormat avc_pix_fmts[] = {
> -    AV_PIX_FMT_MEDIACODEC,
> -    AV_PIX_FMT_YUV420P,
> -    AV_PIX_FMT_NV12,
> -    AV_PIX_FMT_NONE
> -};
> +// filled in by mediacodec_init_static_data()
> +static enum AVPixelFormat probed_pix_fmts[FF_ARRAY_ELEMS(color_formats)+1];
>  
>  static void mediacodec_output_format(AVCodecContext *avctx)
>  {
> @@ -534,6 +531,69 @@ static av_cold int mediacodec_close(AVCodecContext *avctx)
>      return 0;
>  }
>  
> +static av_cold void mediacodec_init_static_data(FFCodec *ffcodec)
> +{
> +    const char *codec_mime = ffcodec->p.id == AV_CODEC_ID_H264 ? "video/avc" : "video/hevc";
> +    FFAMediaCodec *codec;
> +    int num_pix_fmts = 0;
> +    int use_ndk_codec = !av_jni_get_java_vm(NULL);
> +
> +    if (!(codec = ff_AMediaCodec_createEncoderByType(codec_mime, use_ndk_codec))) {
> +        av_log(NULL, AV_LOG_ERROR, "Failed to create encoder for type %s\n", codec_mime);
> +        return;
> +    }
> +
> +    for (int i = 0; i < FF_ARRAY_ELEMS(color_formats); i++) {
> +        if (color_formats[i].pix_fmt == AV_PIX_FMT_MEDIACODEC) {
> +            // assumme AV_PIX_FMT_MEDIACODEC always works
> +            // we don't have a context at this point with which to test it
> +            probed_pix_fmts[num_pix_fmts++] = color_formats[i].pix_fmt;
> +        } else {
> +            FFAMediaFormat *format;
> +            int ret;
> +
> +            if (!(format = ff_AMediaFormat_new(use_ndk_codec))) {
> +                av_log(NULL, AV_LOG_ERROR, "Failed to create media format\n");
> +                ff_AMediaCodec_delete(codec);
> +                continue;

Here is a use-after-free (codec) issue.

> +            }
> +
> +            ff_AMediaFormat_setString(format, "mime", codec_mime);
> +            ff_AMediaFormat_setInt32(format, "width", 1280);
> +            ff_AMediaFormat_setInt32(format, "height", 720);
> +            ff_AMediaFormat_setInt32(format, "color-format", color_formats[i].color_format);
> +            ff_AMediaFormat_setInt32(format, "bitrate", 1000000);
> +            ff_AMediaFormat_setInt32(format, "bitrate-mode", BITRATE_MODE_VBR);
> +            ff_AMediaFormat_setInt32(format, "frame-rate", 30);
> +            ff_AMediaFormat_setInt32(format, "i-frame-interval", 1);
> +
> +            // no need to set profile, level or number of B-frames it seems
> +            ret = ff_AMediaCodec_getConfigureFlagEncode(codec);
> +            ret = ff_AMediaCodec_configure(codec, format, NULL, NULL, ret);
> +            if (ret) {
> +                av_log(NULL, AV_LOG_ERROR, "MediaCodec configure failed, %s\n", av_err2str(ret));
> +                goto bailout;
> +            }
> +
> +            ret = ff_AMediaCodec_start(codec);
> +            if (ret) {
> +                av_log(NULL, AV_LOG_ERROR, "MediaCodec failed to start, %s\n", av_err2str(ret));
> +                goto bailout;
> +            }
> +            ff_AMediaCodec_stop(codec);
> +
> +            probed_pix_fmts[num_pix_fmts++] = color_formats[i].pix_fmt;
> +        bailout:
> +            // format is never NULL here
> +            ff_AMediaFormat_delete(format);
> +        }
> +    }
> +
> +    probed_pix_fmts[num_pix_fmts] = AV_PIX_FMT_NONE;
> +    ffcodec->p.pix_fmts = probed_pix_fmts;
> +    ff_AMediaCodec_delete(codec);
> +}
> +
>  static const AVCodecHWConfigInternal *const mediacodec_hw_configs[] = {
>      &(const AVCodecHWConfigInternal) {
>          .public          = {
> @@ -579,7 +639,7 @@ static const AVClass name ## _mediacodec_class = {  \
>  
>  #define DECLARE_MEDIACODEC_ENCODER(short_name, long_name, codec_id)     \
>  MEDIACODEC_ENCODER_CLASS(short_name)                                    \
> -const FFCodec ff_ ## short_name ## _mediacodec_encoder = {              \
> +FFCodec ff_ ## short_name ## _mediacodec_encoder = {              \
>      .p.name           = #short_name "_mediacodec",                      \
>      CODEC_LONG_NAME(long_name " Android MediaCodec encoder"),           \
>      .p.type           = AVMEDIA_TYPE_VIDEO,                             \
> @@ -587,7 +647,6 @@ const FFCodec ff_ ## short_name ## _mediacodec_encoder = {              \
>      .p.capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY           \
>                          | AV_CODEC_CAP_HARDWARE,                        \
>      .priv_data_size   = sizeof(MediaCodecEncContext),                   \
> -    .p.pix_fmts       = avc_pix_fmts,                                   \
>      .init             = mediacodec_init,                                \
>      FF_CODEC_RECEIVE_PACKET_CB(mediacodec_encode),                      \
>      .close            = mediacodec_close,                               \
> @@ -595,6 +654,7 @@ const FFCodec ff_ ## short_name ## _mediacodec_encoder = {              \
>      .caps_internal    = FF_CODEC_CAP_INIT_CLEANUP,                      \
>      .p.wrapper_name = "mediacodec",                                     \
>      .hw_configs     = mediacodec_hw_configs,                            \
> +    .init_static_data = mediacodec_init_static_data,                    \
>  };                                                                      \
>  
>  #if CONFIG_H264_MEDIACODEC_ENCODER
> -- 
> 2.30.2
> 

init_static_data should be determinate, no matter when it was called, it should
give the same result. In addition to the 'different MediaCodec backends support
different pixel format' issue, another concern of this method is that it's not
determinate, it can give different results at different time/condition.

MediaCodec can fail for all kinds of reasons, and it can fail dynamically. For
example, the supported instance is limited (getMaxSupportedInstances()). Some
low end/legacy chip only support one instance. So it can fail when another app
or another SDK in the same app has already created a codec instance. It can
fail when out of other resouce (like GPU memory) temporarily. Since
init_static_data() only being called once, there is no way to recover from a
temporary failure.

If there is no other reason, stick to AV_PIX_FMT_MEDIACODEC/AV_PIX_FMT_NV12
should be safe.

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

Re: [FFmpeg-devel] [PATCH 1/2] lavc/mediacodecenc: Probe supported pixel formats

[Tomas Härdin]

lör 2023-01-07 klockan 02:03 +0800 skrev Zhao Zhili:
> On Fri, 2023-01-06 at 16:42 +0100, Tomas Härdin wrote:
> 
> > +            if (!(format = ff_AMediaFormat_new(use_ndk_codec))) {
> > +                av_log(NULL, AV_LOG_ERROR, "Failed to create media
> > format\n");
> > +                ff_AMediaCodec_delete(codec);
> > +                continue;
> 
> Here is a use-after-free (codec) issue.

Oops. I had the call to ff_AMediaCodec_createEncoderByType() inside the
loop earlier. Will fix.


> init_static_data should be determinate, no matter when it was called,
> it should
> give the same result.

You mean across devices? That obviously won't work. On the same device?
I would assume it always returns the same results, modulo what you
wrote below.

> In addition to the 'different MediaCodec backends support
> different pixel format' issue, another concern of this method is that
> it's not
> determinate, it can give different results at different
> time/condition.
> 
> MediaCodec can fail for all kinds of reasons, and it can fail
> dynamically. For
> example, the supported instance is limited
> (getMaxSupportedInstances()). Some
> low end/legacy chip only support one instance. So it can fail when
> another app
> or another SDK in the same app has already created a codec instance.

Won't the encoder fail anyway in that case? Also will the JNI probe
still fail in that case?

> It can
> fail when out of other resouce (like GPU memory) temporarily. Since
> init_static_data() only being called once, there is no way to recover
> from a
> temporary failure.

Well, the code can try to probe the color formats more than once inside
the function. But that feels very wrong.

/Tomas

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

Re: [FFmpeg-devel] [PATCH 1/2] lavc/mediacodecenc: Probe supported pixel formats

[Zhao Zhili]

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Tomas Härdin
> Sent: 2023年1月7日 2:22
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavc/mediacodecenc: Probe supported pixel formats
> 
> lör 2023-01-07 klockan 02:03 +0800 skrev Zhao Zhili:
> > On Fri, 2023-01-06 at 16:42 +0100, Tomas Härdin wrote:
> >
> > > +            if (!(format = ff_AMediaFormat_new(use_ndk_codec))) {
> > > +                av_log(NULL, AV_LOG_ERROR, "Failed to create media
> > > format\n");
> > > +                ff_AMediaCodec_delete(codec);
> > > +                continue;
> >
> > Here is a use-after-free (codec) issue.
> 
> Oops. I had the call to ff_AMediaCodec_createEncoderByType() inside the
> loop earlier. Will fix.
> 
> 
> > init_static_data should be determinate, no matter when it was called,
> > it should
> > give the same result.
> 
> You mean across devices? That obviously won't work. On the same device?
> I would assume it always returns the same results, modulo what you
> wrote below.

I mean on the same device.

> 
> > In addition to the 'different MediaCodec backends support
> > different pixel format' issue, another concern of this method is that
> > it's not
> > determinate, it can give different results at different
> > time/condition.
> >
> > MediaCodec can fail for all kinds of reasons, and it can fail
> > dynamically. For
> > example, the supported instance is limited
> > (getMaxSupportedInstances()). Some
> > low end/legacy chip only support one instance. So it can fail when
> > another app
> > or another SDK in the same app has already created a codec instance.
> 
> Won't the encoder fail anyway in that case? Also will the JNI probe
> still fail in that case?

Create encoder can fail, and recover after a while. If JNI probe depends on
a codec instance, it has the same issue. Use the codeclist API should be fine,
but then we don't know which omx codec is the default one.

The JNI probe can tell which pixel format is supported after encoder configure
failed at runtime. That's a safe and valid usecase. And it can be implemented
out side of FFmpeg, so it can be called earlier and multiple times, without the
limitation of init_static_data() being called only once.

> 
> > It can
> > fail when out of other resouce (like GPU memory) temporarily. Since
> > init_static_data() only being called once, there is no way to recover
> > from a
> > temporary failure.
> 
> Well, the code can try to probe the color formats more than once inside
> the function. But that feels very wrong.

That's the problem, init_static_data() can't do retry by design. Probe multiple
times inside init_static_data() doesn't work, unless there is a sleep after each
loop, and we can't put sleep inside init_static_data().

> 
> /Tomas
> 
> _______________________________________________
> 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".

Re: [FFmpeg-devel] [PATCH 1/2] lavc/mediacodecenc: Probe supported pixel formats

[Tomas Härdin]

> > > In addition to the 'different MediaCodec backends support
> > > different pixel format' issue, another concern of this method is
> > > that
> > > it's not
> > > determinate, it can give different results at different
> > > time/condition.
> > > 
> > > MediaCodec can fail for all kinds of reasons, and it can fail
> > > dynamically. For
> > > example, the supported instance is limited
> > > (getMaxSupportedInstances()). Some
> > > low end/legacy chip only support one instance. So it can fail
> > > when
> > > another app
> > > or another SDK in the same app has already created a codec
> > > instance.
> > 
> > Won't the encoder fail anyway in that case? Also will the JNI probe
> > still fail in that case?
> 
> Create encoder can fail, and recover after a while. If JNI probe
> depends on
> a codec instance, it has the same issue. Use the codeclist API should
> be fine,
> but then we don't know which omx codec is the default one.

I did see the codeclist API but using a codec instance seemed simpler
than replicating much of ff_AMediaCodecList_getCodecNameByType().

Is omx used only for software decoders or does it also handle hardware
ones?

I feel it bears pointing out again that for users in the US having the
software codecs is also sometimes desirable.


> > > It can
> > > fail when out of other resouce (like GPU memory) temporarily.
> > > Since
> > > init_static_data() only being called once, there is no way to
> > > recover
> > > from a
> > > temporary failure.
> > 
> > Well, the code can try to probe the color formats more than once
> > inside
> > the function. But that feels very wrong.
> 
> That's the problem, init_static_data() can't do retry by design.
> Probe multiple
> times inside init_static_data() doesn't work, unless there is a sleep
> after each
> loop, and we can't put sleep inside init_static_data().

Indeed that would be very wrong. Is there some sort of mutex we can
instruct users to lock before running ffmpeg?

/Tomas

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