| Message ID | MN2PR04MB5981654844F0AFE21023D65ABAD59@MN2PR04MB5981.namprd04.prod.outlook.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [FFmpeg-devel] Why does this break FATE? | expand |
| Context | Check | Description |
|---|---|---|
| andriy/commit_msg_x86 | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | fail | Make fate failed |
| andriy/commit_msg_ppc | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | fail | Make fate failed |
On 9/8/2021 10:14 PM, Soft Works wrote: > Test seek-lavf-asf failed. Look at tests/data/fate/seek-lavf-asf.err for details. > make: *** [/build/ffmpeg-git/tests/Makefile:256: fate-seek-lavf-asf] Error 139 > > $ cat tests/data/fate/seek-lavf-asf.err > /build/ffmpeg-git/tests/fate-run.sh: line 78: 21786 Segmentation fault $target_exec $target_path/"$@" > > > It's the same on both Windows/MSYS2 and Linux. Let's see how patchwork results will be... Please, don't send patches just to have patchwork run FATE for you. It litters the mailing list. Do it locally. > > > Signed-off-by: softworkz <softworkz@hotmail.com> > --- > libavcodec/version.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/version.h b/libavcodec/version.h > index 83db2b242a..d162607f4b 100644 > --- a/libavcodec/version.h > +++ b/libavcodec/version.h > @@ -27,9 +27,9 @@ > > #include "libavutil/version.h" > > -#define LIBAVCODEC_VERSION_MAJOR 59 > -#define LIBAVCODEC_VERSION_MINOR 7 > -#define LIBAVCODEC_VERSION_MICRO 102 > +#define LIBAVCODEC_VERSION_MAJOR 60 Bumping major version of any library without the required considerations will more likely than not break many tests and scenarios. If you just want to bump version for a new addition or change, bump minor and/or micro. > +#define LIBAVCODEC_VERSION_MINOR 0 > +#define LIBAVCODEC_VERSION_MICRO 100 > > #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ > LIBAVCODEC_VERSION_MINOR, \ >
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > James Almer > Sent: Thursday, 9 September 2021 03:18 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] Why does this break FATE? > > On 9/8/2021 10:14 PM, Soft Works wrote: > > Test seek-lavf-asf failed. Look at tests/data/fate/seek-lavf- > asf.err for details. > > make: *** [/build/ffmpeg-git/tests/Makefile:256: fate-seek-lavf- > asf] Error 139 > > > > $ cat tests/data/fate/seek-lavf-asf.err > > /build/ffmpeg-git/tests/fate-run.sh: line 78: 21786 Segmentation > fault $target_exec $target_path/"$@" > > > > > > It's the same on both Windows/MSYS2 and Linux. Let's see how > patchwork results will be... > > Please, don't send patches just to have patchwork run FATE for you. > It > litters the mailing list. Do it locally. As written above I _did_ run it locally on both Linux and Windows/MSYS. > > --- > > libavcodec/version.h | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/libavcodec/version.h b/libavcodec/version.h > > index 83db2b242a..d162607f4b 100644 > > --- a/libavcodec/version.h > > +++ b/libavcodec/version.h > > @@ -27,9 +27,9 @@ > > > > #include "libavutil/version.h" > > > > -#define LIBAVCODEC_VERSION_MAJOR 59 > > -#define LIBAVCODEC_VERSION_MINOR 7 > > -#define LIBAVCODEC_VERSION_MICRO 102 > > +#define LIBAVCODEC_VERSION_MAJOR 60 > > Bumping major version of any library without the required > considerations > will more likely than not break many tests and scenarios. > If you just want to bump version for a new addition or change, bump > minor and/or micro. The patch I'm working on requires a major version bump, that's why I had it included in the commit where it belongs to. Needless to say that it was the very last thing I suspected for causing this. So how can this be that a version change without anything else can cause a test to fail?` softworkz PS: Patchwork shows the same error
On 9/8/2021 10:29 PM, Soft Works wrote: > > >> -----Original Message----- >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of >> James Almer >> Sent: Thursday, 9 September 2021 03:18 >> To: ffmpeg-devel@ffmpeg.org >> Subject: Re: [FFmpeg-devel] [PATCH] Why does this break FATE? >> >> On 9/8/2021 10:14 PM, Soft Works wrote: >>> Test seek-lavf-asf failed. Look at tests/data/fate/seek-lavf- >> asf.err for details. >>> make: *** [/build/ffmpeg-git/tests/Makefile:256: fate-seek-lavf- >> asf] Error 139 >>> >>> $ cat tests/data/fate/seek-lavf-asf.err >>> /build/ffmpeg-git/tests/fate-run.sh: line 78: 21786 Segmentation >> fault $target_exec $target_path/"$@" >>> >>> >>> It's the same on both Windows/MSYS2 and Linux. Let's see how >> patchwork results will be... >> >> Please, don't send patches just to have patchwork run FATE for you. >> It >> litters the mailing list. Do it locally. > > As written above I _did_ run it locally on both Linux and Windows/MSYS. That was more than enough to know that the failure you saw was not a fluke. > >>> --- >>> libavcodec/version.h | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/libavcodec/version.h b/libavcodec/version.h >>> index 83db2b242a..d162607f4b 100644 >>> --- a/libavcodec/version.h >>> +++ b/libavcodec/version.h >>> @@ -27,9 +27,9 @@ >>> >>> #include "libavutil/version.h" >>> >>> -#define LIBAVCODEC_VERSION_MAJOR 59 >>> -#define LIBAVCODEC_VERSION_MINOR 7 >>> -#define LIBAVCODEC_VERSION_MICRO 102 >>> +#define LIBAVCODEC_VERSION_MAJOR 60 >> >> Bumping major version of any library without the required >> considerations >> will more likely than not break many tests and scenarios. >> If you just want to bump version for a new addition or change, bump >> minor and/or micro. > > The patch I'm working on requires a major version bump, that's why > I had it included in the commit where it belongs to. Since we haven't had a release since the last major bump, we can still apply ABI (not API) breaking changes, if needed. So send your patch for consideration, without bumping major. > > Needless to say that it was the very last thing I suspected for > causing this. > > So how can this be that a version change without anything else > can cause a test to fail?` > > > softworkz > > PS: Patchwork shows the same error > _______________________________________________ > 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". >
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > James Almer > Sent: Thursday, 9 September 2021 03:34 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] Why does this break FATE? > > On 9/8/2021 10:29 PM, Soft Works wrote: > > > > > >> -----Original Message----- > >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > >> James Almer > >> Sent: Thursday, 9 September 2021 03:18 > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] [PATCH] Why does this break FATE? > >> > >> On 9/8/2021 10:14 PM, Soft Works wrote: > >>> Test seek-lavf-asf failed. Look at tests/data/fate/seek-lavf- > >> asf.err for details. > >>> make: *** [/build/ffmpeg-git/tests/Makefile:256: fate-seek-lavf- > >> asf] Error 139 > >>> > >>> $ cat tests/data/fate/seek-lavf-asf.err > >>> /build/ffmpeg-git/tests/fate-run.sh: line 78: 21786 Segmentation > >> fault $target_exec $target_path/"$@" > >>> > >>> > >>> It's the same on both Windows/MSYS2 and Linux. Let's see how > >> patchwork results will be... > >> > >> Please, don't send patches just to have patchwork run FATE for > you. > >> It > >> litters the mailing list. Do it locally. > > > > As written above I _did_ run it locally on both Linux and > Windows/MSYS. > > That was more than enough to know that the failure you saw was not a > fluke. Might be true, but it doesn't seem right to me that this is happening and submitting a patch for demonstration seemed to be an adequate measure to present the issue. > Since we haven't had a release since the last major bump, we can > still > apply ABI (not API) breaking changes, if needed. Based on the fact that requirements are strict about MINOR bumps and mandating them to be included in the very commit that is requiring the bump, I didn't expect that there's a different strategy for MAJOR bumps. Anyway - will use minor bumps, then. Thanks, softworkz
On 9/8/2021 10:49 PM, Soft Works wrote: > > >> -----Original Message----- >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of >> James Almer >> Sent: Thursday, 9 September 2021 03:34 >> To: ffmpeg-devel@ffmpeg.org >> Subject: Re: [FFmpeg-devel] [PATCH] Why does this break FATE? >> >> On 9/8/2021 10:29 PM, Soft Works wrote: >>> >>> >>>> -----Original Message----- >>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of >>>> James Almer >>>> Sent: Thursday, 9 September 2021 03:18 >>>> To: ffmpeg-devel@ffmpeg.org >>>> Subject: Re: [FFmpeg-devel] [PATCH] Why does this break FATE? >>>> >>>> On 9/8/2021 10:14 PM, Soft Works wrote: >>>>> Test seek-lavf-asf failed. Look at tests/data/fate/seek-lavf- >>>> asf.err for details. >>>>> make: *** [/build/ffmpeg-git/tests/Makefile:256: fate-seek-lavf- >>>> asf] Error 139 >>>>> >>>>> $ cat tests/data/fate/seek-lavf-asf.err >>>>> /build/ffmpeg-git/tests/fate-run.sh: line 78: 21786 Segmentation >>>> fault $target_exec $target_path/"$@" >>>>> >>>>> >>>>> It's the same on both Windows/MSYS2 and Linux. Let's see how >>>> patchwork results will be... >>>> >>>> Please, don't send patches just to have patchwork run FATE for >> you. >>>> It >>>> litters the mailing list. Do it locally. >>> >>> As written above I _did_ run it locally on both Linux and >> Windows/MSYS. >> >> That was more than enough to know that the failure you saw was not a >> fluke. > > Might be true, but it doesn't seem right to me that this is happening > and submitting a patch for demonstration seemed to be an adequate > measure to present the issue. > >> Since we haven't had a release since the last major bump, we can >> still >> apply ABI (not API) breaking changes, if needed. > > Based on the fact that requirements are strict about MINOR bumps and > mandating them to be included in the very commit that is requiring > the bump, I didn't expect that there's a different strategy for > MAJOR bumps. A major bump is done once every few years to remove deprecated APIs and open the ABI for changes. After a major bump takes place, there's an "Unstable ABI" period where one can make such breaking changes (Altering field offsets in public structs, adding new fields or changing their types on structs that have their size tied to the ABI, changing public enum and #define values, etc). A single major bump should encompass every breaking change during this short "unstable" period. In contrast, every new API addition requires its own minor or micro bump. > > Anyway - will use minor bumps, then. > > Thanks, > softworkz > > _______________________________________________ > 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". >
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > James Almer > Sent: Thursday, 9 September 2021 03:57 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] Why does this break FATE? > [..] > > > > Based on the fact that requirements are strict about MINOR bumps > and > > mandating them to be included in the very commit that is requiring > > the bump, I didn't expect that there's a different strategy for > > MAJOR bumps. > > A major bump is done once every few years to remove deprecated APIs > and > open the ABI for changes. After a major bump takes place, there's an > "Unstable ABI" period where one can make such breaking changes > (Altering > field offsets in public structs, adding new fields or changing their > types on structs that have their size tied to the ABI, changing > public > enum and #define values, etc). > > A single major bump should encompass every breaking change during > this > short "unstable" period. Why does there have to be an "unstable" period instead of making the MAJOR bumps directly in those commits that are breaking ABI compatibility, Is it about "saving" numbers? softworkz
On 9/8/2021 11:24 PM, Soft Works wrote: > > >> -----Original Message----- >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of >> James Almer >> Sent: Thursday, 9 September 2021 03:57 >> To: ffmpeg-devel@ffmpeg.org >> Subject: Re: [FFmpeg-devel] [PATCH] Why does this break FATE? >> > > [..] > >>> >>> Based on the fact that requirements are strict about MINOR bumps >> and >>> mandating them to be included in the very commit that is requiring >>> the bump, I didn't expect that there's a different strategy for >>> MAJOR bumps. >> >> A major bump is done once every few years to remove deprecated APIs >> and >> open the ABI for changes. After a major bump takes place, there's an >> "Unstable ABI" period where one can make such breaking changes >> (Altering >> field offsets in public structs, adding new fields or changing their >> types on structs that have their size tied to the ABI, changing >> public >> enum and #define values, etc). >> >> A single major bump should encompass every breaking change during >> this >> short "unstable" period. > > Why does there have to be an "unstable" period instead of making the > MAJOR bumps directly in those commits that are breaking ABI compatibility, > Is it about "saving" numbers? To keep the bumping of major revision to a single number every few years, yes. We decided to go the opposite way of x264. Also, the FF_API defines would need to be updated way too often otherwise. > > softworkz > _______________________________________________ > 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". >
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > James Almer > Sent: Thursday, 9 September 2021 04:31 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] Why does this break FATE? > > On 9/8/2021 11:24 PM, Soft Works wrote: > > > > > >> -----Original Message----- > >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > >> James Almer > >> Sent: Thursday, 9 September 2021 03:57 > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] [PATCH] Why does this break FATE? > >> > > > > [..] > > > >>> > >>> Based on the fact that requirements are strict about MINOR bumps > >> and > >>> mandating them to be included in the very commit that is > requiring > >>> the bump, I didn't expect that there's a different strategy for > >>> MAJOR bumps. > >> > >> A major bump is done once every few years to remove deprecated > APIs > >> and > >> open the ABI for changes. After a major bump takes place, there's > an > >> "Unstable ABI" period where one can make such breaking changes > >> (Altering > >> field offsets in public structs, adding new fields or changing > their > >> types on structs that have their size tied to the ABI, changing > >> public > >> enum and #define values, etc). > >> > >> A single major bump should encompass every breaking change during > >> this > >> short "unstable" period. > > > > Why does there have to be an "unstable" period instead of making > the > > MAJOR bumps directly in those commits that are breaking ABI > compatibility, > > Is it about "saving" numbers? > > To keep the bumping of major revision to a single number every few > years, yes. We decided to go the opposite way of x264. > Also, the FF_API defines would need to be updated way too often > otherwise. OK understood. Thanks a lot for explaining! sw
Soft Works: > Test seek-lavf-asf failed. Look at tests/data/fate/seek-lavf-asf.err for details. > make: *** [/build/ffmpeg-git/tests/Makefile:256: fate-seek-lavf-asf] Error 139 > > $ cat tests/data/fate/seek-lavf-asf.err > /build/ffmpeg-git/tests/fate-run.sh: line 78: 21786 Segmentation fault $target_exec $target_path/"$@" > > > It's the same on both Windows/MSYS2 and Linux. Let's see how patchwork results will be... > > > Signed-off-by: softworkz <softworkz@hotmail.com> > --- > libavcodec/version.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/version.h b/libavcodec/version.h > index 83db2b242a..d162607f4b 100644 > --- a/libavcodec/version.h > +++ b/libavcodec/version.h > @@ -27,9 +27,9 @@ > > #include "libavutil/version.h" > > -#define LIBAVCODEC_VERSION_MAJOR 59 > -#define LIBAVCODEC_VERSION_MINOR 7 > -#define LIBAVCODEC_VERSION_MICRO 102 > +#define LIBAVCODEC_VERSION_MAJOR 60 > +#define LIBAVCODEC_VERSION_MINOR 0 > +#define LIBAVCODEC_VERSION_MICRO 100 > > #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ > LIBAVCODEC_VERSION_MINOR, \ > It segfaults because asf_read_pts() uses an uninitialized stack packet which av_read_frame() unrefs as soon as FF_API_INIT_PACKET is 0 (which your bump did). (And actually you could have easily found this out yourself by just using a debugger.) - Andreas
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Andreas Rheinhardt > Sent: Thursday, 9 September 2021 11:50 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] Why does this break FATE? > > Soft Works: > > Test seek-lavf-asf failed. Look at tests/data/fate/seek-lavf- > asf.err for details. > > make: *** [/build/ffmpeg-git/tests/Makefile:256: fate-seek-lavf- > asf] Error 139 > > > > $ cat tests/data/fate/seek-lavf-asf.err > > /build/ffmpeg-git/tests/fate-run.sh: line 78: 21786 Segmentation > fault $target_exec $target_path/"$@" > > > > > > It's the same on both Windows/MSYS2 and Linux. Let's see how > patchwork results will be... > > > > > > Signed-off-by: softworkz <softworkz@hotmail.com> > > --- > > libavcodec/version.h | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/libavcodec/version.h b/libavcodec/version.h > > index 83db2b242a..d162607f4b 100644 > > --- a/libavcodec/version.h > > +++ b/libavcodec/version.h > > @@ -27,9 +27,9 @@ > > > > #include "libavutil/version.h" > > > > -#define LIBAVCODEC_VERSION_MAJOR 59 > > -#define LIBAVCODEC_VERSION_MINOR 7 > > -#define LIBAVCODEC_VERSION_MICRO 102 > > +#define LIBAVCODEC_VERSION_MAJOR 60 > > +#define LIBAVCODEC_VERSION_MINOR 0 > > +#define LIBAVCODEC_VERSION_MICRO 100 > > > > #define LIBAVCODEC_VERSION_INT > AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ > > > LIBAVCODEC_VERSION_MINOR, \ > > > It segfaults because asf_read_pts() uses an uninitialized stack > packet > which av_read_frame() unrefs as soon as FF_API_INIT_PACKET is 0 > (which > your bump did). (And actually you could have easily found this out > yourself by just using a debugger.) I don't have a setup for debugging tests (yet), but I should have looked a bit down in version.h Due to the fact that it was just a single test failing, I hadn't expected a cause at a global level like it turned out to be. Thanks for analysing, softworkz
diff --git a/libavcodec/version.h b/libavcodec/version.h index 83db2b242a..d162607f4b 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -27,9 +27,9 @@ #include "libavutil/version.h" -#define LIBAVCODEC_VERSION_MAJOR 59 -#define LIBAVCODEC_VERSION_MINOR 7 -#define LIBAVCODEC_VERSION_MICRO 102 +#define LIBAVCODEC_VERSION_MAJOR 60 +#define LIBAVCODEC_VERSION_MINOR 0 +#define LIBAVCODEC_VERSION_MICRO 100 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ LIBAVCODEC_VERSION_MINOR, \
Test seek-lavf-asf failed. Look at tests/data/fate/seek-lavf-asf.err for details. make: *** [/build/ffmpeg-git/tests/Makefile:256: fate-seek-lavf-asf] Error 139 $ cat tests/data/fate/seek-lavf-asf.err /build/ffmpeg-git/tests/fate-run.sh: line 78: 21786 Segmentation fault $target_exec $target_path/"$@" It's the same on both Windows/MSYS2 and Linux. Let's see how patchwork results will be... Signed-off-by: softworkz <softworkz@hotmail.com> --- libavcodec/version.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)