Il martedì 7 giugno 2022, 13:56:37 CEST, Anton Khirnov <an...@khirnov.net>
ha scritto:
>Quoting Paolo Prete (2022-06-07 12:59:05)
>> What you say is true, IMHO, as long as the functions (in which the
>> code is split) do really group logically related tasks and they have
>> names that summarize what they are doing. In the examined case this is
>> not true, see for example (in muxing.c): static void
>> open_video(AVFormatContext *oc, const AVCodec *codec, OutputStream
>> *ost, AVDictionary *opt_arg)
>>
>> The name "open_video" is too generic and it doesn't let the user know
>> what the function is actually doing, without jumping from line to line
>> in the code. In fact the function mixes different tasks: it opens the
>> codec, it allocates a frame, it copies the stream parameters to the
>> muxer.
>All the things it does relate to preparing video encoding. You might
>argue that the function name is suboptimal, in which case it should be
>improved. But it is certainly not true that the function just groups
>random unrelated code.
Not true. There's a step, inside the function, that does _not_ relate to
preparing video encoding, then it should not be grouped into the same logical
unit:
/* copy the stream parameters to the muxer */ ret =
avcodec_parameters_from_context(ost->st->codecpar, c);
Then: how would you call the function? Obviously, "prepare_video_encoding()"
would not be appropriate.
>> Same thing for write_audio_frame(), and in fact a comment is
>> put just above the function, and it says: "encode one audio frame and
>> send it to the muxer,,," ...which is obscure from the function's name
>> (and, again, the user is forced to jump often from a chunk to another
>> chunk of code in order to understand what the code is _generally_
>> doing).Note too that this can't be fixed by using more explicative or
>> longer names, because the functions mixes tasks which are _different_.
>> Therefore, these functions in many cases do not improve readability
>> and IMHO is better to have a longer code instead of forcing grouping
>> different tasks in the same function with an ambiguous name.
>Your argument seems to amount to "the existing structure is imperfect,
>so it is better to have no structure at all",
Avoiding to split code, when not so useful, doesn't mean to "have no structure
at all".The code I pasted has its own precise structure.
with which I disagree.
>And even if you convinced me, adding a whole new example while keeping
>the old one is not a good solution - people would just be confused by
>multiple examples doing the same thing in different ways.
The new example doesn't do the same thing in different ways. In fact:
1) It reads from file, with customizable params (sample rate, sample fmt,
channels ...) and not from a dummy generated audio. 2) It uses a custom I/O
callback for accessing muxed data3) it operates on audio only.
You could argue that some tasks are already inside muxing.c, but this applies
to encode_audio.c too. And having encode_audio.c + muxing.c doesn't confuse
people IMHO.
_______________________________________________
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".
