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

[James Almer]

On 4/9/2019 8:59 PM, Baptiste Coudurier wrote:
> 
>> 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.

More than one ffmpeg release will ship with libavcodec/libavformat
sharing the same soname. Backwards compatibility is expected, hence the
split between major, minor and micro, as you're meant to be able to drop
a newer libavcodec library, say for example ffmpeg 4.1's
libavcodec.58.so, and it should work with an existing libavformat.so.58
that was linked against ffmpeg 4.0's libavcodec.58.so.

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

It's the cleanest way and the one used in all cases previous to this
one. There has been work during the last bump to remove internal structs
from the ABI precisely because they constrained development. Among them
was GetBitsContext, which you're now trying to reintroduce and thus
invalidate all that previous cleaning work, and only to get six values
for a single muxer.

Two thirds of SPS is not hard to implement, so i really don't understand
why you're so adamantly against it.

> 
> — 
> Baptiste
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>