[FFmpeg-devel] [PATCH v6 1/3] hevc_mp4toannexb: Insert correct parameter sets before IRAP
Michael Niedermayer
michael at niedermayer.cc
Sun Nov 24 01:02:06 EET 2019
On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote:
> From: Andriy Gelman <andriy.gelman at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20191124/ae6a9332/attachment.sig>
More information about the ffmpeg-devel
mailing list