Hi Aaron,

>> +            ret = ff_alloc_afd_sei(frame, 0, &sei_data, &sei_size);
>> +            if (ret < 0) {
>> +                for (i = 0; i < num_payloads; i++)
>> +                    av_free(x4->pic.extra_sei.payloads[i].payload);
>> +                av_free(x4->pic.extra_sei.payloads);
> 
> Seems like it would be appropriate to use av_freep() for the last one to make 
> sure that payloads is zeroed out before returning.  Applies in other places.

No objection.

>>  int64_t ff_guess_coded_bitrate(AVCodecContext *avctx)
>>  {
>>      AVRational framerate = avctx->framerate;
> 
> I don't know enough about this to review the technical aspects of this code, 
> but I will say that these changes don't help those users that are encoding in 
> H.264 but using hardware encoders such as Intel QuickSync.

Correct.  As indicated in the original patch description, other encoders would 
need to be modified to take advantage of the feature if desired.  I’ll be doing 
VA-API soon enough since that is a use case I need, but the maintainers of the 
other hwaccel will have to decide whether to support it.  As with the A53 SEI, 
the business for constructing the payload is in utils.c, but each encoder will 
have to add code to send the SEI to the encoder (the means of which is specific 
to the encoder and cannot be common code).

If the other encoders make no changes then the AFD payload will be silently 
discarded, hence they are no worse off then they are today.

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

Reply via email to