On 04/02/17 20:26, Takayuki 'January June' Suwa wrote:
> This adds "-profile[:v] profile_name"-style option IOW.
> "constrained," "baseline," "main" and "high" will work well on Raspbian
> Jessie / RasPi3B.
> The others aren't checked due to unsupported.
The idea of this patch looks sensible to me, but I'm not sure about some of the
profile details.
> ---
> libavcodec/omx.c | 91
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 91 insertions(+)
>
> diff --git a/libavcodec/omx.c b/libavcodec/omx.c
> index 16df50e..31a6627 100644
> --- a/libavcodec/omx.c
> +++ b/libavcodec/omx.c
> @@ -44,6 +44,20 @@
> #include "h264.h"
> #include "internal.h"
>
> +enum {
> + H264_OMX_PROFILE_CONSTRAINED = 0,
> + H264_OMX_PROFILE_BASELINE,
> + H264_OMX_PROFILE_MAIN,
> + H264_OMX_PROFILE_EXTENDED,
> + H264_OMX_PROFILE_HIGH,
> + H264_OMX_PROFILE_HIGH10,
> + H264_OMX_PROFILE_HIGH10INTRA,
> + H264_OMX_PROFILE_HIGH422,
> + H264_OMX_PROFILE_HIGH422INTRA,
> + H264_OMX_PROFILE_HIGH444,
> + H264_OMX_PROFILE_HIGH444INTRA,
> +};
You don't need to make a new enum here - FF_PROFILE_* already contains the
values you want.
> +
> #ifdef OMX_SKIP64BIT
> static OMX_TICKS to_omx_ticks(int64_t value)
> {
> @@ -226,6 +240,7 @@ typedef struct OMXCodecContext {
> int output_buf_size;
>
> int input_zerocopy;
> + int profile;
> } OMXCodecContext;
>
> static void append_buffer(pthread_mutex_t *mutex, pthread_cond_t *cond,
> @@ -523,7 +538,71 @@ static av_cold int omx_component_init(AVCodecContext
> *avctx, const char *role)
> CHECK(err);
> avc.nBFrames = 0;
> avc.nPFrames = avctx->gop_size - 1;
> + switch (s->profile) {
> + case H264_OMX_PROFILE_CONSTRAINED:
> + avctx->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;
> + avc.eProfile = OMX_VIDEO_AVCProfileBaseline;
How does OpenMAX divine that constraint_set1_flag should be set in this case?
> + break;
> + case H264_OMX_PROFILE_BASELINE:
> + avctx->profile = FF_PROFILE_H264_BASELINE;
> + avc.eProfile = OMX_VIDEO_AVCProfileBaseline;
> + avc.bEnableFMO = 1;
> + avc.bEnableASO = 1;
> + avc.bEnableRS = 1;
None of these options make any sense without additional configuration. Since
noone uses these features anyway, it is probably easier to just not bother with
baseline profile at all since it can only cause confusion.
(I'm assuming that using baseline profile on your device actually produces a
stream conforming to constrained baseline profile such that not setting
constraint_set1_flag would just be unhelpful - indeed, maybe it always sets it.
If this is wrong (e.g. if ffmpeg is unable to decode the output, since it
doesn't support baseline profile) then please do say so, and also share samples
of the output.)
> + break;
> + case H264_OMX_PROFILE_MAIN:
> + avctx->profile = FF_PROFILE_H264_MAIN;
> + avc.eProfile = OMX_VIDEO_AVCProfileMain;
> + break;
> + case H264_OMX_PROFILE_EXTENDED:
> + avctx->profile = FF_PROFILE_H264_EXTENDED;
> + avc.eProfile = OMX_VIDEO_AVCProfileExtended;
> + avc.bEnableFMO = 1;
> + avc.bEnableASO = 1;
> + avc.bEnableRS = 1;
As for baseline, I don't think this is useful.
> + break;
> + case H264_OMX_PROFILE_HIGH:
> + avctx->profile = FF_PROFILE_H264_HIGH;
> + avc.eProfile = OMX_VIDEO_AVCProfileHigh;
> + break;
> + case H264_OMX_PROFILE_HIGH10:
> + avctx->profile = FF_PROFILE_H264_HIGH_10;
> + avc.eProfile = OMX_VIDEO_AVCProfileHigh10;
> + break;
Isn't this going to want a different pixel format on input? Only YUV420P is
currently in the list of usable pixfmts. Similarly for all of the other
non-8-bit-4:2:0 profiles.
> + case H264_OMX_PROFILE_HIGH10INTRA:
> + avctx->profile = FF_PROFILE_H264_HIGH_10_INTRA;
> + avc.eProfile = OMX_VIDEO_AVCProfileHigh10;
> + avc.bconstIpred = 1;
The OpenMAX IL 1.1.2 specification says: "bconstIpred is a Boolean value that
enables or disables intra-prediction." - I would hope that intra prediction
would always be enabled, not doing so would be madness. From the name it
sounds more like it might control constrained_intra_pred_flag, though it would
be useful to have some clarification before setting it. In any case, it
doesn't appear to be relevant to the profile.
So, since the eProfile value is the same as for high 10, how will OpenMAX
divine that constraint_set3_flag should be set? Similarly for the other intra
profiles below.
> + break;
> + case H264_OMX_PROFILE_HIGH422:
> + avctx->profile = FF_PROFILE_H264_HIGH_422;
> + avc.eProfile = OMX_VIDEO_AVCProfileHigh422;
> + break;
> + case H264_OMX_PROFILE_HIGH422INTRA:
> + avctx->profile = FF_PROFILE_H264_HIGH_422_INTRA;
> + avc.eProfile = OMX_VIDEO_AVCProfileHigh422;
> + avc.bconstIpred = 1;
> + break;
> + case H264_OMX_PROFILE_HIGH444:
> + avctx->profile = FF_PROFILE_H264_HIGH_444;
> + avc.eProfile = OMX_VIDEO_AVCProfileHigh444;
> + break;
> + case H264_OMX_PROFILE_HIGH444INTRA:
> + avctx->profile = FF_PROFILE_H264_HIGH_444_INTRA;
> + avc.eProfile = OMX_VIDEO_AVCProfileHigh444;
> + avc.bconstIpred = 1;
> + break;
> + default:
> + av_log(avctx, AV_LOG_ERROR, "Unknown profile %d\n", s->profile);
> + return AVERROR_UNKNOWN;
> + }
> err = OMX_SetParameter(s->handle, OMX_IndexParamVideoAvc, &avc);
> + switch (err) {
> + case OMX_ErrorUnsupportedSetting:
> + case OMX_ErrorUnsupportedIndex:
> + av_log(avctx, AV_LOG_ERROR, "Unsupported profile %d\n",
> s->profile);
> + return AVERROR_UNKNOWN;
> + }
> CHECK(err);
> }
>
> @@ -884,6 +963,18 @@ static const AVOption options[] = {
> { "omx_libname", "OpenMAX library name", OFFSET(libname),
> AV_OPT_TYPE_STRING, { 0 }, 0, 0, VDE },
> { "omx_libprefix", "OpenMAX library prefix", OFFSET(libprefix),
> AV_OPT_TYPE_STRING, { 0 }, 0, 0, VDE },
> { "zerocopy", "Try to avoid copying input frames if possible",
> OFFSET(input_zerocopy), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, VE },
> + { "profile", "Set the encoding profile", OFFSET(profile),
> AV_OPT_TYPE_INT, { .i64 = H264_OMX_PROFILE_HIGH },
> H264_OMX_PROFILE_CONSTRAINED, H264_OMX_PROFILE_HIGH444INTRA, VE, "profile" },
> + { "constrained", "", 0, AV_OPT_TYPE_CONST, {
> .i64 = H264_OMX_PROFILE_CONSTRAINED }, 0, 0, VE, "profile" },
The profile is called constrained baseline (see H.264 A.2.1.1) - please use
that name.
> + { "baseline", "", 0, AV_OPT_TYPE_CONST, {
> .i64 = H264_OMX_PROFILE_BASELINE }, 0, 0, VE, "profile" },
> + { "main", "", 0, AV_OPT_TYPE_CONST, {
> .i64 = H264_OMX_PROFILE_MAIN }, 0, 0, VE, "profile" },
> + { "extended", "", 0, AV_OPT_TYPE_CONST, {
> .i64 = H264_OMX_PROFILE_EXTENDED }, 0, 0, VE, "profile" },
> + { "high", "", 0, AV_OPT_TYPE_CONST, {
> .i64 = H264_OMX_PROFILE_HIGH }, 0, 0, VE, "profile" },
> + { "high10", "", 0, AV_OPT_TYPE_CONST, {
> .i64 = H264_OMX_PROFILE_HIGH10 }, 0, 0, VE, "profile" },
> + { "high10intra", "", 0, AV_OPT_TYPE_CONST, {
> .i64 = H264_OMX_PROFILE_HIGH10INTRA }, 0, 0, VE, "profile" },
> + { "high422", "", 0, AV_OPT_TYPE_CONST, {
> .i64 = H264_OMX_PROFILE_HIGH422 }, 0, 0, VE, "profile" },
> + { "high422intra", "", 0, AV_OPT_TYPE_CONST, {
> .i64 = H264_OMX_PROFILE_HIGH422INTRA }, 0, 0, VE, "profile" },
> + { "high444", "", 0, AV_OPT_TYPE_CONST, {
> .i64 = H264_OMX_PROFILE_HIGH444 }, 0, 0, VE, "profile" },
> + { "high444intra", "", 0, AV_OPT_TYPE_CONST, {
> .i64 = H264_OMX_PROFILE_HIGH444INTRA }, 0, 0, VE, "profile" },
> { NULL }
> };
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
