[FFmpeg-devel] [PATCH 5/9] sbc: implement SBC encoder (low-complexity subband codec)

[Aurelien Jacobs]

On Wed, Feb 28, 2018 at 12:59:40AM +0000, Rostislav Pehlivanov wrote:
> On 27 February 2018 at 23:56, Aurelien Jacobs <aurel at gnuage.org> wrote:
> 
> >
> > So I've updated the patch with only a msbc and a delay option and
> > I've added some sane parameters decisions, mainly based on the following
> > study :
> > https://pdfs.semanticscholar.org/1f19/561d03bc88b67728375566c95bbf77
> > e730d5.pdf
> > This seems to give pretty good results.
> >
> >
> I think you ought to use a float for this, like:
> 
> >{ "sbc_delay", "Maximum delay in milliseconds", offsetof(<context>,
> <option>), AV_OPT_TYPE_FLOAT, { .dbl = <some reasonable default> }, <min>,
> <max>, <flags>, <option name in structure> },

Why would I want to use float for this ??
AV_OPT_TYPE_DURATION is more appropriate for this. It has microsecond
precision which is more than enough, and you can still use a "float"
string as a parameter (eg. -sbc_delay 0.0035 for a 3.5 ms maximum delay).

> Apart from that, what's up with the weird if (!sbc->init) { } block in the
> main encoding function? Shouldn't that be done in sbc_encode_init()?

I seem to remember there was a good reason for this initially, but the
code went thru quite some refactoring since then, and there is no good
reason for this anymore.
I've now cleaned up and simplified this init code.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-sbc-implement-SBC-encoder-low-complexity-subband-cod.patch
Type: text/x-diff
Size: 53398 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180301/6e769a17/attachment.patch>