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
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
