Message ID | 57CB527D.3060804@impactstudiopro.com |
---|---|
State | Accepted |
Headers | show |
On Sat, Sep 03, 2016 at 03:45:17PM -0700, Jonathan Campbell wrote: > On 09/03/2016 07:07 AM, James Almer wrote: > > On 9/3/2016 7:50 AM, Jonathan Campbell wrote: > >> +int av_lfg_init_from_data(AVLFG *c, const uint8_t *data, unsigned int length) { > >> + unsigned int beg, end, segm; > >> + const AVCRC *avcrc; > >> + uint32_t crc = 1; > >> + > >> + c->index = 0; > > > > The AVLFG struct should IMO remain untouched if init is guaranteed to fail, > > as it would be the case with an out or range length value, so move this after > > that check. > > > >> + avcrc = av_crc_get_table(AV_CRC_32_IEEE); /* This can't fail. It's a well-defined table in crc.c */ > > > > You could also move this one below. > > > >> + > >> + /* avoid integer overflow in the loop below. */ > >> + if (length > (UINT_MAX / 128U)) return AVERROR(EINVAL); > >> + > >> + /* across 64 segments of the incoming data, > >> + * do a running crc of each segment and store the crc as the state for that slot. > >> + * this works even if the length of the segment is 0 bytes. */ > >> + beg = 0; > >> + for (segm = 0;segm < 64;segm++) { > >> + end = (((segm + 1) * length) / 64); > >> + crc = av_crc(avcrc, crc, data + beg, end - beg); > > > > The thing about speed made me wonder, wouldn't using adler32 be faster? > > If so, this feature could maybe even be enabled by default with it. > > > >> + c->state[segm] = (unsigned int)crc; > >> + beg = end; > >> + } > >> + > >> + return 0; > >> +} > >> diff --git a/libavutil/lfg.h b/libavutil/lfg.h > >> index ec90562..72eb673 100644 > >> --- a/libavutil/lfg.h > >> +++ b/libavutil/lfg.h > >> @@ -22,6 +22,8 @@ > >> #ifndef AVUTIL_LFG_H > >> #define AVUTIL_LFG_H > >> > >> +#include <stdint.h> /* uint8_t type */ > >> + > >> typedef struct AVLFG { > >> unsigned int state[64]; > >> int index; > >> @@ -29,6 +31,13 @@ typedef struct AVLFG { > >> > >> void av_lfg_init(AVLFG *c, unsigned int seed); > >> > >> +/* > > > > This is meant for doxygen, so /** like in every other function. > > > >> + * Seed the state of the ALFG using binary data. > >> + * > >> + * Return value: 0 on success, negative value (AVERROR) on failure. > >> + */ > >> +int av_lfg_init_from_data(AVLFG *c, const uint8_t *data, unsigned int length); > >> + > >> /** > >> * Get the next random unsigned 32-bit number using an ALFG. > >> * > > > > No more comments for me. > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > Here you go. combined the related patches, added APIdocs update and pushed thanks [...]
diff --git a/libavutil/version.h b/libavutil/version.h index 7d32c7b..60b58eb 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,7 +79,7 @@ */ #define LIBAVUTIL_VERSION_MAJOR 55 -#define LIBAVUTIL_VERSION_MINOR 29 +#define LIBAVUTIL_VERSION_MINOR 30 #define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \