[FFmpeg-devel] [PATCH 5/8] Support encoding of Active Format Description (AFD) in libx264

[Devin Heitmueller]

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