Il Mercoledì 29 Marzo 2017 11:58, wm4 <nfx...@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


   
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to