On 09/02/17 20:54, Mark Thompson wrote:
> On 08/02/17 19:21, Takayuki 'January June' Suwa wrote:
>> From: Takayuki 'January June' Suwa <jjs...@users.noreply.github.com>
>>
>> This adds "-profile[:v] profile_name"-style option IOW.
>>
>> Now default/unknown profile means FF_PROFILE_H264_HIGH strictly, for both
>> better coding style and preserving the original behavior.
>> ---
>> libavcodec/omx.c | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/libavcodec/omx.c b/libavcodec/omx.c
>> index 16df50e..6ce71e9 100644
>> --- a/libavcodec/omx.c
>> +++ b/libavcodec/omx.c
>> @@ -226,6 +226,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,6 +524,21 @@ 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 FF_PROFILE_H264_BASELINE:
>> + avctx->profile = s->profile;
>
> AVCodecContext.profile is a user-set input field of AVCodecContext for
> encoders - you should only be reading it here, not writing to it (see
> <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/avcodec.h;h=1e681e989bef52d56717af78705c78f4b170b30c;hb=HEAD#l3188>).
>
> I think the right thing to do here is to follow libx264 and do:
>
> if (s->profile is set) {
> use s->profile;
> } else if (avctx->profile is set) {
> use avctx->profile;
> } else {
> use the default profile (i.e. high);
> }
>
> See
> <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/libx264.c;h=b11ede61988639ea82f7f8f378ef45792fda779d;hb=HEAD#l676>
> (the default is hidden inside libx264 by just not specifying the profile at
> all). While this ends up being equivalent for the ffmpeg utility, it makes
> it easier and more consistent for lavc users because they can use the common
> option to all encoders rather than having to set a private option for this
> encoder.
To clarify, since that was a bit unclear: if the user hasn't supplied any
particular profile then the default should be to not set the profile parameter
at all (i.e. let the OpenMAX implementation choose).
>> + avc.eProfile = OMX_VIDEO_AVCProfileBaseline;
>> + break;
>> + case FF_PROFILE_H264_MAIN:
>> + avctx->profile = s->profile;
>> + avc.eProfile = OMX_VIDEO_AVCProfileMain;
>> + break;
>> + case FF_PROFILE_H264_HIGH:
>> + default:
>> + avctx->profile = s->profile;
>> + avc.eProfile = OMX_VIDEO_AVCProfileHigh;
>> + break;
>> + }
>> err = OMX_SetParameter(s->handle, OMX_IndexParamVideoAvc, &avc);
>> CHECK(err);
>> }
>> @@ -884,6 +900,10 @@ 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 = 0 }, 0,
>> FF_PROFILE_H264_HIGH, VE, "profile" },
>> + { "baseline", "", 0,
>> AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_H264_BASELINE }, 0, 0, VE, "profile"
>> },
>> + { "main", "", 0,
>> AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_H264_MAIN }, 0, 0, VE, "profile"
>> },
>> + { "high", "", 0,
>> AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_H264_HIGH }, 0, 0, VE, "profile"
>> },
>> { NULL }
>> };
>
> The options look good to me now. (Slightly disappointed that it's baseline
> rather than constrained baseline, but I can see that that's an OpenMAX
> problem which we can't solve here.)
>
> Thanks,
>
> - Mark
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
