diff mbox

[FFmpeg-devel,4/4] zmbvenc: use unsigned values for score calculations

Message ID 20181219220003.27225-4-matthew.w.fearnley@gmail.com
State New
Headers show

Commit Message

matthew.w.fearnley@gmail.com Dec. 19, 2018, 10 p.m. UTC
From: Matthew Fearnley <matthew.w.fearnley@gmail.com>

All the values in score_tab are positive or 0, and so should be the sum
returned by block_cmp().

The logic in zmbv_me() assumes that all 'bv' values will be non-negative,
in order to guarantee an early return if ever bv==0.
---
 libavcodec/zmbvenc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Tomas Härdin Dec. 20, 2018, 4:30 p.m. UTC | #1
ons 2018-12-19 klockan 22:00 +0000 skrev matthew.w.fearnley@gmail.com:
> > From: Matthew Fearnley <matthew.w.fearnley@gmail.com>
> 
> All the values in score_tab are positive or 0, and so should be the sum
> returned by block_cmp().
> 
> The logic in zmbv_me() assumes that all 'bv' values will be non-negative,
> in order to guarantee an early return if ever bv==0.
> ---
>  libavcodec/zmbvenc.c | 7 ++++---

Trivial enough. You could probably roll many or even all of these
patches together

/Tomas
matthew.w.fearnley@gmail.com Dec. 20, 2018, 5:48 p.m. UTC | #2
On Thu, 20 Dec 2018 at 16:30, Tomas Härdin <tjoppen@acc.umu.se> wrote:

> Trivial enough. You could probably roll many or even all of these
> patches together
>

Thanks for your feedback.  I was reluctant to submit so many patches, but I
worried at the time that combining them would require an overly long commit
message.  Maybe I'm being too verbose though...
What's the best way to submit a combined patch - from a Patchwork/mailing
list perspective?

Thanks again for your reviews.  I'm happy to make any changes necessary for
them to be committed.
Tomas Härdin Dec. 22, 2018, 12:15 p.m. UTC | #3
tor 2018-12-20 klockan 17:48 +0000 skrev Matthew Fearnley:
> > On Thu, 20 Dec 2018 at 16:30, Tomas Härdin <tjoppen@acc.umu.se> wrote:
> 
> > Trivial enough. You could probably roll many or even all of these
> > patches together
> > 
> 
> Thanks for your feedback.  I was reluctant to submit so many patches, but I
> worried at the time that combining them would require an overly long commit
> message.  Maybe I'm being too verbose though...
> What's the best way to submit a combined patch - from a Patchwork/mailing
> list perspective?

How large a patch should be is somewhat arbitrary. I tend to try to
keep each patch related to a single "concept". And of course separating
 cosmetic and functional changes

It's mostly the having to have PATCH 3/4 before PATCH 1/4 prompting
this suggestion. Or maybe rolling them together

The primary reason for making nice patches it for making runs of git-
bisect easier

/Tomas
matthew.w.fearnley@gmail.com Dec. 22, 2018, 3:54 p.m. UTC | #4
> On 22 Dec 2018, at 12:15, Tomas Härdin <tjoppen@acc.umu.se> wrote:
> 
> tor 2018-12-20 klockan 17:48 +0000 skrev Matthew Fearnley:
>>>>> On Thu, 20 Dec 2018 at 16:30, Tomas Härdin <tjoppen@acc.umu.se> wrote:
>>> 
>>> Trivial enough. You could probably roll many or even all of these
>>> patches together
>> 
>> Thanks for your feedback.  I was reluctant to submit so many patches, but I
>> worried at the time that combining them would require an overly long commit
>> message.  Maybe I'm being too verbose though...
>> What's the best way to submit a combined patch - from a Patchwork/mailing
>> list perspective?
> 
> How large a patch should be is somewhat arbitrary. I tend to try to
> keep each patch related to a single "concept". And of course separating
> cosmetic and functional changes
> 
> It's mostly the having to have PATCH 3/4 before PATCH 1/4 prompting
> this suggestion. Or maybe rolling them together

If I roll the patches together (and include the additional suggestions in PATCH 1), would it be best to just email a new patch, and have these closed?

Still struggling with general procedure here a bit...
Thanks for bearing with me.

Matthew
Tomas Härdin Dec. 25, 2018, 9:36 a.m. UTC | #5
lör 2018-12-22 klockan 15:54 +0000 skrev Matthew Fearnley:
> > On 22 Dec 2018, at 12:15, Tomas Härdin <tjoppen@acc.umu.se> wrote:
> > 
> > tor 2018-12-20 klockan 17:48 +0000 skrev Matthew Fearnley:
> > > > > > On Thu, 20 Dec 2018 at 16:30, Tomas Härdin <tjoppen@acc.umu
> > > > > > .se> wrote:
> > > > 
> > > > Trivial enough. You could probably roll many or even all of
> > > > these
> > > > patches together
> > > 
> > > Thanks for your feedback.  I was reluctant to submit so many
> > > patches, but I
> > > worried at the time that combining them would require an overly
> > > long commit
> > > message.  Maybe I'm being too verbose though...
> > > What's the best way to submit a combined patch - from a
> > > Patchwork/mailing
> > > list perspective?
> > 
> > How large a patch should be is somewhat arbitrary. I tend to try to
> > keep each patch related to a single "concept". And of course
> > separating
> > cosmetic and functional changes
> > 
> > It's mostly the having to have PATCH 3/4 before PATCH 1/4 prompting
> > this suggestion. Or maybe rolling them together
> 
> If I roll the patches together (and include the additional
> suggestions in PATCH 1), would it be best to just email a new patch,
> and have these closed?

If you have a new set of patches then a new thread is best, yes

/Tomas
diff mbox

Patch

diff --git a/libavcodec/zmbvenc.c b/libavcodec/zmbvenc.c
index 0ebae1b254..9e1b44f15d 100644
--- a/libavcodec/zmbvenc.c
+++ b/libavcodec/zmbvenc.c
@@ -63,11 +63,11 @@  typedef struct ZmbvEncContext {
  * XXX should be optimized and moved to DSPContext
  * TODO handle out of edge ME
  */
-static inline int block_cmp(ZmbvEncContext *c, uint8_t *src, int stride,
+static inline unsigned int block_cmp(ZmbvEncContext *c, uint8_t *src, int stride,
                             uint8_t *src2, int stride2, int bw, int bh,
                             int *xored)
 {
-    int sum = 0;
+    unsigned int sum = 0;
     int i, j;
     uint16_t histogram[256] = {0};
 
@@ -99,7 +99,8 @@  static inline int block_cmp(ZmbvEncContext *c, uint8_t *src, int stride,
 static int zmbv_me(ZmbvEncContext *c, uint8_t *src, int sstride, uint8_t *prev,
                    int pstride, int x, int y, int *mx, int *my, int *xored)
 {
-    int dx, dy, tx, ty, tv, bv, bw, bh;
+    int dx, dy, tx, ty, bw, bh;
+    unsigned int tv, bv;
     int txored;
 
     *mx = *my = 0;