Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Soft Works <softworkz@hotmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded architecture
Date: Tue, 5 Apr 2022 21:05:57 +0000
Message-ID: <DM8P223MB036517C3185EE467B97EF6B6BAE49@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <164918796468.24258.6158464741625303482@lain.red.khirnov.net>



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Anton Khirnov
> Sent: Tuesday, April 5, 2022 9:46 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded
> architecture
> 
> Quoting Michael Niedermayer (2022-04-05 21:15:42)
> > On Mon, Apr 04, 2022 at 01:29:48PM +0200, Anton Khirnov wrote:
> > > Hi,
> > > this WIP patchset is the first major part of my ongoing work to
> change
> > > ffmpeg.c architecture such that every
> > > - demuxer
> > > - decoder
> > > - filtergraph
> > > - encoder
> > > - muxer
> > > lives in its own thread. The advantages of doing this, beyond
> increased
> > > throughput, would be enforced separation between these components,
> > > making the code more local and easier to reason about.
> > >
> > > This set implements threading for muxers. My tentative plan is to
> > > continue with encoders and then filters. The patches still need
> some
> > > polishing, especially the last one. Two FATE tests do not yet
> pass, this
> > > will be fixed in later iterations.
> > >
> > > Meanwhile, comments on the overall approach are especially
> welcome.
> >
> > I agree that cleanup/modularization to make the code easier to
> > understand is a good idea!
> > Didnt really look at the patchset yet.
> > I assume these changes have no real disadvantage ?
> 
> Playing the devil's advocate, I can think of the following:
> 1) ffmpeg.c will hard-depend on threads
> 2) execution flow will become non-deterministic
> 3) overall resource usage will likely go up due to inter-thread
>    synchronization and overhead related to new objects
> 4) large-scale code changes always carry a higher risk of regressions
> 
> re 1): should not be a problem for any serious system
> re 2): I spent a lot of effort to ensure the _output_ remains
>        deterministic (it actually becomes more predictable for some
>        cases)
> re 3): I expect the impact to be small and negligible, respectively,
> but
>        would have to be measured once the conversion is complete
> re 4): the only way to avoid this completely would be to stop
>        development
> 
> Overall, I believe the advantages far outweigh the potential
> negatives.

Hi,

do I understand it right that there won't be a single-thread
operation mode that replicates/corresponds the current behavior?

Not that I wouldn't welcome the performance improvements, but one
concern I have is debugging filtergraph operations. This is already 
a pretty tedious task in itself, because many relevant decisions 
are made in sub-sub-sub-sub-sub-functions, spread over many places.
When adding an additional - not even deterministic - part to the 
game, it won't make things easier. It could even create situations
where it could no longer be possible to replicate an error in a 
debugger - in case the existence of a debugger would cause a variance
within the constraints of the non-determinism range. 

From another point of view, this is a change, so fundamental like
ffmpeg(.c) hasn't seen in a long time.
I would at least suppose that this could cause issues at many ends,
and from experience, there may be additional ends where it's rather
unexpected to  have effects.

In that context, I think that doing a change of such a wide scope
in an irreversible way like this, would impose quite a burden on
many other developers, because sooner or later, other developers
will run into situations where something is no longer working like 
before and you'll regularly wonder whether this might be a consequence
of ffmpeg.c threading change or caused by other changes.
But then, you won't be able anymore to bisect on that suspicion,
because the threading change can't be reverted and (as long as it's
not shortly after the change) there might have been too many other 
changes to easily port them back to a state before the threading
change.

I wonder whether this couldn't be done in a way that the current
behavior can be preserved and activated by option?

Wouldn't it be possible to follow an approach like this:

- Assuming the code would be fine and it would mark the desired 
  end result
- Put it aside and start over from the current HEAD
- Iteratively morph the code current code in a (possibly) long
  sequence of refactoring operations where every single one
  (and hence in sum) are semantically neutral - until the code
  is turned more and more into what has already been developed
- eventually, only few differences will be left, and these can 
  be made switchable by an option - as a result, both - old and
  new operation modes would be available.

I don't know whether there's a name to this approach, probably 
there is, yet I never cared. Way more important is that I always 
had good results following this methodology.
The funny thing about it is, that when you have a reliable tooling
for refactoring, you can even stop thinking (well, sort of..)
while transforming the code. Also, when you can't imagine how
the end result would look like or wonder whether it would 
work out at all, it's fun to watch the morphing (if you're 
doing it no-brain-wise)

softworkz








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

  reply	other threads:[~2022-04-05 21:06 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-04 11:29 Anton Khirnov
2022-04-04 11:29 ` [FFmpeg-devel] [PATCH 01/49] fftools/ffmpeg: drop an obsolete hack Anton Khirnov
2022-04-04 11:29 ` [FFmpeg-devel] [PATCH 02/49] fftools/ffmpeg: move a comment to a more appropriate place Anton Khirnov
2022-04-06 11:20   ` James Almer
2022-04-04 11:29 ` [FFmpeg-devel] [PATCH 03/49] fftools/ffmpeg: stop using OutputStream.frame_number for streamcopy Anton Khirnov
2022-04-06 11:20   ` James Almer
2022-04-04 11:29 ` [FFmpeg-devel] [PATCH 04/49] fftools/ffmpeg: pass the muxer context explicitly to some functions Anton Khirnov
2022-04-04 11:29 ` [FFmpeg-devel] [PATCH 05/49] fftools/ffmpeg: store the output file index in OutputFile Anton Khirnov
2022-04-04 11:29 ` [FFmpeg-devel] [PATCH 06/49] fftools/ffmpeg: move some muxing-related code into a separate file Anton Khirnov
2022-04-04 11:29 ` [FFmpeg-devel] [PATCH 07/49] fftools/ffmpeg: move writing the trailer to ffmpeg_mux.c Anton Khirnov
2022-04-04 11:29 ` [FFmpeg-devel] [PATCH 08/49] fftools/ffmpeg: move freeing the output file " Anton Khirnov
2022-04-04 11:29 ` [FFmpeg-devel] [PATCH 09/49] fftools/ffmpeg: store output format separately from the muxer context Anton Khirnov
2022-04-06 12:00   ` James Almer
2022-04-13 10:18     ` Anton Khirnov
2022-04-04 11:29 ` [FFmpeg-devel] [PATCH 10/49] fftools/ffmpeg_mux: add private " Anton Khirnov
2022-04-04 11:29 ` [FFmpeg-devel] [PATCH 11/49] fftools/ffmpeg: add a helper function to access output file size Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 12/49] fftools/ffmpeg: fix the type of limit_filesize Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 13/49] fftools/ffmpeg: refactor limiting output file size with -fs Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 14/49] fftools/ffmpeg: set want_sdp when initializing the muxer Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 15/49] fftools/ffmpeg: write the header for stream-less outputs " Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 16/49] fftools/ffmpeg: move closing the file into of_write_trailer() Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 17/49] fftools/ffmpeg: refactor the code checking for bitexact output Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 18/49] fftools/ffmpeg: access output file chapters through a wrapper Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 19/49] fftools/ffmpeg: do not log to the muxer context Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 20/49] fftools/ffmpeg: move the mux queue into muxer private data Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 21/49] fftools/ffmpeg_mux: split queuing packets into a separate function Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 22/49] fftools/ffmpeg_mux: split of_write_packet() Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 23/49] fftools/ffmpeg: move output file opts into private context Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 24/49] fftools/ffmpeg: move processing video stats to ffmpeg_mux Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 25/49] fftools/ffmpeg_mux: drop a useless check and reduce indentation Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 26/49] fftools/ffmpeg_mux: stop using AVStream.nb_frames in do_video_stats() Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 27/49] fftools/ffmpeg_mux: stop using av_stream_get_end_pts() " Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 28/49] fftools/ffmpeg_mux: merge variable declaration and initialization Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 29/49] fftools/ffmpeg_mux: move processing AV_PKT_DATA_QUALITY_STATS to do_video_stats() Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 30/49] fftools/ffmpeg: share the code encoding a single frame between video and audio Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 31/49] fftools/ffmpeg: reuse the encoding code for flushing encoders Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 32/49] fftools/ffmpeg: reindent after previous commit Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 33/49] fftools/ffmpeg: use refcounted packets for encoded subtitles Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 34/49] fftools/ffmpeg: rework -shortest implementation Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 35/49] fftools/ffmpeg: use the sync queues to handle -frames Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 36/49] fftools/ffmpeg: stop using OutputStream.frame_number in print_report() Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 37/49] fftools/ffmpeg: only set OutputStream.frame_number for video encoding Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 38/49] fftools/ffmpeg: make the muxer AVFormatContext private to ffmpeg_mux.c Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 39/49] fftools/ffmpeg_mux: return errors from of_submit_packet() Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 40/49] fftools/ffmpeg_mux: return errors from submit_packet() Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 41/49] fftools/ffmpeg_mux: return errors from write_packet() Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 42/49] fftools/ffmpeg_mux: simplify submit_packet() Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 43/49] fftools/ffmpeg_mux: return errors from update_video_stats() Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 44/49] fftools/ffmpeg_mux: do not call exit_program() in print_sdp() Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 45/49] fftools/ffmpeg: stop using av_stream_get_end_pts() Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 46/49] fftools/ffmpeg: do not write the output file header from init_output_stream() Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 47/49] fftools/ffmpeg: depend on threads Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 48/49] fftools: add a multistream thread-safe queue Anton Khirnov
2022-04-04 11:30 ` [FFmpeg-devel] [PATCH 49/49] fftools/ffmpeg: move each muxer to a separate thread Anton Khirnov
2022-04-05  9:00 ` [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded architecture Anton Khirnov
2022-04-05 19:15 ` Michael Niedermayer
2022-04-05 19:46   ` Anton Khirnov
2022-04-05 21:05     ` Soft Works [this message]
2022-04-05 21:18       ` Paul B Mahol
2022-04-05 21:19         ` Soft Works
2022-04-06 11:17           ` Paul B Mahol
2022-04-06 15:46             ` Soft Works
2022-04-06 15:54               ` Matt Zagrabelny
2022-04-06  8:41     ` Anton Khirnov
2022-04-06 16:29       ` Soft Works
2022-04-06 17:38         ` Paul B Mahol
2022-04-06 17:56           ` Kieran Kunhya
2022-04-06 18:53             ` Soft Works
2022-04-07  8:32       ` Anton Khirnov
2022-04-08 15:27         ` Soft Works
2022-04-11  8:28         ` Anton Khirnov
2022-04-11 20:09           ` Soft Works
2022-04-11 20:52             ` Paul B Mahol
2022-04-11 20:58               ` Soft Works
2022-04-12  9:29                 ` Paul B Mahol
2022-04-12 22:43                   ` Soft Works
2022-04-13  9:42                     ` Nicolas George
2022-04-13 22:14                       ` Soft Works
2022-04-14 10:02                     ` Paul B Mahol
2022-04-14 21:37                       ` Soft Works
2022-04-06 10:02 ` 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=DM8P223MB036517C3185EE467B97EF6B6BAE49@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM \
    --to=softworkz@hotmail.com \
    --cc=ffmpeg-devel@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