Re: [FFmpeg-devel] [PATCH] mpeg2dec: fix decoding field pictures
> On Fri, Feb 09, 2018 at 02:03:24AM +0900, Nekopanda wrote: >> - Fix field selection for skipped macroblocks >> >> For B field pictures, the spec says, >> >> > The prediction shall be made from the field of the same parity as the > field being predicted. >> >> I did it. >> >> - Fix motion vector rounding for chroma components >> >> In 16x8 motion compensation, for lower 16x8 region, the input to > mpeg_motion() for motion_y was "motion_y + 16", which causes wrong > rounding. For 4:2:0, chroma scaling for y is dividing by two and rounding > toward > zero. When motion_y < 0 and motion_y + 16 > 0, the rounding direction of > "motion_y" and "motion_y + 16" is different and rounding > "motion_y + 16" would be incorrect. >> >> We should input "motion_y" as is to round correctly. I add > "is_16x8" flag to do that. > > please split this patch in 2. > I think these are 2 independant bugfixes. > > also please update the fate checksums so make fate does not break after > either patch I split into 2 patches and sent them. They also include new checksums. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg2dec: fix decoding field pictures
On Fri, Feb 09, 2018 at 02:03:24AM +0900, Nekopanda wrote: > - Fix field selection for skipped macroblocks > > For B field pictures, the spec says, > > > The prediction shall be made from the field of the same parity as the field > > being predicted. > > I did it. > > - Fix motion vector rounding for chroma components > > In 16x8 motion compensation, for lower 16x8 region, the input to > mpeg_motion() for motion_y was "motion_y + 16", which causes wrong rounding. > For 4:2:0, chroma scaling for y is dividing by two and rounding toward zero. > When motion_y < 0 and motion_y + 16 > 0, the rounding direction of "motion_y" > and "motion_y + 16" is different and rounding "motion_y + 16" would be > incorrect. > > We should input "motion_y" as is to round correctly. I add "is_16x8" flag to > do that. please split this patch in 2. I think these are 2 independant bugfixes. also please update the fate checksums so make fate does not break after either patch thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When you are offended at any man's fault, turn to yourself and study your own failings. Then you will forget your anger. -- Epictetus signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg2dec: fix decoding field pictures
> From: Carl Eugen Hoyos > > 2018-02-09 3:51 GMT+01:00 Nekopanda > : >>> From: Carl Eugen Hoyos > Yes, I have files that show artifacts without this change. > Recently, Japanese satellite television started to use field pictures > widely, and many of that show artifacts. We need this change to remove the artifacts. >>> >>> Could you provide a sample with visible artefacts? >> >> Sorry, all of them are copyrighted materials and difficult to share. > > Please find a few seconds of commercials that show the issue > and share them. > > Many thousand samples were provided here, very few were > not copyrighted. OK. Here is. http://nekopandanet.sakura.ne.jp/tmp/v/field-pic-bug.mpg Decoded frame with artifacts http://nekopandanet.sakura.ne.jp/tmp/v/pic-09.png ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg2dec: fix decoding field pictures
2018-02-09 3:51 GMT+01:00 Nekopanda : >> From: Carl Eugen Hoyos >>> Yes, I have files that show artifacts without this change. Recently, >>> Japanese satellite television started to use field pictures widely, >>> and many of that show artifacts. We need this change to remove >>> the artifacts. >> >> Could you provide a sample with visible artefacts? > > Sorry, all of them are copyrighted materials and difficult to share. Please find a few seconds of commercials that show the issue and share them. Many thousand samples were provided here, very few were not copyrighted. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg2dec: fix decoding field pictures
> From: Carl Eugen Hoyos > > 2018-02-09 3:07 GMT+01:00 Nekopanda > : >>> How did you find this bug ? >>> >>> do you have a testcase / file which shows artifacts without this change > ? >> >> Yes, I have files that show artifacts without this change. Recently, >> Japanese satellite television started to use field pictures widely, >> and many of that show artifacts. We need this change to remove >> the artifacts. > > Could you provide a sample with visible artefacts? Sorry, all of them are copyrighted materials and difficult to share. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg2dec: fix decoding field pictures
2018-02-09 3:07 GMT+01:00 Nekopanda : >> How did you find this bug ? >> >> do you have a testcase / file which shows artifacts without this change ? > > Yes, I have files that show artifacts without this change. Recently, > Japanese satellite television started to use field pictures widely, > and many of that show artifacts. We need this change to remove > the artifacts. Could you provide a sample with visible artefacts? Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg2dec: fix decoding field pictures
> How did you find this bug ? > > do you have a testcase / file which shows artifacts without this change ? Yes, I have files that show artifacts without this change. Recently, Japanese satellite television started to use field pictures widely, and many of that show artifacts. We need this change to remove the artifacts. > assuming the change is correct then the patch needs to update several fate > checksums. As is it would break make fate currently the following tests fail with this change. filter-w3fdif-complex filter-w3fdif-simple filter-yadif10 filter-yadif16 filter-yadif-mode0 filter-yadif-mode1 mpeg2-field-enc mpeg2-ticket186 mpeg2-ticket6677 - Original Message - > From: Michael Niedermayer > To: FFmpeg development discussions and patches > Cc: > Date: 2018/2/9, Fri 09:55 > Subject: Re: [FFmpeg-devel] [PATCH] mpeg2dec: fix decoding field pictures > > On Fri, Feb 09, 2018 at 02:03:24AM +0900, Nekopanda wrote: >> - Fix field selection for skipped macroblocks >> >> For B field pictures, the spec says, >> >> > The prediction shall be made from the field of the same parity as the > field being predicted. >> >> I did it. >> >> - Fix motion vector rounding for chroma components >> >> In 16x8 motion compensation, for lower 16x8 region, the input to > mpeg_motion() for motion_y was "motion_y + 16", which causes wrong > rounding. For 4:2:0, chroma scaling for y is dividing by two and rounding > toward > zero. When motion_y < 0 and motion_y + 16 > 0, the rounding direction of > "motion_y" and "motion_y + 16" is different and rounding > "motion_y + 16" would be incorrect. >> >> We should input "motion_y" as is to round correctly. I add > "is_16x8" flag to do that. >> --- >> libavcodec/mpeg12dec.c | 2 ++ >> libavcodec/mpegvideo_motion.c | 30 -- >> 2 files changed, 18 insertions(+), 14 deletions(-) > > How did you find this bug ? > > do you have a testcase / file which shows artifacts without this change ? > > assuming the change is correct then the patch needs to update several fate > checksums. As is it would break make fate > > thanks > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > "Nothing to hide" only works if the folks in power share the values of > you and everyone you know entirely and always will -- Tom Scott > > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg2dec: fix decoding field pictures
On Fri, Feb 09, 2018 at 02:03:24AM +0900, Nekopanda wrote: > - Fix field selection for skipped macroblocks > > For B field pictures, the spec says, > > > The prediction shall be made from the field of the same parity as the field > > being predicted. > > I did it. > > - Fix motion vector rounding for chroma components > > In 16x8 motion compensation, for lower 16x8 region, the input to > mpeg_motion() for motion_y was "motion_y + 16", which causes wrong rounding. > For 4:2:0, chroma scaling for y is dividing by two and rounding toward zero. > When motion_y < 0 and motion_y + 16 > 0, the rounding direction of "motion_y" > and "motion_y + 16" is different and rounding "motion_y + 16" would be > incorrect. > > We should input "motion_y" as is to round correctly. I add "is_16x8" flag to > do that. > --- > libavcodec/mpeg12dec.c| 2 ++ > libavcodec/mpegvideo_motion.c | 30 -- > 2 files changed, 18 insertions(+), 14 deletions(-) How did you find this bug ? do you have a testcase / file which shows artifacts without this change ? assuming the change is correct then the patch needs to update several fate checksums. As is it would break make fate thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB "Nothing to hide" only works if the folks in power share the values of you and everyone you know entirely and always will -- Tom Scott signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
