| Message ID | 20200824174034.17837-2-gautamramk@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,RFC,v3,1/5] libavcodec/jpeg2000: Make tag tree functions non static | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On Mon, Aug 24, 2020 at 11:10:31PM +0530, gautamramk@gmail.com wrote: > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > The implementation of tag tree encoding was incorrect. > However, this error was not visible as the current j2k > encoder encodes only 1 layer. > This patch fixes tag tree coding for JPEG2000 such tag > tree coding would work for multi layer encoding. > --- > libavcodec/j2kenc.c | 41 ++++++++++++++++++++++++----------------- > libavcodec/jpeg2000.c | 1 + > libavcodec/jpeg2000.h | 1 + > 3 files changed, 26 insertions(+), 17 deletions(-) > > diff --git a/libavcodec/j2kenc.c b/libavcodec/j2kenc.c > index 16863f8e8c..d9777fe07f 100644 > --- a/libavcodec/j2kenc.c > +++ b/libavcodec/j2kenc.c > @@ -242,27 +242,34 @@ static void j2k_flush(Jpeg2000EncoderContext *s) > static void tag_tree_code(Jpeg2000EncoderContext *s, Jpeg2000TgtNode *node, int threshold) > { > Jpeg2000TgtNode *stack[30]; > - int sp = 1, curval = 0; > - stack[0] = node; > + int sp = -1, curval = 0; > > - node = node->parent; > - while(node){ > - if (node->vis){ > - curval = node->val; > - break; > - } > - node->vis++; > - stack[sp++] = node; > + while(node->parent){ > + stack[++sp] = node; > node = node->parent; > } > - while(--sp >= 0){ > - if (stack[sp]->val >= threshold){ > - put_bits(s, 0, threshold - curval); > - break; > + > + while (1) { > + if (curval > node->temp_val) > + node->temp_val = curval; > + else { > + curval = node->temp_val; > } > - put_bits(s, 0, stack[sp]->val - curval); > - put_bits(s, 1, 1); > - curval = stack[sp]->val; > + while (curval < threshold) { > + if (curval >= node->val) { > + if (!node->vis) { > + node->vis = 1; > + put_bits(s, 1, 1); > + } > + break; > + } > + put_bits(s, 0, 1); > + curval++; > + } why is the put_bits(s, 0, stack[sp]->val - curval); changed into 1 bit at a time calls ? a single call should still work with the modified loop a single call may be faster ... thx [...]
On Tue, Aug 25, 2020 at 4:08 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Mon, Aug 24, 2020 at 11:10:31PM +0530, gautamramk@gmail.com wrote: > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > The implementation of tag tree encoding was incorrect. > > However, this error was not visible as the current j2k > > encoder encodes only 1 layer. > > This patch fixes tag tree coding for JPEG2000 such tag > > tree coding would work for multi layer encoding. > > --- > > libavcodec/j2kenc.c | 41 ++++++++++++++++++++++++----------------- > > libavcodec/jpeg2000.c | 1 + > > libavcodec/jpeg2000.h | 1 + > > 3 files changed, 26 insertions(+), 17 deletions(-) > > > > diff --git a/libavcodec/j2kenc.c b/libavcodec/j2kenc.c > > index 16863f8e8c..d9777fe07f 100644 > > --- a/libavcodec/j2kenc.c > > +++ b/libavcodec/j2kenc.c > > @@ -242,27 +242,34 @@ static void j2k_flush(Jpeg2000EncoderContext *s) > > static void tag_tree_code(Jpeg2000EncoderContext *s, Jpeg2000TgtNode *node, int threshold) > > { > > Jpeg2000TgtNode *stack[30]; > > - int sp = 1, curval = 0; > > - stack[0] = node; > > + int sp = -1, curval = 0; > > > > - node = node->parent; > > - while(node){ > > - if (node->vis){ > > - curval = node->val; > > - break; > > - } > > - node->vis++; > > - stack[sp++] = node; > > + while(node->parent){ > > + stack[++sp] = node; > > node = node->parent; > > } > > - while(--sp >= 0){ > > - if (stack[sp]->val >= threshold){ > > - put_bits(s, 0, threshold - curval); > > - break; > > + > > + while (1) { > > + if (curval > node->temp_val) > > + node->temp_val = curval; > > + else { > > + curval = node->temp_val; > > } > > - put_bits(s, 0, stack[sp]->val - curval); > > - put_bits(s, 1, 1); > > - curval = stack[sp]->val; > > + while (curval < threshold) { > > + if (curval >= node->val) { > > + if (!node->vis) { > > + node->vis = 1; > > + put_bits(s, 1, 1); > > + } > > + break; > > + } > > + put_bits(s, 0, 1); > > + curval++; > > + } > > why is the put_bits(s, 0, stack[sp]->val - curval); > changed into 1 bit at a time calls ? > a single call should still work with the modified loop > a single call may be faster ... My bad, I oversimplified the implementation as it was hard to debug. Now since I am sure its correct, i'll remove the loop. > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Dictatorship naturally arises out of democracy, and the most aggravated > form of tyranny and slavery out of the most extreme liberty. -- Plato > _______________________________________________ > 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 --git a/libavcodec/j2kenc.c b/libavcodec/j2kenc.c index 16863f8e8c..d9777fe07f 100644 --- a/libavcodec/j2kenc.c +++ b/libavcodec/j2kenc.c @@ -242,27 +242,34 @@ static void j2k_flush(Jpeg2000EncoderContext *s) static void tag_tree_code(Jpeg2000EncoderContext *s, Jpeg2000TgtNode *node, int threshold) { Jpeg2000TgtNode *stack[30]; - int sp = 1, curval = 0; - stack[0] = node; + int sp = -1, curval = 0; - node = node->parent; - while(node){ - if (node->vis){ - curval = node->val; - break; - } - node->vis++; - stack[sp++] = node; + while(node->parent){ + stack[++sp] = node; node = node->parent; } - while(--sp >= 0){ - if (stack[sp]->val >= threshold){ - put_bits(s, 0, threshold - curval); - break; + + while (1) { + if (curval > node->temp_val) + node->temp_val = curval; + else { + curval = node->temp_val; } - put_bits(s, 0, stack[sp]->val - curval); - put_bits(s, 1, 1); - curval = stack[sp]->val; + while (curval < threshold) { + if (curval >= node->val) { + if (!node->vis) { + node->vis = 1; + put_bits(s, 1, 1); + } + break; + } + put_bits(s, 0, 1); + curval++; + } + node->temp_val = curval; + if (sp < 0) + break; + node = stack[sp--]; } } diff --git a/libavcodec/jpeg2000.c b/libavcodec/jpeg2000.c index 26e09fbe38..3d3e7ec313 100644 --- a/libavcodec/jpeg2000.c +++ b/libavcodec/jpeg2000.c @@ -88,6 +88,7 @@ void ff_tag_tree_zero(Jpeg2000TgtNode *t, int w, int h) for (i = 0; i < siz; i++) { t[i].val = 0; + t[i].temp_val = 0; t[i].vis = 0; } } diff --git a/libavcodec/jpeg2000.h b/libavcodec/jpeg2000.h index c3437b02fe..ad58b1ae88 100644 --- a/libavcodec/jpeg2000.h +++ b/libavcodec/jpeg2000.h @@ -127,6 +127,7 @@ typedef struct Jpeg2000T1Context { typedef struct Jpeg2000TgtNode { uint8_t val; + uint8_t temp_val; uint8_t vis; struct Jpeg2000TgtNode *parent; } Jpeg2000TgtNode;
From: Gautam Ramakrishnan <gautamramk@gmail.com> The implementation of tag tree encoding was incorrect. However, this error was not visible as the current j2k encoder encodes only 1 layer. This patch fixes tag tree coding for JPEG2000 such tag tree coding would work for multi layer encoding. --- libavcodec/j2kenc.c | 41 ++++++++++++++++++++++++----------------- libavcodec/jpeg2000.c | 1 + libavcodec/jpeg2000.h | 1 + 3 files changed, 26 insertions(+), 17 deletions(-)