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

[Baptiste Coudurier]

> On Apr 9, 2019, at 4:51 PM, James Almer <jamrial at gmail.com> wrote:
> 
> On 4/9/2019 8:22 PM, Baptiste Coudurier wrote:
>> Hi James,
>> 
>>> On Apr 9, 2019, at 4:09 PM, James Almer <jamrial at gmail.com> wrote:
>>> 
>>>> […]
>>> 
>>> Ok so, the only fields you need from the sps are constraint_set_flags,
>>> frame_mbs_only_flag, bit_depth_luma, profile_idc, level_idc and sar,
>>> meaning you don't even need to parse the sps until the very end (sar is
>>> the furthest value, and right at the beginning of vui_parameters).
>>> I'd very much prefer if we can avoid making
>>> ff_h264_decode_seq_parameter_set() avpriv and with it H264ParamSets part
>>> of the ABI, so i think it's worth copying the required bitstream parsing
>>> code at least until the beginning of vui_parameters. It was done for
>>> hevc and av1, so it can be done for h264 as well.
>> 
>> Since vui parsing is needed, that would be the entire "ff_h264_decode_seq_parameter_set()” function.
> 
> If you only need up to sar, then yo don't need to parse the entire VUI.

It still requires copying the entire ff_h264_decode_seq_parameter_set(), even though the sub functions are not needed. 

>> 
>> I disagree with the copying. I also disagree with the copying for AVC1 and HEVC btw.
> 
> Using avpriv opens a whole can of worms that will be unpredictable in
> the future. Just look at recent commits like
> f4ea930a119298c6110ee4e3d24219a66e27e230 that started storing previously
> skipped values. If hevc_pc was made avpriv for isombff/matroska muxing
> purposes and its structs part of the ABI, this wouldn't have been
> possible until a major bump.

When did the sharing policy between avformat and avcodec (in the sense avformat using avcodec) change ?
It seems perfectly natural to me to have libavformat require the libavcodec version that it was compiled with,
and check it at runtime.

> 
> sps parsing can and should be done in libavformat for this. Otherwise
> you'll be constraining future development in other areas. So please,
> take libavcodec/cbs_h264* and libavformat/av1.c as basis and write a
> simple sps parser that takes the raw bitstream values and does nothing
> else. You only need six values.

I disagree with the copying and we will need more opinions on this to settle
the argument, maybe a vote.

— 
Baptiste