[FFmpeg-devel] CNG (consistent noise generation) patch for AC-3 decoder

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.

Jonathan Campbell
From 5de30d309fb48b26acd8d95f14ef4d4064450ddc Mon Sep 17 00:00:00 2001
From: Jonathan Campbell <jonathan@castus.tv>
Date: Sat, 3 Sep 2016 03:20:41 -0700
Subject: [PATCH 1/4] libavutil version bump, av_lfg_init_from_data() incoming

---
 libavutil/version.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

[...]