[FFmpeg-devel] [PATCH][ticket #5522] lavc/cfhd: interlaced frame decoding added
Michael Niedermayer
michael at niedermayer.cc
Sun Jun 24 01:34:43 EEST 2018
Hi
On Tue, May 22, 2018 at 09:10:21PM +0530, Gagandeep Singh wrote:
> ticket #5522 output of given samples significantly improved
> ---
> libavcodec/cfhd.c | 181 +++++++++++++++++++++++++++++++++++++++++++-----------
> libavcodec/cfhd.h | 9 +++
> 2 files changed, 155 insertions(+), 35 deletions(-)
>
> diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
> index 7ceb803595..051d210355 100644
> --- a/libavcodec/cfhd.c
> +++ b/libavcodec/cfhd.c
> @@ -49,12 +49,15 @@ enum CFHDParam {
> SubbandNumber = 48,
> Quantization = 53,
> ChannelNumber = 62,
> + SampleFlags = 68,
> BitsPerComponent = 101,
> ChannelWidth = 104,
> ChannelHeight = 105,
> PrescaleShift = 109,
> };
>
> +
> +
> static av_cold int cfhd_init(AVCodecContext *avctx)
> {
> CFHDContext *s = avctx->priv_data;
> @@ -72,6 +75,13 @@ static void init_plane_defaults(CFHDContext *s)
> s->subband_num_actual = 0;
> }
>
> +static void init_peak_table_defaults(CFHDContext *s)
> +{
> + s->peak.level = 0;
> + s->peak.offset = 0;
> + s->peak.base = NULL;
> +}
> +
> static void init_frame_defaults(CFHDContext *s)
> {
> s->coded_width = 0;
> @@ -86,15 +96,44 @@ static void init_frame_defaults(CFHDContext *s)
> s->wavelet_depth = 3;
> s->pshift = 1;
> s->codebook = 0;
> + s->difference_coding = 0;
> + s->progressive = 0;
> init_plane_defaults(s);
> + init_peak_table_defaults(s);
> }
>
> /* TODO: merge with VLC tables or use LUT */
> -static inline int dequant_and_decompand(int level, int quantisation)
> +static inline int dequant_and_decompand(int level, int quantisation, int codebook)
> +{
> + if (codebook == 0 || codebook == 1) {
> + int64_t abslevel = abs(level);
> + if (level < 264)
> + return (abslevel + ((768 * abslevel * abslevel * abslevel) / (255 * 255 * 255))) *
> + FFSIGN(level) * quantisation;
> + else
> + return level * quantisation;
> + } else
> + return level * quantisation;
> +}
> +
> +static inline void difference_coding(int16_t *band, int width, int height)
> +{
> +
> + int i,j;
> + for (i = 0; i < height; i++) {
> + for (j = 1; j < width; j++) {
> + band[j] += band[j-1];
> + }
> + band += width;
> + }
> +}
> +
> +static inline void peak_table(int16_t *band, Peak *peak, int length)
> {
> - int64_t abslevel = abs(level);
> - return (abslevel + ((768 * abslevel * abslevel * abslevel) / (255 * 255 * 255))) *
> - FFSIGN(level) * quantisation;
> + int i;
> + for (i = 0; i < length; i++)
> + if (abs(band[i]) > peak->level)
> + band[i] = *(peak->base++);
This directly reads from the bytestream cast to int16.
This is likely wrong for bigendian
> }
>
> static inline void process_alpha(int16_t *alpha, int width)
> @@ -154,6 +193,18 @@ static inline void filter(int16_t *output, ptrdiff_t out_stride,
> }
> }
>
> +static inline void interlaced_vertical_filter(int16_t *output, int16_t *low, int16_t *high,
> + int width, int linesize, int plane)
> +{
> + int i;
> + int16_t even, odd;
> + for (i = 0; i < width; i++) {
> + even = (low[i] - high[i])/2;
> + odd = (low[i] + high[i])/2;
> + output[i] = av_clip_uintp2(even, 10);
> + output[i + linesize] = av_clip_uintp2(odd, 10);
> + }
> +}
> static void horiz_filter(int16_t *output, int16_t *low, int16_t *high,
> int width)
> {
> @@ -295,6 +346,9 @@ static int cfhd_decode(AVCodecContext *avctx, void *data, int *got_frame,
> uint16_t data = bytestream2_get_be16(&gb);
> if (abs_tag8 >= 0x60 && abs_tag8 <= 0x6f) {
> av_log(avctx, AV_LOG_DEBUG, "large len %x\n", ((tagu & 0xff) << 16) | data);
> + } else if (tag == SampleFlags) {
> + av_log(avctx, AV_LOG_DEBUG, "Progressive?%"PRIu16"\n", data);
> + s->progressive = data & 0x0001;
> } else if (tag == ImageWidth) {
> av_log(avctx, AV_LOG_DEBUG, "Width %"PRIu16"\n", data);
> s->coded_width = data;
> @@ -393,6 +447,8 @@ static int cfhd_decode(AVCodecContext *avctx, void *data, int *got_frame,
> }
> av_log(avctx, AV_LOG_DEBUG, "Transform-type? %"PRIu16"\n", data);
> } else if (abstag >= 0x4000 && abstag <= 0x40ff) {
> + if (abstag == 0x4001)
> + s->peak.level = 0;
> av_log(avctx, AV_LOG_DEBUG, "Small chunk length %d %s\n", data * 4, tag < 0 ? "optional" : "required");
> bytestream2_skipu(&gb, data * 4);
> } else if (tag == 23) {
> @@ -450,7 +506,8 @@ static int cfhd_decode(AVCodecContext *avctx, void *data, int *got_frame,
> s->codebook = data;
> av_log(avctx, AV_LOG_DEBUG, "Codebook %i\n", s->codebook);
> } else if (tag == 72) {
> - s->codebook = data;
> + s->codebook = data & 0xf;
> + s->difference_coding = (data >> 4) & 1;
> av_log(avctx, AV_LOG_DEBUG, "Other codebook? %i\n", s->codebook);
> } else if (tag == 70) {
> av_log(avctx, AV_LOG_DEBUG, "Subsampling or bit-depth flag? %i\n", data);
> @@ -477,6 +534,19 @@ static int cfhd_decode(AVCodecContext *avctx, void *data, int *got_frame,
> } else if (tag == -85) {
> av_log(avctx, AV_LOG_DEBUG, "Cropped height %"PRIu16"\n", data);
> s->cropped_height = data;
> + } else if (tag == -75) {
> + s->peak.offset &= ~0xffff;
> + s->peak.offset |= (data & 0xffff);
> + s->peak.base = (int16_t *) gb.buffer;
> + s->peak.level = 0;
> + } else if (tag == -76) {
> + s->peak.offset &= 0xffff;
> + s->peak.offset |= (data & 0xffff)<<16;
Undefined shift, Issue 8695 in oss-fuzz
If you fix this please add "Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg"
> + s->peak.base = (int16_t *) gb.buffer;
> + s->peak.level = 0;
> + } else if (tag == -74 && s->peak.offset) {
> + s->peak.level = data;
> + s->peak.base += s->peak.offset / 2 - 2;
Is the offset checked anywhere ?
adding a unchecked value to a pointer is a undefined operation
but more so i do not see anything checking the offst or pointer
before it is dereferenced. If so this would allow one to crash the
decoder with an out of array read
All input data has to be checked if some value could cause undefined
behavior, crashes, or worse.
The fuzzers did not find this one yet. This was just spotted by me when
looking at the undefined shift.
Also, why are people attacking each other over spliting this?
Split or not split, i think more energy should be put into reviewing
patches than attacking each other
Thanks
PS: for reference this was pushed to master in 9cefb9e7ec508900ba147e6977590f03456aa15c
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180624/6353add0/attachment.sig>
More information about the ffmpeg-devel
mailing list