Re: [FFmpeg-devel] [PATCH v2 2/6] avformat: add Android content resolver protocol support

[Zhao Zhili <quinkblack@foxmail.com>]

Sorry for the long delay of review.

> On Mar 5, 2024, at 03:37, Matthieu Bouron <matthieu.bouron@gmail.com> wrote:
> 
> On Tue, Feb 27, 2024 at 03:50:38PM +0100, Matthieu Bouron wrote:
>> Handles Android content-uri starting with content://.
>> ---
> 
> [...]
> 

> Handles Android content-uri starting with content://.
> ---
>  configure               |   2 +
>  doc/APIchanges          |   3 +
>  libavcodec/jni.c        |   1 +
>  libavformat/Makefile    |   1 +
>  libavformat/file.c      | 157 ++++++++++++++++++++++++++++++++++++++++
>  libavformat/protocols.c |   1 +
>  6 files changed, 165 insertions(+)
> 
> diff --git a/configure b/configure
> index bb5e630bad..790a1df7c8 100755
> --- a/configure
> +++ b/configure
> @@ -3655,6 +3655,8 @@ xcbgrab_indev_suggest="libxcb_shm libxcb_shape libxcb_xfixes"
>  xv_outdev_deps="xlib_xv xlib_x11 xlib_xext"
>  
>  # protocols
> +android_content_protocol_deps="jni"
> +android_content_protocol_select="file_protocol"
>  async_protocol_deps="threads"
>  bluray_protocol_deps="libbluray"
>  ffrtmpcrypt_protocol_conflict="librtmp_protocol"
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 10f6667e9e..258e953ca6 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
>  
>  API changes, most recent first:
>  
> +2024-02-xx - xxxxxxxxxx - lavc 60.41.100 - jni.h
> +  Add av_jni_set_android_app_ctx() and av_jni_get_android_app_ctx().

Should be added together with patch 1/6.

> +
>  2024-02-26 - xxxxxxxxxx - lavf 60.22.101 - avformat.h
>    AV_DISPOSITION_DEPENDENT may now also be used for video streams
>    intended to be merged with other video streams for presentation.
> diff --git a/libavcodec/jni.c b/libavcodec/jni.c
> index cfe95bd1ec..1193c608c3 100644
> --- a/libavcodec/jni.c
> +++ b/libavcodec/jni.c
> @@ -35,6 +35,7 @@
>  #include "ffjni.h"
>  
>  static void *java_vm;
> +static void *android_app_ctx;
>  static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
>  
>  int av_jni_set_java_vm(void *vm, void *log_ctx)
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 4a380668bd..08fe98a535 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -657,6 +657,7 @@ OBJS-$(CONFIG_LIBOPENMPT_DEMUXER)        += libopenmpt.o
>  OBJS-$(CONFIG_VAPOURSYNTH_DEMUXER)       += vapoursynth.o
>  
>  # protocols I/O
> +OBJS-$(CONFIG_ANDROID_CONTENT_PROTOCOL)  += file.o
>  OBJS-$(CONFIG_ASYNC_PROTOCOL)            += async.o
>  OBJS-$(CONFIG_APPLEHTTP_PROTOCOL)        += hlsproto.o
>  OBJS-$(CONFIG_BLURAY_PROTOCOL)           += bluray.o
> diff --git a/libavformat/file.c b/libavformat/file.c
> index 64df7ff6fb..1b2b69f090 100644
> --- a/libavformat/file.c
> +++ b/libavformat/file.c
> @@ -40,6 +40,12 @@
>  #include <stdlib.h>
>  #include "os_support.h"
>  #include "url.h"
> +#if CONFIG_ANDROID_CONTENT_PROTOCOL
> +#include <jni.h>
> +#include "libavcodec/jni.h"
> +#include "libavcodec/ffjni.c"
> +#endif
> +
>  
>  /* Some systems may not have S_ISFIFO */
>  #ifndef S_ISFIFO
> @@ -101,6 +107,21 @@ typedef struct FileContext {
>      int64_t initial_pos;
>  } FileContext;
>  
> +
> +#if CONFIG_ANDROID_CONTENT_PROTOCOL
> +static const AVOption android_content_options[] = {
> +    { "blocksize", "set I/O operation maximum block size", offsetof(FileContext, blocksize), AV_OPT_TYPE_INT, { .i64 = INT_MAX }, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM },
> +    { NULL }
> +};
> +
> +static const AVClass android_content_class = {
> +    .class_name = "android_content",
> +    .item_name  = av_default_item_name,
> +    .option     = android_content_options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};
> +#endif
> +
>  static const AVOption file_options[] = {
>      { "truncate", "truncate existing files on write", offsetof(FileContext, trunc), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, AV_OPT_FLAG_ENCODING_PARAM },
>      { "blocksize", "set I/O operation maximum block size", offsetof(FileContext, blocksize), AV_OPT_TYPE_INT, { .i64 = INT_MAX }, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM },
> @@ -524,3 +545,139 @@ const URLProtocol ff_fd_protocol = {
>  };
>  
>  #endif /* CONFIG_FD_PROTOCOL */
> +
> +#if CONFIG_ANDROID_CONTENT_PROTOCOL
> +
> +struct JFields {
> +    jclass uri_class;
> +    jmethodID parse_id;
> +
> +    jclass context_class;
> +    jmethodID get_content_resolver_id;
> +
> +    jclass content_resolver_class;
> +    jmethodID open_file_descriptor_id;
> +
> +    jclass parcel_file_descriptor_class;
> +    jmethodID detach_fd_id;
> +};

typedef struct is the preferred coding style of FFmpeg, it’s mentioned in
developer.texi.

> +
> +#define OFFSET(x) offsetof(struct JFields, x)
> +static const struct FFJniField jfields_mapping[] = {
> +    { "android/net/Uri", NULL, NULL, FF_JNI_CLASS, OFFSET(uri_class), 1 },
> +        { "android/net/Uri", "parse", "(Ljava/lang/String;)Landroid/net/Uri;", FF_JNI_STATIC_METHOD, OFFSET(parse_id), 1 },
> +
> +    { "android/content/Context", NULL, NULL, FF_JNI_CLASS, OFFSET(context_class), 1 },
> +        { "android/content/Context", "getContentResolver", "()Landroid/content/ContentResolver;", FF_JNI_METHOD, OFFSET(get_content_resolver_id), 1 },
> +
> +    { "android/content/ContentResolver", NULL, NULL, FF_JNI_CLASS, OFFSET(content_resolver_class), 1 },
> +        { "android/content/ContentResolver", "openFileDescriptor", "(Landroid/net/Uri;Ljava/lang/String;)Landroid/os/ParcelFileDescriptor;", FF_JNI_METHOD, OFFSET(open_file_descriptor_id), 1 },
> +
> +    { "android/os/ParcelFileDescriptor", NULL, NULL, FF_JNI_CLASS, OFFSET(parcel_file_descriptor_class), 1 },
> +        { "android/os/ParcelFileDescriptor", "detachFd", "()I", FF_JNI_METHOD, OFFSET(detach_fd_id), 1 },
> +
> +    { NULL }
> +};
> +#undef OFFSET
> +
> +static int android_content_open(URLContext *h, const char *filename, int flags)
> +{
> +    FileContext *c = h->priv_data;
> +    int fd, ret;
> +    const char *mode_str = "r";
> +
> +    JNIEnv *env;
> +    struct JFields jfields = { 0 };
> +    jobject application_context = NULL;
> +    jobject url = NULL;
> +    jobject mode = NULL;
> +    jobject uri = NULL;
> +    jobject content_resolver = NULL;
> +    jobject parcel_file_descriptor = NULL;
> +
> +    env = ff_jni_get_env(c);
> +    if (!env) {
> +        return AVERROR(EINVAL);
> +    }
> +
> +    ret = ff_jni_init_jfields(env, &jfields, jfields_mapping, 0, c);
> +    if (ret < 0) {
> +        av_log(c, AV_LOG_ERROR, "failed to initialize jni fields\n");
> +        return ret;
> +    }
> +
> +    application_context = av_jni_get_android_app_ctx();
> +    if (!application_context) {
> +        av_log(c, AV_LOG_ERROR, "application context is not set\n");
> +        ret = AVERROR_EXTERNAL;
> +        goto done;
> +    }
> +
> +    url = ff_jni_utf_chars_to_jstring(env, filename, c);
> +    if (!url) {
> +        ret = AVERROR_EXTERNAL;
> +        goto done;
> +    }
> +
> +    if (flags & AVIO_FLAG_WRITE && flags & AVIO_FLAG_READ)
> +        mode_str = "rw";
> +    else if (flags & AVIO_FLAG_WRITE)
> +        mode_str = "w";
> +
> +    mode = ff_jni_utf_chars_to_jstring(env, mode_str, c);
> +    if (!mode) {
> +        ret = AVERROR_EXTERNAL;
> +        goto done;
> +    }
> +
> +    uri = (*env)->CallStaticObjectMethod(env, jfields.uri_class, jfields.parse_id, url);
> +    ret = ff_jni_exception_check(env, 1, c);
> +    if (ret < 0)
> +        goto done;
> +
> +    content_resolver = (*env)->CallObjectMethod(env, application_context, jfields.get_content_resolver_id);
> +    ret = ff_jni_exception_check(env, 1, c);
> +    if (ret < 0)
> +        goto done;
> +
> +    parcel_file_descriptor = (*env)->CallObjectMethod(env, content_resolver, jfields.open_file_descriptor_id, uri, mode);
> +    ret = ff_jni_exception_check(env, 1, c);
> +    if (ret < 0)
> +        goto done;
> +
> +    fd = (*env)->CallIntMethod(env, parcel_file_descriptor, jfields.detach_fd_id);
> +    ret = ff_jni_exception_check(env, 1, c);
> +    if (ret < 0)
> +        goto done;
> +
> +#if HAVE_SETMODE
> +    setmode(fd, O_BINARY);
> +#endif

Since it’s android specific protocol, the conditional compile is always false, isn’t it?

> +    c->fd = fd;
> +    h->is_streamed = 0;

Firstly, is_streamed is default to false, so the above line is unnecessary.
Secondly, I’m not sure whether it’s always seeable.

> +
> +done:
> +    (*env)->DeleteLocalRef(env, url);
> +    (*env)->DeleteLocalRef(env, mode);
> +    (*env)->DeleteLocalRef(env, uri);
> +    (*env)->DeleteLocalRef(env, content_resolver);
> +    (*env)->DeleteLocalRef(env, parcel_file_descriptor);
> +    ff_jni_reset_jfields(env, &jfields, jfields_mapping, 0, c);
> +
> +    return ret;
> +}
> +
> +URLProtocol ff_android_content_protocol = {
> +    .name                = "content",
> +    .url_open            = android_content_open,
> +    .url_read            = file_read,
> +    .url_write           = file_write,
> +    .url_seek            = file_seek,
> +    .url_close           = file_close,
> +    .url_get_file_handle = file_get_handle,
> +    .url_check           = NULL,
> +    .priv_data_size      = sizeof(FileContext),
> +    .priv_data_class     = &android_content_class,
> +};
> +
> +#endif /* CONFIG_ANDROID_CONTENT_PROTOCOL */
> diff --git a/libavformat/protocols.c b/libavformat/protocols.c
> index 360018b17c..93a6d67261 100644
> --- a/libavformat/protocols.c
> +++ b/libavformat/protocols.c
> @@ -24,6 +24,7 @@
>  
>  #include "url.h"
>  
> +extern const URLProtocol ff_android_content_protocol;
>  extern const URLProtocol ff_async_protocol;
>  extern const URLProtocol ff_bluray_protocol;
>  extern const URLProtocol ff_cache_protocol;
> -- 
> 2.44.0
> 

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