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 <quinkbl...@foxmail.com>
Date: Sunday, 7. May 2023 at 11.57
To: Samuel Raposo Vieira Mira <samuel.m...@qt.io>, FFmpeg development
discussions and patches <ffmpeg-devel@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?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
