Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Anton Khirnov <anton@khirnov.net>
To: FFmpeg development discussions and patches
	<ffmpeg-devel@ffmpeg.org>,
	tc@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH v3] ffmpeg CLI multithreading
Date: Wed, 06 Dec 2023 18:31:27 +0100
Message-ID: <170188388713.8914.8464001514053857598@lain.khirnov.net> (raw)
In-Reply-To: <20231206103002.30084-1-anton@khirnov.net>

Hi all,

As usual when someone disagrees with him, Nicolas converged to being
utterly unreasonable and deaf to all arguments. I see no point in
discussing this with him any further and intend to push the set
tomorrow, unless somebody else has substantial objections.

I've considered asking for a TC vote, but as he is not suggesting any
viable alternative, there is really nothing to vote on. So the purpose
of this email is just summarizing the dispute, so others can understand
it more easily.

The issue concerns sub2video code, which allows converting bitmap
subtitles to video streams, mainly for hardsubbing purposes. As
subtitles are typically sparse in time, the demuxer that produces the
subtitle stream emits "heartbeat" frames that are sent to the
filtering code just like real subtitle frames.

The code in ffmpeg_filter.c then decides whether these heartbeat frames
should be sent to the filtergraph or ignored. The problem is that this
decision is currently made in a way that depends on what frames
previously arrived on _other_ filtergraph inputs (e.g. on video frames
in a graph with a subtitle and a video input). However, the inputs are
not synchronized, and the interleaving of frames on different inputs is
effectively arbitrary. E.g. it depends on the video decoder delay (and
thus on the number of frame threads, when frame threading is used).

The reason this arbitrariness has not become a major issue until now, is
that it is deterministic for a given run on a given machine (sub2video
FATE tests do not use a frame-threaded decoder, and so do not exhibit
the problem). With ffmpeg CLI becoming fully parallel, the results
become non-deterministic and change from run to run, which forces me to
do something about this.

My solution in patch 01/10 changes the filtering code to always send the
heartbeat frames to the filtergraph. This not only makes the results
deterministic, but also improves subtitle timing in FATE tests.

Nicolas presented a testcase that consists of taking a video+subtitle
streams from a single source, offsetting them against each other by a
fixed delay, and overlaying subtitle onto video. After my patch, this
results in the filtergraph buffering a number of heartbeat frames
proportional to the offset, which causes higher memory consumption.

However,
* the testcase suffers from the above problem - its output can change
  significantly depending on the number of decoder frame threads; this
  is fixed by my patch;
* the extra buffering added by the patch is similar to what would be
  added by the muxer interleaving queue, were the streams remuxed rather
  than overlaid;
* the buffering can be avoided entirely by opening the input twice.

I thus consider his argument (that the patch is "breaking" the testcase)
invalid, as the testcase is
* contrived;
* already broken;
* actually fixed by my patch.

Nicolas has also NOT suggested any viable alternative approach.

-- 
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".

  parent reply	other threads:[~2023-12-06 17:31 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-06 10:27 Anton Khirnov
2023-12-06 10:27 ` [FFmpeg-devel] [PATCH 01/10] fftools/ffmpeg_filter: make sub2video heartbeat more robust Anton Khirnov
2023-12-06 10:27 ` [FFmpeg-devel] [PATCH 02/10] fftools/ffmpeg_filter: move filtering to a separate thread Anton Khirnov
2023-12-06 10:27 ` [FFmpeg-devel] [PATCH 03/10] fftools/ffmpeg_filter: buffer sub2video heartbeat frames like other frames Anton Khirnov
2023-12-06 10:27 ` [FFmpeg-devel] [PATCH 04/10] fftools/ffmpeg_filter: reindent Anton Khirnov
2023-12-06 10:27 ` [FFmpeg-devel] [PATCH 05/10] fftools/ffmpeg_mux: add muxing thread private data Anton Khirnov
2023-12-06 10:27 ` [FFmpeg-devel] [PATCH 06/10] fftools/ffmpeg_mux: move bitstream filtering to the muxer thread Anton Khirnov
2023-12-06 10:27 ` [FFmpeg-devel] [PATCH 07/10] fftools/ffmpeg_demux: switch from AVThreadMessageQueue to ThreadQueue Anton Khirnov
2023-12-06 10:27 ` [FFmpeg-devel] [PATCH 08/10] fftools/ffmpeg_enc: move encoding to a separate thread Anton Khirnov
2023-12-06 10:27 ` [FFmpeg-devel] [PATCH 09/10] fftools/ffmpeg: add thread-aware transcode scheduling infrastructure Anton Khirnov
2023-12-06 10:27 ` [FFmpeg-devel] [PATCH 10/10] fftools/ffmpeg: convert to a threaded architecture Anton Khirnov
2023-12-06 11:22   ` Paul B Mahol
2023-12-06 11:31     ` Anton Khirnov
2023-12-06 10:51 ` [FFmpeg-devel] [PATCH] web: add a news entry for ffmpeg CLI threading Anton Khirnov
2023-12-06 10:55 ` [FFmpeg-devel] [PATCH v3] ffmpeg CLI multithreading Nicolas George
2023-12-06 12:06   ` Zhao Zhili
2023-12-06 12:10     ` Nicolas George
2023-12-06 12:30       ` Anton Khirnov
2023-12-06 12:58         ` Nicolas George
2023-12-06 17:31 ` Anton Khirnov [this message]
2023-12-06 20:14   ` Nicolas George
2023-12-06 20:53     ` Vittorio Giovara
2023-12-06 19:29 ` Marton Balint
2023-12-06 19:36   ` Anton Khirnov
2023-12-06 20:16     ` Nicolas George
     [not found]     ` <6F5C6E5F-1538-47F0-9C71-CA10A1D38C3F@cosmin.at>
2023-12-06 20:29       ` Cosmin Stejerean via ffmpeg-devel
2023-12-06 21:00         ` Anton Khirnov
2023-12-06 20:29     ` Anton Khirnov
2023-12-06 20:03 ` Vittorio Giovara
2023-12-06 20:21 ` Michael Niedermayer
2023-12-07 10:52   ` Anton Khirnov
2023-12-07 18:10     ` Michael Niedermayer
2023-12-07 18:27     ` Michael Niedermayer
2023-12-11  9:06       ` Anton Khirnov
2024-01-15 23:51         ` Marth64
2024-01-24 12:51           ` Anton Khirnov

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=170188388713.8914.8464001514053857598@lain.khirnov.net \
    --to=anton@khirnov.net \
    --cc=ffmpeg-devel@ffmpeg.org \
    --cc=tc@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