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".
next prev parent 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