[FFmpeg-devel] [PATCH] doc/examples/muxing: code rewrite with improved readability and fixed issues

[Paolo Prete]

    Il sabato 18 giugno 2022, 19:07:11 CEST, Andreas Rheinhardt <andreas.rheinhardt at outlook.com> ha scritto:  
 
 >>Paolo Prete:
>> Please review this. It's a code rewrite of doc/examples/muxing.c which improves readability and fixes issues.
>> More specifically:
>> *) Original functions of muxing.c have generic/unclear/ambiguous names and they don't group logically relatedblocks of >>code: this makes the code hard to read.See for example open_audio/video() and add_stream(), which mix initialization of >>the encoders' and muxers' stuff.A redundant structure with an ambiguous name ("struct OutputStream"), which is not part of >>API, is widely used too.The patch uses functions with clearer names and with blocks of code strictly logically related, such >>as: init_encoder(), init_avframe(), init_muxer(), convert_frame(), encode_frame(), mux_encoded_pkt()...

>Using a structure for the user's data is actually intentional, because
>users will probably use one, too. And given that it is the user's
>structure it is of course not part of the public API.
This is not what I meant. The API already has all the needed structures for managing, by the user, all the flow of an A/V pipeline. There's no need of another struct (which is therefore redundant and with a meaningless name) for exchanging data between functions. Please consider the code of the patch in a different perspective. It explicitly, in the main() function, creates an encoder, a converter and a muxer and the required input/output frames: nothing more, nothing else. And it simply calls fill_frame(), convert_frame(), encode_frame(), mux_encoded_pkt(). This is what I suggest as a readable code which represents an A/V pipeline. These function have names that represent exactly what they do, instead of using generic/misleading names such as "open_audio()" or "write_frame()" etc. For doing that I had to rewrite the code from scratch, then it's not possible to split the patch into small pieces, as you ask.


> There is just one thing that I immediately noticed:
>
>
> sizeof(AVCodecParameters) is not public, you must not put
> AVCodecParameters on the stack.

i just fixed this, thanks. See the attached patch in response to Leo Izen