[FFmpeg-devel] [PATCH v2] avcodec/mediacodec: Add AV1 encoder

[Samuel Raposo Vieira Mira]

All comments done. Thanks for the review :)

> By the way, which hardware did you use to run the test?
I used a Samsung S21 and a Pixel 6.



From: Zhao Zhili <quinkblack at foxmail.com>
Date: Sunday, 7. May 2023 at 11.57
To: Samuel Raposo Vieira Mira <samuel.mira at qt.io>, FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec/mediacodec: Add AV1 encoder
On Tue, 2023-05-02 at 12:22 +0000, Samuel Raposo Vieira Mira wrote:
> @@ -3162,6 +3162,7 @@ aac_mf_encoder_deps="mediafoundation"
> ac3_mf_encoder_deps="mediafoundation"
> av1_cuvid_decoder_deps="cuvid CUVIDAV1PICPARAMS"
> av1_mediacodec_decoder_deps="mediacodec"
> +av1_mediacodec_encoder_deps="mediacodec"

This patch format is broken too.

>
> diff --git a/libavcodec/mediacodec_wrapper.c b/libavcodec/mediacodec_wrapper.c
> index 1c29bb7406..015f275a0f 100644
> --- a/libavcodec/mediacodec_wrapper.c
> +++ b/libavcodec/mediacodec_wrapper.c
> @@ -35,6 +35,8 @@
> #include "ffjni.h"
> #include "mediacodec_wrapper.h"
> +#include "libavutil/pixdesc.h"
> +

Please keep the include in some order.

> struct JNIAMediaCodecListFields {
>      jclass mediacodec_list_class;
> @@ -345,6 +347,11 @@ int ff_AMediaCodecProfile_getProfileFromAVCodecContext(AVCodecContext *avctx)
>      static const int MPEG4ProfileAdvancedScalable = 0x4000;
>      static const int MPEG4ProfileAdvancedSimple   = 0x8000;

Missing an empty line between MPEG4 and AV1.

> +    static const int AV1ProfileMain8  = 0x1;
> +    static const int AV1ProfileMain10 = 0x2;
> +    static const int AV1ProfileMain10HDR10     = 0x1000;
> +    static const int AV1ProfileMain10HDR10Plus = 0x2000;
> +
>      // Unused yet.
>      (void)AVCProfileConstrainedHigh;
>      (void)HEVCProfileMain10HDR10;
> @@ -353,6 +360,8 @@ int ff_AMediaCodecProfile_getProfileFromAVCodecContext(AVCodecContext *avctx)
>      (void)VP9Profile3HDR;
>      (void)VP9Profile2HDR10Plus;
>      (void)VP9Profile3HDR10Plus;
> +    (void)AV1ProfileMain10HDR10;
> +    (void)AV1ProfileMain10HDR10Plus;
>      if (avctx->codec_id == AV_CODEC_ID_H264) {
>          switch(avctx->profile) {
> @@ -436,6 +445,9 @@ int ff_AMediaCodecProfile_getProfileFromAVCodecContext(AVCodecContext *avctx)
>          default:
>              break;
>          }
> +    } else if(avctx->codec_id == AV_CODEC_ID_AV1) {
> +        const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(avctx->pix_fmt);
> +        return desc != NULL && desc->comp[0].depth == 8? AV1ProfileMain8 : AV1ProfileMain10;
>      }

I'm not sure about how mediacodec handles 10bit encoding and
need some tests. Let's keep the function simple first by just
map avctx->profile to MediaCodec profile.

>      return -1;
> diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
> index e4b583a542..10da43c3e7 100644
> --- a/libavcodec/mediacodecenc.c
> +++ b/libavcodec/mediacodecenc.c
> @@ -170,6 +170,9 @@ static av_cold int mediacodec_init(AVCodecContext *avctx)
>      case AV_CODEC_ID_MPEG4:
>          codec_mime = "video/mp4v-es";
>          break;
> +    case AV_CODEC_ID_AV1:
> +        codec_mime = "video/av01";
> +        break;
>      default:
>          av_assert0(0);
>      }
> @@ -779,16 +782,16 @@ DECLARE_MEDIACODEC_ENCODER(hevc, "H.265", AV_CODEC_ID_HEVC)
>  enum MediaCodecVP9Level {
>      VP9Level1  = 0x1,
> -    VP9Level11  = 0x2,
> +    VP9Level11 = 0x2,
>      VP9Level2  = 0x4,
> -    VP9Level21  = 0x8,
> -    VP9Level3 = 0x10,
> +    VP9Level21 = 0x8,
> +    VP9Level3  = 0x10,
>      VP9Level31 = 0x20,
>      VP9Level4  = 0x40,
> -    VP9Level41  = 0x80,
> -    VP9Level5 = 0x100,
> +    VP9Level41 = 0x80,
> +    VP9Level5  = 0x100,
>      VP9Level51 = 0x200,
> -    VP9Level52  = 0x400,
> +    VP9Level52 = 0x400,
>      VP9Level6  = 0x800,
>      VP9Level61 = 0x1000,
>      VP9Level62 = 0x2000,
> @@ -837,15 +840,15 @@ DECLARE_MEDIACODEC_ENCODER(vp9, "VP9", AV_CODEC_ID_VP9)
>  enum MediaCodecMpeg4Level {
>      MPEG4Level0  = 0x01,
> -    MPEG4Level0b  = 0x02,
> -    MPEG4Level1 = 0x04,
> +    MPEG4Level0b = 0x02,
> +    MPEG4Level1  = 0x04,
>      MPEG4Level2  = 0x08,
> -    MPEG4Level3 = 0x10,
> +    MPEG4Level3  = 0x10,
>      MPEG4Level3b = 0x18,
> -    MPEG4Level4 = 0x20,
> -    MPEG4Level4a  = 0x40,
> +    MPEG4Level4  = 0x20,
> +    MPEG4Level4a = 0x40,
>      MPEG4Level5  = 0x80,
> -    MPEG4Level6 = 0x100,
> +    MPEG4Level6  = 0x100,
> };

Don't do unrelated code format in this patch.


By the way, which hardware did you use to run the test?