[FFmpeg-devel] [PATCH] Add android_capture indev

[Moritz Barsnick]

In addition to Nicolas's, further (style) comments:

> Subject: [PATCH] Add android_capture indev
More like:
    avdevice: add android_capture [or android_camera] indev

>  configure                     |   6 +
>  libavdevice/Makefile          |   1 +
>  libavdevice/alldevices.c      |   1 +
>  libavdevice/android_capture.c | 782 ++++++++++++++++++++++++++++++++++++++++++
>  libavdevice/android_capture.h |  77 +++++
>  5 files changed, 867 insertions(+)
>  create mode 100644 libavdevice/android_capture.c
>  create mode 100644 libavdevice/android_capture.h

A Changelog entry, documentation in doc/indevs.texi, and a libavdevice
version bump need to be added.

>  # indevs / outdevs
> +android_capture_indev_deps="android mediandk camera2ndk pthreads"
> +android_capture_indev_extralibs="-landroid -lmediandk -lcamera2ndk"
>  alsa_indev_deps="alsa"
>  alsa_outdev_deps="alsa"

Alphabetical order.

> +    av_log(avctx, AV_LOG_ERROR, "Error %d on camera with id %s.\n", error,
> +            ACameraDevice_getId(device));

As Nicolas suggested, can you stringify the error codes?

> +        av_log(avctx, AV_LOG_ERROR,
> +                "Failed to get camera id list, camera_status_t: %d.\n", ret);

camera_status_t seems like an internal naming. (Also media_status_t and
the likes.) Not sure whether other words would be better. Without
stringification, it's only useful to the developer anyway.

> +        av_log(avctx, AV_LOG_ERROR,
> +                "real-time buffer of [%s input] too full or near too full (%d%% of size: %d [rtbufsize parameter])! frame dropped!\n",
> +                stream_name, buffer_fullness, avctx->max_picture_buffer);

I (personally) mislike exclamation marks in messages.

> +    { "sample_rate", "set audio sample rate", OFFSET(sample_rate), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, DEC },
> +    { "sample_size", "set audio sample size", OFFSET(sample_size), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 16, DEC },
> +    { "channels", "set number of audio channels, such as 1 or 2", OFFSET(channels), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, DEC },
> +    { "audio_buffer_size", "set audio device buffer latency size in milliseconds (default is the device's default)", OFFSET(audio_buffer_size), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, DEC },

While these aren't being used, don't expose them. (Will disappear
anyway if you split the functionality.)

> +    .long_name      = NULL_IF_CONFIG_SMALL("Android camera/microphone input source via Android NDK APIs (Audio yet to be implemented)"),

Perhaps a bit verbose, but if you split it up into video and audio as
suggested by Nicolas, you can shorten it. (I'm not sure "via Android
NDK APIs" is the correct to present.)

Moritz