[FFmpeg-devel,v2] hevc_mp4toannexb: Insert correct parameter sets before IRAP

Submitted by Andriy Gelman on Aug. 19, 2019, 2:37 a.m.

Details

Message ID 20190819023757.20900-1-andriy.gelman@gmail.com
State New
Headers show

Commit Message

Andriy Gelman Aug. 19, 2019, 2:37 a.m.
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.

This commit keeps track of the vps/sps/pps parameter sets during the
conversion. The correct combination is inserted at the start of the
first IRAP unit instead of the original extradata. SEI prefix nal units
are also cached and inserted after the parameter sets.

This commit also makes an update to the hevc-bsf-mp4toannexb fate
test since the result before this patch contained duplicate vps/sps/pps
parameter sets in-band.
---
 libavcodec/hevc_mp4toannexb_bsf.c | 475 +++++++++++++++++++++++++++---
 tests/fate/hevc.mak               |   2 +-
 2 files changed, 437 insertions(+), 40 deletions(-)

Comments

Michael Niedermayer Aug. 19, 2019, 11:01 p.m.
On Sun, Aug 18, 2019 at 10:37:57PM -0400, Andriy Gelman wrote:
[...]
> diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
> index 559c3898bc..46f130f4d1 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 = d41d8cd98f00b204e9800998ecf8427e

is it intended that this is empty ?
touch zero
 md5sum zero
d41d8cd98f00b204e9800998ecf8427e  zero

thanks

[...]
Andriy Gelman Aug. 20, 2019, 2:28 a.m.
On Tue, 20. Aug 01:01, Michael Niedermayer wrote:
> On Sun, Aug 18, 2019 at 10:37:57PM -0400, Andriy Gelman wrote:
> [...]
> > diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
> > index 559c3898bc..46f130f4d1 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 = d41d8cd98f00b204e9800998ecf8427e
> 
> is it intended that this is empty ?
> touch zero
>  md5sum zero
> d41d8cd98f00b204e9800998ecf8427e  zero

Sorry, the correct value should be 2e9f6a1b7dd8c7a6fd465622af06626b
I'll update this in the next iteration

Thanks, 
Andriy
Andreas Rheinhardt Aug. 20, 2019, 7:54 a.m.
Hello,

I have not looked at the *PS and the SEI stuff yet, but here is
already my review of the general code.

Andriy Gelman:
> 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.
> 
> This commit keeps track of the vps/sps/pps parameter sets during the
> conversion. The correct combination is inserted at the start of the
> first IRAP unit instead of the original extradata. SEI prefix nal units
> are also cached and inserted after the parameter sets.
> 
> This commit also makes an update to the hevc-bsf-mp4toannexb fate
> test since the result before this patch contained duplicate vps/sps/pps
> parameter sets in-band.
> ---
>  libavcodec/hevc_mp4toannexb_bsf.c | 475 +++++++++++++++++++++++++++---
>  tests/fate/hevc.mak               |   2 +-
>  2 files changed, 437 insertions(+), 40 deletions(-)
> 
> diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c
> index 09bce5b34c..7f3d68252d 100644
> --- a/libavcodec/hevc_mp4toannexb_bsf.c
> +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> @@ -28,19 +28,212 @@
>  #include "bsf.h"
>  #include "bytestream.h"
>  #include "hevc.h"
> -
> -#define MIN_HEVCC_LENGTH 23
> +#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)
> +
> +#define WRITE_NAL(pkt, prev_size, in_buffer) do {                            \
> +    AV_WB32((pkt)->data + (prev_size), 1);                                   \
> +    prev_size += 4;                                                          \
> +    memcpy((pkt)->data + (prev_size), (in_buffer)->data, (in_buffer)->size); \
> +    prev_size += (in_buffer)->size;                                          \
> +} while (0)
> +
> +typedef struct Param {
> +    AVBufferRef *raw_data; /*store a copy of the raw data to construct extradata*/
> +    int ref;  /*stores the ref of the higher level parameter set*/
> +} 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];
> +
> +    AVBufferRef *sei_prefix;
> +} ParamSets;
>  
>  typedef struct HEVCBSFContext {
> -    uint8_t  length_size;
> -    int      extradata_parsed;
> +    uint8_t      length_size;
> +    int          extradata_parsed;
> +    ParamSets    ps; /*make own of version of HEVCParamSets store copy of the bytestream*/
>  } HEVCBSFContext;
>  
> +
> +static int parse_vps(AVBSFContext* ctx, H2645NAL *nal)
> +{
> +    int vps_id = 0;
> +    Param *param_ptr;
> +
> +    HEVCBSFContext *s = ctx->priv_data;
> +    ParamSets *ps     = &s->ps;
> +
> +    GetBitContext *gb = &nal->gb;
> +    int nal_size      = nal->raw_size;
> +
> +    vps_id = get_bits(gb, 4);
> +    if (vps_id >= HEVC_MAX_VPS_COUNT) {
> +        av_log(ctx, AV_LOG_ERROR, "VPS id out of range: %d\n", vps_id);
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    param_ptr = &ps->vps_list[vps_id];
> +    /*init raw_data if needed*/
> +    if (!param_ptr->raw_data) {
> +        param_ptr->raw_data = av_buffer_allocz(nal_size);
> +        param_ptr->ref      = 0;
> +        if (!param_ptr->raw_data)
> +            return AVERROR(ENOMEM);
> +    }
> +
> +    if (param_ptr->raw_data && param_ptr->raw_data->size == nal_size &&
> +        !memcmp(param_ptr->raw_data->data, nal->raw_data, nal_size)) {
> +        av_log(ctx, AV_LOG_DEBUG, "Parsed VPS id: %d. Copy already exists in parameter set\n", vps_id);
> +    } else {
> +        AVBufferRef *vps_buf = av_buffer_allocz(nal_size);
> +        if (!vps_buf)
> +            return AVERROR(ENOMEM);
> +
> +        /*copy raw data into vps_buf buffer and replace existing copy in ps*/
> +        memcpy(vps_buf->data, nal->raw_data, nal_size);
> +        av_buffer_unref(&param_ptr->raw_data);
> +        param_ptr->raw_data = vps_buf;
> +    }
> +    return 0;
> +}
> +
> +static int parse_sps(AVBSFContext *ctx, H2645NAL *nal)
> +{
> +    int sps_id, vps_ref, max_sub_layers_minus1;
> +    int i;
> +    Param *param_ptr;
> +
> +    HEVCBSFContext *s = (HEVCBSFContext*)ctx->priv_data;
> +    ParamSets *ps     = &s->ps;
> +
> +    GetBitContext *gb = &nal->gb;
> +    int nal_size      = nal->raw_size;
> +
> +    uint8_t sub_layer_profile_present_flag[HEVC_MAX_SUB_LAYERS];
> +    uint8_t sub_layer_level_present_flag[HEVC_MAX_SUB_LAYERS];
> +
> +    vps_ref = get_bits(gb, 4);
> +    if (vps_ref >= HEVC_MAX_VPS_COUNT) {
> +        av_log(ctx, AV_LOG_ERROR, "VPS id out of range: %d\n", vps_ref);
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    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_long(gb);
> +    if (sps_id >= HEVC_MAX_SPS_COUNT) {
> +        av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_id);
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    param_ptr = &ps->sps_list[sps_id];
> +    if (!param_ptr->raw_data) {
> +        param_ptr->raw_data = av_buffer_allocz(nal_size);
> +        param_ptr->ref      = 0;
> +        if (!param_ptr->raw_data)
> +            return AVERROR(ENOMEM);
> +    }
> +
> +    if (param_ptr->raw_data && param_ptr->raw_data->size == nal_size &&
> +        !memcmp(param_ptr->raw_data->data, nal->raw_data, nal_size)) {
> +        av_log(ctx, AV_LOG_DEBUG, "Parsed SPS id: %d. Copy already exists in parameter set\n", sps_id);
> +    } else {
> +        AVBufferRef *sps_buf = av_buffer_allocz(nal_size);
> +        if (!sps_buf)
> +            return AVERROR(ENOMEM);
> +
> +        /*copy raw data into vps_buf buffer and replace existing copy in ps*/
> +        memcpy(sps_buf->data, nal->raw_data, nal_size);
> +        av_buffer_unref(&param_ptr->raw_data);
> +        param_ptr->raw_data = sps_buf;
> +        param_ptr->ref      = vps_ref;
> +    }
> +    return 0;
> +}
> +
> +static int parse_pps(AVBSFContext *ctx, H2645NAL *nal)
> +{
> +    int pps_id, sps_ref;
> +    Param *param_ptr;
> +
> +    HEVCBSFContext *s = ctx->priv_data;
> +    ParamSets *ps     = &s->ps;
> +
> +    GetBitContext *gb = &nal->gb;
> +    int nal_size      = nal->raw_size;
> +
> +    pps_id = get_ue_golomb_long(gb);
> +    if (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_long(gb);
> +    if (sps_ref >= HEVC_MAX_SPS_COUNT) {
> +        av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_ref);
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    param_ptr = &ps->pps_list[pps_id];
> +    if (!param_ptr->raw_data) {
> +        param_ptr->raw_data = av_buffer_allocz(nal_size);
> +        param_ptr->ref      = 0;
> +        if (!param_ptr->raw_data)
> +            return AVERROR(ENOMEM);
> +    }
> +
> +    if (param_ptr->raw_data && param_ptr->raw_data->size == nal_size &&
> +        !memcmp(param_ptr->raw_data->data, nal->raw_data, nal_size)) {
> +        av_log(ctx, AV_LOG_DEBUG, "Parsed PPS id: %d. Copy already exists in parameter set\n", pps_id);
> +    } else {
> +        AVBufferRef *pps_buf = av_buffer_allocz(nal_size);
> +        if (!pps_buf)
> +            return AVERROR(ENOMEM);
> +
> +        /*copy raw data into vps_buf buffer and replace existing copy in ps*/
> +        memcpy(pps_buf->data, nal->raw_data, nal_size);
> +        av_buffer_unref(&param_ptr->raw_data);
> +        param_ptr->raw_data = pps_buf;
> +        param_ptr->ref      = sps_ref;
> +    }
> +    return 0;
> +}
> +
>  static int hevc_extradata_to_annexb(AVBSFContext *ctx)
>  {
> -    GetByteContext gb;
>      int length_size, num_arrays, i, j;
>      int ret = 0;
> +    GetByteContext gb;
>  

Un

>      uint8_t *new_extradata = NULL;
>      size_t   new_extradata_size = 0;
> @@ -94,10 +287,95 @@ fail:
>      return ret;
>  }
>  
> +static int avcc_to_annexb(AVBSFContext *ctx, AVPacket **pkt, int nalu_length_size)
> +{
> +    int ret, i, prev_size;
> +    GetByteContext gb;
> +
> +    AVPacket* pkt_out = av_packet_alloc();
> +    if (!pkt_out)
> +        return AVERROR(ENOMEM);

I initially wanted to write that all you change in this subfunction is
the packet's data, so that you do not need to allocate a new packet at
all; all you need is an AVBufferRef equivalent of av_grow_packet. As
it happens, I, too, have encountered such a need (can be used
advantageously in several bitstream filters) and I have implemented
such a function
(https://github.com/mkver/FFmpeg/commit/a119e056a27736feb88f191ddce93b0a0d8dfb41),
but have not yet sent this stuff to the ML because I was interrupted
by other stuff.
But then I read on and saw that this whole function can be omitted.
See below.

> +
> +    ret = av_new_packet(pkt_out, 0);
> +    if (ret < 0)
> +        goto fail;
> +
> +    bytestream2_init(&gb, (*pkt)->data, (*pkt)->size);
> +    while (bytestream2_get_bytes_left(&gb)) {
> +        uint32_t nalu_size = 0;
> +        for (i = 0; i < nalu_length_size; i++)
> +            nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb);
> +
> +        if (SIZE_MAX - nalu_size < 4) {
> +            ret = AVERROR_INVALIDDATA;
> +            goto fail;
> +        }
> +
> +        prev_size = pkt_out->size;
> +        ret = av_grow_packet(pkt_out, 4 + nalu_size);
> +        if (ret < 0)
> +            goto fail;
> +
> +        AV_WB32(pkt_out->data + prev_size, 1);
> +        ret = bytestream2_get_buffer(&gb, pkt_out->data + prev_size + 4, nalu_size);
> +        if (ret != nalu_size)
> +            av_log(ctx, AV_LOG_WARNING, "Corrupted avcc nal unit. Read %d/%d bytes\n", ret, nalu_size);
> +    }
> +
> +    ret = av_packet_copy_props(pkt_out, *pkt);
> +    if (ret < 0)
> +        goto fail;
> +
> +    /*unref avcc version and replace pkt with pkt_out*/
> +    av_packet_free(pkt);
> +    *pkt = pkt_out;
> +
> +    return 0;
> +
> +fail:
> +    av_packet_free(&pkt_out);
> +    return ret;
> +}
> +
> +static int update_sei(AVBufferRef **sei, H2645NAL *nal)
> +{
> +    AVBufferRef *tmp;
> +
> +    av_buffer_unref(sei);
> +    tmp = av_buffer_alloc(nal->raw_size);
> +    if (!tmp)
> +        return AVERROR(ENOMEM);
> +
> +    memcpy(tmp->data, nal->raw_data, nal->raw_size);
> +    *sei = tmp;
> +
> +    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_mp4toannexb_init(AVBSFContext *ctx)
>  {
>      HEVCBSFContext *s = ctx->priv_data;
> -    int ret;
> +    H2645Packet pkt;
> +    int i, ret = 0;
>  
>      if (ctx->par_in->extradata_size < MIN_HEVCC_LENGTH ||
>          AV_RB24(ctx->par_in->extradata) == 1           ||
> @@ -110,7 +388,114 @@ static int hevc_mp4toannexb_init(AVBSFContext *ctx)
>              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 (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) {
> +                ret = update_sei(&s->ps.sei_prefix, 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_buffer_unref(&ps->vps_list[i].raw_data);
> +    for (i = 0; i < FF_ARRAY_ELEMS(ps->sps_list); i++)
> +       av_buffer_unref(&ps->sps_list[i].raw_data);
> +    for (i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++)
> +       av_buffer_unref(&ps->pps_list[i].raw_data);
> +
> +    av_buffer_unref(&ps->sei_prefix);
> +}
> +
> +static void hevc_mp4toannexb_close(AVBSFContext *ctx)
> +{
> +    HEVCBSFContext *s = (HEVCBSFContext*)ctx->priv_data;
> +    ps_uninit(&s->ps);
> +}
> +
> +static int write_extradata(AVBSFContext *ctx, AVPacket *pkt_out, H2645NAL *nal_irap)
> +{
> +    int ref, ret, prev_size;
> +    int new_extradata_size = 0;
> +
> +    HEVCBSFContext *s = (HEVCBSFContext*)ctx->priv_data;
> +    ParamSets *ps     = &s->ps;
> +    GetBitContext *gb = &nal_irap->gb;
> +
> +    /* currently active pps parameter set */
> +    const Param *vps;
> +    const Param *sps;
> +    const Param *pps;
> +
> +    skip_bits1(gb); /*first_slice_ic_pic_flag*/
> +    skip_bits1(gb); /*no_output_of_prior_pics_flag*/
> +
> +    ref = get_ue_golomb_long(gb);
> +    if (ref >= HEVC_MAX_PPS_COUNT || ps->pps_list[ref].raw_data == NULL) {
> +        av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d\n", ref);
> +        return AVERROR_INVALIDDATA;
> +    }
> +    pps                 = &ps->pps_list[ref];
> +    new_extradata_size += pps->raw_data->size + 4;
> +    ref                 = pps->ref;
> +
> +    if (ref >= HEVC_MAX_SPS_COUNT || ps->sps_list[ref].raw_data == NULL) {
> +        av_log(ctx, AV_LOG_ERROR, "Invalid SPS: %d\n", ref);
> +        return AVERROR_INVALIDDATA;
> +    }
> +    sps                 = &ps->sps_list[ref];
> +    new_extradata_size += sps->raw_data->size + 4;
> +    ref                 = sps->ref;
> +
> +    if (ref >= HEVC_MAX_VPS_COUNT || ps->vps_list[ref].raw_data == NULL) {
> +        av_log(ctx, AV_LOG_ERROR, "Invalid VPS: %d\n", ref);
> +        return AVERROR_INVALIDDATA;
>      }
> +    vps                 = &ps->vps_list[ref];
> +    new_extradata_size += vps->raw_data->size + 4;
> +
> +    if (ps->sei_prefix)
> +        new_extradata_size += ps->sei_prefix->size + 4;
> +
> +    prev_size = pkt_out->size;
> +    ret = av_grow_packet(pkt_out, new_extradata_size);
> +    if (ret < 0)
> +        return AVERROR(ENOMEM);
> +
> +    WRITE_NAL(pkt_out, prev_size, vps->raw_data);
> +    WRITE_NAL(pkt_out, prev_size, sps->raw_data);
> +    WRITE_NAL(pkt_out, prev_size, pps->raw_data);
> +
> +    if (ps->sei_prefix)
> +        WRITE_NAL(pkt_out, prev_size, ps->sei_prefix);
>  
>      return 0;
>  }
> @@ -119,62 +504,73 @@ 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, prev_size;
> +    int ret = 0;
> +    int first_irap = 0;
>  
>      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);
> +    /*convert packet from avcc to annexb*/
> +    ret = avcc_to_annexb(ctx, &in, s->length_size);
> +    if (ret < 0) {
> +        av_packet_free(&in);
> +        return ret;
> +    }
>  
> -    while (bytestream2_get_bytes_left(&gb)) {
> -        uint32_t nalu_size = 0;
> -        int      nalu_type;
> -        int is_irap, add_extradata, extra_size, prev_size;
> +    /*segment annexb packet into nal units*/
> +    memset(&pkt, 0, sizeof(H2645Packet));
> +    ret = ff_h2645_packet_split(&pkt, in->data, in->size, ctx, 0, 0, AV_CODEC_ID_HEVC, 1, 0);

ff_h2645_packet_split is perfectly fine with mp4-style data. There is
no need to convert it to annex b before this step. Avoiding this has
the benefit of it not mishandling trailing 0x00 in this case.
ret = ff_h2645_packet_split(&pkt, in->data, in->size, ctx, 1,
s->length_size, AV_CODEC_ID_HEVC, 1, 0);
should do the trick.
That should already provide a pretty good speed boost. Another
possible way for speed improvement is not using ff_h2645_packet_split,
but using ff_h2645_extract_rbsp on the NAL units that we are
interested in and simply copy the other NAL units. (Looking for 0x03
escapes is expensive.)

> +    if (ret < 0)
> +        goto done;
>  
> -        for (i = 0; i < s->length_size; i++)
> -            nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb);
> +    for (i = 0; i < pkt.nb_nals; ++i) {
>  
> -        nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f;
> +        H2645NAL *nal = &pkt.nals[i];
>  
> -        /* 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;
> +        /*current segmentation algorithm includes next 0x00 from next nal unit*/
> +        if (nal->raw_data[nal->raw_size - 1] == 0x00)
> +            nal->raw_size--;
>  
> -        if (SIZE_MAX - nalu_size < 4 ||
> -            SIZE_MAX - 4 - nalu_size < extra_size) {
> -            ret = AVERROR_INVALIDDATA;
> -            goto fail;
> +        if (IS_PARAMSET(nal)) {
> +            ret = update_paramset(ctx, nal);
> +            if (ret < 0)
> +                goto done;
> +            continue;
>          }
>  
> -        prev_size = out->size;
> +        if (nal->type == HEVC_NAL_SEI_PREFIX) {
> +            ret = update_sei(&s->ps.sei_prefix, nal);
> +            if (ret < 0)
> +                goto done;
> +            continue;
> +        }
>  
> -        ret = av_grow_packet(out, 4 + nalu_size + extra_size);
> -        if (ret < 0)
> -            goto fail;
> +        if (!first_irap && IS_IRAP(nal)) {
> +            ret = write_extradata(ctx, out, nal);
> +            if (ret < 0)
> +                goto done;
> +            first_irap = 1;
> +        }
>  
> -        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);
> +        /*write to output packet*/
> +        prev_size = out->size;
> +        av_grow_packet(out, nal->raw_size + 4);
> +        WRITE_NAL(out, prev_size, nal);

The WRITE_NAL macro as it exists is not applicable here: It will write
the NAL's data, which is unescaped (the 0x03 escape bytes have been
removed), but you should write the raw data (including the 0x03).
Doing it this way will result in corrupted output.

>      }
>  
> -    ret = av_packet_copy_props(out, in);
> -    if (ret < 0)
> -        goto fail;
> +done:
> +    ff_h2645_packet_uninit(&pkt);
>  
> -fail:
>      if (ret < 0)
>          av_packet_unref(out);
>      av_packet_free(&in);
> @@ -190,6 +586,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..46f130f4d1 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 = d41d8cd98f00b204e9800998ecf8427e
>  
>  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
> 

- Andreas
Andriy Gelman Aug. 20, 2019, 4:21 p.m.
Andreas, 

On Tue, 20. Aug 07:54, Andreas Rheinhardt wrote:
> Hello,
> 
> I have not looked at the *PS and the SEI stuff yet, but here is
> already my review of the general code.
> 
> Andriy Gelman:
> > 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.
> > 
> > This commit keeps track of the vps/sps/pps parameter sets during the
> > conversion. The correct combination is inserted at the start of the
> > first IRAP unit instead of the original extradata. SEI prefix nal units
> > are also cached and inserted after the parameter sets.
> > 
> > This commit also makes an update to the hevc-bsf-mp4toannexb fate
> > test since the result before this patch contained duplicate vps/sps/pps
> > parameter sets in-band.
> > ---
> >  libavcodec/hevc_mp4toannexb_bsf.c | 475 +++++++++++++++++++++++++++---
> >  tests/fate/hevc.mak               |   2 +-
> >  2 files changed, 437 insertions(+), 40 deletions(-)
> > 
> > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c
> > index 09bce5b34c..7f3d68252d 100644
> > --- a/libavcodec/hevc_mp4toannexb_bsf.c
> > +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> > @@ -28,19 +28,212 @@
> >  #include "bsf.h"
> >  #include "bytestream.h"
> >  #include "hevc.h"
> > -
> > -#define MIN_HEVCC_LENGTH 23
> > +#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)
> > +
> > +#define WRITE_NAL(pkt, prev_size, in_buffer) do {                            \
> > +    AV_WB32((pkt)->data + (prev_size), 1);                                   \
> > +    prev_size += 4;                                                          \
> > +    memcpy((pkt)->data + (prev_size), (in_buffer)->data, (in_buffer)->size); \
> > +    prev_size += (in_buffer)->size;                                          \
> > +} while (0)
> > +
> > +typedef struct Param {
> > +    AVBufferRef *raw_data; /*store a copy of the raw data to construct extradata*/
> > +    int ref;  /*stores the ref of the higher level parameter set*/
> > +} 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];
> > +
> > +    AVBufferRef *sei_prefix;
> > +} ParamSets;
> >  
> >  typedef struct HEVCBSFContext {
> > -    uint8_t  length_size;
> > -    int      extradata_parsed;
> > +    uint8_t      length_size;
> > +    int          extradata_parsed;
> > +    ParamSets    ps; /*make own of version of HEVCParamSets store copy of the bytestream*/
> >  } HEVCBSFContext;
> >  
> > +
> > +static int parse_vps(AVBSFContext* ctx, H2645NAL *nal)
> > +{
> > +    int vps_id = 0;
> > +    Param *param_ptr;
> > +
> > +    HEVCBSFContext *s = ctx->priv_data;
> > +    ParamSets *ps     = &s->ps;
> > +
> > +    GetBitContext *gb = &nal->gb;
> > +    int nal_size      = nal->raw_size;
> > +
> > +    vps_id = get_bits(gb, 4);
> > +    if (vps_id >= HEVC_MAX_VPS_COUNT) {
> > +        av_log(ctx, AV_LOG_ERROR, "VPS id out of range: %d\n", vps_id);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    param_ptr = &ps->vps_list[vps_id];
> > +    /*init raw_data if needed*/
> > +    if (!param_ptr->raw_data) {
> > +        param_ptr->raw_data = av_buffer_allocz(nal_size);
> > +        param_ptr->ref      = 0;
> > +        if (!param_ptr->raw_data)
> > +            return AVERROR(ENOMEM);
> > +    }
> > +
> > +    if (param_ptr->raw_data && param_ptr->raw_data->size == nal_size &&
> > +        !memcmp(param_ptr->raw_data->data, nal->raw_data, nal_size)) {
> > +        av_log(ctx, AV_LOG_DEBUG, "Parsed VPS id: %d. Copy already exists in parameter set\n", vps_id);
> > +    } else {
> > +        AVBufferRef *vps_buf = av_buffer_allocz(nal_size);
> > +        if (!vps_buf)
> > +            return AVERROR(ENOMEM);
> > +
> > +        /*copy raw data into vps_buf buffer and replace existing copy in ps*/
> > +        memcpy(vps_buf->data, nal->raw_data, nal_size);
> > +        av_buffer_unref(&param_ptr->raw_data);
> > +        param_ptr->raw_data = vps_buf;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int parse_sps(AVBSFContext *ctx, H2645NAL *nal)
> > +{
> > +    int sps_id, vps_ref, max_sub_layers_minus1;
> > +    int i;
> > +    Param *param_ptr;
> > +
> > +    HEVCBSFContext *s = (HEVCBSFContext*)ctx->priv_data;
> > +    ParamSets *ps     = &s->ps;
> > +
> > +    GetBitContext *gb = &nal->gb;
> > +    int nal_size      = nal->raw_size;
> > +
> > +    uint8_t sub_layer_profile_present_flag[HEVC_MAX_SUB_LAYERS];
> > +    uint8_t sub_layer_level_present_flag[HEVC_MAX_SUB_LAYERS];
> > +
> > +    vps_ref = get_bits(gb, 4);
> > +    if (vps_ref >= HEVC_MAX_VPS_COUNT) {
> > +        av_log(ctx, AV_LOG_ERROR, "VPS id out of range: %d\n", vps_ref);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    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_long(gb);
> > +    if (sps_id >= HEVC_MAX_SPS_COUNT) {
> > +        av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_id);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    param_ptr = &ps->sps_list[sps_id];
> > +    if (!param_ptr->raw_data) {
> > +        param_ptr->raw_data = av_buffer_allocz(nal_size);
> > +        param_ptr->ref      = 0;
> > +        if (!param_ptr->raw_data)
> > +            return AVERROR(ENOMEM);
> > +    }
> > +
> > +    if (param_ptr->raw_data && param_ptr->raw_data->size == nal_size &&
> > +        !memcmp(param_ptr->raw_data->data, nal->raw_data, nal_size)) {
> > +        av_log(ctx, AV_LOG_DEBUG, "Parsed SPS id: %d. Copy already exists in parameter set\n", sps_id);
> > +    } else {
> > +        AVBufferRef *sps_buf = av_buffer_allocz(nal_size);
> > +        if (!sps_buf)
> > +            return AVERROR(ENOMEM);
> > +
> > +        /*copy raw data into vps_buf buffer and replace existing copy in ps*/
> > +        memcpy(sps_buf->data, nal->raw_data, nal_size);
> > +        av_buffer_unref(&param_ptr->raw_data);
> > +        param_ptr->raw_data = sps_buf;
> > +        param_ptr->ref      = vps_ref;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int parse_pps(AVBSFContext *ctx, H2645NAL *nal)
> > +{
> > +    int pps_id, sps_ref;
> > +    Param *param_ptr;
> > +
> > +    HEVCBSFContext *s = ctx->priv_data;
> > +    ParamSets *ps     = &s->ps;
> > +
> > +    GetBitContext *gb = &nal->gb;
> > +    int nal_size      = nal->raw_size;
> > +
> > +    pps_id = get_ue_golomb_long(gb);
> > +    if (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_long(gb);
> > +    if (sps_ref >= HEVC_MAX_SPS_COUNT) {
> > +        av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_ref);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    param_ptr = &ps->pps_list[pps_id];
> > +    if (!param_ptr->raw_data) {
> > +        param_ptr->raw_data = av_buffer_allocz(nal_size);
> > +        param_ptr->ref      = 0;
> > +        if (!param_ptr->raw_data)
> > +            return AVERROR(ENOMEM);
> > +    }
> > +
> > +    if (param_ptr->raw_data && param_ptr->raw_data->size == nal_size &&
> > +        !memcmp(param_ptr->raw_data->data, nal->raw_data, nal_size)) {
> > +        av_log(ctx, AV_LOG_DEBUG, "Parsed PPS id: %d. Copy already exists in parameter set\n", pps_id);
> > +    } else {
> > +        AVBufferRef *pps_buf = av_buffer_allocz(nal_size);
> > +        if (!pps_buf)
> > +            return AVERROR(ENOMEM);
> > +
> > +        /*copy raw data into vps_buf buffer and replace existing copy in ps*/
> > +        memcpy(pps_buf->data, nal->raw_data, nal_size);
> > +        av_buffer_unref(&param_ptr->raw_data);
> > +        param_ptr->raw_data = pps_buf;
> > +        param_ptr->ref      = sps_ref;
> > +    }
> > +    return 0;
> > +}
> > +
> >  static int hevc_extradata_to_annexb(AVBSFContext *ctx)
> >  {
> > -    GetByteContext gb;
> >      int length_size, num_arrays, i, j;
> >      int ret = 0;
> > +    GetByteContext gb;
> >  
> 
> Un
> 
> >      uint8_t *new_extradata = NULL;
> >      size_t   new_extradata_size = 0;
> > @@ -94,10 +287,95 @@ fail:
> >      return ret;
> >  }
> >  
> > +static int avcc_to_annexb(AVBSFContext *ctx, AVPacket **pkt, int nalu_length_size)
> > +{
> > +    int ret, i, prev_size;
> > +    GetByteContext gb;
> > +
> > +    AVPacket* pkt_out = av_packet_alloc();
> > +    if (!pkt_out)
> > +        return AVERROR(ENOMEM);
> 
> I initially wanted to write that all you change in this subfunction is
> the packet's data, so that you do not need to allocate a new packet at
> all; all you need is an AVBufferRef equivalent of av_grow_packet. As
> it happens, I, too, have encountered such a need (can be used
> advantageously in several bitstream filters) and I have implemented
> such a function
> (https://github.com/mkver/FFmpeg/commit/a119e056a27736feb88f191ddce93b0a0d8dfb41),
> but have not yet sent this stuff to the ML because I was interrupted
> by other stuff.
> But then I read on and saw that this whole function can be omitted.
> See below.
> 
> > +
> > +    ret = av_new_packet(pkt_out, 0);
> > +    if (ret < 0)
> > +        goto fail;
> > +
> > +    bytestream2_init(&gb, (*pkt)->data, (*pkt)->size);
> > +    while (bytestream2_get_bytes_left(&gb)) {
> > +        uint32_t nalu_size = 0;
> > +        for (i = 0; i < nalu_length_size; i++)
> > +            nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb);
> > +
> > +        if (SIZE_MAX - nalu_size < 4) {
> > +            ret = AVERROR_INVALIDDATA;
> > +            goto fail;
> > +        }
> > +
> > +        prev_size = pkt_out->size;
> > +        ret = av_grow_packet(pkt_out, 4 + nalu_size);
> > +        if (ret < 0)
> > +            goto fail;
> > +
> > +        AV_WB32(pkt_out->data + prev_size, 1);
> > +        ret = bytestream2_get_buffer(&gb, pkt_out->data + prev_size + 4, nalu_size);
> > +        if (ret != nalu_size)
> > +            av_log(ctx, AV_LOG_WARNING, "Corrupted avcc nal unit. Read %d/%d bytes\n", ret, nalu_size);
> > +    }
> > +
> > +    ret = av_packet_copy_props(pkt_out, *pkt);
> > +    if (ret < 0)
> > +        goto fail;
> > +
> > +    /*unref avcc version and replace pkt with pkt_out*/
> > +    av_packet_free(pkt);
> > +    *pkt = pkt_out;
> > +
> > +    return 0;
> > +
> > +fail:
> > +    av_packet_free(&pkt_out);
> > +    return ret;
> > +}
> > +
> > +static int update_sei(AVBufferRef **sei, H2645NAL *nal)
> > +{
> > +    AVBufferRef *tmp;
> > +
> > +    av_buffer_unref(sei);
> > +    tmp = av_buffer_alloc(nal->raw_size);
> > +    if (!tmp)
> > +        return AVERROR(ENOMEM);
> > +
> > +    memcpy(tmp->data, nal->raw_data, nal->raw_size);
> > +    *sei = tmp;
> > +
> > +    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_mp4toannexb_init(AVBSFContext *ctx)
> >  {
> >      HEVCBSFContext *s = ctx->priv_data;
> > -    int ret;
> > +    H2645Packet pkt;
> > +    int i, ret = 0;
> >  
> >      if (ctx->par_in->extradata_size < MIN_HEVCC_LENGTH ||
> >          AV_RB24(ctx->par_in->extradata) == 1           ||
> > @@ -110,7 +388,114 @@ static int hevc_mp4toannexb_init(AVBSFContext *ctx)
> >              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 (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) {
> > +                ret = update_sei(&s->ps.sei_prefix, 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_buffer_unref(&ps->vps_list[i].raw_data);
> > +    for (i = 0; i < FF_ARRAY_ELEMS(ps->sps_list); i++)
> > +       av_buffer_unref(&ps->sps_list[i].raw_data);
> > +    for (i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++)
> > +       av_buffer_unref(&ps->pps_list[i].raw_data);
> > +
> > +    av_buffer_unref(&ps->sei_prefix);
> > +}
> > +
> > +static void hevc_mp4toannexb_close(AVBSFContext *ctx)
> > +{
> > +    HEVCBSFContext *s = (HEVCBSFContext*)ctx->priv_data;
> > +    ps_uninit(&s->ps);
> > +}
> > +
> > +static int write_extradata(AVBSFContext *ctx, AVPacket *pkt_out, H2645NAL *nal_irap)
> > +{
> > +    int ref, ret, prev_size;
> > +    int new_extradata_size = 0;
> > +
> > +    HEVCBSFContext *s = (HEVCBSFContext*)ctx->priv_data;
> > +    ParamSets *ps     = &s->ps;
> > +    GetBitContext *gb = &nal_irap->gb;
> > +
> > +    /* currently active pps parameter set */
> > +    const Param *vps;
> > +    const Param *sps;
> > +    const Param *pps;
> > +
> > +    skip_bits1(gb); /*first_slice_ic_pic_flag*/
> > +    skip_bits1(gb); /*no_output_of_prior_pics_flag*/
> > +
> > +    ref = get_ue_golomb_long(gb);
> > +    if (ref >= HEVC_MAX_PPS_COUNT || ps->pps_list[ref].raw_data == NULL) {
> > +        av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d\n", ref);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +    pps                 = &ps->pps_list[ref];
> > +    new_extradata_size += pps->raw_data->size + 4;
> > +    ref                 = pps->ref;
> > +
> > +    if (ref >= HEVC_MAX_SPS_COUNT || ps->sps_list[ref].raw_data == NULL) {
> > +        av_log(ctx, AV_LOG_ERROR, "Invalid SPS: %d\n", ref);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +    sps                 = &ps->sps_list[ref];
> > +    new_extradata_size += sps->raw_data->size + 4;
> > +    ref                 = sps->ref;
> > +
> > +    if (ref >= HEVC_MAX_VPS_COUNT || ps->vps_list[ref].raw_data == NULL) {
> > +        av_log(ctx, AV_LOG_ERROR, "Invalid VPS: %d\n", ref);
> > +        return AVERROR_INVALIDDATA;
> >      }
> > +    vps                 = &ps->vps_list[ref];
> > +    new_extradata_size += vps->raw_data->size + 4;
> > +
> > +    if (ps->sei_prefix)
> > +        new_extradata_size += ps->sei_prefix->size + 4;
> > +
> > +    prev_size = pkt_out->size;
> > +    ret = av_grow_packet(pkt_out, new_extradata_size);
> > +    if (ret < 0)
> > +        return AVERROR(ENOMEM);
> > +
> > +    WRITE_NAL(pkt_out, prev_size, vps->raw_data);
> > +    WRITE_NAL(pkt_out, prev_size, sps->raw_data);
> > +    WRITE_NAL(pkt_out, prev_size, pps->raw_data);
> > +
> > +    if (ps->sei_prefix)
> > +        WRITE_NAL(pkt_out, prev_size, ps->sei_prefix);
> >  
> >      return 0;
> >  }
> > @@ -119,62 +504,73 @@ 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, prev_size;
> > +    int ret = 0;
> > +    int first_irap = 0;
> >  
> >      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);
> > +    /*convert packet from avcc to annexb*/
> > +    ret = avcc_to_annexb(ctx, &in, s->length_size);
> > +    if (ret < 0) {
> > +        av_packet_free(&in);
> > +        return ret;
> > +    }
> >  
> > -    while (bytestream2_get_bytes_left(&gb)) {
> > -        uint32_t nalu_size = 0;
> > -        int      nalu_type;
> > -        int is_irap, add_extradata, extra_size, prev_size;
> > +    /*segment annexb packet into nal units*/
> > +    memset(&pkt, 0, sizeof(H2645Packet));
> > +    ret = ff_h2645_packet_split(&pkt, in->data, in->size, ctx, 0, 0, AV_CODEC_ID_HEVC, 1, 0);
> 
> ff_h2645_packet_split is perfectly fine with mp4-style data. There is
> no need to convert it to annex b before this step. Avoiding this has
> the benefit of it not mishandling trailing 0x00 in this case.
> ret = ff_h2645_packet_split(&pkt, in->data, in->size, ctx, 1,
> s->length_size, AV_CODEC_ID_HEVC, 1, 0);
> should do the trick.
> That should already provide a pretty good speed boost. Another
> possible way for speed improvement is not using ff_h2645_packet_split,
> but using ff_h2645_extract_rbsp on the NAL units that we are
> interested in and simply copy the other NAL units. (Looking for 0x03
> escapes is expensive.)
> 

ok, I'll look into both options. 

> > +    if (ret < 0)
> > +        goto done;
> >  
> > -        for (i = 0; i < s->length_size; i++)
> > -            nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb);
> > +    for (i = 0; i < pkt.nb_nals; ++i) {
> >  
> > -        nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f;
> > +        H2645NAL *nal = &pkt.nals[i];
> >  
> > -        /* 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;
> > +        /*current segmentation algorithm includes next 0x00 from next nal unit*/
> > +        if (nal->raw_data[nal->raw_size - 1] == 0x00)
> > +            nal->raw_size--;
> >  
> > -        if (SIZE_MAX - nalu_size < 4 ||
> > -            SIZE_MAX - 4 - nalu_size < extra_size) {
> > -            ret = AVERROR_INVALIDDATA;
> > -            goto fail;
> > +        if (IS_PARAMSET(nal)) {
> > +            ret = update_paramset(ctx, nal);
> > +            if (ret < 0)
> > +                goto done;
> > +            continue;
> >          }
> >  
> > -        prev_size = out->size;
> > +        if (nal->type == HEVC_NAL_SEI_PREFIX) {
> > +            ret = update_sei(&s->ps.sei_prefix, nal);
> > +            if (ret < 0)
> > +                goto done;
> > +            continue;
> > +        }
> >  
> > -        ret = av_grow_packet(out, 4 + nalu_size + extra_size);
> > -        if (ret < 0)
> > -            goto fail;
> > +        if (!first_irap && IS_IRAP(nal)) {
> > +            ret = write_extradata(ctx, out, nal);
> > +            if (ret < 0)
> > +                goto done;
> > +            first_irap = 1;
> > +        }
> >  
> > -        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);
> > +        /*write to output packet*/
> > +        prev_size = out->size;
> > +        av_grow_packet(out, nal->raw_size + 4);
> > +        WRITE_NAL(out, prev_size, nal);
> 
> The WRITE_NAL macro as it exists is not applicable here: It will write
> the NAL's data, which is unescaped (the 0x03 escape bytes have been
> removed), but you should write the raw data (including the 0x03).
> Doing it this way will result in corrupted output.
> 
Yes, yes, good catch

Andriy
Andriy Gelman Sept. 5, 2019, 3:18 a.m.
Andreas, 
Thanks again for reviewing this patch.

On Tue, 20. Aug 07:54, Andreas Rheinhardt wrote:
> Hello,
> 
> I have not looked at the *PS and the SEI stuff yet, but here is
> already my review of the general code.
> 
> Andriy Gelman:
> > 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.
> > 
> > This commit keeps track of the vps/sps/pps parameter sets during the
> > conversion. The correct combination is inserted at the start of the
> > first IRAP unit instead of the original extradata. SEI prefix nal units
> > are also cached and inserted after the parameter sets.
> > 
> > This commit also makes an update to the hevc-bsf-mp4toannexb fate
> > test since the result before this patch contained duplicate vps/sps/pps
> > parameter sets in-band.
> > ---
> >  libavcodec/hevc_mp4toannexb_bsf.c | 475 +++++++++++++++++++++++++++---
> >  tests/fate/hevc.mak               |   2 +-
> >  2 files changed, 437 insertions(+), 40 deletions(-)
> > 
> > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c
> > index 09bce5b34c..7f3d68252d 100644
> > --- a/libavcodec/hevc_mp4toannexb_bsf.c
> > +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> > @@ -28,19 +28,212 @@
> >  #include "bsf.h"
> >  #include "bytestream.h"
> >  #include "hevc.h"
> > -
> > -#define MIN_HEVCC_LENGTH 23
> > +#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)
> > +
> > +#define WRITE_NAL(pkt, prev_size, in_buffer) do {                            \
> > +    AV_WB32((pkt)->data + (prev_size), 1);                                   \
> > +    prev_size += 4;                                                          \
> > +    memcpy((pkt)->data + (prev_size), (in_buffer)->data, (in_buffer)->size); \
> > +    prev_size += (in_buffer)->size;                                          \
> > +} while (0)
> > +
> > +typedef struct Param {
> > +    AVBufferRef *raw_data; /*store a copy of the raw data to construct extradata*/
> > +    int ref;  /*stores the ref of the higher level parameter set*/
> > +} 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];
> > +
> > +    AVBufferRef *sei_prefix;
> > +} ParamSets;
> >  
> >  typedef struct HEVCBSFContext {
> > -    uint8_t  length_size;
> > -    int      extradata_parsed;
> > +    uint8_t      length_size;
> > +    int          extradata_parsed;
> > +    ParamSets    ps; /*make own of version of HEVCParamSets store copy of the bytestream*/
> >  } HEVCBSFContext;
> >  
> > +
> > +static int parse_vps(AVBSFContext* ctx, H2645NAL *nal)
> > +{
> > +    int vps_id = 0;
> > +    Param *param_ptr;
> > +
> > +    HEVCBSFContext *s = ctx->priv_data;
> > +    ParamSets *ps     = &s->ps;
> > +
> > +    GetBitContext *gb = &nal->gb;
> > +    int nal_size      = nal->raw_size;
> > +
> > +    vps_id = get_bits(gb, 4);
> > +    if (vps_id >= HEVC_MAX_VPS_COUNT) {
> > +        av_log(ctx, AV_LOG_ERROR, "VPS id out of range: %d\n", vps_id);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    param_ptr = &ps->vps_list[vps_id];
> > +    /*init raw_data if needed*/
> > +    if (!param_ptr->raw_data) {
> > +        param_ptr->raw_data = av_buffer_allocz(nal_size);
> > +        param_ptr->ref      = 0;
> > +        if (!param_ptr->raw_data)
> > +            return AVERROR(ENOMEM);
> > +    }
> > +
> > +    if (param_ptr->raw_data && param_ptr->raw_data->size == nal_size &&
> > +        !memcmp(param_ptr->raw_data->data, nal->raw_data, nal_size)) {
> > +        av_log(ctx, AV_LOG_DEBUG, "Parsed VPS id: %d. Copy already exists in parameter set\n", vps_id);
> > +    } else {
> > +        AVBufferRef *vps_buf = av_buffer_allocz(nal_size);
> > +        if (!vps_buf)
> > +            return AVERROR(ENOMEM);
> > +
> > +        /*copy raw data into vps_buf buffer and replace existing copy in ps*/
> > +        memcpy(vps_buf->data, nal->raw_data, nal_size);
> > +        av_buffer_unref(&param_ptr->raw_data);
> > +        param_ptr->raw_data = vps_buf;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int parse_sps(AVBSFContext *ctx, H2645NAL *nal)
> > +{
> > +    int sps_id, vps_ref, max_sub_layers_minus1;
> > +    int i;
> > +    Param *param_ptr;
> > +
> > +    HEVCBSFContext *s = (HEVCBSFContext*)ctx->priv_data;
> > +    ParamSets *ps     = &s->ps;
> > +
> > +    GetBitContext *gb = &nal->gb;
> > +    int nal_size      = nal->raw_size;
> > +
> > +    uint8_t sub_layer_profile_present_flag[HEVC_MAX_SUB_LAYERS];
> > +    uint8_t sub_layer_level_present_flag[HEVC_MAX_SUB_LAYERS];
> > +
> > +    vps_ref = get_bits(gb, 4);
> > +    if (vps_ref >= HEVC_MAX_VPS_COUNT) {
> > +        av_log(ctx, AV_LOG_ERROR, "VPS id out of range: %d\n", vps_ref);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    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_long(gb);
> > +    if (sps_id >= HEVC_MAX_SPS_COUNT) {
> > +        av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_id);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    param_ptr = &ps->sps_list[sps_id];
> > +    if (!param_ptr->raw_data) {
> > +        param_ptr->raw_data = av_buffer_allocz(nal_size);
> > +        param_ptr->ref      = 0;
> > +        if (!param_ptr->raw_data)
> > +            return AVERROR(ENOMEM);
> > +    }
> > +
> > +    if (param_ptr->raw_data && param_ptr->raw_data->size == nal_size &&
> > +        !memcmp(param_ptr->raw_data->data, nal->raw_data, nal_size)) {
> > +        av_log(ctx, AV_LOG_DEBUG, "Parsed SPS id: %d. Copy already exists in parameter set\n", sps_id);
> > +    } else {
> > +        AVBufferRef *sps_buf = av_buffer_allocz(nal_size);
> > +        if (!sps_buf)
> > +            return AVERROR(ENOMEM);
> > +
> > +        /*copy raw data into vps_buf buffer and replace existing copy in ps*/
> > +        memcpy(sps_buf->data, nal->raw_data, nal_size);
> > +        av_buffer_unref(&param_ptr->raw_data);
> > +        param_ptr->raw_data = sps_buf;
> > +        param_ptr->ref      = vps_ref;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int parse_pps(AVBSFContext *ctx, H2645NAL *nal)
> > +{
> > +    int pps_id, sps_ref;
> > +    Param *param_ptr;
> > +
> > +    HEVCBSFContext *s = ctx->priv_data;
> > +    ParamSets *ps     = &s->ps;
> > +
> > +    GetBitContext *gb = &nal->gb;
> > +    int nal_size      = nal->raw_size;
> > +
> > +    pps_id = get_ue_golomb_long(gb);
> > +    if (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_long(gb);
> > +    if (sps_ref >= HEVC_MAX_SPS_COUNT) {
> > +        av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_ref);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    param_ptr = &ps->pps_list[pps_id];
> > +    if (!param_ptr->raw_data) {
> > +        param_ptr->raw_data = av_buffer_allocz(nal_size);
> > +        param_ptr->ref      = 0;
> > +        if (!param_ptr->raw_data)
> > +            return AVERROR(ENOMEM);
> > +    }
> > +
> > +    if (param_ptr->raw_data && param_ptr->raw_data->size == nal_size &&
> > +        !memcmp(param_ptr->raw_data->data, nal->raw_data, nal_size)) {
> > +        av_log(ctx, AV_LOG_DEBUG, "Parsed PPS id: %d. Copy already exists in parameter set\n", pps_id);
> > +    } else {
> > +        AVBufferRef *pps_buf = av_buffer_allocz(nal_size);
> > +        if (!pps_buf)
> > +            return AVERROR(ENOMEM);
> > +
> > +        /*copy raw data into vps_buf buffer and replace existing copy in ps*/
> > +        memcpy(pps_buf->data, nal->raw_data, nal_size);
> > +        av_buffer_unref(&param_ptr->raw_data);
> > +        param_ptr->raw_data = pps_buf;
> > +        param_ptr->ref      = sps_ref;
> > +    }
> > +    return 0;
> > +}
> > +
> >  static int hevc_extradata_to_annexb(AVBSFContext *ctx)
> >  {
> > -    GetByteContext gb;
> >      int length_size, num_arrays, i, j;
> >      int ret = 0;
> > +    GetByteContext gb;
> >  
> 
> Un
> 
> >      uint8_t *new_extradata = NULL;
> >      size_t   new_extradata_size = 0;
> > @@ -94,10 +287,95 @@ fail:
> >      return ret;
> >  }
> >  
> > +static int avcc_to_annexb(AVBSFContext *ctx, AVPacket **pkt, int nalu_length_size)
> > +{
> > +    int ret, i, prev_size;
> > +    GetByteContext gb;
> > +
> > +    AVPacket* pkt_out = av_packet_alloc();
> > +    if (!pkt_out)
> > +        return AVERROR(ENOMEM);
> 
> I initially wanted to write that all you change in this subfunction is
> the packet's data, so that you do not need to allocate a new packet at
> all; all you need is an AVBufferRef equivalent of av_grow_packet. As
> it happens, I, too, have encountered such a need (can be used
> advantageously in several bitstream filters) and I have implemented
> such a function
> (https://github.com/mkver/FFmpeg/commit/a119e056a27736feb88f191ddce93b0a0d8dfb41),
> but have not yet sent this stuff to the ML because I was interrupted
> by other stuff.
> But then I read on and saw that this whole function can be omitted.
> See below.
> 
> > +
> > +    ret = av_new_packet(pkt_out, 0);
> > +    if (ret < 0)
> > +        goto fail;
> > +
> > +    bytestream2_init(&gb, (*pkt)->data, (*pkt)->size);
> > +    while (bytestream2_get_bytes_left(&gb)) {
> > +        uint32_t nalu_size = 0;
> > +        for (i = 0; i < nalu_length_size; i++)
> > +            nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb);
> > +
> > +        if (SIZE_MAX - nalu_size < 4) {
> > +            ret = AVERROR_INVALIDDATA;
> > +            goto fail;
> > +        }
> > +
> > +        prev_size = pkt_out->size;
> > +        ret = av_grow_packet(pkt_out, 4 + nalu_size);
> > +        if (ret < 0)
> > +            goto fail;
> > +
> > +        AV_WB32(pkt_out->data + prev_size, 1);
> > +        ret = bytestream2_get_buffer(&gb, pkt_out->data + prev_size + 4, nalu_size);
> > +        if (ret != nalu_size)
> > +            av_log(ctx, AV_LOG_WARNING, "Corrupted avcc nal unit. Read %d/%d bytes\n", ret, nalu_size);
> > +    }
> > +
> > +    ret = av_packet_copy_props(pkt_out, *pkt);
> > +    if (ret < 0)
> > +        goto fail;
> > +
> > +    /*unref avcc version and replace pkt with pkt_out*/
> > +    av_packet_free(pkt);
> > +    *pkt = pkt_out;
> > +
> > +    return 0;
> > +
> > +fail:
> > +    av_packet_free(&pkt_out);
> > +    return ret;
> > +}
> > +
> > +static int update_sei(AVBufferRef **sei, H2645NAL *nal)
> > +{
> > +    AVBufferRef *tmp;
> > +
> > +    av_buffer_unref(sei);
> > +    tmp = av_buffer_alloc(nal->raw_size);
> > +    if (!tmp)
> > +        return AVERROR(ENOMEM);
> > +
> > +    memcpy(tmp->data, nal->raw_data, nal->raw_size);
> > +    *sei = tmp;
> > +
> > +    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_mp4toannexb_init(AVBSFContext *ctx)
> >  {
> >      HEVCBSFContext *s = ctx->priv_data;
> > -    int ret;
> > +    H2645Packet pkt;
> > +    int i, ret = 0;
> >  
> >      if (ctx->par_in->extradata_size < MIN_HEVCC_LENGTH ||
> >          AV_RB24(ctx->par_in->extradata) == 1           ||
> > @@ -110,7 +388,114 @@ static int hevc_mp4toannexb_init(AVBSFContext *ctx)
> >              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 (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) {
> > +                ret = update_sei(&s->ps.sei_prefix, 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_buffer_unref(&ps->vps_list[i].raw_data);
> > +    for (i = 0; i < FF_ARRAY_ELEMS(ps->sps_list); i++)
> > +       av_buffer_unref(&ps->sps_list[i].raw_data);
> > +    for (i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++)
> > +       av_buffer_unref(&ps->pps_list[i].raw_data);
> > +
> > +    av_buffer_unref(&ps->sei_prefix);
> > +}
> > +
> > +static void hevc_mp4toannexb_close(AVBSFContext *ctx)
> > +{
> > +    HEVCBSFContext *s = (HEVCBSFContext*)ctx->priv_data;
> > +    ps_uninit(&s->ps);
> > +}
> > +
> > +static int write_extradata(AVBSFContext *ctx, AVPacket *pkt_out, H2645NAL *nal_irap)
> > +{
> > +    int ref, ret, prev_size;
> > +    int new_extradata_size = 0;
> > +
> > +    HEVCBSFContext *s = (HEVCBSFContext*)ctx->priv_data;
> > +    ParamSets *ps     = &s->ps;
> > +    GetBitContext *gb = &nal_irap->gb;
> > +
> > +    /* currently active pps parameter set */
> > +    const Param *vps;
> > +    const Param *sps;
> > +    const Param *pps;
> > +
> > +    skip_bits1(gb); /*first_slice_ic_pic_flag*/
> > +    skip_bits1(gb); /*no_output_of_prior_pics_flag*/
> > +
> > +    ref = get_ue_golomb_long(gb);
> > +    if (ref >= HEVC_MAX_PPS_COUNT || ps->pps_list[ref].raw_data == NULL) {
> > +        av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d\n", ref);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +    pps                 = &ps->pps_list[ref];
> > +    new_extradata_size += pps->raw_data->size + 4;
> > +    ref                 = pps->ref;
> > +
> > +    if (ref >= HEVC_MAX_SPS_COUNT || ps->sps_list[ref].raw_data == NULL) {
> > +        av_log(ctx, AV_LOG_ERROR, "Invalid SPS: %d\n", ref);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +    sps                 = &ps->sps_list[ref];
> > +    new_extradata_size += sps->raw_data->size + 4;
> > +    ref                 = sps->ref;
> > +
> > +    if (ref >= HEVC_MAX_VPS_COUNT || ps->vps_list[ref].raw_data == NULL) {
> > +        av_log(ctx, AV_LOG_ERROR, "Invalid VPS: %d\n", ref);
> > +        return AVERROR_INVALIDDATA;
> >      }
> > +    vps                 = &ps->vps_list[ref];
> > +    new_extradata_size += vps->raw_data->size + 4;
> > +
> > +    if (ps->sei_prefix)
> > +        new_extradata_size += ps->sei_prefix->size + 4;
> > +
> > +    prev_size = pkt_out->size;
> > +    ret = av_grow_packet(pkt_out, new_extradata_size);
> > +    if (ret < 0)
> > +        return AVERROR(ENOMEM);
> > +
> > +    WRITE_NAL(pkt_out, prev_size, vps->raw_data);
> > +    WRITE_NAL(pkt_out, prev_size, sps->raw_data);
> > +    WRITE_NAL(pkt_out, prev_size, pps->raw_data);
> > +
> > +    if (ps->sei_prefix)
> > +        WRITE_NAL(pkt_out, prev_size, ps->sei_prefix);
> >  
> >      return 0;
> >  }
> > @@ -119,62 +504,73 @@ 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, prev_size;
> > +    int ret = 0;
> > +    int first_irap = 0;
> >  
> >      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);
> > +    /*convert packet from avcc to annexb*/
> > +    ret = avcc_to_annexb(ctx, &in, s->length_size);
> > +    if (ret < 0) {
> > +        av_packet_free(&in);
> > +        return ret;
> > +    }
> >  
> > -    while (bytestream2_get_bytes_left(&gb)) {
> > -        uint32_t nalu_size = 0;
> > -        int      nalu_type;
> > -        int is_irap, add_extradata, extra_size, prev_size;
> > +    /*segment annexb packet into nal units*/
> > +    memset(&pkt, 0, sizeof(H2645Packet));
> > +    ret = ff_h2645_packet_split(&pkt, in->data, in->size, ctx, 0, 0, AV_CODEC_ID_HEVC, 1, 0);
> 
> ff_h2645_packet_split is perfectly fine with mp4-style data. There is
> no need to convert it to annex b before this step. Avoiding this has
> the benefit of it not mishandling trailing 0x00 in this case.
> ret = ff_h2645_packet_split(&pkt, in->data, in->size, ctx, 1,
> s->length_size, AV_CODEC_ID_HEVC, 1, 0);
> should do the trick.

I added this change.

> That should already provide a pretty good speed boost. Another
> possible way for speed improvement is not using ff_h2645_packet_split,
> but using ff_h2645_extract_rbsp on the NAL units that we are
> interested in and simply copy the other NAL units. (Looking for 0x03
> escapes is expensive.)
> 

But still working on this version.

> > +    if (ret < 0)
> > +        goto done;
> >  
> > -        for (i = 0; i < s->length_size; i++)
> > -            nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb);
> > +    for (i = 0; i < pkt.nb_nals; ++i) {
> >  
> > -        nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f;
> > +        H2645NAL *nal = &pkt.nals[i];
> >  
> > -        /* 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;
> > +        /*current segmentation algorithm includes next 0x00 from next nal unit*/
> > +        if (nal->raw_data[nal->raw_size - 1] == 0x00)
> > +            nal->raw_size--;
> >  
> > -        if (SIZE_MAX - nalu_size < 4 ||
> > -            SIZE_MAX - 4 - nalu_size < extra_size) {
> > -            ret = AVERROR_INVALIDDATA;
> > -            goto fail;
> > +        if (IS_PARAMSET(nal)) {
> > +            ret = update_paramset(ctx, nal);
> > +            if (ret < 0)
> > +                goto done;
> > +            continue;
> >          }
> >  
> > -        prev_size = out->size;
> > +        if (nal->type == HEVC_NAL_SEI_PREFIX) {
> > +            ret = update_sei(&s->ps.sei_prefix, nal);
> > +            if (ret < 0)
> > +                goto done;
> > +            continue;
> > +        }
> >  
> > -        ret = av_grow_packet(out, 4 + nalu_size + extra_size);
> > -        if (ret < 0)
> > -            goto fail;
> > +        if (!first_irap && IS_IRAP(nal)) {
> > +            ret = write_extradata(ctx, out, nal);
> > +            if (ret < 0)
> > +                goto done;
> > +            first_irap = 1;
> > +        }
> >  
> > -        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);
> > +        /*write to output packet*/
> > +        prev_size = out->size;
> > +        av_grow_packet(out, nal->raw_size + 4);
> > +        WRITE_NAL(out, prev_size, nal);
> 
> The WRITE_NAL macro as it exists is not applicable here: It will write
> the NAL's data, which is unescaped (the 0x03 escape bytes have been
> removed), but you should write the raw data (including the 0x03).
> Doing it this way will result in corrupted output.
> 
> >      }
> >  
> > -    ret = av_packet_copy_props(out, in);
> > -    if (ret < 0)
> > -        goto fail;
> > +done:
> > +    ff_h2645_packet_uninit(&pkt);
> >  
> > -fail:
> >      if (ret < 0)
> >          av_packet_unref(out);
> >      av_packet_free(&in);
> > @@ -190,6 +586,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..46f130f4d1 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 = d41d8cd98f00b204e9800998ecf8427e
> >  
> >  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
> > 
> 
> - Andreas

Andriy

Patch hide | download patch | download mbox

diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c
index 09bce5b34c..7f3d68252d 100644
--- a/libavcodec/hevc_mp4toannexb_bsf.c
+++ b/libavcodec/hevc_mp4toannexb_bsf.c
@@ -28,19 +28,212 @@ 
 #include "bsf.h"
 #include "bytestream.h"
 #include "hevc.h"
-
-#define MIN_HEVCC_LENGTH 23
+#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)
+
+#define WRITE_NAL(pkt, prev_size, in_buffer) do {                            \
+    AV_WB32((pkt)->data + (prev_size), 1);                                   \
+    prev_size += 4;                                                          \
+    memcpy((pkt)->data + (prev_size), (in_buffer)->data, (in_buffer)->size); \
+    prev_size += (in_buffer)->size;                                          \
+} while (0)
+
+typedef struct Param {
+    AVBufferRef *raw_data; /*store a copy of the raw data to construct extradata*/
+    int ref;  /*stores the ref of the higher level parameter set*/
+} 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];
+
+    AVBufferRef *sei_prefix;
+} ParamSets;
 
 typedef struct HEVCBSFContext {
-    uint8_t  length_size;
-    int      extradata_parsed;
+    uint8_t      length_size;
+    int          extradata_parsed;
+    ParamSets    ps; /*make own of version of HEVCParamSets store copy of the bytestream*/
 } HEVCBSFContext;
 
+
+static int parse_vps(AVBSFContext* ctx, H2645NAL *nal)
+{
+    int vps_id = 0;
+    Param *param_ptr;
+
+    HEVCBSFContext *s = ctx->priv_data;
+    ParamSets *ps     = &s->ps;
+
+    GetBitContext *gb = &nal->gb;
+    int nal_size      = nal->raw_size;
+
+    vps_id = get_bits(gb, 4);
+    if (vps_id >= HEVC_MAX_VPS_COUNT) {
+        av_log(ctx, AV_LOG_ERROR, "VPS id out of range: %d\n", vps_id);
+        return AVERROR_INVALIDDATA;
+    }
+
+    param_ptr = &ps->vps_list[vps_id];
+    /*init raw_data if needed*/
+    if (!param_ptr->raw_data) {
+        param_ptr->raw_data = av_buffer_allocz(nal_size);
+        param_ptr->ref      = 0;
+        if (!param_ptr->raw_data)
+            return AVERROR(ENOMEM);
+    }
+
+    if (param_ptr->raw_data && param_ptr->raw_data->size == nal_size &&
+        !memcmp(param_ptr->raw_data->data, nal->raw_data, nal_size)) {
+        av_log(ctx, AV_LOG_DEBUG, "Parsed VPS id: %d. Copy already exists in parameter set\n", vps_id);
+    } else {
+        AVBufferRef *vps_buf = av_buffer_allocz(nal_size);
+        if (!vps_buf)
+            return AVERROR(ENOMEM);
+
+        /*copy raw data into vps_buf buffer and replace existing copy in ps*/
+        memcpy(vps_buf->data, nal->raw_data, nal_size);
+        av_buffer_unref(&param_ptr->raw_data);
+        param_ptr->raw_data = vps_buf;
+    }
+    return 0;
+}
+
+static int parse_sps(AVBSFContext *ctx, H2645NAL *nal)
+{
+    int sps_id, vps_ref, max_sub_layers_minus1;
+    int i;
+    Param *param_ptr;
+
+    HEVCBSFContext *s = (HEVCBSFContext*)ctx->priv_data;
+    ParamSets *ps     = &s->ps;
+
+    GetBitContext *gb = &nal->gb;
+    int nal_size      = nal->raw_size;
+
+    uint8_t sub_layer_profile_present_flag[HEVC_MAX_SUB_LAYERS];
+    uint8_t sub_layer_level_present_flag[HEVC_MAX_SUB_LAYERS];
+
+    vps_ref = get_bits(gb, 4);
+    if (vps_ref >= HEVC_MAX_VPS_COUNT) {
+        av_log(ctx, AV_LOG_ERROR, "VPS id out of range: %d\n", vps_ref);
+        return AVERROR_INVALIDDATA;
+    }
+
+    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_long(gb);
+    if (sps_id >= HEVC_MAX_SPS_COUNT) {
+        av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_id);
+        return AVERROR_INVALIDDATA;
+    }
+
+    param_ptr = &ps->sps_list[sps_id];
+    if (!param_ptr->raw_data) {
+        param_ptr->raw_data = av_buffer_allocz(nal_size);
+        param_ptr->ref      = 0;
+        if (!param_ptr->raw_data)
+            return AVERROR(ENOMEM);
+    }
+
+    if (param_ptr->raw_data && param_ptr->raw_data->size == nal_size &&
+        !memcmp(param_ptr->raw_data->data, nal->raw_data, nal_size)) {
+        av_log(ctx, AV_LOG_DEBUG, "Parsed SPS id: %d. Copy already exists in parameter set\n", sps_id);
+    } else {
+        AVBufferRef *sps_buf = av_buffer_allocz(nal_size);
+        if (!sps_buf)
+            return AVERROR(ENOMEM);
+
+        /*copy raw data into vps_buf buffer and replace existing copy in ps*/
+        memcpy(sps_buf->data, nal->raw_data, nal_size);
+        av_buffer_unref(&param_ptr->raw_data);
+        param_ptr->raw_data = sps_buf;
+        param_ptr->ref      = vps_ref;
+    }
+    return 0;
+}
+
+static int parse_pps(AVBSFContext *ctx, H2645NAL *nal)
+{
+    int pps_id, sps_ref;
+    Param *param_ptr;
+
+    HEVCBSFContext *s = ctx->priv_data;
+    ParamSets *ps     = &s->ps;
+
+    GetBitContext *gb = &nal->gb;
+    int nal_size      = nal->raw_size;
+
+    pps_id = get_ue_golomb_long(gb);
+    if (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_long(gb);
+    if (sps_ref >= HEVC_MAX_SPS_COUNT) {
+        av_log(ctx, AV_LOG_ERROR, "SPS id out of range: %d\n", sps_ref);
+        return AVERROR_INVALIDDATA;
+    }
+
+    param_ptr = &ps->pps_list[pps_id];
+    if (!param_ptr->raw_data) {
+        param_ptr->raw_data = av_buffer_allocz(nal_size);
+        param_ptr->ref      = 0;
+        if (!param_ptr->raw_data)
+            return AVERROR(ENOMEM);
+    }
+
+    if (param_ptr->raw_data && param_ptr->raw_data->size == nal_size &&
+        !memcmp(param_ptr->raw_data->data, nal->raw_data, nal_size)) {
+        av_log(ctx, AV_LOG_DEBUG, "Parsed PPS id: %d. Copy already exists in parameter set\n", pps_id);
+    } else {
+        AVBufferRef *pps_buf = av_buffer_allocz(nal_size);
+        if (!pps_buf)
+            return AVERROR(ENOMEM);
+
+        /*copy raw data into vps_buf buffer and replace existing copy in ps*/
+        memcpy(pps_buf->data, nal->raw_data, nal_size);
+        av_buffer_unref(&param_ptr->raw_data);
+        param_ptr->raw_data = pps_buf;
+        param_ptr->ref      = sps_ref;
+    }
+    return 0;
+}
+
 static int hevc_extradata_to_annexb(AVBSFContext *ctx)
 {
-    GetByteContext gb;
     int length_size, num_arrays, i, j;
     int ret = 0;
+    GetByteContext gb;
 
     uint8_t *new_extradata = NULL;
     size_t   new_extradata_size = 0;
@@ -94,10 +287,95 @@  fail:
     return ret;
 }
 
+static int avcc_to_annexb(AVBSFContext *ctx, AVPacket **pkt, int nalu_length_size)
+{
+    int ret, i, prev_size;
+    GetByteContext gb;
+
+    AVPacket* pkt_out = av_packet_alloc();
+    if (!pkt_out)
+        return AVERROR(ENOMEM);
+
+    ret = av_new_packet(pkt_out, 0);
+    if (ret < 0)
+        goto fail;
+
+    bytestream2_init(&gb, (*pkt)->data, (*pkt)->size);
+    while (bytestream2_get_bytes_left(&gb)) {
+        uint32_t nalu_size = 0;
+        for (i = 0; i < nalu_length_size; i++)
+            nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb);
+
+        if (SIZE_MAX - nalu_size < 4) {
+            ret = AVERROR_INVALIDDATA;
+            goto fail;
+        }
+
+        prev_size = pkt_out->size;
+        ret = av_grow_packet(pkt_out, 4 + nalu_size);
+        if (ret < 0)
+            goto fail;
+
+        AV_WB32(pkt_out->data + prev_size, 1);
+        ret = bytestream2_get_buffer(&gb, pkt_out->data + prev_size + 4, nalu_size);
+        if (ret != nalu_size)
+            av_log(ctx, AV_LOG_WARNING, "Corrupted avcc nal unit. Read %d/%d bytes\n", ret, nalu_size);
+    }
+
+    ret = av_packet_copy_props(pkt_out, *pkt);
+    if (ret < 0)
+        goto fail;
+
+    /*unref avcc version and replace pkt with pkt_out*/
+    av_packet_free(pkt);
+    *pkt = pkt_out;
+
+    return 0;
+
+fail:
+    av_packet_free(&pkt_out);
+    return ret;
+}
+
+static int update_sei(AVBufferRef **sei, H2645NAL *nal)
+{
+    AVBufferRef *tmp;
+
+    av_buffer_unref(sei);
+    tmp = av_buffer_alloc(nal->raw_size);
+    if (!tmp)
+        return AVERROR(ENOMEM);
+
+    memcpy(tmp->data, nal->raw_data, nal->raw_size);
+    *sei = tmp;
+
+    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_mp4toannexb_init(AVBSFContext *ctx)
 {
     HEVCBSFContext *s = ctx->priv_data;
-    int ret;
+    H2645Packet pkt;
+    int i, ret = 0;
 
     if (ctx->par_in->extradata_size < MIN_HEVCC_LENGTH ||
         AV_RB24(ctx->par_in->extradata) == 1           ||
@@ -110,7 +388,114 @@  static int hevc_mp4toannexb_init(AVBSFContext *ctx)
             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 (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) {
+                ret = update_sei(&s->ps.sei_prefix, 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_buffer_unref(&ps->vps_list[i].raw_data);
+    for (i = 0; i < FF_ARRAY_ELEMS(ps->sps_list); i++)
+       av_buffer_unref(&ps->sps_list[i].raw_data);
+    for (i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++)
+       av_buffer_unref(&ps->pps_list[i].raw_data);
+
+    av_buffer_unref(&ps->sei_prefix);
+}
+
+static void hevc_mp4toannexb_close(AVBSFContext *ctx)
+{
+    HEVCBSFContext *s = (HEVCBSFContext*)ctx->priv_data;
+    ps_uninit(&s->ps);
+}
+
+static int write_extradata(AVBSFContext *ctx, AVPacket *pkt_out, H2645NAL *nal_irap)
+{
+    int ref, ret, prev_size;
+    int new_extradata_size = 0;
+
+    HEVCBSFContext *s = (HEVCBSFContext*)ctx->priv_data;
+    ParamSets *ps     = &s->ps;
+    GetBitContext *gb = &nal_irap->gb;
+
+    /* currently active pps parameter set */
+    const Param *vps;
+    const Param *sps;
+    const Param *pps;
+
+    skip_bits1(gb); /*first_slice_ic_pic_flag*/
+    skip_bits1(gb); /*no_output_of_prior_pics_flag*/
+
+    ref = get_ue_golomb_long(gb);
+    if (ref >= HEVC_MAX_PPS_COUNT || ps->pps_list[ref].raw_data == NULL) {
+        av_log(ctx, AV_LOG_ERROR, "Invalid PPS: %d\n", ref);
+        return AVERROR_INVALIDDATA;
+    }
+    pps                 = &ps->pps_list[ref];
+    new_extradata_size += pps->raw_data->size + 4;
+    ref                 = pps->ref;
+
+    if (ref >= HEVC_MAX_SPS_COUNT || ps->sps_list[ref].raw_data == NULL) {
+        av_log(ctx, AV_LOG_ERROR, "Invalid SPS: %d\n", ref);
+        return AVERROR_INVALIDDATA;
+    }
+    sps                 = &ps->sps_list[ref];
+    new_extradata_size += sps->raw_data->size + 4;
+    ref                 = sps->ref;
+
+    if (ref >= HEVC_MAX_VPS_COUNT || ps->vps_list[ref].raw_data == NULL) {
+        av_log(ctx, AV_LOG_ERROR, "Invalid VPS: %d\n", ref);
+        return AVERROR_INVALIDDATA;
     }
+    vps                 = &ps->vps_list[ref];
+    new_extradata_size += vps->raw_data->size + 4;
+
+    if (ps->sei_prefix)
+        new_extradata_size += ps->sei_prefix->size + 4;
+
+    prev_size = pkt_out->size;
+    ret = av_grow_packet(pkt_out, new_extradata_size);
+    if (ret < 0)
+        return AVERROR(ENOMEM);
+
+    WRITE_NAL(pkt_out, prev_size, vps->raw_data);
+    WRITE_NAL(pkt_out, prev_size, sps->raw_data);
+    WRITE_NAL(pkt_out, prev_size, pps->raw_data);
+
+    if (ps->sei_prefix)
+        WRITE_NAL(pkt_out, prev_size, ps->sei_prefix);
 
     return 0;
 }
@@ -119,62 +504,73 @@  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, prev_size;
+    int ret = 0;
+    int first_irap = 0;
 
     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);
+    /*convert packet from avcc to annexb*/
+    ret = avcc_to_annexb(ctx, &in, s->length_size);
+    if (ret < 0) {
+        av_packet_free(&in);
+        return ret;
+    }
 
-    while (bytestream2_get_bytes_left(&gb)) {
-        uint32_t nalu_size = 0;
-        int      nalu_type;
-        int is_irap, add_extradata, extra_size, prev_size;
+    /*segment annexb packet into nal units*/
+    memset(&pkt, 0, sizeof(H2645Packet));
+    ret = ff_h2645_packet_split(&pkt, in->data, in->size, ctx, 0, 0, AV_CODEC_ID_HEVC, 1, 0);
+    if (ret < 0)
+        goto done;
 
-        for (i = 0; i < s->length_size; i++)
-            nalu_size = (nalu_size << 8) | bytestream2_get_byte(&gb);
+    for (i = 0; i < pkt.nb_nals; ++i) {
 
-        nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f;
+        H2645NAL *nal = &pkt.nals[i];
 
-        /* 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;
+        /*current segmentation algorithm includes next 0x00 from next nal unit*/
+        if (nal->raw_data[nal->raw_size - 1] == 0x00)
+            nal->raw_size--;
 
-        if (SIZE_MAX - nalu_size < 4 ||
-            SIZE_MAX - 4 - nalu_size < extra_size) {
-            ret = AVERROR_INVALIDDATA;
-            goto fail;
+        if (IS_PARAMSET(nal)) {
+            ret = update_paramset(ctx, nal);
+            if (ret < 0)
+                goto done;
+            continue;
         }
 
-        prev_size = out->size;
+        if (nal->type == HEVC_NAL_SEI_PREFIX) {
+            ret = update_sei(&s->ps.sei_prefix, nal);
+            if (ret < 0)
+                goto done;
+            continue;
+        }
 
-        ret = av_grow_packet(out, 4 + nalu_size + extra_size);
-        if (ret < 0)
-            goto fail;
+        if (!first_irap && IS_IRAP(nal)) {
+            ret = write_extradata(ctx, out, nal);
+            if (ret < 0)
+                goto done;
+            first_irap = 1;
+        }
 
-        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);
+        /*write to output packet*/
+        prev_size = out->size;
+        av_grow_packet(out, nal->raw_size + 4);
+        WRITE_NAL(out, prev_size, nal);
     }
 
-    ret = av_packet_copy_props(out, in);
-    if (ret < 0)
-        goto fail;
+done:
+    ff_h2645_packet_uninit(&pkt);
 
-fail:
     if (ret < 0)
         av_packet_unref(out);
     av_packet_free(&in);
@@ -190,6 +586,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..46f130f4d1 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 = d41d8cd98f00b204e9800998ecf8427e
 
 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