-- Best regards, Dmytro
> On 11 Jan 2018, at 23:57, Paul B Mahol <one...@gmail.com> wrote: > >> 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. Ok, will give it a try > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel