On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote: > From: Andriy Gelman <andriy.gel...@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 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship naturally arises out of democracy, and the most aggravated form of tyranny and slavery out of the most extreme liberty. -- Plato
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".