[FFmpeg-devel] [PATCH 3/3] avformat: deprecate muxing uncoded frames

[Nicolas George]

Marton Balint (12020-04-11):
> The extra allocations and deallocations are barely measurable. So far
> uncoded_frame was used for realtime outputs. So you get, I don't know 100
> packets/frames per second? Allocating those is peanuts, and I have to stress
> that you are not allocating the data, just the AVFrame struct.

Dynamic allocations are expensive. A hundred per second, multiplied by
maybe ten or a thousand for a project that handles several clients
simultaneously, that may make the difference.

But I grant you, there are already a lot of allocations, especially if
interleaving of streams is in action.

> I'd also argue if it is simple for the application. Might be for some
> specific use cases, certainly not for e.g. ffmpeg.c or any other application
> which tries to support multiple output formats. Maybe it is no accident that
> I am not aware of any project on the internet who picked up using this API.
> Are you?

I use it in a few of my projects, at least.

> Maintenance for *us* is significant, see the mux.c patches Andreas sent to
> patch the various bugs caused by exceptions in the muxing code. And his
> preferred solution is to wrap the frame in an AVPacket where the destructor
> frees the AVFrame. Just like AVCODEC_ID_WRAPPED_FRAME does it.

Yes, I remembered these issues had to be fixed when designed uncoded
frames.

> Sorry, I don't see the benefits of keeping the API, I only see the huge
> maintenance burden our our part. I'd just drop it at the next bump.

After a closer look, I say this:

For the maintenance burden of mux.c, go ahead, remove everything related
to uncoded frame. In this part of code, uncoded frames and wrapped
frames are the same thing, just implemented differently. The wrapped
frame implementation is cleaner, so let us choose this one.

But in that case, av_write_uncoded_frame() just becomes an utility
function that does wrapped_avframe_encode() and av_write_frame(). It
makes writing applications easier: let us keep it that way. Utility
functions are not maintenance burden.

And at the other end, when calling output format callbacks, the handling
for write_uncoded_frame() is six lines of leaf code. We can keep it if
it makes decoders simpler.

Which leads me to an actual proposition:

Rework your patch to clean up wrapped frames, but leave alone the bits
about uncoded frames that are not in the way: they will be broken, let
them be. When you post your patch, I will try to fix them without
getting in the way. And then we cans discuss with something practical
under our eyes.

> IMHO writing uncoded frame for pulse/alsa audio is pointless. It does not
> support planar, and the performance gain (especially for realtime output) is
> so insignificant, it does not worth maintaining such a feature.

Our ALSA code does not, but ALSA itself supports planar formats, and
adding it to our code is quite easy.

Also, please remember that real-time device exist on machines with very
constrained hardware too.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200414/2715cf3c/attachment.sig>