diff mbox series

[FFmpeg-devel] Subject: [PATCH] Cleanup code in truemotion1 decoder

Message ID 5fd309896c6b7278ec4a93029f8827f2@e.email
State New
Headers show
Series [FFmpeg-devel] Subject: [PATCH] Cleanup code in truemotion1 decoder | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Mapul Bhola Aug. 1, 2021, 4:22 p.m. UTC
Per Andreas Rheinhardt request i'm splitting the working patches in two.
---
This cleans up the code in the decode24bit and decode16bit functions by putting it in way that expresses the true intent while making it easier to read.
 libavcodec/truemotion1.c | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

Comments

Paul B Mahol Aug. 1, 2021, 5:36 p.m. UTC | #1
probably ok
Mapul Bhola Aug. 14, 2021, 3:16 a.m. UTC | #2
Mike, i was told you can push this?

August 11, 2021 9:04 AM, ffmpegandmahanstreamer@e.email wrote:

> ping
> 
> August 1, 2021 12:22 PM, ffmpegandmahanstreamer@e.email wrote:
> 
>> Per Andreas Rheinhardt request i'm splitting the working patches in two.
>> ---
>> This cleans up the code in the decode24bit and decode16bit functions by putting it in way that
>> expresses the true intent while making it easier to read.
>> libavcodec/truemotion1.c | 36 ++++++++++++------------------------
>> 1 file changed, 12 insertions(+), 24 deletions(-)
>> 
>> diff --git a/libavcodec/truemotion1.c b/libavcodec/truemotion1.c
>> index 32d8fb4005..50c90e732d 100644
>> --- a/libavcodec/truemotion1.c
>> +++ b/libavcodec/truemotion1.c
>> @@ -655,25 +655,19 @@ static void truemotion1_decode_16bit(TrueMotion1Context *s)
>> while (pixels_left > 0) {
>> 
>> if (keyframe || ((mb_change_byte & mb_change_byte_mask) == 0)) {
>> -
>> +
>> switch (y & 3) {
>> case 0:
>> /* if macroblock width is 2, apply C-Y-C-Y; else
>> * apply C-Y-Y */
>> + APPLY_C_PREDICTOR();
>> + APPLY_Y_PREDICTOR();
>> + OUTPUT_PIXEL_PAIR();
>> if (s->block_width == 2) {
>> APPLY_C_PREDICTOR();
>> - APPLY_Y_PREDICTOR();
>> - OUTPUT_PIXEL_PAIR();
>> - APPLY_C_PREDICTOR();
>> - APPLY_Y_PREDICTOR();
>> - OUTPUT_PIXEL_PAIR();
>> - } else {
>> - APPLY_C_PREDICTOR();
>> - APPLY_Y_PREDICTOR();
>> - OUTPUT_PIXEL_PAIR();
>> - APPLY_Y_PREDICTOR();
>> - OUTPUT_PIXEL_PAIR();
>> }
>> + APPLY_Y_PREDICTOR();
>> + OUTPUT_PIXEL_PAIR();
>> break;
>> 
>> case 1:
>> @@ -781,25 +775,19 @@ static void truemotion1_decode_24bit(TrueMotion1Context *s)
>> while (pixels_left > 0) {
>> 
>> if (keyframe || ((mb_change_byte & mb_change_byte_mask) == 0)) {
>> -
>> +
>> switch (y & 3) {
>> case 0:
>> /* if macroblock width is 2, apply C-Y-C-Y; else
>> * apply C-Y-Y */
>> + APPLY_C_PREDICTOR_24();
>> + APPLY_Y_PREDICTOR_24();
>> + OUTPUT_PIXEL_PAIR();
>> if (s->block_width == 2) {
>> APPLY_C_PREDICTOR_24();
>> - APPLY_Y_PREDICTOR_24();
>> - OUTPUT_PIXEL_PAIR();
>> - APPLY_C_PREDICTOR_24();
>> - APPLY_Y_PREDICTOR_24();
>> - OUTPUT_PIXEL_PAIR();
>> - } else {
>> - APPLY_C_PREDICTOR_24();
>> - APPLY_Y_PREDICTOR_24();
>> - OUTPUT_PIXEL_PAIR();
>> - APPLY_Y_PREDICTOR_24();
>> - OUTPUT_PIXEL_PAIR();
>> }
>> + APPLY_Y_PREDICTOR_24();
>> + OUTPUT_PIXEL_PAIR();
>> break;
>> 
>> case 1:
>> --
>> 2.24.3
Michael Niedermayer Aug. 14, 2021, 8:23 a.m. UTC | #3
On Sun, Aug 01, 2021 at 04:22:28PM +0000, ffmpegandmahanstreamer@e.email wrote:
> Per Andreas Rheinhardt request i'm splitting the working patches in two.

And this results in a commit message like this:
Author: ffmpegandmahanstreamer@e.email <ffmpegandmahanstreamer@e.email>
Date:   Sun Aug 1 16:22:28 2021 +0000

    Subject: [PATCH] Cleanup code in truemotion1 decoder
    
    Per Andreas Rheinhardt request i'm splitting the working patches in two.

Thats not good
"Subject: [PATCH]" doesnt belong in it
"Cleanup code in truemotion1 decoder" is too terse, what is cleaned upo why and how

Your Author name is also missing, is that intended ?
    
  
> ---
> This cleans up the code in the decode24bit and decode16bit functions by putting it in way that expresses the true intent while making it easier to read.
>  libavcodec/truemotion1.c | 36 ++++++++++++------------------------
>  1 file changed, 12 insertions(+), 24 deletions(-)
> 
> diff --git a/libavcodec/truemotion1.c b/libavcodec/truemotion1.c
> index 32d8fb4005..50c90e732d 100644
> --- a/libavcodec/truemotion1.c
> +++ b/libavcodec/truemotion1.c
> @@ -655,25 +655,19 @@ static void truemotion1_decode_16bit(TrueMotion1Context *s)
>          while (pixels_left > 0) {
>  
>              if (keyframe || ((mb_change_byte & mb_change_byte_mask) == 0)) {
> -
> +                
>                  switch (y & 3) {

unrelated
trailing whitespace

[...]
Mapul Bhola Aug. 14, 2021, 11:26 a.m. UTC | #4
Sent new patch.
Once again this proves the superioity of the graphical stuff. The whole host of issues in the first set of this email would have been avoided, and i wouldnt have to create the patch all over again to fix something.

August 14, 2021 4:23 AM, "Michael Niedermayer" <michael@niedermayer.cc> wrote:

> On Sun, Aug 01, 2021 at 04:22:28PM +0000, ffmpegandmahanstreamer@e.email wrote:
> 
>> Per Andreas Rheinhardt request i'm splitting the working patches in two.
> 
> And this results in a commit message like this:
> Author: ffmpegandmahanstreamer@e.email <ffmpegandmahanstreamer@e.email>
> Date: Sun Aug 1 16:22:28 2021 +0000
> 
> Subject: [PATCH] Cleanup code in truemotion1 decoder
> 
> Per Andreas Rheinhardt request i'm splitting the working patches in two.
> 
> Thats not good
> "Subject: [PATCH]" doesnt belong in it
> "Cleanup code in truemotion1 decoder" is too terse, what is cleaned upo why and how
> 
> Your Author name is also missing, is that intended ?
> 
>> ---
>> This cleans up the code in the decode24bit and decode16bit functions by putting it in way that
>> expresses the true intent while making it easier to read.
>> libavcodec/truemotion1.c | 36 ++++++++++++------------------------
>> 1 file changed, 12 insertions(+), 24 deletions(-)
>> 
>> diff --git a/libavcodec/truemotion1.c b/libavcodec/truemotion1.c
>> index 32d8fb4005..50c90e732d 100644
>> --- a/libavcodec/truemotion1.c
>> +++ b/libavcodec/truemotion1.c
>> @@ -655,25 +655,19 @@ static void truemotion1_decode_16bit(TrueMotion1Context *s)
>> while (pixels_left > 0) {
>> 
>> if (keyframe || ((mb_change_byte & mb_change_byte_mask) == 0)) {
>> -
>> +
>> switch (y & 3) {
> 
> unrelated
> trailing whitespace
> 
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Nations do behave wisely once they have exhausted all other alternatives.
> -- Abba Eban
> 
> _______________________________________________
> 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".
Nicolas George Aug. 14, 2021, 11:43 a.m. UTC | #5
ffmpegandmahanstreamer (12021-08-14):
> Once again this proves the superioity of the graphical stuff.

Start with not top-posting and we may take your opinion on the
superiority of stuff seriously.
Mapul Bhola Aug. 14, 2021, 11:51 a.m. UTC | #6
August 14, 2021 7:43 AM, "Nicolas George" <george@nsup.org> wrote:

> ffmpegandmahanstreamer (12021-08-14):
> 
>> Once again this proves the superioity of the graphical stuff.
> 
> Start with not top-posting and we may take your opinion on the
> superiority of stuff seriously.
I top post only when i want to reply to the message as a whole.
Here, im bottom posting because im responding to your specific part.
Bottom (literally, like way bottom) posting is worse than top posting, because then you need to scroll all the way down.
Hence why top-posting was invented
> 
> --
> Nicolas George
> 
> _______________________________________________
> 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".
Nicolas George Aug. 14, 2021, 11:56 a.m. UTC | #7
ffmpegandmahanstreamer (12021-08-14):
> I top post only when i want to reply to the message as a whole.

The rules of the list are clear: DO NOT TOP-POST

> Bottom (literally, like way bottom) posting is worse than top posting, because then you need to scroll all the way down.

Then be extra-respectful of your peers and trim your quote, just like I
did.

> Hence why top-posting was invented

Top-posting was not invented, it originally was just laziness and
ignorance by people discovering e-mail without being taught.
Mapul Bhola Aug. 14, 2021, 4:46 p.m. UTC | #8
August 14, 2021 7:56 AM, "Nicolas George" <george@nsup.org> wrote:

> ffmpegandmahanstreamer (12021-08-14):
> 
>> I top post only when i want to reply to the message as a whole.
> 
> The rules of the list are clear: DO NOT TOP-POST
The rules of the list also say you should be mean to others.
> 
>> Bottom (literally, like way bottom) posting is worse than top posting, because then you need to
>> scroll all the way down.
> 
> Then be extra-respectful of your peers and trim your quote, just like I
> did.
> 
Why does it matter to you so much anyway?
I don't really care about it, if the rules say dont top post, i wont, but again if i do it once in a while it doesnt make big difference.

>> Hence why top-posting was invented
> 
> Top-posting was not invented, it originally was just laziness and
> ignorance by people discovering e-mail without being taught.
> 
Not true. 
> --
> Nicolas George
> 
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavcodec/truemotion1.c b/libavcodec/truemotion1.c
index 32d8fb4005..50c90e732d 100644
--- a/libavcodec/truemotion1.c
+++ b/libavcodec/truemotion1.c
@@ -655,25 +655,19 @@  static void truemotion1_decode_16bit(TrueMotion1Context *s)
         while (pixels_left > 0) {
 
             if (keyframe || ((mb_change_byte & mb_change_byte_mask) == 0)) {
-
+                
                 switch (y & 3) {
                 case 0:
                     /* if macroblock width is 2, apply C-Y-C-Y; else
                      * apply C-Y-Y */
+                    APPLY_C_PREDICTOR();
+                    APPLY_Y_PREDICTOR();
+                    OUTPUT_PIXEL_PAIR();
                     if (s->block_width == 2) {
                         APPLY_C_PREDICTOR();
-                        APPLY_Y_PREDICTOR();
-                        OUTPUT_PIXEL_PAIR();
-                        APPLY_C_PREDICTOR();
-                        APPLY_Y_PREDICTOR();
-                        OUTPUT_PIXEL_PAIR();
-                    } else {
-                        APPLY_C_PREDICTOR();
-                        APPLY_Y_PREDICTOR();
-                        OUTPUT_PIXEL_PAIR();
-                        APPLY_Y_PREDICTOR();
-                        OUTPUT_PIXEL_PAIR();
                     }
+                    APPLY_Y_PREDICTOR();
+                    OUTPUT_PIXEL_PAIR();
                     break;
 
                 case 1:
@@ -781,25 +775,19 @@  static void truemotion1_decode_24bit(TrueMotion1Context *s)
         while (pixels_left > 0) {
 
             if (keyframe || ((mb_change_byte & mb_change_byte_mask) == 0)) {
-
+                
                 switch (y & 3) {
                 case 0:
                     /* if macroblock width is 2, apply C-Y-C-Y; else
                      * apply C-Y-Y */
+                    APPLY_C_PREDICTOR_24();
+                    APPLY_Y_PREDICTOR_24();
+                    OUTPUT_PIXEL_PAIR();
                     if (s->block_width == 2) {
                         APPLY_C_PREDICTOR_24();
-                        APPLY_Y_PREDICTOR_24();
-                        OUTPUT_PIXEL_PAIR();
-                        APPLY_C_PREDICTOR_24();
-                        APPLY_Y_PREDICTOR_24();
-                        OUTPUT_PIXEL_PAIR();
-                    } else {
-                        APPLY_C_PREDICTOR_24();
-                        APPLY_Y_PREDICTOR_24();
-                        OUTPUT_PIXEL_PAIR();
-                        APPLY_Y_PREDICTOR_24();
-                        OUTPUT_PIXEL_PAIR();
                     }
+                    APPLY_Y_PREDICTOR_24();
+                    OUTPUT_PIXEL_PAIR();
                     break;
 
                 case 1: