From patchwork Mon Feb 15 23:12:42 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Thompson X-Patchwork-Id: 25652 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 93128449248 for ; Tue, 16 Feb 2021 01:20:24 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 54C636881A1; Tue, 16 Feb 2021 01:20:24 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 3F6A1680A4F for ; Tue, 16 Feb 2021 01:20:17 +0200 (EET) Received: by mail-wm1-f48.google.com with SMTP id o24so10847471wmh.5 for ; Mon, 15 Feb 2021 15:20:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jkqxz-net.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=kyJT35E4qIur+ChUUh5mqEwcu94NqcGg8ZgboPeHrD8=; b=PqcXUeMnRSgriaJbnHb5ctQtMlOXXic6lN0zQJ+PojfeEY0abWru58xv5JoD4TKYIS xSkS9KBzaArC5kWTOui/TwuN2jXkoXI1TdHJQv8WDGwRFtM8xZ1gnqh/10hpqZKgW8KT 9gD6QmxqNwscoR3LdmGBScbvLx0Jnk5BljNHJ7DgJdah7XyMpu1ZZ3ti6w46NP2pzEA+ rQlqMzNFRKvi6xb6cYu0/G3z1cDtQ/g6bB+WyH8UERFpZKkvzmx+B3mCEhsTh9OMByUm eMQkdRRkqMa89XqtbEt8vtQQehHB9P2q1dG4JmDnZXI/QLwskH7+wDOQqQvml6+THpgH LjXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=kyJT35E4qIur+ChUUh5mqEwcu94NqcGg8ZgboPeHrD8=; b=cyzI/0v35OBvCZFgjkse3Z5LyeXZcI5QBhRuGRKzT0LPBm6wgLEN3s7nuWL1MA/TAO 3Ou4rF5Qbjl5PO3zBpGumEnJOXOuINWzFMjMo1bupQD5pVoaNh4bHCEFnmk7aYGl8xlu Vq2XpK0azJJ17S90ODRnpEWpo0Se7BHt2PhHSCssXZ65TZMgMsrXgzCOVQn8Sx5vuEEC 6cKxNtaDzuciNOWuCADLD7CgpIQpL7p6tG4Ne0lQcP64CYreyxMmEWi0pN66UdYNCNUh keUVjWn+Sj6LowvJnnSMBTLkwYzrJjOCg9nSApzz6snFhH0PmaxnaCfoNUM2KxLnRZ8y 3BjA== X-Gm-Message-State: AOAM533p/afQ/hcDS4kFySYBXOYMt49fHmHEUm5H0yrZRsDZycerc+BQ HYBW9lj3EdbT6aoVAk/LUg6H3wb9rRUalA== X-Google-Smtp-Source: ABdhPJzccJ7BnE84urF3H+qR3j9fGhXQp/dUD2Z/TIRotOjxpYE3/LfG6eFYhiexOIhR3xaQcp2gMQ== X-Received: by 2002:a05:600c:21ca:: with SMTP id x10mr914914wmj.94.1613430766413; Mon, 15 Feb 2021 15:12:46 -0800 (PST) Received: from [192.168.0.3] (cpc91226-cmbg18-2-0-cust7.5-4.cable.virginm.net. [82.0.29.8]) by smtp.gmail.com with ESMTPSA id w9sm931431wmk.16.2021.02.15.15.12.45 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 15 Feb 2021 15:12:46 -0800 (PST) To: ffmpeg-devel@ffmpeg.org References: <20210125141507.11012-1-nuomi2021@gmail.com> <20210211143611.5663-1-nuomi2021@gmail.com> <20210211143611.5663-2-nuomi2021@gmail.com> <15ee3f05-7275-47f3-92f9-584ee0fbefd6@gmail.com> From: Mark Thompson Message-ID: Date: Mon, 15 Feb 2021 23:12:42 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <15ee3f05-7275-47f3-92f9-584ee0fbefd6@gmail.com> Content-Language: en-US Subject: [FFmpeg-devel] [PATCH v2] cbs_h2645: Implement replace-PS with a table rather than many functions X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" While this is mode source code, it generates less binary code and can be more cleanly extended to H.266. --- On 14/02/2021 19:45, Andreas Rheinhardt wrote: > Nuo Mi: >> From: Mark Thompson >> >> --- >> libavcodec/cbs_h2645.c | 171 +++++++++++++++++++++++++++++++---------- >> 1 file changed, 130 insertions(+), 41 deletions(-) >> >> ... >> +static int cbs_h2645_replace_ps(CodedBitstreamContext *ctx, >> + CodedBitstreamUnit *unit) >> +{ >> + typedef struct { >> + int nal_unit_type; >> + int max_count; >> + const char *name; > > If this were a char name[4], these structs could be put in .rodata > instead of .data (or .data.rel.ro) when using position-independent code. True, that would be nicer. New patch enclosing, which also uses some macros to simplify the arrays (and adds some explanatory comments). This includes the no-active case, so extension to H.266 requires no additional logic: #define H266_PS_TYPE(cname, uname, id_name) \ H2645_PS_TYPE(H266, VVC_NUT_ ## cname, cname, uname, \ VVC_MAX_ ## cname ## _COUNT, \ uname ## _ ## id_name, common) Then, "H266_PS_TYPE(APS, aps, adaptation)", etc. Thanks, - Mark libavcodec/cbs_h2645.c | 161 ++++++++++++++++++++++++++++++----------- 1 file changed, 120 insertions(+), 41 deletions(-) diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c index 6005d46e0d..6c44e781fa 100644 --- a/libavcodec/cbs_h2645.c +++ b/libavcodec/cbs_h2645.c @@ -661,38 +661,117 @@ static int cbs_h2645_split_fragment(CodedBitstreamContext *ctx, return 0; } -#define cbs_h2645_replace_ps(h26n, ps_name, ps_var, id_element) \ -static int cbs_h26 ## h26n ## _replace_ ## ps_var(CodedBitstreamContext *ctx, \ - CodedBitstreamUnit *unit) \ -{ \ - CodedBitstreamH26 ## h26n ## Context *priv = ctx->priv_data; \ - H26 ## h26n ## Raw ## ps_name *ps_var = unit->content; \ - unsigned int id = ps_var->id_element; \ - int err; \ - if (id >= FF_ARRAY_ELEMS(priv->ps_var)) { \ - av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid " #ps_name \ - " id : %d.\n", id); \ - return AVERROR_INVALIDDATA; \ - } \ - err = ff_cbs_make_unit_refcounted(ctx, unit); \ - if (err < 0) \ - return err; \ - if (priv->ps_var[id] == priv->active_ ## ps_var) \ - priv->active_ ## ps_var = NULL ; \ - av_buffer_unref(&priv->ps_var ## _ref[id]); \ - av_assert0(unit->content_ref); \ - priv->ps_var ## _ref[id] = av_buffer_ref(unit->content_ref); \ - if (!priv->ps_var ## _ref[id]) \ - return AVERROR(ENOMEM); \ - priv->ps_var[id] = (H26 ## h26n ## Raw ## ps_name *)priv->ps_var ## _ref[id]->data; \ - return 0; \ -} +static int cbs_h2645_replace_ps(CodedBitstreamContext *ctx, + CodedBitstreamUnit *unit) +{ + typedef struct { + // Codec this parameter set exists in. + enum AVCodecID codec_id; + // The NAL unit type corresponding to this parameter set type. + int nal_unit_type; + // Name of the parameter set. This field is large enough to + // contain a string of the form "XPS". + char name[4]; + // The maximum number of this type of parameter set which might + // be stored. The greatest valid id is also one less than this. + int id_count; + // Offset of the ID field (uint8_t) in the decomposed raw + // parameter set structure. + size_t id_offset; + // Offset of the reference array (AVBufferRef*[]) in the codec + // private context. + size_t ref_array_offset; + // Offset of the pointer array (CodecRawXPS*[]) in the codec + // private context. + size_t ptr_array_offset; + // Offset of the active field (const CodecRawXPS*) in the codec + // private context, or zero if this codec does not have active + // parameter sets. + size_t active_offset; + } PSType; + +#define H2645_PS_TYPE(codec, nal, cname, uname, count, id_name, active_field) { \ + .codec_id = AV_CODEC_ID_ ## codec, \ + .nal_unit_type = nal, \ + .name = #cname, \ + .id_count = count, \ + .id_offset = offsetof(codec ## Raw ## cname, \ + id_name ## _parameter_set_id), \ + .ref_array_offset = offsetof(CodedBitstream ## codec ## Context, \ + uname ## _ref), \ + .ptr_array_offset = offsetof(CodedBitstream ## codec ## Context, \ + uname), \ + .active_offset = offsetof(CodedBitstream ## codec ## Context, \ + active_field), \ + } +#define H264_PS_TYPE(cname, uname, id_name) \ + H2645_PS_TYPE(H264, H264_NAL_ ## cname, cname, uname, \ + H264_MAX_ ## cname ## _COUNT, \ + id_name, active_ ## uname) +#define H265_PS_TYPE(cname, uname, id_name) \ + H2645_PS_TYPE(H265, HEVC_NAL_ ## cname, cname, uname, \ + HEVC_MAX_ ## cname ## _COUNT, \ + uname ## _ ## id_name, active_ ## uname) + + static const PSType ps_types[] = { + H264_PS_TYPE(SPS, sps, seq), + H264_PS_TYPE(PPS, pps, pic), + H265_PS_TYPE(VPS, vps, video), + H265_PS_TYPE(SPS, sps, seq), + H265_PS_TYPE(PPS, pps, pic), + }; + + const PSType *ps_type; + AVBufferRef **ref_array; + void **ptr_array; + int err, id, i; + + ps_type = NULL; + for (i = 0; i < FF_ARRAY_ELEMS(ps_types); i++) { + if (ps_types[i].codec_id == ctx->codec->codec_id && + ps_types[i].nal_unit_type == unit->type) { + ps_type = &ps_types[i]; + break; + } + } + av_assert0(ps_type); + + id = *((uint8_t*)unit->content + ps_type->id_offset); + + if (id >= ps_type->id_count) { + av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid %s id: %d.\n", + ps_type->name, id); + return AVERROR_INVALIDDATA; + } + + err = ff_cbs_make_unit_refcounted(ctx, unit); + if (err < 0) + return err; + + ref_array = (AVBufferRef**)((uint8_t*)ctx->priv_data + + ps_type->ref_array_offset); + ptr_array = (void**) ((uint8_t*)ctx->priv_data + + ps_type->ptr_array_offset); -cbs_h2645_replace_ps(4, SPS, sps, seq_parameter_set_id) -cbs_h2645_replace_ps(4, PPS, pps, pic_parameter_set_id) -cbs_h2645_replace_ps(5, VPS, vps, vps_video_parameter_set_id) -cbs_h2645_replace_ps(5, SPS, sps, sps_seq_parameter_set_id) -cbs_h2645_replace_ps(5, PPS, pps, pps_pic_parameter_set_id) + if (ps_type->active_offset) { + void **active = (void**)((uint8_t*)ctx->priv_data + + ps_type->active_offset); + + if (ptr_array[id] == *active) { + // The old active parameter set is being overwritten, so it can't + // be active after this point. + *active = NULL; + } + } + av_buffer_unref(&ref_array[id]); + + ref_array[id] = av_buffer_ref(unit->content_ref); + if (!ref_array[id]) + return AVERROR(ENOMEM); + ptr_array[id] = ref_array[id]->data; + + return 0; +} static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, CodedBitstreamUnit *unit) @@ -717,7 +796,7 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h264_replace_sps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -739,7 +818,7 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h264_replace_pps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -836,7 +915,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_vps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -849,7 +928,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_sps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -863,7 +942,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_pps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -1007,7 +1086,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h264_replace_sps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -1031,7 +1110,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h264_replace_pps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -1124,7 +1203,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_vps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -1138,7 +1217,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_sps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; } @@ -1152,7 +1231,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_pps(ctx, unit); + err = cbs_h2645_replace_ps(ctx, unit); if (err < 0) return err; }