[FFmpeg-devel] avformat/mxfenc: SMPTE RDD 48:2018 Amd 1:2022 (FFV1 in MXF) support

[Jerome Martinez]

On 04/04/2023 19:32, Michael Niedermayer wrote:
> On Tue, Apr 04, 2023 at 04:57:03PM +0200, Jerome Martinez wrote:
>> On 04/04/2023 16:43, Michael Niedermayer wrote:
>>> On Mon, Apr 03, 2023 at 12:07:06AM +0200, Jerome Martinez wrote:
>>>> On 02/04/2023 22:07, Michael Niedermayer wrote:
>>>>
>>>> +        if (f->version == 4 && f->micro_version > 2) {
>>>> +            av_log(f->avctx, AV_LOG_ERROR, "unsupported version 4 micro_version %d\n",
>>>> +                f->micro_version);
>>>> +            return AVERROR_PATCHWELCOME;
>>>> +        }
>>>>        }
>>>>        f->ac = get_symbol(c, state, 0);
>>> you do not know if the decoder will have any problem with these files
>>
>> But you don't don't if the decoder will have no problem, it seems safer to
>> me to reject something we are not sure to support.
> "each new micro-version after this first stable variant is compatible with the previous micro-version: decoders **SHOULD NOT** reject FFV1 bitstreams due to an unknown micro-version equal or above the micro-version considered as stable."

This sentence is about a stable version, and version 4 is not stable, so 
FFV1 version 4 micro_version 3 is not necessarily compatible with FFV1 
version 4 micro_version 2, and so on, until we freeze the micro-version 
considered as stable. It is actually same for FFV1 version 3 e.g. "if 
(f->version == 3 && f->micro_version > 1 || f->version > 3) 
get_rac(&fs->c, (uint8_t[]) { 129 });" seems to say that version 3 
micro_version 2 is not compatible with version 3 micro_version 1 (and 
acceptable, only version 3 micro_version 5 and above must be compatible 
with version 3 micro_version 4 because micro_version 4 is the one 
flagged as stable).
Reason I suggested this micro_version check for version 4 (I didn't add 
it for version 3 because version 3 has a micro-version considered as 
stable).
Anyway, it is not important to me, just apply the patch you prefer.


>
>
> [...]
>>   libavcodec/ffv1dec.c |    5 +++++
>>   libavformat/mxfenc.c |    4 ++++
>>   2 files changed, 9 insertions(+)
>> 9b094eb0bd0888725a4a3fac925ef1fa733a48c3  0001-avcodec-ffv1dec-reject-unsupported-ffv1-versions.patch
>>  From dc0382709e548ef2514198bc866028066134d33e Mon Sep 17 00:00:00 2001
>> From: Jerome Martinez <jerome at mediaarea.net>
>> Date: Mon, 3 Apr 2023 00:04:53 +0200
>> Subject: [PATCH] avcodec/ffv1dec: reject unsupported ffv1 versions
>>
>> And add similar check in libavformat/mxfenc
>> ---
>>   libavcodec/ffv1dec.c | 5 +++++
>>   libavformat/mxfenc.c | 4 ++++
>>   2 files changed, 9 insertions(+)
> the patch is mostly ok
> iam a bit undecided if a decoder change and a muxer bugfix belong in the
> same patch, so do as you prefer

I think that both parts are about the same thing (and in the future both 
should be changed at the same time) but it is really subjective, let me 
know if you prefer that I send 2 separated patches and I'll do it, else 
please apply one of the already proposed patch.

Jérôme