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

[Baptiste Coudurier]

Hi Tomas

> On May 12, 2019, at 6:47 AM, Tomas Härdin <tjoppen at acc.umu.se> wrote:
> 
> 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?

get_bits doesn’t work for n == 0

>> 
>> +
>> +    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?

That’s how it’s done in h264_ps.c currently.
 
>> +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.

Yes, it does

>> +
>>  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

Yes, it will get caught when this happens.

— 
Baptiste