Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Soft Works <softworkz@hotmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v2] tests/fate/vcodec: Limit mem alignment for vsynth..mpeg2-422 tests
Date: Mon, 30 May 2022 10:56:16 +0000
Message-ID: <DM8P223MB0365837E838E8AC53E973810BADD9@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <165390625527.5088.1732998558650448774@lain.red.khirnov.net>



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Anton Khirnov
> Sent: Monday, May 30, 2022 12:24 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v2] tests/fate/vcodec: Limit mem
> alignment for vsynth..mpeg2-422 tests
> 
> Quoting Soft Works (2022-05-30 10:08:11)
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Anton Khirnov
> > > Sent: Monday, May 30, 2022 9:35 AM
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH v2] tests/fate/vcodec: Limit
> mem
> > > alignment for vsynth..mpeg2-422 tests
> > >
> > > Quoting Soft Works (2022-05-28 15:17:54)
> > > > Do you have a better idea?
> > > >
> > > > The one advantage of this method is that you don’t need to
> change
> > > compilation parameters
> > > > nor  any source code. It’s only a runtime flag being set only
> for
> > > this specific family of tests.
> > >
> > > At the very least, I would expect the commit message to explain
> what
> > > exactly the problem is, and why is it fixed in this seemingly ad-
> hoc
> > > manner.
> > >
> > > "limit mem alignment to fix failing tests" explains nothing.
> >
> >
> > There was a longer conversation ("FATE Errors") with a number of
> > people, I'm not sure whether you followed.
> 
> I saw the thread, I did not read it in enough detail to understand
> the
> cause of the errors.
> 
> > I submitted this as a possible way to work around the issue. If you
> > find that patch acceptable, then I'll gladly adjust the commit
> message
> > with a detailed explanation.
> > It's just that I learned that it's not very effective to spend a
> lot
> > of time on things that are likely to get rejected or ignored.


> > Well, the test .mak files are full of hacks then, with the many
> super-
> > special flags in place that do not reflect any real world use at
> all.
> 
> There are AFAIK zero tests that override cpuflags.
I meant other flags, but well, that's not getting us anywhere..


> From a first glance, this looks very ad-hoc (i.e. a hack). But ugly
> hacks may sometimes be temporarily acceptable if a proper solution
> requires too much work and the problem needs to be fixed ASAP.

Yes, that's how I see the situation.


> So whether the patch is acceptable or not really depends on what the
> problem is and why are you fixing it in this specific way.


From what I gathered from James' and Paul's responses, there is no
easy fix for this.

Personally, I am not familiar with the code nor do I have the capacity
to resolve the issue at its core.

> Most of the code is supposed to be arch-independent. A test producing
> different results on different architectures should be considered a
> bug.

Yes, I fully agree to that. But those FATE tests have already fulfilled
their duty: 

	the problem is now a known problem

It can be put on a list, a trac entry can be made and somebody can 
work on a fix for it at some time.

It just doesn't make sense to keep those tests failing up until that
point in time. Being unable to get clean FATE runs locally is 
affecting productivity.

When you or one of the others in this conversation would be affected,
I wonder whether such discussion about a trivial issue would have
been necessary.

(trivial: the tests should not fail; probably non-trivial: the 
actual fix)

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".

  reply	other threads:[~2022-05-30 10:56 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-28  9:08 [FFmpeg-devel] [PATCH] " softworkz
2022-05-28 10:19 ` [FFmpeg-devel] [PATCH v2] " softworkz
2022-05-28 13:10   ` Kieran Kunhya
2022-05-28 13:17     ` Soft Works
2022-05-30  7:35     ` Anton Khirnov
2022-05-30  8:08       ` Soft Works
2022-05-30  8:17         ` Soft Works
2022-05-30  8:35       ` Soft Works
2022-05-30  9:06         ` Paul B Mahol
2022-05-30  9:26           ` Soft Works
2022-05-30 10:31           ` Anton Khirnov
2022-05-30 10:24       ` Anton Khirnov
2022-05-30 10:56         ` Soft Works [this message]
2022-05-30 10:27       ` Anton Khirnov
2022-05-30 10:50         ` Andreas Rheinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM8P223MB0365837E838E8AC53E973810BADD9@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM \
    --to=softworkz@hotmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git