[FFmpeg-devel] [PATCH 13/13] fftools/ffmpeg_demux: merge streams in a LCEVC stream group

[Anton Khirnov]

Quoting James Almer (2024-09-06 16:05:33)
> On 9/6/2024 10:33 AM, Anton Khirnov wrote:
> > This is not the the only place where you're adding a check for discard,
> 
> Because it's the only code i don't want to trigger for non-standalone 
> output packets.

???

> 
> > and I strongly dislike that 'discard' now does not mean 'discard'
> > anymore.
> 
> No, the packet is discarded in the end, and never makes anywhere on its 
> own unless it's used by an output.

That's a bit too many qualifiers for my taste. With current code,
discard means this:
* discard=1: stream is not processed in any way
* discard=0: stream is processed

If you want to add more possible states, do so explicitly. That is
probably best achieved by changing it to a mask (called something like
'used'), with values like USED_DIRECT and USED_LCEVC (I'm not convinced
it is useful to introduce generic notions like "streamgroup bistream
filters" until there is more than one user for them).

> > Furthermore, I really dislike how invasive such an obscure feature is.
> 
> Ideally, lavf would export the LCEVC stream payload as packet side data 
> in the video's stream, but that's apparently only possible with mov/mp4 
> and maybe matroska, not TS. Hence a Stream Group is used and everything 
> left to the caller.
> 
> That said, is this so invasive? This patch adds 
> DemuxStreamGroup/InputStreamGroup, cleanly initializes them like we 
> already do with DemuxStream/InputStream, and then factorizes the bsf 
> packet loop so it can be used for the stream bsf and the group bsf.
> The only truly big change is making libavformat output streams that are 
> not strictly used by an output, which is achieved by making 
> AVStream->discard not be set, and keep relying on DemuxStream->discard 
> to know what stream is disabled or not.

You are introducing a bunch of new core concepts that do not apply to
anything except LCEVC, and rewriting a lot of core demuxing code around
them. So yes, that looks invasive to me.

-- 
Anton Khirnov