I've resubmitted the patch in a more reviewable format here:
http://ffmpeg.org/pipermail/ffmpeg-devel/2017-June/212532.html
having said that:
On Mon, 2017-06-19 at 14:22 +0200, Moritz Barsnick wrote:
> On Sat, Jun 17, 2017 at 19:13:52 +0200, Hein-Pieter van Braam wrote:
> > + s->pixel_ptr = frame->data[0] + x + y*frame-
> > >linesize[0];
I'll fix this, I copy/pasted this from elsewhere in the file, kind of
sloppy.
> Style: You might want to add spaces around '*'.
>
> > + if (! opcode) {
>
> Style: You should drop the space.
You mean the space between the ! and opcode? The space between if and
opening parenthesis should stay, right?
>
> > + if (opcode < 0) {
> > + off_x = ((unsigned short)opcode - 0xC000) % frame-
> > >linesize[0];
> > + off_y = ((unsigned short)opcode - 0xC000) / frame-
> > >linesize[0];
> > + copy_from(s, s->last_frame, frame, off_x, off_y);
> > + }
> > + if (opcode > 0) {
>
> else if?
>
> > + if (opcode < 0) {
> > + off_x = ((unsigned short)opcode - 0xC000)
> > % s->cur_decode_frame->linesize[0];
> > + off_y = ((unsigned short)opcode - 0xC000)
> > / s->cur_decode_frame->linesize[0];
> > + copy_from(s, s->prev_decode_frame, s-
> > >cur_decode_frame, off_x, off_y);
> > + }
> > + if (opcode > 0) {
I'll fix both of these
> Same here.
>
> > + if (s->frame_format == 0x06)
> > + ipvideo_decode_format_06_opcodes(s, frame);
> > +
> > + if (s->frame_format == 0x10)
> > + ipvideo_decode_format_10_opcodes(s, frame);
> > +
> > + if (s->frame_format == 0x11)
> > + ipvideo_decode_format_11_opcodes(s, frame);
>
> Unless ipvideo_decode_format_NN_opcodes() modifies s->frame_format,
> which doesn't seem to be the case, this is either if/else if/else if,
> or switch/case.
This got replaced with a switch case in the new patchset
> Cheers,
> Moritz
Thanks for the review!
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
