| Message ID | 20180321164958.15469-1-deepgagan231197@gmail.com |
|---|---|
| State | Accepted |
| Commit | c64c97b972c7325a71440a352a7d541a8c92b2da |
| Headers | show |
On Wed, 21 Mar 2018, 22:20 Gagandeep Singh, <deepgagan231197@gmail.com> wrote: > alpha decompanding curve added to post process the decoded alpha channel > --- > libavcodec/cfhd.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c > index fd5555834b..e35732df45 100644 > --- a/libavcodec/cfhd.c > +++ b/libavcodec/cfhd.c > @@ -37,6 +37,9 @@ > #include "thread.h" > #include "cfhd.h" > > +#define ALPHA_COMPAND_DC_OFFSET 256 > +#define ALPHA_COMPAND_GAIN 9400 > + > enum CFHDParam { > ChannelCount = 12, > SubbandCount = 14, > @@ -94,6 +97,20 @@ static inline int dequant_and_decompand(int level, int > quantisation) > FFSIGN(level) * quantisation; > } > > +static inline void process_alpha(int16_t *alpha, int width) > +{ > + int i, channel; > + for (i = 0; i < width; i++) { > + channel = alpha[i]; > + channel -= ALPHA_COMPAND_DC_OFFSET; > + channel <<= 3; > + channel *= ALPHA_COMPAND_GAIN; > + channel >>= 16; > + channel = av_clip_uintp2(channel, 12); > + alpha[i] = channel; > + } > +} > + > static inline void filter(int16_t *output, ptrdiff_t out_stride, > int16_t *low, ptrdiff_t low_stride, > int16_t *high, ptrdiff_t high_stride, > @@ -792,6 +809,8 @@ static int cfhd_decode(AVCodecContext *avctx, void > *data, int *got_frame, > high = s->plane[plane].l_h[7]; > for (i = 0; i < lowpass_height * 2; i++) { > horiz_filter_clip(dst, low, high, lowpass_width, s->bpc); > + if (act_plane == 3) > + process_alpha(dst, lowpass_width * 2); > low += lowpass_width; > high += lowpass_width; > dst += pic->linesize[act_plane] / 2; > -- > 2.14.1 > ticket #6265 >
2018-03-21 17:52 GMT+01:00, Gagandeep Singh <deepgagan231197@gmail.com>: > On Wed, 21 Mar 2018, 22:20 Gagandeep Singh, <deepgagan231197@gmail.com> > wrote: > ticket #6265 Should be part of the commit message. Carl Eugen
On Thu 22 Mar, 2018, 1:54 AM Carl Eugen Hoyos, <ceffmpeg@gmail.com> wrote: > 2018-03-21 17:52 GMT+01:00, Gagandeep Singh <deepgagan231197@gmail.com>: > > On Wed, 21 Mar 2018, 22:20 Gagandeep Singh, <deepgagan231197@gmail.com> > > wrote: > > > ticket #6265 > > Should be part of the commit message. > > Carl Eugen > So, should I send the mail again with proper commit this time. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Wed, Mar 21, 2018 at 10:19:58PM +0530, Gagandeep Singh wrote: > alpha decompanding curve added to post process the decoded alpha channel > --- > libavcodec/cfhd.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c > index fd5555834b..e35732df45 100644 > --- a/libavcodec/cfhd.c > +++ b/libavcodec/cfhd.c > @@ -37,6 +37,9 @@ > #include "thread.h" > #include "cfhd.h" > > +#define ALPHA_COMPAND_DC_OFFSET 256 > +#define ALPHA_COMPAND_GAIN 9400 > + > enum CFHDParam { > ChannelCount = 12, > SubbandCount = 14, > @@ -94,6 +97,20 @@ static inline int dequant_and_decompand(int level, int quantisation) > FFSIGN(level) * quantisation; > } > > +static inline void process_alpha(int16_t *alpha, int width) > +{ > + int i, channel; > + for (i = 0; i < width; i++) { > + channel = alpha[i]; > + channel -= ALPHA_COMPAND_DC_OFFSET; > + channel <<= 3; > + channel *= ALPHA_COMPAND_GAIN; Any reason why you can't merge the << 3 (ie. * 8) with the * ALPHA_COMPAND_GAIN ? > + channel >>= 16; > + channel = av_clip_uintp2(channel, 12); > + alpha[i] = channel; Here you should affect the result of av_clip_uintp2 directly to alpha[i]. Actually, I think it would be more readable by dropping the channel intermediate variable entirely. You could write this function like this (untested): static inline void process_alpha(int16_t *alpha, int width) { for (int i = 0; i < width; i++) alpha[i] = av_clip_uintp2(((alpha[i] - 256) * 75200) >> 16, 12); } Of course, you can use DC_OFFSET and GAIN constants in there if you think it is more readable.
On Fri, 23 Mar 2018, 04:26 Aurelien Jacobs, <aurel@gnuage.org> wrote: > On Wed, Mar 21, 2018 at 10:19:58PM +0530, Gagandeep Singh wrote: > > alpha decompanding curve added to post process the decoded alpha channel > > --- > > libavcodec/cfhd.c | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c > > index fd5555834b..e35732df45 100644 > > --- a/libavcodec/cfhd.c > > +++ b/libavcodec/cfhd.c > > @@ -37,6 +37,9 @@ > > #include "thread.h" > > #include "cfhd.h" > > > > +#define ALPHA_COMPAND_DC_OFFSET 256 > > +#define ALPHA_COMPAND_GAIN 9400 > > + > > enum CFHDParam { > > ChannelCount = 12, > > SubbandCount = 14, > > @@ -94,6 +97,20 @@ static inline int dequant_and_decompand(int level, > int quantisation) > > FFSIGN(level) * quantisation; > > } > > > > +static inline void process_alpha(int16_t *alpha, int width) > > +{ > > + int i, channel; > > + for (i = 0; i < width; i++) { > > + channel = alpha[i]; > > + channel -= ALPHA_COMPAND_DC_OFFSET; > > + channel <<= 3; > > + channel *= ALPHA_COMPAND_GAIN; > > Any reason why you can't merge the << 3 (ie. * 8) with the > * ALPHA_COMPAND_GAIN ? > > > + channel >>= 16; > > > + channel = av_clip_uintp2(channel, 12); > > + alpha[i] = channel; > > Here you should affect the result of av_clip_uintp2 directly to alpha[i]. > > Actually, I think it would be more readable by dropping the channel > intermediate variable entirely. You could write this function like this > (untested): > > static inline void process_alpha(int16_t *alpha, int width) > { > for (int i = 0; i < width; i++) > alpha[i] = av_clip_uintp2(((alpha[i] - 256) * 75200) >> 16, 12); > } > > Of course, you can use DC_OFFSET and GAIN constants in there if you > think it is more readable. > I will test it, I remember the problem was with the bit shifting in alpha as it is originally smaller than channel, so I used 32 bit channel, but I will see if it can also work your way. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Fri, 23 Mar 2018, 11:04 Gagandeep Singh, <deepgagan231197@gmail.com> wrote: > > > On Fri, 23 Mar 2018, 04:26 Aurelien Jacobs, <aurel@gnuage.org> wrote: > >> On Wed, Mar 21, 2018 at 10:19:58PM +0530, Gagandeep Singh wrote: >> > alpha decompanding curve added to post process the decoded alpha channel >> > --- >> > libavcodec/cfhd.c | 19 +++++++++++++++++++ >> > 1 file changed, 19 insertions(+) >> > >> > diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c >> > index fd5555834b..e35732df45 100644 >> > --- a/libavcodec/cfhd.c >> > +++ b/libavcodec/cfhd.c >> > @@ -37,6 +37,9 @@ >> > #include "thread.h" >> > #include "cfhd.h" >> > >> > +#define ALPHA_COMPAND_DC_OFFSET 256 >> > +#define ALPHA_COMPAND_GAIN 9400 >> > + >> > enum CFHDParam { >> > ChannelCount = 12, >> > SubbandCount = 14, >> > @@ -94,6 +97,20 @@ static inline int dequant_and_decompand(int level, >> int quantisation) >> > FFSIGN(level) * quantisation; >> > } >> > >> > +static inline void process_alpha(int16_t *alpha, int width) >> > +{ >> > + int i, channel; >> > + for (i = 0; i < width; i++) { >> > + channel = alpha[i]; >> > + channel -= ALPHA_COMPAND_DC_OFFSET; >> > + channel <<= 3; >> > + channel *= ALPHA_COMPAND_GAIN; >> >> Any reason why you can't merge the << 3 (ie. * 8) with the >> * ALPHA_COMPAND_GAIN ? >> >> > + channel >>= 16; >> >> > + channel = av_clip_uintp2(channel, 12); >> > + alpha[i] = channel; >> >> Here you should affect the result of av_clip_uintp2 directly to alpha[i]. >> >> Actually, I think it would be more readable by dropping the channel >> intermediate variable entirely. You could write this function like this >> (untested): >> >> static inline void process_alpha(int16_t *alpha, int width) >> { >> for (int i = 0; i < width; i++) >> alpha[i] = av_clip_uintp2(((alpha[i] - 256) * 75200) >> 16, 12); >> } >> >> Of course, you can use DC_OFFSET and GAIN constants in there if you >> think it is more readable. >> > > I will test it, I remember the problem was with the bit shifting in alpha > as it is originally smaller than channel, so I used 32 bit channel, but I > will see if it can also work your way. > Basically the max alpha value (4095) was overflowing on directly using alpha, I might be able to shorten it using some type change > _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >
diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c index fd5555834b..e35732df45 100644 --- a/libavcodec/cfhd.c +++ b/libavcodec/cfhd.c @@ -37,6 +37,9 @@ #include "thread.h" #include "cfhd.h" +#define ALPHA_COMPAND_DC_OFFSET 256 +#define ALPHA_COMPAND_GAIN 9400 + enum CFHDParam { ChannelCount = 12, SubbandCount = 14, @@ -94,6 +97,20 @@ static inline int dequant_and_decompand(int level, int quantisation) FFSIGN(level) * quantisation; } +static inline void process_alpha(int16_t *alpha, int width) +{ + int i, channel; + for (i = 0; i < width; i++) { + channel = alpha[i]; + channel -= ALPHA_COMPAND_DC_OFFSET; + channel <<= 3; + channel *= ALPHA_COMPAND_GAIN; + channel >>= 16; + channel = av_clip_uintp2(channel, 12); + alpha[i] = channel; + } +} + static inline void filter(int16_t *output, ptrdiff_t out_stride, int16_t *low, ptrdiff_t low_stride, int16_t *high, ptrdiff_t high_stride, @@ -792,6 +809,8 @@ static int cfhd_decode(AVCodecContext *avctx, void *data, int *got_frame, high = s->plane[plane].l_h[7]; for (i = 0; i < lowpass_height * 2; i++) { horiz_filter_clip(dst, low, high, lowpass_width, s->bpc); + if (act_plane == 3) + process_alpha(dst, lowpass_width * 2); low += lowpass_width; high += lowpass_width; dst += pic->linesize[act_plane] / 2;