[FFmpeg-devel] [PATCH] Add android_capture indev

[Michael Niedermayer]

On Fri, Nov 10, 2017 at 09:41:04PM +0100, Felix Matouschek wrote:
> Hello,
> 
> here is a new version of the patch with further fixes.
[...]

> +static const char *camera_status_string(camera_status_t val)
> +{
> +    switch(val) {
> +        case ACAMERA_OK:
> +            return "ACAMERA_OK";

if the identifer and the string always match you could do this
with a macro avoiding the neede to duplcate each string
see AV_STRINGIFY


[...]
> +static int read_closing(AndroidCameraCtx *ctx)
> +{
> +    int read_closing;
> +    pthread_mutex_lock(&ctx->read_closing_mutex);
> +    read_closing = ctx->read_closing;
> +    pthread_mutex_unlock(&ctx->read_closing_mutex);
> +    return read_closing;
> +}
> +
> +static void set_read_closing(AndroidCameraCtx *ctx, int read_closing)
> +{
> +    pthread_mutex_lock(&ctx->read_closing_mutex);
> +    ctx->read_closing = read_closing;
> +    pthread_mutex_unlock(&ctx->read_closing_mutex);
> +}

Thic can be simplified using C11 atomics


[...]
> +static void match_video_size(AVFormatContext *avctx)
> +{
> +    AndroidCameraCtx *ctx = avctx->priv_data;
> +    ACameraMetadata_const_entry available_configs;
> +    int ret = -1;
> +
> +    ACameraMetadata_getConstEntry(ctx->camera_metadata,
> +                                  ACAMERA_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,
> +                                  &available_configs);
> +
> +    for (int i = 0; i < available_configs.count; i++) {
> +        int32_t input = available_configs.data.i32[i * 4 + 3];
> +        int32_t format = available_configs.data.i32[i * 4 + 0];
> +
> +        if (input) {
> +            continue;
> +        }
> +
> +        if (format == IMAGE_FORMAT_ANDROID) {
> +            int32_t width = available_configs.data.i32[i * 4 + 1];
> +            int32_t height = available_configs.data.i32[i * 4 + 2];
> +
> +            //Same ratio

> +            if (ctx->requested_width * ctx->requested_height == width * height) {

the product of these can maybe overflow i think


> +                ctx->width = width;
> +                ctx->height = height;
> +                ret = 0;
> +                break;
> +            }
> +        }
> +    }
> +

> +    if (ret < 0 || ctx->width == 0 || ctx->height == 0) {
> +        ctx->width = available_configs.data.i32[1];
> +        ctx->height = available_configs.data.i32[2];
> +        ret = -1;
> +
> +        av_log(avctx, AV_LOG_WARNING,
> +               "Requested video_size %dx%d not available, falling back to %dx%d\n",
> +               ctx->requested_width, ctx->requested_height, ctx->width, ctx->height);
> +    }

you set ret but nothing uses it

> +
> +    return;
> +}

[...]
> +static int add_video_stream(AVFormatContext *avctx)
> +{
> +    AndroidCameraCtx *ctx = avctx->priv_data;
> +    AVStream *st;
> +    AVCodecParameters *codecpar;
> +
> +    st = avformat_new_stream(avctx, NULL);
> +    if (!st) {
> +        return AVERROR(ENOMEM);
> +    }
> +
> +    st->id = VIDEO_STREAM_INDEX;

> +    st->avg_frame_rate = (AVRational) { ctx->framerate_range[1], 1 };
> +    st->r_frame_rate = (AVRational) { ctx->framerate_range[1], 1 };

Are these values always correct ?


[...]
> +    if (ctx->camera_id) {
> +        av_freep(&ctx->camera_id);
> +    }

the if() is unneeded


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171129/a1ec65e1/attachment.sig>