>On 11/24/17, 9:06 PM, "Jeyapal, Karthick" <kjeya...@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" <s...@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

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to