[FFmpeg-devel] [PATCH] avformat/mxfenc: support XAVC long gop

[Tomas Härdin]

fre 2019-05-10 klockan 08:50 -0700 skrev Baptiste Coudurier:
> +static inline int get_ue_golomb(GetBitContext *gb) {
> +    int i, v;
> +    for (i = 0; i < 32 && !get_bits1(gb); i++)
> +        ;
> +    for (v = 1; i--;)
> +        v = (v << 1) | get_bits1(gb);

Isn't there already a function to get variable number of bits?

> +    return v - 1;
> +}

> +
> +static inline int get_se_golomb(GetBitContext *gb) {
> +    int v = get_ue_golomb(gb) + 1;
> +    int sign = -(v & 1);
> +    return ((v >> 1) ^ sign) - sign;
> +}
> +
> +H264SequenceParameterSet *ff_avc_decode_sps(const uint8_t *buf, int buf_size)
> +{
> +    int i, j, ret, rbsp_size, aspect_ratio_idc, pic_order_cnt_type;
> +    int num_ref_frames_in_pic_order_cnt_cycle;
> +    int delta_scale, lastScale = 8, nextScale = 8;
> +    int sizeOfScalingList;
> +    H264SequenceParameterSet *sps = NULL;
> +    GetBitContext gb;
> +    uint8_t *rbsp_buf;
> +
> +    rbsp_buf = ff_nal_unit_extract_rbsp(buf, buf_size, &rbsp_size, 0);
> +    if (!rbsp_buf)
> +        return NULL;
> +
> +    ret = init_get_bits8(&gb, rbsp_buf, rbsp_size);
> +    if (ret < 0)
> +        goto end;
> +
> +    sps = av_mallocz(sizeof(*sps));
> +    if (!sps)
> +        goto end;
> +
> +    sps->profile_idc = get_bits(&gb, 8);
> +    sps->constraint_set_flags |= get_bits1(&gb) << 0; // constraint_set0_flag
> +    sps->constraint_set_flags |= get_bits1(&gb) << 1; // constraint_set1_flag
> +    sps->constraint_set_flags |= get_bits1(&gb) << 2; // constraint_set2_flag
> +    sps->constraint_set_flags |= get_bits1(&gb) << 3; // constraint_set3_flag
> +    sps->constraint_set_flags |= get_bits1(&gb) << 4; // constraint_set4_flag
> +    sps->constraint_set_flags |= get_bits1(&gb) << 5; // constraint_set5_flag

Why not just get 6 bits at once?

> +    skip_bits(&gb, 2); // reserved_zero_2bits
> +    sps->level_idc = get_bits(&gb, 8);
> +    sps->id = get_ue_golomb(&gb);
> +
> +    if (sps->profile_idc == 100 || sps->profile_idc == 110 ||
> +        sps->profile_idc == 122 || sps->profile_idc == 244 || sps->profile_idc ==  44 ||
> +        sps->profile_idc ==  83 || sps->profile_idc ==  86 || sps->profile_idc == 118 ||
> +        sps->profile_idc == 128 || sps->profile_idc == 138 || sps->profile_idc == 139 ||
> +        sps->profile_idc == 134) {

Maybe put these in a table instead? I guess it works this way, just a
bit verbose. They could do with sorting, unless there's a specific
reason for this ordering
 
> +static void mxf_write_local_tags(AVIOContext *pb, const MXFLocalTagPair *local_tags, int count)
> +{
> +    int i;
> +    for (i = 0; i < count; i++) {
> +        avio_wb16(pb, local_tags[i].local_tag);
> +        avio_write(pb, local_tags[i].uid, 16);
> +    }
> +}

This function could be used to simplify mxf_write_primer_pack(). But
that probably belongs in a separate patch.

> +
>  static void mxf_write_primer_pack(AVFormatContext *s)
>  {
>      MXFContext *mxf = s->priv_data;
>      AVIOContext *pb = s->pb;
>      int local_tag_number, i = 0;
> +    int avc_tags_count = 0;
>  
>      local_tag_number = FF_ARRAY_ELEMS(mxf_local_tag_batch);
>      local_tag_number += mxf->store_user_comments * FF_ARRAY_ELEMS(mxf_user_comments_local_tag);
>  
> +    for (i = 0; i < s->nb_streams; i++) {
> +        MXFStreamContext *sc = s->streams[i]->priv_data;
> +        if (s->streams[i]->codecpar->codec_id == AV_CODEC_ID_H264 && !sc->avc_intra) {
> +            avc_tags_count = FF_ARRAY_ELEMS(mxf_avc_subdescriptor_local_tags);
> +            local_tag_number += avc_tags_count;

This will output a broken file if there's more than one XAVC stream.
Not possible now I think, but will be a problem is someone decides to
give higher operational patterns a try

> +        }
> +    }
> +
>      avio_write(pb, primer_pack_key, 16);
>      klv_encode_ber_length(pb, local_tag_number * 18 + 8);
>  
> @@ -608,6 +637,8 @@ static void mxf_write_primer_pack(AVFormatContext *s)
>              avio_wb16(pb, mxf_user_comments_local_tag[i].local_tag);
>              avio_write(pb, mxf_user_comments_local_tag[i].uid, 16);
>          }
> +    if (avc_tags_count > 0)
> +        mxf_write_local_tags(pb, mxf_avc_subdescriptor_local_tags, avc_tags_count);
>  }

No other comments for now. Don't know enough about SPS/PPS stuff to
comment on specifics about that.

/Tomas