[FFmpeg-devel] [PATCH] hevc_parser: drop the use of SliceHeader
James Almer
jamrial at gmail.com
Wed May 27 15:41:50 EEST 2020
On 5/27/2020 6:03 AM, Anton Khirnov wrote:
> It is only used to store a few local variables within one function,
> which is better accomplished by just declaring them on stack explicitly.
>
> Move SliceHeader back from hevc_ps.h to hevdec.h, since it is not
> related to parameters sets.
> ---
> libavcodec/hevc_parser.c | 57 +++++++++++++++--------------
> libavcodec/hevc_ps.h | 77 ----------------------------------------
> libavcodec/hevcdec.h | 77 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 107 insertions(+), 104 deletions(-)
>
> diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c
> index 84f19b485c..5af4b788d5 100644
> --- a/libavcodec/hevc_parser.c
> +++ b/libavcodec/hevc_parser.c
> @@ -42,7 +42,6 @@ typedef struct HEVCParserContext {
> H2645Packet pkt;
> HEVCParamSets ps;
> HEVCSEI sei;
> - SliceHeader sh;
>
> int is_avc;
> int nal_length_size;
> @@ -58,26 +57,28 @@ static int hevc_parse_slice_header(AVCodecParserContext *s, H2645NAL *nal,
> HEVCParserContext *ctx = s->priv_data;
> HEVCParamSets *ps = &ctx->ps;
> HEVCSEI *sei = &ctx->sei;
> - SliceHeader *sh = &ctx->sh;
> GetBitContext *gb = &nal->gb;
> const HEVCWindow *ow;
> int i, num = 0, den = 0;
>
> - sh->first_slice_in_pic_flag = get_bits1(gb);
> + unsigned int pps_id, first_slice_in_pic_flag, dependent_slice_segment_flag;
> + enum HEVCSliceType slice_type;
> +
> + first_slice_in_pic_flag = get_bits1(gb);
> s->picture_structure = sei->picture_timing.picture_struct;
> s->field_order = sei->picture_timing.picture_struct;
>
> if (IS_IRAP_NAL(nal)) {
> s->key_frame = 1;
> - sh->no_output_of_prior_pics_flag = get_bits1(gb);
> + skip_bits1(gb); // no_output_of_prior_pics_flag
> }
>
> - sh->pps_id = get_ue_golomb(gb);
> - if (sh->pps_id >= HEVC_MAX_PPS_COUNT || !ps->pps_list[sh->pps_id]) {
> - av_log(avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", sh->pps_id);
> + pps_id = get_ue_golomb(gb);
> + if (pps_id >= HEVC_MAX_PPS_COUNT || !ps->pps_list[pps_id]) {
> + av_log(avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id);
> return AVERROR_INVALIDDATA;
> }
> - ps->pps = (HEVCPPS*)ps->pps_list[sh->pps_id]->data;
> + ps->pps = (HEVCPPS*)ps->pps_list[pps_id]->data;
>
> if (ps->pps->sps_id >= HEVC_MAX_SPS_COUNT || !ps->sps_list[ps->pps->sps_id]) {
> av_log(avctx, AV_LOG_ERROR, "SPS id out of range: %d\n", ps->pps->sps_id);
> @@ -109,51 +110,53 @@ static int hevc_parse_slice_header(AVCodecParserContext *s, H2645NAL *nal,
> av_reduce(&avctx->framerate.den, &avctx->framerate.num,
> num, den, 1 << 30);
>
> - if (!sh->first_slice_in_pic_flag) {
> + if (!first_slice_in_pic_flag) {
> + unsigned int slice_segment_addr;
> int slice_address_length;
>
> if (ps->pps->dependent_slice_segments_enabled_flag)
> - sh->dependent_slice_segment_flag = get_bits1(gb);
> + dependent_slice_segment_flag = get_bits1(gb);
> else
> - sh->dependent_slice_segment_flag = 0;
> + dependent_slice_segment_flag = 0;
>
> slice_address_length = av_ceil_log2_c(ps->sps->ctb_width *
> ps->sps->ctb_height);
> - sh->slice_segment_addr = get_bitsz(gb, slice_address_length);
> - if (sh->slice_segment_addr >= ps->sps->ctb_width * ps->sps->ctb_height) {
> + slice_segment_addr = get_bitsz(gb, slice_address_length);
> + if (slice_segment_addr >= ps->sps->ctb_width * ps->sps->ctb_height) {
> av_log(avctx, AV_LOG_ERROR, "Invalid slice segment address: %u.\n",
> - sh->slice_segment_addr);
> + slice_segment_addr);
> return AVERROR_INVALIDDATA;
> }
> } else
> - sh->dependent_slice_segment_flag = 0;
> + dependent_slice_segment_flag = 0;
>
> - if (sh->dependent_slice_segment_flag)
> + if (dependent_slice_segment_flag)
> return 0; /* break; */
>
> for (i = 0; i < ps->pps->num_extra_slice_header_bits; i++)
> skip_bits(gb, 1); // slice_reserved_undetermined_flag[]
>
> - sh->slice_type = get_ue_golomb(gb);
> - if (!(sh->slice_type == HEVC_SLICE_I || sh->slice_type == HEVC_SLICE_P ||
> - sh->slice_type == HEVC_SLICE_B)) {
> + slice_type = get_ue_golomb(gb);
> + if (!(slice_type == HEVC_SLICE_I || slice_type == HEVC_SLICE_P ||
> + slice_type == HEVC_SLICE_B)) {
> av_log(avctx, AV_LOG_ERROR, "Unknown slice type: %d.\n",
> - sh->slice_type);
> + slice_type);
> return AVERROR_INVALIDDATA;
> }
> - s->pict_type = sh->slice_type == HEVC_SLICE_B ? AV_PICTURE_TYPE_B :
> - sh->slice_type == HEVC_SLICE_P ? AV_PICTURE_TYPE_P :
> - AV_PICTURE_TYPE_I;
> + s->pict_type = slice_type == HEVC_SLICE_B ? AV_PICTURE_TYPE_B :
> + slice_type == HEVC_SLICE_P ? AV_PICTURE_TYPE_P :
> + AV_PICTURE_TYPE_I;
>
> if (ps->pps->output_flag_present_flag)
> - sh->pic_output_flag = get_bits1(gb);
> + skip_bits1(gb); // pic_output_flag
>
> if (ps->sps->separate_colour_plane_flag)
> - sh->colour_plane_id = get_bits(gb, 2);
> + skip_bits(gb, 2); // colour_plane_id
>
> if (!IS_IDR_NAL(nal)) {
> - sh->pic_order_cnt_lsb = get_bits(gb, ps->sps->log2_max_poc_lsb);
> - s->output_picture_number = ctx->poc = ff_hevc_compute_poc(ps->sps, ctx->pocTid0, sh->pic_order_cnt_lsb, nal->type);
> + int pic_order_cnt_lsb = get_bits(gb, ps->sps->log2_max_poc_lsb);
> + s->output_picture_number = ctx->poc =
> + ff_hevc_compute_poc(ps->sps, ctx->pocTid0, pic_order_cnt_lsb, nal->type);
> } else
> s->output_picture_number = ctx->poc = 0;
>
> diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h
> index 238edd3ddc..2b3614482e 100644
> --- a/libavcodec/hevc_ps.h
> +++ b/libavcodec/hevc_ps.h
> @@ -46,83 +46,6 @@ typedef struct LongTermRPS {
> uint8_t nb_refs;
> } LongTermRPS;
LongTermRPS should be moved back as well. And it could all be done as a
git revert 4aaace8b25 commit instead, immediately after you removed
SliceHeader usage from hevc_parser.
LGTM either way. Moving SliceHeader to hevc_ps.h was one of the things i
wasn't happy about when i decoupled the parser from the decoder back then.
More information about the ffmpeg-devel
mailing list