[FFmpeg-devel] [PATCH v12 5/9] libavcodec/dnxucdec: DNxUncompressed decoder

[Anton Khirnov]

Quoting Marton Balint (2024-10-23 15:19:27)
> 
> 
> On Tue, 22 Oct 2024, Anton Khirnov wrote:
> 
> > 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.
> 
> As far as I know, the codec tag is embedded somewhere inside the various 
> boxes of DNXUC (pack box, optional icmp box, optional fill box, sinf box). 
> So I don't quite see how can you easily get that (or find where the signal 
> data actually starts) without parsing the box sequence.

The decoder in this patch does not see any of it, it only gets raw video
and the codec tag.

Looking at this a bit closer, seems Martin's intent is to do it in the
parser, which is wrong as parser are not supposed to perform
transformations like this.
The relevant code is on the order of 10-20 lines, so I don't really see
a problem with moving that into the demuxer. Either way it should not be
in a parser.

> >> Also I might want to wrap DNXUC essence to another container, or remux
> >> it to MXF again.
> >
> > And where is the problem here?
> 
> If demuxing destroys the original DNXUC essence with its box structure, 
> then you will need something on the muxing side that re-creates this. If a 
> DNXUC packet contained the whole 'pack' box, this would not be needed. 

And what is the problem with that?

This reminds me a bit of the s302m question that was resolved by TC
earlier this year [1]. The point there was that demuxers should try to
export data in the most generally useful format and not just blindly
pass through whatever is stored in the container. Is DNXUC useful
outside of MXF? What do our callers gain by getting the box structure
rather than raw video+codec tag? If it's only useful for remuxing then
it should be handled inside lavf.

> This is also why I don't quite like that the dnxuc_parser right now skips 
> some header bytes and points the packet buffer to the signal data. IMHO 
> the DNXUC packet should really be the 'pack' box itself, and the decoder 
> should parse that.

I agree that the parser is definitely not the place to do this.

> >> 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".
> 
> DNXUC is not only raw data, but the whole box structure with raw or 
> compressed data inside.
> The format even allows to store the planes separately.

I am not saying the codec ID should not exist and EVERYTHING should be
handled by the demuxer, just that it should be done when the data
directly maps to our notion of raw video. Which also neatly separates
the cases where the decoder actually does nontrivial work rather than
just set up some pointers, which have radically different performance
characteristics.

[1] https://lists.ffmpeg.org//pipermail/ffmpeg-devel/2024-April/325588.html

-- 
Anton Khirnov