From: Pierre-Anthony Lemieux <pal@sandflow.com> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: support of 2 fields in 1 AVPacket Date: Sat, 3 Feb 2024 16:20:15 -0800 Message-ID: <CAAqQPR45G+q4RKF1jARt63GOCqMj8gFFVr5zU9MTM1pRzGw3Eg@mail.gmail.com> (raw) In-Reply-To: <8d05aa8f8b574a1630c0e623459e7d571f465aeb.camel@haerdin.se> [-- Attachment #1: Type: text/plain, Size: 3706 bytes --] On Sat, Feb 3, 2024 at 12:04 PM Tomas Härdin <git@haerdin.se> wrote: > > lör 2024-02-03 klockan 20:58 +0100 skrev 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. > > It should also be said that what this patch effectively does is > silently convert SEPARATE_FIELDS to MIXED_FIELDS. What if I want to > transcode J2K to lower bitrate but keep it SEPARATE_FIELDS? See attached table from SMPTE ST 422, which specifies wrapping of J2K in MXF. > > /Tomas > _______________________________________________ > 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". [-- Attachment #2: image.png --] [-- Type: image/png, Size: 208404 bytes --] [-- Attachment #3: Type: text/plain, Size: 251 bytes --] _______________________________________________ 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-04 0:20 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 2024-02-03 19:58 ` Tomas Härdin 2024-02-03 20:04 ` Tomas Härdin 2024-02-04 0:20 ` Pierre-Anthony Lemieux [this message] 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=CAAqQPR45G+q4RKF1jARt63GOCqMj8gFFVr5zU9MTM1pRzGw3Eg@mail.gmail.com \ --to=pal@sandflow.com \ --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