On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote: > From: Andriy Gelman <[email protected]> > > 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 [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
