[09/15] vc1dec: Try to fix vc1_mc_4mv_chroma4() parameters

Message ID 1369729571-75967-9-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö May 28, 2013, 8:26 a.m.
From: Michael Niedermayer <michaelni@gmx.at>

This fixes several chroma artifacts in several videos.
---
Should this be squashed into the previous commit, or keep
them separate (where the previous refactors and adds new possible
parameters while this one actually takes them into use)?
---
 libavcodec/vc1dec.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Kostya Shishkov May 28, 2013, 9:40 a.m. | #1
On Tue, May 28, 2013 at 11:26:05AM +0300, Martin Storsjö wrote:
> From: Michael Niedermayer <michaelni@gmx.at>
> 
> This fixes several chroma artifacts in several videos.
> ---
> Should this be squashed into the previous commit, or keep
> them separate (where the previous refactors and adds new possible
> parameters while this one actually takes them into use)?

Squash - if it introduces new parameters it should use them properly from the
start.

> ---
>  libavcodec/vc1dec.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
> index d956f7f..426bc7a 100644
> --- a/libavcodec/vc1dec.c
> +++ b/libavcodec/vc1dec.c
> @@ -4532,6 +4532,7 @@ static int vc1_decode_b_mb_intfr(VC1Context *v)
>                          vc1_mc_4mv_luma(v, i, 1, 1);
>                      }
>                      vc1_mc_4mv_chroma4(v, 0, 0, 0);
> +                    vc1_mc_4mv_chroma4(v, 1, 1, 1);
>                  } else {
>                      vc1_mc_1mv(v, 0);
>                      vc1_interp_mc(v);
> @@ -4551,6 +4552,7 @@ static int vc1_decode_b_mb_intfr(VC1Context *v)
>                  }
>  
>                  vc1_mc_4mv_chroma4(v, 0, 0, 0);
> +                vc1_mc_4mv_chroma4(v, 1, 1, 1);
>              } else if (bmvtype == BMV_TYPE_INTERPOLATED) {
>                  mvbp = v->twomvbp;
>                  dmv_x = dmv_y = 0;
> @@ -4598,7 +4600,7 @@ static int vc1_decode_b_mb_intfr(VC1Context *v)
>                  vc1_mc_4mv_luma(v, 1, dir, 0);
>                  vc1_mc_4mv_luma(v, 2, dir2, 0);
>                  vc1_mc_4mv_luma(v, 3, dir2, 0);
> -                vc1_mc_4mv_chroma4(v, 0, 0, 0);
> +                vc1_mc_4mv_chroma4(v, dir, dir2, 0);
>              } else {
>                  dir = bmvtype == BMV_TYPE_BACKWARD;
>  
> -- 

probably OK
Martin Storsjö May 28, 2013, 9:41 a.m. | #2
On Tue, 28 May 2013, Kostya Shishkov wrote:

> On Tue, May 28, 2013 at 11:26:05AM +0300, Martin Storsjö wrote:
>> From: Michael Niedermayer <michaelni@gmx.at>
>>
>> This fixes several chroma artifacts in several videos.
>> ---
>> Should this be squashed into the previous commit, or keep
>> them separate (where the previous refactors and adds new possible
>> parameters while this one actually takes them into use)?
>
> Squash - if it introduces new parameters it should use them properly from the
> start.

Right, even for the new function calls as well? This patch adds 2 new and 
changes one existing.

// Martin
Kostya Shishkov May 28, 2013, 9:50 a.m. | #3
On Tue, May 28, 2013 at 12:41:58PM +0300, Martin Storsjö wrote:
> On Tue, 28 May 2013, Kostya Shishkov wrote:
> 
> >On Tue, May 28, 2013 at 11:26:05AM +0300, Martin Storsjö wrote:
> >>From: Michael Niedermayer <michaelni@gmx.at>
> >>
> >>This fixes several chroma artifacts in several videos.
> >>---
> >>Should this be squashed into the previous commit, or keep
> >>them separate (where the previous refactors and adds new possible
> >>parameters while this one actually takes them into use)?
> >
> >Squash - if it introduces new parameters it should use them properly from the
> >start.
> 
> Right, even for the new function calls as well? This patch adds 2
> new and changes one existing.

Yes

Patch

diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
index d956f7f..426bc7a 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -4532,6 +4532,7 @@  static int vc1_decode_b_mb_intfr(VC1Context *v)
                         vc1_mc_4mv_luma(v, i, 1, 1);
                     }
                     vc1_mc_4mv_chroma4(v, 0, 0, 0);
+                    vc1_mc_4mv_chroma4(v, 1, 1, 1);
                 } else {
                     vc1_mc_1mv(v, 0);
                     vc1_interp_mc(v);
@@ -4551,6 +4552,7 @@  static int vc1_decode_b_mb_intfr(VC1Context *v)
                 }
 
                 vc1_mc_4mv_chroma4(v, 0, 0, 0);
+                vc1_mc_4mv_chroma4(v, 1, 1, 1);
             } else if (bmvtype == BMV_TYPE_INTERPOLATED) {
                 mvbp = v->twomvbp;
                 dmv_x = dmv_y = 0;
@@ -4598,7 +4600,7 @@  static int vc1_decode_b_mb_intfr(VC1Context *v)
                 vc1_mc_4mv_luma(v, 1, dir, 0);
                 vc1_mc_4mv_luma(v, 2, dir2, 0);
                 vc1_mc_4mv_luma(v, 3, dir2, 0);
-                vc1_mc_4mv_chroma4(v, 0, 0, 0);
+                vc1_mc_4mv_chroma4(v, dir, dir2, 0);
             } else {
                 dir = bmvtype == BMV_TYPE_BACKWARD;