| Message ID | df69c129c7a5dfe558ef0299adfdea17@e.email |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/truemotion1: Cleanup prediction code in truemotion1_decode_16bit and truemotion1_decode_24bit | expand |
| 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 | warning | Make fate failed |
ping August 19, 2021 12:48 PM, ffmpegandmahanstreamer@e.email wrote: > ping > > August 14, 2021 7:29 AM, ffmpegandmahanstreamer@e.email wrote: > >> 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 | 35 ++++++++++++----------------------- >> 1 file changed, 12 insertions(+), 23 deletions(-) >> >> diff --git a/libavcodec/truemotion1.c b/libavcodec/truemotion1.c >> index 32d8fb4005..3f13de0ff8 100644 >> --- a/libavcodec/truemotion1.c >> +++ b/libavcodec/truemotion1.c >> @@ -660,20 +660,14 @@ static void truemotion1_decode_16bit(TrueMotion1Context *s) >> 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_C_PREDICTOR(); >> } >> + APPLY_Y_PREDICTOR(); >> + OUTPUT_PIXEL_PAIR(); >> break; >> >> case 1: >> @@ -786,22 +780,17 @@ static void truemotion1_decode_24bit(TrueMotion1Context *s) >> 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: >> case 3: >> /* always apply 2 Y predictors on these iterations */ >> -- >> 2.24.3
On Sat, Aug 14, 2021 at 11:29:40 +0000, ffmpegandmahanstreamer@e.email wrote: Just formal stuff: > /* 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) { Devs prefer that you omit the curly brackets, if the block only contains one line. (I don't personally agree fully, but that doesn't matter.) > - OUTPUT_PIXEL_PAIR(); > + APPLY_C_PREDICTOR(); Indentation seems wrong. > + OUTPUT_PIXEL_PAIR(); > break; > > + > case 1: You added an additional empty line. As much as you praise the "graphical stuff" over the mailing list approach: Was your graphical editor not capable of showing you these issues? Hint: Review your commit (git diff before commit, git show afterwards) before submitting it. (And whether "graphical stuff" is greater or not, you would still have to correct errors by creating a modified commit. And with github pull requests, you'd need to re-branch or to force-push. Not much ease in the process there.) Moritz
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Moritz Barsnick > Sent: Thursday, 26 August 2021 09:25 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH] avcodec/truemotion1: Cleanup > prediction code in truemotion1_decode_16bit and > truemotion1_decode_24bit > > As much as you praise the "graphical stuff" over the mailing list > approach: Was your graphical editor not capable of showing you these > issues? Hint: Review your commit (git diff before commit, git show > afterwards) before submitting it. (And whether "graphical stuff" is > greater or not, you would still have to correct errors by creating a > modified commit. And with github pull requests, you'd need to re- > branch > or to force-push. Not much ease in the process there.) Force-pushing to the PR branch is nicely easing the process IMO. softworkz
diff --git a/libavcodec/truemotion1.c b/libavcodec/truemotion1.c index 32d8fb4005..3f13de0ff8 100644 --- a/libavcodec/truemotion1.c +++ b/libavcodec/truemotion1.c @@ -660,20 +660,14 @@ static void truemotion1_decode_16bit(TrueMotion1Context *s) 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_C_PREDICTOR(); } + APPLY_Y_PREDICTOR(); + OUTPUT_PIXEL_PAIR(); break; case 1: @@ -786,22 +780,17 @@ static void truemotion1_decode_24bit(TrueMotion1Context *s) 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: case 3: /* always apply 2 Y predictors on these iterations */