Hi, I'm bumping this patch proposal for avoiding a situation where
FFmpeg skips half the visual content when 2 jpeg2000 codestreams are
present in one frame. I re-reviewed this discussion and think I answered
all concerns. I'm hesitant with patch v3 because I consider that
touching frame_worker_thread for this feature is not so useful but I am
ok with either patch v2 or v3.
Thanks much,
Jérôme
On 20/02/2024 16:07, Jerome Martinez wrote:
Attached is an updated version of the patch proposal.
About the idea to keep separate fields in the output AVFrame, I note
from the discussion that it is commonly accepted that up to now it is
expected that the AVPacket contains what is in the MXF element and
that the AVFrame contains a frame and never a field, and additionally
I see in e.g. mpeg12dec.c that the decoder interleaves separate fields:
mb_y <<= field_pic;
if (s2->picture_structure == PICT_BOTTOM_FIELD)
mb_y++;
And mpegvideo_parser.c creates a AVPacket with both fields in AVPacket
even if they are separated, this patch keeps the AVPacket from e.g.
MXF with both fields in it and does something similar to what do other
decoders with separate fields in the output AVFRame.
About the detection of the 2 separated fields in 1 packet in the MXF
file (I2 mode), it is doable in the example file provided in the first
patch proposal to catch it by checking the essence label but other
files I have don't have the specific essence label (they use the
"undefined" essence label, legit) so IMO we should not rely on it and
there is actually no practical advantage in catching that from the
container.
In practice this new patch proposal is slightly more complex, with one
recursive call to jpeg2000_decode_frame() when there are 2 codestreams
in 1 AVPacket, but it has a negligible performance impact (few
comparisons and bool checks) when there is only one codestream in the
AVPacket (the currently only supported method, remind that currently
FFmpeg completely discards the 2nd codestream present in the AVPacket)
and it relies on the current jpeg2000_read_main_headers() function for
catching the end of the first codestream (safer than the quick find of
EOC/SOC in the first version).
It also changes the jpeg2000_decode_frame return value to the count of
bytes parsed, it seems that it was what is expected but in practice it
was not doing that, fixing the return value could be a separate patch
if preferred.
Jérôme
On 02/02/2024 16:55, Jerome Martinez wrote:
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.
Example file:
https://www.digitizationguidelines.gov/guidelines/MXF_sampleFiles/RDD48-sample12-gf-jpeg2000-ntsc-4.2.zip
Before this patch:
Stream #0:0: Video: jpeg2000, yuv422p10le(bottom coded first
(swapped)), 720x243, lossless, SAR 9:20 DAR 4:3, 29.97 tbr, 29.97
tbn, 29.97 tbc
After this patch:
Stream #0:0: Video: jpeg2000, yuv422p10le(bottom coded first
(swapped)), 720x486, lossless, SAR 9:10 DAR 4:3, 29.97 fps, 29.97
tbr, 29.97 tbn
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
