[FFmpeg-devel,V5,2/2] configure: replace 'pr' with printf since busybox does not support pr

Submitted by Guo, Yejun on April 24, 2019, 1:13 p.m.

Details

Message ID 1556111601-7107-1-git-send-email-yejun.guo@intel.com
State New
Headers show

Commit Message

Guo, Yejun April 24, 2019, 1:13 p.m.
This patch is based on https://trac.ffmpeg.org/ticket/5680 provided by
Kylie McClain <somasis@exherbo.org> at Wed, 29 Jun 2016 16:37:20 -0400,
and have some changes.

contributor: Kylie McClain <somasis@exherbo.org>
contributor: avih <avihpit@yahoo.com>
Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
---
 configure | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

avih April 24, 2019, 1:22 p.m.
>  log_file(){
> -    log BEGIN $1
> -    pr -n -t $1 >> $logfile
> -    log END $1
> +    log BEGIN "$1"
> +    log_file_i=1
> +    while IFS= read -r log_file_line;do
> +        printf '%5s  %s\n' "${log_file_i}" "${log_file_line}"
> +        log_file_i=$(($log_file_i+1))
> +    done < "$1" >> "$logfile"
> +    log END "$1"
> }

Looks good to me, no further comments (but I don't push).
Guo, Yejun April 28, 2019, 12:38 a.m.
> From: avih [mailto:avihpit@yahoo.com]

> Sent: Wednesday, April 24, 2019 9:23 PM

> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>

> Cc: Guo, Yejun <yejun.guo@intel.com>

> Subject: Re: [FFmpeg-devel] [PATCH V5 2/2] configure: replace 'pr' with printf

> since busybox does not support pr

> 

> >  log_file(){

> > -    log BEGIN $1

> > -    pr -n -t $1 >> $logfile

> > -    log END $1

> > +    log BEGIN "$1"

> > +    log_file_i=1

> > +    while IFS= read -r log_file_line;do

> > +        printf '%5s  %s\n' "${log_file_i}" "${log_file_line}"

> > +        log_file_i=$(($log_file_i+1))

> > +    done < "$1" >> "$logfile"

> > +    log END "$1"

> > }

> 

> Looks good to me, no further comments (but I don't push).


this patch set asks for push, or more comments, thanks.
Alexander Strasser May 4, 2019, 7:42 p.m.
Hi all!

On 2019-04-28 00:38 +0000, Guo, Yejun wrote:
> > From: avih [mailto:avihpit@yahoo.com]
> > Sent: Wednesday, April 24, 2019 9:23 PM
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Cc: Guo, Yejun <yejun.guo@intel.com>
> > Subject: Re: [FFmpeg-devel] [PATCH V5 2/2] configure: replace 'pr' with printf
> > since busybox does not support pr
> >
> > >  log_file(){
> > > -    log BEGIN $1
> > > -    pr -n -t $1 >> $logfile
> > > -    log END $1
> > > +    log BEGIN "$1"
> > > +    log_file_i=1
> > > +    while IFS= read -r log_file_line;do


> > > +        printf '%5s  %s\n' "${log_file_i}" "${log_file_line}"

I would like to do minimal adjustment to the line quoted above:

               printf '%5d\t%s\n' "$log_file_i" "$log_file_line"

The \t makes the output equal to the current output. I would
prefer the %d because it makes the format a bit easier to grasp.

The removed {} pairs around log_file_i and log_file_line, aren't
needed and without them the style should be more consistent.


> > > +        log_file_i=$(($log_file_i+1))
> > > +    done < "$1" >> "$logfile"
> > > +    log END "$1"
> > > }
> >
> > Looks good to me, no further comments (but I don't push).
>
> this patch set asks for push, or more comments, thanks.

It's faster than the current pr implementation.

If there are no objections to this patch in general and
to my suggested modifications in particular, I intent
to push it next week on friday.


Thanks
  Alexander
Guo, Yejun May 5, 2019, 1:38 a.m.
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of

> Alexander Strasser

> Sent: Sunday, May 05, 2019 3:42 AM

> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>

> Subject: Re: [FFmpeg-devel] [PATCH V5 2/2] configure: replace 'pr' with printf

> since busybox does not support pr

> 

> Hi all!

> 

> On 2019-04-28 00:38 +0000, Guo, Yejun wrote:

> > > From: avih [mailto:avihpit@yahoo.com]

> > > Sent: Wednesday, April 24, 2019 9:23 PM

> > > To: FFmpeg development discussions and patches

> <ffmpeg-devel@ffmpeg.org>

> > > Cc: Guo, Yejun <yejun.guo@intel.com>

> > > Subject: Re: [FFmpeg-devel] [PATCH V5 2/2] configure: replace 'pr' with

> printf

> > > since busybox does not support pr

> > >

> > > >  log_file(){

> > > > -    log BEGIN $1

> > > > -    pr -n -t $1 >> $logfile

> > > > -    log END $1

> > > > +    log BEGIN "$1"

> > > > +    log_file_i=1

> > > > +    while IFS= read -r log_file_line;do

> 

> 

> > > > +        printf '%5s  %s\n' "${log_file_i}" "${log_file_line}"

> 

> I would like to do minimal adjustment to the line quoted above:

> 

>                printf '%5d\t%s\n' "$log_file_i" "$log_file_line"


It's good.

> 

> The \t makes the output equal to the current output. I would

> prefer the %d because it makes the format a bit easier to grasp.

> 

> The removed {} pairs around log_file_i and log_file_line, aren't

> needed and without them the style should be more consistent.

> 

> 

> > > > +        log_file_i=$(($log_file_i+1))

> > > > +    done < "$1" >> "$logfile"

> > > > +    log END "$1"

> > > > }

> > >

> > > Looks good to me, no further comments (but I don't push).

> >

> > this patch set asks for push, or more comments, thanks.

> 

> It's faster than the current pr implementation.

> 

> If there are no objections to this patch in general and

> to my suggested modifications in particular, I intent

> to push it next week on friday.

> 

> 

> Thanks

>   Alexander

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

> 

> To unsubscribe, visit link above, or email

> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Alexander Strasser May 15, 2019, 9:10 p.m.
On 2019-05-05 01:38 +0000, Guo, Yejun wrote:
>
> > -----Original Message-----
> > Alexander Strasser
> > Sent: Sunday, May 05, 2019 3:42 AM
> >
> > On 2019-04-28 00:38 +0000, Guo, Yejun wrote:
> > > > From: avih [mailto:avihpit@yahoo.com]
> > > > Sent: Wednesday, April 24, 2019 9:23 PM
> > > > To: FFmpeg development discussions and patches
> > <ffmpeg-devel@ffmpeg.org>
> > > > Cc: Guo, Yejun <yejun.guo@intel.com>
> > > > Subject: Re: [FFmpeg-devel] [PATCH V5 2/2] configure: replace 'pr' with
> > printf
> > > > since busybox does not support pr
> > > >
> > > > >  log_file(){
> > > > > -    log BEGIN $1
> > > > > -    pr -n -t $1 >> $logfile
> > > > > -    log END $1
> > > > > +    log BEGIN "$1"
> > > > > +    log_file_i=1
> > > > > +    while IFS= read -r log_file_line;do
> >
> >
> > > > > +        printf '%5s  %s\n' "${log_file_i}" "${log_file_line}"
> >
> > I would like to do minimal adjustment to the line quoted above:
> >
> >                printf '%5d\t%s\n' "$log_file_i" "$log_file_line"
>
> It's good.
>
> >
> > The \t makes the output equal to the current output. I would
> > prefer the %d because it makes the format a bit easier to grasp.
> >
> > The removed {} pairs around log_file_i and log_file_line, aren't
> > needed and without them the style should be more consistent.
> >
> >
> > > > > +        log_file_i=$(($log_file_i+1))
> > > > > +    done < "$1" >> "$logfile"
> > > > > +    log END "$1"
> > > > > }
> > > >
> > > > Looks good to me, no further comments (but I don't push).
> > >
> > > this patch set asks for push, or more comments, thanks.
> >
> > It's faster than the current pr implementation.
> >
> > If there are no objections to this patch in general and
> > to my suggested modifications in particular, I intent
> > to push it next week on friday.

Pushed.

Thanks to Yejun and Avi.


  Alexander

Patch hide | download patch | download mbox

diff --git a/configure b/configure
index ad1b183..4215765 100755
--- a/configure
+++ b/configure
@@ -502,9 +502,13 @@  log(){
 }
 
 log_file(){
-    log BEGIN $1
-    pr -n -t $1 >> $logfile
-    log END $1
+    log BEGIN "$1"
+    log_file_i=1
+    while IFS= read -r log_file_line;do
+        printf '%5s   %s\n' "${log_file_i}" "${log_file_line}"
+        log_file_i=$(($log_file_i+1))
+    done < "$1" >> "$logfile"
+    log END "$1"
 }
 
 warn(){