On Mon, 6 Apr 2020, Nicolas George wrote:

Marton Balint (12020-04-06):
The same goal can be achieved using the WRAPPED_AVFRAME codec with the existing
API.

These two APIs are somewhat redundant, but we need to discuss which one
we want to keep.

WRAPPED_AVFRAME is nice because it goes through the normal code path,
and therefore requires no exception. But on the other hand, it requires
many allocations and de-allocations, which is not good since the purpose
of these API was to be more efficient: if the application has a frame,
which is the most likely, it's more efficient to just pass it. And it's
also simpler for the application, less code, less bugs, less
maintenance.

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.

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?

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.


My opinion: merge the two features, keep the simpler code when the two
overlap:

- Keep the WRAPPED_AVFRAME codec and its encoder/decoder pair, for when
 generic code expects an AVPacket.

- For muxers implementations, keep the write_uncoded_frame callback,
 it's simpler. If a packet with WRAPPED_AVFRAME arrives, have the
 framework de-wrap the frame and call write_uncoded_frame.

- Let applications give uncoded unwrapped frames directly. It's simpler,
 it's more efficient, it's more type-safe.

- Possibly later, introduce the symmetric read_uncoded_frame callback.

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.

[...]

 libavdevice/alsa_enc.c           |  4 ++++

Any way, I don't think we should deprecate something when we have not
yet moved our code to its replacement. You can count on me for porting
ALSA, when we have decided in which direction we go.


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.

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to