On Thu, Apr 28, 2011 at 7:35 AM, Justin Ruggles <[email protected]> wrote: > Hi, > > On 04/27/2011 08:19 PM, Aℓex Converse wrote: > >> + frame_size = (h >> 16) & 0xffff; >> + channels = 2 + 2*((h >> 14) & 0x03); >> + id = (h >> 6) & 0xff; >> + bits = 16 + 4*((h >> 4) & 0x03); > > > This would be easier to read if aligned. like this: > > frame_size = (h >> 16) & 0xffff; > channels = ((h >> 14) & 0x0003) * 2 + 2; > id = (h >> 6) & 0x00ff; > bits = ((h >> 4) & 0x0003) * 4 + 16; > >> + int frame_size, buf_size = avpkt->size; >> + >> + frame_size = s302m_parse_frame_header(avctx, buf, buf_size); >> + if (frame_size < 0) >> + return -1; > > > frame_size isn't used anywhere else in this function, so it is not needed. > >> + if (avctx->bits_per_coded_sample == 24) { > ... >> + *data_size = (uint8_t*)o - (uint8_t*)data; >> + } else if (avctx->bits_per_coded_sample == 20) { > ... >> + *data_size = (uint8_t*)o - (uint8_t*)data; >> + } else { > ... >> + *data_size = (uint8_t*)o - (uint8_t*)data; >> + } > > > the 3 duplicate lines can be moved to after the if/else. >
This gets messy because o is a different type in the final else. > The reset looks good after Diego's comments are addressed. > And a fate test would be nice. > > Thanks, > Justin _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
