| Message ID | 20190828153237.30155-1-andrey.semashev@gmail.com |
|---|---|
| State | Accepted |
| Commit | 6d9d053edbedbb0d9fc0a97141b7524d8658be58 |
| Headers | show |
Ping? On 2019-08-28 18:32, Andrey Semashev wrote: > Because lavf_container is sometimes called with only 2 arguments, > fate tests produce bash errors like this: > > tests/fate-run.sh: 299: test: =: unexpected operator > > This commit fixes this. > --- > tests/fate-run.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/fate-run.sh b/tests/fate-run.sh > index 2f1991da52..aec12c16a3 100755 > --- a/tests/fate-run.sh > +++ b/tests/fate-run.sh > @@ -296,7 +296,7 @@ lavf_container(){ > outdir="tests/data/lavf" > file=${outdir}/lavf.$t > do_avconv $file $DEC_OPTS -f image2 -c:v pgmyuv -i $raw_src $DEC_OPTS -ar 44100 -f s16le $1 -i $pcm_src "$ENC_OPTS -metadata title=lavftest" -b:a 64k -t 1 -qscale:v 10 $2 > - test $3 = "disable_crc" || > + test "$3" = "disable_crc" || > do_avconv_crc $file $DEC_OPTS -i $target_path/$file $3 > } >
Could someone take a look at this small fix, please? I'd like it to make it into 4.2.1. On 2019-09-02 13:54, Andrey Semashev wrote: > Ping? > > On 2019-08-28 18:32, Andrey Semashev wrote: >> Because lavf_container is sometimes called with only 2 arguments, >> fate tests produce bash errors like this: >> >> tests/fate-run.sh: 299: test: =: unexpected operator >> >> This commit fixes this. >> --- >> tests/fate-run.sh | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tests/fate-run.sh b/tests/fate-run.sh >> index 2f1991da52..aec12c16a3 100755 >> --- a/tests/fate-run.sh >> +++ b/tests/fate-run.sh >> @@ -296,7 +296,7 @@ lavf_container(){ >> outdir="tests/data/lavf" >> file=${outdir}/lavf.$t >> do_avconv $file $DEC_OPTS -f image2 -c:v pgmyuv -i $raw_src >> $DEC_OPTS -ar 44100 -f s16le $1 -i $pcm_src "$ENC_OPTS -metadata >> title=lavftest" -b:a 64k -t 1 -qscale:v 10 $2 >> - test $3 = "disable_crc" || >> + test "$3" = "disable_crc" || >> do_avconv_crc $file $DEC_OPTS -i $target_path/$file $3 >> } > >
On Wed, Aug 28, 2019 at 06:32:37PM +0300, Andrey Semashev wrote: > Because lavf_container is sometimes called with only 2 arguments, > fate tests produce bash errors like this: > > tests/fate-run.sh: 299: test: =: unexpected operator > > This commit fixes this. > --- > tests/fate-run.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I think this change is correct but shell is not my area ... [...]
On Sat, Sep 07, 2019 at 05:19:55PM +0200, Michael Niedermayer wrote: > On Wed, Aug 28, 2019 at 06:32:37PM +0300, Andrey Semashev wrote: > > Because lavf_container is sometimes called with only 2 arguments, > > fate tests produce bash errors like this: > > > > tests/fate-run.sh: 299: test: =: unexpected operator > > > > This commit fixes this. > > --- > > tests/fate-run.sh | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > I think this change is correct but shell is not my area ... If the $2 is two arguments between with space, the patch is needed. Why I haven't got such errors when I'm running fate? > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Its not that you shouldnt use gotos but rather that you should write > readable code and code with gotos often but not always is less readable > _______________________________________________ > 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".
On 2019-09-07 18:32, Limin Wang wrote: > On Sat, Sep 07, 2019 at 05:19:55PM +0200, Michael Niedermayer wrote: >> On Wed, Aug 28, 2019 at 06:32:37PM +0300, Andrey Semashev wrote: >>> Because lavf_container is sometimes called with only 2 arguments, >>> fate tests produce bash errors like this: >>> >>> tests/fate-run.sh: 299: test: =: unexpected operator >>> >>> This commit fixes this. >>> --- >>> tests/fate-run.sh | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> I think this change is correct but shell is not my area ... > If the $2 is two arguments between with space, the patch is needed. $2 is a single argument that may contain spaces. It is interpreted as a single argument as long as it is enclosed in quotes in lavf_container invokation. The problem is caused not by $2 but by the fact there is no $3. See lavf_container_attach, lavf_container_timecode_nodrop, lavf_container_timecode_drop and lavf_container_timecode functions, as well as a few tests in tests/fate/lavf-container.mak. > Why I haven't got such errors when I'm running fate? I noticed the errors when the tests failed (due to my local changes not relevant to this patch). I don't know the details about how tests are run, but maybe the output is suppressed when the tests pass?
On Sun, Sep 08, 2019 at 12:46:25AM +0300, Andrey Semashev wrote: > On 2019-09-07 18:32, Limin Wang wrote: > >On Sat, Sep 07, 2019 at 05:19:55PM +0200, Michael Niedermayer wrote: > >>On Wed, Aug 28, 2019 at 06:32:37PM +0300, Andrey Semashev wrote: > >>>Because lavf_container is sometimes called with only 2 arguments, > >>>fate tests produce bash errors like this: > >>> > >>> tests/fate-run.sh: 299: test: =: unexpected operator > >>> > >>>This commit fixes this. > >>>--- > >>> tests/fate-run.sh | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >>I think this change is correct but shell is not my area ... > >If the $2 is two arguments between with space, the patch is needed. > > $2 is a single argument that may contain spaces. It is interpreted > as a single argument as long as it is enclosed in quotes in > lavf_container invokation. > > The problem is caused not by $2 but by the fact there is no $3. See > lavf_container_attach, lavf_container_timecode_nodrop, > lavf_container_timecode_drop and lavf_container_timecode functions, > as well as a few tests in tests/fate/lavf-container.mak. > > >Why I haven't got such errors when I'm running fate? > > I noticed the errors when the tests failed (due to my local changes > not relevant to this patch). I don't know the details about how > tests are run, but maybe the output is suppressed when the tests > pass? The patch looks good to me, but I don't have push access. Please ask for other developer to push it. > _______________________________________________ > 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".
On Sun, Sep 08, 2019 at 06:45:30PM +0800, Limin Wang wrote: > On Sun, Sep 08, 2019 at 12:46:25AM +0300, Andrey Semashev wrote: > > On 2019-09-07 18:32, Limin Wang wrote: > > >On Sat, Sep 07, 2019 at 05:19:55PM +0200, Michael Niedermayer wrote: > > >>On Wed, Aug 28, 2019 at 06:32:37PM +0300, Andrey Semashev wrote: > > >>>Because lavf_container is sometimes called with only 2 arguments, > > >>>fate tests produce bash errors like this: > > >>> > > >>> tests/fate-run.sh: 299: test: =: unexpected operator > > >>> > > >>>This commit fixes this. > > >>>--- > > >>> tests/fate-run.sh | 2 +- > > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >>I think this change is correct but shell is not my area ... > > >If the $2 is two arguments between with space, the patch is needed. > > > > $2 is a single argument that may contain spaces. It is interpreted > > as a single argument as long as it is enclosed in quotes in > > lavf_container invokation. > > > > The problem is caused not by $2 but by the fact there is no $3. See > > lavf_container_attach, lavf_container_timecode_nodrop, > > lavf_container_timecode_drop and lavf_container_timecode functions, > > as well as a few tests in tests/fate/lavf-container.mak. > > > > >Why I haven't got such errors when I'm running fate? > > > > I noticed the errors when the tests failed (due to my local changes > > not relevant to this patch). I don't know the details about how > > tests are run, but maybe the output is suppressed when the tests > > pass? > The patch looks good to me, but I don't have push access. Please ask for > other developer to push it. will apply thx [...]
diff --git a/tests/fate-run.sh b/tests/fate-run.sh index 2f1991da52..aec12c16a3 100755 --- a/tests/fate-run.sh +++ b/tests/fate-run.sh @@ -296,7 +296,7 @@ lavf_container(){ outdir="tests/data/lavf" file=${outdir}/lavf.$t do_avconv $file $DEC_OPTS -f image2 -c:v pgmyuv -i $raw_src $DEC_OPTS -ar 44100 -f s16le $1 -i $pcm_src "$ENC_OPTS -metadata title=lavftest" -b:a 64k -t 1 -qscale:v 10 $2 - test $3 = "disable_crc" || + test "$3" = "disable_crc" || do_avconv_crc $file $DEC_OPTS -i $target_path/$file $3 }