| Message ID | 57CAAAF9.6000207@impactstudiopro.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi! 2016-09-03 12:50 GMT+02:00 Jonathan Campbell <jonathan@impactstudiopro.com>: > Here you go (as attachments). The changes to lfg and the version bump must be one patch. > + { "cons_noisegen", "enable consistent noise generation", OFFSET(consistent_noise_generation), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, PAR }, If this change makes sense why is it not the default? Carl Eugen
On 9/3/2016 8:09 AM, Carl Eugen Hoyos wrote:
> The changes to lfg and the version bump must be one patch.
They will be squashed. See
https://ffmpeg.org/pipermail/ffmpeg-devel/2016-August/198446.html
https://ffmpeg.org/pipermail/ffmpeg-devel/2016-August/198448.html
https://ffmpeg.org/pipermail/ffmpeg-devel/2016-August/198449.html
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.
On 09/03/2016 04:09 AM, Carl Eugen Hoyos wrote: > Hi! > > 2016-09-03 12:50 GMT+02:00 Jonathan Campbell <jonathan@impactstudiopro.com>: > >> Here you go (as attachments). > > The changes to lfg and the version bump must be one patch. > >> + { "cons_noisegen", "enable consistent noise generation", OFFSET(consistent_noise_generation), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, PAR }, > > If this change makes sense why is it not the default? > > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > The option is intended for non-linear editing software that needs to be able to decode AC-3 from any arbitrary point in the stream. Making the dithering noise consistent means the AC-3 decoder will produce the same audio samples on output no matter where you begin decoding or how many frames you've decoded. If the dithering noise is allowed to vary, then decoding from point A to B, freeing the decoder context, then later allocating a context and resuming decode from point B to C, will produce a slight discontinuity at point B where the decoded audio is put together because the dithering noise applied to some frequency bands came out differently between the two decodes. If the noise is made consistent, then the decoded audio at point B will come out the exact same as if you had decoded continuously from point A to C. But, if you just want to play a media file and you don't care about that consistency, then the CPU work to ensure a consistent decode like that is a waste of effort. Playing, streaming and transcoding sequentially is probably 99 percent of FFMPEG's general use, right? I'm well aware the AC-3 decoder has a window delay in the frequency domain. Decoding from point B to C actually means going to point B, stepping back 1 to 2 AC-3 frames, decoding up to B and discarding the audio, then taking decoded audio from point B on. Do you understand now why this is useful for NLE software, but should not be enabled by default? Jonathan Campbell
On 9/3/2016 11:07 AM, James Almer wrote: >> > + >> > + /* 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? Nevermind, probably not a good idea. adler32 wouldn't be a good seed for lfg.
Hi! 2016-09-03 18:27 GMT+02:00 Jonathan Campbell <jonathan@impactstudiopro.com>: > Do you understand now why this is useful for NLE software, Of course. > but should not be enabled by default? Only if I completely underestimate the additional cpu usage. Thank you, Carl Eugen
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, \