[FFmpeg-devel] [PATCH v2 2/3] avcodec/libx264:setting profile and level in avcodec context

[Dixit, Vishwanath]

>On 11/24/17, 9:06 PM, "Jeyapal, Karthick" <kjeyapal at akamai.com> wrote:
>
>Thanks for your comments. I have uploaded new patchset v4 with suggested corrections.
>Please ignore patchset v3.
>
>On 11/24/17, 4:26 PM, "Mark Thompson" <sw at jkqxz.net> wrote:
>[…]
>>> +    s = x264_encoder_headers(x4->enc, &nal, &nnal);
>>> +    avctx->profile = nal->p_payload[5];
>>
>>AVCodecContext.profile should include some of the constraint_set_flags - see <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/avcodec.h#l2842>.
>Great! I was not aware of this. This would also resolve my constraint_set_flags problem in hlsenc.
>>
>>> +    avctx->level = nal->p_payload[7];
>>
>>I don't much like the hard-coding of the offsets here.  Maybe add some asserts so that it fails very quickly if something ever changes?  (I don't think it will with libx264, but if it does then this is going to be putting nonsense in the metadata.)
>Have added asserts to check start code and nal type.
>>
>>>  
>>> -        s = x264_encoder_headers(x4->enc, &nal, &nnal);
>>> +    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
>>>          avctx->extradata = p = av_mallocz(s + AV_INPUT_BUFFER_PADDING_SIZE);
>>>          if (!p)
>>>              return AVERROR(ENOMEM);
>>> 
>>
>>I think I preferred the version which only wrote the value if it isn't already set.  If the user specifies a profile then it should use that or fail.
>I am ok with both approaches. Let us take a final decision on this based on the result of the other patch submitted by carl.
>http://ffmpeg.org/pipermail/ffmpeg-devel/2017-November/220701.html
>
>regards,
>Karthick

I have submitted patch set version v6 https://patchwork.ffmpeg.org/patch/6771/ which writes profile and level only if it is not already set. Earlier Mark Thompson had objected to setting profile and level unconditionally, and Carl sent a patch in avcodec.h to change the API definition in order to set profile and level by encoders. http://ffmpeg.org/pipermail/ffmpeg-devel/2017-November/220701.html. But that patch has not been merged yet and that discussion has gone cold. In the interest of some resolution for this patchset, we propose that this patch(v6) which writes profile and level only if is not set, as the patch with least objections. In this way, the current API behavior is not changed. Could this patchset atleast be merged? It doesn’t make sense to hold off merging the entire feature, till the API definition is changed.

Regards,
Vishwanath