[FFmpeg-devel] [PATCH V5 1/2] configure: sort decoder/encoder/filter/... names in alphabet order

Alexander Strasser eclipse7 at gmx.net
Tue May 7 09:49:04 EEST 2019



Am 7. Mai 2019 04:07:23 MESZ schrieb "Guo, Yejun" <yejun.guo at intel.com>:
>
>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
>Of
>> Alexander Strasser
>> Sent: Tuesday, May 07, 2019 6:21 AM
>> To: FFmpeg development discussions and patches
><ffmpeg-devel at ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH V5 1/2] configure: sort
>> decoder/encoder/filter/... names in alphabet order
>> 
>> On 2019-05-05 21:14 +0000, avih wrote:
>> > > I guess you were looking at the right patch. I mean this one:
[...]
>> > > >
>> > > > In other words: we should write new configure code in awk.
>> > > >
>> > > > Did I misinterpret the statement or its implications?
>> > >
>> > > You got me totally wrong :(
>> >
>> > I'm only human, it happens. But you didn't explain what you
>actually meant.
>> 
>> It's a communication problem. I tried to explain why I prefer
>> the awk version over the shell version of this patch in the email
>> before. Seems I was too vague.
>> 
>> I will try to answer your specific questions. If there is more
>> to discuss, please don't hesitate to ask.
>> 
>> > Specifically:
>> >
>> > - What makes this patch a good candidate to use awk rather than
>shell like
>> >   the rest of configure?
>> 
>> I would like to rephrase this question a bit. I guess the other
>aspect
>> will be in my answer to your next question anyway.
>> 
>> patch1 (awk) configure: print_in_columns: replace pr with awk
>version:
>> http://ffmpeg.org/pipermail/ffmpeg-devel/2019-May/243380.html
>> patch2 (shell) configure: sort decoder/encoder/filter/... names in
>alphabet
>> order (v5 as posted in this thread)
>> 
>> - Why do you prefer patch1 over patch2?
>> 
>> 1. Statistics
>>     * patch1: 1 file changed, 16 insertions(+), 2 deletions(-)
>>     * patch2: 1 file changed, 24 insertions(+), 2 deletions(-)
>
>I have no idea which one is better, but I think this point is not the
>reason.

My fault. I wanted to add that those points are in no particular order and that this one is not a very strong Point; but it's an indication.

>we can refine the shell patch as below with 15 insertions(+), 2
>deletions(-1). Actually, the line number does not mean something. :)
>
>print_in_columns() {
># the input should not contain chars such as '*', otherwise it will be
>expanded.
>    set -- $(tr ' ' '\n' | sort)
>    test $ncols -lt 24 && col_width=$ncols || col_width=24
>    cols=$(($ncols / $col_width))
>    rows=$(($(($# + $cols - 1)) / $cols))
>    cols_seq=$(seq $cols)
>    for row in $(seq $rows); do
>        print_index=$row
>        print_line=""
>        for col in $cols_seq; do
>test $print_index -le $# && eval print_line='"$print_line
>"${'$print_index'}'
>            print_index=$(($print_index + $rows))
>        done
>        printf "%-${col_width}s" $print_line; printf "\n"
>    done | sed 's/ *$//'
>}
>
>the usage of "test && ||" is already used in configure.

Lines can bei sqeezed, but that isn't a very good Idea beyond some point. I'm quite sure the shell Implementation will end up with a few lines more or I will not feel comfortable pushing a patch that has been compacted too much.

As I said above already, that alone isn't a strong reason to prefer patch2. Though all points together are in my opinion.


Best regards,
  Alexander

>> 2. patch2 uses lots of variables (which is good in itself), but those
>>    should be local and we cannot express that portably in shell.
>>    In turn we have raised potential for clashes now or in the future.
>> 3. patch2 uses eval combined with non-trivial quoting, which is hard
>>    to read and hard to grasp quickly. It's not wrong, but it's not as
>>    easy and straight forward as in patch1.
>> 4. Depending on the input (stdin) unexpected things can happen in the
>>    "eval" and "set" lines. One example is given in the comment.
>> 5. patch2 to uses shell for the parts easily expressed in shell and
>>    uses awk for the parts that are non-trivial in shell.
>> 6. The awk implementation should be easier to read and understand for
>>    the majority of readers/developers.
>> 
>> 
>> > - What should be the general criteria to choose a scripting
>language for
>> >   future patches?
>> 
>> I don't see anywhere that we switch away from shell for configure. So
>in
>> general things should be implemented in shell. Shell though is not
>really
>> suited for implementing all kinds of algorithms. OTOH shell is an
>excellent
>> choice for starting processes that communicate via
>stdin/stdout/stderr
>> and plugging them together.
>> 
>> So I think programming shell, sticking only to shell itself
>(builtins) is
>> almost never a good option. Shell is best when coordinating the
>execution
>> of other programs. Sometimes when you do not have a particular
>program
>> around to help you in your shell script, that void needs to be
>filled.
>> For this purpose awk is often a good choice, because it was
>especially
>> designed to be a fit for this purpose amongst other things. Using
>only awk
>> to implement bigger systems or as a general purpose programming
>language
>> is IMHO nearly as wrong as using shell-only.
>> 
>> 
>> >     On Saturday, May 4, 2019 10:43 PM, Alexander Strasser
>> <eclipse7 at gmx.net> wrote:
>> [...]
>> 
>> 
>> I'm feeling like I might not have expressed my reasoning good and
>> precise enough in every level of detail. I apologize for that. My
>> natural language skills have their limits and I am not a native
>> English speaker. I will try to refine specific points, should you
>> or anyone else have more questions or comments.



More information about the ffmpeg-devel mailing list