On 8/25/2016 6:53 PM, Burt P. wrote:
> On Thu, Aug 25, 2016 at 8:56 AM, Diego Biurrun <[email protected]> wrote:
>> On Wed, Aug 24, 2016 at 07:34:22AM -0500, Burt P wrote:
>>
>> This probably deserves a mention in doc/general.texi and a version bump
>> for libavfilter.
>>
>>> +typedef struct HDCDContext {
>>> + const AVClass *class;
>>> +
>>> + hdcd_simple_t *shdcd;
>>
>> The _t namespace is reserved for POSIX, the library should not invade it.
>
> Um, I think it is too late to change it.
Seeing this is kinda important, i think it's a good reason to make the
change even if you already made a release.
The library is currently used by nothing, so you can safely bump the
soname version and make sure to tag release 0.1 as a bad/buggy one.
Pity it wasn't pointed in the first patch you sent, though.
>
>>> +static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>>> +{
>>> + out = ff_get_audio_buffer(outlink, in->nb_samples);
>>> + if (!out) {
>>> + av_frame_free(&in);
>>> + return AVERROR(ENOMEM);
>>> + }
>>> + result = av_frame_copy_props(out, in);
>>> + if (result) {
>>> + av_frame_free(&in);
>>> + return result;
>>> + }
>>
>> Where does out get freed if av_frame_copy_props() fails?
>
> Done.
>
>>> +static av_cold void uninit(AVFilterContext *ctx)
>>> +{
>>> + shdcd_detect_str(s->shdcd, detect_str, sizeof(detect_str));
>>> + shdcd_free(s->shdcd);
>>
>> Why does the library use an shdcd instead of an hdcd prefix?
>
> It is the "simple" api of the library. Maybe it should have been
> hdcds_ or something,
> but it isn't.
You can change this alongside the above.
>
>>
>>> +static void af_hdcd_log(const void *priv, const char* fmt, va_list args)
>>
>> char *fmt
>>
>
> Done.
>
>
> For indentation and other cosmetic changes, could you submit a patch
> after this? I don't feel I will be able to get it spaces-perfect for
> you.
>
> Thanks for the comments.
> --
> Burt
> _______________________________________________
> libav-devel mailing list
> [email protected]
> https://lists.libav.org/mailman/listinfo/libav-devel
>
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
