| Message ID | 20191016025040.31273-2-andriy.gelman@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 15. Oct 22:50, Andriy Gelman wrote: > From: Andriy Gelman <andriy.gelman@gmail.com> > > Fixes #7799 > > Currently, the mp4toannexb filter always inserts the same extradata at > the start of the first IRAP unit. As in ticket #7799, this can lead to > decoding errors if modified parameter sets are signalled in-band. > > Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: > -RAP_B_Bossen_1.bit > -RPS_C_ericsson_5.bit > -SLIST_A_Sony_4.bit > -SLIST_B_Sony_8.bit > -SLIST_C_Sony_3.bit > -SLIST_D_Sony_9.bit > -TSKIP_A_MS_2.bit > > This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets > during the conversion. The correct combination is inserted at the start > of the first IRAP. SEIs from extradata are inserted before each IRAP. > > This commit also makes an update to the hevc-bsf-mp4toannexb fate test > since the result before this patch contained duplicate parameter sets > in-band. > --- > libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- > tests/fate/hevc.mak | 2 +- > 2 files changed, 472 insertions(+), 33 deletions(-) > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c > index 09bce5b34c..1ca5f13807 100644 > --- a/libavcodec/hevc_mp4toannexb_bsf.c > +++ b/libavcodec/hevc_mp4toannexb_bsf.c > @@ -23,19 +23,224 @@ > > #include "libavutil/intreadwrite.h" > #include "libavutil/mem.h" > +#include "libavutil/avassert.h" > > #include "avcodec.h" > #include "bsf.h" > #include "bytestream.h" > #include "hevc.h" > +#include "h2645_parse.h" > +#include "hevc_ps.h" > +#include "golomb.h" > > #define MIN_HEVCC_LENGTH 23 > +#define PROFILE_WITHOUT_IDC_BITS 88 > +#define IS_IRAP(s) ((s)->type >= 16 && (s)->type <= 23) > +#define IS_PARAMSET(s) ((s)->type >= 32 && (s)->type <= 34) > + > + /*reserved VCLs not included*/ > +#define IS_VCL(s) ((s)->type <= 9 || ((s)->type >= 16 && (s)->type <= 21)) > +#define HEVC_NAL_HEADER_BITS 16 > + > +/* > + * Copies data from input buffer to output buffer. Appends annexb startcode. > + * out must be allocated at least out_len + in_len + write_startcode * 4. > + */ > +#define WRITE_NAL(out, out_len, in, in_len, write_startcode) do { \ > + if ((write_startcode)) { \ > + AV_WB32((out) + (out_len), 1); \ > + (out_len) += 4; \ > + } \ > + memcpy((out) + (out_len), (in), (in_len)); \ > + (out_len) += (in_len); \ > +} while (0) > + > +typedef struct Param { > + uint8_t *raw_data; /* raw data to store param set payload*/ > + int raw_size; /* size of raw_data*/ > + size_t allocated_size; /* allocated size of raw_data*/ > + int ref; /* stores the ref of the higher level parameter set*/ > + int is_signalled; /* indicates whether this param has already been signalled in the cvs*/ > +} Param; > + > +/* modified version of HEVCParamSets to store bytestream and reference to previous level*/ > +typedef struct ParamSets { > + Param vps_list[HEVC_MAX_VPS_COUNT]; > + Param sps_list[HEVC_MAX_SPS_COUNT]; > + Param pps_list[HEVC_MAX_PPS_COUNT]; > + > + Param sei; /* cached SEIs from extradatat*/ > +} ParamSets; > > typedef struct HEVCBSFContext { > uint8_t length_size; > int extradata_parsed; > + ParamSets ps; /* cached VPS/SPS/PPS parameter sets + SEI from extradata*/ > } HEVCBSFContext; > > +static int update_cached_paramset(AVBSFContext *ctx, Param *cached_param, > + H2645NAL *nal, int ref) > +{ > + int ret; > + > + if (cached_param->raw_data && cached_param->raw_size == nal->raw_size + 4 && > + !memcmp(cached_param->raw_data + 4, nal->raw_data, nal->raw_size)) { > + av_log(ctx, AV_LOG_DEBUG, "NAL unit: %d. Copy already exists in parameter set.\n", nal->type); > + } else { > + if (nal->raw_size + 4 > cached_param->allocated_size) { > + ret = av_reallocp(&cached_param->raw_data, nal->raw_size + 4); > + if (ret < 0) > + return ret; > + cached_param->allocated_size = nal->raw_size + 4; > + } > + cached_param->raw_size = 0; > + WRITE_NAL(cached_param->raw_data, cached_param->raw_size, nal->raw_data, nal->raw_size, 1); > + cached_param->ref = ref; > + cached_param->is_signalled = 0; > + av_assert1(cached_param->raw_size == nal->raw_size + 4); > + } > + return 0; > +} > + > +static int parse_vps(AVBSFContext *ctx, H2645NAL *nal) > +{ > + int vps_id, ret; > + > + HEVCBSFContext *s = ctx->priv_data; > + ParamSets *ps = &s->ps; > + > + GetBitContext *gb = &nal->gb; > + > + vps_id = get_bits(gb, 4); > + > + if (get_bits_left(gb) < 0) { > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in VPS id: %d\n", vps_id); > + return AVERROR_INVALIDDATA; > + } > + > + av_log(ctx, AV_LOG_TRACE, "Updating VPS id: %d\n", vps_id); > + ret = update_cached_paramset(ctx, &ps->vps_list[vps_id], nal, 0); > + return ret; > +} > + > +static int parse_sps(AVBSFContext *ctx, H2645NAL *nal) > +{ > + int i, ret; > + int sps_id, vps_ref, max_sub_layers_minus1; > + > + HEVCBSFContext *s = ctx->priv_data; > + ParamSets *ps = &s->ps; > + > + uint8_t sub_layer_profile_present_flag[HEVC_MAX_SUB_LAYERS]; > + uint8_t sub_layer_level_present_flag[HEVC_MAX_SUB_LAYERS]; > + > + GetBitContext *gb = &nal->gb; > + > + vps_ref = get_bits(gb, 4); > + > + max_sub_layers_minus1 = get_bits(gb, 3); > + skip_bits1(gb); /* sps_temporal_id_nesting_flag*/ > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > + skip_bits(gb, 8); /* general_level_idc*/ > + > + for (i = 0; i < max_sub_layers_minus1; i++) { > + sub_layer_profile_present_flag[i] = get_bits1(gb); > + sub_layer_level_present_flag[i] = get_bits1(gb); > + } > + > + if (max_sub_layers_minus1 > 0) > + for (i = max_sub_layers_minus1; i < 8; i++) > + skip_bits(gb, 2); /* reserved_zero_2bits[i]*/ > + > + for (i = 0; i < max_sub_layers_minus1; i++) { > + if (sub_layer_profile_present_flag[i]) > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > + if (sub_layer_level_present_flag[i]) > + skip_bits(gb, 8); /* sub_layer_level_idc*/ > + } > + > + /*we only need the sps_id index*/ > + sps_id = get_ue_golomb(gb); > + if (sps_id < 0 || sps_id >= HEVC_MAX_SPS_COUNT) { > + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_id); > + return AVERROR_INVALIDDATA; > + } > + > + if (get_bits_left(gb) < 0) { > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in SPS id: %d\n", sps_id); > + return AVERROR_INVALIDDATA; > + } > + > + av_log(ctx, AV_LOG_TRACE, "Updating SPS id: %d, VPS ref: %d\n", sps_id, vps_ref); > + ret = update_cached_paramset(ctx, &ps->sps_list[sps_id], nal, vps_ref); > + return ret; > +} > + > +static int parse_pps(AVBSFContext *ctx, H2645NAL *nal) > +{ > + int ret; > + int pps_id, sps_ref; > + > + HEVCBSFContext *s = ctx->priv_data; > + ParamSets *ps = &s->ps; > + > + GetBitContext *gb = &nal->gb; > + > + pps_id = get_ue_golomb(gb); > + if (pps_id < 0 || pps_id >= HEVC_MAX_PPS_COUNT) { > + av_log(ctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id); > + return AVERROR_INVALIDDATA; > + } > + > + sps_ref = get_ue_golomb(gb); > + if (sps_ref < 0 || sps_ref >= HEVC_MAX_SPS_COUNT) { > + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_ref); > + return AVERROR_INVALIDDATA; > + } > + > + if (get_bits_left(gb) < 0) { > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in PPS id: %d\n", pps_id); > + return AVERROR_INVALIDDATA; > + } > + > + av_log(ctx, AV_LOG_TRACE, "Updating PPS id: %d, SPS ref: %d\n", pps_id, sps_ref); > + ret = update_cached_paramset(ctx, &ps->pps_list[pps_id], nal, sps_ref); > + return ret; > +} > + > +static int append_sei_annexb(Param *sei, H2645NAL *nal) > +{ > + int ret; > + > + ret = av_reallocp(&sei->raw_data, sei->allocated_size + nal->raw_size + 4); > + if (ret < 0) > + return ret; > + sei->allocated_size += nal->raw_size + 4; > + > + WRITE_NAL(sei->raw_data, sei->raw_size, nal->raw_data, nal->raw_size, 1); > + av_assert1(sei->raw_size == sei->allocated_size); > + return 0; > +} > + > +static int update_paramset(AVBSFContext *ctx, H2645NAL *nal) > +{ > + int ret; > + switch (nal->type) { > + case (HEVC_NAL_VPS): > + if ((ret = parse_vps(ctx, nal)) < 0) > + return ret; > + break; > + case (HEVC_NAL_SPS): > + if ((ret = parse_sps(ctx, nal)) < 0) > + return ret; > + break; > + case (HEVC_NAL_PPS): > + if ((ret = parse_pps(ctx, nal)) < 0) > + return ret; > + } > + return 0; > +} > + > static int hevc_extradata_to_annexb(AVBSFContext *ctx) > { > GetByteContext gb; > @@ -97,6 +302,7 @@ fail: > static int hevc_mp4toannexb_init(AVBSFContext *ctx) > { > HEVCBSFContext *s = ctx->priv_data; > + H2645Packet pkt; > int ret; > > if (ctx->par_in->extradata_size < MIN_HEVCC_LENGTH || > @@ -104,77 +310,309 @@ static int hevc_mp4toannexb_init(AVBSFContext *ctx) > AV_RB32(ctx->par_in->extradata) == 1) { > av_log(ctx, AV_LOG_VERBOSE, > "The input looks like it is Annex B already\n"); > + return 0; > } else { > ret = hevc_extradata_to_annexb(ctx); > if (ret < 0) > return ret; > s->length_size = ret; > s->extradata_parsed = 1; > + > + memset(&pkt, 0, sizeof(H2645Packet)); > + ret = ff_h2645_packet_split(&pkt, ctx->par_out->extradata, ctx->par_out->extradata_size, > + ctx, 0, 0, AV_CODEC_ID_HEVC, 1, 0); > + if (ret < 0) > + goto done; > + > + for (int i = 0; i < pkt.nb_nals; ++i) { > + H2645NAL *nal = &pkt.nals[i]; > + > + /*current segmentation algorithm includes next 0x00 from next nal unit*/ > + if (nal->raw_data[nal->raw_size - 1] == 0x00) > + nal->raw_size--; > + > + if (IS_PARAMSET(nal)) { > + ret = update_paramset(ctx, nal); > + if (ret < 0) > + goto done; > + continue; > + } > + > + if (nal->type == HEVC_NAL_SEI_PREFIX || nal->type == HEVC_NAL_SEI_SUFFIX) { > + ret = append_sei_annexb(&s->ps.sei, nal); > + if (ret < 0) > + goto done; > + } > + } > } > +done: > + ff_h2645_packet_uninit(&pkt); > + return ret; > +} > + > +static void ps_uninit(ParamSets *ps) > +{ > + int i; > + for (i = 0; i < FF_ARRAY_ELEMS(ps->vps_list); i++) { > + av_freep(&ps->vps_list[i].raw_data); > + ps->vps_list[i].allocated_size = 0; > + ps->vps_list[i].raw_size = 0; > + } > + for (i = 0; i < FF_ARRAY_ELEMS(ps->sps_list); i++) { > + av_freep(&ps->sps_list[i].raw_data); > + ps->sps_list[i].allocated_size = 0; > + ps->sps_list[i].raw_size = 0; > + } > + for (i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++) { > + av_freep(&ps->pps_list[i].raw_data); > + ps->pps_list[i].allocated_size = 0; > + ps->pps_list[i].raw_size = 0; > + } > + av_freep(&ps->sei); > + ps->sei.allocated_size = 0; > + ps->sei.raw_size = 0; > +} > + > +static void hevc_mp4toannexb_close(AVBSFContext *ctx) > +{ > + HEVCBSFContext *s = ctx->priv_data; > + ps_uninit(&s->ps); > +} > + > +static int get_vcl_pps_id(AVBSFContext *ctx, H2645NAL *nal) > +{ > + GetBitContext *gb = &nal->gb; > + int pps_id, ret; > + > + /* re-initialize because bitstream touched in write_extradata*/ > + ret = init_get_bits(gb, nal->data, nal->size_bits); > + if (ret < 0) > + return ret; > + skip_bits(gb, HEVC_NAL_HEADER_BITS); > + > + skip_bits1(gb); /* first_slice_segment_in_pic_flag*/ > + > + if (IS_IRAP(nal)) > + skip_bits1(gb); /* no_output_of_prior_pics_flag*/ > + > + pps_id = get_ue_golomb(gb); > + if (pps_id < 0 || pps_id >= HEVC_MAX_PPS_COUNT) { > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS id: %d\n", pps_id); > + return AVERROR_INVALIDDATA; > + } > + return pps_id; > +} > + > +static int write_vcl(AVBSFContext *ctx, AVPacket *pkt_out, int *prev_size, > + H2645NAL *nal) > +{ > + HEVCBSFContext *s = ctx->priv_data; > + ParamSets *ps = &s->ps; > + int ret, pps_id; > + Param *pps; > + > + /* get frame pps*/ > + pps_id = get_vcl_pps_id(ctx, nal); > + if (pps_id < 0) > + return AVERROR_INVALIDDATA; > + av_log(ctx, AV_LOG_TRACE, "VCL PPS id: %d\n", pps_id); > + > + /* write pps if not signalled*/ > + pps = &ps->pps_list[pps_id]; > + if (!pps->raw_data) { > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d doesn't exist\n", pps_id); > + return AVERROR_INVALIDDATA; > + } > + > + if (!pps->is_signalled) { > + av_log(ctx, AV_LOG_TRACE, "Referenced PPS: %d has not been signalled. Writing PPS to output stream\n", pps_id); > + ret = av_grow_packet(pkt_out, pps->raw_size); > + if (ret < 0) > + return ret; > + WRITE_NAL(pkt_out->data, *prev_size, pps->raw_data, pps->raw_size, 0); > + pps->is_signalled = 1; > + } > + > + /* write actual packet*/ > + ret = av_grow_packet(pkt_out, nal->raw_size + 4); > + if (ret < 0) > + return ret; > + WRITE_NAL(pkt_out->data, *prev_size, nal->raw_data, nal->raw_size, 1); > > return 0; > } > > +static int write_extradata(AVBSFContext *ctx, AVPacket *pkt_out, int *prev_size, > + H2645NAL *nal_irap) > +{ > + int ref, ret; > + int new_extradata_size = 0; > + > + HEVCBSFContext *s = ctx->priv_data; > + ParamSets *ps = &s->ps; > + GetBitContext *gb = &nal_irap->gb; > + > + /* active parameter sets for the new irap*/ > + const Param *vps; > + const Param *sps; > + Param *pps; /* non-const because we have to update flag that pps_id is signalled in cvs*/ > + > + skip_bits1(gb); /* first_slice_ic_pic_flag*/ > + skip_bits1(gb); /* no_output_of_prior_pics_flag*/ > + > + ref = get_ue_golomb(gb); > + if (ref < 0 || ref >= HEVC_MAX_PPS_COUNT || !ps->pps_list[ref].raw_data) { > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d\n", ref); > + return AVERROR_INVALIDDATA; > + } > + av_log(ctx, AV_LOG_TRACE, "Writing PPS id: %d\n", ref); > + pps = &ps->pps_list[ref]; > + new_extradata_size += pps->raw_size; > + ref = pps->ref; > + > + if (!ps->sps_list[ref].raw_data) { > + av_log(ctx, AV_LOG_ERROR, "Invalid SPS: %d\n", ref); > + return AVERROR_INVALIDDATA; > + } > + av_log(ctx, AV_LOG_TRACE, "Writing SPS id: %d\n", ref); > + sps = &ps->sps_list[ref]; > + new_extradata_size += sps->raw_size; > + ref = sps->ref; > + > + if (!ps->vps_list[ref].raw_data) { > + av_log(ctx, AV_LOG_ERROR, "Invalid VPS: %d\n", ref); > + return AVERROR_INVALIDDATA; > + } > + av_log(ctx, AV_LOG_TRACE, "Writing VPS id: %d\n", ref); > + vps = &ps->vps_list[ref]; > + new_extradata_size += vps->raw_size; > + > + if (ps->sei.raw_data) > + new_extradata_size += ps->sei.raw_size; > + > + ret = av_grow_packet(pkt_out, new_extradata_size); > + if (ret < 0) > + return ret; > + > + WRITE_NAL(pkt_out->data, *prev_size, vps->raw_data, vps->raw_size, 0); > + WRITE_NAL(pkt_out->data, *prev_size, sps->raw_data, sps->raw_size, 0); > + WRITE_NAL(pkt_out->data, *prev_size, pps->raw_data, pps->raw_size, 0); > + pps->is_signalled = 1; > + > + if (ps->sei.raw_data) > + WRITE_NAL(pkt_out->data, *prev_size, ps->sei.raw_data, ps->sei.raw_size, 0); > + > + av_assert1(*prev_size == pkt_out->size); > + return 0; > +} > + > +/* > + * Function converts mp4 access unit into annexb > + * Output packet structure > + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] > + * or > + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] > + */ > static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > { > HEVCBSFContext *s = ctx->priv_data; > AVPacket *in; > - GetByteContext gb; > - > - int got_irap = 0; > - int i, ret = 0; > + H2645Packet pkt; > + int i, j, prev_size, ret; > + int irap_done; > > ret = ff_bsf_get_packet(ctx, &in); > if (ret < 0) > return ret; > > + /* output the annexb nalu if extradata is not parsed*/ > if (!s->extradata_parsed) { > av_packet_move_ref(out, in); > av_packet_free(&in); > return 0; > } > > - bytestream2_init(&gb, in->data, in->size); > + memset(&pkt, 0, sizeof(H2645Packet)); > + ret = ff_h2645_packet_split(&pkt, in->data, in->size, ctx, 1, s->length_size, AV_CODEC_ID_HEVC, 1, 0); > + if (ret < 0) > + goto done; > + > + irap_done = 1; /* 1 means that there is no irap or the irap has already been processed.*/ > + for (i = 0; i < pkt.nb_nals; i++) { > + if (IS_IRAP(&pkt.nals[i])) { > + irap_done = 0; > + break; > + } > + } > + > + prev_size = out->size; /* prev_size stores the current length of output packet */ > + av_assert1(prev_size == 0); > > - while (bytestream2_get_bytes_left(&gb)) { > - uint32_t nalu_size = 0; > - int nalu_type; > - int is_irap, add_extradata, extra_size, prev_size; > + for (i = 0; i < pkt.nb_nals; i++) { > + H2645NAL *nal = &pkt.nals[i]; > + > + if (IS_PARAMSET(nal)) { > + ret = update_paramset(ctx, nal); > + if (ret < 0) > + goto done; > + continue; > + } > > - for (i = 0; i < s->length_size; i++) > - nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb); > + if (!irap_done && IS_IRAP(nal)) { /* append extradata and sei before first irap*/ > > - nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f; > + /* reset the pps signalled flag in this cvs*/ > + for (j = 0; j < HEVC_MAX_PPS_COUNT; j++) > + s->ps.pps_list[j].is_signalled = 0; > > - /* prepend extradata to IRAP frames */ > - is_irap = nalu_type >= 16 && nalu_type <= 23; > - add_extradata = is_irap && !got_irap; > - extra_size = add_extradata * ctx->par_out->extradata_size; > - got_irap |= is_irap; > + ret = write_extradata(ctx, out, &prev_size, nal); > + if (ret < 0) > + goto done; > + > + /* append any past SEI prefix nals*/ > + for (j = 0; j < i; j++) { > + H2645NAL *nal_past = &pkt.nals[j]; > + if (nal_past->type == HEVC_NAL_SEI_PREFIX) { > + ret = av_grow_packet(out, nal_past->raw_size + 4); > + if (ret < 0) > + goto done; > + WRITE_NAL(out->data, prev_size, nal_past->raw_data, nal_past->raw_size, 1); > + } > + } > > - if (SIZE_MAX - nalu_size < 4 || > - SIZE_MAX - 4 - nalu_size < extra_size) { > - ret = AVERROR_INVALIDDATA; > - goto fail; > + /* write irap nal unit*/ > + ret = write_vcl(ctx, out, &prev_size, nal); > + if (ret < 0) > + goto done; > + irap_done = 1; > + continue; > } > > - prev_size = out->size; > + /* do not write any sei prefix nal units if irap exists but has not been > + * processed*/ > + if (nal->type == HEVC_NAL_SEI_PREFIX && !irap_done) > + continue; > > - ret = av_grow_packet(out, 4 + nalu_size + extra_size); > - if (ret < 0) > - goto fail; > + if (IS_VCL(nal)) { > + ret = write_vcl(ctx, out, &prev_size, nal); > + if (ret < 0) > + goto done; > + continue; > + } > > - if (add_extradata) > - memcpy(out->data + prev_size, ctx->par_out->extradata, extra_size); > - AV_WB32(out->data + prev_size + extra_size, 1); > - bytestream2_get_buffer(&gb, out->data + prev_size + 4 + extra_size, nalu_size); > + /*copy any other nal units to the output - i.e. SEI_SUFFIX*/ > + ret = av_grow_packet(out, nal->raw_size + 4); > + if (ret < 0) > + goto done; > + WRITE_NAL(out->data, prev_size, nal->raw_data, nal->raw_size, 1); > } > > + av_assert1(prev_size == out->size); > + > ret = av_packet_copy_props(out, in); > - if (ret < 0) > - goto fail; > > -fail: > +done: > + ff_h2645_packet_uninit(&pkt); > + > if (ret < 0) > av_packet_unref(out); > av_packet_free(&in); > @@ -190,6 +628,7 @@ const AVBitStreamFilter ff_hevc_mp4toannexb_bsf = { > .name = "hevc_mp4toannexb", > .priv_data_size = sizeof(HEVCBSFContext), > .init = hevc_mp4toannexb_init, > + .close = hevc_mp4toannexb_close, > .filter = hevc_mp4toannexb_filter, > .codec_ids = codec_ids, > }; > diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak > index 559c3898bc..4f812b0834 100644 > --- a/tests/fate/hevc.mak > +++ b/tests/fate/hevc.mak > @@ -238,7 +238,7 @@ FATE_HEVC-$(call ALLYES, HEVC_DEMUXER MOV_DEMUXER HEVC_MP4TOANNEXB_BSF MOV_MUXER > fate-hevc-bsf-mp4toannexb: tests/data/hevc-mp4.mov > fate-hevc-bsf-mp4toannexb: CMD = md5 -i $(TARGET_PATH)/tests/data/hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc > fate-hevc-bsf-mp4toannexb: CMP = oneline > -fate-hevc-bsf-mp4toannexb: REF = 1873662a3af1848c37e4eb25722c8df9 > +fate-hevc-bsf-mp4toannexb: REF = 3c9d998a3aa2b9e0fb1c1f434952bf8b > > fate-hevc-skiploopfilter: CMD = framemd5 -skip_loop_filter nokey -i $(TARGET_SAMPLES)/hevc-conformance/SAO_D_Samsung_5.bit -sws_flags bitexact > FATE_HEVC += fate-hevc-skiploopfilter > -- > 2.23.0 > ping
On Thu, 24. Oct 20:50, Andriy Gelman wrote: > On Tue, 15. Oct 22:50, Andriy Gelman wrote: > > From: Andriy Gelman <andriy.gelman@gmail.com> > > > > Fixes #7799 > > > > Currently, the mp4toannexb filter always inserts the same extradata at > > the start of the first IRAP unit. As in ticket #7799, this can lead to > > decoding errors if modified parameter sets are signalled in-band. > > > > Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: > > -RAP_B_Bossen_1.bit > > -RPS_C_ericsson_5.bit > > -SLIST_A_Sony_4.bit > > -SLIST_B_Sony_8.bit > > -SLIST_C_Sony_3.bit > > -SLIST_D_Sony_9.bit > > -TSKIP_A_MS_2.bit > > > > This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets > > during the conversion. The correct combination is inserted at the start > > of the first IRAP. SEIs from extradata are inserted before each IRAP. > > > > This commit also makes an update to the hevc-bsf-mp4toannexb fate test > > since the result before this patch contained duplicate parameter sets > > in-band. > > --- > > libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- > > tests/fate/hevc.mak | 2 +- > > 2 files changed, 472 insertions(+), 33 deletions(-) > > > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c > > index 09bce5b34c..1ca5f13807 100644 > > --- a/libavcodec/hevc_mp4toannexb_bsf.c > > +++ b/libavcodec/hevc_mp4toannexb_bsf.c > > @@ -23,19 +23,224 @@ > > > > #include "libavutil/intreadwrite.h" > > #include "libavutil/mem.h" > > +#include "libavutil/avassert.h" > > > > #include "avcodec.h" > > #include "bsf.h" > > #include "bytestream.h" > > #include "hevc.h" > > +#include "h2645_parse.h" > > +#include "hevc_ps.h" > > +#include "golomb.h" > > > > #define MIN_HEVCC_LENGTH 23 > > +#define PROFILE_WITHOUT_IDC_BITS 88 > > +#define IS_IRAP(s) ((s)->type >= 16 && (s)->type <= 23) > > +#define IS_PARAMSET(s) ((s)->type >= 32 && (s)->type <= 34) > > + > > + /*reserved VCLs not included*/ > > +#define IS_VCL(s) ((s)->type <= 9 || ((s)->type >= 16 && (s)->type <= 21)) > > +#define HEVC_NAL_HEADER_BITS 16 > > + > > +/* > > + * Copies data from input buffer to output buffer. Appends annexb startcode. > > + * out must be allocated at least out_len + in_len + write_startcode * 4. > > + */ > > +#define WRITE_NAL(out, out_len, in, in_len, write_startcode) do { \ > > + if ((write_startcode)) { \ > > + AV_WB32((out) + (out_len), 1); \ > > + (out_len) += 4; \ > > + } \ > > + memcpy((out) + (out_len), (in), (in_len)); \ > > + (out_len) += (in_len); \ > > +} while (0) > > + > > +typedef struct Param { > > + uint8_t *raw_data; /* raw data to store param set payload*/ > > + int raw_size; /* size of raw_data*/ > > + size_t allocated_size; /* allocated size of raw_data*/ > > + int ref; /* stores the ref of the higher level parameter set*/ > > + int is_signalled; /* indicates whether this param has already been signalled in the cvs*/ > > +} Param; > > + > > +/* modified version of HEVCParamSets to store bytestream and reference to previous level*/ > > +typedef struct ParamSets { > > + Param vps_list[HEVC_MAX_VPS_COUNT]; > > + Param sps_list[HEVC_MAX_SPS_COUNT]; > > + Param pps_list[HEVC_MAX_PPS_COUNT]; > > + > > + Param sei; /* cached SEIs from extradatat*/ > > +} ParamSets; > > > > typedef struct HEVCBSFContext { > > uint8_t length_size; > > int extradata_parsed; > > + ParamSets ps; /* cached VPS/SPS/PPS parameter sets + SEI from extradata*/ > > } HEVCBSFContext; > > > > +static int update_cached_paramset(AVBSFContext *ctx, Param *cached_param, > > + H2645NAL *nal, int ref) > > +{ > > + int ret; > > + > > + if (cached_param->raw_data && cached_param->raw_size == nal->raw_size + 4 && > > + !memcmp(cached_param->raw_data + 4, nal->raw_data, nal->raw_size)) { > > + av_log(ctx, AV_LOG_DEBUG, "NAL unit: %d. Copy already exists in parameter set.\n", nal->type); > > + } else { > > + if (nal->raw_size + 4 > cached_param->allocated_size) { > > + ret = av_reallocp(&cached_param->raw_data, nal->raw_size + 4); > > + if (ret < 0) > > + return ret; > > + cached_param->allocated_size = nal->raw_size + 4; > > + } > > + cached_param->raw_size = 0; > > + WRITE_NAL(cached_param->raw_data, cached_param->raw_size, nal->raw_data, nal->raw_size, 1); > > + cached_param->ref = ref; > > + cached_param->is_signalled = 0; > > + av_assert1(cached_param->raw_size == nal->raw_size + 4); > > + } > > + return 0; > > +} > > + > > +static int parse_vps(AVBSFContext *ctx, H2645NAL *nal) > > +{ > > + int vps_id, ret; > > + > > + HEVCBSFContext *s = ctx->priv_data; > > + ParamSets *ps = &s->ps; > > + > > + GetBitContext *gb = &nal->gb; > > + > > + vps_id = get_bits(gb, 4); > > + > > + if (get_bits_left(gb) < 0) { > > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in VPS id: %d\n", vps_id); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + av_log(ctx, AV_LOG_TRACE, "Updating VPS id: %d\n", vps_id); > > + ret = update_cached_paramset(ctx, &ps->vps_list[vps_id], nal, 0); > > + return ret; > > +} > > + > > +static int parse_sps(AVBSFContext *ctx, H2645NAL *nal) > > +{ > > + int i, ret; > > + int sps_id, vps_ref, max_sub_layers_minus1; > > + > > + HEVCBSFContext *s = ctx->priv_data; > > + ParamSets *ps = &s->ps; > > + > > + uint8_t sub_layer_profile_present_flag[HEVC_MAX_SUB_LAYERS]; > > + uint8_t sub_layer_level_present_flag[HEVC_MAX_SUB_LAYERS]; > > + > > + GetBitContext *gb = &nal->gb; > > + > > + vps_ref = get_bits(gb, 4); > > + > > + max_sub_layers_minus1 = get_bits(gb, 3); > > + skip_bits1(gb); /* sps_temporal_id_nesting_flag*/ > > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > > + skip_bits(gb, 8); /* general_level_idc*/ > > + > > + for (i = 0; i < max_sub_layers_minus1; i++) { > > + sub_layer_profile_present_flag[i] = get_bits1(gb); > > + sub_layer_level_present_flag[i] = get_bits1(gb); > > + } > > + > > + if (max_sub_layers_minus1 > 0) > > + for (i = max_sub_layers_minus1; i < 8; i++) > > + skip_bits(gb, 2); /* reserved_zero_2bits[i]*/ > > + > > + for (i = 0; i < max_sub_layers_minus1; i++) { > > + if (sub_layer_profile_present_flag[i]) > > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > > + if (sub_layer_level_present_flag[i]) > > + skip_bits(gb, 8); /* sub_layer_level_idc*/ > > + } > > + > > + /*we only need the sps_id index*/ > > + sps_id = get_ue_golomb(gb); > > + if (sps_id < 0 || sps_id >= HEVC_MAX_SPS_COUNT) { > > + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_id); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + if (get_bits_left(gb) < 0) { > > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in SPS id: %d\n", sps_id); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + av_log(ctx, AV_LOG_TRACE, "Updating SPS id: %d, VPS ref: %d\n", sps_id, vps_ref); > > + ret = update_cached_paramset(ctx, &ps->sps_list[sps_id], nal, vps_ref); > > + return ret; > > +} > > + > > +static int parse_pps(AVBSFContext *ctx, H2645NAL *nal) > > +{ > > + int ret; > > + int pps_id, sps_ref; > > + > > + HEVCBSFContext *s = ctx->priv_data; > > + ParamSets *ps = &s->ps; > > + > > + GetBitContext *gb = &nal->gb; > > + > > + pps_id = get_ue_golomb(gb); > > + if (pps_id < 0 || pps_id >= HEVC_MAX_PPS_COUNT) { > > + av_log(ctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + sps_ref = get_ue_golomb(gb); > > + if (sps_ref < 0 || sps_ref >= HEVC_MAX_SPS_COUNT) { > > + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_ref); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + if (get_bits_left(gb) < 0) { > > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in PPS id: %d\n", pps_id); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + av_log(ctx, AV_LOG_TRACE, "Updating PPS id: %d, SPS ref: %d\n", pps_id, sps_ref); > > + ret = update_cached_paramset(ctx, &ps->pps_list[pps_id], nal, sps_ref); > > + return ret; > > +} > > + > > +static int append_sei_annexb(Param *sei, H2645NAL *nal) > > +{ > > + int ret; > > + > > + ret = av_reallocp(&sei->raw_data, sei->allocated_size + nal->raw_size + 4); > > + if (ret < 0) > > + return ret; > > + sei->allocated_size += nal->raw_size + 4; > > + > > + WRITE_NAL(sei->raw_data, sei->raw_size, nal->raw_data, nal->raw_size, 1); > > + av_assert1(sei->raw_size == sei->allocated_size); > > + return 0; > > +} > > + > > +static int update_paramset(AVBSFContext *ctx, H2645NAL *nal) > > +{ > > + int ret; > > + switch (nal->type) { > > + case (HEVC_NAL_VPS): > > + if ((ret = parse_vps(ctx, nal)) < 0) > > + return ret; > > + break; > > + case (HEVC_NAL_SPS): > > + if ((ret = parse_sps(ctx, nal)) < 0) > > + return ret; > > + break; > > + case (HEVC_NAL_PPS): > > + if ((ret = parse_pps(ctx, nal)) < 0) > > + return ret; > > + } > > + return 0; > > +} > > + > > static int hevc_extradata_to_annexb(AVBSFContext *ctx) > > { > > GetByteContext gb; > > @@ -97,6 +302,7 @@ fail: > > static int hevc_mp4toannexb_init(AVBSFContext *ctx) > > { > > HEVCBSFContext *s = ctx->priv_data; > > + H2645Packet pkt; > > int ret; > > > > if (ctx->par_in->extradata_size < MIN_HEVCC_LENGTH || > > @@ -104,77 +310,309 @@ static int hevc_mp4toannexb_init(AVBSFContext *ctx) > > AV_RB32(ctx->par_in->extradata) == 1) { > > av_log(ctx, AV_LOG_VERBOSE, > > "The input looks like it is Annex B already\n"); > > + return 0; > > } else { > > ret = hevc_extradata_to_annexb(ctx); > > if (ret < 0) > > return ret; > > s->length_size = ret; > > s->extradata_parsed = 1; > > + > > + memset(&pkt, 0, sizeof(H2645Packet)); > > + ret = ff_h2645_packet_split(&pkt, ctx->par_out->extradata, ctx->par_out->extradata_size, > > + ctx, 0, 0, AV_CODEC_ID_HEVC, 1, 0); > > + if (ret < 0) > > + goto done; > > + > > + for (int i = 0; i < pkt.nb_nals; ++i) { > > + H2645NAL *nal = &pkt.nals[i]; > > + > > + /*current segmentation algorithm includes next 0x00 from next nal unit*/ > > + if (nal->raw_data[nal->raw_size - 1] == 0x00) > > + nal->raw_size--; > > + > > + if (IS_PARAMSET(nal)) { > > + ret = update_paramset(ctx, nal); > > + if (ret < 0) > > + goto done; > > + continue; > > + } > > + > > + if (nal->type == HEVC_NAL_SEI_PREFIX || nal->type == HEVC_NAL_SEI_SUFFIX) { > > + ret = append_sei_annexb(&s->ps.sei, nal); > > + if (ret < 0) > > + goto done; > > + } > > + } > > } > > +done: > > + ff_h2645_packet_uninit(&pkt); > > + return ret; > > +} > > + > > +static void ps_uninit(ParamSets *ps) > > +{ > > + int i; > > + for (i = 0; i < FF_ARRAY_ELEMS(ps->vps_list); i++) { > > + av_freep(&ps->vps_list[i].raw_data); > > + ps->vps_list[i].allocated_size = 0; > > + ps->vps_list[i].raw_size = 0; > > + } > > + for (i = 0; i < FF_ARRAY_ELEMS(ps->sps_list); i++) { > > + av_freep(&ps->sps_list[i].raw_data); > > + ps->sps_list[i].allocated_size = 0; > > + ps->sps_list[i].raw_size = 0; > > + } > > + for (i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++) { > > + av_freep(&ps->pps_list[i].raw_data); > > + ps->pps_list[i].allocated_size = 0; > > + ps->pps_list[i].raw_size = 0; > > + } > > + av_freep(&ps->sei); > > + ps->sei.allocated_size = 0; > > + ps->sei.raw_size = 0; > > +} > > + > > +static void hevc_mp4toannexb_close(AVBSFContext *ctx) > > +{ > > + HEVCBSFContext *s = ctx->priv_data; > > + ps_uninit(&s->ps); > > +} > > + > > +static int get_vcl_pps_id(AVBSFContext *ctx, H2645NAL *nal) > > +{ > > + GetBitContext *gb = &nal->gb; > > + int pps_id, ret; > > + > > + /* re-initialize because bitstream touched in write_extradata*/ > > + ret = init_get_bits(gb, nal->data, nal->size_bits); > > + if (ret < 0) > > + return ret; > > + skip_bits(gb, HEVC_NAL_HEADER_BITS); > > + > > + skip_bits1(gb); /* first_slice_segment_in_pic_flag*/ > > + > > + if (IS_IRAP(nal)) > > + skip_bits1(gb); /* no_output_of_prior_pics_flag*/ > > + > > + pps_id = get_ue_golomb(gb); > > + if (pps_id < 0 || pps_id >= HEVC_MAX_PPS_COUNT) { > > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS id: %d\n", pps_id); > > + return AVERROR_INVALIDDATA; > > + } > > + return pps_id; > > +} > > + > > +static int write_vcl(AVBSFContext *ctx, AVPacket *pkt_out, int *prev_size, > > + H2645NAL *nal) > > +{ > > + HEVCBSFContext *s = ctx->priv_data; > > + ParamSets *ps = &s->ps; > > + int ret, pps_id; > > + Param *pps; > > + > > + /* get frame pps*/ > > + pps_id = get_vcl_pps_id(ctx, nal); > > + if (pps_id < 0) > > + return AVERROR_INVALIDDATA; > > + av_log(ctx, AV_LOG_TRACE, "VCL PPS id: %d\n", pps_id); > > + > > + /* write pps if not signalled*/ > > + pps = &ps->pps_list[pps_id]; > > + if (!pps->raw_data) { > > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d doesn't exist\n", pps_id); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + if (!pps->is_signalled) { > > + av_log(ctx, AV_LOG_TRACE, "Referenced PPS: %d has not been signalled. Writing PPS to output stream\n", pps_id); > > + ret = av_grow_packet(pkt_out, pps->raw_size); > > + if (ret < 0) > > + return ret; > > + WRITE_NAL(pkt_out->data, *prev_size, pps->raw_data, pps->raw_size, 0); > > + pps->is_signalled = 1; > > + } > > + > > + /* write actual packet*/ > > + ret = av_grow_packet(pkt_out, nal->raw_size + 4); > > + if (ret < 0) > > + return ret; > > + WRITE_NAL(pkt_out->data, *prev_size, nal->raw_data, nal->raw_size, 1); > > > > return 0; > > } > > > > +static int write_extradata(AVBSFContext *ctx, AVPacket *pkt_out, int *prev_size, > > + H2645NAL *nal_irap) > > +{ > > + int ref, ret; > > + int new_extradata_size = 0; > > + > > + HEVCBSFContext *s = ctx->priv_data; > > + ParamSets *ps = &s->ps; > > + GetBitContext *gb = &nal_irap->gb; > > + > > + /* active parameter sets for the new irap*/ > > + const Param *vps; > > + const Param *sps; > > + Param *pps; /* non-const because we have to update flag that pps_id is signalled in cvs*/ > > + > > + skip_bits1(gb); /* first_slice_ic_pic_flag*/ > > + skip_bits1(gb); /* no_output_of_prior_pics_flag*/ > > + > > + ref = get_ue_golomb(gb); > > + if (ref < 0 || ref >= HEVC_MAX_PPS_COUNT || !ps->pps_list[ref].raw_data) { > > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d\n", ref); > > + return AVERROR_INVALIDDATA; > > + } > > + av_log(ctx, AV_LOG_TRACE, "Writing PPS id: %d\n", ref); > > + pps = &ps->pps_list[ref]; > > + new_extradata_size += pps->raw_size; > > + ref = pps->ref; > > + > > + if (!ps->sps_list[ref].raw_data) { > > + av_log(ctx, AV_LOG_ERROR, "Invalid SPS: %d\n", ref); > > + return AVERROR_INVALIDDATA; > > + } > > + av_log(ctx, AV_LOG_TRACE, "Writing SPS id: %d\n", ref); > > + sps = &ps->sps_list[ref]; > > + new_extradata_size += sps->raw_size; > > + ref = sps->ref; > > + > > + if (!ps->vps_list[ref].raw_data) { > > + av_log(ctx, AV_LOG_ERROR, "Invalid VPS: %d\n", ref); > > + return AVERROR_INVALIDDATA; > > + } > > + av_log(ctx, AV_LOG_TRACE, "Writing VPS id: %d\n", ref); > > + vps = &ps->vps_list[ref]; > > + new_extradata_size += vps->raw_size; > > + > > + if (ps->sei.raw_data) > > + new_extradata_size += ps->sei.raw_size; > > + > > + ret = av_grow_packet(pkt_out, new_extradata_size); > > + if (ret < 0) > > + return ret; > > + > > + WRITE_NAL(pkt_out->data, *prev_size, vps->raw_data, vps->raw_size, 0); > > + WRITE_NAL(pkt_out->data, *prev_size, sps->raw_data, sps->raw_size, 0); > > + WRITE_NAL(pkt_out->data, *prev_size, pps->raw_data, pps->raw_size, 0); > > + pps->is_signalled = 1; > > + > > + if (ps->sei.raw_data) > > + WRITE_NAL(pkt_out->data, *prev_size, ps->sei.raw_data, ps->sei.raw_size, 0); > > + > > + av_assert1(*prev_size == pkt_out->size); > > + return 0; > > +} > > + > > +/* > > + * Function converts mp4 access unit into annexb > > + * Output packet structure > > + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] > > + * or > > + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] > > + */ > > static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > > { > > HEVCBSFContext *s = ctx->priv_data; > > AVPacket *in; > > - GetByteContext gb; > > - > > - int got_irap = 0; > > - int i, ret = 0; > > + H2645Packet pkt; > > + int i, j, prev_size, ret; > > + int irap_done; > > > > ret = ff_bsf_get_packet(ctx, &in); > > if (ret < 0) > > return ret; > > > > + /* output the annexb nalu if extradata is not parsed*/ > > if (!s->extradata_parsed) { > > av_packet_move_ref(out, in); > > av_packet_free(&in); > > return 0; > > } > > > > - bytestream2_init(&gb, in->data, in->size); > > + memset(&pkt, 0, sizeof(H2645Packet)); > > + ret = ff_h2645_packet_split(&pkt, in->data, in->size, ctx, 1, s->length_size, AV_CODEC_ID_HEVC, 1, 0); > > + if (ret < 0) > > + goto done; > > + > > + irap_done = 1; /* 1 means that there is no irap or the irap has already been processed.*/ > > + for (i = 0; i < pkt.nb_nals; i++) { > > + if (IS_IRAP(&pkt.nals[i])) { > > + irap_done = 0; > > + break; > > + } > > + } > > + > > + prev_size = out->size; /* prev_size stores the current length of output packet */ > > + av_assert1(prev_size == 0); > > > > - while (bytestream2_get_bytes_left(&gb)) { > > - uint32_t nalu_size = 0; > > - int nalu_type; > > - int is_irap, add_extradata, extra_size, prev_size; > > + for (i = 0; i < pkt.nb_nals; i++) { > > + H2645NAL *nal = &pkt.nals[i]; > > + > > + if (IS_PARAMSET(nal)) { > > + ret = update_paramset(ctx, nal); > > + if (ret < 0) > > + goto done; > > + continue; > > + } > > > > - for (i = 0; i < s->length_size; i++) > > - nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb); > > + if (!irap_done && IS_IRAP(nal)) { /* append extradata and sei before first irap*/ > > > > - nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f; > > + /* reset the pps signalled flag in this cvs*/ > > + for (j = 0; j < HEVC_MAX_PPS_COUNT; j++) > > + s->ps.pps_list[j].is_signalled = 0; > > > > - /* prepend extradata to IRAP frames */ > > - is_irap = nalu_type >= 16 && nalu_type <= 23; > > - add_extradata = is_irap && !got_irap; > > - extra_size = add_extradata * ctx->par_out->extradata_size; > > - got_irap |= is_irap; > > + ret = write_extradata(ctx, out, &prev_size, nal); > > + if (ret < 0) > > + goto done; > > + > > + /* append any past SEI prefix nals*/ > > + for (j = 0; j < i; j++) { > > + H2645NAL *nal_past = &pkt.nals[j]; > > + if (nal_past->type == HEVC_NAL_SEI_PREFIX) { > > + ret = av_grow_packet(out, nal_past->raw_size + 4); > > + if (ret < 0) > > + goto done; > > + WRITE_NAL(out->data, prev_size, nal_past->raw_data, nal_past->raw_size, 1); > > + } > > + } > > > > - if (SIZE_MAX - nalu_size < 4 || > > - SIZE_MAX - 4 - nalu_size < extra_size) { > > - ret = AVERROR_INVALIDDATA; > > - goto fail; > > + /* write irap nal unit*/ > > + ret = write_vcl(ctx, out, &prev_size, nal); > > + if (ret < 0) > > + goto done; > > + irap_done = 1; > > + continue; > > } > > > > - prev_size = out->size; > > + /* do not write any sei prefix nal units if irap exists but has not been > > + * processed*/ > > + if (nal->type == HEVC_NAL_SEI_PREFIX && !irap_done) > > + continue; > > > > - ret = av_grow_packet(out, 4 + nalu_size + extra_size); > > - if (ret < 0) > > - goto fail; > > + if (IS_VCL(nal)) { > > + ret = write_vcl(ctx, out, &prev_size, nal); > > + if (ret < 0) > > + goto done; > > + continue; > > + } > > > > - if (add_extradata) > > - memcpy(out->data + prev_size, ctx->par_out->extradata, extra_size); > > - AV_WB32(out->data + prev_size + extra_size, 1); > > - bytestream2_get_buffer(&gb, out->data + prev_size + 4 + extra_size, nalu_size); > > + /*copy any other nal units to the output - i.e. SEI_SUFFIX*/ > > + ret = av_grow_packet(out, nal->raw_size + 4); > > + if (ret < 0) > > + goto done; > > + WRITE_NAL(out->data, prev_size, nal->raw_data, nal->raw_size, 1); > > } > > > > + av_assert1(prev_size == out->size); > > + > > ret = av_packet_copy_props(out, in); > > - if (ret < 0) > > - goto fail; > > > > -fail: > > +done: > > + ff_h2645_packet_uninit(&pkt); > > + > > if (ret < 0) > > av_packet_unref(out); > > av_packet_free(&in); > > @@ -190,6 +628,7 @@ const AVBitStreamFilter ff_hevc_mp4toannexb_bsf = { > > .name = "hevc_mp4toannexb", > > .priv_data_size = sizeof(HEVCBSFContext), > > .init = hevc_mp4toannexb_init, > > + .close = hevc_mp4toannexb_close, > > .filter = hevc_mp4toannexb_filter, > > .codec_ids = codec_ids, > > }; > > diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak > > index 559c3898bc..4f812b0834 100644 > > --- a/tests/fate/hevc.mak > > +++ b/tests/fate/hevc.mak > > @@ -238,7 +238,7 @@ FATE_HEVC-$(call ALLYES, HEVC_DEMUXER MOV_DEMUXER HEVC_MP4TOANNEXB_BSF MOV_MUXER > > fate-hevc-bsf-mp4toannexb: tests/data/hevc-mp4.mov > > fate-hevc-bsf-mp4toannexb: CMD = md5 -i $(TARGET_PATH)/tests/data/hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc > > fate-hevc-bsf-mp4toannexb: CMP = oneline > > -fate-hevc-bsf-mp4toannexb: REF = 1873662a3af1848c37e4eb25722c8df9 > > +fate-hevc-bsf-mp4toannexb: REF = 3c9d998a3aa2b9e0fb1c1f434952bf8b > > > > fate-hevc-skiploopfilter: CMD = framemd5 -skip_loop_filter nokey -i $(TARGET_SAMPLES)/hevc-conformance/SAO_D_Samsung_5.bit -sws_flags bitexact > > FATE_HEVC += fate-hevc-skiploopfilter > > -- > > 2.23.0 > > > > ping > ping
On Wed, 30. Oct 09:45, Andriy Gelman wrote: > On Thu, 24. Oct 20:50, Andriy Gelman wrote: > > On Tue, 15. Oct 22:50, Andriy Gelman wrote: > > > From: Andriy Gelman <andriy.gelman@gmail.com> > > > > > > Fixes #7799 > > > > > > Currently, the mp4toannexb filter always inserts the same extradata at > > > the start of the first IRAP unit. As in ticket #7799, this can lead to > > > decoding errors if modified parameter sets are signalled in-band. > > > > > > Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: > > > -RAP_B_Bossen_1.bit > > > -RPS_C_ericsson_5.bit > > > -SLIST_A_Sony_4.bit > > > -SLIST_B_Sony_8.bit > > > -SLIST_C_Sony_3.bit > > > -SLIST_D_Sony_9.bit > > > -TSKIP_A_MS_2.bit > > > > > > This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets > > > during the conversion. The correct combination is inserted at the start > > > of the first IRAP. SEIs from extradata are inserted before each IRAP. > > > > > > This commit also makes an update to the hevc-bsf-mp4toannexb fate test > > > since the result before this patch contained duplicate parameter sets > > > in-band. > > > --- > > > libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- > > > tests/fate/hevc.mak | 2 +- > > > 2 files changed, 472 insertions(+), 33 deletions(-) > > > > > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c > > > index 09bce5b34c..1ca5f13807 100644 > > > --- a/libavcodec/hevc_mp4toannexb_bsf.c > > > +++ b/libavcodec/hevc_mp4toannexb_bsf.c > > > @@ -23,19 +23,224 @@ > > > > > > #include "libavutil/intreadwrite.h" > > > #include "libavutil/mem.h" > > > +#include "libavutil/avassert.h" > > > > > > #include "avcodec.h" > > > #include "bsf.h" > > > #include "bytestream.h" > > > #include "hevc.h" > > > +#include "h2645_parse.h" > > > +#include "hevc_ps.h" > > > +#include "golomb.h" > > > > > > #define MIN_HEVCC_LENGTH 23 > > > +#define PROFILE_WITHOUT_IDC_BITS 88 > > > +#define IS_IRAP(s) ((s)->type >= 16 && (s)->type <= 23) > > > +#define IS_PARAMSET(s) ((s)->type >= 32 && (s)->type <= 34) > > > + > > > + /*reserved VCLs not included*/ > > > +#define IS_VCL(s) ((s)->type <= 9 || ((s)->type >= 16 && (s)->type <= 21)) > > > +#define HEVC_NAL_HEADER_BITS 16 > > > + > > > +/* > > > + * Copies data from input buffer to output buffer. Appends annexb startcode. > > > + * out must be allocated at least out_len + in_len + write_startcode * 4. > > > + */ > > > +#define WRITE_NAL(out, out_len, in, in_len, write_startcode) do { \ > > > + if ((write_startcode)) { \ > > > + AV_WB32((out) + (out_len), 1); \ > > > + (out_len) += 4; \ > > > + } \ > > > + memcpy((out) + (out_len), (in), (in_len)); \ > > > + (out_len) += (in_len); \ > > > +} while (0) > > > + > > > +typedef struct Param { > > > + uint8_t *raw_data; /* raw data to store param set payload*/ > > > + int raw_size; /* size of raw_data*/ > > > + size_t allocated_size; /* allocated size of raw_data*/ > > > + int ref; /* stores the ref of the higher level parameter set*/ > > > + int is_signalled; /* indicates whether this param has already been signalled in the cvs*/ > > > +} Param; > > > + > > > +/* modified version of HEVCParamSets to store bytestream and reference to previous level*/ > > > +typedef struct ParamSets { > > > + Param vps_list[HEVC_MAX_VPS_COUNT]; > > > + Param sps_list[HEVC_MAX_SPS_COUNT]; > > > + Param pps_list[HEVC_MAX_PPS_COUNT]; > > > + > > > + Param sei; /* cached SEIs from extradatat*/ > > > +} ParamSets; > > > > > > typedef struct HEVCBSFContext { > > > uint8_t length_size; > > > int extradata_parsed; > > > + ParamSets ps; /* cached VPS/SPS/PPS parameter sets + SEI from extradata*/ > > > } HEVCBSFContext; > > > > > > +static int update_cached_paramset(AVBSFContext *ctx, Param *cached_param, > > > + H2645NAL *nal, int ref) > > > +{ > > > + int ret; > > > + > > > + if (cached_param->raw_data && cached_param->raw_size == nal->raw_size + 4 && > > > + !memcmp(cached_param->raw_data + 4, nal->raw_data, nal->raw_size)) { > > > + av_log(ctx, AV_LOG_DEBUG, "NAL unit: %d. Copy already exists in parameter set.\n", nal->type); > > > + } else { > > > + if (nal->raw_size + 4 > cached_param->allocated_size) { > > > + ret = av_reallocp(&cached_param->raw_data, nal->raw_size + 4); > > > + if (ret < 0) > > > + return ret; > > > + cached_param->allocated_size = nal->raw_size + 4; > > > + } > > > + cached_param->raw_size = 0; > > > + WRITE_NAL(cached_param->raw_data, cached_param->raw_size, nal->raw_data, nal->raw_size, 1); > > > + cached_param->ref = ref; > > > + cached_param->is_signalled = 0; > > > + av_assert1(cached_param->raw_size == nal->raw_size + 4); > > > + } > > > + return 0; > > > +} > > > + > > > +static int parse_vps(AVBSFContext *ctx, H2645NAL *nal) > > > +{ > > > + int vps_id, ret; > > > + > > > + HEVCBSFContext *s = ctx->priv_data; > > > + ParamSets *ps = &s->ps; > > > + > > > + GetBitContext *gb = &nal->gb; > > > + > > > + vps_id = get_bits(gb, 4); > > > + > > > + if (get_bits_left(gb) < 0) { > > > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in VPS id: %d\n", vps_id); > > > + return AVERROR_INVALIDDATA; > > > + } > > > + > > > + av_log(ctx, AV_LOG_TRACE, "Updating VPS id: %d\n", vps_id); > > > + ret = update_cached_paramset(ctx, &ps->vps_list[vps_id], nal, 0); > > > + return ret; > > > +} > > > + > > > +static int parse_sps(AVBSFContext *ctx, H2645NAL *nal) > > > +{ > > > + int i, ret; > > > + int sps_id, vps_ref, max_sub_layers_minus1; > > > + > > > + HEVCBSFContext *s = ctx->priv_data; > > > + ParamSets *ps = &s->ps; > > > + > > > + uint8_t sub_layer_profile_present_flag[HEVC_MAX_SUB_LAYERS]; > > > + uint8_t sub_layer_level_present_flag[HEVC_MAX_SUB_LAYERS]; > > > + > > > + GetBitContext *gb = &nal->gb; > > > + > > > + vps_ref = get_bits(gb, 4); > > > + > > > + max_sub_layers_minus1 = get_bits(gb, 3); > > > + skip_bits1(gb); /* sps_temporal_id_nesting_flag*/ > > > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > > > + skip_bits(gb, 8); /* general_level_idc*/ > > > + > > > + for (i = 0; i < max_sub_layers_minus1; i++) { > > > + sub_layer_profile_present_flag[i] = get_bits1(gb); > > > + sub_layer_level_present_flag[i] = get_bits1(gb); > > > + } > > > + > > > + if (max_sub_layers_minus1 > 0) > > > + for (i = max_sub_layers_minus1; i < 8; i++) > > > + skip_bits(gb, 2); /* reserved_zero_2bits[i]*/ > > > + > > > + for (i = 0; i < max_sub_layers_minus1; i++) { > > > + if (sub_layer_profile_present_flag[i]) > > > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > > > + if (sub_layer_level_present_flag[i]) > > > + skip_bits(gb, 8); /* sub_layer_level_idc*/ > > > + } > > > + > > > + /*we only need the sps_id index*/ > > > + sps_id = get_ue_golomb(gb); > > > + if (sps_id < 0 || sps_id >= HEVC_MAX_SPS_COUNT) { > > > + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_id); > > > + return AVERROR_INVALIDDATA; > > > + } > > > + > > > + if (get_bits_left(gb) < 0) { > > > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in SPS id: %d\n", sps_id); > > > + return AVERROR_INVALIDDATA; > > > + } > > > + > > > + av_log(ctx, AV_LOG_TRACE, "Updating SPS id: %d, VPS ref: %d\n", sps_id, vps_ref); > > > + ret = update_cached_paramset(ctx, &ps->sps_list[sps_id], nal, vps_ref); > > > + return ret; > > > +} > > > + > > > +static int parse_pps(AVBSFContext *ctx, H2645NAL *nal) > > > +{ > > > + int ret; > > > + int pps_id, sps_ref; > > > + > > > + HEVCBSFContext *s = ctx->priv_data; > > > + ParamSets *ps = &s->ps; > > > + > > > + GetBitContext *gb = &nal->gb; > > > + > > > + pps_id = get_ue_golomb(gb); > > > + if (pps_id < 0 || pps_id >= HEVC_MAX_PPS_COUNT) { > > > + av_log(ctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id); > > > + return AVERROR_INVALIDDATA; > > > + } > > > + > > > + sps_ref = get_ue_golomb(gb); > > > + if (sps_ref < 0 || sps_ref >= HEVC_MAX_SPS_COUNT) { > > > + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_ref); > > > + return AVERROR_INVALIDDATA; > > > + } > > > + > > > + if (get_bits_left(gb) < 0) { > > > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in PPS id: %d\n", pps_id); > > > + return AVERROR_INVALIDDATA; > > > + } > > > + > > > + av_log(ctx, AV_LOG_TRACE, "Updating PPS id: %d, SPS ref: %d\n", pps_id, sps_ref); > > > + ret = update_cached_paramset(ctx, &ps->pps_list[pps_id], nal, sps_ref); > > > + return ret; > > > +} > > > + > > > +static int append_sei_annexb(Param *sei, H2645NAL *nal) > > > +{ > > > + int ret; > > > + > > > + ret = av_reallocp(&sei->raw_data, sei->allocated_size + nal->raw_size + 4); > > > + if (ret < 0) > > > + return ret; > > > + sei->allocated_size += nal->raw_size + 4; > > > + > > > + WRITE_NAL(sei->raw_data, sei->raw_size, nal->raw_data, nal->raw_size, 1); > > > + av_assert1(sei->raw_size == sei->allocated_size); > > > + return 0; > > > +} > > > + > > > +static int update_paramset(AVBSFContext *ctx, H2645NAL *nal) > > > +{ > > > + int ret; > > > + switch (nal->type) { > > > + case (HEVC_NAL_VPS): > > > + if ((ret = parse_vps(ctx, nal)) < 0) > > > + return ret; > > > + break; > > > + case (HEVC_NAL_SPS): > > > + if ((ret = parse_sps(ctx, nal)) < 0) > > > + return ret; > > > + break; > > > + case (HEVC_NAL_PPS): > > > + if ((ret = parse_pps(ctx, nal)) < 0) > > > + return ret; > > > + } > > > + return 0; > > > +} > > > + > > > static int hevc_extradata_to_annexb(AVBSFContext *ctx) > > > { > > > GetByteContext gb; > > > @@ -97,6 +302,7 @@ fail: > > > static int hevc_mp4toannexb_init(AVBSFContext *ctx) > > > { > > > HEVCBSFContext *s = ctx->priv_data; > > > + H2645Packet pkt; > > > int ret; > > > > > > if (ctx->par_in->extradata_size < MIN_HEVCC_LENGTH || > > > @@ -104,77 +310,309 @@ static int hevc_mp4toannexb_init(AVBSFContext *ctx) > > > AV_RB32(ctx->par_in->extradata) == 1) { > > > av_log(ctx, AV_LOG_VERBOSE, > > > "The input looks like it is Annex B already\n"); > > > + return 0; > > > } else { > > > ret = hevc_extradata_to_annexb(ctx); > > > if (ret < 0) > > > return ret; > > > s->length_size = ret; > > > s->extradata_parsed = 1; > > > + > > > + memset(&pkt, 0, sizeof(H2645Packet)); > > > + ret = ff_h2645_packet_split(&pkt, ctx->par_out->extradata, ctx->par_out->extradata_size, > > > + ctx, 0, 0, AV_CODEC_ID_HEVC, 1, 0); > > > + if (ret < 0) > > > + goto done; > > > + > > > + for (int i = 0; i < pkt.nb_nals; ++i) { > > > + H2645NAL *nal = &pkt.nals[i]; > > > + > > > + /*current segmentation algorithm includes next 0x00 from next nal unit*/ > > > + if (nal->raw_data[nal->raw_size - 1] == 0x00) > > > + nal->raw_size--; > > > + > > > + if (IS_PARAMSET(nal)) { > > > + ret = update_paramset(ctx, nal); > > > + if (ret < 0) > > > + goto done; > > > + continue; > > > + } > > > + > > > + if (nal->type == HEVC_NAL_SEI_PREFIX || nal->type == HEVC_NAL_SEI_SUFFIX) { > > > + ret = append_sei_annexb(&s->ps.sei, nal); > > > + if (ret < 0) > > > + goto done; > > > + } > > > + } > > > } > > > +done: > > > + ff_h2645_packet_uninit(&pkt); > > > + return ret; > > > +} > > > + > > > +static void ps_uninit(ParamSets *ps) > > > +{ > > > + int i; > > > + for (i = 0; i < FF_ARRAY_ELEMS(ps->vps_list); i++) { > > > + av_freep(&ps->vps_list[i].raw_data); > > > + ps->vps_list[i].allocated_size = 0; > > > + ps->vps_list[i].raw_size = 0; > > > + } > > > + for (i = 0; i < FF_ARRAY_ELEMS(ps->sps_list); i++) { > > > + av_freep(&ps->sps_list[i].raw_data); > > > + ps->sps_list[i].allocated_size = 0; > > > + ps->sps_list[i].raw_size = 0; > > > + } > > > + for (i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++) { > > > + av_freep(&ps->pps_list[i].raw_data); > > > + ps->pps_list[i].allocated_size = 0; > > > + ps->pps_list[i].raw_size = 0; > > > + } > > > + av_freep(&ps->sei); > > > + ps->sei.allocated_size = 0; > > > + ps->sei.raw_size = 0; > > > +} > > > + > > > +static void hevc_mp4toannexb_close(AVBSFContext *ctx) > > > +{ > > > + HEVCBSFContext *s = ctx->priv_data; > > > + ps_uninit(&s->ps); > > > +} > > > + > > > +static int get_vcl_pps_id(AVBSFContext *ctx, H2645NAL *nal) > > > +{ > > > + GetBitContext *gb = &nal->gb; > > > + int pps_id, ret; > > > + > > > + /* re-initialize because bitstream touched in write_extradata*/ > > > + ret = init_get_bits(gb, nal->data, nal->size_bits); > > > + if (ret < 0) > > > + return ret; > > > + skip_bits(gb, HEVC_NAL_HEADER_BITS); > > > + > > > + skip_bits1(gb); /* first_slice_segment_in_pic_flag*/ > > > + > > > + if (IS_IRAP(nal)) > > > + skip_bits1(gb); /* no_output_of_prior_pics_flag*/ > > > + > > > + pps_id = get_ue_golomb(gb); > > > + if (pps_id < 0 || pps_id >= HEVC_MAX_PPS_COUNT) { > > > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS id: %d\n", pps_id); > > > + return AVERROR_INVALIDDATA; > > > + } > > > + return pps_id; > > > +} > > > + > > > +static int write_vcl(AVBSFContext *ctx, AVPacket *pkt_out, int *prev_size, > > > + H2645NAL *nal) > > > +{ > > > + HEVCBSFContext *s = ctx->priv_data; > > > + ParamSets *ps = &s->ps; > > > + int ret, pps_id; > > > + Param *pps; > > > + > > > + /* get frame pps*/ > > > + pps_id = get_vcl_pps_id(ctx, nal); > > > + if (pps_id < 0) > > > + return AVERROR_INVALIDDATA; > > > + av_log(ctx, AV_LOG_TRACE, "VCL PPS id: %d\n", pps_id); > > > + > > > + /* write pps if not signalled*/ > > > + pps = &ps->pps_list[pps_id]; > > > + if (!pps->raw_data) { > > > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d doesn't exist\n", pps_id); > > > + return AVERROR_INVALIDDATA; > > > + } > > > + > > > + if (!pps->is_signalled) { > > > + av_log(ctx, AV_LOG_TRACE, "Referenced PPS: %d has not been signalled. Writing PPS to output stream\n", pps_id); > > > + ret = av_grow_packet(pkt_out, pps->raw_size); > > > + if (ret < 0) > > > + return ret; > > > + WRITE_NAL(pkt_out->data, *prev_size, pps->raw_data, pps->raw_size, 0); > > > + pps->is_signalled = 1; > > > + } > > > + > > > + /* write actual packet*/ > > > + ret = av_grow_packet(pkt_out, nal->raw_size + 4); > > > + if (ret < 0) > > > + return ret; > > > + WRITE_NAL(pkt_out->data, *prev_size, nal->raw_data, nal->raw_size, 1); > > > > > > return 0; > > > } > > > > > > +static int write_extradata(AVBSFContext *ctx, AVPacket *pkt_out, int *prev_size, > > > + H2645NAL *nal_irap) > > > +{ > > > + int ref, ret; > > > + int new_extradata_size = 0; > > > + > > > + HEVCBSFContext *s = ctx->priv_data; > > > + ParamSets *ps = &s->ps; > > > + GetBitContext *gb = &nal_irap->gb; > > > + > > > + /* active parameter sets for the new irap*/ > > > + const Param *vps; > > > + const Param *sps; > > > + Param *pps; /* non-const because we have to update flag that pps_id is signalled in cvs*/ > > > + > > > + skip_bits1(gb); /* first_slice_ic_pic_flag*/ > > > + skip_bits1(gb); /* no_output_of_prior_pics_flag*/ > > > + > > > + ref = get_ue_golomb(gb); > > > + if (ref < 0 || ref >= HEVC_MAX_PPS_COUNT || !ps->pps_list[ref].raw_data) { > > > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d\n", ref); > > > + return AVERROR_INVALIDDATA; > > > + } > > > + av_log(ctx, AV_LOG_TRACE, "Writing PPS id: %d\n", ref); > > > + pps = &ps->pps_list[ref]; > > > + new_extradata_size += pps->raw_size; > > > + ref = pps->ref; > > > + > > > + if (!ps->sps_list[ref].raw_data) { > > > + av_log(ctx, AV_LOG_ERROR, "Invalid SPS: %d\n", ref); > > > + return AVERROR_INVALIDDATA; > > > + } > > > + av_log(ctx, AV_LOG_TRACE, "Writing SPS id: %d\n", ref); > > > + sps = &ps->sps_list[ref]; > > > + new_extradata_size += sps->raw_size; > > > + ref = sps->ref; > > > + > > > + if (!ps->vps_list[ref].raw_data) { > > > + av_log(ctx, AV_LOG_ERROR, "Invalid VPS: %d\n", ref); > > > + return AVERROR_INVALIDDATA; > > > + } > > > + av_log(ctx, AV_LOG_TRACE, "Writing VPS id: %d\n", ref); > > > + vps = &ps->vps_list[ref]; > > > + new_extradata_size += vps->raw_size; > > > + > > > + if (ps->sei.raw_data) > > > + new_extradata_size += ps->sei.raw_size; > > > + > > > + ret = av_grow_packet(pkt_out, new_extradata_size); > > > + if (ret < 0) > > > + return ret; > > > + > > > + WRITE_NAL(pkt_out->data, *prev_size, vps->raw_data, vps->raw_size, 0); > > > + WRITE_NAL(pkt_out->data, *prev_size, sps->raw_data, sps->raw_size, 0); > > > + WRITE_NAL(pkt_out->data, *prev_size, pps->raw_data, pps->raw_size, 0); > > > + pps->is_signalled = 1; > > > + > > > + if (ps->sei.raw_data) > > > + WRITE_NAL(pkt_out->data, *prev_size, ps->sei.raw_data, ps->sei.raw_size, 0); > > > + > > > + av_assert1(*prev_size == pkt_out->size); > > > + return 0; > > > +} > > > + > > > +/* > > > + * Function converts mp4 access unit into annexb > > > + * Output packet structure > > > + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] > > > + * or > > > + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] > > > + */ > > > static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > > > { > > > HEVCBSFContext *s = ctx->priv_data; > > > AVPacket *in; > > > - GetByteContext gb; > > > - > > > - int got_irap = 0; > > > - int i, ret = 0; > > > + H2645Packet pkt; > > > + int i, j, prev_size, ret; > > > + int irap_done; > > > > > > ret = ff_bsf_get_packet(ctx, &in); > > > if (ret < 0) > > > return ret; > > > > > > + /* output the annexb nalu if extradata is not parsed*/ > > > if (!s->extradata_parsed) { > > > av_packet_move_ref(out, in); > > > av_packet_free(&in); > > > return 0; > > > } > > > > > > - bytestream2_init(&gb, in->data, in->size); > > > + memset(&pkt, 0, sizeof(H2645Packet)); > > > + ret = ff_h2645_packet_split(&pkt, in->data, in->size, ctx, 1, s->length_size, AV_CODEC_ID_HEVC, 1, 0); > > > + if (ret < 0) > > > + goto done; > > > + > > > + irap_done = 1; /* 1 means that there is no irap or the irap has already been processed.*/ > > > + for (i = 0; i < pkt.nb_nals; i++) { > > > + if (IS_IRAP(&pkt.nals[i])) { > > > + irap_done = 0; > > > + break; > > > + } > > > + } > > > + > > > + prev_size = out->size; /* prev_size stores the current length of output packet */ > > > + av_assert1(prev_size == 0); > > > > > > - while (bytestream2_get_bytes_left(&gb)) { > > > - uint32_t nalu_size = 0; > > > - int nalu_type; > > > - int is_irap, add_extradata, extra_size, prev_size; > > > + for (i = 0; i < pkt.nb_nals; i++) { > > > + H2645NAL *nal = &pkt.nals[i]; > > > + > > > + if (IS_PARAMSET(nal)) { > > > + ret = update_paramset(ctx, nal); > > > + if (ret < 0) > > > + goto done; > > > + continue; > > > + } > > > > > > - for (i = 0; i < s->length_size; i++) > > > - nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb); > > > + if (!irap_done && IS_IRAP(nal)) { /* append extradata and sei before first irap*/ > > > > > > - nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f; > > > + /* reset the pps signalled flag in this cvs*/ > > > + for (j = 0; j < HEVC_MAX_PPS_COUNT; j++) > > > + s->ps.pps_list[j].is_signalled = 0; > > > > > > - /* prepend extradata to IRAP frames */ > > > - is_irap = nalu_type >= 16 && nalu_type <= 23; > > > - add_extradata = is_irap && !got_irap; > > > - extra_size = add_extradata * ctx->par_out->extradata_size; > > > - got_irap |= is_irap; > > > + ret = write_extradata(ctx, out, &prev_size, nal); > > > + if (ret < 0) > > > + goto done; > > > + > > > + /* append any past SEI prefix nals*/ > > > + for (j = 0; j < i; j++) { > > > + H2645NAL *nal_past = &pkt.nals[j]; > > > + if (nal_past->type == HEVC_NAL_SEI_PREFIX) { > > > + ret = av_grow_packet(out, nal_past->raw_size + 4); > > > + if (ret < 0) > > > + goto done; > > > + WRITE_NAL(out->data, prev_size, nal_past->raw_data, nal_past->raw_size, 1); > > > + } > > > + } > > > > > > - if (SIZE_MAX - nalu_size < 4 || > > > - SIZE_MAX - 4 - nalu_size < extra_size) { > > > - ret = AVERROR_INVALIDDATA; > > > - goto fail; > > > + /* write irap nal unit*/ > > > + ret = write_vcl(ctx, out, &prev_size, nal); > > > + if (ret < 0) > > > + goto done; > > > + irap_done = 1; > > > + continue; > > > } > > > > > > - prev_size = out->size; > > > + /* do not write any sei prefix nal units if irap exists but has not been > > > + * processed*/ > > > + if (nal->type == HEVC_NAL_SEI_PREFIX && !irap_done) > > > + continue; > > > > > > - ret = av_grow_packet(out, 4 + nalu_size + extra_size); > > > - if (ret < 0) > > > - goto fail; > > > + if (IS_VCL(nal)) { > > > + ret = write_vcl(ctx, out, &prev_size, nal); > > > + if (ret < 0) > > > + goto done; > > > + continue; > > > + } > > > > > > - if (add_extradata) > > > - memcpy(out->data + prev_size, ctx->par_out->extradata, extra_size); > > > - AV_WB32(out->data + prev_size + extra_size, 1); > > > - bytestream2_get_buffer(&gb, out->data + prev_size + 4 + extra_size, nalu_size); > > > + /*copy any other nal units to the output - i.e. SEI_SUFFIX*/ > > > + ret = av_grow_packet(out, nal->raw_size + 4); > > > + if (ret < 0) > > > + goto done; > > > + WRITE_NAL(out->data, prev_size, nal->raw_data, nal->raw_size, 1); > > > } > > > > > > + av_assert1(prev_size == out->size); > > > + > > > ret = av_packet_copy_props(out, in); > > > - if (ret < 0) > > > - goto fail; > > > > > > -fail: > > > +done: > > > + ff_h2645_packet_uninit(&pkt); > > > + > > > if (ret < 0) > > > av_packet_unref(out); > > > av_packet_free(&in); > > > @@ -190,6 +628,7 @@ const AVBitStreamFilter ff_hevc_mp4toannexb_bsf = { > > > .name = "hevc_mp4toannexb", > > > .priv_data_size = sizeof(HEVCBSFContext), > > > .init = hevc_mp4toannexb_init, > > > + .close = hevc_mp4toannexb_close, > > > .filter = hevc_mp4toannexb_filter, > > > .codec_ids = codec_ids, > > > }; > > > diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak > > > index 559c3898bc..4f812b0834 100644 > > > --- a/tests/fate/hevc.mak > > > +++ b/tests/fate/hevc.mak > > > @@ -238,7 +238,7 @@ FATE_HEVC-$(call ALLYES, HEVC_DEMUXER MOV_DEMUXER HEVC_MP4TOANNEXB_BSF MOV_MUXER > > > fate-hevc-bsf-mp4toannexb: tests/data/hevc-mp4.mov > > > fate-hevc-bsf-mp4toannexb: CMD = md5 -i $(TARGET_PATH)/tests/data/hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc > > > fate-hevc-bsf-mp4toannexb: CMP = oneline > > > -fate-hevc-bsf-mp4toannexb: REF = 1873662a3af1848c37e4eb25722c8df9 > > > +fate-hevc-bsf-mp4toannexb: REF = 3c9d998a3aa2b9e0fb1c1f434952bf8b > > > > > > fate-hevc-skiploopfilter: CMD = framemd5 -skip_loop_filter nokey -i $(TARGET_SAMPLES)/hevc-conformance/SAO_D_Samsung_5.bit -sws_flags bitexact > > > FATE_HEVC += fate-hevc-skiploopfilter > > > -- > > > 2.23.0 > > > > > > > ping > > > ping > ping
On Tue, 05. Nov 09:25, Andriy Gelman wrote: > On Wed, 30. Oct 09:45, Andriy Gelman wrote: > > On Thu, 24. Oct 20:50, Andriy Gelman wrote: > > > On Tue, 15. Oct 22:50, Andriy Gelman wrote: > > > > From: Andriy Gelman <andriy.gelman@gmail.com> > > > > > > > > Fixes #7799 > > > > > > > > Currently, the mp4toannexb filter always inserts the same extradata at > > > > the start of the first IRAP unit. As in ticket #7799, this can lead to > > > > decoding errors if modified parameter sets are signalled in-band. > > > > > > > > Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: > > > > -RAP_B_Bossen_1.bit > > > > -RPS_C_ericsson_5.bit > > > > -SLIST_A_Sony_4.bit > > > > -SLIST_B_Sony_8.bit > > > > -SLIST_C_Sony_3.bit > > > > -SLIST_D_Sony_9.bit > > > > -TSKIP_A_MS_2.bit > > > > > > > > This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets > > > > during the conversion. The correct combination is inserted at the start > > > > of the first IRAP. SEIs from extradata are inserted before each IRAP. > > > > > > > > This commit also makes an update to the hevc-bsf-mp4toannexb fate test > > > > since the result before this patch contained duplicate parameter sets > > > > in-band. > > > > --- > > > > libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- > > > > tests/fate/hevc.mak | 2 +- > > > > 2 files changed, 472 insertions(+), 33 deletions(-) > > > > > > > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c > > > > index 09bce5b34c..1ca5f13807 100644 > > > > --- a/libavcodec/hevc_mp4toannexb_bsf.c > > > > +++ b/libavcodec/hevc_mp4toannexb_bsf.c > > > > @@ -23,19 +23,224 @@ > > > > > > > > #include "libavutil/intreadwrite.h" > > > > #include "libavutil/mem.h" > > > > +#include "libavutil/avassert.h" > > > > > > > > #include "avcodec.h" > > > > #include "bsf.h" > > > > #include "bytestream.h" > > > > #include "hevc.h" > > > > +#include "h2645_parse.h" > > > > +#include "hevc_ps.h" > > > > +#include "golomb.h" > > > > > > > > #define MIN_HEVCC_LENGTH 23 > > > > +#define PROFILE_WITHOUT_IDC_BITS 88 > > > > +#define IS_IRAP(s) ((s)->type >= 16 && (s)->type <= 23) > > > > +#define IS_PARAMSET(s) ((s)->type >= 32 && (s)->type <= 34) > > > > + > > > > + /*reserved VCLs not included*/ > > > > +#define IS_VCL(s) ((s)->type <= 9 || ((s)->type >= 16 && (s)->type <= 21)) > > > > +#define HEVC_NAL_HEADER_BITS 16 > > > > + > > > > +/* > > > > + * Copies data from input buffer to output buffer. Appends annexb startcode. > > > > + * out must be allocated at least out_len + in_len + write_startcode * 4. > > > > + */ > > > > +#define WRITE_NAL(out, out_len, in, in_len, write_startcode) do { \ > > > > + if ((write_startcode)) { \ > > > > + AV_WB32((out) + (out_len), 1); \ > > > > + (out_len) += 4; \ > > > > + } \ > > > > + memcpy((out) + (out_len), (in), (in_len)); \ > > > > + (out_len) += (in_len); \ > > > > +} while (0) > > > > + > > > > +typedef struct Param { > > > > + uint8_t *raw_data; /* raw data to store param set payload*/ > > > > + int raw_size; /* size of raw_data*/ > > > > + size_t allocated_size; /* allocated size of raw_data*/ > > > > + int ref; /* stores the ref of the higher level parameter set*/ > > > > + int is_signalled; /* indicates whether this param has already been signalled in the cvs*/ > > > > +} Param; > > > > + > > > > +/* modified version of HEVCParamSets to store bytestream and reference to previous level*/ > > > > +typedef struct ParamSets { > > > > + Param vps_list[HEVC_MAX_VPS_COUNT]; > > > > + Param sps_list[HEVC_MAX_SPS_COUNT]; > > > > + Param pps_list[HEVC_MAX_PPS_COUNT]; > > > > + > > > > + Param sei; /* cached SEIs from extradatat*/ > > > > +} ParamSets; > > > > > > > > typedef struct HEVCBSFContext { > > > > uint8_t length_size; > > > > int extradata_parsed; > > > > + ParamSets ps; /* cached VPS/SPS/PPS parameter sets + SEI from extradata*/ > > > > } HEVCBSFContext; > > > > > > > > +static int update_cached_paramset(AVBSFContext *ctx, Param *cached_param, > > > > + H2645NAL *nal, int ref) > > > > +{ > > > > + int ret; > > > > + > > > > + if (cached_param->raw_data && cached_param->raw_size == nal->raw_size + 4 && > > > > + !memcmp(cached_param->raw_data + 4, nal->raw_data, nal->raw_size)) { > > > > + av_log(ctx, AV_LOG_DEBUG, "NAL unit: %d. Copy already exists in parameter set.\n", nal->type); > > > > + } else { > > > > + if (nal->raw_size + 4 > cached_param->allocated_size) { > > > > + ret = av_reallocp(&cached_param->raw_data, nal->raw_size + 4); > > > > + if (ret < 0) > > > > + return ret; > > > > + cached_param->allocated_size = nal->raw_size + 4; > > > > + } > > > > + cached_param->raw_size = 0; > > > > + WRITE_NAL(cached_param->raw_data, cached_param->raw_size, nal->raw_data, nal->raw_size, 1); > > > > + cached_param->ref = ref; > > > > + cached_param->is_signalled = 0; > > > > + av_assert1(cached_param->raw_size == nal->raw_size + 4); > > > > + } > > > > + return 0; > > > > +} > > > > + > > > > +static int parse_vps(AVBSFContext *ctx, H2645NAL *nal) > > > > +{ > > > > + int vps_id, ret; > > > > + > > > > + HEVCBSFContext *s = ctx->priv_data; > > > > + ParamSets *ps = &s->ps; > > > > + > > > > + GetBitContext *gb = &nal->gb; > > > > + > > > > + vps_id = get_bits(gb, 4); > > > > + > > > > + if (get_bits_left(gb) < 0) { > > > > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in VPS id: %d\n", vps_id); > > > > + return AVERROR_INVALIDDATA; > > > > + } > > > > + > > > > + av_log(ctx, AV_LOG_TRACE, "Updating VPS id: %d\n", vps_id); > > > > + ret = update_cached_paramset(ctx, &ps->vps_list[vps_id], nal, 0); > > > > + return ret; > > > > +} > > > > + > > > > +static int parse_sps(AVBSFContext *ctx, H2645NAL *nal) > > > > +{ > > > > + int i, ret; > > > > + int sps_id, vps_ref, max_sub_layers_minus1; > > > > + > > > > + HEVCBSFContext *s = ctx->priv_data; > > > > + ParamSets *ps = &s->ps; > > > > + > > > > + uint8_t sub_layer_profile_present_flag[HEVC_MAX_SUB_LAYERS]; > > > > + uint8_t sub_layer_level_present_flag[HEVC_MAX_SUB_LAYERS]; > > > > + > > > > + GetBitContext *gb = &nal->gb; > > > > + > > > > + vps_ref = get_bits(gb, 4); > > > > + > > > > + max_sub_layers_minus1 = get_bits(gb, 3); > > > > + skip_bits1(gb); /* sps_temporal_id_nesting_flag*/ > > > > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > > > > + skip_bits(gb, 8); /* general_level_idc*/ > > > > + > > > > + for (i = 0; i < max_sub_layers_minus1; i++) { > > > > + sub_layer_profile_present_flag[i] = get_bits1(gb); > > > > + sub_layer_level_present_flag[i] = get_bits1(gb); > > > > + } > > > > + > > > > + if (max_sub_layers_minus1 > 0) > > > > + for (i = max_sub_layers_minus1; i < 8; i++) > > > > + skip_bits(gb, 2); /* reserved_zero_2bits[i]*/ > > > > + > > > > + for (i = 0; i < max_sub_layers_minus1; i++) { > > > > + if (sub_layer_profile_present_flag[i]) > > > > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > > > > + if (sub_layer_level_present_flag[i]) > > > > + skip_bits(gb, 8); /* sub_layer_level_idc*/ > > > > + } > > > > + > > > > + /*we only need the sps_id index*/ > > > > + sps_id = get_ue_golomb(gb); > > > > + if (sps_id < 0 || sps_id >= HEVC_MAX_SPS_COUNT) { > > > > + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_id); > > > > + return AVERROR_INVALIDDATA; > > > > + } > > > > + > > > > + if (get_bits_left(gb) < 0) { > > > > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in SPS id: %d\n", sps_id); > > > > + return AVERROR_INVALIDDATA; > > > > + } > > > > + > > > > + av_log(ctx, AV_LOG_TRACE, "Updating SPS id: %d, VPS ref: %d\n", sps_id, vps_ref); > > > > + ret = update_cached_paramset(ctx, &ps->sps_list[sps_id], nal, vps_ref); > > > > + return ret; > > > > +} > > > > + > > > > +static int parse_pps(AVBSFContext *ctx, H2645NAL *nal) > > > > +{ > > > > + int ret; > > > > + int pps_id, sps_ref; > > > > + > > > > + HEVCBSFContext *s = ctx->priv_data; > > > > + ParamSets *ps = &s->ps; > > > > + > > > > + GetBitContext *gb = &nal->gb; > > > > + > > > > + pps_id = get_ue_golomb(gb); > > > > + if (pps_id < 0 || pps_id >= HEVC_MAX_PPS_COUNT) { > > > > + av_log(ctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id); > > > > + return AVERROR_INVALIDDATA; > > > > + } > > > > + > > > > + sps_ref = get_ue_golomb(gb); > > > > + if (sps_ref < 0 || sps_ref >= HEVC_MAX_SPS_COUNT) { > > > > + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_ref); > > > > + return AVERROR_INVALIDDATA; > > > > + } > > > > + > > > > + if (get_bits_left(gb) < 0) { > > > > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in PPS id: %d\n", pps_id); > > > > + return AVERROR_INVALIDDATA; > > > > + } > > > > + > > > > + av_log(ctx, AV_LOG_TRACE, "Updating PPS id: %d, SPS ref: %d\n", pps_id, sps_ref); > > > > + ret = update_cached_paramset(ctx, &ps->pps_list[pps_id], nal, sps_ref); > > > > + return ret; > > > > +} > > > > + > > > > +static int append_sei_annexb(Param *sei, H2645NAL *nal) > > > > +{ > > > > + int ret; > > > > + > > > > + ret = av_reallocp(&sei->raw_data, sei->allocated_size + nal->raw_size + 4); > > > > + if (ret < 0) > > > > + return ret; > > > > + sei->allocated_size += nal->raw_size + 4; > > > > + > > > > + WRITE_NAL(sei->raw_data, sei->raw_size, nal->raw_data, nal->raw_size, 1); > > > > + av_assert1(sei->raw_size == sei->allocated_size); > > > > + return 0; > > > > +} > > > > + > > > > +static int update_paramset(AVBSFContext *ctx, H2645NAL *nal) > > > > +{ > > > > + int ret; > > > > + switch (nal->type) { > > > > + case (HEVC_NAL_VPS): > > > > + if ((ret = parse_vps(ctx, nal)) < 0) > > > > + return ret; > > > > + break; > > > > + case (HEVC_NAL_SPS): > > > > + if ((ret = parse_sps(ctx, nal)) < 0) > > > > + return ret; > > > > + break; > > > > + case (HEVC_NAL_PPS): > > > > + if ((ret = parse_pps(ctx, nal)) < 0) > > > > + return ret; > > > > + } > > > > + return 0; > > > > +} > > > > + > > > > static int hevc_extradata_to_annexb(AVBSFContext *ctx) > > > > { > > > > GetByteContext gb; > > > > @@ -97,6 +302,7 @@ fail: > > > > static int hevc_mp4toannexb_init(AVBSFContext *ctx) > > > > { > > > > HEVCBSFContext *s = ctx->priv_data; > > > > + H2645Packet pkt; > > > > int ret; > > > > > > > > if (ctx->par_in->extradata_size < MIN_HEVCC_LENGTH || > > > > @@ -104,77 +310,309 @@ static int hevc_mp4toannexb_init(AVBSFContext *ctx) > > > > AV_RB32(ctx->par_in->extradata) == 1) { > > > > av_log(ctx, AV_LOG_VERBOSE, > > > > "The input looks like it is Annex B already\n"); > > > > + return 0; > > > > } else { > > > > ret = hevc_extradata_to_annexb(ctx); > > > > if (ret < 0) > > > > return ret; > > > > s->length_size = ret; > > > > s->extradata_parsed = 1; > > > > + > > > > + memset(&pkt, 0, sizeof(H2645Packet)); > > > > + ret = ff_h2645_packet_split(&pkt, ctx->par_out->extradata, ctx->par_out->extradata_size, > > > > + ctx, 0, 0, AV_CODEC_ID_HEVC, 1, 0); > > > > + if (ret < 0) > > > > + goto done; > > > > + > > > > + for (int i = 0; i < pkt.nb_nals; ++i) { > > > > + H2645NAL *nal = &pkt.nals[i]; > > > > + > > > > + /*current segmentation algorithm includes next 0x00 from next nal unit*/ > > > > + if (nal->raw_data[nal->raw_size - 1] == 0x00) > > > > + nal->raw_size--; > > > > + > > > > + if (IS_PARAMSET(nal)) { > > > > + ret = update_paramset(ctx, nal); > > > > + if (ret < 0) > > > > + goto done; > > > > + continue; > > > > + } > > > > + > > > > + if (nal->type == HEVC_NAL_SEI_PREFIX || nal->type == HEVC_NAL_SEI_SUFFIX) { > > > > + ret = append_sei_annexb(&s->ps.sei, nal); > > > > + if (ret < 0) > > > > + goto done; > > > > + } > > > > + } > > > > } > > > > +done: > > > > + ff_h2645_packet_uninit(&pkt); > > > > + return ret; > > > > +} > > > > + > > > > +static void ps_uninit(ParamSets *ps) > > > > +{ > > > > + int i; > > > > + for (i = 0; i < FF_ARRAY_ELEMS(ps->vps_list); i++) { > > > > + av_freep(&ps->vps_list[i].raw_data); > > > > + ps->vps_list[i].allocated_size = 0; > > > > + ps->vps_list[i].raw_size = 0; > > > > + } > > > > + for (i = 0; i < FF_ARRAY_ELEMS(ps->sps_list); i++) { > > > > + av_freep(&ps->sps_list[i].raw_data); > > > > + ps->sps_list[i].allocated_size = 0; > > > > + ps->sps_list[i].raw_size = 0; > > > > + } > > > > + for (i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++) { > > > > + av_freep(&ps->pps_list[i].raw_data); > > > > + ps->pps_list[i].allocated_size = 0; > > > > + ps->pps_list[i].raw_size = 0; > > > > + } > > > > + av_freep(&ps->sei); > > > > + ps->sei.allocated_size = 0; > > > > + ps->sei.raw_size = 0; > > > > +} > > > > + > > > > +static void hevc_mp4toannexb_close(AVBSFContext *ctx) > > > > +{ > > > > + HEVCBSFContext *s = ctx->priv_data; > > > > + ps_uninit(&s->ps); > > > > +} > > > > + > > > > +static int get_vcl_pps_id(AVBSFContext *ctx, H2645NAL *nal) > > > > +{ > > > > + GetBitContext *gb = &nal->gb; > > > > + int pps_id, ret; > > > > + > > > > + /* re-initialize because bitstream touched in write_extradata*/ > > > > + ret = init_get_bits(gb, nal->data, nal->size_bits); > > > > + if (ret < 0) > > > > + return ret; > > > > + skip_bits(gb, HEVC_NAL_HEADER_BITS); > > > > + > > > > + skip_bits1(gb); /* first_slice_segment_in_pic_flag*/ > > > > + > > > > + if (IS_IRAP(nal)) > > > > + skip_bits1(gb); /* no_output_of_prior_pics_flag*/ > > > > + > > > > + pps_id = get_ue_golomb(gb); > > > > + if (pps_id < 0 || pps_id >= HEVC_MAX_PPS_COUNT) { > > > > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS id: %d\n", pps_id); > > > > + return AVERROR_INVALIDDATA; > > > > + } > > > > + return pps_id; > > > > +} > > > > + > > > > +static int write_vcl(AVBSFContext *ctx, AVPacket *pkt_out, int *prev_size, > > > > + H2645NAL *nal) > > > > +{ > > > > + HEVCBSFContext *s = ctx->priv_data; > > > > + ParamSets *ps = &s->ps; > > > > + int ret, pps_id; > > > > + Param *pps; > > > > + > > > > + /* get frame pps*/ > > > > + pps_id = get_vcl_pps_id(ctx, nal); > > > > + if (pps_id < 0) > > > > + return AVERROR_INVALIDDATA; > > > > + av_log(ctx, AV_LOG_TRACE, "VCL PPS id: %d\n", pps_id); > > > > + > > > > + /* write pps if not signalled*/ > > > > + pps = &ps->pps_list[pps_id]; > > > > + if (!pps->raw_data) { > > > > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d doesn't exist\n", pps_id); > > > > + return AVERROR_INVALIDDATA; > > > > + } > > > > + > > > > + if (!pps->is_signalled) { > > > > + av_log(ctx, AV_LOG_TRACE, "Referenced PPS: %d has not been signalled. Writing PPS to output stream\n", pps_id); > > > > + ret = av_grow_packet(pkt_out, pps->raw_size); > > > > + if (ret < 0) > > > > + return ret; > > > > + WRITE_NAL(pkt_out->data, *prev_size, pps->raw_data, pps->raw_size, 0); > > > > + pps->is_signalled = 1; > > > > + } > > > > + > > > > + /* write actual packet*/ > > > > + ret = av_grow_packet(pkt_out, nal->raw_size + 4); > > > > + if (ret < 0) > > > > + return ret; > > > > + WRITE_NAL(pkt_out->data, *prev_size, nal->raw_data, nal->raw_size, 1); > > > > > > > > return 0; > > > > } > > > > > > > > +static int write_extradata(AVBSFContext *ctx, AVPacket *pkt_out, int *prev_size, > > > > + H2645NAL *nal_irap) > > > > +{ > > > > + int ref, ret; > > > > + int new_extradata_size = 0; > > > > + > > > > + HEVCBSFContext *s = ctx->priv_data; > > > > + ParamSets *ps = &s->ps; > > > > + GetBitContext *gb = &nal_irap->gb; > > > > + > > > > + /* active parameter sets for the new irap*/ > > > > + const Param *vps; > > > > + const Param *sps; > > > > + Param *pps; /* non-const because we have to update flag that pps_id is signalled in cvs*/ > > > > + > > > > + skip_bits1(gb); /* first_slice_ic_pic_flag*/ > > > > + skip_bits1(gb); /* no_output_of_prior_pics_flag*/ > > > > + > > > > + ref = get_ue_golomb(gb); > > > > + if (ref < 0 || ref >= HEVC_MAX_PPS_COUNT || !ps->pps_list[ref].raw_data) { > > > > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d\n", ref); > > > > + return AVERROR_INVALIDDATA; > > > > + } > > > > + av_log(ctx, AV_LOG_TRACE, "Writing PPS id: %d\n", ref); > > > > + pps = &ps->pps_list[ref]; > > > > + new_extradata_size += pps->raw_size; > > > > + ref = pps->ref; > > > > + > > > > + if (!ps->sps_list[ref].raw_data) { > > > > + av_log(ctx, AV_LOG_ERROR, "Invalid SPS: %d\n", ref); > > > > + return AVERROR_INVALIDDATA; > > > > + } > > > > + av_log(ctx, AV_LOG_TRACE, "Writing SPS id: %d\n", ref); > > > > + sps = &ps->sps_list[ref]; > > > > + new_extradata_size += sps->raw_size; > > > > + ref = sps->ref; > > > > + > > > > + if (!ps->vps_list[ref].raw_data) { > > > > + av_log(ctx, AV_LOG_ERROR, "Invalid VPS: %d\n", ref); > > > > + return AVERROR_INVALIDDATA; > > > > + } > > > > + av_log(ctx, AV_LOG_TRACE, "Writing VPS id: %d\n", ref); > > > > + vps = &ps->vps_list[ref]; > > > > + new_extradata_size += vps->raw_size; > > > > + > > > > + if (ps->sei.raw_data) > > > > + new_extradata_size += ps->sei.raw_size; > > > > + > > > > + ret = av_grow_packet(pkt_out, new_extradata_size); > > > > + if (ret < 0) > > > > + return ret; > > > > + > > > > + WRITE_NAL(pkt_out->data, *prev_size, vps->raw_data, vps->raw_size, 0); > > > > + WRITE_NAL(pkt_out->data, *prev_size, sps->raw_data, sps->raw_size, 0); > > > > + WRITE_NAL(pkt_out->data, *prev_size, pps->raw_data, pps->raw_size, 0); > > > > + pps->is_signalled = 1; > > > > + > > > > + if (ps->sei.raw_data) > > > > + WRITE_NAL(pkt_out->data, *prev_size, ps->sei.raw_data, ps->sei.raw_size, 0); > > > > + > > > > + av_assert1(*prev_size == pkt_out->size); > > > > + return 0; > > > > +} > > > > + > > > > +/* > > > > + * Function converts mp4 access unit into annexb > > > > + * Output packet structure > > > > + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] > > > > + * or > > > > + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] > > > > + */ > > > > static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > > > > { > > > > HEVCBSFContext *s = ctx->priv_data; > > > > AVPacket *in; > > > > - GetByteContext gb; > > > > - > > > > - int got_irap = 0; > > > > - int i, ret = 0; > > > > + H2645Packet pkt; > > > > + int i, j, prev_size, ret; > > > > + int irap_done; > > > > > > > > ret = ff_bsf_get_packet(ctx, &in); > > > > if (ret < 0) > > > > return ret; > > > > > > > > + /* output the annexb nalu if extradata is not parsed*/ > > > > if (!s->extradata_parsed) { > > > > av_packet_move_ref(out, in); > > > > av_packet_free(&in); > > > > return 0; > > > > } > > > > > > > > - bytestream2_init(&gb, in->data, in->size); > > > > + memset(&pkt, 0, sizeof(H2645Packet)); > > > > + ret = ff_h2645_packet_split(&pkt, in->data, in->size, ctx, 1, s->length_size, AV_CODEC_ID_HEVC, 1, 0); > > > > + if (ret < 0) > > > > + goto done; > > > > + > > > > + irap_done = 1; /* 1 means that there is no irap or the irap has already been processed.*/ > > > > + for (i = 0; i < pkt.nb_nals; i++) { > > > > + if (IS_IRAP(&pkt.nals[i])) { > > > > + irap_done = 0; > > > > + break; > > > > + } > > > > + } > > > > + > > > > + prev_size = out->size; /* prev_size stores the current length of output packet */ > > > > + av_assert1(prev_size == 0); > > > > > > > > - while (bytestream2_get_bytes_left(&gb)) { > > > > - uint32_t nalu_size = 0; > > > > - int nalu_type; > > > > - int is_irap, add_extradata, extra_size, prev_size; > > > > + for (i = 0; i < pkt.nb_nals; i++) { > > > > + H2645NAL *nal = &pkt.nals[i]; > > > > + > > > > + if (IS_PARAMSET(nal)) { > > > > + ret = update_paramset(ctx, nal); > > > > + if (ret < 0) > > > > + goto done; > > > > + continue; > > > > + } > > > > > > > > - for (i = 0; i < s->length_size; i++) > > > > - nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb); > > > > + if (!irap_done && IS_IRAP(nal)) { /* append extradata and sei before first irap*/ > > > > > > > > - nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f; > > > > + /* reset the pps signalled flag in this cvs*/ > > > > + for (j = 0; j < HEVC_MAX_PPS_COUNT; j++) > > > > + s->ps.pps_list[j].is_signalled = 0; > > > > > > > > - /* prepend extradata to IRAP frames */ > > > > - is_irap = nalu_type >= 16 && nalu_type <= 23; > > > > - add_extradata = is_irap && !got_irap; > > > > - extra_size = add_extradata * ctx->par_out->extradata_size; > > > > - got_irap |= is_irap; > > > > + ret = write_extradata(ctx, out, &prev_size, nal); > > > > + if (ret < 0) > > > > + goto done; > > > > + > > > > + /* append any past SEI prefix nals*/ > > > > + for (j = 0; j < i; j++) { > > > > + H2645NAL *nal_past = &pkt.nals[j]; > > > > + if (nal_past->type == HEVC_NAL_SEI_PREFIX) { > > > > + ret = av_grow_packet(out, nal_past->raw_size + 4); > > > > + if (ret < 0) > > > > + goto done; > > > > + WRITE_NAL(out->data, prev_size, nal_past->raw_data, nal_past->raw_size, 1); > > > > + } > > > > + } > > > > > > > > - if (SIZE_MAX - nalu_size < 4 || > > > > - SIZE_MAX - 4 - nalu_size < extra_size) { > > > > - ret = AVERROR_INVALIDDATA; > > > > - goto fail; > > > > + /* write irap nal unit*/ > > > > + ret = write_vcl(ctx, out, &prev_size, nal); > > > > + if (ret < 0) > > > > + goto done; > > > > + irap_done = 1; > > > > + continue; > > > > } > > > > > > > > - prev_size = out->size; > > > > + /* do not write any sei prefix nal units if irap exists but has not been > > > > + * processed*/ > > > > + if (nal->type == HEVC_NAL_SEI_PREFIX && !irap_done) > > > > + continue; > > > > > > > > - ret = av_grow_packet(out, 4 + nalu_size + extra_size); > > > > - if (ret < 0) > > > > - goto fail; > > > > + if (IS_VCL(nal)) { > > > > + ret = write_vcl(ctx, out, &prev_size, nal); > > > > + if (ret < 0) > > > > + goto done; > > > > + continue; > > > > + } > > > > > > > > - if (add_extradata) > > > > - memcpy(out->data + prev_size, ctx->par_out->extradata, extra_size); > > > > - AV_WB32(out->data + prev_size + extra_size, 1); > > > > - bytestream2_get_buffer(&gb, out->data + prev_size + 4 + extra_size, nalu_size); > > > > + /*copy any other nal units to the output - i.e. SEI_SUFFIX*/ > > > > + ret = av_grow_packet(out, nal->raw_size + 4); > > > > + if (ret < 0) > > > > + goto done; > > > > + WRITE_NAL(out->data, prev_size, nal->raw_data, nal->raw_size, 1); > > > > } > > > > > > > > + av_assert1(prev_size == out->size); > > > > + > > > > ret = av_packet_copy_props(out, in); > > > > - if (ret < 0) > > > > - goto fail; > > > > > > > > -fail: > > > > +done: > > > > + ff_h2645_packet_uninit(&pkt); > > > > + > > > > if (ret < 0) > > > > av_packet_unref(out); > > > > av_packet_free(&in); > > > > @@ -190,6 +628,7 @@ const AVBitStreamFilter ff_hevc_mp4toannexb_bsf = { > > > > .name = "hevc_mp4toannexb", > > > > .priv_data_size = sizeof(HEVCBSFContext), > > > > .init = hevc_mp4toannexb_init, > > > > + .close = hevc_mp4toannexb_close, > > > > .filter = hevc_mp4toannexb_filter, > > > > .codec_ids = codec_ids, > > > > }; > > > > diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak > > > > index 559c3898bc..4f812b0834 100644 > > > > --- a/tests/fate/hevc.mak > > > > +++ b/tests/fate/hevc.mak > > > > @@ -238,7 +238,7 @@ FATE_HEVC-$(call ALLYES, HEVC_DEMUXER MOV_DEMUXER HEVC_MP4TOANNEXB_BSF MOV_MUXER > > > > fate-hevc-bsf-mp4toannexb: tests/data/hevc-mp4.mov > > > > fate-hevc-bsf-mp4toannexb: CMD = md5 -i $(TARGET_PATH)/tests/data/hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc > > > > fate-hevc-bsf-mp4toannexb: CMP = oneline > > > > -fate-hevc-bsf-mp4toannexb: REF = 1873662a3af1848c37e4eb25722c8df9 > > > > +fate-hevc-bsf-mp4toannexb: REF = 3c9d998a3aa2b9e0fb1c1f434952bf8b > > > > > > > > fate-hevc-skiploopfilter: CMD = framemd5 -skip_loop_filter nokey -i $(TARGET_SAMPLES)/hevc-conformance/SAO_D_Samsung_5.bit -sws_flags bitexact > > > > FATE_HEVC += fate-hevc-skiploopfilter > > > > -- > > > > 2.23.0 > > > > > > > > > > ping > > > > > ping > > > ping > ping
On Sun, 10. Nov 14:55, Andriy Gelman wrote: > On Tue, 05. Nov 09:25, Andriy Gelman wrote: > > On Wed, 30. Oct 09:45, Andriy Gelman wrote: > > > On Thu, 24. Oct 20:50, Andriy Gelman wrote: > > > > On Tue, 15. Oct 22:50, Andriy Gelman wrote: > > > > > From: Andriy Gelman <andriy.gelman@gmail.com> > > > > > > > > > > Fixes #7799 > > > > > > > > > > Currently, the mp4toannexb filter always inserts the same extradata at > > > > > the start of the first IRAP unit. As in ticket #7799, this can lead to > > > > > decoding errors if modified parameter sets are signalled in-band. > > > > > > > > > > Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: > > > > > -RAP_B_Bossen_1.bit > > > > > -RPS_C_ericsson_5.bit > > > > > -SLIST_A_Sony_4.bit > > > > > -SLIST_B_Sony_8.bit > > > > > -SLIST_C_Sony_3.bit > > > > > -SLIST_D_Sony_9.bit > > > > > -TSKIP_A_MS_2.bit > > > > > > > > > > This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets > > > > > during the conversion. The correct combination is inserted at the start > > > > > of the first IRAP. SEIs from extradata are inserted before each IRAP. > > > > > > > > > > This commit also makes an update to the hevc-bsf-mp4toannexb fate test > > > > > since the result before this patch contained duplicate parameter sets > > > > > in-band. > > > > > --- > > > > > libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- > > > > > tests/fate/hevc.mak | 2 +- > > > > > 2 files changed, 472 insertions(+), 33 deletions(-) > > > > > > > > > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c > > > > > index 09bce5b34c..1ca5f13807 100644 > > > > > --- a/libavcodec/hevc_mp4toannexb_bsf.c > > > > > +++ b/libavcodec/hevc_mp4toannexb_bsf.c > > > > > @@ -23,19 +23,224 @@ > > > > > > > > > > #include "libavutil/intreadwrite.h" > > > > > #include "libavutil/mem.h" > > > > > +#include "libavutil/avassert.h" > > > > > > > > > > #include "avcodec.h" > > > > > #include "bsf.h" > > > > > #include "bytestream.h" > > > > > #include "hevc.h" > > > > > +#include "h2645_parse.h" > > > > > +#include "hevc_ps.h" > > > > > +#include "golomb.h" > > > > > > > > > > #define MIN_HEVCC_LENGTH 23 > > > > > +#define PROFILE_WITHOUT_IDC_BITS 88 > > > > > +#define IS_IRAP(s) ((s)->type >= 16 && (s)->type <= 23) > > > > > +#define IS_PARAMSET(s) ((s)->type >= 32 && (s)->type <= 34) > > > > > + > > > > > + /*reserved VCLs not included*/ > > > > > +#define IS_VCL(s) ((s)->type <= 9 || ((s)->type >= 16 && (s)->type <= 21)) > > > > > +#define HEVC_NAL_HEADER_BITS 16 > > > > > + > > > > > +/* > > > > > + * Copies data from input buffer to output buffer. Appends annexb startcode. > > > > > + * out must be allocated at least out_len + in_len + write_startcode * 4. > > > > > + */ > > > > > +#define WRITE_NAL(out, out_len, in, in_len, write_startcode) do { \ > > > > > + if ((write_startcode)) { \ > > > > > + AV_WB32((out) + (out_len), 1); \ > > > > > + (out_len) += 4; \ > > > > > + } \ > > > > > + memcpy((out) + (out_len), (in), (in_len)); \ > > > > > + (out_len) += (in_len); \ > > > > > +} while (0) > > > > > + > > > > > +typedef struct Param { > > > > > + uint8_t *raw_data; /* raw data to store param set payload*/ > > > > > + int raw_size; /* size of raw_data*/ > > > > > + size_t allocated_size; /* allocated size of raw_data*/ > > > > > + int ref; /* stores the ref of the higher level parameter set*/ > > > > > + int is_signalled; /* indicates whether this param has already been signalled in the cvs*/ > > > > > +} Param; > > > > > + > > > > > +/* modified version of HEVCParamSets to store bytestream and reference to previous level*/ > > > > > +typedef struct ParamSets { > > > > > + Param vps_list[HEVC_MAX_VPS_COUNT]; > > > > > + Param sps_list[HEVC_MAX_SPS_COUNT]; > > > > > + Param pps_list[HEVC_MAX_PPS_COUNT]; > > > > > + > > > > > + Param sei; /* cached SEIs from extradatat*/ > > > > > +} ParamSets; > > > > > > > > > > typedef struct HEVCBSFContext { > > > > > uint8_t length_size; > > > > > int extradata_parsed; > > > > > + ParamSets ps; /* cached VPS/SPS/PPS parameter sets + SEI from extradata*/ > > > > > } HEVCBSFContext; > > > > > > > > > > +static int update_cached_paramset(AVBSFContext *ctx, Param *cached_param, > > > > > + H2645NAL *nal, int ref) > > > > > +{ > > > > > + int ret; > > > > > + > > > > > + if (cached_param->raw_data && cached_param->raw_size == nal->raw_size + 4 && > > > > > + !memcmp(cached_param->raw_data + 4, nal->raw_data, nal->raw_size)) { > > > > > + av_log(ctx, AV_LOG_DEBUG, "NAL unit: %d. Copy already exists in parameter set.\n", nal->type); > > > > > + } else { > > > > > + if (nal->raw_size + 4 > cached_param->allocated_size) { > > > > > + ret = av_reallocp(&cached_param->raw_data, nal->raw_size + 4); > > > > > + if (ret < 0) > > > > > + return ret; > > > > > + cached_param->allocated_size = nal->raw_size + 4; > > > > > + } > > > > > + cached_param->raw_size = 0; > > > > > + WRITE_NAL(cached_param->raw_data, cached_param->raw_size, nal->raw_data, nal->raw_size, 1); > > > > > + cached_param->ref = ref; > > > > > + cached_param->is_signalled = 0; > > > > > + av_assert1(cached_param->raw_size == nal->raw_size + 4); > > > > > + } > > > > > + return 0; > > > > > +} > > > > > + > > > > > +static int parse_vps(AVBSFContext *ctx, H2645NAL *nal) > > > > > +{ > > > > > + int vps_id, ret; > > > > > + > > > > > + HEVCBSFContext *s = ctx->priv_data; > > > > > + ParamSets *ps = &s->ps; > > > > > + > > > > > + GetBitContext *gb = &nal->gb; > > > > > + > > > > > + vps_id = get_bits(gb, 4); > > > > > + > > > > > + if (get_bits_left(gb) < 0) { > > > > > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in VPS id: %d\n", vps_id); > > > > > + return AVERROR_INVALIDDATA; > > > > > + } > > > > > + > > > > > + av_log(ctx, AV_LOG_TRACE, "Updating VPS id: %d\n", vps_id); > > > > > + ret = update_cached_paramset(ctx, &ps->vps_list[vps_id], nal, 0); > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static int parse_sps(AVBSFContext *ctx, H2645NAL *nal) > > > > > +{ > > > > > + int i, ret; > > > > > + int sps_id, vps_ref, max_sub_layers_minus1; > > > > > + > > > > > + HEVCBSFContext *s = ctx->priv_data; > > > > > + ParamSets *ps = &s->ps; > > > > > + > > > > > + uint8_t sub_layer_profile_present_flag[HEVC_MAX_SUB_LAYERS]; > > > > > + uint8_t sub_layer_level_present_flag[HEVC_MAX_SUB_LAYERS]; > > > > > + > > > > > + GetBitContext *gb = &nal->gb; > > > > > + > > > > > + vps_ref = get_bits(gb, 4); > > > > > + > > > > > + max_sub_layers_minus1 = get_bits(gb, 3); > > > > > + skip_bits1(gb); /* sps_temporal_id_nesting_flag*/ > > > > > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > > > > > + skip_bits(gb, 8); /* general_level_idc*/ > > > > > + > > > > > + for (i = 0; i < max_sub_layers_minus1; i++) { > > > > > + sub_layer_profile_present_flag[i] = get_bits1(gb); > > > > > + sub_layer_level_present_flag[i] = get_bits1(gb); > > > > > + } > > > > > + > > > > > + if (max_sub_layers_minus1 > 0) > > > > > + for (i = max_sub_layers_minus1; i < 8; i++) > > > > > + skip_bits(gb, 2); /* reserved_zero_2bits[i]*/ > > > > > + > > > > > + for (i = 0; i < max_sub_layers_minus1; i++) { > > > > > + if (sub_layer_profile_present_flag[i]) > > > > > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > > > > > + if (sub_layer_level_present_flag[i]) > > > > > + skip_bits(gb, 8); /* sub_layer_level_idc*/ > > > > > + } > > > > > + > > > > > + /*we only need the sps_id index*/ > > > > > + sps_id = get_ue_golomb(gb); > > > > > + if (sps_id < 0 || sps_id >= HEVC_MAX_SPS_COUNT) { > > > > > + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_id); > > > > > + return AVERROR_INVALIDDATA; > > > > > + } > > > > > + > > > > > + if (get_bits_left(gb) < 0) { > > > > > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in SPS id: %d\n", sps_id); > > > > > + return AVERROR_INVALIDDATA; > > > > > + } > > > > > + > > > > > + av_log(ctx, AV_LOG_TRACE, "Updating SPS id: %d, VPS ref: %d\n", sps_id, vps_ref); > > > > > + ret = update_cached_paramset(ctx, &ps->sps_list[sps_id], nal, vps_ref); > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static int parse_pps(AVBSFContext *ctx, H2645NAL *nal) > > > > > +{ > > > > > + int ret; > > > > > + int pps_id, sps_ref; > > > > > + > > > > > + HEVCBSFContext *s = ctx->priv_data; > > > > > + ParamSets *ps = &s->ps; > > > > > + > > > > > + GetBitContext *gb = &nal->gb; > > > > > + > > > > > + pps_id = get_ue_golomb(gb); > > > > > + if (pps_id < 0 || pps_id >= HEVC_MAX_PPS_COUNT) { > > > > > + av_log(ctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id); > > > > > + return AVERROR_INVALIDDATA; > > > > > + } > > > > > + > > > > > + sps_ref = get_ue_golomb(gb); > > > > > + if (sps_ref < 0 || sps_ref >= HEVC_MAX_SPS_COUNT) { > > > > > + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_ref); > > > > > + return AVERROR_INVALIDDATA; > > > > > + } > > > > > + > > > > > + if (get_bits_left(gb) < 0) { > > > > > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in PPS id: %d\n", pps_id); > > > > > + return AVERROR_INVALIDDATA; > > > > > + } > > > > > + > > > > > + av_log(ctx, AV_LOG_TRACE, "Updating PPS id: %d, SPS ref: %d\n", pps_id, sps_ref); > > > > > + ret = update_cached_paramset(ctx, &ps->pps_list[pps_id], nal, sps_ref); > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static int append_sei_annexb(Param *sei, H2645NAL *nal) > > > > > +{ > > > > > + int ret; > > > > > + > > > > > + ret = av_reallocp(&sei->raw_data, sei->allocated_size + nal->raw_size + 4); > > > > > + if (ret < 0) > > > > > + return ret; > > > > > + sei->allocated_size += nal->raw_size + 4; > > > > > + > > > > > + WRITE_NAL(sei->raw_data, sei->raw_size, nal->raw_data, nal->raw_size, 1); > > > > > + av_assert1(sei->raw_size == sei->allocated_size); > > > > > + return 0; > > > > > +} > > > > > + > > > > > +static int update_paramset(AVBSFContext *ctx, H2645NAL *nal) > > > > > +{ > > > > > + int ret; > > > > > + switch (nal->type) { > > > > > + case (HEVC_NAL_VPS): > > > > > + if ((ret = parse_vps(ctx, nal)) < 0) > > > > > + return ret; > > > > > + break; > > > > > + case (HEVC_NAL_SPS): > > > > > + if ((ret = parse_sps(ctx, nal)) < 0) > > > > > + return ret; > > > > > + break; > > > > > + case (HEVC_NAL_PPS): > > > > > + if ((ret = parse_pps(ctx, nal)) < 0) > > > > > + return ret; > > > > > + } > > > > > + return 0; > > > > > +} > > > > > + > > > > > static int hevc_extradata_to_annexb(AVBSFContext *ctx) > > > > > { > > > > > GetByteContext gb; > > > > > @@ -97,6 +302,7 @@ fail: > > > > > static int hevc_mp4toannexb_init(AVBSFContext *ctx) > > > > > { > > > > > HEVCBSFContext *s = ctx->priv_data; > > > > > + H2645Packet pkt; > > > > > int ret; > > > > > > > > > > if (ctx->par_in->extradata_size < MIN_HEVCC_LENGTH || > > > > > @@ -104,77 +310,309 @@ static int hevc_mp4toannexb_init(AVBSFContext *ctx) > > > > > AV_RB32(ctx->par_in->extradata) == 1) { > > > > > av_log(ctx, AV_LOG_VERBOSE, > > > > > "The input looks like it is Annex B already\n"); > > > > > + return 0; > > > > > } else { > > > > > ret = hevc_extradata_to_annexb(ctx); > > > > > if (ret < 0) > > > > > return ret; > > > > > s->length_size = ret; > > > > > s->extradata_parsed = 1; > > > > > + > > > > > + memset(&pkt, 0, sizeof(H2645Packet)); > > > > > + ret = ff_h2645_packet_split(&pkt, ctx->par_out->extradata, ctx->par_out->extradata_size, > > > > > + ctx, 0, 0, AV_CODEC_ID_HEVC, 1, 0); > > > > > + if (ret < 0) > > > > > + goto done; > > > > > + > > > > > + for (int i = 0; i < pkt.nb_nals; ++i) { > > > > > + H2645NAL *nal = &pkt.nals[i]; > > > > > + > > > > > + /*current segmentation algorithm includes next 0x00 from next nal unit*/ > > > > > + if (nal->raw_data[nal->raw_size - 1] == 0x00) > > > > > + nal->raw_size--; > > > > > + > > > > > + if (IS_PARAMSET(nal)) { > > > > > + ret = update_paramset(ctx, nal); > > > > > + if (ret < 0) > > > > > + goto done; > > > > > + continue; > > > > > + } > > > > > + > > > > > + if (nal->type == HEVC_NAL_SEI_PREFIX || nal->type == HEVC_NAL_SEI_SUFFIX) { > > > > > + ret = append_sei_annexb(&s->ps.sei, nal); > > > > > + if (ret < 0) > > > > > + goto done; > > > > > + } > > > > > + } > > > > > } > > > > > +done: > > > > > + ff_h2645_packet_uninit(&pkt); > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static void ps_uninit(ParamSets *ps) > > > > > +{ > > > > > + int i; > > > > > + for (i = 0; i < FF_ARRAY_ELEMS(ps->vps_list); i++) { > > > > > + av_freep(&ps->vps_list[i].raw_data); > > > > > + ps->vps_list[i].allocated_size = 0; > > > > > + ps->vps_list[i].raw_size = 0; > > > > > + } > > > > > + for (i = 0; i < FF_ARRAY_ELEMS(ps->sps_list); i++) { > > > > > + av_freep(&ps->sps_list[i].raw_data); > > > > > + ps->sps_list[i].allocated_size = 0; > > > > > + ps->sps_list[i].raw_size = 0; > > > > > + } > > > > > + for (i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++) { > > > > > + av_freep(&ps->pps_list[i].raw_data); > > > > > + ps->pps_list[i].allocated_size = 0; > > > > > + ps->pps_list[i].raw_size = 0; > > > > > + } > > > > > + av_freep(&ps->sei); > > > > > + ps->sei.allocated_size = 0; > > > > > + ps->sei.raw_size = 0; > > > > > +} > > > > > + > > > > > +static void hevc_mp4toannexb_close(AVBSFContext *ctx) > > > > > +{ > > > > > + HEVCBSFContext *s = ctx->priv_data; > > > > > + ps_uninit(&s->ps); > > > > > +} > > > > > + > > > > > +static int get_vcl_pps_id(AVBSFContext *ctx, H2645NAL *nal) > > > > > +{ > > > > > + GetBitContext *gb = &nal->gb; > > > > > + int pps_id, ret; > > > > > + > > > > > + /* re-initialize because bitstream touched in write_extradata*/ > > > > > + ret = init_get_bits(gb, nal->data, nal->size_bits); > > > > > + if (ret < 0) > > > > > + return ret; > > > > > + skip_bits(gb, HEVC_NAL_HEADER_BITS); > > > > > + > > > > > + skip_bits1(gb); /* first_slice_segment_in_pic_flag*/ > > > > > + > > > > > + if (IS_IRAP(nal)) > > > > > + skip_bits1(gb); /* no_output_of_prior_pics_flag*/ > > > > > + > > > > > + pps_id = get_ue_golomb(gb); > > > > > + if (pps_id < 0 || pps_id >= HEVC_MAX_PPS_COUNT) { > > > > > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS id: %d\n", pps_id); > > > > > + return AVERROR_INVALIDDATA; > > > > > + } > > > > > + return pps_id; > > > > > +} > > > > > + > > > > > +static int write_vcl(AVBSFContext *ctx, AVPacket *pkt_out, int *prev_size, > > > > > + H2645NAL *nal) > > > > > +{ > > > > > + HEVCBSFContext *s = ctx->priv_data; > > > > > + ParamSets *ps = &s->ps; > > > > > + int ret, pps_id; > > > > > + Param *pps; > > > > > + > > > > > + /* get frame pps*/ > > > > > + pps_id = get_vcl_pps_id(ctx, nal); > > > > > + if (pps_id < 0) > > > > > + return AVERROR_INVALIDDATA; > > > > > + av_log(ctx, AV_LOG_TRACE, "VCL PPS id: %d\n", pps_id); > > > > > + > > > > > + /* write pps if not signalled*/ > > > > > + pps = &ps->pps_list[pps_id]; > > > > > + if (!pps->raw_data) { > > > > > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d doesn't exist\n", pps_id); > > > > > + return AVERROR_INVALIDDATA; > > > > > + } > > > > > + > > > > > + if (!pps->is_signalled) { > > > > > + av_log(ctx, AV_LOG_TRACE, "Referenced PPS: %d has not been signalled. Writing PPS to output stream\n", pps_id); > > > > > + ret = av_grow_packet(pkt_out, pps->raw_size); > > > > > + if (ret < 0) > > > > > + return ret; > > > > > + WRITE_NAL(pkt_out->data, *prev_size, pps->raw_data, pps->raw_size, 0); > > > > > + pps->is_signalled = 1; > > > > > + } > > > > > + > > > > > + /* write actual packet*/ > > > > > + ret = av_grow_packet(pkt_out, nal->raw_size + 4); > > > > > + if (ret < 0) > > > > > + return ret; > > > > > + WRITE_NAL(pkt_out->data, *prev_size, nal->raw_data, nal->raw_size, 1); > > > > > > > > > > return 0; > > > > > } > > > > > > > > > > +static int write_extradata(AVBSFContext *ctx, AVPacket *pkt_out, int *prev_size, > > > > > + H2645NAL *nal_irap) > > > > > +{ > > > > > + int ref, ret; > > > > > + int new_extradata_size = 0; > > > > > + > > > > > + HEVCBSFContext *s = ctx->priv_data; > > > > > + ParamSets *ps = &s->ps; > > > > > + GetBitContext *gb = &nal_irap->gb; > > > > > + > > > > > + /* active parameter sets for the new irap*/ > > > > > + const Param *vps; > > > > > + const Param *sps; > > > > > + Param *pps; /* non-const because we have to update flag that pps_id is signalled in cvs*/ > > > > > + > > > > > + skip_bits1(gb); /* first_slice_ic_pic_flag*/ > > > > > + skip_bits1(gb); /* no_output_of_prior_pics_flag*/ > > > > > + > > > > > + ref = get_ue_golomb(gb); > > > > > + if (ref < 0 || ref >= HEVC_MAX_PPS_COUNT || !ps->pps_list[ref].raw_data) { > > > > > + av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d\n", ref); > > > > > + return AVERROR_INVALIDDATA; > > > > > + } > > > > > + av_log(ctx, AV_LOG_TRACE, "Writing PPS id: %d\n", ref); > > > > > + pps = &ps->pps_list[ref]; > > > > > + new_extradata_size += pps->raw_size; > > > > > + ref = pps->ref; > > > > > + > > > > > + if (!ps->sps_list[ref].raw_data) { > > > > > + av_log(ctx, AV_LOG_ERROR, "Invalid SPS: %d\n", ref); > > > > > + return AVERROR_INVALIDDATA; > > > > > + } > > > > > + av_log(ctx, AV_LOG_TRACE, "Writing SPS id: %d\n", ref); > > > > > + sps = &ps->sps_list[ref]; > > > > > + new_extradata_size += sps->raw_size; > > > > > + ref = sps->ref; > > > > > + > > > > > + if (!ps->vps_list[ref].raw_data) { > > > > > + av_log(ctx, AV_LOG_ERROR, "Invalid VPS: %d\n", ref); > > > > > + return AVERROR_INVALIDDATA; > > > > > + } > > > > > + av_log(ctx, AV_LOG_TRACE, "Writing VPS id: %d\n", ref); > > > > > + vps = &ps->vps_list[ref]; > > > > > + new_extradata_size += vps->raw_size; > > > > > + > > > > > + if (ps->sei.raw_data) > > > > > + new_extradata_size += ps->sei.raw_size; > > > > > + > > > > > + ret = av_grow_packet(pkt_out, new_extradata_size); > > > > > + if (ret < 0) > > > > > + return ret; > > > > > + > > > > > + WRITE_NAL(pkt_out->data, *prev_size, vps->raw_data, vps->raw_size, 0); > > > > > + WRITE_NAL(pkt_out->data, *prev_size, sps->raw_data, sps->raw_size, 0); > > > > > + WRITE_NAL(pkt_out->data, *prev_size, pps->raw_data, pps->raw_size, 0); > > > > > + pps->is_signalled = 1; > > > > > + > > > > > + if (ps->sei.raw_data) > > > > > + WRITE_NAL(pkt_out->data, *prev_size, ps->sei.raw_data, ps->sei.raw_size, 0); > > > > > + > > > > > + av_assert1(*prev_size == pkt_out->size); > > > > > + return 0; > > > > > +} > > > > > + > > > > > +/* > > > > > + * Function converts mp4 access unit into annexb > > > > > + * Output packet structure > > > > > + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] > > > > > + * or > > > > > + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] > > > > > + */ > > > > > static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > > > > > { > > > > > HEVCBSFContext *s = ctx->priv_data; > > > > > AVPacket *in; > > > > > - GetByteContext gb; > > > > > - > > > > > - int got_irap = 0; > > > > > - int i, ret = 0; > > > > > + H2645Packet pkt; > > > > > + int i, j, prev_size, ret; > > > > > + int irap_done; > > > > > > > > > > ret = ff_bsf_get_packet(ctx, &in); > > > > > if (ret < 0) > > > > > return ret; > > > > > > > > > > + /* output the annexb nalu if extradata is not parsed*/ > > > > > if (!s->extradata_parsed) { > > > > > av_packet_move_ref(out, in); > > > > > av_packet_free(&in); > > > > > return 0; > > > > > } > > > > > > > > > > - bytestream2_init(&gb, in->data, in->size); > > > > > + memset(&pkt, 0, sizeof(H2645Packet)); > > > > > + ret = ff_h2645_packet_split(&pkt, in->data, in->size, ctx, 1, s->length_size, AV_CODEC_ID_HEVC, 1, 0); > > > > > + if (ret < 0) > > > > > + goto done; > > > > > + > > > > > + irap_done = 1; /* 1 means that there is no irap or the irap has already been processed.*/ > > > > > + for (i = 0; i < pkt.nb_nals; i++) { > > > > > + if (IS_IRAP(&pkt.nals[i])) { > > > > > + irap_done = 0; > > > > > + break; > > > > > + } > > > > > + } > > > > > + > > > > > + prev_size = out->size; /* prev_size stores the current length of output packet */ > > > > > + av_assert1(prev_size == 0); > > > > > > > > > > - while (bytestream2_get_bytes_left(&gb)) { > > > > > - uint32_t nalu_size = 0; > > > > > - int nalu_type; > > > > > - int is_irap, add_extradata, extra_size, prev_size; > > > > > + for (i = 0; i < pkt.nb_nals; i++) { > > > > > + H2645NAL *nal = &pkt.nals[i]; > > > > > + > > > > > + if (IS_PARAMSET(nal)) { > > > > > + ret = update_paramset(ctx, nal); > > > > > + if (ret < 0) > > > > > + goto done; > > > > > + continue; > > > > > + } > > > > > > > > > > - for (i = 0; i < s->length_size; i++) > > > > > - nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb); > > > > > + if (!irap_done && IS_IRAP(nal)) { /* append extradata and sei before first irap*/ > > > > > > > > > > - nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f; > > > > > + /* reset the pps signalled flag in this cvs*/ > > > > > + for (j = 0; j < HEVC_MAX_PPS_COUNT; j++) > > > > > + s->ps.pps_list[j].is_signalled = 0; > > > > > > > > > > - /* prepend extradata to IRAP frames */ > > > > > - is_irap = nalu_type >= 16 && nalu_type <= 23; > > > > > - add_extradata = is_irap && !got_irap; > > > > > - extra_size = add_extradata * ctx->par_out->extradata_size; > > > > > - got_irap |= is_irap; > > > > > + ret = write_extradata(ctx, out, &prev_size, nal); > > > > > + if (ret < 0) > > > > > + goto done; > > > > > + > > > > > + /* append any past SEI prefix nals*/ > > > > > + for (j = 0; j < i; j++) { > > > > > + H2645NAL *nal_past = &pkt.nals[j]; > > > > > + if (nal_past->type == HEVC_NAL_SEI_PREFIX) { > > > > > + ret = av_grow_packet(out, nal_past->raw_size + 4); > > > > > + if (ret < 0) > > > > > + goto done; > > > > > + WRITE_NAL(out->data, prev_size, nal_past->raw_data, nal_past->raw_size, 1); > > > > > + } > > > > > + } > > > > > > > > > > - if (SIZE_MAX - nalu_size < 4 || > > > > > - SIZE_MAX - 4 - nalu_size < extra_size) { > > > > > - ret = AVERROR_INVALIDDATA; > > > > > - goto fail; > > > > > + /* write irap nal unit*/ > > > > > + ret = write_vcl(ctx, out, &prev_size, nal); > > > > > + if (ret < 0) > > > > > + goto done; > > > > > + irap_done = 1; > > > > > + continue; > > > > > } > > > > > > > > > > - prev_size = out->size; > > > > > + /* do not write any sei prefix nal units if irap exists but has not been > > > > > + * processed*/ > > > > > + if (nal->type == HEVC_NAL_SEI_PREFIX && !irap_done) > > > > > + continue; > > > > > > > > > > - ret = av_grow_packet(out, 4 + nalu_size + extra_size); > > > > > - if (ret < 0) > > > > > - goto fail; > > > > > + if (IS_VCL(nal)) { > > > > > + ret = write_vcl(ctx, out, &prev_size, nal); > > > > > + if (ret < 0) > > > > > + goto done; > > > > > + continue; > > > > > + } > > > > > > > > > > - if (add_extradata) > > > > > - memcpy(out->data + prev_size, ctx->par_out->extradata, extra_size); > > > > > - AV_WB32(out->data + prev_size + extra_size, 1); > > > > > - bytestream2_get_buffer(&gb, out->data + prev_size + 4 + extra_size, nalu_size); > > > > > + /*copy any other nal units to the output - i.e. SEI_SUFFIX*/ > > > > > + ret = av_grow_packet(out, nal->raw_size + 4); > > > > > + if (ret < 0) > > > > > + goto done; > > > > > + WRITE_NAL(out->data, prev_size, nal->raw_data, nal->raw_size, 1); > > > > > } > > > > > > > > > > + av_assert1(prev_size == out->size); > > > > > + > > > > > ret = av_packet_copy_props(out, in); > > > > > - if (ret < 0) > > > > > - goto fail; > > > > > > > > > > -fail: > > > > > +done: > > > > > + ff_h2645_packet_uninit(&pkt); > > > > > + > > > > > if (ret < 0) > > > > > av_packet_unref(out); > > > > > av_packet_free(&in); > > > > > @@ -190,6 +628,7 @@ const AVBitStreamFilter ff_hevc_mp4toannexb_bsf = { > > > > > .name = "hevc_mp4toannexb", > > > > > .priv_data_size = sizeof(HEVCBSFContext), > > > > > .init = hevc_mp4toannexb_init, > > > > > + .close = hevc_mp4toannexb_close, > > > > > .filter = hevc_mp4toannexb_filter, > > > > > .codec_ids = codec_ids, > > > > > }; > > > > > diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak > > > > > index 559c3898bc..4f812b0834 100644 > > > > > --- a/tests/fate/hevc.mak > > > > > +++ b/tests/fate/hevc.mak > > > > > @@ -238,7 +238,7 @@ FATE_HEVC-$(call ALLYES, HEVC_DEMUXER MOV_DEMUXER HEVC_MP4TOANNEXB_BSF MOV_MUXER > > > > > fate-hevc-bsf-mp4toannexb: tests/data/hevc-mp4.mov > > > > > fate-hevc-bsf-mp4toannexb: CMD = md5 -i $(TARGET_PATH)/tests/data/hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc > > > > > fate-hevc-bsf-mp4toannexb: CMP = oneline > > > > > -fate-hevc-bsf-mp4toannexb: REF = 1873662a3af1848c37e4eb25722c8df9 > > > > > +fate-hevc-bsf-mp4toannexb: REF = 3c9d998a3aa2b9e0fb1c1f434952bf8b > > > > > > > > > > fate-hevc-skiploopfilter: CMD = framemd5 -skip_loop_filter nokey -i $(TARGET_SAMPLES)/hevc-conformance/SAO_D_Samsung_5.bit -sws_flags bitexact > > > > > FATE_HEVC += fate-hevc-skiploopfilter > > > > > -- > > > > > 2.23.0 > > > > > > > > > > > > > ping > > > > > > > ping > > > > > ping > > > ping > ping
On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote: > From: Andriy Gelman <andriy.gelman@gmail.com> > > Fixes #7799 > > Currently, the mp4toannexb filter always inserts the same extradata at > the start of the first IRAP unit. As in ticket #7799, this can lead to > decoding errors if modified parameter sets are signalled in-band. > > Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: > -RAP_B_Bossen_1.bit > -RPS_C_ericsson_5.bit > -SLIST_A_Sony_4.bit > -SLIST_B_Sony_8.bit > -SLIST_C_Sony_3.bit > -SLIST_D_Sony_9.bit > -TSKIP_A_MS_2.bit > > This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets > during the conversion. The correct combination is inserted at the start > of the first IRAP. SEIs from extradata are inserted before each IRAP. > > This commit also makes an update to the hevc-bsf-mp4toannexb fate test > since the result before this patch contained duplicate parameter sets > in-band. > --- > libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- > tests/fate/hevc.mak | 2 +- > 2 files changed, 472 insertions(+), 33 deletions(-) > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c > index 09bce5b34c..1ca5f13807 100644 > --- a/libavcodec/hevc_mp4toannexb_bsf.c > +++ b/libavcodec/hevc_mp4toannexb_bsf.c > @@ -23,19 +23,224 @@ > > #include "libavutil/intreadwrite.h" > #include "libavutil/mem.h" > +#include "libavutil/avassert.h" > > #include "avcodec.h" > #include "bsf.h" > #include "bytestream.h" > #include "hevc.h" > +#include "h2645_parse.h" > +#include "hevc_ps.h" > +#include "golomb.h" > > #define MIN_HEVCC_LENGTH 23 > +#define PROFILE_WITHOUT_IDC_BITS 88 > +#define IS_IRAP(s) ((s)->type >= 16 && (s)->type <= 23) This is kind of duplicated, it would be more ideal if no macros are duplicated > +#define IS_PARAMSET(s) ((s)->type >= 32 && (s)->type <= 34) > + > + /*reserved VCLs not included*/ > +#define IS_VCL(s) ((s)->type <= 9 || ((s)->type >= 16 && (s)->type <= 21)) > +#define HEVC_NAL_HEADER_BITS 16 [...] > +static int parse_sps(AVBSFContext *ctx, H2645NAL *nal) > +{ > + int i, ret; > + int sps_id, vps_ref, max_sub_layers_minus1; > + > + HEVCBSFContext *s = ctx->priv_data; > + ParamSets *ps = &s->ps; > + > + uint8_t sub_layer_profile_present_flag[HEVC_MAX_SUB_LAYERS]; > + uint8_t sub_layer_level_present_flag[HEVC_MAX_SUB_LAYERS]; > + > + GetBitContext *gb = &nal->gb; > + > + vps_ref = get_bits(gb, 4); > + > + max_sub_layers_minus1 = get_bits(gb, 3); > + skip_bits1(gb); /* sps_temporal_id_nesting_flag*/ > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > + skip_bits(gb, 8); /* general_level_idc*/ > + > + for (i = 0; i < max_sub_layers_minus1; i++) { > + sub_layer_profile_present_flag[i] = get_bits1(gb); > + sub_layer_level_present_flag[i] = get_bits1(gb); > + } > + > + if (max_sub_layers_minus1 > 0) > + for (i = max_sub_layers_minus1; i < 8; i++) > + skip_bits(gb, 2); /* reserved_zero_2bits[i]*/ > + > + for (i = 0; i < max_sub_layers_minus1; i++) { > + if (sub_layer_profile_present_flag[i]) > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > + if (sub_layer_level_present_flag[i]) > + skip_bits(gb, 8); /* sub_layer_level_idc*/ > + } > + > + /*we only need the sps_id index*/ > + sps_id = get_ue_golomb(gb); > + if (sps_id < 0 || sps_id >= HEVC_MAX_SPS_COUNT) { > + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_id); > + return AVERROR_INVALIDDATA; > + } > + > + if (get_bits_left(gb) < 0) { > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in SPS id: %d\n", sps_id); > + return AVERROR_INVALIDDATA; > + } > + > + av_log(ctx, AV_LOG_TRACE, "Updating SPS id: %d, VPS ref: %d\n", sps_id, vps_ref); > + ret = update_cached_paramset(ctx, &ps->sps_list[sps_id], nal, vps_ref); > + return ret; the intermediate ret is useless here, the same issue occurs in other cases too [...] > + > static int hevc_extradata_to_annexb(AVBSFContext *ctx) > { > GetByteContext gb; > @@ -97,6 +302,7 @@ fail: > static int hevc_mp4toannexb_init(AVBSFContext *ctx) > { > HEVCBSFContext *s = ctx->priv_data; > + H2645Packet pkt; > int ret; > > if (ctx->par_in->extradata_size < MIN_HEVCC_LENGTH || > @@ -104,77 +310,309 @@ static int hevc_mp4toannexb_init(AVBSFContext *ctx) > AV_RB32(ctx->par_in->extradata) == 1) { > av_log(ctx, AV_LOG_VERBOSE, > "The input looks like it is Annex B already\n"); > + return 0; > } else { > ret = hevc_extradata_to_annexb(ctx); > if (ret < 0) > return ret; > s->length_size = ret; > s->extradata_parsed = 1; > + > + memset(&pkt, 0, sizeof(H2645Packet)); > + ret = ff_h2645_packet_split(&pkt, ctx->par_out->extradata, ctx->par_out->extradata_size, > + ctx, 0, 0, AV_CODEC_ID_HEVC, 1, 0); > + if (ret < 0) > + goto done; > + > + for (int i = 0; i < pkt.nb_nals; ++i) { > + H2645NAL *nal = &pkt.nals[i]; > + > + /*current segmentation algorithm includes next 0x00 from next nal unit*/ > + if (nal->raw_data[nal->raw_size - 1] == 0x00) Can raw_size be 0 here ? a check or assert on raw_size > 0 might make this more robust [...] > +/* > + * Function converts mp4 access unit into annexb > + * Output packet structure > + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] > + * or > + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] > + */ > static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > { > HEVCBSFContext *s = ctx->priv_data; > AVPacket *in; > - GetByteContext gb; > - > - int got_irap = 0; > - int i, ret = 0; > + H2645Packet pkt; > + int i, j, prev_size, ret; > + int irap_done; > > ret = ff_bsf_get_packet(ctx, &in); > if (ret < 0) > return ret; > > + /* output the annexb nalu if extradata is not parsed*/ > if (!s->extradata_parsed) { > av_packet_move_ref(out, in); > av_packet_free(&in); > return 0; > } > > - bytestream2_init(&gb, in->data, in->size); Is this really better without using an existing bytestream* API ? Also as you mentioned, the nuh_layer_id != 0 issue which mkver found. That also should be fixed Thanks [...]
On Sun, 24. Nov 00:02, Michael Niedermayer wrote: > On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote: > > From: Andriy Gelman <andriy.gelman@gmail.com> > > > > Fixes #7799 > > > > Currently, the mp4toannexb filter always inserts the same extradata at > > the start of the first IRAP unit. As in ticket #7799, this can lead to > > decoding errors if modified parameter sets are signalled in-band. > > > > Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: > > -RAP_B_Bossen_1.bit > > -RPS_C_ericsson_5.bit > > -SLIST_A_Sony_4.bit > > -SLIST_B_Sony_8.bit > > -SLIST_C_Sony_3.bit > > -SLIST_D_Sony_9.bit > > -TSKIP_A_MS_2.bit > > > > This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets > > during the conversion. The correct combination is inserted at the start > > of the first IRAP. SEIs from extradata are inserted before each IRAP. > > > > This commit also makes an update to the hevc-bsf-mp4toannexb fate test > > since the result before this patch contained duplicate parameter sets > > in-band. > > --- > > libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- > > tests/fate/hevc.mak | 2 +- > > 2 files changed, 472 insertions(+), 33 deletions(-) > > > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c > > index 09bce5b34c..1ca5f13807 100644 > > --- a/libavcodec/hevc_mp4toannexb_bsf.c > > +++ b/libavcodec/hevc_mp4toannexb_bsf.c > > @@ -23,19 +23,224 @@ > > > > #include "libavutil/intreadwrite.h" > > #include "libavutil/mem.h" > > +#include "libavutil/avassert.h" > > > > #include "avcodec.h" > > #include "bsf.h" > > #include "bytestream.h" > > #include "hevc.h" > > +#include "h2645_parse.h" > > +#include "hevc_ps.h" > > +#include "golomb.h" > > > > #define MIN_HEVCC_LENGTH 23 > > +#define PROFILE_WITHOUT_IDC_BITS 88 > > > +#define IS_IRAP(s) ((s)->type >= 16 && (s)->type <= 23) > > This is kind of duplicated, it would be more ideal if no macros are duplicated The same macro is defined hevdec.h. I'll use this one. > > > > +#define IS_PARAMSET(s) ((s)->type >= 32 && (s)->type <= 34) > > + > > + /*reserved VCLs not included*/ > > +#define IS_VCL(s) ((s)->type <= 9 || ((s)->type >= 16 && (s)->type <= 21)) > > +#define HEVC_NAL_HEADER_BITS 16 > > > > [...] > > > +static int parse_sps(AVBSFContext *ctx, H2645NAL *nal) > > +{ > > + int i, ret; > > + int sps_id, vps_ref, max_sub_layers_minus1; > > + > > + HEVCBSFContext *s = ctx->priv_data; > > + ParamSets *ps = &s->ps; > > + > > + uint8_t sub_layer_profile_present_flag[HEVC_MAX_SUB_LAYERS]; > > + uint8_t sub_layer_level_present_flag[HEVC_MAX_SUB_LAYERS]; > > + > > + GetBitContext *gb = &nal->gb; > > + > > + vps_ref = get_bits(gb, 4); > > + > > + max_sub_layers_minus1 = get_bits(gb, 3); > > + skip_bits1(gb); /* sps_temporal_id_nesting_flag*/ > > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > > + skip_bits(gb, 8); /* general_level_idc*/ > > + > > + for (i = 0; i < max_sub_layers_minus1; i++) { > > + sub_layer_profile_present_flag[i] = get_bits1(gb); > > + sub_layer_level_present_flag[i] = get_bits1(gb); > > + } > > + > > + if (max_sub_layers_minus1 > 0) > > + for (i = max_sub_layers_minus1; i < 8; i++) > > + skip_bits(gb, 2); /* reserved_zero_2bits[i]*/ > > + > > + for (i = 0; i < max_sub_layers_minus1; i++) { > > + if (sub_layer_profile_present_flag[i]) > > + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ > > + if (sub_layer_level_present_flag[i]) > > + skip_bits(gb, 8); /* sub_layer_level_idc*/ > > + } > > + > > + /*we only need the sps_id index*/ > > + sps_id = get_ue_golomb(gb); > > + if (sps_id < 0 || sps_id >= HEVC_MAX_SPS_COUNT) { > > + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_id); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + if (get_bits_left(gb) < 0) { > > + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in SPS id: %d\n", sps_id); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + av_log(ctx, AV_LOG_TRACE, "Updating SPS id: %d, VPS ref: %d\n", sps_id, vps_ref); > > + ret = update_cached_paramset(ctx, &ps->sps_list[sps_id], nal, vps_ref); > > + return ret; > > the intermediate ret is useless here, the same issue occurs in other cases too ok, will update. > [...] > > > + > > static int hevc_extradata_to_annexb(AVBSFContext *ctx) > > { > > GetByteContext gb; > > @@ -97,6 +302,7 @@ fail: > > static int hevc_mp4toannexb_init(AVBSFContext *ctx) > > { > > HEVCBSFContext *s = ctx->priv_data; > > + H2645Packet pkt; > > int ret; > > > > if (ctx->par_in->extradata_size < MIN_HEVCC_LENGTH || > > @@ -104,77 +310,309 @@ static int hevc_mp4toannexb_init(AVBSFContext *ctx) > > AV_RB32(ctx->par_in->extradata) == 1) { > > av_log(ctx, AV_LOG_VERBOSE, > > "The input looks like it is Annex B already\n"); > > + return 0; > > } else { > > ret = hevc_extradata_to_annexb(ctx); > > if (ret < 0) > > return ret; > > s->length_size = ret; > > s->extradata_parsed = 1; > > + > > + memset(&pkt, 0, sizeof(H2645Packet)); > > + ret = ff_h2645_packet_split(&pkt, ctx->par_out->extradata, ctx->par_out->extradata_size, > > + ctx, 0, 0, AV_CODEC_ID_HEVC, 1, 0); > > + if (ret < 0) > > + goto done; > > + > > + for (int i = 0; i < pkt.nb_nals; ++i) { > > + H2645NAL *nal = &pkt.nals[i]; > > + > > > + /*current segmentation algorithm includes next 0x00 from next nal unit*/ > > + if (nal->raw_data[nal->raw_size - 1] == 0x00) > > Can raw_size be 0 here ? > a check or assert on raw_size > 0 might make this more robust ff_h2645_packet_split throws away any packets where rbsp_size <= 0 (h2645_parse.c:508). Because raw_size >= rbsp_size, implies raw_size > 0. I'll add av_assert0 as you suggested. Also patch 3/3 removes this line altogether by using idea that extradata is in HVCC format and there is no need to segment by searching for the startcode. I can squash if you think it's best. > > [...] > > +/* > > + * Function converts mp4 access unit into annexb > > + * Output packet structure > > + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] > > + * or > > + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] > > + */ > > static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > > { > > HEVCBSFContext *s = ctx->priv_data; > > AVPacket *in; > > - GetByteContext gb; > > - > > - int got_irap = 0; > > - int i, ret = 0; > > + H2645Packet pkt; > > + int i, j, prev_size, ret; > > + int irap_done; > > > > ret = ff_bsf_get_packet(ctx, &in); > > if (ret < 0) > > return ret; > > > > + /* output the annexb nalu if extradata is not parsed*/ > > if (!s->extradata_parsed) { > > av_packet_move_ref(out, in); > > av_packet_free(&in); > > return 0; > > } > > > > > - bytestream2_init(&gb, in->data, in->size); > > Is this really better without using an existing bytestream* API ? If I use the API, then I'd have to call bytestraem2_init for each nal unit. I also don't use the bytestream2_get_byte function. It seemed simpler to use the WRITE_NAL macro. But maybe I've misunderstood your question. > > Also as you mentioned, the nuh_layer_id != 0 issue which mkver found. > That also should be fixed I'll add an option to ff_h2645_packet_split to not remove such packets. Thank you very much for reviewing.
On Sun, Nov 24, 2019 at 11:29:18AM -0500, Andriy Gelman wrote: > On Sun, 24. Nov 00:02, Michael Niedermayer wrote: > > On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote: > > > From: Andriy Gelman <andriy.gelman@gmail.com> > > > > > > Fixes #7799 > > > > > > Currently, the mp4toannexb filter always inserts the same extradata at > > > the start of the first IRAP unit. As in ticket #7799, this can lead to > > > decoding errors if modified parameter sets are signalled in-band. > > > > > > Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: > > > -RAP_B_Bossen_1.bit > > > -RPS_C_ericsson_5.bit > > > -SLIST_A_Sony_4.bit > > > -SLIST_B_Sony_8.bit > > > -SLIST_C_Sony_3.bit > > > -SLIST_D_Sony_9.bit > > > -TSKIP_A_MS_2.bit > > > > > > This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets > > > during the conversion. The correct combination is inserted at the start > > > of the first IRAP. SEIs from extradata are inserted before each IRAP. > > > > > > This commit also makes an update to the hevc-bsf-mp4toannexb fate test > > > since the result before this patch contained duplicate parameter sets > > > in-band. > > > --- > > > libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- > > > tests/fate/hevc.mak | 2 +- > > > 2 files changed, 472 insertions(+), 33 deletions(-) [...] > > > > [...] > > > +/* > > > + * Function converts mp4 access unit into annexb > > > + * Output packet structure > > > + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] > > > + * or > > > + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] > > > + */ > > > static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > > > { > > > HEVCBSFContext *s = ctx->priv_data; > > > AVPacket *in; > > > - GetByteContext gb; > > > - > > > - int got_irap = 0; > > > - int i, ret = 0; > > > + H2645Packet pkt; > > > + int i, j, prev_size, ret; > > > + int irap_done; > > > > > > ret = ff_bsf_get_packet(ctx, &in); > > > if (ret < 0) > > > return ret; > > > > > > + /* output the annexb nalu if extradata is not parsed*/ > > > if (!s->extradata_parsed) { > > > av_packet_move_ref(out, in); > > > av_packet_free(&in); > > > return 0; > > > } > > > > > > > > - bytestream2_init(&gb, in->data, in->size); > > > > Is this really better without using an existing bytestream* API ? > > If I use the API, then I'd have to call bytestraem2_init for each nal unit. I > also don't use the bytestream2_get_byte function. It seemed simpler to use the WRITE_NAL macro. > > But maybe I've misunderstood your question. i had nothing really specific in mind, just the feeling that using none of the existing APIs there is a bit odd. but more specifically, what about the write side ? thx [...]
On Mon, 25. Nov 01:50, Michael Niedermayer wrote: > On Sun, Nov 24, 2019 at 11:29:18AM -0500, Andriy Gelman wrote: > > On Sun, 24. Nov 00:02, Michael Niedermayer wrote: > > > On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote: > > > > From: Andriy Gelman <andriy.gelman@gmail.com> > > > > > > > > Fixes #7799 > > > > > > > > Currently, the mp4toannexb filter always inserts the same extradata at > > > > the start of the first IRAP unit. As in ticket #7799, this can lead to > > > > decoding errors if modified parameter sets are signalled in-band. > > > > > > > > Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: > > > > -RAP_B_Bossen_1.bit > > > > -RPS_C_ericsson_5.bit > > > > -SLIST_A_Sony_4.bit > > > > -SLIST_B_Sony_8.bit > > > > -SLIST_C_Sony_3.bit > > > > -SLIST_D_Sony_9.bit > > > > -TSKIP_A_MS_2.bit > > > > > > > > This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets > > > > during the conversion. The correct combination is inserted at the start > > > > of the first IRAP. SEIs from extradata are inserted before each IRAP. > > > > > > > > This commit also makes an update to the hevc-bsf-mp4toannexb fate test > > > > since the result before this patch contained duplicate parameter sets > > > > in-band. > > > > --- > > > > libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- > > > > tests/fate/hevc.mak | 2 +- > > > > 2 files changed, 472 insertions(+), 33 deletions(-) > [...] > > > > > > [...] > > > > +/* > > > > + * Function converts mp4 access unit into annexb > > > > + * Output packet structure > > > > + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] > > > > + * or > > > > + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] > > > > + */ > > > > static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > > > > { > > > > HEVCBSFContext *s = ctx->priv_data; > > > > AVPacket *in; > > > > - GetByteContext gb; > > > > - > > > > - int got_irap = 0; > > > > - int i, ret = 0; > > > > + H2645Packet pkt; > > > > + int i, j, prev_size, ret; > > > > + int irap_done; > > > > > > > > ret = ff_bsf_get_packet(ctx, &in); > > > > if (ret < 0) > > > > return ret; > > > > > > > > + /* output the annexb nalu if extradata is not parsed*/ > > > > if (!s->extradata_parsed) { > > > > av_packet_move_ref(out, in); > > > > av_packet_free(&in); > > > > return 0; > > > > } > > > > > > > > > > > - bytestream2_init(&gb, in->data, in->size); > > > > > > Is this really better without using an existing bytestream* API ? > > > > If I use the API, then I'd have to call bytestraem2_init for each nal unit. I > > also don't use the bytestream2_get_byte function. It seemed simpler to use the WRITE_NAL macro. > > > > But maybe I've misunderstood your question. > > i had nothing really specific in mind, just the feeling that using none of > the existing APIs there is a bit odd. > > but more specifically, what about the write side ? If I use the bytestream* API, then I'd have to add extra boiler plate code each time I resize the output with av_grow_packet(). I'd have to add something like: prev_size = bytestream2_tell_p(...); bytestream2_init_writer(...); bytestream2_skip_p(..., prev_size); Or maybe the API needs an extra function that reinitializes the pointers but keeps the current state of the writer.
On Mon, Nov 25, 2019 at 09:35:04PM -0500, Andriy Gelman wrote: > On Mon, 25. Nov 01:50, Michael Niedermayer wrote: > > On Sun, Nov 24, 2019 at 11:29:18AM -0500, Andriy Gelman wrote: > > > On Sun, 24. Nov 00:02, Michael Niedermayer wrote: > > > > On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote: > > > > > From: Andriy Gelman <andriy.gelman@gmail.com> > > > > > > > > > > Fixes #7799 > > > > > > > > > > Currently, the mp4toannexb filter always inserts the same extradata at > > > > > the start of the first IRAP unit. As in ticket #7799, this can lead to > > > > > decoding errors if modified parameter sets are signalled in-band. > > > > > > > > > > Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: > > > > > -RAP_B_Bossen_1.bit > > > > > -RPS_C_ericsson_5.bit > > > > > -SLIST_A_Sony_4.bit > > > > > -SLIST_B_Sony_8.bit > > > > > -SLIST_C_Sony_3.bit > > > > > -SLIST_D_Sony_9.bit > > > > > -TSKIP_A_MS_2.bit > > > > > > > > > > This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets > > > > > during the conversion. The correct combination is inserted at the start > > > > > of the first IRAP. SEIs from extradata are inserted before each IRAP. > > > > > > > > > > This commit also makes an update to the hevc-bsf-mp4toannexb fate test > > > > > since the result before this patch contained duplicate parameter sets > > > > > in-band. > > > > > --- > > > > > libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- > > > > > tests/fate/hevc.mak | 2 +- > > > > > 2 files changed, 472 insertions(+), 33 deletions(-) > > [...] > > > > > > > > [...] > > > > > +/* > > > > > + * Function converts mp4 access unit into annexb > > > > > + * Output packet structure > > > > > + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] > > > > > + * or > > > > > + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] > > > > > + */ > > > > > static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > > > > > { > > > > > HEVCBSFContext *s = ctx->priv_data; > > > > > AVPacket *in; > > > > > - GetByteContext gb; > > > > > - > > > > > - int got_irap = 0; > > > > > - int i, ret = 0; > > > > > + H2645Packet pkt; > > > > > + int i, j, prev_size, ret; > > > > > + int irap_done; > > > > > > > > > > ret = ff_bsf_get_packet(ctx, &in); > > > > > if (ret < 0) > > > > > return ret; > > > > > > > > > > + /* output the annexb nalu if extradata is not parsed*/ > > > > > if (!s->extradata_parsed) { > > > > > av_packet_move_ref(out, in); > > > > > av_packet_free(&in); > > > > > return 0; > > > > > } > > > > > > > > > > > > > > - bytestream2_init(&gb, in->data, in->size); > > > > > > > > Is this really better without using an existing bytestream* API ? > > > > > > If I use the API, then I'd have to call bytestraem2_init for each nal unit. I > > > also don't use the bytestream2_get_byte function. It seemed simpler to use the WRITE_NAL macro. > > > > > > But maybe I've misunderstood your question. > > > > i had nothing really specific in mind, just the feeling that using none of > > the existing APIs there is a bit odd. > > > > but more specifically, what about the write side ? > > If I use the bytestream* API, then I'd have to add extra boiler plate code each time I resize the output with av_grow_packet(). I'd have to add something like: > > prev_size = bytestream2_tell_p(...); > bytestream2_init_writer(...); > bytestream2_skip_p(..., prev_size); > > Or maybe the API needs an extra function that reinitializes the pointers but > keeps the current state of the writer. grow/realloc seems suboptimal to me for any API. cant you find out how much space is needed and allocate/grow just once then init the bytestream2 on that ? thx [...]
On Tue, 26. Nov 10:52, Michael Niedermayer wrote: > On Mon, Nov 25, 2019 at 09:35:04PM -0500, Andriy Gelman wrote: > > On Mon, 25. Nov 01:50, Michael Niedermayer wrote: > > > On Sun, Nov 24, 2019 at 11:29:18AM -0500, Andriy Gelman wrote: > > > > On Sun, 24. Nov 00:02, Michael Niedermayer wrote: > > > > > On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote: > > > > > > From: Andriy Gelman <andriy.gelman@gmail.com> > > > > > > > > > > > > Fixes #7799 > > > > > > > > > > > > Currently, the mp4toannexb filter always inserts the same extradata at > > > > > > the start of the first IRAP unit. As in ticket #7799, this can lead to > > > > > > decoding errors if modified parameter sets are signalled in-band. > > > > > > > > > > > > Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: > > > > > > -RAP_B_Bossen_1.bit > > > > > > -RPS_C_ericsson_5.bit > > > > > > -SLIST_A_Sony_4.bit > > > > > > -SLIST_B_Sony_8.bit > > > > > > -SLIST_C_Sony_3.bit > > > > > > -SLIST_D_Sony_9.bit > > > > > > -TSKIP_A_MS_2.bit > > > > > > > > > > > > This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets > > > > > > during the conversion. The correct combination is inserted at the start > > > > > > of the first IRAP. SEIs from extradata are inserted before each IRAP. > > > > > > > > > > > > This commit also makes an update to the hevc-bsf-mp4toannexb fate test > > > > > > since the result before this patch contained duplicate parameter sets > > > > > > in-band. > > > > > > --- > > > > > > libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- > > > > > > tests/fate/hevc.mak | 2 +- > > > > > > 2 files changed, 472 insertions(+), 33 deletions(-) > > > [...] > > > > > > > > > > [...] > > > > > > +/* > > > > > > + * Function converts mp4 access unit into annexb > > > > > > + * Output packet structure > > > > > > + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] > > > > > > + * or > > > > > > + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] > > > > > > + */ > > > > > > static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > > > > > > { > > > > > > HEVCBSFContext *s = ctx->priv_data; > > > > > > AVPacket *in; > > > > > > - GetByteContext gb; > > > > > > - > > > > > > - int got_irap = 0; > > > > > > - int i, ret = 0; > > > > > > + H2645Packet pkt; > > > > > > + int i, j, prev_size, ret; > > > > > > + int irap_done; > > > > > > > > > > > > ret = ff_bsf_get_packet(ctx, &in); > > > > > > if (ret < 0) > > > > > > return ret; > > > > > > > > > > > > + /* output the annexb nalu if extradata is not parsed*/ > > > > > > if (!s->extradata_parsed) { > > > > > > av_packet_move_ref(out, in); > > > > > > av_packet_free(&in); > > > > > > return 0; > > > > > > } > > > > > > > > > > > > > > > > > - bytestream2_init(&gb, in->data, in->size); > > > > > > > > > > Is this really better without using an existing bytestream* API ? > > > > > > > > If I use the API, then I'd have to call bytestraem2_init for each nal unit. I > > > > also don't use the bytestream2_get_byte function. It seemed simpler to use the WRITE_NAL macro. > > > > > > > > But maybe I've misunderstood your question. > > > > > > i had nothing really specific in mind, just the feeling that using none of > > > the existing APIs there is a bit odd. > > > > > > but more specifically, what about the write side ? > > > > If I use the bytestream* API, then I'd have to add extra boiler plate code each time I resize the output with av_grow_packet(). I'd have to add something like: > > > > prev_size = bytestream2_tell_p(...); > > bytestream2_init_writer(...); > > bytestream2_skip_p(..., prev_size); > > > > Or maybe the API needs an extra function that reinitializes the pointers but > > keeps the current state of the writer. > > grow/realloc seems suboptimal to me for any API. > cant you find out how much space is needed and allocate/grow just once then > init the bytestream2 on that ? > ok, I'll do it this way. Thanks,
On Tue, 26. Nov 07:24, Andriy Gelman wrote: > On Tue, 26. Nov 10:52, Michael Niedermayer wrote: > > On Mon, Nov 25, 2019 at 09:35:04PM -0500, Andriy Gelman wrote: > > > On Mon, 25. Nov 01:50, Michael Niedermayer wrote: > > > > On Sun, Nov 24, 2019 at 11:29:18AM -0500, Andriy Gelman wrote: > > > > > On Sun, 24. Nov 00:02, Michael Niedermayer wrote: > > > > > > On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote: > > > > > > > From: Andriy Gelman <andriy.gelman@gmail.com> > > > > > > > > > > > > > > Fixes #7799 > > > > > > > > > > > > > > Currently, the mp4toannexb filter always inserts the same extradata at > > > > > > > the start of the first IRAP unit. As in ticket #7799, this can lead to > > > > > > > decoding errors if modified parameter sets are signalled in-band. > > > > > > > > > > > > > > Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: > > > > > > > -RAP_B_Bossen_1.bit > > > > > > > -RPS_C_ericsson_5.bit > > > > > > > -SLIST_A_Sony_4.bit > > > > > > > -SLIST_B_Sony_8.bit > > > > > > > -SLIST_C_Sony_3.bit > > > > > > > -SLIST_D_Sony_9.bit > > > > > > > -TSKIP_A_MS_2.bit > > > > > > > > > > > > > > This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets > > > > > > > during the conversion. The correct combination is inserted at the start > > > > > > > of the first IRAP. SEIs from extradata are inserted before each IRAP. > > > > > > > > > > > > > > This commit also makes an update to the hevc-bsf-mp4toannexb fate test > > > > > > > since the result before this patch contained duplicate parameter sets > > > > > > > in-band. > > > > > > > --- > > > > > > > libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- > > > > > > > tests/fate/hevc.mak | 2 +- > > > > > > > 2 files changed, 472 insertions(+), 33 deletions(-) > > > > [...] > > > > > > > > > > > > [...] > > > > > > > +/* > > > > > > > + * Function converts mp4 access unit into annexb > > > > > > > + * Output packet structure > > > > > > > + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] > > > > > > > + * or > > > > > > > + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] > > > > > > > + */ > > > > > > > static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > > > > > > > { > > > > > > > HEVCBSFContext *s = ctx->priv_data; > > > > > > > AVPacket *in; > > > > > > > - GetByteContext gb; > > > > > > > - > > > > > > > - int got_irap = 0; > > > > > > > - int i, ret = 0; > > > > > > > + H2645Packet pkt; > > > > > > > + int i, j, prev_size, ret; > > > > > > > + int irap_done; > > > > > > > > > > > > > > ret = ff_bsf_get_packet(ctx, &in); > > > > > > > if (ret < 0) > > > > > > > return ret; > > > > > > > > > > > > > > + /* output the annexb nalu if extradata is not parsed*/ > > > > > > > if (!s->extradata_parsed) { > > > > > > > av_packet_move_ref(out, in); > > > > > > > av_packet_free(&in); > > > > > > > return 0; > > > > > > > } > > > > > > > > > > > > > > > > > > > > - bytestream2_init(&gb, in->data, in->size); > > > > > > > > > > > > Is this really better without using an existing bytestream* API ? > > > > > > > > > > If I use the API, then I'd have to call bytestraem2_init for each nal unit. I > > > > > also don't use the bytestream2_get_byte function. It seemed simpler to use the WRITE_NAL macro. > > > > > > > > > > But maybe I've misunderstood your question. > > > > > > > > i had nothing really specific in mind, just the feeling that using none of > > > > the existing APIs there is a bit odd. > > > > > > > > but more specifically, what about the write side ? > > > > > > If I use the bytestream* API, then I'd have to add extra boiler plate code each time I resize the output with av_grow_packet(). I'd have to add something like: > > > > > > prev_size = bytestream2_tell_p(...); > > > bytestream2_init_writer(...); > > > bytestream2_skip_p(..., prev_size); > > > > > > Or maybe the API needs an extra function that reinitializes the pointers but > > > keeps the current state of the writer. > > > > grow/realloc seems suboptimal to me for any API. > > cant you find out how much space is needed and allocate/grow just once then > > init the bytestream2 on that ? > > > > ok, I'll do it this way. > After spending so much time on this patch I've just discovered that the hevc_metadata bsf can do the same job as hevc_mp4toannexb. The former also inserts correct parameter sets and so fixes #7799... :( So instead of: ./ffmpeg -i in.mp4 -bsf:v hevc_mp4toannexb -codec:v copy out.hevc The user can just run: ./ffmpeg -i in.mp4 -bsf:v hevc_metadata -codec:v copy out.hevc One difference is that hevc_metadata currently only keeps the base layers (nuh_layer_id == 0), whereas hevc_mp4toannexb copies everything (before my patch). hevc_metadata will have a slightly higher complexity as it parses the full parameter sets. Can you give me advice on best way to proceed? If the nuh_layer_id issue is solved, can we remove the hevc_mp4toannexb filter and automatically insert hevc_metadata filter if the former is selected by the user? Thanks, -- Andriy
Andriy Gelman: > On Tue, 26. Nov 07:24, Andriy Gelman wrote: >> On Tue, 26. Nov 10:52, Michael Niedermayer wrote: >>> On Mon, Nov 25, 2019 at 09:35:04PM -0500, Andriy Gelman wrote: >>>> On Mon, 25. Nov 01:50, Michael Niedermayer wrote: >>>>> On Sun, Nov 24, 2019 at 11:29:18AM -0500, Andriy Gelman wrote: >>>>>> On Sun, 24. Nov 00:02, Michael Niedermayer wrote: >>>>>>> On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote: >>>>>>>> From: Andriy Gelman <andriy.gelman@gmail.com> >>>>>>>> >>>>>>>> Fixes #7799 >>>>>>>> >>>>>>>> Currently, the mp4toannexb filter always inserts the same extradata at >>>>>>>> the start of the first IRAP unit. As in ticket #7799, this can lead to >>>>>>>> decoding errors if modified parameter sets are signalled in-band. >>>>>>>> >>>>>>>> Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: >>>>>>>> -RAP_B_Bossen_1.bit >>>>>>>> -RPS_C_ericsson_5.bit >>>>>>>> -SLIST_A_Sony_4.bit >>>>>>>> -SLIST_B_Sony_8.bit >>>>>>>> -SLIST_C_Sony_3.bit >>>>>>>> -SLIST_D_Sony_9.bit >>>>>>>> -TSKIP_A_MS_2.bit >>>>>>>> >>>>>>>> This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets >>>>>>>> during the conversion. The correct combination is inserted at the start >>>>>>>> of the first IRAP. SEIs from extradata are inserted before each IRAP. >>>>>>>> >>>>>>>> This commit also makes an update to the hevc-bsf-mp4toannexb fate test >>>>>>>> since the result before this patch contained duplicate parameter sets >>>>>>>> in-band. >>>>>>>> --- >>>>>>>> libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- >>>>>>>> tests/fate/hevc.mak | 2 +- >>>>>>>> 2 files changed, 472 insertions(+), 33 deletions(-) >>>>> [...] >>>>>>> >>>>>>> [...] >>>>>>>> +/* >>>>>>>> + * Function converts mp4 access unit into annexb >>>>>>>> + * Output packet structure >>>>>>>> + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] >>>>>>>> + * or >>>>>>>> + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] >>>>>>>> + */ >>>>>>>> static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) >>>>>>>> { >>>>>>>> HEVCBSFContext *s = ctx->priv_data; >>>>>>>> AVPacket *in; >>>>>>>> - GetByteContext gb; >>>>>>>> - >>>>>>>> - int got_irap = 0; >>>>>>>> - int i, ret = 0; >>>>>>>> + H2645Packet pkt; >>>>>>>> + int i, j, prev_size, ret; >>>>>>>> + int irap_done; >>>>>>>> >>>>>>>> ret = ff_bsf_get_packet(ctx, &in); >>>>>>>> if (ret < 0) >>>>>>>> return ret; >>>>>>>> >>>>>>>> + /* output the annexb nalu if extradata is not parsed*/ >>>>>>>> if (!s->extradata_parsed) { >>>>>>>> av_packet_move_ref(out, in); >>>>>>>> av_packet_free(&in); >>>>>>>> return 0; >>>>>>>> } >>>>>>>> >>>>>>> >>>>>>>> - bytestream2_init(&gb, in->data, in->size); >>>>>>> >>>>>>> Is this really better without using an existing bytestream* API ? >>>>>> >>>>>> If I use the API, then I'd have to call bytestraem2_init for each nal unit. I >>>>>> also don't use the bytestream2_get_byte function. It seemed simpler to use the WRITE_NAL macro. >>>>>> >>>>>> But maybe I've misunderstood your question. >>>>> >>>>> i had nothing really specific in mind, just the feeling that using none of >>>>> the existing APIs there is a bit odd. >>>>> >>>>> but more specifically, what about the write side ? >>>> >>>> If I use the bytestream* API, then I'd have to add extra boiler plate code each time I resize the output with av_grow_packet(). I'd have to add something like: >>>> >>>> prev_size = bytestream2_tell_p(...); >>>> bytestream2_init_writer(...); >>>> bytestream2_skip_p(..., prev_size); >>>> >>>> Or maybe the API needs an extra function that reinitializes the pointers but >>>> keeps the current state of the writer. >>> >>> grow/realloc seems suboptimal to me for any API. >>> cant you find out how much space is needed and allocate/grow just once then >>> init the bytestream2 on that ? >>> >> >> ok, I'll do it this way. >> > > After spending so much time on this patch I've just discovered that the > hevc_metadata bsf can do the same job as hevc_mp4toannexb. The former also > inserts correct parameter sets and so fixes #7799... :( > > So instead of: > ./ffmpeg -i in.mp4 -bsf:v hevc_mp4toannexb -codec:v copy out.hevc > > The user can just run: > ./ffmpeg -i in.mp4 -bsf:v hevc_metadata -codec:v copy out.hevc > > One difference is that hevc_metadata currently only keeps the base layers > (nuh_layer_id == 0), whereas hevc_mp4toannexb copies everything (before my > patch). hevc_metadata will have a slightly higher complexity as it parses the full > parameter sets. > > Can you give me advice on best way to proceed? > > If the nuh_layer_id issue is solved, can we remove the hevc_mp4toannexb filter > and automatically insert hevc_metadata filter if the former is selected by > the user? > Hello Andriy, 1. hevc_metadata inserts parameter sets? That is absolutely new to me. How did you check this? (It does not insert parameter sets, but it outputs annex b, so the sample from #7799 is left alone by hevc_mp4toannexb after hevc_metadata, hence the updated in-band extradata is not masked by non-updated header parameter sets). 2. hevc_metadata is based upon cbs and cbs for H.264 and H.265 uses the h2645_parse functions and the nuh_layer_id issue affects it, too. 3. cbs is (currently) slow. That is because it lets the h2645_parse functions unescape the whole units and the reassembles them at the end. Even though I have an idea for a patch that would greatly speed this up, it will never be as fast as an approach that does not rely on cbs. 4. Furthermore, cbs is very picky and drops whole packets because of minor errors. Not what you would want in an auto-inserted bsf. 5. Sorry for not reviewing your patch any further. - Andreas
On Thu, 28. Nov 15:28, Andreas Rheinhardt wrote: > Andriy Gelman: > > On Tue, 26. Nov 07:24, Andriy Gelman wrote: > >> On Tue, 26. Nov 10:52, Michael Niedermayer wrote: > >>> On Mon, Nov 25, 2019 at 09:35:04PM -0500, Andriy Gelman wrote: > >>>> On Mon, 25. Nov 01:50, Michael Niedermayer wrote: > >>>>> On Sun, Nov 24, 2019 at 11:29:18AM -0500, Andriy Gelman wrote: > >>>>>> On Sun, 24. Nov 00:02, Michael Niedermayer wrote: > >>>>>>> On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote: > >>>>>>>> From: Andriy Gelman <andriy.gelman@gmail.com> > >>>>>>>> > >>>>>>>> Fixes #7799 > >>>>>>>> > >>>>>>>> Currently, the mp4toannexb filter always inserts the same extradata at > >>>>>>>> the start of the first IRAP unit. As in ticket #7799, this can lead to > >>>>>>>> decoding errors if modified parameter sets are signalled in-band. > >>>>>>>> > >>>>>>>> Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: > >>>>>>>> -RAP_B_Bossen_1.bit > >>>>>>>> -RPS_C_ericsson_5.bit > >>>>>>>> -SLIST_A_Sony_4.bit > >>>>>>>> -SLIST_B_Sony_8.bit > >>>>>>>> -SLIST_C_Sony_3.bit > >>>>>>>> -SLIST_D_Sony_9.bit > >>>>>>>> -TSKIP_A_MS_2.bit > >>>>>>>> > >>>>>>>> This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets > >>>>>>>> during the conversion. The correct combination is inserted at the start > >>>>>>>> of the first IRAP. SEIs from extradata are inserted before each IRAP. > >>>>>>>> > >>>>>>>> This commit also makes an update to the hevc-bsf-mp4toannexb fate test > >>>>>>>> since the result before this patch contained duplicate parameter sets > >>>>>>>> in-band. > >>>>>>>> --- > >>>>>>>> libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- > >>>>>>>> tests/fate/hevc.mak | 2 +- > >>>>>>>> 2 files changed, 472 insertions(+), 33 deletions(-) > >>>>> [...] > >>>>>>> > >>>>>>> [...] > >>>>>>>> +/* > >>>>>>>> + * Function converts mp4 access unit into annexb > >>>>>>>> + * Output packet structure > >>>>>>>> + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] > >>>>>>>> + * or > >>>>>>>> + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] > >>>>>>>> + */ > >>>>>>>> static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) > >>>>>>>> { > >>>>>>>> HEVCBSFContext *s = ctx->priv_data; > >>>>>>>> AVPacket *in; > >>>>>>>> - GetByteContext gb; > >>>>>>>> - > >>>>>>>> - int got_irap = 0; > >>>>>>>> - int i, ret = 0; > >>>>>>>> + H2645Packet pkt; > >>>>>>>> + int i, j, prev_size, ret; > >>>>>>>> + int irap_done; > >>>>>>>> > >>>>>>>> ret = ff_bsf_get_packet(ctx, &in); > >>>>>>>> if (ret < 0) > >>>>>>>> return ret; > >>>>>>>> > >>>>>>>> + /* output the annexb nalu if extradata is not parsed*/ > >>>>>>>> if (!s->extradata_parsed) { > >>>>>>>> av_packet_move_ref(out, in); > >>>>>>>> av_packet_free(&in); > >>>>>>>> return 0; > >>>>>>>> } > >>>>>>>> > >>>>>>> > >>>>>>>> - bytestream2_init(&gb, in->data, in->size); > >>>>>>> > >>>>>>> Is this really better without using an existing bytestream* API ? > >>>>>> > >>>>>> If I use the API, then I'd have to call bytestraem2_init for each nal unit. I > >>>>>> also don't use the bytestream2_get_byte function. It seemed simpler to use the WRITE_NAL macro. > >>>>>> > >>>>>> But maybe I've misunderstood your question. > >>>>> > >>>>> i had nothing really specific in mind, just the feeling that using none of > >>>>> the existing APIs there is a bit odd. > >>>>> > >>>>> but more specifically, what about the write side ? > >>>> > >>>> If I use the bytestream* API, then I'd have to add extra boiler plate code each time I resize the output with av_grow_packet(). I'd have to add something like: > >>>> > >>>> prev_size = bytestream2_tell_p(...); > >>>> bytestream2_init_writer(...); > >>>> bytestream2_skip_p(..., prev_size); > >>>> > >>>> Or maybe the API needs an extra function that reinitializes the pointers but > >>>> keeps the current state of the writer. > >>> > >>> grow/realloc seems suboptimal to me for any API. > >>> cant you find out how much space is needed and allocate/grow just once then > >>> init the bytestream2 on that ? > >>> > >> > >> ok, I'll do it this way. > >> > > > > After spending so much time on this patch I've just discovered that the > > hevc_metadata bsf can do the same job as hevc_mp4toannexb. The former also > > inserts correct parameter sets and so fixes #7799... :( > > > > So instead of: > > ./ffmpeg -i in.mp4 -bsf:v hevc_mp4toannexb -codec:v copy out.hevc > > > > The user can just run: > > ./ffmpeg -i in.mp4 -bsf:v hevc_metadata -codec:v copy out.hevc > > > > One difference is that hevc_metadata currently only keeps the base layers > > (nuh_layer_id == 0), whereas hevc_mp4toannexb copies everything (before my > > patch). hevc_metadata will have a slightly higher complexity as it parses the full > > parameter sets. > > > > Can you give me advice on best way to proceed? > > > > If the nuh_layer_id issue is solved, can we remove the hevc_mp4toannexb filter > > and automatically insert hevc_metadata filter if the former is selected by > > the user? > > > Hello Andriy, > > 1. hevc_metadata inserts parameter sets? That is absolutely new to me. > How did you check this? (It does not insert parameter sets, but it > outputs annex b, so the sample from #7799 is left alone by > hevc_mp4toannexb after hevc_metadata, hence the updated in-band > extradata is not masked by non-updated header parameter sets). You are right, it doesn't insert the referenced parameter sets, but just doesn't overwrite them by the inserting the old extradata. I suppose if the media is spliced, then my patch would be still be better than hevc_metadata because all the parameter are guaranteed to be in the cvs. But there's still overlap imo. > 2. hevc_metadata is based upon cbs and cbs for H.264 and H.265 uses > the h2645_parse functions and the nuh_layer_id issue affects it, too. > 3. cbs is (currently) slow. That is because it lets the h2645_parse > functions unescape the whole units and the reassembles them at the > end. Even though I have an idea for a patch that would greatly speed > this up, it will never be as fast as an approach that does not rely on > cbs. I'm also using ff_h2645_packet_split in my patch, too. It gives the rbsp on all the NALs. I think it's possible to set some kind of a bound where only the first x bytes are needed. > 4. Furthermore, cbs is very picky and drops whole packets because of > minor errors. Not what you would want in an auto-inserted bsf. I've only glanced over cbs so can't really comment. Thanks,
Andriy Gelman: > On Thu, 28. Nov 15:28, Andreas Rheinhardt wrote: >> Andriy Gelman: >>> On Tue, 26. Nov 07:24, Andriy Gelman wrote: >>>> On Tue, 26. Nov 10:52, Michael Niedermayer wrote: >>>>> On Mon, Nov 25, 2019 at 09:35:04PM -0500, Andriy Gelman wrote: >>>>>> On Mon, 25. Nov 01:50, Michael Niedermayer wrote: >>>>>>> On Sun, Nov 24, 2019 at 11:29:18AM -0500, Andriy Gelman wrote: >>>>>>>> On Sun, 24. Nov 00:02, Michael Niedermayer wrote: >>>>>>>>> On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote: >>>>>>>>>> From: Andriy Gelman <andriy.gelman@gmail.com> >>>>>>>>>> >>>>>>>>>> Fixes #7799 >>>>>>>>>> >>>>>>>>>> Currently, the mp4toannexb filter always inserts the same extradata at >>>>>>>>>> the start of the first IRAP unit. As in ticket #7799, this can lead to >>>>>>>>>> decoding errors if modified parameter sets are signalled in-band. >>>>>>>>>> >>>>>>>>>> Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: >>>>>>>>>> -RAP_B_Bossen_1.bit >>>>>>>>>> -RPS_C_ericsson_5.bit >>>>>>>>>> -SLIST_A_Sony_4.bit >>>>>>>>>> -SLIST_B_Sony_8.bit >>>>>>>>>> -SLIST_C_Sony_3.bit >>>>>>>>>> -SLIST_D_Sony_9.bit >>>>>>>>>> -TSKIP_A_MS_2.bit >>>>>>>>>> >>>>>>>>>> This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets >>>>>>>>>> during the conversion. The correct combination is inserted at the start >>>>>>>>>> of the first IRAP. SEIs from extradata are inserted before each IRAP. >>>>>>>>>> >>>>>>>>>> This commit also makes an update to the hevc-bsf-mp4toannexb fate test >>>>>>>>>> since the result before this patch contained duplicate parameter sets >>>>>>>>>> in-band. >>>>>>>>>> --- >>>>>>>>>> libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- >>>>>>>>>> tests/fate/hevc.mak | 2 +- >>>>>>>>>> 2 files changed, 472 insertions(+), 33 deletions(-) >>>>>>> [...] >>>>>>>>> >>>>>>>>> [...] >>>>>>>>>> +/* >>>>>>>>>> + * Function converts mp4 access unit into annexb >>>>>>>>>> + * Output packet structure >>>>>>>>>> + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] >>>>>>>>>> + * or >>>>>>>>>> + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] >>>>>>>>>> + */ >>>>>>>>>> static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) >>>>>>>>>> { >>>>>>>>>> HEVCBSFContext *s = ctx->priv_data; >>>>>>>>>> AVPacket *in; >>>>>>>>>> - GetByteContext gb; >>>>>>>>>> - >>>>>>>>>> - int got_irap = 0; >>>>>>>>>> - int i, ret = 0; >>>>>>>>>> + H2645Packet pkt; >>>>>>>>>> + int i, j, prev_size, ret; >>>>>>>>>> + int irap_done; >>>>>>>>>> >>>>>>>>>> ret = ff_bsf_get_packet(ctx, &in); >>>>>>>>>> if (ret < 0) >>>>>>>>>> return ret; >>>>>>>>>> >>>>>>>>>> + /* output the annexb nalu if extradata is not parsed*/ >>>>>>>>>> if (!s->extradata_parsed) { >>>>>>>>>> av_packet_move_ref(out, in); >>>>>>>>>> av_packet_free(&in); >>>>>>>>>> return 0; >>>>>>>>>> } >>>>>>>>>> >>>>>>>>> >>>>>>>>>> - bytestream2_init(&gb, in->data, in->size); >>>>>>>>> >>>>>>>>> Is this really better without using an existing bytestream* API ? >>>>>>>> >>>>>>>> If I use the API, then I'd have to call bytestraem2_init for each nal unit. I >>>>>>>> also don't use the bytestream2_get_byte function. It seemed simpler to use the WRITE_NAL macro. >>>>>>>> >>>>>>>> But maybe I've misunderstood your question. >>>>>>> >>>>>>> i had nothing really specific in mind, just the feeling that using none of >>>>>>> the existing APIs there is a bit odd. >>>>>>> >>>>>>> but more specifically, what about the write side ? >>>>>> >>>>>> If I use the bytestream* API, then I'd have to add extra boiler plate code each time I resize the output with av_grow_packet(). I'd have to add something like: >>>>>> >>>>>> prev_size = bytestream2_tell_p(...); >>>>>> bytestream2_init_writer(...); >>>>>> bytestream2_skip_p(..., prev_size); >>>>>> >>>>>> Or maybe the API needs an extra function that reinitializes the pointers but >>>>>> keeps the current state of the writer. >>>>> >>>>> grow/realloc seems suboptimal to me for any API. >>>>> cant you find out how much space is needed and allocate/grow just once then >>>>> init the bytestream2 on that ? >>>>> >>>> >>>> ok, I'll do it this way. >>>> >>> >>> After spending so much time on this patch I've just discovered that the >>> hevc_metadata bsf can do the same job as hevc_mp4toannexb. The former also >>> inserts correct parameter sets and so fixes #7799... :( >>> >>> So instead of: >>> ./ffmpeg -i in.mp4 -bsf:v hevc_mp4toannexb -codec:v copy out.hevc >>> >>> The user can just run: >>> ./ffmpeg -i in.mp4 -bsf:v hevc_metadata -codec:v copy out.hevc >>> >>> One difference is that hevc_metadata currently only keeps the base layers >>> (nuh_layer_id == 0), whereas hevc_mp4toannexb copies everything (before my >>> patch). hevc_metadata will have a slightly higher complexity as it parses the full >>> parameter sets. >>> >>> Can you give me advice on best way to proceed? >>> >>> If the nuh_layer_id issue is solved, can we remove the hevc_mp4toannexb filter >>> and automatically insert hevc_metadata filter if the former is selected by >>> the user? >>> >> Hello Andriy, >> >> 1. hevc_metadata inserts parameter sets? That is absolutely new to me. >> How did you check this? (It does not insert parameter sets, but it >> outputs annex b, so the sample from #7799 is left alone by >> hevc_mp4toannexb after hevc_metadata, hence the updated in-band >> extradata is not masked by non-updated header parameter sets). > > You are right, it doesn't insert the referenced parameter sets, > but just doesn't overwrite them by the inserting the old extradata. > > I suppose if the media is spliced, then my patch would be still be better > than hevc_metadata because all the parameter are guaranteed to be in the cvs. > But there's still overlap imo. > There is overlap in that both output annex b and that cbs could be used for your purpose if you don't care about speed, but that's about it. E.g. if you convert a mp4 HEVC (or H.264 for that matter) without in-band parameter sets to annex b via hevc_metadata/h264_metadata (or filter_units) and write the output to an elementary stream, it will not have any parameter sets and be unplayable. (I consider this a bug in the elementary stream muxer. But low priority.) >> 2. hevc_metadata is based upon cbs and cbs for H.264 and H.265 uses >> the h2645_parse functions and the nuh_layer_id issue affects it, too. > >> 3. cbs is (currently) slow. That is because it lets the h2645_parse >> functions unescape the whole units and the reassembles them at the >> end. Even though I have an idea for a patch that would greatly speed >> this up, it will never be as fast as an approach that does not rely on >> cbs. > > I'm also using ff_h2645_packet_split in my patch, too. It gives the rbsp on all the > NALs. I think it's possible to set some kind of a bound where only the first x bytes are needed. > Yes, I have been thinking about such a thing, too. One could avoid this by not calling ff_h2645_packet_split(), but instead calling ff_h2645_extract_rbsp() with an artificially small length (depending upon the type of the unit and how far away the information you want may be). (This means that you have to parse the sizes of the NAL units yourself, but them being length-prefixed means that this is trivial.) - Andreas
diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c index 09bce5b34c..1ca5f13807 100644 --- a/libavcodec/hevc_mp4toannexb_bsf.c +++ b/libavcodec/hevc_mp4toannexb_bsf.c @@ -23,19 +23,224 @@ #include "libavutil/intreadwrite.h" #include "libavutil/mem.h" +#include "libavutil/avassert.h" #include "avcodec.h" #include "bsf.h" #include "bytestream.h" #include "hevc.h" +#include "h2645_parse.h" +#include "hevc_ps.h" +#include "golomb.h" #define MIN_HEVCC_LENGTH 23 +#define PROFILE_WITHOUT_IDC_BITS 88 +#define IS_IRAP(s) ((s)->type >= 16 && (s)->type <= 23) +#define IS_PARAMSET(s) ((s)->type >= 32 && (s)->type <= 34) + + /*reserved VCLs not included*/ +#define IS_VCL(s) ((s)->type <= 9 || ((s)->type >= 16 && (s)->type <= 21)) +#define HEVC_NAL_HEADER_BITS 16 + +/* + * Copies data from input buffer to output buffer. Appends annexb startcode. + * out must be allocated at least out_len + in_len + write_startcode * 4. + */ +#define WRITE_NAL(out, out_len, in, in_len, write_startcode) do { \ + if ((write_startcode)) { \ + AV_WB32((out) + (out_len), 1); \ + (out_len) += 4; \ + } \ + memcpy((out) + (out_len), (in), (in_len)); \ + (out_len) += (in_len); \ +} while (0) + +typedef struct Param { + uint8_t *raw_data; /* raw data to store param set payload*/ + int raw_size; /* size of raw_data*/ + size_t allocated_size; /* allocated size of raw_data*/ + int ref; /* stores the ref of the higher level parameter set*/ + int is_signalled; /* indicates whether this param has already been signalled in the cvs*/ +} Param; + +/* modified version of HEVCParamSets to store bytestream and reference to previous level*/ +typedef struct ParamSets { + Param vps_list[HEVC_MAX_VPS_COUNT]; + Param sps_list[HEVC_MAX_SPS_COUNT]; + Param pps_list[HEVC_MAX_PPS_COUNT]; + + Param sei; /* cached SEIs from extradatat*/ +} ParamSets; typedef struct HEVCBSFContext { uint8_t length_size; int extradata_parsed; + ParamSets ps; /* cached VPS/SPS/PPS parameter sets + SEI from extradata*/ } HEVCBSFContext; +static int update_cached_paramset(AVBSFContext *ctx, Param *cached_param, + H2645NAL *nal, int ref) +{ + int ret; + + if (cached_param->raw_data && cached_param->raw_size == nal->raw_size + 4 && + !memcmp(cached_param->raw_data + 4, nal->raw_data, nal->raw_size)) { + av_log(ctx, AV_LOG_DEBUG, "NAL unit: %d. Copy already exists in parameter set.\n", nal->type); + } else { + if (nal->raw_size + 4 > cached_param->allocated_size) { + ret = av_reallocp(&cached_param->raw_data, nal->raw_size + 4); + if (ret < 0) + return ret; + cached_param->allocated_size = nal->raw_size + 4; + } + cached_param->raw_size = 0; + WRITE_NAL(cached_param->raw_data, cached_param->raw_size, nal->raw_data, nal->raw_size, 1); + cached_param->ref = ref; + cached_param->is_signalled = 0; + av_assert1(cached_param->raw_size == nal->raw_size + 4); + } + return 0; +} + +static int parse_vps(AVBSFContext *ctx, H2645NAL *nal) +{ + int vps_id, ret; + + HEVCBSFContext *s = ctx->priv_data; + ParamSets *ps = &s->ps; + + GetBitContext *gb = &nal->gb; + + vps_id = get_bits(gb, 4); + + if (get_bits_left(gb) < 0) { + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in VPS id: %d\n", vps_id); + return AVERROR_INVALIDDATA; + } + + av_log(ctx, AV_LOG_TRACE, "Updating VPS id: %d\n", vps_id); + ret = update_cached_paramset(ctx, &ps->vps_list[vps_id], nal, 0); + return ret; +} + +static int parse_sps(AVBSFContext *ctx, H2645NAL *nal) +{ + int i, ret; + int sps_id, vps_ref, max_sub_layers_minus1; + + HEVCBSFContext *s = ctx->priv_data; + ParamSets *ps = &s->ps; + + uint8_t sub_layer_profile_present_flag[HEVC_MAX_SUB_LAYERS]; + uint8_t sub_layer_level_present_flag[HEVC_MAX_SUB_LAYERS]; + + GetBitContext *gb = &nal->gb; + + vps_ref = get_bits(gb, 4); + + max_sub_layers_minus1 = get_bits(gb, 3); + skip_bits1(gb); /* sps_temporal_id_nesting_flag*/ + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ + skip_bits(gb, 8); /* general_level_idc*/ + + for (i = 0; i < max_sub_layers_minus1; i++) { + sub_layer_profile_present_flag[i] = get_bits1(gb); + sub_layer_level_present_flag[i] = get_bits1(gb); + } + + if (max_sub_layers_minus1 > 0) + for (i = max_sub_layers_minus1; i < 8; i++) + skip_bits(gb, 2); /* reserved_zero_2bits[i]*/ + + for (i = 0; i < max_sub_layers_minus1; i++) { + if (sub_layer_profile_present_flag[i]) + skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /* profile_tier_level*/ + if (sub_layer_level_present_flag[i]) + skip_bits(gb, 8); /* sub_layer_level_idc*/ + } + + /*we only need the sps_id index*/ + sps_id = get_ue_golomb(gb); + if (sps_id < 0 || sps_id >= HEVC_MAX_SPS_COUNT) { + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_id); + return AVERROR_INVALIDDATA; + } + + if (get_bits_left(gb) < 0) { + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in SPS id: %d\n", sps_id); + return AVERROR_INVALIDDATA; + } + + av_log(ctx, AV_LOG_TRACE, "Updating SPS id: %d, VPS ref: %d\n", sps_id, vps_ref); + ret = update_cached_paramset(ctx, &ps->sps_list[sps_id], nal, vps_ref); + return ret; +} + +static int parse_pps(AVBSFContext *ctx, H2645NAL *nal) +{ + int ret; + int pps_id, sps_ref; + + HEVCBSFContext *s = ctx->priv_data; + ParamSets *ps = &s->ps; + + GetBitContext *gb = &nal->gb; + + pps_id = get_ue_golomb(gb); + if (pps_id < 0 || pps_id >= HEVC_MAX_PPS_COUNT) { + av_log(ctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id); + return AVERROR_INVALIDDATA; + } + + sps_ref = get_ue_golomb(gb); + if (sps_ref < 0 || sps_ref >= HEVC_MAX_SPS_COUNT) { + av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_ref); + return AVERROR_INVALIDDATA; + } + + if (get_bits_left(gb) < 0) { + av_log(ctx, AV_LOG_ERROR, "Over-read bitstream in PPS id: %d\n", pps_id); + return AVERROR_INVALIDDATA; + } + + av_log(ctx, AV_LOG_TRACE, "Updating PPS id: %d, SPS ref: %d\n", pps_id, sps_ref); + ret = update_cached_paramset(ctx, &ps->pps_list[pps_id], nal, sps_ref); + return ret; +} + +static int append_sei_annexb(Param *sei, H2645NAL *nal) +{ + int ret; + + ret = av_reallocp(&sei->raw_data, sei->allocated_size + nal->raw_size + 4); + if (ret < 0) + return ret; + sei->allocated_size += nal->raw_size + 4; + + WRITE_NAL(sei->raw_data, sei->raw_size, nal->raw_data, nal->raw_size, 1); + av_assert1(sei->raw_size == sei->allocated_size); + return 0; +} + +static int update_paramset(AVBSFContext *ctx, H2645NAL *nal) +{ + int ret; + switch (nal->type) { + case (HEVC_NAL_VPS): + if ((ret = parse_vps(ctx, nal)) < 0) + return ret; + break; + case (HEVC_NAL_SPS): + if ((ret = parse_sps(ctx, nal)) < 0) + return ret; + break; + case (HEVC_NAL_PPS): + if ((ret = parse_pps(ctx, nal)) < 0) + return ret; + } + return 0; +} + static int hevc_extradata_to_annexb(AVBSFContext *ctx) { GetByteContext gb; @@ -97,6 +302,7 @@ fail: static int hevc_mp4toannexb_init(AVBSFContext *ctx) { HEVCBSFContext *s = ctx->priv_data; + H2645Packet pkt; int ret; if (ctx->par_in->extradata_size < MIN_HEVCC_LENGTH || @@ -104,77 +310,309 @@ static int hevc_mp4toannexb_init(AVBSFContext *ctx) AV_RB32(ctx->par_in->extradata) == 1) { av_log(ctx, AV_LOG_VERBOSE, "The input looks like it is Annex B already\n"); + return 0; } else { ret = hevc_extradata_to_annexb(ctx); if (ret < 0) return ret; s->length_size = ret; s->extradata_parsed = 1; + + memset(&pkt, 0, sizeof(H2645Packet)); + ret = ff_h2645_packet_split(&pkt, ctx->par_out->extradata, ctx->par_out->extradata_size, + ctx, 0, 0, AV_CODEC_ID_HEVC, 1, 0); + if (ret < 0) + goto done; + + for (int i = 0; i < pkt.nb_nals; ++i) { + H2645NAL *nal = &pkt.nals[i]; + + /*current segmentation algorithm includes next 0x00 from next nal unit*/ + if (nal->raw_data[nal->raw_size - 1] == 0x00) + nal->raw_size--; + + if (IS_PARAMSET(nal)) { + ret = update_paramset(ctx, nal); + if (ret < 0) + goto done; + continue; + } + + if (nal->type == HEVC_NAL_SEI_PREFIX || nal->type == HEVC_NAL_SEI_SUFFIX) { + ret = append_sei_annexb(&s->ps.sei, nal); + if (ret < 0) + goto done; + } + } } +done: + ff_h2645_packet_uninit(&pkt); + return ret; +} + +static void ps_uninit(ParamSets *ps) +{ + int i; + for (i = 0; i < FF_ARRAY_ELEMS(ps->vps_list); i++) { + av_freep(&ps->vps_list[i].raw_data); + ps->vps_list[i].allocated_size = 0; + ps->vps_list[i].raw_size = 0; + } + for (i = 0; i < FF_ARRAY_ELEMS(ps->sps_list); i++) { + av_freep(&ps->sps_list[i].raw_data); + ps->sps_list[i].allocated_size = 0; + ps->sps_list[i].raw_size = 0; + } + for (i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++) { + av_freep(&ps->pps_list[i].raw_data); + ps->pps_list[i].allocated_size = 0; + ps->pps_list[i].raw_size = 0; + } + av_freep(&ps->sei); + ps->sei.allocated_size = 0; + ps->sei.raw_size = 0; +} + +static void hevc_mp4toannexb_close(AVBSFContext *ctx) +{ + HEVCBSFContext *s = ctx->priv_data; + ps_uninit(&s->ps); +} + +static int get_vcl_pps_id(AVBSFContext *ctx, H2645NAL *nal) +{ + GetBitContext *gb = &nal->gb; + int pps_id, ret; + + /* re-initialize because bitstream touched in write_extradata*/ + ret = init_get_bits(gb, nal->data, nal->size_bits); + if (ret < 0) + return ret; + skip_bits(gb, HEVC_NAL_HEADER_BITS); + + skip_bits1(gb); /* first_slice_segment_in_pic_flag*/ + + if (IS_IRAP(nal)) + skip_bits1(gb); /* no_output_of_prior_pics_flag*/ + + pps_id = get_ue_golomb(gb); + if (pps_id < 0 || pps_id >= HEVC_MAX_PPS_COUNT) { + av_log(ctx, AV_LOG_ERROR, "Invalid PPS id: %d\n", pps_id); + return AVERROR_INVALIDDATA; + } + return pps_id; +} + +static int write_vcl(AVBSFContext *ctx, AVPacket *pkt_out, int *prev_size, + H2645NAL *nal) +{ + HEVCBSFContext *s = ctx->priv_data; + ParamSets *ps = &s->ps; + int ret, pps_id; + Param *pps; + + /* get frame pps*/ + pps_id = get_vcl_pps_id(ctx, nal); + if (pps_id < 0) + return AVERROR_INVALIDDATA; + av_log(ctx, AV_LOG_TRACE, "VCL PPS id: %d\n", pps_id); + + /* write pps if not signalled*/ + pps = &ps->pps_list[pps_id]; + if (!pps->raw_data) { + av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d doesn't exist\n", pps_id); + return AVERROR_INVALIDDATA; + } + + if (!pps->is_signalled) { + av_log(ctx, AV_LOG_TRACE, "Referenced PPS: %d has not been signalled. Writing PPS to output stream\n", pps_id); + ret = av_grow_packet(pkt_out, pps->raw_size); + if (ret < 0) + return ret; + WRITE_NAL(pkt_out->data, *prev_size, pps->raw_data, pps->raw_size, 0); + pps->is_signalled = 1; + } + + /* write actual packet*/ + ret = av_grow_packet(pkt_out, nal->raw_size + 4); + if (ret < 0) + return ret; + WRITE_NAL(pkt_out->data, *prev_size, nal->raw_data, nal->raw_size, 1); return 0; } +static int write_extradata(AVBSFContext *ctx, AVPacket *pkt_out, int *prev_size, + H2645NAL *nal_irap) +{ + int ref, ret; + int new_extradata_size = 0; + + HEVCBSFContext *s = ctx->priv_data; + ParamSets *ps = &s->ps; + GetBitContext *gb = &nal_irap->gb; + + /* active parameter sets for the new irap*/ + const Param *vps; + const Param *sps; + Param *pps; /* non-const because we have to update flag that pps_id is signalled in cvs*/ + + skip_bits1(gb); /* first_slice_ic_pic_flag*/ + skip_bits1(gb); /* no_output_of_prior_pics_flag*/ + + ref = get_ue_golomb(gb); + if (ref < 0 || ref >= HEVC_MAX_PPS_COUNT || !ps->pps_list[ref].raw_data) { + av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d\n", ref); + return AVERROR_INVALIDDATA; + } + av_log(ctx, AV_LOG_TRACE, "Writing PPS id: %d\n", ref); + pps = &ps->pps_list[ref]; + new_extradata_size += pps->raw_size; + ref = pps->ref; + + if (!ps->sps_list[ref].raw_data) { + av_log(ctx, AV_LOG_ERROR, "Invalid SPS: %d\n", ref); + return AVERROR_INVALIDDATA; + } + av_log(ctx, AV_LOG_TRACE, "Writing SPS id: %d\n", ref); + sps = &ps->sps_list[ref]; + new_extradata_size += sps->raw_size; + ref = sps->ref; + + if (!ps->vps_list[ref].raw_data) { + av_log(ctx, AV_LOG_ERROR, "Invalid VPS: %d\n", ref); + return AVERROR_INVALIDDATA; + } + av_log(ctx, AV_LOG_TRACE, "Writing VPS id: %d\n", ref); + vps = &ps->vps_list[ref]; + new_extradata_size += vps->raw_size; + + if (ps->sei.raw_data) + new_extradata_size += ps->sei.raw_size; + + ret = av_grow_packet(pkt_out, new_extradata_size); + if (ret < 0) + return ret; + + WRITE_NAL(pkt_out->data, *prev_size, vps->raw_data, vps->raw_size, 0); + WRITE_NAL(pkt_out->data, *prev_size, sps->raw_data, sps->raw_size, 0); + WRITE_NAL(pkt_out->data, *prev_size, pps->raw_data, pps->raw_size, 0); + pps->is_signalled = 1; + + if (ps->sei.raw_data) + WRITE_NAL(pkt_out->data, *prev_size, ps->sei.raw_data, ps->sei.raw_size, 0); + + av_assert1(*prev_size == pkt_out->size); + return 0; +} + +/* + * Function converts mp4 access unit into annexb + * Output packet structure + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX] + * or + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX] + */ static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) { HEVCBSFContext *s = ctx->priv_data; AVPacket *in; - GetByteContext gb; - - int got_irap = 0; - int i, ret = 0; + H2645Packet pkt; + int i, j, prev_size, ret; + int irap_done; ret = ff_bsf_get_packet(ctx, &in); if (ret < 0) return ret; + /* output the annexb nalu if extradata is not parsed*/ if (!s->extradata_parsed) { av_packet_move_ref(out, in); av_packet_free(&in); return 0; } - bytestream2_init(&gb, in->data, in->size); + memset(&pkt, 0, sizeof(H2645Packet)); + ret = ff_h2645_packet_split(&pkt, in->data, in->size, ctx, 1, s->length_size, AV_CODEC_ID_HEVC, 1, 0); + if (ret < 0) + goto done; + + irap_done = 1; /* 1 means that there is no irap or the irap has already been processed.*/ + for (i = 0; i < pkt.nb_nals; i++) { + if (IS_IRAP(&pkt.nals[i])) { + irap_done = 0; + break; + } + } + + prev_size = out->size; /* prev_size stores the current length of output packet */ + av_assert1(prev_size == 0); - while (bytestream2_get_bytes_left(&gb)) { - uint32_t nalu_size = 0; - int nalu_type; - int is_irap, add_extradata, extra_size, prev_size; + for (i = 0; i < pkt.nb_nals; i++) { + H2645NAL *nal = &pkt.nals[i]; + + if (IS_PARAMSET(nal)) { + ret = update_paramset(ctx, nal); + if (ret < 0) + goto done; + continue; + } - for (i = 0; i < s->length_size; i++) - nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb); + if (!irap_done && IS_IRAP(nal)) { /* append extradata and sei before first irap*/ - nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f; + /* reset the pps signalled flag in this cvs*/ + for (j = 0; j < HEVC_MAX_PPS_COUNT; j++) + s->ps.pps_list[j].is_signalled = 0; - /* prepend extradata to IRAP frames */ - is_irap = nalu_type >= 16 && nalu_type <= 23; - add_extradata = is_irap && !got_irap; - extra_size = add_extradata * ctx->par_out->extradata_size; - got_irap |= is_irap; + ret = write_extradata(ctx, out, &prev_size, nal); + if (ret < 0) + goto done; + + /* append any past SEI prefix nals*/ + for (j = 0; j < i; j++) { + H2645NAL *nal_past = &pkt.nals[j]; + if (nal_past->type == HEVC_NAL_SEI_PREFIX) { + ret = av_grow_packet(out, nal_past->raw_size + 4); + if (ret < 0) + goto done; + WRITE_NAL(out->data, prev_size, nal_past->raw_data, nal_past->raw_size, 1); + } + } - if (SIZE_MAX - nalu_size < 4 || - SIZE_MAX - 4 - nalu_size < extra_size) { - ret = AVERROR_INVALIDDATA; - goto fail; + /* write irap nal unit*/ + ret = write_vcl(ctx, out, &prev_size, nal); + if (ret < 0) + goto done; + irap_done = 1; + continue; } - prev_size = out->size; + /* do not write any sei prefix nal units if irap exists but has not been + * processed*/ + if (nal->type == HEVC_NAL_SEI_PREFIX && !irap_done) + continue; - ret = av_grow_packet(out, 4 + nalu_size + extra_size); - if (ret < 0) - goto fail; + if (IS_VCL(nal)) { + ret = write_vcl(ctx, out, &prev_size, nal); + if (ret < 0) + goto done; + continue; + } - if (add_extradata) - memcpy(out->data + prev_size, ctx->par_out->extradata, extra_size); - AV_WB32(out->data + prev_size + extra_size, 1); - bytestream2_get_buffer(&gb, out->data + prev_size + 4 + extra_size, nalu_size); + /*copy any other nal units to the output - i.e. SEI_SUFFIX*/ + ret = av_grow_packet(out, nal->raw_size + 4); + if (ret < 0) + goto done; + WRITE_NAL(out->data, prev_size, nal->raw_data, nal->raw_size, 1); } + av_assert1(prev_size == out->size); + ret = av_packet_copy_props(out, in); - if (ret < 0) - goto fail; -fail: +done: + ff_h2645_packet_uninit(&pkt); + if (ret < 0) av_packet_unref(out); av_packet_free(&in); @@ -190,6 +628,7 @@ const AVBitStreamFilter ff_hevc_mp4toannexb_bsf = { .name = "hevc_mp4toannexb", .priv_data_size = sizeof(HEVCBSFContext), .init = hevc_mp4toannexb_init, + .close = hevc_mp4toannexb_close, .filter = hevc_mp4toannexb_filter, .codec_ids = codec_ids, }; diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak index 559c3898bc..4f812b0834 100644 --- a/tests/fate/hevc.mak +++ b/tests/fate/hevc.mak @@ -238,7 +238,7 @@ FATE_HEVC-$(call ALLYES, HEVC_DEMUXER MOV_DEMUXER HEVC_MP4TOANNEXB_BSF MOV_MUXER fate-hevc-bsf-mp4toannexb: tests/data/hevc-mp4.mov fate-hevc-bsf-mp4toannexb: CMD = md5 -i $(TARGET_PATH)/tests/data/hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc fate-hevc-bsf-mp4toannexb: CMP = oneline -fate-hevc-bsf-mp4toannexb: REF = 1873662a3af1848c37e4eb25722c8df9 +fate-hevc-bsf-mp4toannexb: REF = 3c9d998a3aa2b9e0fb1c1f434952bf8b fate-hevc-skiploopfilter: CMD = framemd5 -skip_loop_filter nokey -i $(TARGET_SAMPLES)/hevc-conformance/SAO_D_Samsung_5.bit -sws_flags bitexact FATE_HEVC += fate-hevc-skiploopfilter
From: Andriy Gelman <andriy.gelman@gmail.com> Fixes #7799 Currently, the mp4toannexb filter always inserts the same extradata at the start of the first IRAP unit. As in ticket #7799, this can lead to decoding errors if modified parameter sets are signalled in-band. Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion: -RAP_B_Bossen_1.bit -RPS_C_ericsson_5.bit -SLIST_A_Sony_4.bit -SLIST_B_Sony_8.bit -SLIST_C_Sony_3.bit -SLIST_D_Sony_9.bit -TSKIP_A_MS_2.bit This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets during the conversion. The correct combination is inserted at the start of the first IRAP. SEIs from extradata are inserted before each IRAP. This commit also makes an update to the hevc-bsf-mp4toannexb fate test since the result before this patch contained duplicate parameter sets in-band. --- libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++-- tests/fate/hevc.mak | 2 +- 2 files changed, 472 insertions(+), 33 deletions(-)