| Message ID | 20190127093619.28904-3-mfcc64@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
2019-01-27 10:36 GMT+01:00, Muhammad Faiz <mfcc64@gmail.com>:
> Fix mismatched checksum on fate-filter-pixfmts-super2xsai.
I believe this patch and 2/5 are unrelated and should be
committed independently of the patchset.
Carl Eugen
On Sun, Jan 27, 2019 at 7:19 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2019-01-27 10:36 GMT+01:00, Muhammad Faiz <mfcc64@gmail.com>: > > Fix mismatched checksum on fate-filter-pixfmts-super2xsai. > > I believe this patch and 2/5 are unrelated and should be > committed independently of the patchset. No. [2/5] and [3/5] depend on [1/5]. [4/5] and [5/5] depend on [1/5] and [2/5].
2019-01-27 15:07 GMT+01:00, Muhammad Faiz <mfcc64@gmail.com>: > On Sun, Jan 27, 2019 at 7:19 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> >> 2019-01-27 10:36 GMT+01:00, Muhammad Faiz <mfcc64@gmail.com>: >> > Fix mismatched checksum on fate-filter-pixfmts-super2xsai. >> >> I believe this patch and 2/5 are unrelated and should be >> committed independently of the patchset. > > No. [2/5] and [3/5] depend on [1/5]. No (at least afaiu). The fate tests for these changes depend on the first patch. But if the tests are currently broken, they cannot be used as reasoning for anything. Instead, the code should be fixed and the tests disabled. (imo) Carl Eugen
On Sun, Jan 27, 2019 at 9:17 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2019-01-27 15:07 GMT+01:00, Muhammad Faiz <mfcc64@gmail.com>: > > On Sun, Jan 27, 2019 at 7:19 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> > >> 2019-01-27 10:36 GMT+01:00, Muhammad Faiz <mfcc64@gmail.com>: > >> > Fix mismatched checksum on fate-filter-pixfmts-super2xsai. > >> > >> I believe this patch and 2/5 are unrelated and should be > >> committed independently of the patchset. > > > > No. [2/5] and [3/5] depend on [1/5]. > > No (at least afaiu). > The fate tests for these changes depend on the first patch. > But if the tests are currently broken, they cannot be used > as reasoning for anything. > Instead, the code should be fixed and the tests disabled. > (imo) Don't forget that 2/5 and 3/5 bugs (and p010/p016 bugs) are revealed by 1/5. It means that 1/5 is important.
2019-01-27 16:12 GMT+01:00, Muhammad Faiz <mfcc64@gmail.com>: > On Sun, Jan 27, 2019 at 9:17 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> >> 2019-01-27 15:07 GMT+01:00, Muhammad Faiz <mfcc64@gmail.com>: >> > On Sun, Jan 27, 2019 at 7:19 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> >> > wrote: >> >> >> >> 2019-01-27 10:36 GMT+01:00, Muhammad Faiz <mfcc64@gmail.com>: >> >> > Fix mismatched checksum on fate-filter-pixfmts-super2xsai. >> >> >> >> I believe this patch and 2/5 are unrelated and should be >> >> committed independently of the patchset. >> > >> > No. [2/5] and [3/5] depend on [1/5]. >> >> No (at least afaiu). >> The fate tests for these changes depend on the first patch. >> But if the tests are currently broken, they cannot be used >> as reasoning for anything. >> Instead, the code should be fixed and the tests disabled. >> (imo) > > Don't forget that 2/5 and 3/5 bugs (and p010/p016 bugs) are > revealed by 1/5. I know, I understood this. > It means that 1/5 is important. I neither said that it is important nor that it is unimportant. I seems to me that this bug fix (for a bug that was thankfully found by a patch written by you) is not related to the fate system. If fixing this bug means that fate breaks, it indicates that fate is wrong and that maybe the fate test should be disabled / replaced. Carl Eugen
diff --git a/libavfilter/vf_super2xsai.c b/libavfilter/vf_super2xsai.c index 87eec04da8..4901e03e23 100644 --- a/libavfilter/vf_super2xsai.c +++ b/libavfilter/vf_super2xsai.c @@ -180,8 +180,8 @@ static void super2xsai(AVFilterContext *ctx, break; default: // bpp = 2 if (s->is_be) { - AV_WB32(dst_line[0] + x * 4, product1a | (product1b << 16)); - AV_WB32(dst_line[1] + x * 4, product2a | (product2b << 16)); + AV_WB32(dst_line[0] + x * 4, (product1a << 16) | product1b); + AV_WB32(dst_line[1] + x * 4, (product2a << 16) | product2b); } else { AV_WL32(dst_line[0] + x * 4, product1a | (product1b << 16)); AV_WL32(dst_line[1] + x * 4, product2a | (product2b << 16)); diff --git a/tests/ref/fate/filter-pixfmts-super2xsai b/tests/ref/fate/filter-pixfmts-super2xsai index d42601dab1..c76d469880 100644 --- a/tests/ref/fate/filter-pixfmts-super2xsai +++ b/tests/ref/fate/filter-pixfmts-super2xsai @@ -1,14 +1,14 @@ abgr e21be14b5fe9d7a29740a418c325b17e argb 563489534663cb2b32beed2b41370c37 bgr24 a933eac9bb53c3ce3c33950b229996b5 -bgr555le d69e39a24027afcb28feaabb46f0948d bgr555le 70b819425f79f823356229b90b41cc84 -bgr565le 78f3b43ddcc1f8558444c97d249a6123 +bgr555le 70b819425f79f823356229b90b41cc84 +bgr565le 6fb9dc50a81b853800ba65d5ec6b8417 bgr565le 6fb9dc50a81b853800ba65d5ec6b8417 bgra e9cc6644e2f35103c241094ab4bb8fec rgb24 3fd7653f414f350ddb0c0a236ce0c809 -rgb555le f2f9f30e8be582729f12a03331e3c635 rgb555le 53325a20c913826566880eb25d1d2946 -rgb565le 340ffed3645809f68346280764ca3de6 +rgb555le 53325a20c913826566880eb25d1d2946 +rgb565le 14fe550f449a7539d9f1e99e85cf40f1 rgb565le 14fe550f449a7539d9f1e99e85cf40f1 rgba 7041184d35c316e73e849504b64bc4f6
Fix mismatched checksum on fate-filter-pixfmts-super2xsai. Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> --- libavfilter/vf_super2xsai.c | 4 ++-- tests/ref/fate/filter-pixfmts-super2xsai | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-)