[FFmpeg-devel] [PATCH 4/4] avcodec/mediacodecenc: add async mode support

[Anton Khirnov]

Quoting Zhao Zhili (2024-11-06 13:31:34)
> @@ -78,6 +80,17 @@ typedef struct MediaCodecEncContext {
>      int extract_extradata;
>      // Ref. MediaFormat KEY_OPERATING_RATE
>      int operating_rate;
> +    int async_mode;
> +
> +    AVMutex input_mutex;
> +    AVCond input_cond;
> +    AVFifo *input_index;
> +
> +    AVMutex output_mutex;
> +    AVCond output_cond;
> +    int encode_status;
> +    AVFifo *output_index;
> +    AVFifo *output_buf_info;

Those seem to always be written and read together, so why not merge them
into one FIFO?

> +static void on_input_available(FFAMediaCodec *codec, void *userdata,
> +                               int32_t index)
> +{
> +    AVCodecContext *avctx = userdata;
> +    MediaCodecEncContext *s = avctx->priv_data;
> +
> +    ff_mutex_lock(&s->input_mutex);
> +
> +    av_fifo_write(s->input_index, &index, 1);

The fifo is dynamic, so this can fail.

> +
> +    ff_mutex_unlock(&s->input_mutex);
> +    ff_cond_signal(&s->input_cond);

This looks wrong - you should signal while holding the mutex, otherwise
the consumer may miss the signal. Same for the other two signalling
sites.

> @@ -395,17 +569,62 @@ bailout:
>      return ret;
>  }
>  
> +static int mediacodec_get_output_index(AVCodecContext *avctx, ssize_t *index,
> +                                       FFAMediaCodecBufferInfo *out_info)
> +{
> +    MediaCodecEncContext *s = avctx->priv_data;
> +    FFAMediaCodec *codec = s->codec;
> +    int64_t timeout_us = s->eof_sent ? OUTPUT_DEQUEUE_TIMEOUT_US : 0;
> +    int n;
> +    int ret;
> +
> +    if (!s->async_mode) {
> +        *index = ff_AMediaCodec_dequeueOutputBuffer(codec, out_info, timeout_us);
> +        return 0;
> +    }
> +
> +    ff_mutex_lock(&s->output_mutex);
> +
> +    n = -1;
> +    while (n < 0 && !s->encode_status) {
> +        if (av_fifo_can_read(s->output_index)) {
> +            av_fifo_read(s->output_index, &n, 1);
> +            av_fifo_read(s->output_buf_info, out_info, 1);

It's simpler and API-compliant to just call av_fifo_read() and check
whether it failed.

-- 
Anton Khirnov