Hi,
On Mon, Aug 06, 2018 at 12:35:15 +0200, Paul B Mahol wrote:
> patch attached.
I can't judge on most of the procedural stuff, but here's some 2 cents:
> libavcodec/Makefile | 1 +
> libavcodec/allcodecs.c | 1 +
> libavcodec/avcodec.h | 1 +
> libavcodec/codec_desc.c | 7 +
> libavcodec/imm4.c | 399
> ++++++++++++++++++++++++++++++++++++++++++++++++
> libavformat/riff.c | 1 +
Changelog and docs missing.
> OBJS-$(CONFIG_INTERPLAY_VIDEO_DECODER) += interplayvideo.o
> +OBJS-$(CONFIG_IMM4_DECODER) += imm4.o
> OBJS-$(CONFIG_JACOSUB_DECODER) += jacosubdec.o ass.o
Alphabetical order?
> extern AVCodec ff_interplay_video_decoder;
> +extern AVCodec ff_imm4_decoder;
> extern AVCodec ff_jpeg2000_encoder;
Alphabetical order?
> +static uint16_t table_7[304] = {
> + 0u, 0u, 0u, 0u, 0u, 0u, 0u, 0u, 16514u, 16514u, 16387u, 16387u,
Inconsisent to declare these explicitly with 'u',
> +static uint8_t table_8[] = {
> + 0, 12, 11, 11, 11, 11, 11, 11, 12, 12,
but not these, but it doesn't really matter.
Shouldn't they be const, BTW?
> + if (x)
> + return table_5[2 * value];
> + else
> + return 15 - table_5[2 * value];
I've seen requests on this list to "simplify" such terms to
return x ? table_5[2 * value] : 15 - table_5[2 * value];
but I don't care, and consider your syntax more readable.
> + if (!c) {
> + f |= 2;
> + av_assert0(0);
> + }
If it always aborts, what's the use of the operation on f?
av_assert0(c); ?
> + printf("gb: %d\n", get_bits_left(gb));
Debug code?
Cheers,
Moritz
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
