[FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: support of 2 fields in 1 AVPacket

[Tomas Härdin]

lör 2024-02-03 klockan 11:51 +0100 skrev Jerome Martinez:
> On 03/02/2024 11:00, Tomas Härdin wrote:
> > fre 2024-02-02 klockan 16:55 +0100 skrev Jerome Martinez:
> > > Before this patch, the FFmpeg MXF parser correctly detects
> > > content
> > > with
> > > 2 fields in 1 AVPacket as e.g. interlaced 720x486 but the FFmpeg
> > > JPEG
> > > 2000 decoder reads the JPEG 2000 SIZ header without understanding
> > > that
> > > the indicated height is the height of 1 field only so overwrites
> > > the
> > > frame size info with e.g. 720x243, and also completely discards
> > > the
> > > second frame, which lead to the decoding of only half of the
> > > stored
> > > content as "progressive" 720x243 flagged interlaced.
> > Is the decoder really the right place to do this? Surely this
> > happens
> > with more codecs than just j2k. Isnt it also a parser's job to do
> > this
> > kind of stuff?
> 
> Both solutions have pro and con:
> - Doing it in the decoder fixes the issue for all containers and only
> one codec
> - Doing it in the demuxer fixes the issue for one container and all
> codecs
> And currently I know only one container and only one codec having
> this 
> issue.

A more proper fix might be to auto-insert a deinterleave filter in
ffmpeg.

> My choice to implement in the decoder comes from the idea that it
> seems 
> more hacky to decode 2 AVPackets (crafted from a single MXF packet), 
> then do a RAM copy of the decoded (so huge) content for interleaving 
> fields into 1 frame (pressure on RAM bandwidth) in the MXF demuxer + 
> adapt frame metadata (height is overwritten by the decoder then need
> to 
> overwrite again the value), doing it in the decoder seems less
> intrusive.

The fastest way, in a player, is probably to do it with a shader. That
should be the least amount of copies and the most cache coherent.

> If moving to the demuxer is the only acceptable solution, I will do
> it 
> but I would like to be sure that there is a consensus on that by
> FFmpeg 
> developers before doing it, in order to not have this extra work 
> rejected due to a future disagreement about where it should go.
> 
> > The logic that scans the packet for two SOI markers shouldn't be
> > necessary if the relevant information is carried over from the MXF
> > demuxer.
> 
> As far as I know there is nothing in the MXF file saying where is the
> second field in the packet, at which MXF metadata do you think?

Well, FrameLayout == SEPARATE_FIELDS, EditRate = 30000/1001 and
FieldDominance == 2. DisplayHeight is the field height as S377 says it
should be for SEPARATE_FIELDS.

Annoyingly EditRate could be the field rate rather than the frame rate.
So there may be no way to a priori know how many fields an essence
element contains.

I think people with knowledge how interlacing is handled in other
formats and codecs might want to chime in.

/Tomas