On 1/11/18, Dmitry Gumenyuk <dmitry.gumen...@gmail.com> wrote:
>
>> On 11 Jan 2018, at 23:02, Paul B Mahol <one...@gmail.com> wrote:
>>
>>> On 1/11/18, Dmitry Gumenyuk <dmitry.gumen...@gmail.com> wrote:
>>>
>>>>> On 11 Jan 2018, at 22:41, Paul B Mahol <one...@gmail.com> wrote:
>>>>>
>>>>> On 1/11/18, Dmitry Gumenyuk <dmitry.gumen...@gmail.com> wrote:
>>>>> Hello
>>>>>
>>>>> 2018-01-10 11:33 GMT+01:00 Moritz Barsnick <barsn...@gmx.net>:
>>>>>>> On Wed, Jan 10, 2018 at 08:58:05 +0100, dmitry.gumen...@gmail.com
>>>>>>> wrote:
>>>>>>>
>>>>>>> +@table @option
>>>>>>> +@item d
>>>>>>> +Dimensions @code{WxH}.
>>>>>>> +@code{W} - number of data values in json, values will be scaled
>>>>>>> according to @code{H}.
>>>>>>> +The default value is @var{640x480}
>>>>>>> +
>>>>>>> +@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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
