[FFmpeg-devel] Buffers Threads and refcounts

[Michael Niedermayer]

Hi all

The fork plans to make some changes on how various buffers work, that
is adding thread safe ref counting and doing all kinds of related and
unrelated changes.
They plan to stash this 5000+ lines diff changing 300+ files into
few commits, to quote:
"The changes are split into per-codec patches to simplify
 review, but will be squashed on push."

See:
http://lists.libav.org/pipermail/libav-devel/2013-January/041437.html
http://lists.libav.org/pipermail/libav-devel/2013-January/041447.html
http://lists.libav.org/pipermail/libav-devel/2013-January/041576.html

This quite large (and ineviable quite buggy) changeset will cause
various problems
* Moving AVFrame to libavutil causes ABI/API issues. Currently
  if a new field is needed its just added and used, after the move
  it will be needed to keep libavcodec, libavutil, libavfilter and
  other users of AVFrame in sync or take extra care on accessing some
  fields that might not have existed in other libs when they where
  build.
* The patchsets remove various buffer pools which will cause
  significant speedloss.
  It should be possible to reimplement these pools though
* Interfacing with any application that doesnt have refcounted frames
  will likely need a memcpy.
* Various fields are removed from public API, these will break
  usage of libavcodec with libpostproc, any reuse of motion vectors
  or macroblock types on reencoding and probably all attempts to
  extract statistics about the internals of a video codec, short of
  parsing av_log() debug output.

Above issues will hit libav as well as ffmpeg if we merge the patches.
ffmpeg will be hit a bit harder because we have more optimizations
that are incompatible with the refcounted buffers.

The advantages of this patchset are
1. It simplifies the code, this is true, but this is
   just because optimizations, features and not understood bugfixes
   are removed. The rest is moving compexity around more than removing
   it. And i doubt the code will be any simpler once all bugs are
   fixed and once it runs at the same speed again.
   (also its 5926 insertions(+), 5464 deletions(-)
    that is 500 lines more than before the patches even with many
    optimizations droped)
2. It simplifies the permissions in libavfilter at the expense of
   speed. Currently libavfilter (of ffmpeg) supports almost any kind
   of buffer, static, read only, .... After the patchset only the most
   common kind of buffers are supported, that IS simpler and slower
   because if your buffer doesnt fit in the API you need to copy it.
   Which way is better, i dont know, but the permission system is
   entirely independant of what these patchsets do. Such change should
   have been done independantly
3. decoders dont return refcounted buffers, this patchset changes this
   making users happier.
   The same effect though can be achived with the current API/ABI by
   just putting a av_refcounted_get_buffer() inside lavc and the
   user app using that.

in some cases it will need fewer memcpies in other cases, it will
need more. And it will do the thread safe refcounting in all cases
and that will slow all cases down, especially small frames like in
some audio decoders/demuxers will suffer from this

So in summary, these patchsets are bad, really bad in the short term
and less so in the long term once all optimizations are put back
and in the distant future it might even be an improvment and lead to
slightly cleaner code overall.

What to do about that ?
If we do not merge them then this means an end to API/ABI
compatibility with libav, this would be a big annoyance to
applications using libav*, thus their oppinon is quite welcome here.

If we do merge them this means alot of work, these are ~ 6000 changed
lines done on a 2 year outdated fork. A fork missing many codecs,
filter and (de)muxers that we do have. The code is buggy on its own
and possible a lesser number of bugs will get introduced from the
merge

These changes should have been done on top of ffmpeg changing all
codecs and filters And should have been reviewed by the authors who
wrote the code that is being changed. But the authors are unwelcome in
libav. The changes also should stay in a branch until all
optimizations are back in place and bugs fixed instead of being
commited to libav.
But i doubt thats how it will be.

If we do merge these patchsets, we must be aware that the code will
likely be in not so good shape afterwards (nor will libav be in better)
and will need time and lots of work.
Also merging these patchsets that where worked on since 3+ months by
libav developers will not be possible in 1 day, it will take longer

Also consider that not merging them and the API/ABI incompatibility
would affect distributions and applications. And that in this case
There may be a need for volunteers to package ffmpeg for some
distributions. (We either way still need volunteers for official
debian & ubuntu packages of ffmpeg)

My current plan is to attempt to integrate the patchsets once they hit
libav, and fix as many of the bugs they contain as i can. But iam
surely not opposed to the alternative if ffmpeg developers and
application developers support going a different path.

Also it would be great if the fork nonsense would end and the libav
developers would join ffmpeg and we could all work together making
these changes without bugs and without speedloss instead of the
duplication of work this now will become ...
Also consider, if i wouldnt have had to work on daily merging of
libav, we now probably would have a HEVC decoder and a useable AAC
encoder but i rather had to spend my time on that. And now i will
have to work on integrating and fixing this instead of more usefull
things ...

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Avoid a single point of failure, be that a person or equipment.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130109/63cef713/attachment.asc>