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

Jeyapal, Karthick kjeyapal at akamai.com
Fri Nov 24 17:35:41 EET 2017


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





More information about the ffmpeg-devel mailing list