[FFmpeg-devel] [PATCH] avfilter: add dumpwave filter.

Dmytro Humeniuk dmitry.gumenyuk at gmail.com
Sun Jan 14 00:59:59 EET 2018


Hi,
> On 12 Jan 2018, at 00:20, Dmitry Gumenyuk <dmitry.gumenyuk at gmail.com> wrote:
> --
> Best regards,
> Dmytro
> 
>> On 11 Jan 2018, at 23:57, Paul B Mahol <onemda at gmail.com> wrote:
>> 
>>> On 1/11/18, Dmitry Gumenyuk <dmitry.gumenyuk at gmail.com> wrote:
>>> 
>>>>> On 11 Jan 2018, at 23:02, Paul B Mahol <onemda at gmail.com> wrote:
>>>>> 
>>>>> On 1/11/18, Dmitry Gumenyuk <dmitry.gumenyuk at gmail.com> wrote:
>>>>> 
>>>>>>> On 11 Jan 2018, at 22:41, Paul B Mahol <onemda at gmail.com> wrote:
>>>>>>> 
>>>>>>> On 1/11/18, Dmitry Gumenyuk <dmitry.gumenyuk at gmail.com> wrote:
>>>>>>> Hello
>>>>>>> 
>>>>>>> 2018-01-10 11:33 GMT+01:00 Moritz Barsnick <barsnick at gmx.net>:
>>>>>>>>> On Wed, Jan 10, 2018 at 08:58:05 +0100, dmitry.gumenyuk at gmail.com
>>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>> + at table @option
>>>>>>>>> + at item d
>>>>>>>>> +Dimensions @code{WxH}.
>>>>>>>>> + at code{W} - number of data values in json, values will be scaled
>>>>>>>>> according to @code{H}.
>>>>>>>>> +The default value is @var{640x480}
>>>>>>>>> +
>>>>>>>>> + at item s
>>>>>>>>> +Samples count per value per channel
>>>>>>>> 
>>>>>>>> In most other filters/filtersources, s+size is used for dimensions,
>>>>>>>> d+duration for time, and n for an amount/number of frames/samples or
>>>>>>>> so. Might be a good idea to align with this. And use aliases (i.e.
>>>>>>>> implement both "s" and "size").
>>>>>>>> 
>>>>>>>>> +static const AVOption dumpwave_options[] = {
>>>>>>>>> +    { "d", "set width and height", OFFSET(w),
>>>>>>>>> AV_OPT_TYPE_IMAGE_SIZE,
>>>>>>>>> {.str = "640x480"}, 0, 0, FLAGS },
>>>>>>>>> +    { "s", "set number of samples per value per channel",
>>>>>>>>> OFFSET(s),
>>>>>>>>> AV_OPT_TYPE_INT64,  {.i64 = 0}, 0, INT64_MAX, FLAGS },
>>>>>>>>> +    { "json", "set json dump file", OFFSET(json),
>>>>>>>>> AV_OPT_TYPE_STRING,
>>>>>>>>> {
>>>>>>>>> .str = NULL }, 0, 0, FLAGS },
>>>>>>>>> +    { NULL }
>>>>>>>> [...]
>>>>>>>>> +static av_cold int init(AVFilterContext *ctx)
>>>>>>>>> +{
>>>>>>>>> +    DumpWaveContext *dumpwave = ctx->priv;
>>>>>>>>> +    if(!dumpwave->s) {
>>>>>>>>> +        dumpwave->is_disabled = 1;
>>>>>>>>> +        av_log(ctx, AV_LOG_ERROR, "Invalid samples per value
>>>>>>>>> config\n");
>>>>>>>>> +    }
>>>>>>>>> +    return 0;
>>>>>>>> 
>>>>>>>> I don't like the idea of having to provide the "s" parameter. Is
>>>>>>>> there
>>>>>>>> really no useful default?
>>>>>>>> 
>>>>>>>> And now, if s=0, what does the filter do? Just sit around? Couldn't
>>>>>>>> it
>>>>>>>> fail instead?
>>>>>>>> 
>>>>>>>> Apart from that:
>>>>>>>> "if (" is the bracket style ffmpeg uses.
>>>>>>>> 
>>>>>>>>> +        if (dumpwave->json && !(dump_fp =
>>>>>>>>> av_fopen_utf8(dumpwave->json,
>>>>>>>>> "w")))
>>>>>>>>> +            av_log(ctx, AV_LOG_WARNING, "Flushing dump failed\n");
>>>>>>>> 
>>>>>>>> I would appreciate evaluation of errno and printing the appropriate
>>>>>>>> string (using av_strerror(), I believe).
>>>>>>>> 
>>>>>>>>> +    switch (inlink->format) {
>>>>>>>>> +        case AV_SAMPLE_FMT_DBLP:
>>>>>>>> 
>>>>>>>> As Kyle hinted: Can this not force a conversion (implicit insertion
>>>>>>>> of
>>>>>>>> aformat filter) to e.g. double by only supporting this format, and
>>>>>>>> reducing this switch to one or two cases? (The filter's output isn't
>>>>>>>> really meant for transparent reuse anyway? af_volumedetect e.g. also
>>>>>>>> supports only one, meaning its output can be a different format than
>>>>>>>> its input.) It's also really hard to go through and later to
>>>>>>>> maintain.
>>>>>>>> It the big switch/case remains, one or two code macros would be
>>>>>>>> appropriate.
>>>>>>> 
>>>>>>> I checked solution used in volumedetect and couldn't find a way to
>>>>>>> read across formats.
>>>>>> 
>>>>>> I do not understand what you are trying to do.
>>>>> Sorry, I'm trying to add support for 8, 16, 24, 32, 64 bit sample
>>>>> formats
>>>>>>> How would you implement such macros? Since version 3.2 astats filter
>>>>>>> uses same approach for reading different formats and as far as I know
>>>>>>> macros harder to debug
>>>>>> 
>>>>>> astats is using all formats because of numerous reasons. astats uses
>>>>>> raw
>>>>>> values,
>>>>>> your filter just convert each raw value to float representation.
>>>>> Is this wrong, as I'd like to have high precision?
>>>> 
>>>> For rendering to small size image?
>>> Data can be used for analysis as well. Any size I would say as user may
>>> define size
>> 
>> Than use floats.
> Ok, will give it a try
Here is updated patch
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: FFmpeg-devel-v2-avfilter-add-dumpwave-filter.patch
Type: application/octet-stream
Size: 62336 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180113/2f125a20/attachment.obj>
-------------- next part --------------




More information about the ffmpeg-devel mailing list