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

[Jerome Martinez]

On 21/02/2024 14:11, Tomas Härdin wrote:

> mxfdec can detect cases where there will be two separate fields in one
> KLV,

In practice the issue is not to detect I2 case in mxfdec (it saves us 
only a little during probing of the first frame, but I could add such 
signaling in a patch after this one because it is actually independent 
from the management of 2 fields in the decoder if we want to support 
buggy files), the issue is to split per field in mxfdec.

SMPTE ST 422 extracts:
"Users are cautioned that the code values for SOC and EOC are not 
protected and can occur within the image size marker segment (SIZ), 
quantization marker segment (QCD), comment marker segment (COM) and 
other places."
"Decoders can derive the bytestream offsets of each field by analysing 
the code stream format within the essence element as described in 
ISO/IEC 15444-1."

Note that this MXF + jp2k spec hints that separating fields should be 
done in the decoder, not the demuxer.
It is impossible to split per field in a codec-neutral manner due to 
lack of metadata for that in MXF, and doing that in mxfdec implies to 
duplicate jp2k header parser code in mxfdec, and to add a similar parser 
per supported video format in the future.

> and the decoder(s) can if I'm not mistaken be instructed to decode
> into an AVFrame with stride and offset set up for interlaced decoding.


I checked the MPEG-2 Video decoder and it does not do what you say, it 
does what I do with this patch:
- mpegvideo_parser (so the parser of raw files, equivalent to mxfdec) 
understands that the stream has 2 separate fields and puts them in 1 
single AVPacket. It could separate them put it doesn't.
- mpeg12dec (equivalent to jpeg2000dec) understands that there are 2 
separate fields and applies a stride and offset internally and outputs a 
frame in AVFrame, not a field. It could provide fields but it doesn't.
I checked only what I know well, MPEG-2 Video, maybe it is done the way 
you indicate elsewhere, currently I see no example about the idea 
that stride and offset should be provided by the demuxer, would you mind 
to show some code in FFmpeg doing this way?
And stride and offset in signaling would mean that the target decoder 
must support stride and offset in signaling (including jpeg2000dec so 
getting a part of my current patch anyway), so no more "universal" as 
far as I understand.

Also:
Other comments say that AVPacket is expected to receive the packet as it 
in the container so no split of packet.
Other comments say that AVFrame is expected to receive a frame and 
never, at least for the moment, a field.
I don't see how to respect theses 2 indications, while 
mpegvideo_parser.c + mpeg12dec.c are conforming to theses indications 
because they do as in this patch.

My understanding of other comments is that my patch proposal is the 
right way to do it, I am lost here with contradictions in the indicated 
path for the support of such file.
e.g. https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/321540.html

> It should be possible to have ffmpeg set up the necessary plumbing for
> this.

But is it how it works elsewhere in FFmpeg? Would such complex and deep 
modifications be accepted by others?
Please confirm that it would be accepted before I go so deep in the 
changes of FFmpeg code, or please indicate where such tip is already 
stored in codec context.
And it is not only plumbing, it is implementing a jp2k parser in mxfdec 
for splitting fields, impossible otherwise to split the fields and it is 
NOT for all codec at once (which is impossible in practice due to the 
need to have codec dependent split of fields).

And in practice, what it the inconvenience to do this way, with very few 
modifications and in a similar manner of what is done elsewhere in 
FFmpeg with separate fields?
Especially with patch v2 which adds nearly (a couple of comparisons 
between integers) no performance impact to previously supported content.

As a summary, IMO this patch is conform to what is done elsewhere in 
FFmpeg for separated fiels e.g. in mpeg12dec and is conform to what is 
hinted in the specification by checking the 2nd codestream offset in the 
decoder rather than in the demuxer.

Jérôme