[FFmpeg-devel] [PATCH] new API usage example (adts-aac encoding from raw audio file)

[Paolo Prete]

    Il Mercoledì 29 Marzo 2017 11:58, wm4 <nfxjfg at googlemail.com> ha scritto:
 
> There are vague plans of disallowing stack allocation of AVPackets by> removing sizeof(AVPacket) from the ABI (like AVFrame etc.), so it might> be better to allocate the packet with the appropriate functions.
Done. (see next patch)


> This function is obviously getting a bit too long. Why not split it> into multiple functions?
Because without functions, the sequence of operations is more clear and easier to read and modify, IMHO, and the user doesn't have to jump from a line to another one in order to understand the code (which is short) in details. The same functions would be called once, and I don't think this is good. Consider that this is an API USAGE example, not a real application.Anyway, this is a personal opinion, and if you (and ffmpeg team) think it's better to use these functions, I'll add them to the code.


> Some encoders (probably not all) signal what rates and formats they> support in the AVCodec struct.
Done for the samplerates (see next patch). About the formats, I left a fixed AV_SAMPLE_FMT_FLTP, given that I see that the aac codec only accepts this format.


> The code would be much easier if not using custom I/O...
The choice of custom I/O is strongly intentional given that it provides access to muxed packets written in memory, and this can be useful for user if they decide to manage these packets. I don't think that's much harder: it consists only of a simple buffer and a 3-lines callback function. Anyway, I added comment that a custom I/O is intentionally used for the reason I just explained (see next patch)


> Like I said somewhere else, it _might_ be better to wait with that until the first packet has been encoded (maybe not with the AAC encoder, but some others potentially). I'll leave this to > others to judge though.
Waiting for other feedbacks, then...


> What's with the const cast?
I casted the input frame according to the function declaration and according to what other examples do with a similar function (see swr_convert() call in resampling_audio.c)


> You need av_frame_make_writeable() on the output frame.
Done (see next patch)


> Also doesn't check for errors.
Done (see next patch)


> Errors in ret_val are discarded?
Corrected in this way (see next patch): I discarded only the EAGAIN return val. Should be ok but I need a feedback. Not sure if it must be added for the cached pkts too.


> Also, if you put the receive call in a loop, it'd actually follow the proper send/receive data flow.
What do you mean exactly? The receive call is already in a loop (while (1))


> This call ( avcodec_send_frame() ) is needed only once.
Done (see next patch)


> Not sure why this logging is done - just seems to inflate the code.
Removed (see next patch)


> I don't remember how the swr_convert_frame API works, but you might have to flush out the resampler too.
This is needed (as the API doc says) when converting sample rate, which doesn't happen in this example