Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Jerome Martinez <jerome@mediaarea.net>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: support of 2 fields in 1 AVPacket
Date: Sat, 3 Feb 2024 11:51:38 +0100
Message-ID: <e1c37d78-331a-4fdd-8033-649d9a085cc7@mediaarea.net> (raw)
In-Reply-To: <0181f3c1559fca8731853f00fa8c0ed5f7acd7d3.camel@haerdin.se>

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.

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.

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?


>   It also makes the j2k decoder harder to ||ize. You might also
> need the StoredF2Offset

I don't change the field order detection by current FFmpeg code, I rely 
on FFmpeg code directly.
In my opinion if MXF field order detection needs to be changed, it would 
be in a separate patch dedicated to that (the field order detection in 
MXF) and it does not impact this patch as it is independent from which 
container is used, using AVCodecContext field order information. Jérôme
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  reply	other threads:[~2024-02-03 10:51 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-02 15:55 Jerome Martinez
2024-02-03 10:00 ` Tomas Härdin
2024-02-03 10:51   ` Jerome Martinez [this message]
2024-02-03 19:58     ` Tomas Härdin
2024-02-03 20:04       ` Tomas Härdin
2024-02-04  0:20         ` Pierre-Anthony Lemieux
2024-02-05  0:19           ` Tomas Härdin
2024-02-15 15:02             ` Jerome Martinez
2024-02-16  0:02               ` Vittorio Giovara
2024-02-16  2:17                 ` Kieran Kunhya
2024-02-18 23:14               ` Tomas Härdin
2024-02-19 11:08                 ` Tomas Härdin
2024-02-19 11:19                   ` Jerome Martinez
2024-02-16 10:32       ` Anton Khirnov
2024-02-18 23:43         ` Marton Balint
2024-02-19  8:22           ` Hendrik Leppkes
2024-02-19 16:14             ` Devin Heitmueller
2024-02-19 11:05           ` Tomas Härdin
2024-02-19 11:07             ` Tomas Härdin
2024-02-05  2:25   ` James Almer
2024-02-05 14:28     ` Tomas Härdin
2024-02-20 15:07 ` [FFmpeg-devel] [PATCH v2] " Jerome Martinez
2024-02-21 13:11   ` Tomas Härdin
2024-02-21 14:27     ` Jerome Martinez
2024-02-24 12:26       ` Tomas Härdin
2024-02-25  4:14         ` [FFmpeg-devel] [PATCH v3] " Jerome Martinez
2024-03-01 22:29           ` Tomas Härdin
2024-03-03 17:07             ` Jerome Martinez
2024-04-24 11:20   ` [FFmpeg-devel] [PATCH v2] " Jerome Martinez
2024-04-24 22:54     ` Tomas Härdin
2024-04-25  0:59     ` Michael Niedermayer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e1c37d78-331a-4fdd-8033-649d9a085cc7@mediaarea.net \
    --to=jerome@mediaarea.net \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git