[FFmpeg-devel] [PATCH v2 1/7] avcodec/mediacodecenc: make each encoder has its own option

[FFmpeg-devel] [PATCH v2 1/7] avcodec/mediacodecenc: make each encoder has its own option

[Zhao Zhili]

From: Zhao Zhili <zhilizhao@tencent.com>

---
 libavcodec/mediacodecenc.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
index 69246ad693..4f9185342f 100644
--- a/libavcodec/mediacodecenc.c
+++ b/libavcodec/mediacodecenc.c
@@ -480,19 +480,18 @@ static const AVCodecHWConfigInternal *const mediacodec_hw_configs[] = {
 
 #define OFFSET(x) offsetof(MediaCodecEncContext, x)
 #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
-static const AVOption common_options[] = {
-    { "ndk_codec", "Use MediaCodec from NDK",
-                    OFFSET(use_ndk_codec), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE },
-    { "codec_name", "Select codec by name",
-                    OFFSET(name), AV_OPT_TYPE_STRING, {0}, 0, 0, VE },
-    { NULL },
-};
+#define COMMON_OPTION                                                                                       \
+    { "ndk_codec", "Use MediaCodec from NDK",                                                               \
+                    OFFSET(use_ndk_codec), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE },                      \
+    { "codec_name", "Select codec by name",                                                                 \
+                    OFFSET(name), AV_OPT_TYPE_STRING, {0}, 0, 0, VE },                                      \
+
 
 #define MEDIACODEC_ENCODER_CLASS(name)              \
 static const AVClass name ## _mediacodec_class = {  \
     .class_name = #name "_mediacodec",              \
     .item_name  = av_default_item_name,             \
-    .option     = common_options,                   \
+    .option     = name ## _options,                 \
     .version    = LIBAVUTIL_VERSION_INT,            \
 };                                                  \
 
@@ -517,9 +516,17 @@ const FFCodec ff_ ## short_name ## _mediacodec_encoder = {              \
 };                                                                      \
 
 #if CONFIG_H264_MEDIACODEC_ENCODER
+static const AVOption h264_options[] = {
+    COMMON_OPTION
+    { NULL, }
+};
 DECLARE_MEDIACODEC_ENCODER(h264, "H.264", AV_CODEC_ID_H264)
 #endif
 
 #if CONFIG_HEVC_MEDIACODEC_ENCODER
+static const AVOption hevc_options[] = {
+    COMMON_OPTION
+    { NULL, }
+};
 DECLARE_MEDIACODEC_ENCODER(hevc, "H.265", AV_CODEC_ID_HEVC)
 #endif
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 2/7] avcodec/mediacodecenc: add bitrate_mode option

[Zhao Zhili]

From: Zhao Zhili <zhilizhao@tencent.com>

---
 libavcodec/mediacodecenc.c | 25 +++++++++++++++++++++++++
 libavcodec/version.h       |  2 +-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
index 4f9185342f..ec0e0b3a86 100644
--- a/libavcodec/mediacodecenc.c
+++ b/libavcodec/mediacodecenc.c
@@ -39,6 +39,17 @@
 #define INPUT_DEQUEUE_TIMEOUT_US 8000
 #define OUTPUT_DEQUEUE_TIMEOUT_US 8000
 
+enum BitrateMode {
+    /* Constant quality mode */
+    BITRATE_MODE_CQ = 0,
+    /* Variable bitrate mode */
+    BITRATE_MODE_VBR = 1,
+    /* Constant bitrate mode */
+    BITRATE_MODE_CBR = 2,
+    /* Constant bitrate mode with frame drops */
+    BITRATE_MODE_CBR_FD = 3,
+};
+
 typedef struct MediaCodecEncContext {
     AVClass *avclass;
     FFAMediaCodec *codec;
@@ -67,6 +78,8 @@ typedef struct MediaCodecEncContext {
     int eof_sent;
 
     AVFrame *frame;
+
+    int bitrate_mode;
 } MediaCodecEncContext;
 
 enum {
@@ -193,6 +206,8 @@ static av_cold int mediacodec_init(AVCodecContext *avctx)
 
     if (avctx->bit_rate)
         ff_AMediaFormat_setInt32(format, "bitrate", avctx->bit_rate);
+    if (s->bitrate_mode >= 0)
+        ff_AMediaFormat_setInt32(format, "bitrate-mode", s->bitrate_mode);
     // frame-rate and i-frame-interval are required to configure codec
     if (avctx->framerate.num >= avctx->framerate.den && avctx->framerate.den > 0) {
         s->fps = avctx->framerate.num / avctx->framerate.den;
@@ -485,6 +500,16 @@ static const AVCodecHWConfigInternal *const mediacodec_hw_configs[] = {
                     OFFSET(use_ndk_codec), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE },                      \
     { "codec_name", "Select codec by name",                                                                 \
                     OFFSET(name), AV_OPT_TYPE_STRING, {0}, 0, 0, VE },                                      \
+    { "bitrate_mode", "Bitrate control method",                                                             \
+                    OFFSET(bitrate_mode), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, VE, "bitrate_mode" },  \
+    { "cq", "Constant quality mode",                                                                        \
+                    0, AV_OPT_TYPE_CONST, {.i64 = BITRATE_MODE_CQ}, 0, 0, VE, "bitrate_mode" },             \
+    { "vbr", "Variable bitrate mode",                                                                       \
+                    0, AV_OPT_TYPE_CONST, {.i64 = BITRATE_MODE_VBR}, 0, 0, VE, "bitrate_mode" },            \
+    { "cbr", "Constant bitrate mode",                                                                       \
+                    0, AV_OPT_TYPE_CONST, {.i64 = BITRATE_MODE_CBR}, 0, 0, VE, "bitrate_mode" },            \
+    { "cbr_fd", "Constant bitrate mode with frame drops",                                                   \
+                    0, AV_OPT_TYPE_CONST, {.i64 = BITRATE_MODE_CBR_FD}, 0, 0, VE, "bitrate_mode" },         \
 
 
 #define MEDIACODEC_ENCODER_CLASS(name)              \
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 9e66920593..527b4dbd0b 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -30,7 +30,7 @@
 #include "version_major.h"
 
 #define LIBAVCODEC_VERSION_MINOR  54
-#define LIBAVCODEC_VERSION_MICRO 101
+#define LIBAVCODEC_VERSION_MICRO 102
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
                                                LIBAVCODEC_VERSION_MINOR, \
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 3/7] avcodec/mediacodecenc: add level option

[Zhao Zhili]

From: Zhao Zhili <zhilizhao@tencent.com>

---
 libavcodec/mediacodecenc.c | 143 ++++++++++++++++++++++++++++++++++++-
 libavcodec/version.h       |   2 +-
 2 files changed, 142 insertions(+), 3 deletions(-)

diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
index ec0e0b3a86..2f78567451 100644
--- a/libavcodec/mediacodecenc.c
+++ b/libavcodec/mediacodecenc.c
@@ -80,6 +80,7 @@ typedef struct MediaCodecEncContext {
     AVFrame *frame;
 
     int bitrate_mode;
+    int level;
 } MediaCodecEncContext;
 
 enum {
@@ -233,6 +234,10 @@ static av_cold int mediacodec_init(AVCodecContext *avctx)
         av_log(avctx, AV_LOG_DEBUG, "set profile to 0x%x\n", ret);
         ff_AMediaFormat_setInt32(format, "profile", ret);
     }
+    if (s->level > 0) {
+        av_log(avctx, AV_LOG_DEBUG, "set level to 0x%x\n", s->level);
+        ff_AMediaFormat_setInt32(format, "level", s->level);
+    }
 
     ret = ff_AMediaCodec_getConfigureFlagEncode(s->codec);
     ret = ff_AMediaCodec_configure(s->codec, format, s->window, NULL, ret);
@@ -541,17 +546,151 @@ const FFCodec ff_ ## short_name ## _mediacodec_encoder = {              \
 };                                                                      \
 
 #if CONFIG_H264_MEDIACODEC_ENCODER
+
+enum MediaCodecAvcLevel {
+    AVCLevel1       = 0x01,
+    AVCLevel1b      = 0x02,
+    AVCLevel11      = 0x04,
+    AVCLevel12      = 0x08,
+    AVCLevel13      = 0x10,
+    AVCLevel2       = 0x20,
+    AVCLevel21      = 0x40,
+    AVCLevel22      = 0x80,
+    AVCLevel3       = 0x100,
+    AVCLevel31      = 0x200,
+    AVCLevel32      = 0x400,
+    AVCLevel4       = 0x800,
+    AVCLevel41      = 0x1000,
+    AVCLevel42      = 0x2000,
+    AVCLevel5       = 0x4000,
+    AVCLevel51      = 0x8000,
+    AVCLevel52      = 0x10000,
+    AVCLevel6       = 0x20000,
+    AVCLevel61      = 0x40000,
+    AVCLevel62      = 0x80000,
+};
+
 static const AVOption h264_options[] = {
     COMMON_OPTION
+    { "level", "Specify level",
+                OFFSET(level), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, VE, "level" },
+    { "1",      "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel1  },  0, 0, VE, "level" },
+    { "1b",     "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel1b }, 0, 0, VE, "level" },
+    { "1.1",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel11 }, 0, 0, VE, "level" },
+    { "1.2",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel12 }, 0, 0, VE, "level" },
+    { "1.3",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel13 }, 0, 0, VE, "level" },
+    { "2",      "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel2  },  0, 0, VE, "level" },
+    { "2.1",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel21 }, 0, 0, VE, "level" },
+    { "2.2",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel22 }, 0, 0, VE, "level" },
+    { "3",      "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel3  },  0, 0, VE, "level" },
+    { "3.1",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel31 }, 0, 0, VE, "level" },
+    { "3.2",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel32 }, 0, 0, VE, "level" },
+    { "4",      "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel4  },  0, 0, VE, "level" },
+    { "4.1",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel41 }, 0, 0, VE, "level" },
+    { "4.2",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel42 }, 0, 0, VE, "level" },
+    { "5",      "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel5  },  0, 0, VE, "level" },
+    { "5.1",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel51 }, 0, 0, VE, "level" },
+    { "5.2",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel52 }, 0, 0, VE, "level" },
+    { "6.0",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel6  }, 0, 0, VE, "level" },
+    { "6.1",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel61 }, 0, 0, VE, "level" },
+    { "6.2",    "", 0, AV_OPT_TYPE_CONST, { .i64 = AVCLevel62 }, 0, 0, VE, "level" },
     { NULL, }
 };
+
 DECLARE_MEDIACODEC_ENCODER(h264, "H.264", AV_CODEC_ID_H264)
-#endif
+
+#endif  // CONFIG_H264_MEDIACODEC_ENCODER
 
 #if CONFIG_HEVC_MEDIACODEC_ENCODER
+
+enum MediaCodecHevcLevel {
+    HEVCMainTierLevel1  = 0x1,
+    HEVCHighTierLevel1  = 0x2,
+    HEVCMainTierLevel2  = 0x4,
+    HEVCHighTierLevel2  = 0x8,
+    HEVCMainTierLevel21 = 0x10,
+    HEVCHighTierLevel21 = 0x20,
+    HEVCMainTierLevel3  = 0x40,
+    HEVCHighTierLevel3  = 0x80,
+    HEVCMainTierLevel31 = 0x100,
+    HEVCHighTierLevel31 = 0x200,
+    HEVCMainTierLevel4  = 0x400,
+    HEVCHighTierLevel4  = 0x800,
+    HEVCMainTierLevel41 = 0x1000,
+    HEVCHighTierLevel41 = 0x2000,
+    HEVCMainTierLevel5  = 0x4000,
+    HEVCHighTierLevel5  = 0x8000,
+    HEVCMainTierLevel51 = 0x10000,
+    HEVCHighTierLevel51 = 0x20000,
+    HEVCMainTierLevel52 = 0x40000,
+    HEVCHighTierLevel52 = 0x80000,
+    HEVCMainTierLevel6  = 0x100000,
+    HEVCHighTierLevel6  = 0x200000,
+    HEVCMainTierLevel61 = 0x400000,
+    HEVCHighTierLevel61 = 0x800000,
+    HEVCMainTierLevel62 = 0x1000000,
+    HEVCHighTierLevel62 = 0x2000000,
+};
+
 static const AVOption hevc_options[] = {
     COMMON_OPTION
+    { "level", "Specify tier and level",
+                OFFSET(level), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, VE, "level" },
+    { "m1",    "Main tier level 1",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCMainTierLevel1  },  0, 0, VE,  "level" },
+    { "h1",    "High tier level 1",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCHighTierLevel1  },  0, 0, VE,  "level" },
+    { "m2",    "Main tier level 2",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCMainTierLevel2  },  0, 0, VE,  "level" },
+    { "h2",    "High tier level 2",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCHighTierLevel2  },  0, 0, VE,  "level" },
+    { "m2.1",  "Main tier level 2.1",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCMainTierLevel21 },  0, 0, VE,  "level" },
+    { "h2.1",  "High tier level 2.1",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCHighTierLevel21 },  0, 0, VE,  "level" },
+    { "m3",    "Main tier level 3",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCMainTierLevel3  },  0, 0, VE,  "level" },
+    { "h3",    "High tier level 3",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCHighTierLevel3  },  0, 0, VE,  "level" },
+    { "m3.1",  "Main tier level 3.1",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCMainTierLevel31 },  0, 0, VE,  "level" },
+    { "h3.1",  "High tier level 3.1",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCHighTierLevel31 },  0, 0, VE,  "level" },
+    { "m4",    "Main tier level 4",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCMainTierLevel4  },  0, 0, VE,  "level" },
+    { "h4",    "High tier level 4",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCHighTierLevel4  },  0, 0, VE,  "level" },
+    { "m4.1",  "Main tier level 4.1",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCMainTierLevel41 },  0, 0, VE,  "level" },
+    { "h4.1",  "High tier level 4.1",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCHighTierLevel41 },  0, 0, VE,  "level" },
+    { "m5",    "Main tier level 5",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCMainTierLevel5  },  0, 0, VE,  "level" },
+    { "h5",    "High tier level 5",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCHighTierLevel5  },  0, 0, VE,  "level" },
+    { "m5.1",  "Main tier level 5.1",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCMainTierLevel51 },  0, 0, VE,  "level" },
+    { "h5.1",  "High tier level 5.1",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCHighTierLevel51 },  0, 0, VE,  "level" },
+    { "m5.2",  "Main tier level 5.2",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCMainTierLevel52 },  0, 0, VE,  "level" },
+    { "h5.2",  "High tier level 5.2",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCHighTierLevel52 },  0, 0, VE,  "level" },
+    { "m6",    "Main tier level 6",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCMainTierLevel6  },  0, 0, VE,  "level" },
+    { "h6",    "High tier level 6",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCHighTierLevel6  },  0, 0, VE,  "level" },
+    { "m6.1",  "Main tier level 6.1",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCMainTierLevel61 },  0, 0, VE,  "level" },
+    { "h6.1",  "High tier level 6.1",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCHighTierLevel61 },  0, 0, VE,  "level" },
+    { "m6.2",  "Main tier level 6.2",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCMainTierLevel62 },  0, 0, VE,  "level" },
+    { "h6.2",  "High tier level 6.2",
+                0, AV_OPT_TYPE_CONST, { .i64 = HEVCHighTierLevel62 },  0, 0, VE,  "level" },
     { NULL, }
 };
+
 DECLARE_MEDIACODEC_ENCODER(hevc, "H.265", AV_CODEC_ID_HEVC)
-#endif
+
+#endif  // CONFIG_HEVC_MEDIACODEC_ENCODER
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 527b4dbd0b..61bdf5806b 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -30,7 +30,7 @@
 #include "version_major.h"
 
 #define LIBAVCODEC_VERSION_MINOR  54
-#define LIBAVCODEC_VERSION_MICRO 102
+#define LIBAVCODEC_VERSION_MICRO 103
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
                                                LIBAVCODEC_VERSION_MINOR, \
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 4/7] avcodec/mediacodecenc: use bsf to handle crop

[Zhao Zhili]

From: Zhao Zhili <zhilizhao@tencent.com>

It's well known that mediacodec encoder requires 16x16 alignment.
Use our bsf to fix the crop info.
---
 configure                  |  2 ++
 libavcodec/mediacodecenc.c | 65 +++++++++++++++++++++++++++++++++++---
 2 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index f4eedfc207..2180ebb4f1 100755
--- a/configure
+++ b/configure
@@ -3169,6 +3169,7 @@ h264_mediacodec_decoder_extralibs="-landroid"
 h264_mediacodec_decoder_select="h264_mp4toannexb_bsf h264_parser"
 h264_mediacodec_encoder_deps="mediacodec"
 h264_mediacodec_encoder_extralibs="-landroid"
+h264_mediacodec_encoder_select="h264_metadata"
 h264_mf_encoder_deps="mediafoundation"
 h264_mmal_decoder_deps="mmal"
 h264_nvenc_encoder_deps="nvenc"
@@ -3190,6 +3191,7 @@ hevc_mediacodec_decoder_extralibs="-landroid"
 hevc_mediacodec_decoder_select="hevc_mp4toannexb_bsf hevc_parser"
 hevc_mediacodec_encoder_deps="mediacodec"
 hevc_mediacodec_encoder_extralibs="-landroid"
+hevc_mediacodec_encoder_select="hevc_metadata"
 hevc_mf_encoder_deps="mediafoundation"
 hevc_nvenc_encoder_deps="nvenc"
 hevc_nvenc_encoder_select="atsc_a53"
diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
index 2f78567451..7f2ae88285 100644
--- a/libavcodec/mediacodecenc.c
+++ b/libavcodec/mediacodecenc.c
@@ -28,6 +28,7 @@
 #include "libavutil/opt.h"
 
 #include "avcodec.h"
+#include "bsf.h"
 #include "codec_internal.h"
 #include "encode.h"
 #include "hwconfig.h"
@@ -78,6 +79,7 @@ typedef struct MediaCodecEncContext {
     int eof_sent;
 
     AVFrame *frame;
+    AVBSFContext *bsf;
 
     int bitrate_mode;
     int level;
@@ -119,6 +121,42 @@ static void mediacodec_output_format(AVCodecContext *avctx)
     ff_AMediaFormat_delete(out_format);
 }
 
+static int mediacodec_init_bsf(AVCodecContext *avctx)
+{
+    MediaCodecEncContext *s = avctx->priv_data;
+    char str[128];
+    int ret;
+    int crop_right = s->width - avctx->width;
+    int crop_bottom = s->height - avctx->height;
+
+    if (!crop_right && !crop_bottom)
+        return 0;
+
+    if (avctx->codec_id == AV_CODEC_ID_H264)
+        ret = snprintf(str, sizeof(str), "h264_metadata=crop_right=%d:crop_bottom=%d",
+                 crop_right, crop_bottom);
+    else if (avctx->codec_id == AV_CODEC_ID_HEVC)
+        ret = snprintf(str, sizeof(str), "hevc_metadata=crop_right=%d:crop_bottom=%d",
+                 crop_right, crop_bottom);
+    else
+        return 0;
+
+    if (ret >= sizeof(str))
+        return AVERROR_BUFFER_TOO_SMALL;
+
+    ret = av_bsf_list_parse_str(str, &s->bsf);
+    if (ret < 0)
+        return ret;
+
+    ret = avcodec_parameters_from_context(s->bsf->par_in, avctx);
+    if (ret < 0)
+        return ret;
+    s->bsf->time_base_in = avctx->time_base;
+    ret = av_bsf_init(s->bsf);
+
+    return ret;
+}
+
 static av_cold int mediacodec_init(AVCodecContext *avctx)
 {
     const char *codec_mime = NULL;
@@ -159,7 +197,7 @@ static av_cold int mediacodec_init(AVCodecContext *avctx)
 
     ff_AMediaFormat_setString(format, "mime", codec_mime);
     s->width = FFALIGN(avctx->width, 16);
-    s->height = avctx->height;
+    s->height = FFALIGN(avctx->height, 16);
     ff_AMediaFormat_setInt32(format, "width", s->width);
     ff_AMediaFormat_setInt32(format, "height", s->height);
 
@@ -252,6 +290,10 @@ static av_cold int mediacodec_init(AVCodecContext *avctx)
         goto bailout;
     }
 
+    ret = mediacodec_init_bsf(avctx);
+    if (ret)
+        goto bailout;
+
     mediacodec_output_format(avctx);
 
     s->frame = av_frame_alloc();
@@ -444,10 +486,24 @@ static int mediacodec_encode(AVCodecContext *avctx, AVPacket *pkt)
     // 2. Got a packet success
     // 3. No AVFrame is available yet (don't return if get_frame return EOF)
     while (1) {
+        if (s->bsf) {
+            ret = av_bsf_receive_packet(s->bsf, pkt);
+            if (!ret)
+                return 0;
+            if (ret != AVERROR(EAGAIN))
+                return ret;
+        }
+
         ret = mediacodec_receive(avctx, pkt, &got_packet);
-        if (!ret)
-            return 0;
-        else if (ret != AVERROR(EAGAIN))
+        if (s->bsf) {
+            if (!ret || ret == AVERROR_EOF)
+                ret = av_bsf_send_packet(s->bsf, pkt);
+        } else {
+            if (!ret)
+                return 0;
+        }
+
+        if (ret != AVERROR(EAGAIN))
             return ret;
 
         if (!s->frame->buf[0]) {
@@ -480,6 +536,7 @@ static av_cold int mediacodec_close(AVCodecContext *avctx)
         s->window = NULL;
     }
 
+    av_bsf_free(&s->bsf);
     av_frame_free(&s->frame);
 
     return 0;
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 3/7] avcodec/mediacodecenc: use bsf to handle crop

[Zhao Zhili]

From: Zhao Zhili <zhilizhao@tencent.com>

It's well known that mediacodec encoder requires 16x16 alignment.
Use our bsf to fix the crop info.
---
v3: don't change the dimension for AV_PIX_FMT_MEDIACODEC. It can have
side effect.

 configure                  |  2 +
 libavcodec/mediacodecenc.c | 78 +++++++++++++++++++++++++++++++++++---
 2 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index f4eedfc207..2180ebb4f1 100755
--- a/configure
+++ b/configure
@@ -3169,6 +3169,7 @@ h264_mediacodec_decoder_extralibs="-landroid"
 h264_mediacodec_decoder_select="h264_mp4toannexb_bsf h264_parser"
 h264_mediacodec_encoder_deps="mediacodec"
 h264_mediacodec_encoder_extralibs="-landroid"
+h264_mediacodec_encoder_select="h264_metadata"
 h264_mf_encoder_deps="mediafoundation"
 h264_mmal_decoder_deps="mmal"
 h264_nvenc_encoder_deps="nvenc"
@@ -3190,6 +3191,7 @@ hevc_mediacodec_decoder_extralibs="-landroid"
 hevc_mediacodec_decoder_select="hevc_mp4toannexb_bsf hevc_parser"
 hevc_mediacodec_encoder_deps="mediacodec"
 hevc_mediacodec_encoder_extralibs="-landroid"
+hevc_mediacodec_encoder_select="hevc_metadata"
 hevc_mf_encoder_deps="mediafoundation"
 hevc_nvenc_encoder_deps="nvenc"
 hevc_nvenc_encoder_select="atsc_a53"
diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
index 2f78567451..4e8716e3a5 100644
--- a/libavcodec/mediacodecenc.c
+++ b/libavcodec/mediacodecenc.c
@@ -28,6 +28,7 @@
 #include "libavutil/opt.h"
 
 #include "avcodec.h"
+#include "bsf.h"
 #include "codec_internal.h"
 #include "encode.h"
 #include "hwconfig.h"
@@ -78,6 +79,7 @@ typedef struct MediaCodecEncContext {
     int eof_sent;
 
     AVFrame *frame;
+    AVBSFContext *bsf;
 
     int bitrate_mode;
     int level;
@@ -119,6 +121,42 @@ static void mediacodec_output_format(AVCodecContext *avctx)
     ff_AMediaFormat_delete(out_format);
 }
 
+static int mediacodec_init_bsf(AVCodecContext *avctx)
+{
+    MediaCodecEncContext *s = avctx->priv_data;
+    char str[128];
+    int ret;
+    int crop_right = s->width - avctx->width;
+    int crop_bottom = s->height - avctx->height;
+
+    if (!crop_right && !crop_bottom)
+        return 0;
+
+    if (avctx->codec_id == AV_CODEC_ID_H264)
+        ret = snprintf(str, sizeof(str), "h264_metadata=crop_right=%d:crop_bottom=%d",
+                 crop_right, crop_bottom);
+    else if (avctx->codec_id == AV_CODEC_ID_HEVC)
+        ret = snprintf(str, sizeof(str), "hevc_metadata=crop_right=%d:crop_bottom=%d",
+                 crop_right, crop_bottom);
+    else
+        return 0;
+
+    if (ret >= sizeof(str))
+        return AVERROR_BUFFER_TOO_SMALL;
+
+    ret = av_bsf_list_parse_str(str, &s->bsf);
+    if (ret < 0)
+        return ret;
+
+    ret = avcodec_parameters_from_context(s->bsf->par_in, avctx);
+    if (ret < 0)
+        return ret;
+    s->bsf->time_base_in = avctx->time_base;
+    ret = av_bsf_init(s->bsf);
+
+    return ret;
+}
+
 static av_cold int mediacodec_init(AVCodecContext *avctx)
 {
     const char *codec_mime = NULL;
@@ -158,8 +196,19 @@ static av_cold int mediacodec_init(AVCodecContext *avctx)
     }
 
     ff_AMediaFormat_setString(format, "mime", codec_mime);
-    s->width = FFALIGN(avctx->width, 16);
-    s->height = avctx->height;
+    // Workaround the alignment requirement of mediacodec. We can't do it
+    // silently for AV_PIX_FMT_MEDIACODEC.
+    if (avctx->pix_fmt != AV_PIX_FMT_MEDIACODEC) {
+        s->width = FFALIGN(avctx->width, 16);
+        s->height = FFALIGN(avctx->height, 16);
+    } else {
+        s->width = avctx->width;
+        s->height = avctx->height;
+        if (s->width % 16 || s->height % 16)
+            av_log(avctx, AV_LOG_WARNING,
+                    "Video size %dx%d isn't align to 16, it may have device compatibility issue\n",
+                    s->width, s->height);
+    }
     ff_AMediaFormat_setInt32(format, "width", s->width);
     ff_AMediaFormat_setInt32(format, "height", s->height);
 
@@ -252,6 +301,10 @@ static av_cold int mediacodec_init(AVCodecContext *avctx)
         goto bailout;
     }
 
+    ret = mediacodec_init_bsf(avctx);
+    if (ret)
+        goto bailout;
+
     mediacodec_output_format(avctx);
 
     s->frame = av_frame_alloc();
@@ -444,10 +497,24 @@ static int mediacodec_encode(AVCodecContext *avctx, AVPacket *pkt)
     // 2. Got a packet success
     // 3. No AVFrame is available yet (don't return if get_frame return EOF)
     while (1) {
+        if (s->bsf) {
+            ret = av_bsf_receive_packet(s->bsf, pkt);
+            if (!ret)
+                return 0;
+            if (ret != AVERROR(EAGAIN))
+                return ret;
+        }
+
         ret = mediacodec_receive(avctx, pkt, &got_packet);
-        if (!ret)
-            return 0;
-        else if (ret != AVERROR(EAGAIN))
+        if (s->bsf) {
+            if (!ret || ret == AVERROR_EOF)
+                ret = av_bsf_send_packet(s->bsf, pkt);
+        } else {
+            if (!ret)
+                return 0;
+        }
+
+        if (ret != AVERROR(EAGAIN))
             return ret;
 
         if (!s->frame->buf[0]) {
@@ -480,6 +547,7 @@ static av_cold int mediacodec_close(AVCodecContext *avctx)
         s->window = NULL;
     }
 
+    av_bsf_free(&s->bsf);
     av_frame_free(&s->frame);
 
     return 0;
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 3/7] avcodec/mediacodecenc: use bsf to handle crop

[Tomas Härdin]

lör 2022-12-10 klockan 01:22 +0800 skrev Zhao Zhili:
> From: Zhao Zhili <zhilizhao@tencent.com>
> 
> It's well known that mediacodec encoder requires 16x16 alignment.
> Use our bsf to fix the crop info.
> ---
> v3: don't change the dimension for AV_PIX_FMT_MEDIACODEC. It can have
> side effect.

Looks like this silently crops? Is that really a good idea? We usually
don't do stuff like that. For example codecs that require even
dimensions complain loudly then fail.

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 3/7] avcodec/mediacodecenc: use bsf to handle crop

["zhilizhao(赵志立)"]

> On Dec 12, 2022, at 23:27, Tomas Härdin <git@haerdin.se> wrote:
> 
> lör 2022-12-10 klockan 01:22 +0800 skrev Zhao Zhili:
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> 
>> It's well known that mediacodec encoder requires 16x16 alignment.
>> Use our bsf to fix the crop info.
>> ---
>> v3: don't change the dimension for AV_PIX_FMT_MEDIACODEC. It can have
>> side effect.
> 
> Looks like this silently crops? Is that really a good idea? We usually
> don't do stuff like that. For example codecs that require even
> dimensions complain loudly then fail.

It’s reasonable to require even dimensions. Require dimensions aligned
to 16 is uncommon. Everyone will complain why 1080x1920 doesn’t work.

A lot of apps just use aligned dimensions. Users have no control on
these apps. It’s not the same with FFmpeg, users (developer or not)
can specify the dimension directly.

If we don’t fix it, either:

1. Reject and fail directly. User complain why.
2. Accept and keep going. Sometimes it works, sometimes don’t. It
depends on the device and get into a confused situation.

I know there are getWidthAlignment()/getHeightAlignment() to get
alignment info of codecs. The results are unreliable. The only
reliable way I can find is don’t depends on those API and fix it
by ourself.

I’d like to know if there are better choices.

> 
> /Tomas
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 3/7] avcodec/mediacodecenc: use bsf to handle crop

[Tomas Härdin]

tis 2022-12-13 klockan 11:20 +0800 skrev zhilizhao(赵志立):
> 
> 
> > On Dec 12, 2022, at 23:27, Tomas Härdin <git@haerdin.se> wrote:
> > 
> > lör 2022-12-10 klockan 01:22 +0800 skrev Zhao Zhili:
> > > From: Zhao Zhili <zhilizhao@tencent.com>
> > > 
> > > It's well known that mediacodec encoder requires 16x16 alignment.
> > > Use our bsf to fix the crop info.
> > > ---
> > > v3: don't change the dimension for AV_PIX_FMT_MEDIACODEC. It can
> > > have
> > > side effect.
> > 
> > Looks like this silently crops? Is that really a good idea? We
> > usually
> > don't do stuff like that. For example codecs that require even
> > dimensions complain loudly then fail.
> 
> It’s reasonable to require even dimensions. Require dimensions
> aligned
> to 16 is uncommon. Everyone will complain why 1080x1920 doesn’t work.
> 
> A lot of apps just use aligned dimensions. Users have no control on
> these apps. It’s not the same with FFmpeg, users (developer or not)
> can specify the dimension directly.

Wait a sec, I think I was misunderstanding what the code is doing.
FFALIGN rounds *up*. Does this mean you insert fake data in the border
that then gets cropped away, meaning the original essence is still
"there"? That's a different thing and probably perfectly OK.

I think we might want something for this inside lavf somewhere, so that
encoders can signal dimension alignment requirements. Some containers
(MXF, MOV) support such cropping in a codec-agnostic manner.

> 
> If we don’t fix it, either:
> 
> 1. Reject and fail directly. User complain why.
> 2. Accept and keep going. Sometimes it works, sometimes don’t. It
> depends on the device and get into a confused situation.
> 
> I know there are getWidthAlignment()/getHeightAlignment() to get
> alignment info of codecs. The results are unreliable. The only
> reliable way I can find is don’t depends on those API and fix it
> by ourself.

Given how temperamental MC seems to be a "belt and braces" approach
might be appropriate when dealing with it. Tell users (ffmpeg.c is a
user here) that dimensions must be aligned by 16x16 and then
automagically doing the required padding and cropping somewhere (lavf
or ffmpeg.c) feels like a decent solution.

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 3/7] avcodec/mediacodecenc: use bsf to handle crop

[Tomas Härdin]

ons 2022-12-14 klockan 18:08 +0100 skrev Tomas Härdin:
> tis 2022-12-13 klockan 11:20 +0800 skrev zhilizhao(赵志立):
> > 
> > 
> > > On Dec 12, 2022, at 23:27, Tomas Härdin <git@haerdin.se> wrote:
> > > 
> > > lör 2022-12-10 klockan 01:22 +0800 skrev Zhao Zhili:
> > > > From: Zhao Zhili <zhilizhao@tencent.com>
> > > > 
> > > > It's well known that mediacodec encoder requires 16x16
> > > > alignment.
> > > > Use our bsf to fix the crop info.
> > > > ---
> > > > v3: don't change the dimension for AV_PIX_FMT_MEDIACODEC. It
> > > > can
> > > > have
> > > > side effect.
> > > 
> > > Looks like this silently crops? Is that really a good idea? We
> > > usually
> > > don't do stuff like that. For example codecs that require even
> > > dimensions complain loudly then fail.
> > 
> > It’s reasonable to require even dimensions. Require dimensions
> > aligned
> > to 16 is uncommon. Everyone will complain why 1080x1920 doesn’t
> > work.
> > 
> > A lot of apps just use aligned dimensions. Users have no control on
> > these apps. It’s not the same with FFmpeg, users (developer or not)
> > can specify the dimension directly.
> 
> Wait a sec, I think I was misunderstanding what the code is doing.
> FFALIGN rounds *up*. Does this mean you insert fake data in the
> border
> that then gets cropped away, meaning the original essence is still
> "there"? That's a different thing and probably perfectly OK.
> 
> I think we might want something for this inside lavf somewhere, so
> that
> encoders can signal dimension alignment requirements. Some containers
> (MXF, MOV) support such cropping in a codec-agnostic manner.
> 
> > 
> > If we don’t fix it, either:
> > 
> > 1. Reject and fail directly. User complain why.
> > 2. Accept and keep going. Sometimes it works, sometimes don’t. It
> > depends on the device and get into a confused situation.
> > 
> > I know there are getWidthAlignment()/getHeightAlignment() to get
> > alignment info of codecs. The results are unreliable. The only
> > reliable way I can find is don’t depends on those API and fix it
> > by ourself.
> 
> Given how temperamental MC seems to be a "belt and braces" approach
> might be appropriate when dealing with it. Tell users (ffmpeg.c is a
> user here) that dimensions must be aligned by 16x16 and then
> automagically doing the required padding and cropping somewhere (lavf
> or ffmpeg.c) feels like a decent solution.

Come to think of it this kind of 16x16 requirement is very common and
is already being handled silently: it's the macroblock size for almost
every DCT codec when using 4:2:0 subsampling.

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 3/7] avcodec/mediacodecenc: use bsf to handle crop

[Zhao Zhili]

On Wed, 2022-12-14 at 18:08 +0100, Tomas Härdin wrote:
> tis 2022-12-13 klockan 11:20 +0800 skrev zhilizhao(赵志立):
> > 
> > > On Dec 12, 2022, at 23:27, Tomas Härdin <git@haerdin.se> wrote:
> > > 
> > > lör 2022-12-10 klockan 01:22 +0800 skrev Zhao Zhili:
> > > > From: Zhao Zhili <zhilizhao@tencent.com>
> > > > 
> > > > It's well known that mediacodec encoder requires 16x16
> > > > alignment.
> > > > Use our bsf to fix the crop info.
> > > > ---
> > > > v3: don't change the dimension for AV_PIX_FMT_MEDIACODEC. It
> > > > can
> > > > have
> > > > side effect.
> > > 
> > > Looks like this silently crops? Is that really a good idea? We
> > > usually
> > > don't do stuff like that. For example codecs that require even
> > > dimensions complain loudly then fail.
> > 
> > It’s reasonable to require even dimensions. Require dimensions
> > aligned
> > to 16 is uncommon. Everyone will complain why 1080x1920 doesn’t
> > work.
> > 
> > A lot of apps just use aligned dimensions. Users have no control on
> > these apps. It’s not the same with FFmpeg, users (developer or not)
> > can specify the dimension directly.
> 
> Wait a sec, I think I was misunderstanding what the code is doing.
> FFALIGN rounds *up*. Does this mean you insert fake data in the
> border
> that then gets cropped away, meaning the original essence is still
> "there"? That's a different thing and probably perfectly OK.

Yes, the dimension passed to MC is rounding up, then use bsf to remove
that border. It depends on our AVFrame data has been aligned properly.
This job is supposed to be done by any decent encoder, not by a
wrapper.

> 
> I think we might want something for this inside lavf somewhere, so
> that
> encoders can signal dimension alignment requirements. Some containers
> (MXF, MOV) support such cropping in a codec-agnostic manner.

From my own experience, dimension mismatch between codec and container
makes a lot of trouble. ISO base format specification specified how to
crop/scale after decoding clear, however, I don't think it has been
widely supported, including FFmpeg. We can fix that inside of FFmpeg,
but we should avoid such cases as much as we can.

> 
> > If we don’t fix it, either:
> > 
> > 1. Reject and fail directly. User complain why.
> > 2. Accept and keep going. Sometimes it works, sometimes don’t. It
> > depends on the device and get into a confused situation.
> > 
> > I know there are getWidthAlignment()/getHeightAlignment() to get
> > alignment info of codecs. The results are unreliable. The only
> > reliable way I can find is don’t depends on those API and fix it
> > by ourself.
> 
> Given how temperamental MC seems to be a "belt and braces" approach
> might be appropriate when dealing with it. Tell users (ffmpeg.c is a
> user here) that dimensions must be aligned by 16x16 and then
> automagically doing the required padding and cropping somewhere (lavf
> or ffmpeg.c) feels like a decent solution.
> 
> /Tomas
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 3/7] avcodec/mediacodecenc: use bsf to handle crop

[Zhao Zhili]

On Thu, 2022-12-15 at 01:37 +0800, Zhao Zhili wrote:
> On Wed, 2022-12-14 at 18:08 +0100, Tomas Härdin wrote:
> > tis 2022-12-13 klockan 11:20 +0800 skrev zhilizhao(赵志立):
> > > > On Dec 12, 2022, at 23:27, Tomas Härdin <git@haerdin.se> wrote:
> > > > 
> > > > lör 2022-12-10 klockan 01:22 +0800 skrev Zhao Zhili:
> > > > > From: Zhao Zhili <zhilizhao@tencent.com>
> > > > > 
> > > > > It's well known that mediacodec encoder requires 16x16
> > > > > alignment.
> > > > > Use our bsf to fix the crop info.
> > > > > ---
> > > > > v3: don't change the dimension for AV_PIX_FMT_MEDIACODEC. It
> > > > > can
> > > > > have
> > > > > side effect.
> > > > 
> > > > Looks like this silently crops? Is that really a good idea? We
> > > > usually
> > > > don't do stuff like that. For example codecs that require even
> > > > dimensions complain loudly then fail.
> > > 
> > > It’s reasonable to require even dimensions. Require dimensions
> > > aligned
> > > to 16 is uncommon. Everyone will complain why 1080x1920 doesn’t
> > > work.
> > > 
> > > A lot of apps just use aligned dimensions. Users have no control
> > > on
> > > these apps. It’s not the same with FFmpeg, users (developer or
> > > not)
> > > can specify the dimension directly.
> > 
> > Wait a sec, I think I was misunderstanding what the code is doing.
> > FFALIGN rounds *up*. Does this mean you insert fake data in the
> > border
> > that then gets cropped away, meaning the original essence is still
> > "there"? That's a different thing and probably perfectly OK.
> 
> Yes, the dimension passed to MC is rounding up, then use bsf to
> remove
> that border. It depends on our AVFrame data has been aligned
> properly.

Actually there's no such dependent.

> This job is supposed to be done by any decent encoder, not by a
> wrapper.
> 
> > I think we might want something for this inside lavf somewhere, so
> > that
> > encoders can signal dimension alignment requirements. Some
> > containers
> > (MXF, MOV) support such cropping in a codec-agnostic manner.
> 
> From my own experience, dimension mismatch between codec and
> container
> makes a lot of trouble. ISO base format specification specified how
> to
> crop/scale after decoding clear, however, I don't think it has been
> widely supported, including FFmpeg. We can fix that inside of FFmpeg,
> but we should avoid such cases as much as we can.
> 
> > > If we don’t fix it, either:
> > > 
> > > 1. Reject and fail directly. User complain why.
> > > 2. Accept and keep going. Sometimes it works, sometimes don’t. It
> > > depends on the device and get into a confused situation.
> > > 
> > > I know there are getWidthAlignment()/getHeightAlignment() to get
> > > alignment info of codecs. The results are unreliable. The only
> > > reliable way I can find is don’t depends on those API and fix it
> > > by ourself.
> > 
> > Given how temperamental MC seems to be a "belt and braces" approach
> > might be appropriate when dealing with it. Tell users (ffmpeg.c is
> > a
> > user here) that dimensions must be aligned by 16x16 and then
> > automagically doing the required padding and cropping somewhere
> > (lavf
> > or ffmpeg.c) feels like a decent solution.
> > 
> > /Tomas
> > 
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 3/7] avcodec/mediacodecenc: use bsf to handle crop

[Tomas Härdin]

tor 2022-12-15 klockan 01:37 +0800 skrev Zhao Zhili:
> On Wed, 2022-12-14 at 18:08 +0100, Tomas Härdin wrote:
> 
> > 
> > I think we might want something for this inside lavf somewhere, so
> > that
> > encoders can signal dimension alignment requirements. Some
> > containers
> > (MXF, MOV) support such cropping in a codec-agnostic manner.
> 
> From my own experience, dimension mismatch between codec and
> container
> makes a lot of trouble. ISO base format specification specified how
> to
> crop/scale after decoding clear, however, I don't think it has been
> widely supported, including FFmpeg. We can fix that inside of FFmpeg,
> but we should avoid such cases as much as we can.

This is the difference between stored, sampled and display dimensions
in MXF. For example 1080i video has StoredHeight = 544, SampledHeight =
540 and DisplayHeight = 540 (see AS-10). When you add VBLANK and HBLANK
to the mix then all three dimensions are typically different.

Anyway specifying at the NAL level whenever the essence isn't a
multiple of 16x16 is obviously normal. The only complication I can
think of is 4:2:2 and 4:4:4. Does MC require 16x16 also in those cases?
I'd expect 16x8 and 8x8 respectively.

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 3/7] avcodec/mediacodecenc: use bsf to handle crop

["zhilizhao(赵志立)"]

> On Dec 21, 2022, at 02:24, Tomas Härdin <git@haerdin.se> wrote:
> 
> tor 2022-12-15 klockan 01:37 +0800 skrev Zhao Zhili:
>> On Wed, 2022-12-14 at 18:08 +0100, Tomas Härdin wrote:
>> 
>>> 
>>> I think we might want something for this inside lavf somewhere, so
>>> that
>>> encoders can signal dimension alignment requirements. Some
>>> containers
>>> (MXF, MOV) support such cropping in a codec-agnostic manner.
>> 
>> From my own experience, dimension mismatch between codec and
>> container
>> makes a lot of trouble. ISO base format specification specified how
>> to
>> crop/scale after decoding clear, however, I don't think it has been
>> widely supported, including FFmpeg. We can fix that inside of FFmpeg,
>> but we should avoid such cases as much as we can.
> 
> This is the difference between stored, sampled and display dimensions
> in MXF. For example 1080i video has StoredHeight = 544, SampledHeight =
> 540 and DisplayHeight = 540 (see AS-10). When you add VBLANK and HBLANK
> to the mix then all three dimensions are typically different.
> 
> Anyway specifying at the NAL level whenever the essence isn't a
> multiple of 16x16 is obviously normal. The only complication I can
> think of is 4:2:2 and 4:4:4. Does MC require 16x16 also in those cases?
> I'd expect 16x8 and 8x8 respectively.

It’s still 16x16. From H.264 specification:

macroblock: A 16x16 block of luma samples and two corresponding blocks of
chroma samples of a picture that has three sample arrays, or a 16x16 block
of samples of a monochrome picture or a picture that is coded using three
separate colour planes.

Macroblock has been replaced by coding tree unit with H.265, which can be
between 16×16 pixels and 64×64 pixels in size.

> 
> /Tomas
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 3/7] avcodec/mediacodecenc: use bsf to handle crop

[Tomas Härdin]

ons 2022-12-21 klockan 15:17 +0800 skrev zhilizhao(赵志立):
> 
> 
> > On Dec 21, 2022, at 02:24, Tomas Härdin <git@haerdin.se> wrote:
> > 
> > tor 2022-12-15 klockan 01:37 +0800 skrev Zhao Zhili:
> > > On Wed, 2022-12-14 at 18:08 +0100, Tomas Härdin wrote:
> > > 
> > > > 
> > > > I think we might want something for this inside lavf somewhere,
> > > > so
> > > > that
> > > > encoders can signal dimension alignment requirements. Some
> > > > containers
> > > > (MXF, MOV) support such cropping in a codec-agnostic manner.
> > > 
> > > From my own experience, dimension mismatch between codec and
> > > container
> > > makes a lot of trouble. ISO base format specification specified
> > > how
> > > to
> > > crop/scale after decoding clear, however, I don't think it has
> > > been
> > > widely supported, including FFmpeg. We can fix that inside of
> > > FFmpeg,
> > > but we should avoid such cases as much as we can.
> > 
> > This is the difference between stored, sampled and display
> > dimensions
> > in MXF. For example 1080i video has StoredHeight = 544,
> > SampledHeight =
> > 540 and DisplayHeight = 540 (see AS-10). When you add VBLANK and
> > HBLANK
> > to the mix then all three dimensions are typically different.
> > 
> > Anyway specifying at the NAL level whenever the essence isn't a
> > multiple of 16x16 is obviously normal. The only complication I can
> > think of is 4:2:2 and 4:4:4. Does MC require 16x16 also in those
> > cases?
> > I'd expect 16x8 and 8x8 respectively.
> 
> It’s still 16x16. From H.264 specification:

I stand corrected. I did give the spec a once-over a while back, but
not everything sticks.

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS

[Zhao Zhili]

From: Zhao Zhili <zhilizhao@tencent.com>

Use input PTS as DTS has multiple problems:
1. If there is no reordering, it's better to just use the output
PTS as DTS, since encoder may change the timestamp value (do it
on purpose or rounding error).

2. If there is reordering, input PTS should be shift a few frames
as DTS to satisfy the requirement of PTS >= DTS. I can't find a
reliable way to determine how many frames to be shift. For example,
we don't known if the encoder use hierarchical B frames. The
max_num_reorder_frames can be get from VUI, but VUI is optional.

3. Encoder dropping frames makes the case worse. Android has an
BITRATE_MODE_CBR_FD option to allow it explicitly.
---
 libavcodec/mediacodecenc.c | 28 +---------------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
index 7f2ae88285..8e28a50e0d 100644
--- a/libavcodec/mediacodecenc.c
+++ b/libavcodec/mediacodecenc.c
@@ -64,18 +64,6 @@ typedef struct MediaCodecEncContext {
 
     uint8_t *extradata;
     int extradata_size;
-
-    // Since MediaCodec doesn't output DTS, use a timestamp queue to save pts
-    // of AVFrame and generate DTS for AVPacket.
-    //
-    // This doesn't work when use Surface as input, in that case frames can be
-    // sent to encoder without our notice. One exception is frames come from
-    // our MediaCodec decoder wrapper, since we can control it's render by
-    // av_mediacodec_release_buffer.
-    int64_t timestamps[32];
-    int ts_head;
-    int ts_tail;
-
     int eof_sent;
 
     AVFrame *frame;
@@ -368,11 +356,6 @@ static int mediacodec_receive(AVCodecContext *avctx,
     }
     memcpy(pkt->data + extradata_size, out_buf + out_info.offset, out_info.size);
     pkt->pts = av_rescale_q(out_info.presentationTimeUs, AV_TIME_BASE_Q, avctx->time_base);
-    if (s->ts_tail != s->ts_head) {
-        pkt->dts = s->timestamps[s->ts_tail];
-        s->ts_tail = (s->ts_tail + 1) % FF_ARRAY_ELEMS(s->timestamps);
-    }
-
     if (out_info.flags & ff_AMediaCodec_getBufferFlagKeyFrame(codec))
         pkt->flags |= AV_PKT_FLAG_KEY;
     ret = 0;
@@ -437,14 +420,8 @@ static int mediacodec_send(AVCodecContext *avctx,
             return ff_AMediaCodec_signalEndOfInputStream(codec);
         }
 
-
-        if (frame->data[3]) {
-            pts = av_rescale_q(frame->pts, avctx->time_base, AV_TIME_BASE_Q);
-            s->timestamps[s->ts_head] = frame->pts;
-            s->ts_head = (s->ts_head + 1) % FF_ARRAY_ELEMS(s->timestamps);
-
+        if (frame->data[3])
             av_mediacodec_release_buffer((AVMediaCodecBuffer *)frame->data[3], 1);
-        }
         return 0;
     }
 
@@ -463,9 +440,6 @@ static int mediacodec_send(AVCodecContext *avctx,
         copy_frame_to_buffer(avctx, frame, input_buf, input_size);
 
         pts = av_rescale_q(frame->pts, avctx->time_base, AV_TIME_BASE_Q);
-
-        s->timestamps[s->ts_head] = frame->pts;
-        s->ts_head = (s->ts_head + 1) % FF_ARRAY_ELEMS(s->timestamps);
     } else {
         flags |= ff_AMediaCodec_getBufferFlagEndOfStream(codec);
         s->eof_sent = 1;
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS

[Tomas Härdin]

ons 2022-12-07 klockan 17:31 +0800 skrev Zhao Zhili:
> From: Zhao Zhili <zhilizhao@tencent.com>
> 
> Use input PTS as DTS has multiple problems:
> 1. If there is no reordering, it's better to just use the output
> PTS as DTS, since encoder may change the timestamp value (do it
> on purpose or rounding error).
> 
> 2. If there is reordering, input PTS should be shift a few frames
> as DTS to satisfy the requirement of PTS >= DTS. I can't find a
> reliable way to determine how many frames to be shift. For example,
> we don't known if the encoder use hierarchical B frames. The
> max_num_reorder_frames can be get from VUI, but VUI is optional.
> 
> 3. Encoder dropping frames makes the case worse. Android has an
> BITRATE_MODE_CBR_FD option to allow it explicitly.

Don't we already have code to parse this stuff from h.264 streams?

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS

["zhilizhao(赵志立)"]

> On Dec 12, 2022, at 23:28, Tomas Härdin <git@haerdin.se> wrote:
> 
> ons 2022-12-07 klockan 17:31 +0800 skrev Zhao Zhili:
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> 
>> Use input PTS as DTS has multiple problems:
>> 1. If there is no reordering, it's better to just use the output
>> PTS as DTS, since encoder may change the timestamp value (do it
>> on purpose or rounding error).
>> 
>> 2. If there is reordering, input PTS should be shift a few frames
>> as DTS to satisfy the requirement of PTS >= DTS. I can't find a
>> reliable way to determine how many frames to be shift. For example,
>> we don't known if the encoder use hierarchical B frames. The
>> max_num_reorder_frames can be get from VUI, but VUI is optional.
>> 
>> 3. Encoder dropping frames makes the case worse. Android has an
>> BITRATE_MODE_CBR_FD option to allow it explicitly.
> 
> Don't we already have code to parse this stuff from h.264 streams?

We have code to parse max_num_reorder_frames, if the stream contains
the field.

We have some code to generate/guess DTS but doesn’t work reliably for
H.264/H.265. I guess it’s hard, if not impossible. We don’t know the
prediction structures. Use encoder’s input PTS as DTS works for some
transport protocols like RTP, and MP4 with negative_cts_offsets,
otherwise it’s broken due to PTS >= DTS.

If we can and decided to implement such algorithm, it should go in
a higher level, so remux raw stream works better.

> 
> /Tomas
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS

[Tomas Härdin]

tis 2022-12-13 klockan 10:55 +0800 skrev zhilizhao(赵志立):
> 
> 
> > On Dec 12, 2022, at 23:28, Tomas Härdin <git@haerdin.se> wrote:
> > 
> > ons 2022-12-07 klockan 17:31 +0800 skrev Zhao Zhili:
> > > From: Zhao Zhili <zhilizhao@tencent.com>
> > > 
> > > Use input PTS as DTS has multiple problems:
> > > 1. If there is no reordering, it's better to just use the output
> > > PTS as DTS, since encoder may change the timestamp value (do it
> > > on purpose or rounding error).
> > > 
> > > 2. If there is reordering, input PTS should be shift a few frames
> > > as DTS to satisfy the requirement of PTS >= DTS. I can't find a
> > > reliable way to determine how many frames to be shift. For
> > > example,
> > > we don't known if the encoder use hierarchical B frames. The
> > > max_num_reorder_frames can be get from VUI, but VUI is optional.
> > > 
> > > 3. Encoder dropping frames makes the case worse. Android has an
> > > BITRATE_MODE_CBR_FD option to allow it explicitly.
> > 
> > Don't we already have code to parse this stuff from h.264 streams?
> 
> We have code to parse max_num_reorder_frames, if the stream contains
> the field.
> 
> We have some code to generate/guess DTS but doesn’t work reliably for
> H.264/H.265. I guess it’s hard, if not impossible. We don’t know the
> prediction structures. Use encoder’s input PTS as DTS works for some
> transport protocols like RTP, and MP4 with negative_cts_offsets,
> otherwise it’s broken due to PTS >= DTS.

Right, H.264 and H.265 support rather arbitrary reference structures. I
guess a pessimistic estimate is good enough

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS

[Anton Khirnov]

Does this mean the encoder will produce packets with dts=AV_NOPTS_VALUE?

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [Internet]Re: [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS

["zhilizhao(赵志立)"]

> On Jan 4, 2023, at 18:16, Anton Khirnov <anton@khirnov.net> wrote:
> 
> Does this mean the encoder will produce packets with dts=AV_NOPTS_VALUE?

MediaCodec should not encode B frames by default, so dts = pts by default.
B frames can be enabled explicitly, in that case dts is AV_NOPTS_VALUE.

Android system’s MP4 muxer works very hard to recreate dts to workaround
the limitation of MediaCodec API. It create ‘valid’ but almost useless
files with a lot of jitters.

> 
> -- 
> Anton Khirnov
> 

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [Internet]Re: [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS

[Tomas Härdin]

ons 2023-01-04 klockan 19:31 +0800 skrev zhilizhao(赵志立):
> 
> > On Jan 4, 2023, at 18:16, Anton Khirnov <anton@khirnov.net> wrote:
> > 
> > Does this mean the encoder will produce packets with
> > dts=AV_NOPTS_VALUE?
> 
> MediaCodec should not encode B frames by default, so dts = pts by
> default.
> B frames can be enabled explicitly, in that case dts is
> AV_NOPTS_VALUE.
> 
> Android system’s MP4 muxer works very hard to recreate dts to
> workaround
> the limitation of MediaCodec API. It create ‘valid’ but almost
> useless
> files with a lot of jitters.

Maybe we should disallow B-frames entirely until the MediaCodec API is
fixed so as to not add to this mess?

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [Internet]Re: [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS

[Zhao Zhili]

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Tomas Härdin
> Sent: 2023年1月4日 22:00
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [Internet]Re: [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS
> 
> ons 2023-01-04 klockan 19:31 +0800 skrev zhilizhao(赵志立):
> >
> > > On Jan 4, 2023, at 18:16, Anton Khirnov <anton@khirnov.net> wrote:
> > >
> > > Does this mean the encoder will produce packets with
> > > dts=AV_NOPTS_VALUE?
> >
> > MediaCodec should not encode B frames by default, so dts = pts by
> > default.
> > B frames can be enabled explicitly, in that case dts is
> > AV_NOPTS_VALUE.
> >
> > Android system’s MP4 muxer works very hard to recreate dts to
> > workaround
> > the limitation of MediaCodec API. It create ‘valid’ but almost
> > useless
> > files with a lot of jitters.
> 
> Maybe we should disallow B-frames entirely until the MediaCodec API is
> fixed so as to not add to this mess?

There are some valid usecases. For example, RTP works fine without DTS.
With some restriction or precondition, use can create DTS too. So I'd like
to keep the feature.

> 
> /Tomas
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [Internet]Re: [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS

[Anton Khirnov]

Quoting Zhao Zhili (2023-01-04 15:46:52)
> 
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Tomas Härdin
> > Sent: 2023年1月4日 22:00
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [Internet]Re: [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS
> > 
> > ons 2023-01-04 klockan 19:31 +0800 skrev zhilizhao(赵志立):
> > >
> > > > On Jan 4, 2023, at 18:16, Anton Khirnov <anton@khirnov.net> wrote:
> > > >
> > > > Does this mean the encoder will produce packets with
> > > > dts=AV_NOPTS_VALUE?
> > >
> > > MediaCodec should not encode B frames by default, so dts = pts by
> > > default.
> > > B frames can be enabled explicitly, in that case dts is
> > > AV_NOPTS_VALUE.
> > >
> > > Android system’s MP4 muxer works very hard to recreate dts to
> > > workaround
> > > the limitation of MediaCodec API. It create ‘valid’ but almost
> > > useless
> > > files with a lot of jitters.
> > 
> > Maybe we should disallow B-frames entirely until the MediaCodec API is
> > fixed so as to not add to this mess?
> 
> There are some valid usecases. For example, RTP works fine without DTS.
> With some restriction or precondition, use can create DTS too. So I'd like
> to keep the feature.

It is currently an API guarantee that all encoders return valid DTS
values, so this encoder is behaving in an invalid way.

If you want to keep B-frame functionality in this half-working state,
I'd say it should be hidden behind something like -strict experimental.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [Internet]Re: [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS

[Zhao Zhili]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton Khirnov
> Sent: 2023年1月4日 23:16
> To: 'FFmpeg development discussions and patches' <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [Internet]Re: [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS
> 
> Quoting Zhao Zhili (2023-01-04 15:46:52)
> >
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Tomas Härdin
> > > Sent: 2023年1月4日 22:00
> > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [Internet]Re: [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS
> > >
> > > ons 2023-01-04 klockan 19:31 +0800 skrev zhilizhao(赵志立):
> > > >
> > > > > On Jan 4, 2023, at 18:16, Anton Khirnov <anton@khirnov.net> wrote:
> > > > >
> > > > > Does this mean the encoder will produce packets with
> > > > > dts=AV_NOPTS_VALUE?
> > > >
> > > > MediaCodec should not encode B frames by default, so dts = pts by
> > > > default.
> > > > B frames can be enabled explicitly, in that case dts is
> > > > AV_NOPTS_VALUE.
> > > >
> > > > Android system’s MP4 muxer works very hard to recreate dts to
> > > > workaround
> > > > the limitation of MediaCodec API. It create ‘valid’ but almost
> > > > useless
> > > > files with a lot of jitters.
> > >
> > > Maybe we should disallow B-frames entirely until the MediaCodec API is
> > > fixed so as to not add to this mess?
> >
> > There are some valid usecases. For example, RTP works fine without DTS.
> > With some restriction or precondition, use can create DTS too. So I'd like
> > to keep the feature.
> 
> It is currently an API guarantee that all encoders return valid DTS
> values, so this encoder is behaving in an invalid way.

It's an reasonable requirement, but could you elaborate on where is the doc
explicitly says that?

> 
> If you want to keep B-frame functionality in this half-working state,
> I'd say it should be hidden behind something like -strict experimental.

Will do.

> 
> --
> Anton Khirnov
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [Internet]Re: [PATCH v2 5/7] avcodec/mediacodecenc: remove the strategy to create DTS

[Anton Khirnov]

Quoting Zhao Zhili (2023-01-04 17:12:56)
> > It is currently an API guarantee that all encoders return valid DTS
> > values, so this encoder is behaving in an invalid way.
> 
> It's an reasonable requirement, but could you elaborate on where is the doc
> explicitly says that?

I am not aware of this being explicitly documented, but then our
documentation is sadly far from complete.

But this was one of the major reasons for adding avcodec_encode_video2()
back in 2012 (later replaced by avcodec_send_frame()+receive_packet()).
Also see 8de1ee9f725aa3c550f425bd3120bcd95d5b2ea8.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 6/7] avcodec/mediacodecenc: add max-bframes support

[Zhao Zhili]

From: Zhao Zhili <zhilizhao@tencent.com>

---
 libavcodec/mediacodecenc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
index 8e28a50e0d..e9cff8167a 100644
--- a/libavcodec/mediacodecenc.c
+++ b/libavcodec/mediacodecenc.c
@@ -264,6 +264,8 @@ static av_cold int mediacodec_init(AVCodecContext *avctx)
         av_log(avctx, AV_LOG_DEBUG, "set level to 0x%x\n", s->level);
         ff_AMediaFormat_setInt32(format, "level", s->level);
     }
+    if (avctx->max_b_frames > 0)
+        ff_AMediaFormat_setInt32(format, "max-bframes", avctx->max_b_frames);
 
     ret = ff_AMediaCodec_getConfigureFlagEncode(s->codec);
     ret = ff_AMediaCodec_configure(s->codec, format, s->window, NULL, ret);
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 7/7] avcodec/mediacodecenc: add pts_as_dts option

[Zhao Zhili]

From: Zhao Zhili <zhilizhao@tencent.com>

It works since most of Android devices don't output B frames by
default. The behavior is documented by Android now, although there
is some exception in history, which should have been fixed now.
---
 libavcodec/mediacodecenc.c | 8 ++++++++
 libavcodec/version.h       | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
index e9cff8167a..41b7afe23d 100644
--- a/libavcodec/mediacodecenc.c
+++ b/libavcodec/mediacodecenc.c
@@ -71,6 +71,7 @@ typedef struct MediaCodecEncContext {
 
     int bitrate_mode;
     int level;
+    int pts_as_dts;
 } MediaCodecEncContext;
 
 enum {
@@ -266,6 +267,8 @@ static av_cold int mediacodec_init(AVCodecContext *avctx)
     }
     if (avctx->max_b_frames > 0)
         ff_AMediaFormat_setInt32(format, "max-bframes", avctx->max_b_frames);
+    if (s->pts_as_dts == -1)
+        s->pts_as_dts = avctx->max_b_frames <= 0;
 
     ret = ff_AMediaCodec_getConfigureFlagEncode(s->codec);
     ret = ff_AMediaCodec_configure(s->codec, format, s->window, NULL, ret);
@@ -358,6 +361,8 @@ static int mediacodec_receive(AVCodecContext *avctx,
     }
     memcpy(pkt->data + extradata_size, out_buf + out_info.offset, out_info.size);
     pkt->pts = av_rescale_q(out_info.presentationTimeUs, AV_TIME_BASE_Q, avctx->time_base);
+    if (s->pts_as_dts)
+        pkt->dts = pkt->pts;
     if (out_info.flags & ff_AMediaCodec_getBufferFlagKeyFrame(codec))
         pkt->flags |= AV_PKT_FLAG_KEY;
     ret = 0;
@@ -548,6 +553,9 @@ static const AVCodecHWConfigInternal *const mediacodec_hw_configs[] = {
                     0, AV_OPT_TYPE_CONST, {.i64 = BITRATE_MODE_CBR}, 0, 0, VE, "bitrate_mode" },            \
     { "cbr_fd", "Constant bitrate mode with frame drops",                                                   \
                     0, AV_OPT_TYPE_CONST, {.i64 = BITRATE_MODE_CBR_FD}, 0, 0, VE, "bitrate_mode" },         \
+    { "pts_as_dts", "Use PTS as DTS. It is enabled automatically if avctx max_b_frames <= 0, "              \
+                    "since most of Android devices don't output B frames by default.",                                 \
+                    OFFSET(pts_as_dts), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE },                         \
 
 
 #define MEDIACODEC_ENCODER_CLASS(name)              \
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 61bdf5806b..dd90cd1335 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -30,7 +30,7 @@
 #include "version_major.h"
 
 #define LIBAVCODEC_VERSION_MINOR  54
-#define LIBAVCODEC_VERSION_MICRO 103
+#define LIBAVCODEC_VERSION_MICRO 104
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
                                                LIBAVCODEC_VERSION_MINOR, \
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".