| Message ID | 20241021195721.892544-7-ms+git@mur.at |
|---|---|
| State | New |
| Headers | show |
| Series | DNxUncompressed decoder | expand |
On 10/21/2024 4:57 PM, Martin Schitter wrote: > --- > libavcodec/Makefile | 1 + > libavcodec/allcodecs.c | 1 + > libavcodec/dnxucdec.c | 338 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 340 insertions(+) > create mode 100644 libavcodec/dnxucdec.c > > diff --git a/libavcodec/Makefile b/libavcodec/Makefile > index dd5d0de..e13b127 100644 > --- a/libavcodec/Makefile > +++ b/libavcodec/Makefile > @@ -328,6 +328,7 @@ OBJS-$(CONFIG_DFPWM_DECODER) += dfpwmdec.o > OBJS-$(CONFIG_DFPWM_ENCODER) += dfpwmenc.o > OBJS-$(CONFIG_DNXHD_DECODER) += dnxhddec.o dnxhddata.o > OBJS-$(CONFIG_DNXHD_ENCODER) += dnxhdenc.o dnxhddata.o > +OBJS-$(CONFIG_DNXUC_DECODER) += dnxucdec.o > OBJS-$(CONFIG_DOLBY_E_DECODER) += dolby_e.o dolby_e_parse.o kbdwin.o > OBJS-$(CONFIG_DPX_DECODER) += dpx.o > OBJS-$(CONFIG_DPX_ENCODER) += dpxenc.o > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c > index c7e5f99..ccca2ad 100644 > --- a/libavcodec/allcodecs.c > +++ b/libavcodec/allcodecs.c > @@ -93,6 +93,7 @@ extern const FFCodec ff_dfa_decoder; > extern const FFCodec ff_dirac_decoder; > extern const FFCodec ff_dnxhd_encoder; > extern const FFCodec ff_dnxhd_decoder; > +extern const FFCodec ff_dnxuc_decoder; > extern const FFCodec ff_dpx_encoder; > extern const FFCodec ff_dpx_decoder; > extern const FFCodec ff_dsicinvideo_decoder; > diff --git a/libavcodec/dnxucdec.c b/libavcodec/dnxucdec.c > new file mode 100644 > index 0000000..9d5847d > --- /dev/null > +++ b/libavcodec/dnxucdec.c > @@ -0,0 +1,338 @@ > +/* > + * Avid DNxUncomressed / SMPTE RDD 50 decoder > + * Copyright (c) 2024 Martin Schitter > + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > +/* > + This decoder for DNxUncompressed video data is mostly based on > + reverse engineering of output generated by DaVinci Resolve 19 > + but was later also checked against the SMPTE RDD 50 specification. > + > + Not all DNxUncompressed pixel format variants are supported, > + but at least an elementary base set is already usable: > + > + - YUV 4:2:2 8/10/12/16bit/half/float (16bit untested) > + YUV 4:4:4 8/16bit/half/float (all untested!) > + - RGB 8/10/12/16bit/half/float (16bit untested) > + Alpha/Y 8/16bit (all untested!) That's not good... [...] > +static int dnxuc_decode_frame(AVCodecContext *avctx, AVFrame *frame, > + int *got_frame, AVPacket *avpkt) > +{ > + char fourcc_buf[AV_FOURCC_MAX_STRING_SIZE]; > + int ret; > + > + av_fourcc_make_string(fourcc_buf, avctx->codec_tag); > + if ((avctx->width % 2) && ((fourcc_buf[0] == 'y' && fourcc_buf[1] == '2') > + ||(fourcc_buf[1] == 'y' && fourcc_buf[2] == '2'))){ > + av_log(avctx, AV_LOG_ERROR, > + "Image width must be a multiple of 2 for YUV 4:2:2 DNxUncompressed!\n"); > + return AVERROR_INVALIDDATA; > + } > + > + switch (avctx->codec_tag) { > + case MKTAG('y','2','0','8'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_UYVY422, 16, pass_through); > + break; > + case MKTAG('y','4','0','8'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444, 24, pass_through); Y408 is mapped to AV_PIX_FMT_AYUV. > + break; > + case MKTAG('y','2','1','0'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV422P10LE, 20, unpack_y210); Y210 has no pixel format, and it's packed, not planar, so definitely not AV_PIX_FMT_YUV422P10LE. > + break; > + case MKTAG('y','4','1','0'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444P10LE, 20, unpack_y410); Y410 is mapped to AV_PIX_FMT_XV30LE. > + break; > + case MKTAG('y','2','1','2'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV422P12LE, 24, unpack_y212); AV_PIX_FMT_Y212? > + break; > + case MKTAG('y','4','1','2'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444P12LE, 24, unpack_y412); This one is probably AV_PIX_FMT_XV36, and definitely not planar. > + break; > + case MKTAG('y','2','1','6'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_UYVY422_16LE, 32, pass_through); The order of y216 appears to be YUYV, not UYVY. https://learn.microsoft.com/en-us/windows/win32/medfound/10-bit-and-16-bit-yuv-video-formats > + break; > + case MKTAG('y','4','1','6'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444_16LE, 48, pass_through); This one is probably AV_PIX_FMT_AYUV64. > + break; > + case MKTAG(' ','y','2','h'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_UYVY422F16LE, 32, pass_through); > + break; > + case MKTAG(' ','y','4','h'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444F16LE, 48, pass_through); > + break; > + case MKTAG(' ','y','2','f'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_UYVY422F32LE, 64, pass_through); > + break; > + case MKTAG(' ','y','4','f'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444F32LE, 96, pass_through); > + break; > + > + case MKTAG('r','g','0','8'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_RGB24, 24, pass_through); > + break; > + case MKTAG('r','g','1','0'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_GBRP10LE, 30, unpack_rg10); > + break; > + case MKTAG('r','g','1','2'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_GBRP12LE, 36, unpack_rg12); > + break; > + case MKTAG('r','g','1','6'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_RGB48LE, 48, pass_through); > + break; > + case MKTAG(' ','r','g','h'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_RGBF16LE, 48, pass_through); > + break; > + case MKTAG(' ','r','g','f'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_RGBF32LE, 96, pass_through); > + break; > + > + case MKTAG(' ','a','0','8'): > + case MKTAG(' ','y','0','8'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_GRAY8, 8, pass_through); > + break; > + case MKTAG(' ','a','1','6'): > + case MKTAG(' ','y','1','6'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_GRAY16LE, 16, pass_through); > + break; > + > + // case MKTAG('r','l','0','8'): TODO: RLE encoded 8bit alpha > + // case MKTAG('r','l','1','6'): TODO: RLE encoded 16bit alpha > + > + default: > + av_log(avctx, AV_LOG_ERROR, > + "Unsupported DNxUncompressed pixel format variant: '%s'\n", > + fourcc_buf); > + return AVERROR_PATCHWELCOME; > + } > + > + if (ret < 0) { > + av_buffer_unref(&frame->buf[0]); > + return ret; > + } > + > + *got_frame = 1; > + > + return avpkt->size; > +} > + > +const FFCodec ff_dnxuc_decoder = { > + .p.name = "dnxuc", > + CODEC_LONG_NAME("DNxUncompressed (SMPTE RDD 50)"), > + .p.type = AVMEDIA_TYPE_VIDEO, > + .p.id = AV_CODEC_ID_DNXUC, > + FF_CODEC_DECODE_CB(dnxuc_decode_frame), > + .p.capabilities = AV_CODEC_CAP_FRAME_THREADS, > +};
On 10/21/2024 5:44 PM, James Almer wrote: > On 10/21/2024 4:57 PM, Martin Schitter wrote: >> + break; >> + case MKTAG('y','2','1','0'): >> + ret = fmt_frame(avctx, frame, avpkt, >> AV_PIX_FMT_YUV422P10LE, 20, unpack_y210); > > Y210 has no pixel format, and it's packed, not planar, so definitely not > AV_PIX_FMT_YUV422P10LE. Nevermind, there's AV_PIX_FMT_Y210.
On 10/21/2024 8:40 PM, martin schitter wrote: > > > On 21.10.24 22:44, James Almer wrote: >> >> That's not good... >> >> [...] > > Whenever and however I change it, there will allways be somebody who > doesn't like it. !:( My point is, it's not a good idea to commit code that's not tested. Assuming that's what you meant. > > This time I spend a lot of time to modify the code as close as possible > to requests asked by one previous reviewer, who insisted, that for any > format that isn't already supported by a simple raw data pass though, I > have to add a fitting pixel format and the necessary input routines. > > In this version I did exactly follow this advice to finally get accepted. > > Only for those six formats, which need anyway some further processing, > because of a very uncommon bitpacking stucture used by DNxUncompressed > for 10 and 12bit formats, I still used some already available similar > planar formats. > > I personally still think it would wise to support just a minimal but > carefully and systematically chosen set of internal pixel formats for > lossless and efficient processing and avoid all other very similar > combinatorial possibilities, but those other reviewer strictly denied > this view. Yes, i agree. But what gets committed should be know to work. > > >>> + break; >>> + case MKTAG('y','4','0','8'): >>> + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444, >>> 24, pass_through); >> >> Y408 is mapped to AV_PIX_FMT_AYUV. > > yes -- but "AYUV" is something different than "YUV" > > > >>> + break; >>> + case MKTAG('y','2','1','0'): >>> + ret = fmt_frame(avctx, frame, avpkt, >>> AV_PIX_FMT_YUV422P10LE, 20, unpack_y210); >> >> Y210 has no pixel format, and it's packed, not planar, so definitely >> not AV_PIX_FMT_YUV422P10LE. > > I just put here the final pixel format, which it will have after > preprocessing, as an argument for this kind of input. The actual > transformation resp. bit reassembling is happening in `unpack_y210()`. > >>> + break; >>> + case MKTAG('y','4','1','0'): >>> + ret = fmt_frame(avctx, frame, avpkt, >>> AV_PIX_FMT_YUV444P10LE, 20, unpack_y410); >> >> Y410 is mapped to AV_PIX_FMT_XV30LE. > > no -- in case of the 10 and 12 bit variants I utilize 16bit aligned > planar storage, because ignoring byte and 32bit boundaries for more > dense storage and optimized pixel data locality isn't always useful. > >>> + break; >>> + case MKTAG('y','2','1','2'): >>> + ret = fmt_frame(avctx, frame, avpkt, >>> AV_PIX_FMT_YUV422P12LE, 24, unpack_y212); >> >> AV_PIX_FMT_Y212? > > detto... > >>> + break; >>> + case MKTAG('y','4','1','2'): >>> + ret = fmt_frame(avctx, frame, avpkt, >>> AV_PIX_FMT_YUV444P12LE, 24, unpack_y412); >> >> This one is probably AV_PIX_FMT_XV36, and definitely not planar. > > detto... > >>> + break; >>> + case MKTAG('y','2','1','6'): >>> + ret = fmt_frame(avctx, frame, avpkt, >>> AV_PIX_FMT_UYVY422_16LE, 32, pass_through); >> >> The order of y216 appears to be YUYV, not UYVY. https:// >> learn.microsoft.com/en-us/windows/win32/medfound/10-bit-and-16-bit- >> yuv- video-formats > > Please also read the SMPTE RDD 50 specification as well! > (https://pub.smpte.org/doc/rdd50/20190730-pub/rdd50-2019.pdf) > > But in fact I would even more suggest looking at the well structured > system of vulkan pixel formats! > > I simply had to implement all this additional pixel formats, because the > byte order of already defined varaints were indeed different. And you > can't simply ignore this fact, if you want to handle the raw data stream > without any other kind of local modification. Ok, looks like that spec defined some crazy packing and reused existing naming schemes. Great. If you're going to unpack these MSB and LSB blocks so the output may be something that can be used by an AVPixFmtDescriptor, then might as well not add so many new pixel formats that will see no other use and instead rely on existing, widely supported ones. As in: > + case MKTAG('y','2','1','6'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_UYVY422_16LE, 32, pass_through); > + break; You could unpack this into AV_PIX_FMT_YUV422P16LE. > + case MKTAG('y','4','0','8'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444, 24, pass_through); > + break; You could rearrange this into AV_PIX_FMT_VYU444, or unpack into AV_PIX_FMT_YUV444P. > + case MKTAG('y','4','1','6'): > + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444_16LE, 48, pass_through); > + break; You could unpack this into AV_PIX_FMT_YUV444P16LE, or rearrange it into AV_PIX_FMT_AYUV64 (setting the alpha channel to opaque). The float ones are ok since we have no YUV float formats. But in any case, I'd like to hear what others think. Thanks for your work.
On 21.10.24 22:44, James Almer wrote: > > That's not good... > > [...] Whenever and however I change it, there will allways be somebody who doesn't like it. !:( This time I spend a lot of time to modify the code as close as possible to requests asked by one previous reviewer, who insisted, that for any format that isn't already supported by a simple raw data pass though, I have to add a fitting pixel format and the necessary input routines. In this version I did exactly follow this advice to finally get accepted. Only for those six formats, which need anyway some further processing, because of a very uncommon bitpacking stucture used by DNxUncompressed for 10 and 12bit formats, I still used some already available similar planar formats. I personally still think it would wise to support just a minimal but carefully and systematically chosen set of internal pixel formats for lossless and efficient processing and avoid all other very similar combinatorial possibilities, but those other reviewer strictly denied this view. >> + break; >> + case MKTAG('y','4','0','8'): >> + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444, >> 24, pass_through); > > Y408 is mapped to AV_PIX_FMT_AYUV. yes -- but "AYUV" is something different than "YUV" >> + break; >> + case MKTAG('y','2','1','0'): >> + ret = fmt_frame(avctx, frame, avpkt, >> AV_PIX_FMT_YUV422P10LE, 20, unpack_y210); > > Y210 has no pixel format, and it's packed, not planar, so definitely not > AV_PIX_FMT_YUV422P10LE. I just put here the final pixel format, which it will have after preprocessing, as an argument for this kind of input. The actual transformation resp. bit reassembling is happening in `unpack_y210()`. >> + break; >> + case MKTAG('y','4','1','0'): >> + ret = fmt_frame(avctx, frame, avpkt, >> AV_PIX_FMT_YUV444P10LE, 20, unpack_y410); > > Y410 is mapped to AV_PIX_FMT_XV30LE. no -- in case of the 10 and 12 bit variants I utilize 16bit aligned planar storage, because ignoring byte and 32bit boundaries for more dense storage and optimized pixel data locality isn't always useful. >> + break; >> + case MKTAG('y','2','1','2'): >> + ret = fmt_frame(avctx, frame, avpkt, >> AV_PIX_FMT_YUV422P12LE, 24, unpack_y212); > > AV_PIX_FMT_Y212? detto... >> + break; >> + case MKTAG('y','4','1','2'): >> + ret = fmt_frame(avctx, frame, avpkt, >> AV_PIX_FMT_YUV444P12LE, 24, unpack_y412); > > This one is probably AV_PIX_FMT_XV36, and definitely not planar. detto... >> + break; >> + case MKTAG('y','2','1','6'): >> + ret = fmt_frame(avctx, frame, avpkt, >> AV_PIX_FMT_UYVY422_16LE, 32, pass_through); > > The order of y216 appears to be YUYV, not UYVY. https:// > learn.microsoft.com/en-us/windows/win32/medfound/10-bit-and-16-bit-yuv- > video-formats Please also read the SMPTE RDD 50 specification as well! (https://pub.smpte.org/doc/rdd50/20190730-pub/rdd50-2019.pdf) But in fact I would even more suggest looking at the well structured system of vulkan pixel formats! I simply had to implement all this additional pixel formats, because the byte order of already defined varaints were indeed different. And you can't simply ignore this fact, if you want to handle the raw data stream without any other kind of local modification. Martin
On 10/21/2024 10:02 PM, martin schitter wrote: > On 22.10.24 00:31, James Almer wrote: >> On 10/21/2024 8:40 PM, martin schitter wrote: >>> On 21.10.24 22:44, James Almer wrote: >>>> >>>> That's not good... >>>> >>>> [...] >>> >>> Whenever and however I change it, there will allways be somebody who >>> doesn't like it. !:( >> >> My point is, it's not a good idea to commit code that's not tested. >> Assuming that's what you meant. > > Yes -- I also don't like to publish untested code. > > Unfortunately I do not have access to any software, which could produce > the needed samples. But I was looking at least for very similar code > structures -- e.g. RGB / YUV 4:4:4 similarities. > > But I definitely will not work on more complex feature suport (like the > combined components etc.) without real world samples. > > All the possible output variants of DaVinci resolve are tested and work! > > There still some unpleasant color deviations noticeable in some cases, > but that's mainly caused elsewhere. > >>> I personally still think it would wise to support just a minimal but >>> carefully and systematically chosen set of internal pixel formats for >>> lossless and efficient processing and avoid all other very similar >>> combinatorial possibilities, but those other reviewer strictly denied >>> this view. >> >> Yes, i agree. But what gets committed should be know to work. > > I hope, that's the case! > > >>>>> + break; >>>>> + case MKTAG('y','2','1','6'): >>>>> + ret = fmt_frame(avctx, frame, avpkt, >>>>> AV_PIX_FMT_UYVY422_16LE, 32, pass_through); >>>> >>>> The order of y216 appears to be YUYV, not UYVY. https:// >>>> learn.microsoft.com/en-us/windows/win32/medfound/10-bit-and-16-bit- >>>> yuv- video-formats >>> >>> Please also read the SMPTE RDD 50 specification as well! >>> (https://pub.smpte.org/doc/rdd50/20190730-pub/rdd50-2019.pdf) >>> >>> But in fact I would even more suggest looking at the well structured >>> system of vulkan pixel formats! >>> >>> I simply had to implement all this additional pixel formats, because >>> the byte order of already defined varaints were indeed different. And >>> you can't simply ignore this fact, if you want to handle the raw data >>> stream without any other kind of local modification. >> >> Ok, looks like that spec defined some crazy packing and reused >> existing naming schemes. Great. > > Yes -- this super dense bit packing looks really crazy in case of an > otherwise extremely bloated "uncompressed" and not just "lossless" > format. But it's still more are less the only option to export all these > different pixel formats (especially the more advanced floating point > ones) together with additional audio, TC etc. out of popular closed > source solutions until now. > > and btw.: FOURCC is naturally case-sensitive ;) > >> If you're going to unpack these MSB and LSB blocks so the output may >> be something that can be used by an AVPixFmtDescriptor, then might as >> well not add so many new pixel formats that will see no other use and >> instead rely on existing, widely supported ones. As in: >> >>> + case MKTAG('y','2','1','6'): >>> + ret = fmt_frame(avctx, frame, avpkt, >>> AV_PIX_FMT_UYVY422_16LE, 32, pass_through); >>> + break; >> >> You could unpack this into AV_PIX_FMT_YUV422P16LE. > > This was exactly my suggestion in the version v10 of these patch series, > but others didn't like it. > >>> + case MKTAG('y','4','0','8'): >>> + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444, >>> 24, pass_through); >>> + break; >> >> You could rearrange this into AV_PIX_FMT_VYU444, or unpack into >> AV_PIX_FMT_YUV444P. > > I know -- it's trivial to choose this way, but there has to be some > common agreement or docu, how it should be done, otherwise we run circles. I didn't participate in earlier rounds, but at least my opinion is that we shouldn't add new pixel formats if they are only used for a single codec and have no RIFF or ISOBMFF mapping. fwiw, YUV444 is fine as a name if we end up adding it, but i dislike UYVY422_16 and YUV444_16. Unfortunately we can't use Y216 (Would need to match existing Y210 and Y212, which are ordered YUYV), or Y416 (ISOBMFF mapped it to AYUV64). > >> You could unpack this into AV_PIX_FMT_YUV444P16LE, or rearrange it >> into AV_PIX_FMT_AYUV64 (setting the alpha channel to opaque). > > detto > >> The float ones are ok since we have no YUV float formats. But in any >> case, I'd like to hear what others think. > > I'm also interested, how others think about this topic. > > In case of the Float variants I'm not really happy about this actual > implementation, because the imported data gets modified in a lossy > manner by the 16bit integer conversion in this case. This doesn't make > any visible difference in practice, but it simply shouldn't happen > nowadays high-end workflows. > > I really hope to work around this issue by an additional vulkan > implementation in the long run. > >> Thanks for your work. >> >> >> _______________________________________________ >> 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". > > _______________________________________________ > 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".
On 21.10.24 23:50, James Almer wrote: > On 10/21/2024 5:44 PM, James Almer wrote: >> On 10/21/2024 4:57 PM, Martin Schitter wrote: >>> + break; >>> + case MKTAG('y','2','1','0'): >>> + ret = fmt_frame(avctx, frame, avpkt, >>> AV_PIX_FMT_YUV422P10LE, 20, unpack_y210); >> >> Y210 has no pixel format, and it's packed, not planar, so definitely >> not AV_PIX_FMT_YUV422P10LE. > > Nevermind, there's AV_PIX_FMT_Y210. Although both entities share a very similar FOURCC code, they are very different bit structures in practice! Sure, both of them refer 'somehow' to 10bit 4:2:2 YUV data and can be lossless converted in both directions, but that's all they have in common. But in fact I could have converted the DNxUncompressed 10 and 12 bit formats to any packed format as well. I just choose the planar variant, because byte ordering changes are easier to handle in this arrangement and there were more systematically ordered variants of this kind already defined. At the beginning I hopped, that this would make the reuse of already defined pixformats more easily and also enable a more consistent code layout for all the required RGB / YUV422 / YUV444 variants... Searching the chaotic list of already defined pixel formats is another topic, where I better keep quiet, because I could otherwise hurt somebody... Martin
On 22.10.24 00:31, James Almer wrote: > On 10/21/2024 8:40 PM, martin schitter wrote: >> On 21.10.24 22:44, James Almer wrote: >>> >>> That's not good... >>> >>> [...] >> >> Whenever and however I change it, there will allways be somebody who >> doesn't like it. !:( > > My point is, it's not a good idea to commit code that's not tested. > Assuming that's what you meant. Yes -- I also don't like to publish untested code. Unfortunately I do not have access to any software, which could produce the needed samples. But I was looking at least for very similar code structures -- e.g. RGB / YUV 4:4:4 similarities. But I definitely will not work on more complex feature suport (like the combined components etc.) without real world samples. All the possible output variants of DaVinci resolve are tested and work! There still some unpleasant color deviations noticeable in some cases, but that's mainly caused elsewhere. >> I personally still think it would wise to support just a minimal but >> carefully and systematically chosen set of internal pixel formats for >> lossless and efficient processing and avoid all other very similar >> combinatorial possibilities, but those other reviewer strictly denied >> this view. > > Yes, i agree. But what gets committed should be know to work. I hope, that's the case! >>>> + break; >>>> + case MKTAG('y','2','1','6'): >>>> + ret = fmt_frame(avctx, frame, avpkt, >>>> AV_PIX_FMT_UYVY422_16LE, 32, pass_through); >>> >>> The order of y216 appears to be YUYV, not UYVY. https:// >>> learn.microsoft.com/en-us/windows/win32/medfound/10-bit-and-16-bit- >>> yuv- video-formats >> >> Please also read the SMPTE RDD 50 specification as well! >> (https://pub.smpte.org/doc/rdd50/20190730-pub/rdd50-2019.pdf) >> >> But in fact I would even more suggest looking at the well structured >> system of vulkan pixel formats! >> >> I simply had to implement all this additional pixel formats, because >> the byte order of already defined varaints were indeed different. And >> you can't simply ignore this fact, if you want to handle the raw data >> stream without any other kind of local modification. > > Ok, looks like that spec defined some crazy packing and reused existing > naming schemes. Great. Yes -- this super dense bit packing looks really crazy in case of an otherwise extremely bloated "uncompressed" and not just "lossless" format. But it's still more are less the only option to export all these different pixel formats (especially the more advanced floating point ones) together with additional audio, TC etc. out of popular closed source solutions until now. and btw.: FOURCC is naturally case-sensitive ;) > If you're going to unpack these MSB and LSB blocks so the output may be > something that can be used by an AVPixFmtDescriptor, then might as well > not add so many new pixel formats that will see no other use and instead > rely on existing, widely supported ones. As in: > >> + case MKTAG('y','2','1','6'): >> + ret = fmt_frame(avctx, frame, avpkt, >> AV_PIX_FMT_UYVY422_16LE, 32, pass_through); >> + break; > > You could unpack this into AV_PIX_FMT_YUV422P16LE. This was exactly my suggestion in the version v10 of these patch series, but others didn't like it. >> + case MKTAG('y','4','0','8'): >> + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444, >> 24, pass_through); >> + break; > > You could rearrange this into AV_PIX_FMT_VYU444, or unpack into > AV_PIX_FMT_YUV444P. I know -- it's trivial to choose this way, but there has to be some common agreement or docu, how it should be done, otherwise we run circles. > You could unpack this into AV_PIX_FMT_YUV444P16LE, or rearrange it into > AV_PIX_FMT_AYUV64 (setting the alpha channel to opaque). detto > The float ones are ok since we have no YUV float formats. But in any > case, I'd like to hear what others think. I'm also interested, how others think about this topic. In case of the Float variants I'm not really happy about this actual implementation, because the imported data gets modified in a lossy manner by the 16bit integer conversion in this case. This doesn't make any visible difference in practice, but it simply shouldn't happen nowadays high-end workflows. I really hope to work around this issue by an additional vulkan implementation in the long run. > Thanks for your work. > > > _______________________________________________ > 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".
On 10/21/2024 8:51 PM, James Almer wrote: > On 10/21/2024 10:02 PM, martin schitter wrote: >> I know -- it's trivial to choose this way, but there has to be some >> common agreement or docu, how it should be done, otherwise we run >> circles. > > I didn't participate in earlier rounds, but at least my opinion is that > we shouldn't add new pixel formats if they are only used for a single > codec and have no RIFF or ISOBMFF mapping. hwaccels like Vulkan/VAAPI/QSV/D3DVA/VideoToolbox defining them is also an argument in favor to add and map them.
Quoting martin schitter (2024-10-22 01:40:07) > > > On 21.10.24 22:44, James Almer wrote: > > > > That's not good... > > > > [...] > > Whenever and however I change it, there will allways be somebody who > doesn't like it. !:( > > This time I spend a lot of time to modify the code as close as possible > to requests asked by one previous reviewer, who insisted, that for any > format that isn't already supported by a simple raw data pass though, I > have to add a fitting pixel format and the necessary input routines. > > In this version I did exactly follow this advice to finally get accepted. > > Only for those six formats, which need anyway some further processing, > because of a very uncommon bitpacking stucture used by DNxUncompressed > for 10 and 12bit formats, I still used some already available similar > planar formats. > > I personally still think it would wise to support just a minimal but > carefully and systematically chosen set of internal pixel formats for > lossless and efficient processing and avoid all other very similar > combinatorial possibilities, but those other reviewer strictly denied > this view. You can refer to me by name you know, this passive-aggressive indirection isn't improving the discussion. But more importantly, I did NOT ask you to add native pixel formats for everything (and you're not doing it in this patch anyway), only use them when they exist, or add a non-alpha version of an existing format with alpha.
Quoting Martin Schitter (2024-10-21 21:57:18) > +static int pass_through(AVCodecContext *avctx, AVFrame *frame, const AVPacket *avpkt) > +{ > + /* there is no need to copy as the data already match > + * a known pixel format */ > + > + frame->buf[0] = av_buffer_ref(avpkt->buf); I said this twice before already - every single format that uses pass_through() should instead be exported by the demuxer as AV_CODEC_ID_RAWVIDEO, because that's what it is. That will also have the following important advantages: * decoding these formats won't pointlessly waste resources and add latency using frame threading, which is useless for them * your decoder can be marked as AV_CODEC_CAP_DR1 > +static int unpack_y212(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt) > +{ > + uint8_t *data = &pkt->data[0]; > + int lw = frame->width; > + int msb_bytes = lw * 2; > + int line_offset = lw * 3; > + int pos_uv = 0, pos_y = 0, pos_in = 0; > + for(int y = 0; y < frame->height; y++){ > + for(int x = 0; x < lw/2; x++){ > + AV_WL16(&frame->data[1][pos_uv], get12(data, pos_in++, msb_bytes)); // u > + AV_WL16(&frame->data[0][pos_y], get12(data, pos_in++, msb_bytes)); // y > + AV_WL16(&frame->data[2][pos_uv], get12(data, pos_in++, msb_bytes)); // v > + AV_WL16(&frame->data[0][pos_y + 2],get12(data, pos_in++, msb_bytes)); // y > + pos_uv += 2; > + pos_y += 4; > + } > + data += line_offset; > + pos_in = 0; > + } > + return pkt->size; > +} These functions are now MUCH more readable, and it becomes clear that you're not accessing frame->linesize, which is immensely suspicious. Lines of a frame do not need to be contiguous in memory, there can be arbitrary amounts of paddding between them.
On 22.10.24 04:37, James Almer wrote: > On 10/21/2024 8:51 PM, James Almer wrote: >> On 10/21/2024 10:02 PM, martin schitter wrote: >> I didn't participate in earlier rounds, but at least my opinion is >> that we shouldn't add new pixel formats if they are only used for a >> single codec and have no RIFF or ISOBMFF mapping. > > hwaccels like Vulkan/VAAPI/QSV/D3DVA/VideoToolbox defining them is also > an argument in favor to add and map them. YES! -- We should look forward and not only glorify the good old FOURCC naming and legacy pixel data arrangements. The more verbose and systematically ordered sets of pixel format definitions used by those newer GPU processing oriented APIs (e.g. the vulkan one: https://docs.vulkan.org/spec/latest/chapters/formats.html) are very useful in practice. Supporting them makes code much more compatible and future-proof. Martin
On Tue, 22 Oct 2024, Anton Khirnov wrote: > Quoting Martin Schitter (2024-10-21 21:57:18) >> +static int pass_through(AVCodecContext *avctx, AVFrame *frame, const AVPacket *avpkt) >> +{ >> + /* there is no need to copy as the data already match >> + * a known pixel format */ >> + >> + frame->buf[0] = av_buffer_ref(avpkt->buf); > > I said this twice before already - every single format that uses > pass_through() should instead be exported by the demuxer as > AV_CODEC_ID_RAWVIDEO, because that's what it is. I don't really want the MXF demuxer/muxer to do DNXUC parsing or creation. Also I might want to wrap DNXUC essence to another container, or remux it to MXF again. So I am not convinced that the current approach is bad. Regards, Marton > > That will also have the following important advantages: > * decoding these formats won't pointlessly waste resources and add > latency using frame threading, which is useless for them > * your decoder can be marked as AV_CODEC_CAP_DR1 > > >> +static int unpack_y212(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt) >> +{ >> + uint8_t *data = &pkt->data[0]; >> + int lw = frame->width; >> + int msb_bytes = lw * 2; >> + int line_offset = lw * 3; >> + int pos_uv = 0, pos_y = 0, pos_in = 0; >> + for(int y = 0; y < frame->height; y++){ >> + for(int x = 0; x < lw/2; x++){ >> + AV_WL16(&frame->data[1][pos_uv], get12(data, pos_in++, msb_bytes)); // u >> + AV_WL16(&frame->data[0][pos_y], get12(data, pos_in++, msb_bytes)); // y >> + AV_WL16(&frame->data[2][pos_uv], get12(data, pos_in++, msb_bytes)); // v >> + AV_WL16(&frame->data[0][pos_y + 2],get12(data, pos_in++, msb_bytes)); // y >> + pos_uv += 2; >> + pos_y += 4; >> + } >> + data += line_offset; >> + pos_in = 0; >> + } >> + return pkt->size; >> +} > > These functions are now MUCH more readable, and it becomes clear that > you're not accessing frame->linesize, which is immensely suspicious. > Lines of a frame do not need to be contiguous in memory, there can be > arbitrary amounts of paddding between them. > > -- > Anton Khirnov > _______________________________________________ > 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". >
Quoting Marton Balint (2024-10-22 20:35:52) > > > On Tue, 22 Oct 2024, Anton Khirnov wrote: > > > Quoting Martin Schitter (2024-10-21 21:57:18) > >> +static int pass_through(AVCodecContext *avctx, AVFrame *frame, const AVPacket *avpkt) > >> +{ > >> + /* there is no need to copy as the data already match > >> + * a known pixel format */ > >> + > >> + frame->buf[0] = av_buffer_ref(avpkt->buf); > > > > I said this twice before already - every single format that uses > > pass_through() should instead be exported by the demuxer as > > AV_CODEC_ID_RAWVIDEO, because that's what it is. > > I don't really want the MXF demuxer/muxer to do DNXUC parsing What parsing is there to do? You just compare against the codec tag. > Also I might want to wrap DNXUC essence to another container, or remux > it to MXF again. And where is the problem here? > So I am not convinced that the current approach is bad. It is bad because it introduces a completely pointless and arbitrary distinction between "rawvideo" and "rawvideo, but EXTRACTED FROM MXF". And also because of the two points I mentioned: > > * decoding these formats won't pointlessly waste resources and add > > latency using frame threading, which is useless for them > > * your decoder can be marked as AV_CODEC_CAP_DR1
On 22.10.24 20:35, Marton Balint wrote: >> On Tue, 22 Oct 2024, Anton Khirnov wrote: >> I said this twice before already - every single format that uses >> pass_through() should instead be exported by the demuxer as >> AV_CODEC_ID_RAWVIDEO, because that's what it is. > > I don't really want the MXF demuxer/muxer to do DNXUC parsing or > creation. Also I might want to wrap DNXUC essence to another container, > or remux it to MXF again. So I am not convinced that the current > approach is bad. > > Regards, > Marton Thanks for this statement resp. assessment of the suggested solution! I see it very much the same way as you do, but it's also very useful to listen to arguments reflecting divergent points of view. At the end it shouldn't be such a drama. The actual code isn't very complex and should be easily modifiable anytime if someone finds a better solution for this task. Martin
diff --git a/libavcodec/Makefile b/libavcodec/Makefile index dd5d0de..e13b127 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -328,6 +328,7 @@ OBJS-$(CONFIG_DFPWM_DECODER) += dfpwmdec.o OBJS-$(CONFIG_DFPWM_ENCODER) += dfpwmenc.o OBJS-$(CONFIG_DNXHD_DECODER) += dnxhddec.o dnxhddata.o OBJS-$(CONFIG_DNXHD_ENCODER) += dnxhdenc.o dnxhddata.o +OBJS-$(CONFIG_DNXUC_DECODER) += dnxucdec.o OBJS-$(CONFIG_DOLBY_E_DECODER) += dolby_e.o dolby_e_parse.o kbdwin.o OBJS-$(CONFIG_DPX_DECODER) += dpx.o OBJS-$(CONFIG_DPX_ENCODER) += dpxenc.o diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c index c7e5f99..ccca2ad 100644 --- a/libavcodec/allcodecs.c +++ b/libavcodec/allcodecs.c @@ -93,6 +93,7 @@ extern const FFCodec ff_dfa_decoder; extern const FFCodec ff_dirac_decoder; extern const FFCodec ff_dnxhd_encoder; extern const FFCodec ff_dnxhd_decoder; +extern const FFCodec ff_dnxuc_decoder; extern const FFCodec ff_dpx_encoder; extern const FFCodec ff_dpx_decoder; extern const FFCodec ff_dsicinvideo_decoder; diff --git a/libavcodec/dnxucdec.c b/libavcodec/dnxucdec.c new file mode 100644 index 0000000..9d5847d --- /dev/null +++ b/libavcodec/dnxucdec.c @@ -0,0 +1,338 @@ +/* + * Avid DNxUncomressed / SMPTE RDD 50 decoder + * Copyright (c) 2024 Martin Schitter + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/* + This decoder for DNxUncompressed video data is mostly based on + reverse engineering of output generated by DaVinci Resolve 19 + but was later also checked against the SMPTE RDD 50 specification. + + Not all DNxUncompressed pixel format variants are supported, + but at least an elementary base set is already usable: + + - YUV 4:2:2 8/10/12/16bit/half/float (16bit untested) + YUV 4:4:4 8/16bit/half/float (all untested!) + - RGB 8/10/12/16bit/half/float (16bit untested) + Alpha/Y 8/16bit (all untested!) + + TODO: Compositions of multiple components aren't supportet until now. + TODO: This also hinders Image+Alpha use in one file. + +*/ + +#include "avcodec.h" +#include "codec_internal.h" +#include "decode.h" +#include "libavutil/imgutils.h" +#include "libavutil/intreadwrite.h" +#include "thread.h" + +static int pass_through(AVCodecContext *avctx, AVFrame *frame, const AVPacket *avpkt) +{ + /* there is no need to copy as the data already match + * a known pixel format */ + + frame->buf[0] = av_buffer_ref(avpkt->buf); + + if (!frame->buf[0]) { + return AVERROR(ENOMEM); + } + + return av_image_fill_arrays(frame->data, frame->linesize, avpkt->data, + avctx->pix_fmt, avctx->width, avctx->height, 1); +} + +/// Unpack 10bit value +static av_always_inline +uint16_t get10(uint8_t *line_data, uint32_t pos, int msb_bytes) +{ + return (line_data[pos] << 2) | + ((line_data[msb_bytes + (pos >> 2)] >> ((pos & 0x3u) << 1)) & 0x3u); +} + +/// Unpack 12bit value +static av_always_inline +uint16_t get12(uint8_t *line_data, uint32_t pos, int msb_bytes) +{ + return (line_data[pos] << 4) | + ((line_data[msb_bytes + (pos >> 1)] >> ((pos & 0x1u) << 2)) & 0xfu); +} + +static int unpack_rg10(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt) +{ + uint8_t *data = &pkt->data[0]; + int lw = frame->width; + int msb_bytes = lw * 3; + int line_offset = lw * 3 + (lw * 3 + 3) / 4; + int pos = 0, pos_in = 0; + for(int y = 0; y < frame->height; y++){ + for(int x = 0; x < lw; x++){ + AV_WL16(&frame->data[2][pos], get10(data, pos_in++, msb_bytes)); // r + AV_WL16(&frame->data[0][pos], get10(data, pos_in++, msb_bytes)); // g + AV_WL16(&frame->data[1][pos], get10(data, pos_in++, msb_bytes)); // b + pos += 2; + } + data += line_offset; + pos_in = 0; + } + return pkt->size; +} + +static int unpack_y410(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt) +{ + uint8_t *data = &pkt->data[0]; + int lw = frame->width; + int msb_bytes = lw * 3; + int line_offset = lw * 3 + (lw * 3 + 3) / 4; + int pos = 0, pos_in = 0; + for(int y = 0; y < frame->height; y++){ + for(int x = 0; x < lw; x++){ + AV_WL16(&frame->data[0][pos], get10(data, pos_in++, msb_bytes)); // y + AV_WL16(&frame->data[1][pos], get10(data, pos_in++, msb_bytes)); // u + AV_WL16(&frame->data[2][pos], get10(data, pos_in++, msb_bytes)); // v + pos += 2; + } + data += line_offset; + pos_in = 0; + } + return pkt->size; +} + +static int unpack_rg12(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt) +{ + uint8_t *data = &pkt->data[0]; + int lw = frame->width; + int msb_bytes = lw * 3; + int line_offset = lw * 3 + (lw * 3 + 1) / 2; + int pos = 0, pos_in = 0; + for(int y = 0; y < frame->height; y++){ + for(int x = 0; x < lw; x++){ + AV_WL16(&frame->data[2][pos], get12(data, pos_in++, msb_bytes)); // r + AV_WL16(&frame->data[0][pos], get12(data, pos_in++, msb_bytes)); // g + AV_WL16(&frame->data[1][pos], get12(data, pos_in++, msb_bytes)); // b + pos += 2; + } + data += line_offset; + pos_in = 0; + } + return pkt->size; +} + +static int unpack_y412(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt) +{ + uint8_t *data = &pkt->data[0]; + int lw = frame->width; + int msb_bytes = lw * 3; + int line_offset = lw * 3 + (lw * 3 + 1) / 2; + int pos = 0, pos_in = 0; + for(int y = 0; y < frame->height; y++){ + for(int x = 0; x < lw; x++){ + AV_WL16(&frame->data[0][pos], get12(data, pos_in++, msb_bytes)); // y + AV_WL16(&frame->data[1][pos], get12(data, pos_in++, msb_bytes)); // u + AV_WL16(&frame->data[2][pos], get12(data, pos_in++, msb_bytes)); // v + pos += 2; + } + data += line_offset; + pos_in = 0; + } + return pkt->size; +} + +static int unpack_y210(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt) +{ + uint8_t *data = &pkt->data[0]; + int lw = frame->width; + int msb_bytes = lw * 2; + int line_offset = lw/2 * 5; + int pos_uv = 0, pos_y = 0, pos_in = 0; + for(int y = 0; y < frame->height; y++){ + for(int x = 0; x < lw/2; x++){ + AV_WL16(&frame->data[1][pos_uv], get10(data, pos_in++, msb_bytes)); // u + AV_WL16(&frame->data[0][pos_y], get10(data, pos_in++, msb_bytes)); // y + AV_WL16(&frame->data[2][pos_uv], get10(data, pos_in++, msb_bytes)); // v + AV_WL16(&frame->data[0][pos_y + 2], get10(data, pos_in++, msb_bytes)); // y + pos_uv += 2; + pos_y += 4; + } + data += line_offset; + pos_in = 0; + } + return pkt->size; +} + +static int unpack_y212(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt) +{ + uint8_t *data = &pkt->data[0]; + int lw = frame->width; + int msb_bytes = lw * 2; + int line_offset = lw * 3; + int pos_uv = 0, pos_y = 0, pos_in = 0; + for(int y = 0; y < frame->height; y++){ + for(int x = 0; x < lw/2; x++){ + AV_WL16(&frame->data[1][pos_uv], get12(data, pos_in++, msb_bytes)); // u + AV_WL16(&frame->data[0][pos_y], get12(data, pos_in++, msb_bytes)); // y + AV_WL16(&frame->data[2][pos_uv], get12(data, pos_in++, msb_bytes)); // v + AV_WL16(&frame->data[0][pos_y + 2],get12(data, pos_in++, msb_bytes)); // y + pos_uv += 2; + pos_y += 4; + } + data += line_offset; + pos_in = 0; + } + return pkt->size; +} + +static int check_pkt_size(AVCodecContext *avctx, AVPacket *avpkt, int bpp) +{ + int needed = ((avctx->width * bpp + 7) / 8) * avctx->height; + if (avpkt->size < needed){ + av_log(avctx, AV_LOG_ERROR, + "Insufficient size of AVPacket data (pkg size: %d needed: %d)\n", avpkt->size, needed); + return AVERROR_INVALIDDATA; + } + return 0; +} + +static int fmt_frame(AVCodecContext *avctx, AVFrame *frame, AVPacket *avpkt, + enum AVPixelFormat pix_fmt, int src_bpp, + int (*frame_handler)(AVCodecContext *avctx, AVFrame *frame, const AVPacket *avpkt)) +{ + int ret; + avctx->pix_fmt = pix_fmt; + + ret = check_pkt_size(avctx, avpkt, src_bpp); + if (ret) + return ret; + + ret = ff_thread_get_buffer(avctx, frame, 0); + if (ret < 0) + return ret; + + return frame_handler(avctx, frame, avpkt); +} + +static int dnxuc_decode_frame(AVCodecContext *avctx, AVFrame *frame, + int *got_frame, AVPacket *avpkt) +{ + char fourcc_buf[AV_FOURCC_MAX_STRING_SIZE]; + int ret; + + av_fourcc_make_string(fourcc_buf, avctx->codec_tag); + if ((avctx->width % 2) && ((fourcc_buf[0] == 'y' && fourcc_buf[1] == '2') + ||(fourcc_buf[1] == 'y' && fourcc_buf[2] == '2'))){ + av_log(avctx, AV_LOG_ERROR, + "Image width must be a multiple of 2 for YUV 4:2:2 DNxUncompressed!\n"); + return AVERROR_INVALIDDATA; + } + + switch (avctx->codec_tag) { + case MKTAG('y','2','0','8'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_UYVY422, 16, pass_through); + break; + case MKTAG('y','4','0','8'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444, 24, pass_through); + break; + case MKTAG('y','2','1','0'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV422P10LE, 20, unpack_y210); + break; + case MKTAG('y','4','1','0'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444P10LE, 20, unpack_y410); + break; + case MKTAG('y','2','1','2'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV422P12LE, 24, unpack_y212); + break; + case MKTAG('y','4','1','2'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444P12LE, 24, unpack_y412); + break; + case MKTAG('y','2','1','6'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_UYVY422_16LE, 32, pass_through); + break; + case MKTAG('y','4','1','6'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444_16LE, 48, pass_through); + break; + case MKTAG(' ','y','2','h'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_UYVY422F16LE, 32, pass_through); + break; + case MKTAG(' ','y','4','h'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444F16LE, 48, pass_through); + break; + case MKTAG(' ','y','2','f'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_UYVY422F32LE, 64, pass_through); + break; + case MKTAG(' ','y','4','f'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_YUV444F32LE, 96, pass_through); + break; + + case MKTAG('r','g','0','8'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_RGB24, 24, pass_through); + break; + case MKTAG('r','g','1','0'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_GBRP10LE, 30, unpack_rg10); + break; + case MKTAG('r','g','1','2'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_GBRP12LE, 36, unpack_rg12); + break; + case MKTAG('r','g','1','6'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_RGB48LE, 48, pass_through); + break; + case MKTAG(' ','r','g','h'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_RGBF16LE, 48, pass_through); + break; + case MKTAG(' ','r','g','f'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_RGBF32LE, 96, pass_through); + break; + + case MKTAG(' ','a','0','8'): + case MKTAG(' ','y','0','8'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_GRAY8, 8, pass_through); + break; + case MKTAG(' ','a','1','6'): + case MKTAG(' ','y','1','6'): + ret = fmt_frame(avctx, frame, avpkt, AV_PIX_FMT_GRAY16LE, 16, pass_through); + break; + + // case MKTAG('r','l','0','8'): TODO: RLE encoded 8bit alpha + // case MKTAG('r','l','1','6'): TODO: RLE encoded 16bit alpha + + default: + av_log(avctx, AV_LOG_ERROR, + "Unsupported DNxUncompressed pixel format variant: '%s'\n", + fourcc_buf); + return AVERROR_PATCHWELCOME; + } + + if (ret < 0) { + av_buffer_unref(&frame->buf[0]); + return ret; + } + + *got_frame = 1; + + return avpkt->size; +} + +const FFCodec ff_dnxuc_decoder = { + .p.name = "dnxuc", + CODEC_LONG_NAME("DNxUncompressed (SMPTE RDD 50)"), + .p.type = AVMEDIA_TYPE_VIDEO, + .p.id = AV_CODEC_ID_DNXUC, + FF_CODEC_DECODE_CB(dnxuc_decode_frame), + .p.capabilities = AV_CODEC_CAP_FRAME_THREADS, +};