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

[Anton Khirnov]

Quoting James Almer (2024-09-24 16:43:08)
> Add the LCEVC data stream payloads as packet side data to the main video
> stream, ensuring the former is always output by the demuxer even if not
> used by the process.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>  configure              |   2 +-
>  fftools/ffmpeg.h       |   6 +
>  fftools/ffmpeg_demux.c | 296 ++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 300 insertions(+), 4 deletions(-)

Overall this patch seems like overgeneralizing from a single sample. Is
there a reason to think a generic stream group would need its own
bitstream filters? Or a "main stream for merged output"? It seems
cleaner to me to just make all of this explicitly LCEVC-specific and not
try to generalize until there actually is more than one user.

> 
> diff --git a/configure b/configure
> index d77a55b653..434c73776b 100755
> --- a/configure
> +++ b/configure
> @@ -4051,7 +4051,7 @@ ffmpeg_deps="avcodec avfilter avformat threads"
>  ffmpeg_select="aformat_filter anull_filter atrim_filter crop_filter
>                 format_filter hflip_filter null_filter rotate_filter
>                 transpose_filter trim_filter vflip_filter"
> -ffmpeg_suggest="ole32 psapi shell32"
> +ffmpeg_suggest="ole32 psapi shell32 lcevc_merge_bsf"
>  ffplay_deps="avcodec avformat avfilter swscale swresample sdl2"
>  ffplay_select="crop_filter transpose_filter hflip_filter vflip_filter rotate_filter"
>  ffplay_suggest="shell32 libplacebo vulkan"
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index 733d551fa4..f598f6a46f 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -492,6 +492,12 @@ typedef struct InputFile {
>       * if new streams appear dynamically during demuxing */
>      InputStream    **streams;
>      int           nb_streams;
> +
> +    /**
> +     * stream groups that ffmpeg is aware of
> +     */
> +    struct InputStreamGroup **stream_groups;
> +    int             nb_stream_groups;

There seems to be no reason for this to be public, so it should not be.

>  } InputFile;
>  
>  enum forced_keyframes_const {
> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index 108a4a94bf..6aa194e7ad 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -40,6 +40,18 @@
>  
>  #include "libavformat/avformat.h"
>  
> +// Defined here until it's needed in other modules and moved to ffmpeg.h

"Local until/unless it needs to be public" applies to everything, not
just this struct.

> +typedef struct InputStreamGroup {
> +    const AVClass        *class;
> +
> +    /* parent source */
> +    struct InputFile     *file;
> +
> +    int                   index;
> +
> +    AVStreamGroup        *stg;
> +} InputStreamGroup;
> +
>  typedef struct DemuxStream {
>      InputStream              ist;
>  
> @@ -56,7 +68,7 @@ typedef struct DemuxStream {
>  #define DECODING_FOR_OST    1
>  #define DECODING_FOR_FILTER 2
>  
> -    /* true if stream data should be discarded */
> +    /* non-0 if stream data is not strictly used by an output */

This is backwards, and highly confusing. It should be a bitmask of "how
the stream is used".

-- 
Anton Khirnov