On Mon, Mar 31, 2014 at 4:37 PM, Katerina Barone-Adesi <[email protected]> wrote: > On Mon, Mar 31, 2014 at 6:01 AM, Timothy Gu <[email protected]> wrote: >> >> Thanks for doing this. Some comments and possible improvements: >> >> * Add support for "sr" as a variable name as in >> > http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=ba856c0be5599f21f241162e1f5f3f2506f3132c >> * Add documentation >> * Remove a couple of extra newlines >> * Change filename to f_settb.c (in a separate patch) >> * Back port rest of FFmpeg's changes to settb
Hello, thanks for your patch > Thanks, Timothy. I've added sr, documentation, ported the > AVFILTER_DEFINE_CLASS macro, and removed some newlines; I think I've > backported all the substantial changes. Please do not do this, defining macros in this way hides bugs and make the code harder to read. Please write down the full AVClass block. > Patch 0002 does the rename. Renames and other cosmetics should be done *before* applying one owns changes. This is done to have a better history tracking of what happened to any file. Also, please rename the file to settb, like the other mixed audio/video filters (no f_ here). Wrt documentation, you should not move paragraphs and add a new section for "multimedia filters" but rather leave settb as is and the asettb section in the audio filter. One final nit, when you see a group of variable intialized together, keep the = aligned + settb->var_values[VAR_SR] = inlink->sample_rate; should be + settb->var_values[VAR_SR] = inlink->sample_rate; Thanks, Vittorio > sr has somewhat odd behavior: ashowinfo's output is always pts:0 > pts_time:0.000000 in the second ashowinfo in $ avconv -i > /usr/share/doc/texlive-doc/latex/animate/files/click.mp3 -af > 'ashowinfo,asettb=sr*10,ashowinfo' /tmp/test1.wav > That said, it's the same in FFmpeg, and the resulting audio file plays > correctly. > > Kat > > _______________________________________________ > libav-devel mailing list > [email protected] > https://lists.libav.org/mailman/listinfo/libav-devel -- Vittorio _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
