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

If AFD side data is present, include it in an H.264 SEI payload when
encoding with libx264.

This is done in the same manner that we currently handle A53 closed
captions (where the business logic for constructing the SEI is in
libavcodec/utils.c), so it should be portable to the other encoder
types (i.e. videotoolbox, etc).

Updated to reflect feedback from Derek Buitenhuis <derek.buitenhuis@gmail.com>

Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
 libavcodec/internal.h |  3 +++
 libavcodec/libx264.c  | 47 +++++++++++++++++++++++++++++++++++++++++++----
 libavcodec/utils.c    | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 4 deletions(-)

On 12/29/2017 10:12 AM, Devin Heitmueller wrote:
> If AFD side data is present, include it in an H.264 SEI payload when
> encoding with libx264.
> 
> This is done in the same manner that we currently handle A53 closed
> captions (where the business logic for constructing the SEI is in
> libavcodec/utils.c), so it should be portable to the other encoder
> types (i.e. videotoolbox, etc).
> 
> Updated to reflect feedback from Derek Buitenhuis <derek.buitenhuis@gmail.com>
> 
> Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
> ---
>   libavcodec/internal.h |  3 +++
>   libavcodec/libx264.c  | 47 +++++++++++++++++++++++++++++++++++++++++++----
>   libavcodec/utils.c    | 36 ++++++++++++++++++++++++++++++++++++
>   3 files changed, 82 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index 6deaf1d204..4db33eb020 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -408,6 +408,9 @@ int ff_side_data_set_encoder_stats(AVPacket *pkt, int quality, int64_t *error, i
>   int ff_alloc_a53_sei(const AVFrame *frame, size_t prefix_len,
>                        void **data, size_t *sei_size);
>   
> +int ff_alloc_afd_sei(const AVFrame *frame, size_t prefix_len,
> +                     void **data, size_t *sei_size);
> +
>   /**
>    * Get an estimated video bitrate based on frame size, frame rate and coded
>    * bits per pixel.
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index 9c67c91f33..9a9667079e 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -86,6 +86,7 @@ typedef struct X264Context {
>       int forced_idr;
>       int coder;
>       int a53_cc;
> +    int afd;
>       int b_frame_strategy;
>       int chroma_offset;
>       int scenechange_threshold;
> @@ -275,6 +276,7 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, const AVFrame *frame,
>       x264_nal_t *nal;
>       int nnal, i, ret;
>       x264_picture_t pic_out = {0};
> +    int num_payloads = 0;
>       int pict_type;
>   
>       x264_picture_init( &x4->pic );
> @@ -323,10 +325,46 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, const AVFrame *frame,
>                   } else {
>                       x4->pic.extra_sei.sei_free = av_free;
>   
> -                    x4->pic.extra_sei.payloads[0].payload_size = sei_size;
> -                    x4->pic.extra_sei.payloads[0].payload = sei_data;
> -                    x4->pic.extra_sei.num_payloads = 1;
> -                    x4->pic.extra_sei.payloads[0].payload_type = 4;
> +                    x4->pic.extra_sei.payloads[num_payloads].payload_size = sei_size;
> +                    x4->pic.extra_sei.payloads[num_payloads].payload = sei_data;
> +                    x4->pic.extra_sei.payloads[num_payloads].payload_type = 4;
> +                    x4->pic.extra_sei.num_payloads++;
> +                    num_payloads++;
> +                }
> +            }
> +        }
> +
> +        /* Active Format Description */
> +        if (x4->afd) {
> +            void *sei_data;
> +            size_t sei_size;
> +
> +            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.

> +                return AVERROR(ENOMEM);
> +            } else if (sei_data) {
> +                x264_sei_payload_t *payloads;
> +                payloads = av_realloc(x4->pic.extra_sei.payloads,
> +                                      sizeof(x4->pic.extra_sei.payloads[0]) * (num_payloads + 1));
> +                if (payloads == NULL) {
> +                    av_log(ctx, AV_LOG_ERROR, "Not enough memory for AFD, skipping\n");
> +                    for (i = 0; i < num_payloads; i++)
> +                        av_free(x4->pic.extra_sei.payloads[i].payload);
> +                    av_free(x4->pic.extra_sei.payloads);
> +                    av_free(sei_data);
> +                    return AVERROR(ENOMEM);
> +                } else {
> +                    x4->pic.extra_sei.payloads = payloads;
> +                    x4->pic.extra_sei.sei_free = av_free;
> +
> +                    x4->pic.extra_sei.payloads[num_payloads].payload_size = sei_size;
> +                    x4->pic.extra_sei.payloads[num_payloads].payload = sei_data;
> +                    x4->pic.extra_sei.payloads[num_payloads].payload_type = 4;
> +                    x4->pic.extra_sei.num_payloads++;
> +                    num_payloads++;
>                   }
>               }
>           }
> @@ -892,6 +930,7 @@ static const AVOption options[] = {
>       {"passlogfile", "Filename for 2 pass stats", OFFSET(stats), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
>       {"wpredp", "Weighted prediction for P-frames", OFFSET(wpredp), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
>       {"a53cc",          "Use A53 Closed Captions (if available)",          OFFSET(a53_cc),        AV_OPT_TYPE_BOOL,   {.i64 = 1}, 0, 1, VE},
> +    {"afd",            "Use Active Format Description (AFD) (if available)",OFFSET(afd),        AV_OPT_TYPE_BOOL,   {.i64 = 1}, 0, 1, VE},
>       {"x264opts", "x264 options", OFFSET(x264opts), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
>       { "crf",           "Select the quality for constant quality mode",    OFFSET(crf),           AV_OPT_TYPE_FLOAT,  {.dbl = -1 }, -1, FLT_MAX, VE },
>       { "crf_max",       "In CRF mode, prevents VBV from lowering quality beyond this point.",OFFSET(crf_max), AV_OPT_TYPE_FLOAT, {.dbl = -1 }, -1, FLT_MAX, VE },
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index baf09119fe..5fc925b8cd 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -2403,6 +2403,42 @@ int ff_alloc_a53_sei(const AVFrame *frame, size_t prefix_len,
>       return 0;
>   }
>   
> +/* Defined in SCTE 128-1 2013 Sec 8.1 */
> +int ff_alloc_afd_sei(const AVFrame *frame, size_t prefix_len,
> +                     void **data, size_t *sei_size)
> +{
> +    AVFrameSideData *side_data = NULL;
> +    uint8_t *sei_data;
> +
> +    if (frame)
> +        side_data = av_frame_get_side_data(frame, AV_FRAME_DATA_AFD);
> +
> +    if (!side_data) {
> +        *data = NULL;
> +        return 0;
> +    }
> +
> +    *sei_size = 9;
> +    *data = av_mallocz(*sei_size + prefix_len);
> +    if (!*data)
> +        return AVERROR(ENOMEM);
> +    sei_data = (uint8_t*)*data + prefix_len;
> +
> +    /* country code (SCTE 128-1 Sec 8.1.1) */
> +    sei_data[0] = 181;
> +    sei_data[1] = 0;
> +    sei_data[2] = 49;
> +
> +    /* country code (SCTE 128-1 Sec 8.1.2) */
> +    AV_WL32(sei_data + 3, MKTAG('D', 'T', 'G', '1'));
> +
> +    /* country code (SCTE 128-1 Sec 8.2.5) */
> +    sei_data[7] = 0x41;
> +    sei_data[8] = 0xf0 | side_data->data[0];
> +
> +    return 0;
> +}
> +
>   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.

Aaron Levinson

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