Il Mercoledì 29 Marzo 2017 11:58, wm4 <[email protected]> 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
_______________________________________________
ffmpeg-devel mailing list
[email protected]
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
