rkmppdec: allow user to force drm:prime format when decoding to prevent #27
Conversation
troublesome players to pick up wrong format
|
i mean something like below, i did not test it, but i think you got what i meant. if someone is interested in softframes, he should use software decoder, or a filter which can exchange in between. I am saying this by experience, because those workarounds and demands do not end. diff --git a/libavcodec/rkmppdec.c b/libavcodec/rkmppdec.c
index 0bd5e66..81c8aa1 100644
--- a/libavcodec/rkmppdec.c
+++ b/libavcodec/rkmppdec.c
@@ -141,59 +141,13 @@
RKMPPDecContext *r = avctx->priv_data;
MppCodingType coding_type = MPP_VIDEO_CodingUnused;
const char *opts_env = NULL;
- int ret, is_fmt_supported = 0;
- enum AVPixelFormat pix_fmts[3] = { AV_PIX_FMT_DRM_PRIME,
- AV_PIX_FMT_NV12,
- AV_PIX_FMT_NONE };
+ int ret;
opts_env = getenv("FFMPEG_RKMPP_DEC_OPT");
if (opts_env && av_set_options_string(r, opts_env, "=", " ") <= 0)
av_log(avctx, AV_LOG_WARNING, "Unable to set decoder options from env\n");
- switch (avctx->pix_fmt) {
- case AV_PIX_FMT_YUV420P:
- case AV_PIX_FMT_YUVJ420P:
- is_fmt_supported = 1;
- break;
- case AV_PIX_FMT_YUV420P10:
- is_fmt_supported =
- avctx->codec_id == AV_CODEC_ID_H264 ||
- avctx->codec_id == AV_CODEC_ID_HEVC ||
- avctx->codec_id == AV_CODEC_ID_VP9 ||
- avctx->codec_id == AV_CODEC_ID_AV1;
- break;
- case AV_PIX_FMT_YUV422P:
- case AV_PIX_FMT_YUV422P10:
- is_fmt_supported =
- avctx->codec_id == AV_CODEC_ID_H264;
- break;
- case AV_PIX_FMT_NONE: /* fallback to drm_prime */
- is_fmt_supported = 1;
- avctx->pix_fmt = AV_PIX_FMT_DRM_PRIME;
- break;
- default:
- is_fmt_supported = 0;
- break;
- }
-
- if (r->forcedrm){
- is_fmt_supported = 1;
- avctx->pix_fmt = AV_PIX_FMT_DRM_PRIME;
- }
-
- if (avctx->pix_fmt != AV_PIX_FMT_DRM_PRIME) {
- if (!is_fmt_supported) {
- av_log(avctx, AV_LOG_ERROR, "MPP doesn't support codec '%s' with pix_fmt '%s'\n",
- avcodec_get_name(avctx->codec_id), av_get_pix_fmt_name(avctx->pix_fmt));
- return AVERROR(ENOSYS);
- }
-
- if ((ret = ff_get_format(avctx, pix_fmts)) < 0) {
- av_log(avctx, AV_LOG_ERROR, "ff_get_format failed: %d\n", ret);
- return ret;
- }
- avctx->pix_fmt = ret;
- }
+ avctx->pix_fmt = AV_PIX_FMT_DRM_PRIME;
if ((coding_type = rkmpp_get_coding_type(avctx)) == MPP_VIDEO_CodingUnused) {
av_log(avctx, AV_LOG_ERROR, "Unknown codec id: %d\n", avctx->codec_id);
@@ -224,9 +178,6 @@
goto fail;
}
- if (avctx->pix_fmt != AV_PIX_FMT_DRM_PRIME)
- r->afbc = 0;
-
if (r->afbc == RKMPP_DEC_AFBC_ON_RGA) {
#if CONFIG_RKRGA
const char *rga_ver = querystring(RGA_VERSION);
@@ -693,9 +644,6 @@
int fast_parse = r->fast_parse;
int mpp_frame_mode = mpp_frame_get_mode(mpp_frame);
const MppFrameFormat mpp_fmt = mpp_frame_get_fmt(mpp_frame);
- enum AVPixelFormat pix_fmts[3] = { AV_PIX_FMT_DRM_PRIME,
- AV_PIX_FMT_NONE,
- AV_PIX_FMT_NONE };
av_log(avctx, AV_LOG_VERBOSE, "Noticed an info change\n");
@@ -704,16 +652,7 @@
r->afbc = 0;
}
- pix_fmts[1] = rkmpp_get_av_format(mpp_fmt & MPP_FRAME_FMT_MASK);
-
- if (avctx->pix_fmt == AV_PIX_FMT_DRM_PRIME)
- avctx->sw_pix_fmt = pix_fmts[1];
- else {
- if ((ret = ff_get_format(avctx, pix_fmts)) < 0)
- goto exit;
- avctx->pix_fmt = ret;
- }
-
+ avctx->sw_pix_fmt = kmpp_get_av_format(mpp_fmt & MPP_FRAME_FMT_MASK);
avctx->width = mpp_frame_get_width(mpp_frame);
avctx->height = mpp_frame_get_height(mpp_frame);
avctx->coded_width = FFALIGN(avctx->width, 64);
@@ -728,7 +667,7 @@
av_get_pix_fmt_name(avctx->pix_fmt),
av_get_pix_fmt_name(avctx->sw_pix_fmt));
- if ((ret = rkmpp_set_buffer_group(avctx, pix_fmts[1], avctx->width, avctx->height)) < 0)
+ if ((ret = rkmpp_set_buffer_group(avctx, avctx->sw_pix_fmt, avctx->width, avctx->height)) < 0)
goto exit;
/* Disable fast parsing for the interlaced video */
@@ -753,53 +692,9 @@
av_log(avctx, AV_LOG_DEBUG, "Received a frame\n");
r->errinfo_cnt = 0;
- switch (avctx->pix_fmt) {
- case AV_PIX_FMT_DRM_PRIME:
- {
- if ((ret = rkmpp_export_frame(avctx, frame, mpp_frame)) < 0)
- goto exit;
- return 0;
- }
- break;
- case AV_PIX_FMT_NV12:
- case AV_PIX_FMT_NV16:
- case AV_PIX_FMT_NV15:
- case AV_PIX_FMT_NV20:
- {
- AVFrame *tmp_frame = av_frame_alloc();
- if (!tmp_frame) {
- ret = AVERROR(ENOMEM);
- goto exit;
- }
- if ((ret = rkmpp_export_frame(avctx, tmp_frame, mpp_frame)) < 0)
- goto exit;
-
- if ((ret = ff_get_buffer(avctx, frame, 0)) < 0) {
- av_log(avctx, AV_LOG_ERROR, "ff_get_buffer failed: %d\n", ret);
- av_frame_free(&tmp_frame);
- goto exit;
- }
- if ((ret = av_hwframe_transfer_data(frame, tmp_frame, 0)) < 0) {
- av_log(avctx, AV_LOG_ERROR, "av_hwframe_transfer_data failed: %d\n", ret);
- av_frame_free(&tmp_frame);
- goto exit;
- }
- if ((ret = av_frame_copy_props(frame, tmp_frame)) < 0) {
- av_log(avctx, AV_LOG_ERROR, "av_frame_copy_props failed: %d\n", ret);
- av_frame_free(&tmp_frame);
- goto exit;
- }
- av_frame_free(&tmp_frame);
- return 0;
- }
- break;
- default:
- {
- ret = AVERROR_BUG;
- goto exit;
- }
- break;
- }
+ if ((ret = rkmpp_export_frame(avctx, frame, mpp_frame)) < 0)
+ goto exit;
+ return 0;
}
exit:
diff --git a/libavcodec/rkmppdec.h b/libavcodec/rkmppdec.h
index e016804..77aa0cb 100644
--- a/libavcodec/rkmppdec.h
+++ b/libavcodec/rkmppdec.h
@@ -104,10 +104,6 @@
};
static const enum AVPixelFormat rkmpp_dec_pix_fmts[] = {
- AV_PIX_FMT_NV12,
- AV_PIX_FMT_NV16,
- AV_PIX_FMT_NV15,
- AV_PIX_FMT_NV20,
AV_PIX_FMT_DRM_PRIME,
AV_PIX_FMT_NONE,
};
|
|
Only set the That means swapping the diff --git a/video/decode/vd_lavc.c b/video/decode/vd_lavc.c
index 2106dcb..86e7ed0 100644
--- a/video/decode/vd_lavc.c
+++ b/video/decode/vd_lavc.c
@@ -335,8 +335,28 @@ static void add_all_hwdec_methods(struct hwdec_info **infos, int *num_infos)
if (!cfg)
break;
- if ((cfg->methods & AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX) ||
- (cfg->methods & AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX))
+ if (cfg->methods & AV_CODEC_HW_CONFIG_METHOD_INTERNAL) {
+ struct hwdec_info info = info_template;
+ info.pix_fmt = cfg->pix_fmt;
+
+ const char *name = wrapper;
+ if (!name)
+ name = av_get_pix_fmt_name(info.pix_fmt);
+ assert(name); // API violation by libavcodec
+
+ snprintf(info.method_name, sizeof(info.method_name), "%s", name);
+
+ // Direct variant.
+ add_hwdec_item(infos, num_infos, info);
+
+ // Copy variant.
+ info.copying = true;
+ info.pix_fmt = AV_PIX_FMT_NONE; // trust it can do sw output
+ add_hwdec_item(infos, num_infos, info);
+
+ found_any = true;
+ } else if ((cfg->methods & AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX) ||
+ (cfg->methods & AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX))
{
struct hwdec_info info = info_template;
info.lavc_device = cfg->device_type;
@@ -372,26 +392,6 @@ static void add_all_hwdec_methods(struct hwdec_info **infos, int *num_infos)
}
add_hwdec_item(infos, num_infos, info);
- found_any = true;
- } else if (cfg->methods & AV_CODEC_HW_CONFIG_METHOD_INTERNAL) {
- struct hwdec_info info = info_template;
- info.pix_fmt = cfg->pix_fmt;
-
- const char *name = wrapper;
- if (!name)
- name = av_get_pix_fmt_name(info.pix_fmt);
- assert(name); // API violation by libavcodec
-
- snprintf(info.method_name, sizeof(info.method_name), "%s", name);
-
- // Direct variant.
- add_hwdec_item(infos, num_infos, info);
-
- // Copy variant.
- info.copying = true;
- info.pix_fmt = AV_PIX_FMT_NONE; // trust it can do sw output
- add_hwdec_item(infos, num_infos, info);
-
found_any = true;
}
} |
|
i had traced it with gdb last night, and noticed that it was actually receiving the pix_fmt from codec parameters, and that is received from the the demuxer, and that was received from the container itself. I will check those ones and your findings as well when i have the environment. What do you think about removing soft frames totally? Are there use cases for those? |
I also have a similar guess. The direct reason for this is that the container types and attributes are different - m3u8/hls vs mp4/mkv, which causes them to run into different logical branches in MPV.
Removing it means that some users can no longer use rkmpp in If they are forced to use a software decoder, these RK3588 users will be in the same situation as Raspberry Pi 5 users. Therefore, I think this decision is a bit too aggressive. |
|
Yeah true, but still i have questions why decode init accepts yuv420p or any other format which is not declared as supported in FFCodec->pix_fmts, yet when yuv420p requested it returns nv12 anyways. Is this correct usage of api? I would expect when the client asks yuv420p, codec would call choose_format callback set by the client, if it returns a format which is valid in FFCodec->pixfmts (in this case closest nv12) then continue with nv12, if the client still insists on yuv420p, then codec should not initialize, because codec never returns yuv420p. What i think is, client should fallback to nv12 if it can handle the conversion, not the decoder, because decoder can never know that the client can handle nv12->yuv420 filters. Ffplay handles this with grapj but thats not the norm, i guess Ps: i will be on vacation next week, will not be able look into this. |
as in case some players can not pickup the right format.
in the cases where it is obvious like moonlight i create mainline PRs to players but there are such corner cases as mpv's i really dont know where the exact issue is.
ie: mpv picks up
correct drm_prime format for https://www.youtube.com/watch?v=aqz-KE-bpKQ, vp9 with mp4
but not for https://youtu.be/BJ3Yv572V1A, h264 with hls. choses nv12.
could be even a demuxer/container issue, who knows.
can we provide such a workaround for those cases?
as a side note, i would just suggest completely removing any form of format which is not drm_prime. It saves from future pain.