Hi both of you,
nice work iterating on this patch set!
I want to discuss a little more, please don't take my comments
the wrong way, I just couldn't get back to this discussion earlier.
On 2019-04-24 13:36 +0000, Guo, Yejun wrote:
> >
> > From: avih [mailto:avih...@yahoo.com]
> > Sent: Wednesday, April 24, 2019 9:22 PM
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Cc: Guo, Yejun <yejun....@intel.com>
> > Subject: Re: [FFmpeg-devel] [PATCH V5 1/2] configure: sort
> > decoder/encoder/filter/... names in alphabet order
> >
> > > print_in_columns() {
> > > - cols=$(expr $ncols / 24)
> > > - cat | tr ' ' '\n' | sort | pr -r "-$cols" -w $ncols -t
> > > + # the input should not contain chars such as '*',
> > > + # otherwise, '*' will be expanded to be all files in the current
> > > + # working directory which don't begin with a dot (`.`)
> > > + set -- $(tr ' ' '\n' | sort)
> > > + col_width=24
> > > + if [ $ncols -lt $col_width ]; then
> > > + col_width=$ncols
> > > + fi
> > > + cols=$(($ncols / $col_width))
> > > + rows=$(($(($# + $cols - 1)) / $cols))
> > > + cols_seq=$(seq $cols)
> > > + rows_seq=$(seq $rows)
> > > + for row in $rows_seq; do
> > > + print_index=$row
> > > + print_line=""
> > > + for col in $cols_seq; do
> > > + if [ $print_index -le $# ]; then
> > > + eval print_line='"$print_line "${'$print_index'}'
> > > + fi
> > > + print_index=$(($print_index + $rows))
> > > + done
> > > + printf "%-${col_width}s" $print_line
> > > + printf "\n"
> > > + done | sed 's/ *$//'
> > > }
> >
> > Looks good to me. No further comments (but I don't push).
> >
> > Next time, know that you can use e.g. `$((x + y))` instead of `$(($x +
> > $y))`,
> > though in this case it doesn't matter and not worth another version.
>
> got it, thanks.
What do you think about using awk instead of shell?
I have 2 POC patches attached. It's probably not 100% correct yet,
but it kind of demonstrates what it would look like.
The main advantage of using awk, is that it's more elegant and
shorter. It's probably also less risky, because it's more isolated,
e.g. as it was explained by avih, there is no widely supported way
for locals across shells. We already use awk in configure for
mandatory functions, so it's no new dependency.
Please comment on the awk approach.
I'm not against the shell way, or a mixed approach, but before going
either way and pushing I would rather have some more testing;
especially on more exotic platforms.
Thank you
Alexander
From d04914411ec07cba29ae9b3cd43408f92c344958 Mon Sep 17 00:00:00 2001
From: Alexander Strasser <eclip...@gmx.net>
Date: Sat, 27 Apr 2019 23:15:08 +0200
Subject: [PATCH 1/2] configure: print_in_columns: Replace pr with awk
Get rid of pr dependency and write the columns strictly
alphabetical for the given rows.
Before pr would attempt to write pages, thus if a page
boundary was reached, the output looked confusing as one
couldn't see there was a new page and the alphabetical
order was disrupted when scanning down one of the columns.
Fixes part of ticket #TODO
---
configure | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index b122b27268..7e739b3e55 100755
--- a/configure
+++ b/configure
@@ -3831,8 +3831,24 @@ die_unknown(){
}
print_in_columns() {
- cols=$(expr $ncols / 24)
- cat | tr ' ' '\n' | sort | pr -r "-$cols" -w $ncols -t
+ sort | tr '\n' ' ' | awk -v width="$ncols" '
+ {
+ num_cols = width / 24; num_rows = (NF + num_cols-1) / num_cols;
+ y = x = 1;
+ for (i = 1; i <= NF; i++) {
+ if (y > num_rows) {
+ y = 1; x++;
+ }
+ d[x,y] = $i;
+ y++;
+ }
+ for (y = 1; y <= num_rows; y++) {
+ for (x = 1; x <= num_cols; x++) {
+ line = sprintf(x != num_cols ? "%s%-24s" : "%s%s", line, d[x,y]);
+ }
+ print line; line = "";
+ }
+ }'
}
show_list() {
--
2.20.1
From 7dc4043db4ea32f6e85b17b6ec8060dac8c85f6b Mon Sep 17 00:00:00 2001
From: Alexander Strasser <eclip...@gmx.net>
Date: Sat, 27 Apr 2019 01:06:59 +0200
Subject: [PATCH 2/2] configure: log_file: Replace pr with awk invocation
Fixes remaining part of ticket #TODO
---
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 7e739b3e55..491d4d50e9 100755
--- a/configure
+++ b/configure
@@ -503,7 +503,7 @@ log(){
log_file(){
log BEGIN $1
- pr -n -t $1 >> $logfile
+ awk '{ printf("%5d\t%s\n", NR, $0) }' $1 >> $logfile
log END $1
}
--
2.20.1
_______________________________________________
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".
