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

The arbitrary short element codes are the ones used by another muxer ( 
files available at 
https://www.digitizationguidelines.gov/guidelines/MXF_app_sampleFiles.html#2022 
)

The support of RGBA descriptor is added, mainly by disabling in the CDCI 
descriptor related code the elements not in the Generic picture 
descriptor, and could be in a separated dedicated patch (move of Generic 
picture descriptor code in a dedicated function?).

Tested with:
ffmpeg -f lavfi -i testsrc=duration=10:size=ntsc:rate=ntsc -field_order 
bb -c:v ffv1 -level 0 test_ffv1_ntsc.mxf
ffmpeg -f lavfi -i testsrc=duration=10:size=pal:rate=pal -field_order tt 
-c:v ffv1 -level 3 test_ffv1_pal.mxf
ffmpeg -f lavfi -i testsrc=duration=10:size=1920x1080 -pix_fmt yuv422p10 
-c:v ffv1 -level 3 test_ffv1_hd.mxf
From 2e38dc0a114a1706f6d108ea9ca3e86083bfc2eb Mon Sep 17 00:00:00 2001
From: Jerome Martinez <jerome@mediaarea.net>
Date: Wed, 4 Jan 2023 13:56:21 +0100
Subject: [PATCH] avformat/mxfenc: SMPTE RDD 48:2018 Amd 1:2022 support

---
 libavformat/Makefile         |   3 +-
 libavformat/mxfenc.c         | 163 ++++++++++++++++++++++++++++++++++++++++++-
 libavformat/rangecoder_dec.c |   1 +
 3 files changed, 164 insertions(+), 3 deletions(-)
 create mode 100644 libavformat/rangecoder_dec.c

lör 2023-01-14 klockan 16:45 +0100 skrev Jerome Martinez:
> The arbitrary short element codes are the ones used by another muxer
> ( 
> files available at 
> https://www.digitizationguidelines.gov/guidelines/MXF_app_sampleFiles.html#2022
>  
> )
> 
> The support of RGBA descriptor is added, mainly by disabling in the
> CDCI 
> descriptor related code the elements not in the Generic picture 
> descriptor, and could be in a separated dedicated patch (move of
> Generic 
> picture descriptor code in a dedicated function?).
> 
> Tested with:
> ffmpeg -f lavfi -i testsrc=duration=10:size=ntsc:rate=ntsc -
> field_order 
> bb -c:v ffv1 -level 0 test_ffv1_ntsc.mxf
> ffmpeg -f lavfi -i testsrc=duration=10:size=pal:rate=pal -field_order
> tt 
> -c:v ffv1 -level 3 test_ffv1_pal.mxf
> ffmpeg -f lavfi -i testsrc=duration=10:size=1920x1080 -pix_fmt
> yuv422p10 
> -c:v ffv1 -level 3 test_ffv1_hd.mxf

JPEG2000 will also need an RGBA descriptor filled out, might be good to
prepare for that.

The ffv1 parsing code in this patch makes me nervous. Isn't the version
available in metadata?

> +        ff_build_rac_states(&c, 0.05 * (1LL << 32), 256 - 8);

(1LL << 32) / 20 ?

/Tomas

On 16/01/2023 15:00, Tomas Härdin wrote:
> JPEG2000 will also need an RGBA descriptor filled out, might be good to
> prepare for that.

this was the idea behind the way it is coded, so there is only a new 
mxf_write_jpeg2000_desc function to write, like the one for FFV1 i.e.
static void mxf_write_jpeg2000_desc(AVFormatContext *s, AVStream *st)
     is_rgb = desc->flags & AV_PIX_FMT_FLAG_RGB;
     pos = mxf_write_cdci_common(s, st, is_rgb ? mxf_rgba_descriptor_key 
: mxf_cdci_descriptor_key);
}

to add.


>
> The ffv1 parsing code in this patch makes me nervous. Isn't the version
> available in metadata?

I implemented a way similar to e.g. mxf_parse_mpeg2_frame by parsing a 
bit the frame.
version and micro_version are available in FFV1Context so could be used 
but I don't know how to get FFV1Context from AVStream or other, need 
help there.


>
>> +        ff_build_rac_states(&c, 0.05 * (1LL << 32), 256 - 8);
> (1LL << 32) / 20 ?

Could be, I don't really care, but this line is copied from ffv1dec.c, I 
think it may be relevant to keep the exact same code for the exact same 
purpose.
Would be no more relevant if version and micro_version can be taken from 
FFV1Context.

Jérôme

mån 2023-01-16 klockan 15:17 +0100 skrev Jerome Martinez:
> On 16/01/2023 15:00, Tomas Härdin wrote:
> > 
> > > +        ff_build_rac_states(&c, 0.05 * (1LL << 32), 256 - 8);
> > (1LL << 32) / 20 ?
> 
> Could be, I don't really care, but this line is copied from ffv1dec.c

Yeah I figured as much after doing some more grepping

> I 
> think it may be relevant to keep the exact same code for the exact
> same 
> purpose.
> Would be no more relevant if version and micro_version can be taken
> from 
> FFV1Context.

Perhaps we can have the ffv1 code in lavc expose a function for this? I
don't really like that we have this kind of parsing inside the muxers.
This goes for MPEG-2 and H.264 as well.

/Tomas

Creating a new subthread because I just noticed something

> +    //Stored height
>      mxf_write_local_tag(s, 4, 0x3202);
>      avio_wb32(pb, stored_height>>sc->interlaced);
> 

Won't this be incorrect for files whose dimensions are multiples of 16
but not multiples of 32? Isn't each field stored separately with
dimensions a multiple of 16? So while for 1080p we'll have

  StoredHeight = 1088
  SampledHeight = 1080

and 1080i:

  StoredHeight = 544
  SampledHeight = 540

Where 544 is a multiple of 16, for say 720p we have

  StoredHeight = 720
  SampledHeight = 720

but for a hypothetical 720i we'd get

  StoredHeight = 360
  SampledHeight = 360

whereas the correct values should be

  StoredHeight = 368
  SampledHeight = 360

?

/Tomas

On 18/01/2023 14:40, Tomas Härdin wrote:
> Creating a new subthread because I just noticed something

I am a bit lost there because the line of code below is not part of this 
FFV1 patch.
Additionally, none on my patches (FFV1 of MXF stored/sampled/displayed 
fix) modifies the discussed behavior (FFmpeg behavior would be same 
before and after this patch for MPEG-2 and AVC), so should not block any 
of them, and a potential fix for that should have its own patch as it 
would be a separate issue.

Anyway:


>
>> +    //Stored height
>>       mxf_write_local_tag(s, 4, 0x3202);
>>       avio_wb32(pb, stored_height>>sc->interlaced);
>>
> Won't this be incorrect for files whose dimensions are multiples of 16
> but not multiples of 32? Isn't each field stored separately with
> dimensions a multiple of 16? So while for 1080p we'll have
>
>    StoredHeight = 1088
>    SampledHeight = 1080
>
> and 1080i:
>
>    StoredHeight = 544
>    SampledHeight = 540
>
> Where 544 is a multiple of 16, for say 720p we have
>
>    StoredHeight = 720
>    SampledHeight = 720
>
> but for a hypothetical 720i we'd get
>
>    StoredHeight = 360
>    SampledHeight = 360
>
> whereas the correct values should be
>
>    StoredHeight = 368
>    SampledHeight = 360

AFAIK, it would depend about if the stream has a picture_structure frame 
(16x16 applies to the frame?) of field (16x16 applies to the field?), 
but I really don't know enough there for having a relevant opinion.

I can just say that I don't change the behavior of FFmpeg in your use 
case, I found the issues when I tried a random width and height of FFV1 
stream then checked with MPEG-2 Video and the sampled width was wrong 
for sure e.g. sampled width of 1920 for a stream having a width of 1912, 
with current FFmpeg code, and for your use case I am sure about nothing 
so I don't change the behavior with my patch, IMO if there is an issue 
with 720i MPEG-2 Video it should be in a separate topic and patch as it 
would modify the "stored_height = (st->codecpar->height+15)/16*16" 
current code (in my patch I just move this code), unless we are sure of 
what should be changed on this side and apply a fix on the way. Better 
to fix 1 issue and let 1 open with no change than fixing no issue 
because we wouldn't be sure for 1 of the 2.

Jérôme

On 18/01/2023 11:12, Tomas Härdin wrote:
> mån 2023-01-16 klockan 15:17 +0100 skrev Jerome Martinez:
> [...]
>> I
>> think it may be relevant to keep the exact same code for the exact
>> same
>> purpose.
>> Would be no more relevant if version and micro_version can be taken
>> from
>> FFV1Context.
> Perhaps we can have the ffv1 code in lavc expose a function for this? I
> don't really like that we have this kind of parsing inside the muxers.
> This goes for MPEG-2 and H.264 as well.

I don't like that too but it is beyond my skills, and as this patch does 
not do worse that what is already implemented, with a very small 
overhead (less that the MPEG-2 and H.264 parts which were accepted to be 
merged as is), I suggest that this is not a blocker here and this part 
of the code could be trashed when lavc is able to expose the codec 
context (if it is not yet able to do so).

ons 2023-01-18 klockan 15:15 +0100 skrev Jerome Martinez:
> On 18/01/2023 14:40, Tomas Härdin wrote:
> > Creating a new subthread because I just noticed something
> 
> I am a bit lost there because the line of code below is not part of
> this 
> FFV1 patch.
> Additionally, none on my patches (FFV1 of MXF
> stored/sampled/displayed 
> fix) modifies the discussed behavior (FFmpeg behavior would be same 
> before and after this patch for MPEG-2 and AVC), so should not block
> any 
> of them, and a potential fix for that should have its own patch as it
> would be a separate issue.

True, it doesn't need to hold up this patch. But some discussion is
warranted I think. I might create a separate patchset for this.

> 
> Anyway:
> 
> 
> > 
> > > +    //Stored height
> > >       mxf_write_local_tag(s, 4, 0x3202);
> > >       avio_wb32(pb, stored_height>>sc->interlaced);
> > > 
> > Won't this be incorrect for files whose dimensions are multiples of
> > 16
> > but not multiples of 32? Isn't each field stored separately with
> > dimensions a multiple of 16? So while for 1080p we'll have
> > 
> >    StoredHeight = 1088
> >    SampledHeight = 1080
> > 
> > and 1080i:
> > 
> >    StoredHeight = 544
> >    SampledHeight = 540
> > 
> > Where 544 is a multiple of 16, for say 720p we have
> > 
> >    StoredHeight = 720
> >    SampledHeight = 720
> > 
> > but for a hypothetical 720i we'd get
> > 
> >    StoredHeight = 360
> >    SampledHeight = 360
> > 
> > whereas the correct values should be
> > 
> >    StoredHeight = 368
> >    SampledHeight = 360
> 
> AFAIK, it would depend about if the stream has a picture_structure
> frame 
> (16x16 applies to the frame?) of field (16x16 applies to the field?),
> but I really don't know enough there for having a relevant opinion.
> 
> I can just say that I don't change the behavior of FFmpeg in your use
> case, I found the issues when I tried a random width and height of
> FFV1 
> stream then checked with MPEG-2 Video and the sampled width was wrong
> for sure e.g. sampled width of 1920 for a stream having a width of
> 1912, 
> with current FFmpeg code, and for your use case I am sure about
> nothing 
> so I don't change the behavior with my patch, IMO if there is an
> issue 
> with 720i MPEG-2 Video it should be in a separate topic and patch as
> it 
> would modify the "stored_height = (st->codecpar->height+15)/16*16" 
> current code (in my patch I just move this code), unless we are sure
> of 
> what should be changed on this side and apply a fix on the way.
> Better 
> to fix 1 issue and let 1 open with no change than fixing no issue 
> because we wouldn't be sure for 1 of the 2.

I suspect we are lucky because 720i doesn't really exist in the real
world, and 576i and 480i are both multiples of 32.

IMO mxfenc shouldn't lie, but looking at S377m StoredWidth/Height are
"best effort" and thus shall be encoded. Their values will depend on
FrameLayout which in turn depends on what you say - how exactly the
interlacing is done.

TL;DR: this patchset doesn't need to be held up by this.

/Tomas

> On Jan 20, 2023, at 10:17 AM, Tomas Härdin <git@haerdin.se> wrote:
> 
> ons 2023-01-18 klockan 15:15 +0100 skrev Jerome Martinez:
>> On 18/01/2023 14:40, Tomas Härdin wrote:
>>> Creating a new subthread because I just noticed something
>> 
>> I am a bit lost there because the line of code below is not part of
>> this 
>> FFV1 patch.
>> Additionally, none on my patches (FFV1 of MXF
>> stored/sampled/displayed 
>> fix) modifies the discussed behavior (FFmpeg behavior would be same 
>> before and after this patch for MPEG-2 and AVC), so should not block
>> any 
>> of them, and a potential fix for that should have its own patch as it
>> would be a separate issue.
> 
> True, it doesn't need to hold up this patch. But some discussion is
> warranted I think. I might create a separate patchset for this.
> 
>> 
>> Anyway:
>> 
>> 
>>> 
>>>> +    //Stored height
>>>>       mxf_write_local_tag(s, 4, 0x3202);
>>>>       avio_wb32(pb, stored_height>>sc->interlaced);
>>>> 
>>> Won't this be incorrect for files whose dimensions are multiples of
>>> 16
>>> but not multiples of 32? Isn't each field stored separately with
>>> dimensions a multiple of 16? So while for 1080p we'll have
>>> 
>>>    StoredHeight = 1088
>>>    SampledHeight = 1080
>>> 
>>> and 1080i:
>>> 
>>>    StoredHeight = 544
>>>    SampledHeight = 540
>>> 
>>> Where 544 is a multiple of 16, for say 720p we have
>>> 
>>>    StoredHeight = 720
>>>    SampledHeight = 720
>>> 
>>> but for a hypothetical 720i we'd get
>>> 
>>>    StoredHeight = 360
>>>    SampledHeight = 360
>>> 
>>> whereas the correct values should be
>>> 
>>>    StoredHeight = 368
>>>    SampledHeight = 360
>> 
>> AFAIK, it would depend about if the stream has a picture_structure
>> frame 
>> (16x16 applies to the frame?) of field (16x16 applies to the field?),
>> but I really don't know enough there for having a relevant opinion.
>> 
>> I can just say that I don't change the behavior of FFmpeg in your use
>> case, I found the issues when I tried a random width and height of
>> FFV1 
>> stream then checked with MPEG-2 Video and the sampled width was wrong
>> for sure e.g. sampled width of 1920 for a stream having a width of
>> 1912, 
>> with current FFmpeg code, and for your use case I am sure about
>> nothing 
>> so I don't change the behavior with my patch, IMO if there is an
>> issue 
>> with 720i MPEG-2 Video it should be in a separate topic and patch as
>> it 
>> would modify the "stored_height = (st->codecpar->height+15)/16*16" 
>> current code (in my patch I just move this code), unless we are sure
>> of 
>> what should be changed on this side and apply a fix on the way.
>> Better 
>> to fix 1 issue and let 1 open with no change than fixing no issue 
>> because we wouldn't be sure for 1 of the 2.
> 
> I suspect we are lucky because 720i doesn't really exist in the real
> world, and 576i and 480i are both multiples of 32.
> 
> IMO mxfenc shouldn't lie, but looking at S377m StoredWidth/Height are
> "best effort" and thus shall be encoded. Their values will depend on
> FrameLayout which in turn depends on what you say - how exactly the
> interlacing is done.
> 
> TL;DR: this patchset doesn't need to be held up by this.

I'm just nudging on the consideration of merging this patch. I've been testing it over the last week with ffv1/mxf content and have found this demuxing support very helpful.
Dave Rice

sön 2023-01-29 klockan 11:36 -0500 skrev Dave Rice:
> 
> 
> > On Jan 20, 2023, at 10:17 AM, Tomas Härdin <git@haerdin.se> wrote:
> > 
> > ons 2023-01-18 klockan 15:15 +0100 skrev Jerome Martinez:
> > > On 18/01/2023 14:40, Tomas Härdin wrote:
> > > > Creating a new subthread because I just noticed something
> > > 
> > > I am a bit lost there because the line of code below is not part
> > > of
> > > this 
> > > FFV1 patch.
> > > Additionally, none on my patches (FFV1 of MXF
> > > stored/sampled/displayed 
> > > fix) modifies the discussed behavior (FFmpeg behavior would be
> > > same 
> > > before and after this patch for MPEG-2 and AVC), so should not
> > > block
> > > any 
> > > of them, and a potential fix for that should have its own patch
> > > as it
> > > would be a separate issue.
> > 
> > True, it doesn't need to hold up this patch. But some discussion is
> > warranted I think. I might create a separate patchset for this.
> > 
> > > 
> > > Anyway:
> > > 
> > > 
> > > > 
> > > > > +    //Stored height
> > > > >       mxf_write_local_tag(s, 4, 0x3202);
> > > > >       avio_wb32(pb, stored_height>>sc->interlaced);
> > > > > 
> > > > Won't this be incorrect for files whose dimensions are
> > > > multiples of
> > > > 16
> > > > but not multiples of 32? Isn't each field stored separately
> > > > with
> > > > dimensions a multiple of 16? So while for 1080p we'll have
> > > > 
> > > >    StoredHeight = 1088
> > > >    SampledHeight = 1080
> > > > 
> > > > and 1080i:
> > > > 
> > > >    StoredHeight = 544
> > > >    SampledHeight = 540
> > > > 
> > > > Where 544 is a multiple of 16, for say 720p we have
> > > > 
> > > >    StoredHeight = 720
> > > >    SampledHeight = 720
> > > > 
> > > > but for a hypothetical 720i we'd get
> > > > 
> > > >    StoredHeight = 360
> > > >    SampledHeight = 360
> > > > 
> > > > whereas the correct values should be
> > > > 
> > > >    StoredHeight = 368
> > > >    SampledHeight = 360
> > > 
> > > AFAIK, it would depend about if the stream has a
> > > picture_structure
> > > frame 
> > > (16x16 applies to the frame?) of field (16x16 applies to the
> > > field?),
> > > but I really don't know enough there for having a relevant
> > > opinion.
> > > 
> > > I can just say that I don't change the behavior of FFmpeg in your
> > > use
> > > case, I found the issues when I tried a random width and height
> > > of
> > > FFV1 
> > > stream then checked with MPEG-2 Video and the sampled width was
> > > wrong
> > > for sure e.g. sampled width of 1920 for a stream having a width
> > > of
> > > 1912, 
> > > with current FFmpeg code, and for your use case I am sure about
> > > nothing 
> > > so I don't change the behavior with my patch, IMO if there is an
> > > issue 
> > > with 720i MPEG-2 Video it should be in a separate topic and patch
> > > as
> > > it 
> > > would modify the "stored_height = (st->codecpar-
> > > >height+15)/16*16" 
> > > current code (in my patch I just move this code), unless we are
> > > sure
> > > of 
> > > what should be changed on this side and apply a fix on the way.
> > > Better 
> > > to fix 1 issue and let 1 open with no change than fixing no issue
> > > because we wouldn't be sure for 1 of the 2.
> > 
> > I suspect we are lucky because 720i doesn't really exist in the
> > real
> > world, and 576i and 480i are both multiples of 32.
> > 
> > IMO mxfenc shouldn't lie, but looking at S377m StoredWidth/Height
> > are
> > "best effort" and thus shall be encoded. Their values will depend
> > on
> > FrameLayout which in turn depends on what you say - how exactly the
> > interlacing is done.
> > 
> > TL;DR: this patchset doesn't need to be held up by this.
> 
> I'm just nudging on the consideration of merging this patch. I've
> been testing it over the last week with ffv1/mxf content and have
> found this demuxing support very helpful.

Surely you mean muxing?

Some FATE tests would be nice.

/Tomas

On 31/01/2023 15:53, Tomas Härdin wrote:
> sön 2023-01-29 klockan 11:36 -0500 skrev Dave Rice:
>>
>>> On Jan 20, 2023, at 10:17 AM, Tomas Härdin <git@haerdin.se> wrote:
>>>
>>> ons 2023-01-18 klockan 15:15 +0100 skrev Jerome Martinez:
>>>> On 18/01/2023 14:40, Tomas Härdin wrote:
>>>>> Creating a new subthread because I just noticed something
>>>> I am a bit lost there because the line of code below is not part
>>>> of
>>>> this
>>>> FFV1 patch.
>>>> Additionally, none on my patches (FFV1 of MXF
>>>> stored/sampled/displayed
>>>> fix) modifies the discussed behavior (FFmpeg behavior would be
>>>> same
>>>> before and after this patch for MPEG-2 and AVC), so should not
>>>> block
>>>> any
>>>> of them, and a potential fix for that should have its own patch
>>>> as it
>>>> would be a separate issue.
>>> True, it doesn't need to hold up this patch. But some discussion is
>>> warranted I think. I might create a separate patchset for this.
>>>
>>>> Anyway:
>>>>
>>>>
>>>>>> +    //Stored height
>>>>>>        mxf_write_local_tag(s, 4, 0x3202);
>>>>>>        avio_wb32(pb, stored_height>>sc->interlaced);
>>>>>>
>>>>> Won't this be incorrect for files whose dimensions are
>>>>> multiples of
>>>>> 16
>>>>> but not multiples of 32? Isn't each field stored separately
>>>>> with
>>>>> dimensions a multiple of 16? So while for 1080p we'll have
>>>>>
>>>>>     StoredHeight = 1088
>>>>>     SampledHeight = 1080
>>>>>
>>>>> and 1080i:
>>>>>
>>>>>     StoredHeight = 544
>>>>>     SampledHeight = 540
>>>>>
>>>>> Where 544 is a multiple of 16, for say 720p we have
>>>>>
>>>>>     StoredHeight = 720
>>>>>     SampledHeight = 720
>>>>>
>>>>> but for a hypothetical 720i we'd get
>>>>>
>>>>>     StoredHeight = 360
>>>>>     SampledHeight = 360
>>>>>
>>>>> whereas the correct values should be
>>>>>
>>>>>     StoredHeight = 368
>>>>>     SampledHeight = 360
>>>> AFAIK, it would depend about if the stream has a
>>>> picture_structure
>>>> frame
>>>> (16x16 applies to the frame?) of field (16x16 applies to the
>>>> field?),
>>>> but I really don't know enough there for having a relevant
>>>> opinion.
>>>>
>>>> I can just say that I don't change the behavior of FFmpeg in your
>>>> use
>>>> case, I found the issues when I tried a random width and height
>>>> of
>>>> FFV1
>>>> stream then checked with MPEG-2 Video and the sampled width was
>>>> wrong
>>>> for sure e.g. sampled width of 1920 for a stream having a width
>>>> of
>>>> 1912,
>>>> with current FFmpeg code, and for your use case I am sure about
>>>> nothing
>>>> so I don't change the behavior with my patch, IMO if there is an
>>>> issue
>>>> with 720i MPEG-2 Video it should be in a separate topic and patch
>>>> as
>>>> it
>>>> would modify the "stored_height = (st->codecpar-
>>>>> height+15)/16*16"
>>>> current code (in my patch I just move this code), unless we are
>>>> sure
>>>> of
>>>> what should be changed on this side and apply a fix on the way.
>>>> Better
>>>> to fix 1 issue and let 1 open with no change than fixing no issue
>>>> because we wouldn't be sure for 1 of the 2.
>>> I suspect we are lucky because 720i doesn't really exist in the
>>> real
>>> world, and 576i and 480i are both multiples of 32.
>>>
>>> IMO mxfenc shouldn't lie, but looking at S377m StoredWidth/Height
>>> are
>>> "best effort" and thus shall be encoded. Their values will depend
>>> on
>>> FrameLayout which in turn depends on what you say - how exactly the
>>> interlacing is done.
>>>
>>> TL;DR: this patchset doesn't need to be held up by this.
>> I'm just nudging on the consideration of merging this patch. I've
>> been testing it over the last week with ffv1/mxf content and have
>> found this demuxing support very helpful.
> Surely you mean muxing?
>
> Some FATE tests would be nice.


Apologizes for the huge delay.
Attached is an updated patch with a FATE test.

Jérôme
From fec4b918dd6e6a067eaeb2cd27f5e42c08dcca2c Mon Sep 17 00:00:00 2001
From: Jerome Martinez <jerome@mediaarea.net>
Date: Tue, 14 Mar 2023 09:49:16 +0100
Subject: [PATCH] avformat/mxfenc: SMPTE RDD 48:2018 Amd 1:2022 support

---
 libavformat/Makefile          |   3 +-
 libavformat/mxfenc.c          | 163 +++++++++++++++++++++++++++++++++++++++++-
 libavformat/rangecoder_dec.c  |   1 +
 tests/fate/lavf-container.mak |   2 +
 tests/ref/lavf/mxf_ffv1       |   3 +
 5 files changed, 169 insertions(+), 3 deletions(-)
 create mode 100644 libavformat/rangecoder_dec.c
 create mode 100644 tests/ref/lavf/mxf_ffv1

diff --git a/libavformat/Makefile b/libavformat/Makefile
index 47bbbbfb2a..048649689b 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -712,7 +712,8 @@ SHLIBOBJS-$(CONFIG_HLS_DEMUXER)          += ac3_channel_layout_tab.o
 SHLIBOBJS-$(CONFIG_MATROSKA_DEMUXER)     += mpeg4audio_sample_rates.o
 SHLIBOBJS-$(CONFIG_MOV_DEMUXER)          += ac3_channel_layout_tab.o
 SHLIBOBJS-$(CONFIG_MP3_MUXER)            += mpegaudiotabs.o
-SHLIBOBJS-$(CONFIG_MXF_MUXER)            += golomb_tab.o
+SHLIBOBJS-$(CONFIG_MXF_MUXER)            += golomb_tab.o \
+                                            rangecoder_dec.o
 SHLIBOBJS-$(CONFIG_NUT_MUXER)            += mpegaudiotabs.o
 SHLIBOBJS-$(CONFIG_RTPDEC)               += jpegtables.o
 SHLIBOBJS-$(CONFIG_RTP_MUXER)            += golomb_tab.o jpegtables.o \
diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index a29d678098..0ff566fbb4 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -51,6 +51,7 @@
 #include "libavcodec/golomb.h"
 #include "libavcodec/h264.h"
 #include "libavcodec/packet_internal.h"
+#include "libavcodec/rangecoder.h"
 #include "libavcodec/startcode.h"
 #include "avformat.h"
 #include "avio_internal.h"
@@ -102,6 +103,7 @@ typedef struct MXFStreamContext {
     int b_picture_count;     ///< maximum number of consecutive b pictures, used in mpeg-2 descriptor
     int low_delay;           ///< low delay, used in mpeg-2 descriptor
     int avc_intra;
+    int micro_version;       ///< format micro_version, used in ffv1 descriptor
 } MXFStreamContext;
 
 typedef struct MXFContainerEssenceEntry {
@@ -130,6 +132,7 @@ enum ULIndex {
     INDEX_H264,
     INDEX_S436M,
     INDEX_PRORES,
+    INDEX_FFV1,
 };
 
 static const struct {
@@ -144,6 +147,7 @@ static const struct {
     { AV_CODEC_ID_JPEG2000,   INDEX_JPEG2000 },
     { AV_CODEC_ID_H264,       INDEX_H264 },
     { AV_CODEC_ID_PRORES,     INDEX_PRORES },
+    { AV_CODEC_ID_FFV1,       INDEX_FFV1 },
     { AV_CODEC_ID_NONE }
 };
 
@@ -151,6 +155,7 @@ static void mxf_write_wav_desc(AVFormatContext *s, AVStream *st);
 static void mxf_write_aes3_desc(AVFormatContext *s, AVStream *st);
 static void mxf_write_mpegvideo_desc(AVFormatContext *s, AVStream *st);
 static void mxf_write_h264_desc(AVFormatContext *s, AVStream *st);
+static void mxf_write_ffv1_desc(AVFormatContext *s, AVStream *st);
 static void mxf_write_cdci_desc(AVFormatContext *s, AVStream *st);
 static void mxf_write_generic_sound_desc(AVFormatContext *s, AVStream *st);
 static void mxf_write_s436m_anc_desc(AVFormatContext *s, AVStream *st);
@@ -208,6 +213,11 @@ static const MXFContainerEssenceEntry mxf_essence_container_uls[] = {
       { 0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0d,0x01,0x03,0x01,0x15,0x01,0x17,0x00 },
       { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0d,0x04,0x01,0x02,0x02,0x03,0x06,0x03,0x00 },
       mxf_write_cdci_desc },
+    // FFV1
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0d,0x0d,0x01,0x03,0x01,0x02,0x23,0x01,0x00 },
+      { 0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0d,0x01,0x03,0x01,0x15,0x01,0x1d,0x00 },
+      { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0d,0x04,0x01,0x02,0x02,0x03,0x09,0x00,0x00 },
+      mxf_write_ffv1_desc },
     { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 },
       { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 },
       { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 },
@@ -232,6 +242,15 @@ static const UID mxf_d10_container_uls[] = {
     { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x01,0x06,0x01 }, // D-10 525/50 NTSC 30mb/s
 };
 
+
+static const UID mxf_ffv1_codec_uls[] = {
+    { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09,0x01,0x00 }, // FFV1 version 0
+    { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09,0x02,0x00 }, // FFV1 version 1
+    { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09,0x03,0x00 }, // FFV1 version 2 (was only experimental)
+    { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09,0x04,0x00 }, // FFV1 version 3
+    { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x02,0x02,0x03,0x09,0x05,0x00 }, // FFV1 version 4
+};
+
 static const uint8_t product_uid[]          = { 0xAD,0xAB,0x44,0x24,0x2f,0x25,0x4d,0xc7,0x92,0xff,0x29,0xbd,0x00,0x0c,0x00,0x02};
 static const uint8_t uuid_base[]            = { 0xAD,0xAB,0x44,0x24,0x2f,0x25,0x4d,0xc7,0x92,0xff };
 static const uint8_t umid_ul[]              = { 0x06,0x0A,0x2B,0x34,0x01,0x01,0x01,0x05,0x01,0x01,0x0D,0x00,0x13 };
@@ -390,6 +409,10 @@ static const MXFLocalTagPair mxf_local_tag_batch[] = {
     { 0x8302, FF_MXF_MasteringDisplayWhitePointChromaticity },
     { 0x8303, FF_MXF_MasteringDisplayMaximumLuminance },
     { 0x8304, FF_MXF_MasteringDisplayMinimumLuminance },
+    // FFV1
+    { 0xDFD9, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x0E,0x04,0x01,0x06,0x0C,0x06,0x00,0x00,0x00}}, /* FFV1 Micro-version */
+    { 0xDFDA, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x0E,0x04,0x01,0x06,0x0C,0x05,0x00,0x00,0x00}}, /* FFV1 Version */
+    { 0xDFDB, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x0E,0x04,0x01,0x06,0x0C,0x01,0x00,0x00,0x00}}, /* FFV1 Initialization Metadata */
 };
 
 #define MXF_NUM_TAGS FF_ARRAY_ELEMS(mxf_local_tag_batch)
@@ -526,7 +549,7 @@ static void mxf_write_primer_pack(AVFormatContext *s)
     MXFContext *mxf = s->priv_data;
     AVIOContext *pb = s->pb;
     int local_tag_number = MXF_NUM_TAGS, i;
-    int will_have_avc_tags = 0, will_have_mastering_tags = 0;
+    int will_have_avc_tags = 0, will_have_mastering_tags = 0, will_have_ffv1_tags = 0;
 
     for (i = 0; i < s->nb_streams; i++) {
         MXFStreamContext *sc = s->streams[i]->priv_data;
@@ -536,6 +559,9 @@ static void mxf_write_primer_pack(AVFormatContext *s)
         if (av_stream_get_side_data(s->streams[i], AV_PKT_DATA_MASTERING_DISPLAY_METADATA, NULL)) {
             will_have_mastering_tags = 1;
         }
+        if (s->streams[i]->codecpar->codec_id == AV_CODEC_ID_FFV1) {
+            will_have_ffv1_tags = 1;
+        }
     }
 
     if (!mxf->store_user_comments) {
@@ -544,8 +570,11 @@ static void mxf_write_primer_pack(AVFormatContext *s)
         mxf_mark_tag_unused(mxf, 0x5003);
     }
 
-    if (!will_have_avc_tags) {
+    if (!will_have_avc_tags && !will_have_ffv1_tags) {
         mxf_mark_tag_unused(mxf, 0x8100);
+    }
+
+    if (!will_have_avc_tags) {
         mxf_mark_tag_unused(mxf, 0x8200);
         mxf_mark_tag_unused(mxf, 0x8201);
         mxf_mark_tag_unused(mxf, 0x8202);
@@ -558,6 +587,12 @@ static void mxf_write_primer_pack(AVFormatContext *s)
         mxf_mark_tag_unused(mxf, 0x8304);
     }
 
+    if (!will_have_ffv1_tags) {
+        mxf_mark_tag_unused(mxf, 0xDFD9);
+        mxf_mark_tag_unused(mxf, 0xDFDA);
+        mxf_mark_tag_unused(mxf, 0xDFDB);
+    }
+
     for (i = 0; i < MXF_NUM_TAGS; i++) {
         if (mxf->unused_tags[i]) {
             local_tag_number--;
@@ -1094,9 +1129,11 @@ static const UID mxf_mpegvideo_descriptor_key = { 0x06,0x0E,0x2B,0x34,0x02,0x53,
 static const UID mxf_wav_descriptor_key       = { 0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x48,0x00 };
 static const UID mxf_aes3_descriptor_key      = { 0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x47,0x00 };
 static const UID mxf_cdci_descriptor_key      = { 0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0D,0x01,0x01,0x01,0x01,0x01,0x28,0x00 };
+static const UID mxf_rgba_descriptor_key      = { 0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0D,0x01,0x01,0x01,0x01,0x01,0x29,0x00 };
 static const UID mxf_generic_sound_descriptor_key = { 0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0D,0x01,0x01,0x01,0x01,0x01,0x42,0x00 };
 
 static const UID mxf_avc_subdescriptor_key = { 0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x6E,0x00 };
+static const UID mxf_ffv1_subdescriptor_key = { 0x06,0x0E,0x2B,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x81,0x03 };
 
 static inline uint16_t rescale_mastering_chroma(AVRational q)
 {
@@ -1198,6 +1235,7 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
         avio_wb32(pb, -((st->codecpar->height - display_height)&1));
     }
 
+    if (key != mxf_rgba_descriptor_key) {
     // component depth
     mxf_write_local_tag(s, 4, 0x3301);
     avio_wb32(pb, sc->component_depth);
@@ -1234,6 +1272,7 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
         mxf_write_local_tag(s, 4, 0x3306);
         avio_wb32(pb, color);
     }
+    } // if (key != mxf_rgba_descriptor_key)
 
     if (sc->signal_standard) {
         mxf_write_local_tag(s, 1, 0x3215);
@@ -1329,6 +1368,13 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
         mxf_write_uuid(pb, AVCSubDescriptor, 0);
     }
 
+    if (st->codecpar->codec_id == AV_CODEC_ID_FFV1) {
+        // write ffv1 sub descriptor ref
+        mxf_write_local_tag(s, 8 + 16, 0x8100);
+        mxf_write_refs_count(pb, 1);
+        mxf_write_uuid(pb, FFV1SubDescriptor, 0);
+    }
+
     return pos;
 }
 
@@ -1387,6 +1433,47 @@ static void mxf_write_h264_desc(AVFormatContext *s, AVStream *st)
     }
 }
 
+static void mxf_write_ffv1_subdesc(AVFormatContext *s, AVStream *st)
+{
+    AVIOContext *pb = s->pb;
+    MXFStreamContext *sc = st->priv_data;
+    int64_t pos;
+
+    avio_write(pb, mxf_ffv1_subdescriptor_key, 16);
+    klv_encode_ber4_length(pb, 0);
+    pos = avio_tell(pb);
+    
+    mxf_write_local_tag(s, 16, 0x3C0A);
+    mxf_write_uuid(pb, FFV1SubDescriptor, 0);
+
+    if (st->codecpar->extradata_size) {
+        mxf_write_local_tag(s, st->codecpar->extradata_size, 0xDFDB);
+        avio_write(pb, st->codecpar->extradata, st->codecpar->extradata_size); // FFV1InitializationMetadata
+    }
+
+    mxf_write_local_tag(s, 2, 0xDFDA);
+    avio_wb16(pb, (*sc->codec_ul)[14]); // FFV1Version
+
+    if (st->codecpar->extradata_size) {
+        mxf_write_local_tag(s, 2, 0xDFD9);
+        avio_wb16(pb, sc->micro_version); // FFV1MicroVersion
+    }
+
+    mxf_update_klv_size(s->pb, pos);
+}
+
+static void mxf_write_ffv1_desc(AVFormatContext *s, AVStream *st)
+{
+    int is_rgb, pos;
+    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(st->codecpar->format);
+    av_assert0(desc);
+    is_rgb = desc->flags & AV_PIX_FMT_FLAG_RGB;
+
+    pos = mxf_write_cdci_common(s, st, is_rgb ? mxf_rgba_descriptor_key : mxf_cdci_descriptor_key);
+    mxf_update_klv_size(s->pb, pos);
+    mxf_write_ffv1_subdesc(s, st);
+}
+
 static void mxf_write_s436m_anc_desc(AVFormatContext *s, AVStream *st)
 {
     int64_t pos = mxf_write_generic_desc(s, st, mxf_s436m_anc_descriptor_key);
@@ -2347,6 +2434,73 @@ static int mxf_parse_h264_frame(AVFormatContext *s, AVStream *st,
     return 1;
 }
 
+static inline int get_ffv1_unsigned_symbol(RangeCoder *c, uint8_t *state) {
+    if(get_rac(c, state+0))
+        return 0;
+    else{
+        int i, e;
+        unsigned a;
+        e= 0;
+        while(get_rac(c, state+1 + FFMIN(e,9))){ //1..10
+            e++;
+            if (e > 31)
+                return AVERROR_INVALIDDATA;
+        }
+
+        a= 1;
+        for(i=e-1; i>=0; i--){
+            a += a + get_rac(c, state+22 + FFMIN(i,9)); //22..31
+        }
+
+        return a;
+    }
+}
+#define FFV1_CONTEXT_SIZE 32
+static int mxf_parse_ffv1_frame(AVFormatContext *s, AVStream *st, AVPacket *pkt)
+{
+    MXFContext *mxf = s->priv_data;
+    MXFStreamContext *sc = st->priv_data;
+    uint8_t state[FFV1_CONTEXT_SIZE];
+    RangeCoder c;
+    unsigned v;
+
+    sc->frame_size = pkt->size;
+
+    if (mxf->header_written)
+        return 1;
+
+    memset(state, 128, sizeof(state));
+    if (st->codecpar->extradata) {
+        ff_init_range_decoder(&c, st->codecpar->extradata, st->codecpar->extradata_size);
+        ff_build_rac_states(&c, 0.05 * (1LL << 32), 256 - 8);
+        v = get_ffv1_unsigned_symbol(&c, state);
+        av_assert0(v >= 2);
+        if (v > 4) {
+            return 0;
+        }
+        sc->micro_version = get_ffv1_unsigned_symbol(&c, state);
+    } else {
+        uint8_t keystate = 128;
+        ff_init_range_decoder(&c, pkt->data, pkt->size);
+        ff_build_rac_states(&c, 0.05 * (1LL << 32), 256 - 8);
+        get_rac(&c, &keystate); // keyframe
+        v = get_ffv1_unsigned_symbol(&c, state);
+        av_assert0(v < 2);
+    }
+    sc->codec_ul = &mxf_ffv1_codec_uls[v];
+    
+    if (st->codecpar->field_order > AV_FIELD_PROGRESSIVE) {
+        sc->interlaced = 1;
+        sc->field_dominance = st->codecpar->field_order == (st->codecpar->field_order == AV_FIELD_TT || st->codecpar->field_order == AV_FIELD_TB) ? 1 : 2;
+    }
+    sc->aspect_ratio.num = st->codecpar->width * st->codecpar->sample_aspect_ratio.num;
+    sc->aspect_ratio.den = st->codecpar->height * st->codecpar->sample_aspect_ratio.den;
+    av_reduce(&sc->aspect_ratio.num, &sc->aspect_ratio.den,
+              sc->aspect_ratio.num, sc->aspect_ratio.den, INT_MAX);
+
+    return 1;
+}
+
 static const UID mxf_mpeg2_codec_uls[] = {
     { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x01,0x10,0x00 }, // MP-ML I-Frame
     { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x01,0x11,0x00 }, // MP-ML Long GOP
@@ -2958,6 +3112,11 @@ static int mxf_write_packet(AVFormatContext *s, AVPacket *pkt)
             av_log(s, AV_LOG_ERROR, "could not get h264 profile\n");
             return -1;
         }
+    } else if (st->codecpar->codec_id == AV_CODEC_ID_FFV1) {
+        if (!mxf_parse_ffv1_frame(s, st, pkt)) {
+            av_log(s, AV_LOG_ERROR, "could not get ffv1 version\n");
+            return -1;
+        }
     }
 
     if (mxf->cbr_index) {
diff --git a/libavformat/rangecoder_dec.c b/libavformat/rangecoder_dec.c
new file mode 100644
index 0000000000..7b727f656e
--- /dev/null
+++ b/libavformat/rangecoder_dec.c
@@ -0,0 +1 @@
+#include "libavcodec/rangecoder.c"
diff --git a/tests/fate/lavf-container.mak b/tests/fate/lavf-container.mak
index 4bf7636b56..214ad6fc0d 100644
--- a/tests/fate/lavf-container.mak
+++ b/tests/fate/lavf-container.mak
@@ -8,6 +8,7 @@ FATE_LAVF_CONTAINER-$(call ENCDEC2, MPEG4,      MP2,       MATROSKA)           +
 FATE_LAVF_CONTAINER-$(call ENCDEC2, MPEG4,      PCM_ALAW,  MOV)                += mov mov_rtphint ismv
 FATE_LAVF_CONTAINER-$(call ENCDEC,  MPEG4,                 MOV)                += mp4
 FATE_LAVF_CONTAINER-$(call ENCDEC2, MPEG1VIDEO, MP2,       MPEG1SYSTEM MPEGPS) += mpg
+FATE_LAVF_CONTAINER-$(call ENCDEC , FFV1,                  MXF)                += mxf_ffv1
 FATE_LAVF_CONTAINER-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF)                += mxf mxf_dv25 mxf_dvcpro50
 FATE_LAVF_CONTAINER-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF_D10 MXF)        += mxf_d10
 FATE_LAVF_CONTAINER-$(call ENCDEC2, DNXHD,      PCM_S16LE, MXF_OPATOM MXF)     += mxf_opatom mxf_opatom_audio
@@ -55,6 +56,7 @@ fate-lavf-mxf: CMD = lavf_container_timecode "-ar 48000 -bf 2 -threads 1"
 fate-lavf-mxf_d10: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,pad=720:608:0:32 -c:v mpeg2video -g 0 -flags +ildct+low_delay -dc 10 -non_linear_quant 1 -intra_vlc 1 -qscale 1 -ps 1 -qmin 1 -rc_max_vbv_use 1 -rc_min_vbv_use 1 -pix_fmt yuv422p -minrate 30000k -maxrate 30000k -b 30000k -bufsize 1200000 -top 1 -rc_init_occupancy 1200000 -qmax 12 -f mxf_d10"
 fate-lavf-mxf_dv25: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,setdar=4/3 -c:v dvvideo -pix_fmt yuv420p -b 25000k -top 0 -f mxf"
 fate-lavf-mxf_dvcpro50: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,setdar=16/9 -c:v dvvideo -pix_fmt yuv422p -b 50000k -top 0 -f mxf"
+fate-lavf-mxf_ffv1: CMD = lavf_container "-an" "-r 25 -vf scale=720:576,setdar=4/3 -c:v ffv1 -level 3 -pix_fmt yuv420p -f mxf"
 fate-lavf-mxf_opatom: CMD = lavf_container "" "-s 1920x1080 -c:v dnxhd -pix_fmt yuv422p -vb 36M -f mxf_opatom -map 0"
 fate-lavf-mxf_opatom_audio: CMD = lavf_container "-ar 48000 -ac 1" "-f mxf_opatom -mxf_audio_edit_rate 25 -map 1"
 fate-lavf-smjpeg:  CMD = lavf_container "" "-f smjpeg"
diff --git a/tests/ref/lavf/mxf_ffv1 b/tests/ref/lavf/mxf_ffv1
new file mode 100644
index 0000000000..b89e6a214e
--- /dev/null
+++ b/tests/ref/lavf/mxf_ffv1
@@ -0,0 +1,3 @@
+0426b93bbf06aedf083f00cd4ebaf36f *tests/data/lavf/lavf.mxf_ffv1
+5587001 tests/data/lavf/lavf.mxf_ffv1
+tests/data/lavf/lavf.mxf_ffv1 CRC=0x02354cdc

tis 2023-03-14 klockan 10:52 +0100 skrev Jerome Martinez:
> On 31/01/2023 15:53, Tomas Härdin wrote:
> > sön 2023-01-29 klockan 11:36 -0500 skrev Dave Rice:
> > > 
> > > I'm just nudging on the consideration of merging this patch. I've
> > > been testing it over the last week with ffv1/mxf content and have
> > > found this demuxing support very helpful.
> > Surely you mean muxing?
> > 
> > Some FATE tests would be nice.
> 
> 
> Apologizes for the huge delay.
> Attached is an updated patch with a FATE test.

Test passes. Maybe someone who knows more about FFV1 wants to chime in?

/Tomas

> On Mar 15, 2023, at 10:00 AM, Tomas Härdin <git@haerdin.se> wrote:
> 
> tis 2023-03-14 klockan 10:52 +0100 skrev Jerome Martinez:
>> On 31/01/2023 15:53, Tomas Härdin wrote:
>>> sön 2023-01-29 klockan 11:36 -0500 skrev Dave Rice:
>>>> 
>>>> I'm just nudging on the consideration of merging this patch. I've
>>>> been testing it over the last week with ffv1/mxf content and have
>>>> found this demuxing support very helpful.
>>> Surely you mean muxing?
>>> 
>>> Some FATE tests would be nice.
>> 
>> 
>> Apologizes for the huge delay.
>> Attached is an updated patch with a FATE test.
> 
> Test passes. Maybe someone who knows more about FFV1 wants to chime in?

I can add that in the development of this patch, a contributor to SMPTE RDD 48:2018 Amd 1:2022 reviewed and approved the output ffv1/mxf files of this patch.
Kind Regards,
Dave Rice

fre 2023-03-24 klockan 09:59 -0400 skrev Dave Rice:
> 
> > On Mar 15, 2023, at 10:00 AM, Tomas Härdin <git@haerdin.se> wrote:
> > 
> > tis 2023-03-14 klockan 10:52 +0100 skrev Jerome Martinez:
> > > On 31/01/2023 15:53, Tomas Härdin wrote:
> > > > sön 2023-01-29 klockan 11:36 -0500 skrev Dave Rice:
> > > > > 
> > > > > I'm just nudging on the consideration of merging this patch.
> > > > > I've
> > > > > been testing it over the last week with ffv1/mxf content and
> > > > > have
> > > > > found this demuxing support very helpful.
> > > > Surely you mean muxing?
> > > > 
> > > > Some FATE tests would be nice.
> > > 
> > > 
> > > Apologizes for the huge delay.
> > > Attached is an updated patch with a FATE test.
> > 
> > Test passes. Maybe someone who knows more about FFV1 wants to chime
> > in?
> 
> I can add that in the development of this patch, a contributor to
> SMPTE RDD 48:2018 Amd 1:2022 reviewed and approved the output
> ffv1/mxf files of this patch.

Good enough. Pushed. I fixed a whitespace issue while I was at it.

/Tomas