[FFmpeg-devel] [PATCH 4/6] avfilter/avfilter: Allow to free non-static pads generically

[Nicolas George]

Andreas Rheinhardt (12021-08-21):
> You are distinguishing between filters with allocated input/output names
> and those with static input/output names among those filters with
> dynamic inputs/outputs and don't see a wide margin between allocated and
> static; I am distinguishing between those filters that have both
> allocated and static input (or output) names and those that don't have
> this mix (among inputs or among outputs) and I see an overwhelming
> majority of the latter, as the former group has only two elements
> (actually, one could make the former group empty, by making the inpads
> using static names static). So the easiest is to use a static flag for
> the common default case and to treat the others by other means (either
> completely separately as in the first version of this patch or by
> setting the flags manually for them).

I am sorry, but the fact that you need a whole paragraph full of nested
conditions to explain your point makes it rather unconvincing.

You are looking at it from the point of view of the person doing the
refactoring. That means you have the whole issue very much in mind. You
have to consider the points of view of somebody who writes new code and
somebody who reads the existing code or a proposed patch.

For somebody who writes new code, explaining "the third argument is 1 if
the name is dynamically allocated, 0 if not" is much simpler than
explaining flags that set other flags.

For somebody who reviews the code, the fact that the information is at
the same place is a huge benefit. Otherwise, we must stop reading the
code linearly to go check if the flag was properly set.

And in this instance, even the framework is simpler if you just add a
third argument to append_in/out_pad().

If you are not convinced, we can try the experiment with an innocent
bystander: pick somebody neutral, explain your proposal from scratch, I
explain my proposal from scratch, and we see what they think.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210821/88bd05fd/attachment.sig>