from:"Tobias Rapp"

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

2018-01-15 Thread Tobias Rapp

On 13.01.2018 23:52, Дмитрий Гуменюк wrote:

Hi,

On 13 Jan 2018, at 01:37, Дмитрий Гуменюк <dmitry.gumen...@gmail.com> wrote:

On 12 Jan 2018, at 13:32, Дмитрий Гуменюк <dmitry.gumen...@gmail.com> wrote:

On 12 Jan 2018, at 13:17, Tobias Rapp <t.r...@noa-archive.com> wrote:

On 12.01.2018 12:16, Дмитрий Гуменюк wrote:

On 11 Jan 2018, at 09:20, Tobias Rapp <t.r...@noa-archive.com> wrote:

On 10.01.2018 18:18, Kyle Swanson wrote:

Hi,
For this to be a part of libavfilter the output needs to be more generic
than the just the Soundcloud format. If we want this to be generally useful
it should probably just output an array of floats between 0.0 and 1.0. The
consumer of this data (JS library, or whatever) can use this in whatever
way it wants.

I agree. If the BWF Peak Envelope output which was suggested in the other
thread does not match your demands and filter implementation is actually
necessary I would prefer if the filter would attach the RMS value(s) as frame
metadata instead of directly dumping to file. Frame metadata can then be re-

RMS values may be counted for several frames or only for a half of a frame

used by other filters or dumped into file by using the existing "ametadata"
filter.

This would be similar to:

ffmpeg -i input-file -f null -filter:a
"asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat"
/dev/null

I like this idea, but won’t asetnsamples affect performance by creating fifo
queue? And it may require some effort to parse long output

I added asetnsamples to define the audio frame size (interval of values from astats). You
can reduce the number of lines printed by ametadata by using the
"key=lavfi.astats.foo" option.

I used asetnsamples as well, and I measured performance while transcoding - it
appears to be slight slower

I think output is now more generic and I got rid of long switch/case, thanks
for support

Here is most recent patch, seems like all comments are addressed, did I miss
something?

I still would prefer to have the value attached as frame metadata, then
dumped into file via the existing "ametadata" filter. Even better would
be to integrate the statistic value (if missing) into the "astats" filter.

If your concern is the output format of "ametadata" then some output
format extension (CSV/JSON) needs to be discussed for ametadata/metadata.

If your concern is performance then please add some numbers. In my tests
using an approx. 5 minutes input WAV file (48kHz, stereo) the run with
"asetnsamples" was considerably faster than the run without (1.7s vs. 13.9s)

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

2018-01-12 Thread Tobias Rapp


On 12.01.2018 12:16, Дмитрий Гуменюк wrote:

Hi

On 11 Jan 2018, at 09:20, Tobias Rapp <t.r...@noa-archive.com> wrote:

On 10.01.2018 18:18, Kyle Swanson wrote:

Hi,
For this to be a part of libavfilter the output needs to be more generic
than the just the Soundcloud format. If we want this to be generally useful
it should probably just output an array of floats between 0.0 and 1.0. The
consumer of this data (JS library, or whatever) can use this in whatever
way it wants.


I agree. If the BWF Peak Envelope output which was suggested in the other 
thread does not match your demands and filter implementation is actually 
necessary I would prefer if the filter would attach the RMS value(s) as frame 
metadata instead of directly dumping to file. Frame metadata can then be re-

RMS values may be counted for several frames or only for a half of a frame

used by other filters or dumped into file by using the existing "ametadata" 
filter.

This would be similar to:

ffmpeg -i input-file -f null -filter:a 
"asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat" 
/dev/null

I like this idea, but won’t asetnsamples affect performance by creating fifo 
queue? And it may require some effort to parse long output


I added asetnsamples to define the audio frame size (interval of values 
from astats). You can reduce the number of lines printed by ametadata by 
using the "key=lavfi.astats.foo" option.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-01-10 Thread Tobias Rapp


On 10.01.2018 09:11, Дмитрий Гуменюк wrote:

Hi,
In my opinion JSON is more flexible, so envelope can be rendered in web app or 
mobile app
This filter is useful when you’d like to generate visual representation of 
audio while doing transcode


I see, but my point is: If you do the image rendering external anyway, 
why not use an existing envelope dumping format? Or alternatively 
connect the rendering application via the av* library interface in C?


Filters into the codebase should be as generic as possible to allow 
re-usage by other filters.





On 10 Jan 2018, at 09:00, Tobias Rapp <t.r...@noa-archive.com> wrote:

On 08.01.2018 01:36, dmitry.gumen...@gmail.com wrote:

From: Dmytro Humeniuk <dmitry.gumen...@gmail.com>
Signed-off-by: Dmytro Humeniuk <dmitry.gumen...@gmail.com>
---
  Changelog  |   1 +
  libavfilter/Makefile   |   1 +
  libavfilter/af_dumpwave.c  | 273 +
  libavfilter/allfilters.c   |   1 +
  libavfilter/version.h  |   4 +-
  tests/fate/filter-audio.mak|   5 +
  tests/ref/fate/filter-dumpwave |   1 +
  7 files changed, 284 insertions(+), 2 deletions(-)
  create mode 100644 libavfilter/af_dumpwave.c
  create mode 100644 tests/ref/fate/filter-dumpwave
[...]


As far as I can see the filter reads audio and writes an envelope curve using 
JSON data format. The JSON data is then rendered into an image outside of 
FFmpeg.

In my opinion it would be better to allow connecting the filter with other filters by 
directly rendering the image within FFmpeg. So the filter should have a generic video 
output instead of the JSON output. This would be similar to the existing 
"showvolume" filter.

If you just want to get the raw envelope data from FFmpeg I suggest to take a look at the 
"write_peak" option of the WAV muxer.

Regards,
Tobias





BTW: Top-posting is unpopular on this list.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-01-10 Thread Tobias Rapp


On 08.01.2018 01:36, dmitry.gumen...@gmail.com wrote:

From: Dmytro Humeniuk 

Signed-off-by: Dmytro Humeniuk 
---
  Changelog  |   1 +
  libavfilter/Makefile   |   1 +
  libavfilter/af_dumpwave.c  | 273 +
  libavfilter/allfilters.c   |   1 +
  libavfilter/version.h  |   4 +-
  tests/fate/filter-audio.mak|   5 +
  tests/ref/fate/filter-dumpwave |   1 +
  7 files changed, 284 insertions(+), 2 deletions(-)
  create mode 100644 libavfilter/af_dumpwave.c
  create mode 100644 tests/ref/fate/filter-dumpwave

[...]


As far as I can see the filter reads audio and writes an envelope curve 
using JSON data format. The JSON data is then rendered into an image 
outside of FFmpeg.


In my opinion it would be better to allow connecting the filter with 
other filters by directly rendering the image within FFmpeg. So the 
filter should have a generic video output instead of the JSON output. 
This would be similar to the existing "showvolume" filter.


If you just want to get the raw envelope data from FFmpeg I suggest to 
take a look at the "write_peak" option of the WAV muxer.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-31 Thread Tobias Rapp


On 30.01.2018 20:37, Michael Niedermayer wrote:

On Tue, Jan 30, 2018 at 08:27:27PM +0700, Muhammad Faiz wrote:

On Tue, Jan 30, 2018 at 7:50 PM, Michael Niedermayer
 wrote:

Its also a step away from supporting plugins.
Why plugins matter ? Because having plugin
support is a big advantage, it allows a much wider
community to work on, write and maintain filters.

With plugins, people can write filters that are
written in languages other than C. Or filters which
some developer in FFmpeg doesnt want. Or they can be
maintained externally by people who just do not like us.
Or by people who perfer a FOSS license different from
LGPL/GPL/BSD. Iam sure others can come up with more
reasons ...

Of course avfilter_register() isnt enough for plugins
but it or something equivalent is needed for plugins.

So i would prefer if avfilter_register() stays supported
indefinitly or in case a different system is written for
plugins then until that system is in place.


It just returns error and logs error message that currently external
filter is unsupported. If someone wants to implement support for
external filter, it can be easily added later using separate
list/table.


Iam not sure "easily" is true

We started out with a fully public API that allowed external filters.
and little by little its removed or made private.
each individual change may be easy to undo but as a whole moving
libavfilter back to having a public API is not that easy anymore

IIRC the arguments to make things private where that people wanted to
improve the API. But from the idea and impression of a plan like:
1. make it private
2. design and implement better API
3. make it public again

Somehow now people lost sight and interrest in 3. and even 2. is becoming
less interresting. But the problem is really without 2 + 3 actually happening
doing 1 seems like not a good idea at all.

To me it seems even mentioning external filters and public API makes some
people angry.
But really that has to be the goal at some point. To again have a public API


I agree that having (again) a public filter API would be good. This 
would give users the freedom to implement their own special-purpose 
filters (see the "dumpwave" discussion).



Is the plan to have 2 seperrate lists at that point ?
one static for internal filters and one dynamic for externally registered
ones ?

Iam not objecting to the patch, theres nothing i have that uses the call
but iam a bit concerned about interrest_to_remove > interrest_in_public_api

thanks


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavfilter: constify filter list

2018-01-31 Thread Tobias Rapp


On 31.01.2018 10:03, wm4 wrote:

On Wed, 31 Jan 2018 09:08:23 +0100
Tobias Rapp <t.r...@noa-archive.com> wrote:


On 30.01.2018 20:37, Michael Niedermayer wrote:

On Tue, Jan 30, 2018 at 08:27:27PM +0700, Muhammad Faiz wrote:

On Tue, Jan 30, 2018 at 7:50 PM, Michael Niedermayer
<mich...@niedermayer.cc> wrote:

Its also a step away from supporting plugins.
Why plugins matter ? Because having plugin
support is a big advantage, it allows a much wider
community to work on, write and maintain filters.

With plugins, people can write filters that are
written in languages other than C. Or filters which
some developer in FFmpeg doesnt want. Or they can be
maintained externally by people who just do not like us.
Or by people who perfer a FOSS license different from
LGPL/GPL/BSD. Iam sure others can come up with more
reasons ...

Of course avfilter_register() isnt enough for plugins
but it or something equivalent is needed for plugins.

So i would prefer if avfilter_register() stays supported
indefinitly or in case a different system is written for
plugins then until that system is in place.


It just returns error and logs error message that currently external
filter is unsupported. If someone wants to implement support for
external filter, it can be easily added later using separate
list/table.


Iam not sure "easily" is true

We started out with a fully public API that allowed external filters.
and little by little its removed or made private.
each individual change may be easy to undo but as a whole moving
libavfilter back to having a public API is not that easy anymore

IIRC the arguments to make things private where that people wanted to
improve the API. But from the idea and impression of a plan like:
1. make it private
2. design and implement better API
3. make it public again

Somehow now people lost sight and interrest in 3. and even 2. is becoming
less interresting. But the problem is really without 2 + 3 actually happening
doing 1 seems like not a good idea at all.

To me it seems even mentioning external filters and public API makes some
people angry.
But really that has to be the goal at some point. To again have a public API


I agree that having (again) a public filter API would be good. This
would give users the freedom to implement their own special-purpose
filters (see the "dumpwave" discussion).


Someone needs to design and write it.

Whether we have external filters is completely orthogonal to this
change. They were not possible before, because not enough API for
implementing them is public. They can be possible in the future, but
then registering them would need a different API anyway (one that
doesn't use global mutable state, but uses some sort of context
instead).


Thanks for clarifying, so don't count my comment as an disagreement on 
the patch.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] fate: add tests for pan audio filter

2018-02-15 Thread Tobias Rapp


On 15.02.2018 09:39, Nicolas George wrote:

Tobias Rapp (2018-02-15):

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  tests/fate/filter-audio.mak   | 30 ++
  tests/ref/fate/filter-pan-mono1   | 26 ++
  tests/ref/fate/filter-pan-mono2   | 26 ++
  tests/ref/fate/filter-pan-stereo1 | 26 ++
  tests/ref/fate/filter-pan-stereo2 | 26 ++
  tests/ref/fate/filter-pan-stereo3 | 26 ++
  tests/ref/fate/filter-pan-stereo4 | 26 ++
  7 files changed, 186 insertions(+)
  create mode 100644 tests/ref/fate/filter-pan-mono1
  create mode 100644 tests/ref/fate/filter-pan-mono2
  create mode 100644 tests/ref/fate/filter-pan-stereo1
  create mode 100644 tests/ref/fate/filter-pan-stereo2
  create mode 100644 tests/ref/fate/filter-pan-stereo3
  create mode 100644 tests/ref/fate/filter-pan-stereo4


I think the change in lswr is sane, but I do not know enough of that
code to be definite. Thanks for tracking down that ticket.

As for this patch, thanks for implementing it. It looks reasonable. But
I am not entirely sure lsws is bit-exact in this kind of situation.


I have run the Fate tests on Linux 32/64bit and Mips/Qemu, but possibly 
the float channel coefficients could be fragile (especially of the 
stereo3 test). Do you have some idea on how to make them more reliable? 
Or shall we give it a try and remove problematic tests when they arise?


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 2/2] fate: add tests for pan audio filter

2018-02-15 Thread Tobias Rapp

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 tests/fate/filter-audio.mak   | 30 ++
 tests/ref/fate/filter-pan-mono1   | 26 ++
 tests/ref/fate/filter-pan-mono2   | 26 ++
 tests/ref/fate/filter-pan-stereo1 | 26 ++
 tests/ref/fate/filter-pan-stereo2 | 26 ++
 tests/ref/fate/filter-pan-stereo3 | 26 ++
 tests/ref/fate/filter-pan-stereo4 | 26 ++
 7 files changed, 186 insertions(+)
 create mode 100644 tests/ref/fate/filter-pan-mono1
 create mode 100644 tests/ref/fate/filter-pan-mono2
 create mode 100644 tests/ref/fate/filter-pan-stereo1
 create mode 100644 tests/ref/fate/filter-pan-stereo2
 create mode 100644 tests/ref/fate/filter-pan-stereo3
 create mode 100644 tests/ref/fate/filter-pan-stereo4

diff --git a/tests/fate/filter-audio.mak b/tests/fate/filter-audio.mak
index bd8b3d3..2a3ba19 100644
--- a/tests/fate/filter-audio.mak
+++ b/tests/fate/filter-audio.mak
@@ -128,6 +128,36 @@ fate-filter-firequalizer: CMP = oneoff
 fate-filter-firequalizer: CMP_UNIT = s16
 fate-filter-firequalizer: SIZE_TOLERANCE = 1058400 - 1097208
 
+FATE_AFILTER-$(call FILTERDEMDECENCMUX, PAN, WAV, PCM_S16LE, PCM_S16LE, WAV) 
+= fate-filter-pan-mono1
+fate-filter-pan-mono1: tests/data/asynth-44100-2.wav
+fate-filter-pan-mono1: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
+fate-filter-pan-mono1: CMD = framecrc -ss 3.14 -i $(SRC) -frames:a 20 
-filter:a "pan=mono|FC=FL"
+
+FATE_AFILTER-$(call FILTERDEMDECENCMUX, PAN, WAV, PCM_S16LE, PCM_S16LE, WAV) 
+= fate-filter-pan-mono2
+fate-filter-pan-mono2: tests/data/asynth-44100-2.wav
+fate-filter-pan-mono2: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
+fate-filter-pan-mono2: CMD = framecrc -ss 3.14 -i $(SRC) -frames:a 20 
-filter:a "pan=1C|c0=c0+c1"
+
+FATE_AFILTER-$(call FILTERDEMDECENCMUX, PAN, WAV, PCM_S16LE, PCM_S16LE, WAV) 
+= fate-filter-pan-stereo1
+fate-filter-pan-stereo1: tests/data/asynth-44100-3.wav
+fate-filter-pan-stereo1: SRC = $(TARGET_PATH)/tests/data/asynth-44100-3.wav
+fate-filter-pan-stereo1: CMD = framecrc -ss 3.14 -i $(SRC) -frames:a 20 
-filter:a "pan=2c|FL=FR|FR=FL"
+
+FATE_AFILTER-$(call FILTERDEMDECENCMUX, PAN, WAV, PCM_S16LE, PCM_S16LE, WAV) 
+= fate-filter-pan-stereo2
+fate-filter-pan-stereo2: tests/data/asynth-44100-3.wav
+fate-filter-pan-stereo2: SRC = $(TARGET_PATH)/tests/data/asynth-44100-3.wav
+fate-filter-pan-stereo2: CMD = framecrc -ss 3.14 -i $(SRC) -frames:a 20 
-filter:a "pan=stereo|c0=c0-c2|c1=c1-c2"
+
+FATE_AFILTER-$(call FILTERDEMDECENCMUX, PAN, WAV, PCM_S16LE, PCM_S16LE, WAV) 
+= fate-filter-pan-stereo3
+fate-filter-pan-stereo3: tests/data/asynth-44100-2.wav
+fate-filter-pan-stereo3: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
+fate-filter-pan-stereo3: CMD = framecrc -ss 3.14 -i $(SRC) -frames:a 20 
-filter:a "pan=FL+FR|FL<3*c0+2*c1|FR<2*c0+3*c1"
+
+FATE_AFILTER-$(call FILTERDEMDECENCMUX, PAN, WAV, PCM_S16LE, PCM_S16LE, WAV) 
+= fate-filter-pan-stereo4
+fate-filter-pan-stereo4: tests/data/asynth-44100-2.wav
+fate-filter-pan-stereo4: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
+fate-filter-pan-stereo4: CMD = framecrc -ss 3.14 -guess_layout_max 0 -i $(SRC) 
-frames:a 20 -filter:a "pan=4C|c0=c0-0.5*c1|c1=c1+0.5*c0|c2=0*c0|c3=0*c0"
+
 FATE_AFILTER_SAMPLES-$(call FILTERDEMDECENCMUX, SILENCEREMOVE, WAV, PCM_S16LE, 
PCM_S16LE, WAV) += fate-filter-silenceremove
 fate-filter-silenceremove: SRC = 
$(TARGET_SAMPLES)/audio-reference/divertimenti_2ch_96kHz_s24.wav
 fate-filter-silenceremove: CMD = framecrc -i $(SRC) -frames:a 30 -af 
silenceremove=0:0:0:-1:0:-90dB
diff --git a/tests/ref/fate/filter-pan-mono1 b/tests/ref/fate/filter-pan-mono1
new file mode 100644
index 000..3bd7c25
--- /dev/null
+++ b/tests/ref/fate/filter-pan-mono1
@@ -0,0 +1,26 @@
+#tb 0: 1/44100
+#media_type 0: audio
+#codec_id 0: pcm_s16le
+#sample_rate 0: 44100
+#channel_layout 0: 4
+#channel_layout_name 0: mono
+0,  0,  0, 1024, 2048, 0x750f0a66
+0,   1024,   1024, 1024, 2048, 0x155cf063
+0,   2048,   2048, 1024, 2048, 0x1e43fc32
+0,   3072,   3072, 1024, 2048, 0x282ffc28
+0,   4096,   4096, 1024, 2048, 0x6d7bf000
+0,   5120,   5120, 1024, 2048, 0xc0b2f411
+0,   6144,   6144, 1024, 2048, 0xd711fb03
+0,   7168,   7168, 1024, 2048, 0x3164189c
+0,   8192,   8192, 1024, 2048, 0x8c69e827
+0,   9216,   9216, 1024, 2048, 0x562d0518
+0,  10240,  10240, 1024, 2048, 0x380aee27
+0,  11264,  11264, 1024, 2048, 0x990a03e4
+0,  12288,  12288, 1024, 2048, 0x68d7ef60
+0,  13312,  13312, 1024, 2048, 0xd13fef9e
+0,  14336,  14336, 1024, 2048, 0x009306e4
+0,  15360,  15360, 1

[FFmpeg-devel] [PATCH 1/2] swresample/rematrix: fix update of channel matrix if input or output layout is undefined

2018-02-15 Thread Tobias Rapp

Prefer direct in/out channel count values over channel layout, when
available. Fixes a pan filter bug (ticket #6790).

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 libswresample/rematrix.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libswresample/rematrix.c b/libswresample/rematrix.c
index 9fcfff1..8227730 100644
--- a/libswresample/rematrix.c
+++ b/libswresample/rematrix.c
@@ -69,8 +69,10 @@ int swr_set_matrix(struct SwrContext *s, const double 
*matrix, int stride)
 return AVERROR(EINVAL);
 memset(s->matrix, 0, sizeof(s->matrix));
 memset(s->matrix_flt, 0, sizeof(s->matrix_flt));
-nb_in  = av_get_channel_layout_nb_channels(s->user_in_ch_layout);
-nb_out = av_get_channel_layout_nb_channels(s->user_out_ch_layout);
+nb_in = (s->user_in_ch_count > 0) ? s->user_in_ch_count :
+av_get_channel_layout_nb_channels(s->user_in_ch_layout);
+nb_out = (s->user_out_ch_count > 0) ? s->user_out_ch_count :
+av_get_channel_layout_nb_channels(s->user_out_ch_layout);
 for (out = 0; out < nb_out; out++) {
 for (in = 0; in < nb_in; in++)
 s->matrix_flt[out][in] = s->matrix[out][in] = matrix[in];
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] doc/filters: add links to ffmpeg-utils and ffmpeg documentation

2018-02-22 Thread Tobias Rapp


On 21.02.2018 23:27, Lou Logan wrote:

On Tue, Feb 20, 2018, at 2:01 AM, Tobias Rapp wrote:

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  doc/filters.texi | 48 ++--
  1 file changed, 26 insertions(+), 22 deletions(-)

[...]

@@ -9322,8 +9323,9 @@ A '|'-separated list of parameters to pass to the
frei0r effect.
  A frei0r effect parameter can be a boolean (its value is either
  "y" or "n"), a double, a color (specified as
  @var{R}/@var{G}/@var{B}, where @var{R}, @var{G}, and @var{B} are
floating point
-numbers between 0.0 and 1.0, inclusive) or by a color description
specified in the "Color"
-section in the ffmpeg-utils manual), a position (specified as @var{X}/
@var{Y}, where
+numbers between 0.0 and 1.0, inclusive) or by a color description
specified in the
+@ref{color syntax,,"Color" section in the ffmpeg-utils manual,ffmpeg-
utils}),


You can remove that errant ")" if you feel like it (I know you didn't introduce 
that).

Otherwise, LGTM.


Have removed the superfluous ")" and slightly adjusted the sentence.


I have never really liked the resulting link structure as shown in the HTML 
output, but I don't hate it enough to actually do anything about it.



Pushed the patch, thanks for review.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] fate: add tests for pan audio filter

2018-02-22 Thread Tobias Rapp


On 19.02.2018 08:47, Tobias Rapp wrote:

On 15.02.2018 10:12, Nicolas George wrote:

Tobias Rapp (2018-02-15):
I have run the Fate tests on Linux 32/64bit and Mips/Qemu, but 
possibly the

float channel coefficients could be fragile (especially of the stereo3
test). Do you have some idea on how to make them more reliable? Or 
shall we

give it a try and remove problematic tests when they arise?


With that kind of varied testing, I think give it a try is a very valid
option. Thanks.


Applied the patch and will keep an eye on FATE.

BTW: Is there a way to filter the status page on fate.ffmpeg.org to 
check for specific failing tests? Or just some way to auto-expand all 
the lists of failing tests so one can use Strg+F for finding?


As a reminder for my future self: Executing the following one-line 
snippet in the Web Console of the browser opens all failing tests:


$(".fa-caret-down").each(function() { try { $(this).click() } catch 
(err) { console.log(err.name) } });


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] doc/filters: add links to ffmpeg-utils and ffmpeg documentation

2018-02-20 Thread Tobias Rapp

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 doc/filters.texi | 48 ++--
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index bd93e0a..0f0ec10 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -1611,7 +1611,7 @@ The filter accepts the syntax
 [@var{sample_rate}:]@var{resampler_options}, where @var{sample_rate}
 expresses a sample rate and @var{resampler_options} is a list of
 @var{key}=@var{value} pairs, separated by ":". See the
-@ref{Resampler Options,,the "Resampler Options" section in the
+@ref{Resampler Options,,"Resampler Options" section in the
 ffmpeg-resampler(1) manual,ffmpeg-resampler}
 for the complete list of supported options.
 
@@ -7449,7 +7449,7 @@ the input width and height. It defaults to 0.
 
 @item color, c
 Specify the color of the box to write. For the general syntax of this option,
-check the "Color" section in the ffmpeg-utils manual. If the special
+check the @ref{color syntax,,"Color" section in the ffmpeg-utils 
manual,ffmpeg-utils}. If the special
 value @code{invert} is used, the box edge color is the same as the
 video with inverted luma.
 
@@ -7552,7 +7552,7 @@ framed. Default to 0.
 
 @item color, c
 Specify the color of the grid. For the general syntax of this option,
-check the "Color" section in the ffmpeg-utils manual. If the special
+check the @ref{color syntax,,"Color" section in the ffmpeg-utils 
manual,ffmpeg-utils}. If the special
 value @code{invert} is used, the grid color is the same as the
 video with inverted luma.
 
@@ -7648,7 +7648,7 @@ The default value of @var{boxborderw} is 0.
 
 @item boxcolor
 The color to be used for drawing box around text. For the syntax of this
-option, check the "Color" section in the ffmpeg-utils manual.
+option, check the @ref{color syntax,,"Color" section in the ffmpeg-utils 
manual,ffmpeg-utils}.
 
 The default value of @var{boxcolor} is "white".
 
@@ -7662,7 +7662,7 @@ The default value of @var{borderw} is 0.
 
 @item bordercolor
 Set the color to be used for drawing border around text. For the syntax of this
-option, check the "Color" section in the ffmpeg-utils manual.
+option, check the @ref{color syntax,,"Color" section in the ffmpeg-utils 
manual,ffmpeg-utils}.
 
 The default value of @var{bordercolor} is "black".
 
@@ -7683,7 +7683,7 @@ If true, check and fix text coords to avoid clipping.
 
 @item fontcolor
 The color to be used for drawing fonts. For the syntax of this option, check
-the "Color" section in the ffmpeg-utils manual.
+the @ref{color syntax,,"Color" section in the ffmpeg-utils 
manual,ffmpeg-utils}.
 
 The default value of @var{fontcolor} is "black".
 
@@ -7746,7 +7746,8 @@ libfreetype flags.
 
 @item shadowcolor
 The color to be used for drawing a shadow behind the drawn text. For the
-syntax of this option, check the "Color" section in the ffmpeg-utils manual.
+syntax of this option, check the @ref{color syntax,,"Color" section in the
+ffmpeg-utils manual,ffmpeg-utils}.
 
 The default value of @var{shadowcolor} is "black".
 
@@ -9322,8 +9323,9 @@ A '|'-separated list of parameters to pass to the frei0r 
effect.
 A frei0r effect parameter can be a boolean (its value is either
 "y" or "n"), a double, a color (specified as
 @var{R}/@var{G}/@var{B}, where @var{R}, @var{G}, and @var{B} are floating point
-numbers between 0.0 and 1.0, inclusive) or by a color description specified in 
the "Color"
-section in the ffmpeg-utils manual), a position (specified as @var{X}/@var{Y}, 
where
+numbers between 0.0 and 1.0, inclusive) or by a color description specified in 
the
+@ref{color syntax,,"Color" section in the ffmpeg-utils manual,ffmpeg-utils}),
+a position (specified as @var{X}/@var{Y}, where
 @var{X} and @var{Y} are floating point numbers) and/or a string.
 
 The number and types of parameters depend on the loaded effect. If an
@@ -11881,7 +11883,8 @@ so the input image is centered on the padded area.
 
 @item color
 Specify the color of the padded area. For the syntax of this option,
-check the "Color" section in the ffmpeg-utils manual.
+check the @ref{color syntax,,"Color" section in the ffmpeg-utils
+manual,ffmpeg-utils}.
 
 The default value of @var{color} is "black".
 
@@ -13273,8 +13276,9 @@ it. Default value is 1.
 
 @item fillcolor, c
 Set the color used to fill the output area not covered by the rotated
-image. For the general syntax of this option, check the "Color" section in the
-ffmpeg-utils manual. If the special value "none" is selected then no
+image. For the general syntax of this option, check the
+@ref{color syntax,,"Color" section in the ffmpeg-utils manual,ffmpeg-utils}.
+If t

Re: [FFmpeg-devel] [PATCH 2/2] fate: add tests for pan audio filter

2018-02-18 Thread Tobias Rapp


On 15.02.2018 10:12, Nicolas George wrote:

Tobias Rapp (2018-02-15):

I have run the Fate tests on Linux 32/64bit and Mips/Qemu, but possibly the
float channel coefficients could be fragile (especially of the stereo3
test). Do you have some idea on how to make them more reliable? Or shall we
give it a try and remove problematic tests when they arise?


With that kind of varied testing, I think give it a try is a very valid
option. Thanks.


Applied the patch and will keep an eye on FATE.

BTW: Is there a way to filter the status page on fate.ffmpeg.org to 
check for specific failing tests? Or just some way to auto-expand all 
the lists of failing tests so one can use Strg+F for finding?


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] swresample/rematrix: fix update of channel matrix if input or output layout is undefined

2018-02-18 Thread Tobias Rapp


On 16.02.2018 21:52, Michael Niedermayer wrote:

On Thu, Feb 15, 2018 at 09:34:55AM +0100, Tobias Rapp wrote:

Prefer direct in/out channel count values over channel layout, when
available. Fixes a pan filter bug (ticket #6790).

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  libswresample/rematrix.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)


LGTM


Applied, thanks for review.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] libavfilter/vf_fps: Rewrite using activate callback

2018-02-19 Thread Tobias Rapp


On 16.02.2018 21:09, calvin.wal...@kepstin.ca wrote:

Oops, I forgot to remove this bit from the changelog:

On Fri, 2018-02-16 at 15:02 -0500, Calvin Walton wrote:

TODO: This is still a work in progress. It may have different
behaviour
in some cases from the old fps filter. I have not yet implemented the
"eof_action" option, since I haven't figured out what it's supposed
to
do.


At this point I'm fairly confident that the behaviour matches the
existing filter, and that I've gotten the eof_action behaviour correct
as well.


I have run this patch against some private test files with filter 
options "fps=1:round=near:eof_action=pass" and found no regressions. So 
eof_action seems to work fine, indeed.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 0/3] Finish new iteration APIs

2018-02-22 Thread Tobias Rapp


On 22.02.2018 16:47, Nicolas George wrote:

Hi.

Not sure where in the thread would be the best place to reply, so I
might as well reply here.

There is one idea I have been toying with for some time that relates to
codec registration. It is significantly different from the direction
this discussion was taking, but I think it offers a lot of advantages.

Let us have "struct AVCodecLibrary" with the lists of codecs, bsfs, etc.
Whenever a function needs to access the lists, have it take an
AVCodecLibrary argument. It can be made automatic most of the time by
adding an AVCodecLibrary field to all AVSomethingContext structs.

[...]

The same structure could be used to set all mutable global state, like
the log level and callback.


I like the idea of storing global context within some structure.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] fftools/ffmpeg: replace call to av_strerror with av_err2str

2018-02-25 Thread Tobias Rapp

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 fftools/ffmpeg.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 32caa4b..3a45f43 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -2175,10 +2175,7 @@ static int ifilter_send_frame(InputFilter *ifilter, 
AVFrame *frame)
 
 ret = reap_filters(1);
 if (ret < 0 && ret != AVERROR_EOF) {
-char errbuf[128];
-av_strerror(ret, errbuf, sizeof(errbuf));
-
-av_log(NULL, AV_LOG_ERROR, "Error while filtering: %s\n", errbuf);
+av_log(NULL, AV_LOG_ERROR, "Error while filtering: %s\n", 
av_err2str(ret));
 return ret;
 }
 
@@ -4638,10 +4635,7 @@ static int transcode(void)
 
 ret = transcode_step();
 if (ret < 0 && ret != AVERROR_EOF) {
-char errbuf[128];
-av_strerror(ret, errbuf, sizeof(errbuf));
-
-av_log(NULL, AV_LOG_ERROR, "Error while filtering: %s\n", errbuf);
+av_log(NULL, AV_LOG_ERROR, "Error while filtering: %s\n", 
av_err2str(ret));
 break;
 }
 
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 1/2] fftools/ffmpeg: fix progress log message in case pts is not available

2018-02-26 Thread Tobias Rapp


On 27.02.2018 01:12, Michael Niedermayer wrote:

On Mon, Feb 26, 2018 at 05:09:04PM +0100, Tobias Rapp wrote:

Move time string formatting into inline function. Also fixes out_time
sign prefix for progress report.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  fftools/ffmpeg.c | 48 +++-
  1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 32caa4b..0097a7d 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1518,6 +1518,27 @@ static int reap_filters(int flush)
  return 0;
  }
  
+static inline char *pts_to_hms_str(char *buf, int64_t pts, unsigned int digits)


char buf[AV_TS_MAX_STRING_SIZE]

or the buf size should be passed too, in fact this might be better anyway


Will change.


+{
+const char *hours_sign;
+int hours, mins;
+double secs;
+
+if (pts == AV_NOPTS_VALUE) {
+snprintf(buf, AV_TS_MAX_STRING_SIZE, "N/A");
+} else {
+hours_sign = (pts < 0) ? "-" : "";
+secs = (double)FFABS(pts) / AV_TIME_BASE;
+mins = (int)secs / 60;
+secs = secs - mins * 60;
+hours = mins / 60;
+mins %= 60;


This is not the same code, also with double it can produce inexact
results and results differing between platforms


I changed secs to double to handle the cases with different number of 
sub-second digits more easily. Would it be OK to output two digits after 
the decimal point in both cases? The progress report contains the 
precise out_time_ms value anyway.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg: replace call to av_strerror with av_err2str

2018-02-26 Thread Tobias Rapp


On 26.02.2018 08:40, Tobias Rapp wrote:

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  fftools/ffmpeg.c | 10 ++
  1 file changed, 2 insertions(+), 8 deletions(-)

[...]


Pushed. Thanks Jan and Michael for review.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/vf_hue: 10bit support

2018-08-03 Thread Tobias Rapp


On 03.08.2018 16:34, Michael Niedermayer wrote:

Signed-off-by: Michael Niedermayer 
---
  libavfilter/vf_hue.c | 103 ++-
  1 file changed, 92 insertions(+), 11 deletions(-)

[...]


Tested here successfully with 10-bit yuvtestsrc data and different hue 
filter option values for h/s/b.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 1/2] tests/audiogen: raise channel count limit to 12

2018-07-26 Thread Tobias Rapp

Signed-off-by: Tobias Rapp 
---
 tests/audiogen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/audiogen.c b/tests/audiogen.c
index 8d596b5..c43bb70 100644
--- a/tests/audiogen.c
+++ b/tests/audiogen.c
@@ -26,7 +26,7 @@
 #include 
 #include 
 
-#define MAX_CHANNELS 8
+#define MAX_CHANNELS 12
 
 static unsigned int myrnd(unsigned int *seed_ptr, int n)
 {
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 2/2] fate: add tests for audio channel up-/downmixing with pan filter

2018-07-26 Thread Tobias Rapp

Add tests for upmixing and downmixing with audio channel counts that
have a corresponding default layout and also tests where there is no
default layout.

Update the existing "stereo4" test so it actually outputs stereo like
the other stereo tests. Rename the previous "stereo4" test into
"upmix1".

Signed-off-by: Tobias Rapp 
---
 tests/fate/filter-audio.mak| 22 +++-
 tests/ref/fate/filter-pan-downmix1 | 26 ++
 tests/ref/fate/filter-pan-downmix2 | 26 ++
 tests/ref/fate/filter-pan-stereo4  | 42 +++---
 .../fate/{filter-pan-stereo4 => filter-pan-upmix1} |  0
 tests/ref/fate/filter-pan-upmix2   | 26 ++
 6 files changed, 120 insertions(+), 22 deletions(-)
 create mode 100644 tests/ref/fate/filter-pan-downmix1
 create mode 100644 tests/ref/fate/filter-pan-downmix2
 copy tests/ref/fate/{filter-pan-stereo4 => filter-pan-upmix1} (100%)
 create mode 100644 tests/ref/fate/filter-pan-upmix2

diff --git a/tests/fate/filter-audio.mak b/tests/fate/filter-audio.mak
index 6125a37..473b8ae 100644
--- a/tests/fate/filter-audio.mak
+++ b/tests/fate/filter-audio.mak
@@ -156,7 +156,27 @@ fate-filter-pan-stereo3: CMD = framecrc -ss 3.14 -i $(SRC) 
-frames:a 20 -filter:
 FATE_AFILTER-$(call FILTERDEMDECENCMUX, PAN, WAV, PCM_S16LE, PCM_S16LE, WAV) 
+= fate-filter-pan-stereo4
 fate-filter-pan-stereo4: tests/data/asynth-44100-2.wav
 fate-filter-pan-stereo4: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
-fate-filter-pan-stereo4: CMD = framecrc -ss 3.14 -guess_layout_max 0 -i $(SRC) 
-frames:a 20 -filter:a "pan=4C|c0=c0-0.5*c1|c1=c1+0.5*c0|c2=0*c0|c3=0*c0"
+fate-filter-pan-stereo4: CMD = framecrc -ss 3.14 -guess_layout_max 0 -i $(SRC) 
-frames:a 20 -filter:a "pan=2C|c0=c0-0.5*c1|c1=c1+0.5*c0"
+
+FATE_AFILTER-$(call FILTERDEMDECENCMUX, PAN, WAV, PCM_S16LE, PCM_S16LE, WAV) 
+= fate-filter-pan-upmix1
+fate-filter-pan-upmix1: tests/data/asynth-44100-2.wav
+fate-filter-pan-upmix1: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
+fate-filter-pan-upmix1: CMD = framecrc -ss 3.14 -guess_layout_max 0 -i $(SRC) 
-frames:a 20 -filter:a "pan=4C|c0=c0-0.5*c1|c1=c1+0.5*c0|c2=0*c0|c3=0*c0"
+
+FATE_AFILTER-$(call FILTERDEMDECENCMUX, PAN, WAV, PCM_S16LE, PCM_S16LE, WAV) 
+= fate-filter-pan-upmix2
+fate-filter-pan-upmix2: tests/data/asynth-44100-4.wav
+fate-filter-pan-upmix2: SRC = $(TARGET_PATH)/tests/data/asynth-44100-4.wav
+fate-filter-pan-upmix2: CMD = framecrc -ss 3.14 -i $(SRC) -frames:a 20 
-filter:a 
"pan=9C|c0=c0-c1|c1=c2+c3|c2=c0+c1|c3=c2-c3|c4=c1-c0|c5=c3+c2|c6=c1+c0|c7=c3-c2|c8=c0-c3"
+
+FATE_AFILTER-$(call FILTERDEMDECENCMUX, PAN, WAV, PCM_S16LE, PCM_S16LE, WAV) 
+= fate-filter-pan-downmix1
+fate-filter-pan-downmix1: tests/data/asynth-44100-4.wav
+fate-filter-pan-downmix1: SRC = $(TARGET_PATH)/tests/data/asynth-44100-4.wav
+fate-filter-pan-downmix1: CMD = framecrc -ss 3.14 -i $(SRC) -frames:a 20 
-filter:a "pan=2c|FLhttp://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] tests/audiogen: raise channel count limit to 12

2018-07-30 Thread Tobias Rapp


On 27.07.2018 21:48, Michael Niedermayer wrote:

On Thu, Jul 26, 2018 at 09:13:41AM +0200, Tobias Rapp wrote:

Signed-off-by: Tobias Rapp 
---
  tests/audiogen.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/audiogen.c b/tests/audiogen.c
index 8d596b5..c43bb70 100644
--- a/tests/audiogen.c
+++ b/tests/audiogen.c
@@ -26,7 +26,7 @@
  #include 
  #include 
  
-#define MAX_CHANNELS 8

+#define MAX_CHANNELS 12


LGTM


Applied, thanks for the review.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] fate: add tests for audio channel up-/downmixing with pan filter

2018-07-30 Thread Tobias Rapp


On 27.07.2018 16:10, Nicolas George wrote:

Tobias Rapp (2018-07-26):

Add tests for upmixing and downmixing with audio channel counts that
have a corresponding default layout and also tests where there is no
default layout.

Update the existing "stereo4" test so it actually outputs stereo like
the other stereo tests. Rename the previous "stereo4" test into
"upmix1".

Signed-off-by: Tobias Rapp 


Looks reasonable.


Applied, thanks for the review.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] fate: add 10-bit test for hue video filter

2018-08-28 Thread Tobias Rapp

Signed-off-by: Tobias Rapp 
---
 tests/fate/filter-video.mak | 3 +++
 tests/ref/fate/filter-hue4  | 1 +
 2 files changed, 4 insertions(+)
 create mode 100644 tests/ref/fate/filter-hue4

diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak
index edd51e1..c87a7ba 100644
--- a/tests/fate/filter-video.mak
+++ b/tests/fate/filter-video.mak
@@ -514,6 +514,9 @@ fate-filter-hue2: CMD = video_filter 
"perms=random,hue=h=18*n" -frames:v 20
 FATE_FILTER_VSYNTH-$(call ALLYES, PERMS_FILTER HUE_FILTER) += fate-filter-hue3
 fate-filter-hue3: CMD = video_filter "perms=random,hue=b=n-10" -frames:v 20
 
+FATE_FILTER_VSYNTH-$(call ALLYES, FORMAT_FILTER PERMS_FILTER HUE_FILTER) += 
fate-filter-hue4
+fate-filter-hue4: CMD = video_filter 
"format=yuv422p10,perms=random,hue=h=18*n:s=n/10" -frames:v 20
+
 FATE_FILTER_VSYNTH-$(CONFIG_IDET_FILTER) += fate-filter-idet
 fate-filter-idet: CMD = framecrc -flags bitexact -idct simple -i $(SRC) -vf 
idet -frames:v 25 -flags +bitexact
 
diff --git a/tests/ref/fate/filter-hue4 b/tests/ref/fate/filter-hue4
new file mode 100644
index 000..2a08c33
--- /dev/null
+++ b/tests/ref/fate/filter-hue4
@@ -0,0 +1 @@
+hue46279ed43527e7b5be645819e08880107
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libswresample: Use channel count if channel layout is undefined

2018-07-20 Thread Tobias Rapp


On 19.07.2018 23:37, Michael Niedermayer wrote:

On Thu, Jul 19, 2018 at 01:53:09PM +0200, Tobias Rapp wrote:

On 18.07.2018 19:31, Marcin Gorzel wrote:

Rematrixing supports up to 64 channels. However, there is only a limited number 
of channel layouts defined. Since the in/out channel count is obtained from the 
channel layout, for undefined layouts (e.g. for 9, 10, 11 channels etc.) the 
rematrixing fails.

In ticket #6790 the problem has been partially addressed by applying a patch to 
swr_set_matrix() in rematrix.c:72.
However, a similar change must be also applied to swri_rematrix_init() in 
rematrix.c:389 and swri_rematrix_init_x86() in rematrix_init.c:36.
---
  libswresample/rematrix.c  | 6 --
  libswresample/x86/rematrix_init.c | 6 --
  2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/libswresample/rematrix.c b/libswresample/rematrix.c
index 8227730056..d1a0cfe7f8 100644
--- a/libswresample/rematrix.c
+++ b/libswresample/rematrix.c
@@ -384,8 +384,10 @@ av_cold static int auto_matrix(SwrContext *s)
  av_cold int swri_rematrix_init(SwrContext *s){
  int i, j;
-int nb_in  = av_get_channel_layout_nb_channels(s->in_ch_layout);
-int nb_out = av_get_channel_layout_nb_channels(s->out_ch_layout);
+int nb_in = (s->in.ch_count > 0) ? s->in.ch_count :
+av_get_channel_layout_nb_channels(s->in_ch_layout);
+int nb_out = (s->out.ch_count > 0) ? s->out.ch_count :
+av_get_channel_layout_nb_channels(s->out_ch_layout);
  s->mix_any_f = NULL;
diff --git a/libswresample/x86/rematrix_init.c 
b/libswresample/x86/rematrix_init.c
index d71b41a73e..4f9c92e4ab 100644
--- a/libswresample/x86/rematrix_init.c
+++ b/libswresample/x86/rematrix_init.c
@@ -33,8 +33,10 @@ D(int16, sse2)
  av_cold int swri_rematrix_init_x86(struct SwrContext *s){
  #if HAVE_X86ASM
  int mm_flags = av_get_cpu_flags();
-int nb_in  = av_get_channel_layout_nb_channels(s->in_ch_layout);
-int nb_out = av_get_channel_layout_nb_channels(s->out_ch_layout);
+int nb_in = (s->in.ch_count > 0) ? s->in.ch_count :
+av_get_channel_layout_nb_channels(s->in_ch_layout);
+int nb_out = (s->out.ch_count > 0) ? s->out.ch_count :
+av_get_channel_layout_nb_channels(s->out_ch_layout);
  int num= nb_in * nb_out;
  int i,j;



Patch looks good to me, except that the title should be updated to reflect
the new logic. I suggest something like "swresample: Use channel count for
rematrix initialization if defined".


The patch does not look good to me
There should be a field that represents the count of channels.
No conditional should be needed

please check that every field is wrong in at least some case.
If true a new field must be added or a existing one needs to be set
differently
But there should be a field that represents the channel count.


If I understand correctly you say that the fall-back to 
av_get_channel_layout_nb_channels() is not needed here as both functions 
are internal and called only as part of swr_init() so we may safely 
assume that the in/out.ch_count fields have been initialized before this 
code is reached.


Fine with me. Marcin, would you update the patch once more? And there 
are some additional FATE tests for the pan filter that I can post once 
this patch is through, if you haven't made up your own tests.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libswresample: Use channel count if channel layout is undefined

2018-07-18 Thread Tobias Rapp


On 13.07.2018 13:43, Marcin Gorzel wrote:

Rematrixing supports up to 64 channels. However, there is only a limited number 
of channel layouts defined. Since the in/out channel count is obtained from the 
channel layout, for undefined layouts (e.g. for 9, 10, 11 channels etc.) the 
rematrixing fails.

In ticket #6790 the problem has been partially addressed by applying a patch to 
swr_set_matrix() in rematrix.c:72.
However, a similar change must be also applied to swri_rematrix_init() in 
rematrix.c:389 and swri_rematrix_init_x86() in rematrix_init.c:36.

This patch adds the following check to the swri_rematrix_init() in 
rematrix.c:389 and swri_rematrix_init_x86() in rematrix_init.c:36: if channel 
layout is non-zero, obtain channel count from the channel layout, otherwise, 
use channel count instead.

It also modifies the checks in swr_set_matrix() in rematrix.c:72 to match the 
above checks. (Since av_get_channel_layout_nb_channels(s->user_in_ch_layout) 
was originally used, there may be preference to rely on the channel layout first 
(if available) before falling back to the user channel count).
---
  libswresample/rematrix.c  | 18 --
  libswresample/x86/rematrix_init.c |  8 ++--
  2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/libswresample/rematrix.c b/libswresample/rematrix.c
index 8227730056..8c9fbf3804 100644
--- a/libswresample/rematrix.c
+++ b/libswresample/rematrix.c
@@ -69,10 +69,12 @@ int swr_set_matrix(struct SwrContext *s, const double 
*matrix, int stride)
  return AVERROR(EINVAL);
  memset(s->matrix, 0, sizeof(s->matrix));
  memset(s->matrix_flt, 0, sizeof(s->matrix_flt));
-nb_in = (s->user_in_ch_count > 0) ? s->user_in_ch_count :
-av_get_channel_layout_nb_channels(s->user_in_ch_layout);
-nb_out = (s->user_out_ch_count > 0) ? s->user_out_ch_count :
-av_get_channel_layout_nb_channels(s->user_out_ch_layout);
+nb_in = s->user_in_ch_layout != 0
+? av_get_channel_layout_nb_channels(s->user_in_ch_layout)
+: s->user_in_ch_count;
+nb_out = s->user_out_ch_layout != 0
+? av_get_channel_layout_nb_channels(s->user_out_ch_layout)
+: s->user_out_ch_count;
  for (out = 0; out < nb_out; out++) {
  for (in = 0; in < nb_in; in++)
  s->matrix_flt[out][in] = s->matrix[out][in] = matrix[in];


Sorry for jumping into the discussion late but I don't think this change 
is necessary. If only one of the user_*_ch_count / user_*_ch_layout 
field pair is set, the outcome will be the same. If both field values 
are set they must be consistent or undefined behavior will occur. So if 
we assume the field values are consistent, why use the value that has to 
be calculated first from the layout mask instead of the explicit count 
value?



@@ -384,8 +386,12 @@ av_cold static int auto_matrix(SwrContext *s)
  
  av_cold int swri_rematrix_init(SwrContext *s){

  int i, j;
-int nb_in  = av_get_channel_layout_nb_channels(s->in_ch_layout);
-int nb_out = av_get_channel_layout_nb_channels(s->out_ch_layout);
+int nb_in  = s->in_ch_layout != 0
+? av_get_channel_layout_nb_channels(s->in_ch_layout)
+: s->user_in_ch_count;
+int nb_out = s->out_ch_layout != 0
+? av_get_channel_layout_nb_channels(s->out_ch_layout)
+: s->user_out_ch_count;
  
  s->mix_any_f = NULL;
  
diff --git a/libswresample/x86/rematrix_init.c b/libswresample/x86/rematrix_init.c

index d71b41a73e..a6ae074926 100644
--- a/libswresample/x86/rematrix_init.c
+++ b/libswresample/x86/rematrix_init.c
@@ -33,8 +33,12 @@ D(int16, sse2)
  av_cold int swri_rematrix_init_x86(struct SwrContext *s){
  #if HAVE_X86ASM
  int mm_flags = av_get_cpu_flags();
-int nb_in  = av_get_channel_layout_nb_channels(s->in_ch_layout);
-int nb_out = av_get_channel_layout_nb_channels(s->out_ch_layout);
+int nb_in  = s->in_ch_layout != 0
+? av_get_channel_layout_nb_channels(s->in_ch_layout)
+: s->user_in_ch_count;
+int nb_out = s->out_ch_layout != 0
+? av_get_channel_layout_nb_channels(s->out_ch_layout)
+: s->user_out_ch_count;
  int num= nb_in * nb_out;
  int i,j;
  



Like said above I think the same effect can be achieved with less CPU 
cycles by using a "(count > 0) ? count : 
av_get_channel_layout_nb_channels(layout)" code pattern.


Also not sure what is the difference between the "in_ch_layout" field 
used here and the "user_in_ch_layout" field used in function swr_set_matrix.


Minor nit: In my personal opinion putting parentheses around the 
condition part of the ternary operator would improve readability.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] swresample: Use channel count in rematrix initialization

2018-07-23 Thread Tobias Rapp


On 21.07.2018 20:31, Marcin Gorzel wrote:

Rematrixing supports up to 64 channels. However, there is only a limited number 
of channel layouts defined. Since the in/out channel count is currently 
obtained from the channel layout, for undefined layouts (e.g. for 9, 10, 11 
channels etc.) the rematrixing fails.

This patch changes rematrix init methods to use in/out channel count directly 
instead of computing it from channel layout.
---
  libswresample/rematrix.c  | 4 ++--
  libswresample/x86/rematrix_init.c | 4 ++--
  2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libswresample/rematrix.c b/libswresample/rematrix.c
index 8227730056..ec1909dc0c 100644
--- a/libswresample/rematrix.c
+++ b/libswresample/rematrix.c
@@ -384,8 +384,8 @@ av_cold static int auto_matrix(SwrContext *s)
  
  av_cold int swri_rematrix_init(SwrContext *s){

  int i, j;
-int nb_in  = av_get_channel_layout_nb_channels(s->in_ch_layout);
-int nb_out = av_get_channel_layout_nb_channels(s->out_ch_layout);
+int nb_in = s->in.ch_count;
+int nb_out = s->out.ch_count;
  
  s->mix_any_f = NULL;
  
diff --git a/libswresample/x86/rematrix_init.c b/libswresample/x86/rematrix_init.c

index d71b41a73e..1cdf97803f 100644
--- a/libswresample/x86/rematrix_init.c
+++ b/libswresample/x86/rematrix_init.c
@@ -33,8 +33,8 @@ D(int16, sse2)
  av_cold int swri_rematrix_init_x86(struct SwrContext *s){
  #if HAVE_X86ASM
  int mm_flags = av_get_cpu_flags();
-int nb_in  = av_get_channel_layout_nb_channels(s->in_ch_layout);
-int nb_out = av_get_channel_layout_nb_channels(s->out_ch_layout);
+int nb_in = s->in.ch_count;
+int nb_out = s->out.ch_count;
  int num= nb_in * nb_out;
  int i,j;
  



Patch looks good to me. Will leave it to Michael to comment/apply the patch.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libswresample: Use channel count if channel layout is undefined

2018-07-19 Thread Tobias Rapp


On 18.07.2018 19:31, Marcin Gorzel wrote:

Rematrixing supports up to 64 channels. However, there is only a limited number 
of channel layouts defined. Since the in/out channel count is obtained from the 
channel layout, for undefined layouts (e.g. for 9, 10, 11 channels etc.) the 
rematrixing fails.

In ticket #6790 the problem has been partially addressed by applying a patch to 
swr_set_matrix() in rematrix.c:72.
However, a similar change must be also applied to swri_rematrix_init() in 
rematrix.c:389 and swri_rematrix_init_x86() in rematrix_init.c:36.
---
  libswresample/rematrix.c  | 6 --
  libswresample/x86/rematrix_init.c | 6 --
  2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/libswresample/rematrix.c b/libswresample/rematrix.c
index 8227730056..d1a0cfe7f8 100644
--- a/libswresample/rematrix.c
+++ b/libswresample/rematrix.c
@@ -384,8 +384,10 @@ av_cold static int auto_matrix(SwrContext *s)
  
  av_cold int swri_rematrix_init(SwrContext *s){

  int i, j;
-int nb_in  = av_get_channel_layout_nb_channels(s->in_ch_layout);
-int nb_out = av_get_channel_layout_nb_channels(s->out_ch_layout);
+int nb_in = (s->in.ch_count > 0) ? s->in.ch_count :
+av_get_channel_layout_nb_channels(s->in_ch_layout);
+int nb_out = (s->out.ch_count > 0) ? s->out.ch_count :
+av_get_channel_layout_nb_channels(s->out_ch_layout);
  
  s->mix_any_f = NULL;
  
diff --git a/libswresample/x86/rematrix_init.c b/libswresample/x86/rematrix_init.c

index d71b41a73e..4f9c92e4ab 100644
--- a/libswresample/x86/rematrix_init.c
+++ b/libswresample/x86/rematrix_init.c
@@ -33,8 +33,10 @@ D(int16, sse2)
  av_cold int swri_rematrix_init_x86(struct SwrContext *s){
  #if HAVE_X86ASM
  int mm_flags = av_get_cpu_flags();
-int nb_in  = av_get_channel_layout_nb_channels(s->in_ch_layout);
-int nb_out = av_get_channel_layout_nb_channels(s->out_ch_layout);
+int nb_in = (s->in.ch_count > 0) ? s->in.ch_count :
+av_get_channel_layout_nb_channels(s->in_ch_layout);
+int nb_out = (s->out.ch_count > 0) ? s->out.ch_count :
+av_get_channel_layout_nb_channels(s->out_ch_layout);
  int num= nb_in * nb_out;
  int i,j;
  



Patch looks good to me, except that the title should be updated to 
reflect the new logic. I suggest something like "swresample: Use channel 
count for rematrix initialization if defined".


If nobody objects within the next days I will amend the title, push the 
patch to master and backport the fix to release branches.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] fate: add 10-bit test for hue video filter

2018-08-31 Thread Tobias Rapp


On 31.08.2018 12:43, Michael Niedermayer wrote:

On Thu, Aug 30, 2018 at 10:27:23AM +0200, Tobias Rapp wrote:

Signed-off-by: Tobias Rapp 
---
  tests/fate/filter-video.mak | 3 +++
  tests/ref/fate/filter-hue4  | 1 +
  2 files changed, 4 insertions(+)
  create mode 100644 tests/ref/fate/filter-hue4


LGTM


Pushed, thanks for the review.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v2] fate: add 10-bit test for hue video filter

2018-08-30 Thread Tobias Rapp

Signed-off-by: Tobias Rapp 
---
 tests/fate/filter-video.mak | 3 +++
 tests/ref/fate/filter-hue4  | 1 +
 2 files changed, 4 insertions(+)
 create mode 100644 tests/ref/fate/filter-hue4

diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak
index edd51e1..8bbdc04 100644
--- a/tests/fate/filter-video.mak
+++ b/tests/fate/filter-video.mak
@@ -514,6 +514,9 @@ fate-filter-hue2: CMD = video_filter 
"perms=random,hue=h=18*n" -frames:v 20
 FATE_FILTER_VSYNTH-$(call ALLYES, PERMS_FILTER HUE_FILTER) += fate-filter-hue3
 fate-filter-hue3: CMD = video_filter "perms=random,hue=b=n-10" -frames:v 20
 
+FATE_FILTER_VSYNTH-$(call ALLYES, FORMAT_FILTER PERMS_FILTER HUE_FILTER) += 
fate-filter-hue4
+fate-filter-hue4: CMD = video_filter 
"format=yuv422p10,perms=random,hue=h=18*n:s=n/10" -frames:v 20 -pix_fmt 
yuv422p10le
+
 FATE_FILTER_VSYNTH-$(CONFIG_IDET_FILTER) += fate-filter-idet
 fate-filter-idet: CMD = framecrc -flags bitexact -idct simple -i $(SRC) -vf 
idet -frames:v 25 -flags +bitexact
 
diff --git a/tests/ref/fate/filter-hue4 b/tests/ref/fate/filter-hue4
new file mode 100644
index 000..2a08c33
--- /dev/null
+++ b/tests/ref/fate/filter-hue4
@@ -0,0 +1 @@
+hue46279ed43527e7b5be645819e08880107
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] New build system

2018-07-06 Thread Tobias Rapp


On 04.07.2018 16:36, Mathieu Duponchelle wrote:

[...]

If there are any other questions related to meson or the FFmpeg meson port
please feel free to ask and I'll do my best to answer them.


What are the dependencies besides Meson and Python v3 itself? Is using 
Ninja mandatory for building or can Make be used as an alternative?


What happens to build targets when using Meson? Are granular things like 
"make all", "make doc", "make fate", "make fate-filter-pixfmts" possible?


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

2018-01-22 Thread Tobias Rapp


On 20.01.2018 20:17, Dmytro Humeniuk wrote:



On 18 Jan 2018, at 17:32, Dmytro Humeniuk <dmitry.gumen...@gmail.com> wrote:



On 18 Jan 2018, at 08:56, Tobias Rapp <t.r...@noa-archive.com> wrote:

On 15.01.2018 13:48, Dmytro Humeniuk wrote:

On 15 Jan 2018, at 09:14, Tobias Rapp <t.r...@noa-archive.com> wrote:

On 13.01.2018 23:52, Дмитрий Гуменюк wrote:

Hi,

On 13 Jan 2018, at 01:37, Дмитрий Гуменюк <dmitry.gumen...@gmail.com> wrote:

Hi


On 12 Jan 2018, at 13:32, Дмитрий Гуменюк <dmitry.gumen...@gmail.com> wrote:


On 12 Jan 2018, at 13:17, Tobias Rapp <t.r...@noa-archive.com> wrote:

On 12.01.2018 12:16, Дмитрий Гуменюк wrote:

Hi

On 11 Jan 2018, at 09:20, Tobias Rapp <t.r...@noa-archive.com> wrote:

On 10.01.2018 18:18, Kyle Swanson wrote:

Hi,
For this to be a part of libavfilter the output needs to be more generic
than the just the Soundcloud format. If we want this to be generally useful
it should probably just output an array of floats between 0.0 and 1.0. The
consumer of this data (JS library, or whatever) can use this in whatever
way it wants.


I agree. If the BWF Peak Envelope output which was suggested in the other 
thread does not match your demands and filter implementation is actually 
necessary I would prefer if the filter would attach the RMS value(s) as frame 
metadata instead of directly dumping to file. Frame metadata can then be re-

RMS values may be counted for several frames or only for a half of a frame

used by other filters or dumped into file by using the existing "ametadata" 
filter.

This would be similar to:

ffmpeg -i input-file -f null -filter:a 
"asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat" 
/dev/null

I like this idea, but won’t asetnsamples affect performance by creating fifo 
queue? And it may require some effort to parse long output


I added asetnsamples to define the audio frame size (interval of values from astats). You 
can reduce the number of lines printed by ametadata by using the 
"key=lavfi.astats.foo" option.

I used asetnsamples as well, and I measured performance while transcoding - it 
appears to be slight slower

I think output is now more generic and I got rid of long switch/case, thanks 
for support

Here is most recent patch, seems like all comments are addressed, did I miss 
something?


I still would prefer to have the value attached as frame metadata, then dumped into file via the 
existing "ametadata" filter. Even better would be to integrate the statistic value (if 
missing) into the "astats" filter.

If your concern is the output format of "ametadata" then some output format 
extension (CSV/JSON) needs to be discussed for ametadata/metadata.

If your concern is performance then please add some numbers. In my tests using an approx. 
5 minutes input WAV file (48kHz, stereo) the run with "asetnsamples" was 
considerably faster than the run without (1.7s vs. 13.9s)

Hi
As I mentioned previously adding metadata to each frame is not possible
as value may be counted for several frames or only for a half of a frame
I used 2 hours long 48kHz mp3 
https://s3-eu-west-1.amazonaws.com/balamii/SynthSystemSystersJAN2018.mp3
For this purposes I set up CentOS AWS EC2 nano instance
Then I transcoded it while filtering like following (just to recreate real 
situation):
1. -filter:a 
"asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat" 
out.mp3
2. -filter:a "dumpwave=n=192197:f=-" out.mp3
Results:
1. 244810550046 nanoseconds
2. 87494286740 nanoseconds
One of the possible use cases - to set up 2 chains of asetnsamples->metadata - 
for example:
"asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat,asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file1.dat”
 for sure it will affect performance
Comparing with "dumpwave=n=192197:f=out1,dumpwave=n= 22050:f=out2"


Sorry, I misunderstood your concerns regarding asetnsamples filter performance. 
The numbers I provided have been for

"asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat"

versus

"astats=metadata=on,ametadata=print:file=stats-file.dat"

When comparing astats+ametadata versus dumpwave it is obvious that a 
specialized filter which only calculates one statistic value is faster than a 
filter that calculates multiple statistics. But still my opinion is that if the 
dumpwave filter is to be added to the codebase it should be more generic (i.e. 
output frame metadata similar to the psnr/ssim filters for video).


Actually current output(normalised float values in range 0...1) was proposed by 
Kyle as more generic.

Ping


What I wrote is my personal opinion. I acknowledge that you have put 
good efforts in implementing the patch and even added FATE tests -- so 
my words must sound disappointing to you. Rest

Re: [FFmpeg-devel] [PATCH] avutil/parseutils: only accept full us duration, do not accept mss duration

2018-03-08 Thread Tobias Rapp


On 08.03.2018 00:14, Hendrik Leppkes wrote:

On Thu, Mar 8, 2018 at 12:05 AM, Aurelien Jacobs  wrote:

On Wed, Mar 07, 2018 at 11:45:03PM +0100, Marton Balint wrote:



On Wed, 7 Mar 2018, Aurelien Jacobs wrote:


On Tue, Mar 06, 2018 at 01:02:48AM +0100, Marton Balint wrote:

Accepting 'u' suffix for a time specification is neither intuitive nor
consistent (now that we don't accept m).


The 'm' SI prefix is still accepted in various time options, and the 'u'
prefix is still accepted in those options even after your patch, so you
can't really argue that this patch improve consistency.
(eg. -black_min_duration 5ms is still accepted).
So this will surprise nobody that I don't like this patch.


This really is a cursed topic, I am not sure I follow, after the patch:

5ms is accepted
5us is accepted
5m is not accepted
5u is not accepted


That is only for AV_OPT_TYPE_DURATION.
All other numeric options type still accept SI prefix without unit.
This include various time options such as black_min_duration.
So after the patch, for black_min_duration:

5m is accepted
5u is accepted



Because those use AV_OPT_TYPE_DOUBLE, a generic type without any
possiblity of a unit.
Ideally those should all be transitioned to AV_OPT_TYPE_DURATION if
they refer to a time for consistency, and not the actual time-type
bend to reflect some generic type. Why would we have the duration type
if its just a copy of the double type anyway?


As a user with technical background I find it logical that 
AV_OPT_TYPE_DURATION is a superset of AV_OPT_TYPE_DOUBLE. Any 
double-formatted string is treated as a value in seconds. Additionally 
AV_OPT_TYPE_DURATION accepts HH:MM:SS formatted strings.


So I think Aurelien has a point here: Why should we enforce a unit 
suffix for AV_OPT_TYPE_DURATION and loose the superset property?


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 1/2] fftools/cmdutils: add logflags option

2018-03-14 Thread Tobias Rapp

Allows to set the AV_LOG_PRINT_LEVEL and AV_LOG_SKIP_REPEATED flags
using a distinct command-line option, similar to other flag options.
Previously only the AV_LOG_SKIP_REPEATED flag was supported as a prefix
to the "loglevel" option value.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 doc/fftools-common-opts.texi | 13 +
 fftools/cmdutils.c   | 31 +++
 fftools/cmdutils.h   |  6 ++
 3 files changed, 50 insertions(+)

diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi
index 185ec21..dd69741 100644
--- a/doc/fftools-common-opts.texi
+++ b/doc/fftools-common-opts.texi
@@ -209,6 +209,19 @@ the environment variable @env{AV_LOG_FORCE_COLOR}.
 The use of the environment variable @env{NO_COLOR} is deprecated and
 will be dropped in a future FFmpeg version.
 
+@item -logflags flags (@emph{global})
+Allows to set or clear logging flags.
+
+Possible flags for this option are:
+@table @option
+@item repeat
+Repeated log output will not be compressed to the first line and the "Last
+message repeated n times" line will be omitted.
+@item level
+Add a @code{[level]} prefix string to each log message. Can be used as an
+alternative to log coloring e.g. when dumping the log to file.
+@end table
+
 @item -report
 Dump full command line and console output to a file named
 @code{@var{program}-@var{MMDD}-@var{HHMMSS}.log} in the current
diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index 0c7d13c..11fe69a 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -514,6 +514,9 @@ void parse_loglevel(int argc, char **argv, const OptionDef 
*options)
 idx = locate_option(argc, argv, options, "v");
 if (idx && argv[idx + 1])
 opt_loglevel(NULL, "loglevel", argv[idx + 1]);
+idx = locate_option(argc, argv, options, "logflags");
+if (idx && argv[idx + 1])
+opt_logflags(NULL, "logflags", argv[idx + 1]);
 idx = locate_option(argc, argv, options, "report");
 if ((env = getenv("FFREPORT")) || idx) {
 init_report(env);
@@ -918,6 +921,34 @@ int opt_loglevel(void *optctx, const char *opt, const char 
*arg)
 return 0;
 }
 
+int opt_logflags(void *optctx, const char *opt, const char *arg)
+{
+static const AVOption logflags_opts[] = {
+{ "flags" , NULL, 0, AV_OPT_TYPE_FLAGS, { .i64 = 0 }, INT64_MIN, 
INT64_MAX, .unit = "flags" },
+/* implement AV_LOG_SKIP_REPEATED using inverse logic for consistency 
with the -loglevel option */
+{ "repeat", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AV_LOG_SKIP_REPEATED 
},.unit = "flags" },
+{ "level" , NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AV_LOG_PRINT_LEVEL   
},.unit = "flags" },
+{ NULL },
+};
+static const AVClass class = {
+.class_name = "logflags",
+.item_name  = av_default_item_name,
+.option = logflags_opts,
+.version= LIBAVUTIL_VERSION_INT,
+};
+const AVClass *pclass = 
+int flags = av_log_get_flags();
+int ret;
+
+flags ^= AV_LOG_SKIP_REPEATED;
+if ((ret = av_opt_eval_flags(, _opts[0], arg, )) < 0)
+return ret;
+flags ^= AV_LOG_SKIP_REPEATED;
+
+av_log_set_flags(flags);
+return 0;
+}
+
 static void expand_filename_template(AVBPrint *bp, const char *template,
  struct tm *tm)
 {
diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
index 8724489..28735b2 100644
--- a/fftools/cmdutils.h
+++ b/fftools/cmdutils.h
@@ -99,6 +99,11 @@ int opt_default(void *optctx, const char *opt, const char 
*arg);
  */
 int opt_loglevel(void *optctx, const char *opt, const char *arg);
 
+/**
+ * Update the log flags of libav* libraries.
+ */
+int opt_logflags(void *optctx, const char *opt, const char *arg);
+
 int opt_report(const char *opt);
 
 int opt_max_alloc(void *optctx, const char *opt, const char *arg);
@@ -236,6 +241,7 @@ void show_help_options(const OptionDef *options, const char 
*msg, int req_flags,
 { "colors",  OPT_EXIT, { .func_arg = show_colors },  
"show available color names" },\
 { "loglevel",HAS_ARG,  { .func_arg = opt_loglevel }, 
"set logging level", "loglevel" }, \
 { "v",   HAS_ARG,  { .func_arg = opt_loglevel }, 
"set logging level", "loglevel" }, \
+{ "logflags",HAS_ARG,  { .func_arg = opt_logflags }, 
"set logging flags", "flags" },\
 { "report",  0,{ (void*)opt_report },
"generate a report" }, \
 { "max_alloc",   HAS_ARG,  { .func_arg = opt_m

[FFmpeg-devel] [PATCH 2/2] avutil/log: print level prefix also when no AVClass context is available

2018-03-14 Thread Tobias Rapp

Adds the level prefix to all log messages, except those with level <=
AV_LOG_QUIET as they seem to be used for flushing the log buffer.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 libavutil/log.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavutil/log.c b/libavutil/log.c
index bd47f2a..9b7d484 100644
--- a/libavutil/log.c
+++ b/libavutil/log.c
@@ -266,11 +266,11 @@ static void format_line(void *avcl, int level, const char 
*fmt, va_list vl,
 av_bprintf(part+1, "[%s @ %p] ",
  avc->item_name(avcl), avcl);
 if(type) type[1] = get_category(avcl);
-
-if (flags & AV_LOG_PRINT_LEVEL)
-av_bprintf(part+2, "[%s] ", get_level_str(level));
 }
 
+if (*print_prefix && (level > AV_LOG_QUIET) && (flags & 
AV_LOG_PRINT_LEVEL))
+av_bprintf(part+2, "[%s] ", get_level_str(level));
+
 av_vbprintf(part+3, fmt, vl);
 
 if(*part[0].str || *part[1].str || *part[2].str || *part[3].str) {
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] avutil/log: print level prefix also when no AVClass context is available

2018-03-16 Thread Tobias Rapp


On 16.03.2018 00:03, Michael Niedermayer wrote:

On Wed, Mar 14, 2018 at 09:55:23AM +0100, Tobias Rapp wrote:

Adds the level prefix to all log messages, except those with level <=
AV_LOG_QUIET as they seem to be used for flushing the log buffer.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  libavutil/log.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)


LGTM

thx


Pushed, thanks for review.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3 2/3] avutil/log: add av_log_set_opts function

2018-04-03 Thread Tobias Rapp


On 03.04.2018 10:25, Paul B Mahol wrote:

On 4/3/18, Tobias Rapp <t.r...@noa-archive.com> wrote:

On 29.03.2018 20:38, Michael Niedermayer wrote:

On Wed, Mar 28, 2018 at 05:03:39PM +0200, Tobias Rapp wrote:

Allows to set log level and flag values from string.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
   doc/APIchanges  |  3 +++
   libavutil/log.c | 76
+
   libavutil/log.h | 16 +++
   libavutil/version.h |  2 +-
   4 files changed, 96 insertions(+), 1 deletion(-)


iam not intending to override anyone blocking this but as i looked at the
code
and it LGTM


Thanks for review. Moved code into fftools/cmdutils.c and pushed to
master. Will send a separate patch to ML for the missing documentation
update.


This was blocked, please revert ASAP!


As far as I understood the responses from wm4 and Michael it was blocked 
to be added in libavutil/log.c but accepted to be added in 
fftools/cmdutils.c (as in version 2 of the patch it was only a 
suggestion to move the code to libavutil).


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH][RFC] avcodec/dpxenc: add option to force color transfer characteristics

2018-04-11 Thread Tobias Rapp


On 11.04.2018 10:23, Kieran O Leary wrote:

Hi Carl,

On Sat, Dec 16, 2017 at 2:31 PM, Carl Eugen Hoyos <ceffm...@gmail.com> wrote:

2017-12-15 22:22 GMT+01:00 Tobias Rapp <t.r...@noa-archive.com>:


+{ "dpx_color_trc",  "Transfer Characteristics", OFFSET(color_trc), AV_OPT_TYPE_INT, 
{ .i64 = DPX_TRC_UNDEFINED }, DPX_TRC_UNDEFINED, DPX_TRC_NB-1, VE, "trc" },


This seems wrong to me, we have colour characteristics in general code.



There is a method in this patch that takes values from -color_trc, is
that not sufficient?

[...]


If I understand it correctly Carl wants to have the DPX_TRC_* enum 
values merged into AVCOL_TRC_*. My feeling is that I currently don't 
have enough knowledge about all those TRC specifications to properly 
sort them into the list, and did not find time to dig into the topic. 
Also I doubt the general usefulness of DPX_TRC_USER_DEFINED or 
DPX_TRC_UNSPECIFIED_VIDEO outside of DPX encoding.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v3] doc: update loglevel option documentation

2018-04-11 Thread Tobias Rapp

Updates documentation after the changes to loglevel flag prefix parsing
in 4b736bc921ed96ad6d312ce0cbe0de29b9e3fe81.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 doc/fftools-common-opts.texi | 37 +
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi
index 185ec21..7787e11 100644
--- a/doc/fftools-common-opts.texi
+++ b/doc/fftools-common-opts.texi
@@ -168,14 +168,24 @@ The returned list cannot be assumed to be always complete.
 ffmpeg -sinks pulse,server=192.168.0.4
 @end example
 
-@item -loglevel [repeat+]@var{loglevel} | -v [repeat+]@var{loglevel}
-Set the logging level used by the library.
-Adding "repeat+" indicates that repeated log output should not be compressed
-to the first line and the "Last message repeated n times" line will be
-omitted. "repeat" can also be used alone.
-If "repeat" is used alone, and with no prior loglevel set, the default
-loglevel will be used. If multiple loglevel parameters are given, using
-'repeat' will not change the loglevel.
+@item -loglevel [@var{flags}+]@var{loglevel} | -v [@var{flags}+]@var{loglevel}
+Set logging level and flags used by the library.
+
+The optional @var{flags} prefix can consist of the following values:
+@table @samp
+@item repeat
+Indicates that repeated log output should not be compressed to the first line
+and the "Last message repeated n times" line will be omitted.
+@item level
+Indicates that log output should add a @code{[level]} prefix to each message
+line. This can be used as an alternative to log coloring, e.g. when dumping the
+log to file.
+@end table
+Flags can also be used alone by adding a '+'/'-' prefix to set/reset a single
+flag without affecting other @var{flags} or changing @var{loglevel}. When
+setting both @var{flags} and @var{loglevel}, a '+' separator is expected
+between the last @var{flags} value and before @var{loglevel}.
+
 @var{loglevel} is a string or a number containing one of the following values:
 @table @samp
 @item quiet, -8
@@ -201,6 +211,17 @@ Show everything, including debugging information.
 @item trace, 56
 @end table
 
+For example to enable repeated log output, add the @code{level} prefix, and set
+@var{loglevel} to @code{verbose}:
+@example
+ffmpeg -loglevel repeat+level+verbose -i input output
+@end example
+Another example that enables repeated log output without affecting current
+state of @code{level} prefix flag or @var{loglevel}:
+@example
+ffmpeg [...] -loglevel +repeat
+@end example
+
 By default the program logs to stderr. If coloring is supported by the
 terminal, colors are used to mark errors and warnings. Log coloring
 can be disabled setting the environment variable
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] doc: update loglevel option documentation

2018-04-11 Thread Tobias Rapp


On 10.04.2018 22:26, Lou Logan wrote:

On Mon, Apr 9, 2018, at 10:59 PM, Tobias Rapp wrote:


+For example to enable repeated log output and set @var{loglevel} to
+@code{verbose}:
+@example
+ffmpeg -loglevel repeat+verbose -i input output
+@end example


Just to show all current flags and the loglevel I prefer:

For example to enable repeated log output, add the @code{level} prefix, and set 
@var{loglevel} to
@code{verbose}:
@example
ffmpeg -loglevel repeat+level+verbose -i input output
@end example


OK, will change.


+Another example that disables the @code{level} prefix without affecting the
+current state of @code{repeat} flag or @var{loglevel}:
+@example
+ffmpeg [...] -loglevel -level
+@end example


I'm not sure how useful this example is. Sure, it displays how to use "-", but is there 
an actual use case for using "-loglevel -level" since the level prefix is not shown by 
default anyway?


Yeah I added this to show that incrementally updating the loglevel 
option and the "-" also works. Can change it to "+" if that is preferred.


BTW: Is there some documentation on the generic flags syntax used in 
FFmpeg for options like "-fflags" or "-cpuflags" and how to combine the 
tokens with "+" or "-"? I couldn't find something on a quick scan.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v2] doc: update loglevel option documentation

2018-04-10 Thread Tobias Rapp

Updates documentation after the changes to loglevel flag prefix parsing
in 4b736bc921ed96ad6d312ce0cbe0de29b9e3fe81.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 doc/fftools-common-opts.texi | 37 +
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi
index 185ec21..aa1bb63 100644
--- a/doc/fftools-common-opts.texi
+++ b/doc/fftools-common-opts.texi
@@ -168,14 +168,24 @@ The returned list cannot be assumed to be always complete.
 ffmpeg -sinks pulse,server=192.168.0.4
 @end example
 
-@item -loglevel [repeat+]@var{loglevel} | -v [repeat+]@var{loglevel}
-Set the logging level used by the library.
-Adding "repeat+" indicates that repeated log output should not be compressed
-to the first line and the "Last message repeated n times" line will be
-omitted. "repeat" can also be used alone.
-If "repeat" is used alone, and with no prior loglevel set, the default
-loglevel will be used. If multiple loglevel parameters are given, using
-'repeat' will not change the loglevel.
+@item -loglevel [@var{flags}+]@var{loglevel} | -v [@var{flags}+]@var{loglevel}
+Set logging level and flags used by the library.
+
+The optional @var{flags} prefix can consist of the following values:
+@table @samp
+@item repeat
+Indicates that repeated log output should not be compressed to the first line
+and the "Last message repeated n times" line will be omitted.
+@item level
+Indicates that log output should add a @code{[level]} prefix to each message
+line. This can be used as an alternative to log coloring, e.g. when dumping the
+log to file.
+@end table
+Flags can also be used alone by adding a '+'/'-' prefix to set/reset a single
+flag without affecting other @var{flags} or changing @var{loglevel}. When
+setting both @var{flags} and @var{loglevel}, a '+' separator is expected
+between the last @var{flags} value and before @var{loglevel}.
+
 @var{loglevel} is a string or a number containing one of the following values:
 @table @samp
 @item quiet, -8
@@ -201,6 +211,17 @@ Show everything, including debugging information.
 @item trace, 56
 @end table
 
+For example to enable repeated log output and set @var{loglevel} to
+@code{verbose}:
+@example
+ffmpeg -loglevel repeat+verbose -i input output
+@end example
+Another example that disables the @code{level} prefix without affecting the
+current state of @code{repeat} flag or @var{loglevel}:
+@example
+ffmpeg [...] -loglevel -level
+@end example
+
 By default the program logs to stderr. If coloring is supported by the
 terminal, colors are used to mark errors and warnings. Log coloring
 can be disabled setting the environment variable
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] doc: update loglevel option documentation

2018-04-12 Thread Tobias Rapp


On 12.04.2018 02:01, Lou Logan wrote:

On Tue, Apr 10, 2018, at 11:11 PM, Tobias Rapp wrote:


BTW: Is there some documentation on the generic flags syntax used in
FFmpeg for options like "-fflags" or "-cpuflags" and how to combine the
tokens with "+" or "-"? I couldn't find something on a quick scan.


Doesn't appear to be documented, but maybe I missed it.

On Tue, Apr 10, 2018, at 11:31 PM, Tobias Rapp wrote:

Updates documentation after the changes to loglevel flag prefix parsing
in 4b736bc921ed96ad6d312ce0cbe0de29b9e3fe81.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  doc/fftools-common-opts.texi | 37 +
  1 file changed, 29 insertions(+), 8 deletions(-)


LGTM, thanks.


Pushed, thanks for the review.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/1] [RFC] ffprobe: report DAR even if SAR is undefined

2018-04-17 Thread Tobias Rapp


On 17.04.2018 07:32, Timo Teras wrote:


[...]

Do note that calculating the reduced DAR requires some mathematics
which may not be simple to do from e.g. shell or simple scripting
languages. This is the primary reason why I was hoping ffprobe to do
this for me.

Would it be acceptable to print the reduced "raw" height:width ratio as
surface_aspect_ratio (or with some other better name)?

The application interpreting the info can then use that instead of
display_aspect_ratio when it's N/A.


Depending on the application use-case it might be sufficient to use a 
float DAR, i.e. when comparing to a list of values like 4:3, 16:9, etc 
(within some "epsilon" delta, of course).


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3 2/3] avutil/log: add av_log_set_opts function

2018-03-29 Thread Tobias Rapp


On 28.03.2018 17:11, wm4 wrote:

On Wed, 28 Mar 2018 17:03:39 +0200
Tobias Rapp <t.r...@noa-archive.com> wrote:


Allows to set log level and flag values from string.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  doc/APIchanges  |  3 +++
  libavutil/log.c | 76 +
  libavutil/log.h | 16 +++
  libavutil/version.h |  2 +-
  4 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 83c7a40..2d14452 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21
  
  API changes, most recent first:
  
+2018-03-xx - xxx - lavu 56.13.100 - log.h

+  Add av_log_set_opts().
+
  2018-03-xx - xxx - lavc 58.16.100 - avcodec.h
Add FF_SUB_CHARENC_MODE_IGNORE.
  
diff --git a/libavutil/log.c b/libavutil/log.c

index 0a99d01..af32cd6 100644
--- a/libavutil/log.c
+++ b/libavutil/log.c
@@ -34,6 +34,7 @@
  #endif
  #include 
  #include 
+#include "avassert.h"
  #include "avutil.h"
  #include "bprint.h"
  #include "common.h"
@@ -402,6 +403,81 @@ void av_log_set_callback(void (*callback)(void*, int, 
const char*, va_list))
  av_log_callback = callback;
  }
  
+int av_log_set_opts(const char *arg)

+{
+const struct { const char *name; int level; } log_levels[] = {
+{ "quiet"  , AV_LOG_QUIET   },
+{ "panic"  , AV_LOG_PANIC   },
+{ "fatal"  , AV_LOG_FATAL   },
+{ "error"  , AV_LOG_ERROR   },
+{ "warning", AV_LOG_WARNING },
+{ "info"   , AV_LOG_INFO},
+{ "verbose", AV_LOG_VERBOSE },
+{ "debug"  , AV_LOG_DEBUG   },
+{ "trace"  , AV_LOG_TRACE   },
+};
+const char *token;
+char *tail;
+int flags = av_log_get_flags();
+int level = av_log_get_level();
+int cmd, i = 0;
+
+av_assert0(arg);
+while (*arg) {
+token = arg;
+if (*token == '+' || *token == '-') {
+cmd = *token++;
+} else {
+cmd = 0;
+}
+if (!i && !cmd) {
+flags = 0;  /* missing relative prefix, build absolute value */
+}
+if (!strncmp(token, "repeat", 6)) {
+if (cmd == '-') {
+flags |= AV_LOG_SKIP_REPEATED;
+} else {
+flags &= ~AV_LOG_SKIP_REPEATED;
+}
+arg = token + 6;
+} else if (!strncmp(token, "level", 5)) {
+if (cmd == '-') {
+flags &= ~AV_LOG_PRINT_LEVEL;
+} else {
+flags |= AV_LOG_PRINT_LEVEL;
+}
+arg = token + 5;
+} else {
+break;
+}
+i++;
+}
+if (!*arg) {
+goto end;
+} else if (*arg == '+') {
+arg++;
+} else if (!i) {
+flags = av_log_get_flags();  /* level value without prefix, reset 
flags */
+}
+
+for (i = 0; i < FF_ARRAY_ELEMS(log_levels); i++) {
+if (!strcmp(arg, log_levels[i].name)) {
+level = log_levels[i].level;
+goto end;
+}
+}
+
+level = strtol(arg, , 10);
+if (*tail) {
+return -1;
+}
+
+end:
+av_log_set_flags(flags);
+av_log_set_level(level);
+return 0;
+}
+
  static void missing_feature_sample(int sample, void *avc, const char *msg,
 va_list argument_list)
  {
diff --git a/libavutil/log.h b/libavutil/log.h
index d9554e6..97010f7 100644
--- a/libavutil/log.h
+++ b/libavutil/log.h
@@ -356,6 +356,22 @@ void av_log_set_flags(int arg);
  int av_log_get_flags(void);
  
  /**

+ * Set log flags and level as an option string. Accepts "repeat" and "level"
+ * flags mapped to AV_LOG_SKIP_REPEATED (inverted) and AV_LOG_PRINT_LEVEL,
+ * followed by the log level specified either by name ("warning", "info",
+ * "verbose", etc.) or by number.
+ *
+ * When flags are prefixed with "+" or "-" the change is relative to the
+ * current flags value. When both flags and level are present a "+" separator
+ * is expected between last flag and before level.
+ *
+ * @param  arg  log option string
+ * @return Returns a negative value if parsing the option string failed,
+ * otherwise returns 0.
+ */
+int av_log_set_opts(const char *arg);
+
+/**
   * @}
   */
  
diff --git a/libavutil/version.h b/libavutil/version.h

index d3dd2df..296c24b 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
   */
  
  #define LIBAVUTIL_VERSION_MAJOR  56

-#define LIBAVUTIL_VERSION_MINOR  12
+#define LIBAVUTIL_VERSION_MINOR  13
  #define LIBAVUTIL_VERSION_MICRO 100
  
  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \


Seems like a step backwards. Why can't

Re: [FFmpeg-devel] [PATCH v3 2/3] avutil/log: add av_log_set_opts function

2018-03-29 Thread Tobias Rapp


On 29.03.2018 12:58, wm4 wrote:

On Thu, 29 Mar 2018 08:58:12 +0200
Tobias Rapp <t.r...@noa-archive.com> wrote:


On 28.03.2018 17:11, wm4 wrote:

On Wed, 28 Mar 2018 17:03:39 +0200
Tobias Rapp <t.r...@noa-archive.com> wrote:
   

Allows to set log level and flag values from string.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
   doc/APIchanges  |  3 +++
   libavutil/log.c | 76 
+
   libavutil/log.h | 16 +++
   libavutil/version.h |  2 +-
   4 files changed, 96 insertions(+), 1 deletion(-)

[...]


Seems like a step backwards. Why can't it stay in the fftools thing?


When v2 of the patch was reviewed in
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-March/227077.html it was
suggested to move the code into libavutil so that other applications can
make use of it. I agree that it can be useful for command-line apps that
interface with libav* to provide a loglevel option which accepts
info/verbose/etc. name strings without the need to do an own
string-to-level parsing.


That seems completely unnecessary. Applications will have their own
conventions and option parsers.


In case the function is placed into fftools applications are forced to 
do their own thing, when it's available inside avutil applications can 
either use it _or_ implement an own parser. Thus I see no benefit in not 
having it inside avutil.


Anyway: my goal still is to add support for setting AV_LOG_PRINT_LEVEL 
via some (ffmpeg) command-line option. If you're blocking addition to 
avutil and Michael accepts updating opt_loglevel() in fftools then 
that's also OK for me.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3 1/3] avutil/log: rename and initialize global log flags variable

2018-04-03 Thread Tobias Rapp


On 29.03.2018 20:40, Michael Niedermayer wrote:

On Wed, Mar 28, 2018 at 05:03:38PM +0200, Tobias Rapp wrote:

Rename global variable for symmetry with av_log_level.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  libavutil/log.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavutil/log.c b/libavutil/log.c
index 9b7d484..0a99d01 100644
--- a/libavutil/log.c
+++ b/libavutil/log.c
@@ -52,7 +52,7 @@ static AVMutex mutex = AV_MUTEX_INITIALIZER;
  #endif
  
  static int av_log_level = AV_LOG_INFO;

-static int flags;
+static int av_log_flags = 0;


This prefix is confusing as the av prefix is intended for
exported public symbols which a static symbol cannot be


Stumbled over the generic name of this global variable when working on 
the av_log_set_opts function. As that patch is dropped I consider this 
patch as irrelevant.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3 2/3] avutil/log: add av_log_set_opts function

2018-04-03 Thread Tobias Rapp


On 29.03.2018 20:38, Michael Niedermayer wrote:

On Wed, Mar 28, 2018 at 05:03:39PM +0200, Tobias Rapp wrote:

Allows to set log level and flag values from string.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  doc/APIchanges  |  3 +++
  libavutil/log.c | 76 +
  libavutil/log.h | 16 +++
  libavutil/version.h |  2 +-
  4 files changed, 96 insertions(+), 1 deletion(-)


iam not intending to override anyone blocking this but as i looked at the code
and it LGTM


Thanks for review. Moved code into fftools/cmdutils.c and pushed to 
master. Will send a separate patch to ML for the missing documentation 
update.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] doc: update loglevel option documentation

2018-04-03 Thread Tobias Rapp

Updates documentation after the changes to loglevel flag prefix parsing
in 4b736bc921ed96ad6d312ce0cbe0de29b9e3fe81.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 doc/fftools-common-opts.texi | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi
index 185ec21..d4ee19d 100644
--- a/doc/fftools-common-opts.texi
+++ b/doc/fftools-common-opts.texi
@@ -168,14 +168,24 @@ The returned list cannot be assumed to be always complete.
 ffmpeg -sinks pulse,server=192.168.0.4
 @end example
 
-@item -loglevel [repeat+]@var{loglevel} | -v [repeat+]@var{loglevel}
-Set the logging level used by the library.
-Adding "repeat+" indicates that repeated log output should not be compressed
-to the first line and the "Last message repeated n times" line will be
-omitted. "repeat" can also be used alone.
-If "repeat" is used alone, and with no prior loglevel set, the default
-loglevel will be used. If multiple loglevel parameters are given, using
-'repeat' will not change the loglevel.
+@item -loglevel [@var{flags}+]@var{loglevel} | -v [@var{flags}+]@var{loglevel}
+Set logging level and flags used by the library.
+
+The optional @var{flags} prefix can consist of the following values:
+@table @samp
+@item repeat
+Indicates that repeated log output should not be compressed to the first line
+and the "Last message repeated n times" line will be omitted.
+@item level
+Indicates that log output should add a @code{[level]} prefix to each message
+line. This can be used as an alternative to log coloring, e.g. when dumping the
+log to file.
+@end table
+Flags can also be used alone by adding a '+'/'-' prefix to set/reset a
+single flag without affecting other flags or changing the log level. When
+setting both, flags and level, a '+' separator is expected between last
+flag and before level.
+
 @var{loglevel} is a string or a number containing one of the following values:
 @table @samp
 @item quiet, -8
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v3 1/3] avutil/log: rename and initialize global log flags variable

2018-03-28 Thread Tobias Rapp

Rename global variable for symmetry with av_log_level.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 libavutil/log.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavutil/log.c b/libavutil/log.c
index 9b7d484..0a99d01 100644
--- a/libavutil/log.c
+++ b/libavutil/log.c
@@ -52,7 +52,7 @@ static AVMutex mutex = AV_MUTEX_INITIALIZER;
 #endif
 
 static int av_log_level = AV_LOG_INFO;
-static int flags;
+static int av_log_flags = 0;
 
 #define NB_LEVELS 8
 #if defined(_WIN32) && HAVE_SETCONSOLETEXTATTRIBUTE
@@ -268,7 +268,7 @@ static void format_line(void *avcl, int level, const char 
*fmt, va_list vl,
 if(type) type[1] = get_category(avcl);
 }
 
-if (*print_prefix && (level > AV_LOG_QUIET) && (flags & 
AV_LOG_PRINT_LEVEL))
+if (*print_prefix && (level > AV_LOG_QUIET) && (av_log_flags & 
AV_LOG_PRINT_LEVEL))
 av_bprintf(part+2, "[%s] ", get_level_str(level));
 
 av_vbprintf(part+3, fmt, vl);
@@ -325,7 +325,7 @@ void av_log_default_callback(void* ptr, int level, const 
char* fmt, va_list vl)
 is_atty = isatty(2) ? 1 : -1;
 #endif
 
-if (print_prefix && (flags & AV_LOG_SKIP_REPEATED) && !strcmp(line, prev) 
&&
+if (print_prefix && (av_log_flags & AV_LOG_SKIP_REPEATED) && !strcmp(line, 
prev) &&
 *line && line[strlen(line) - 1] != '\r'){
 count++;
 if (is_atty == 1)
@@ -389,12 +389,12 @@ void av_log_set_level(int level)
 
 void av_log_set_flags(int arg)
 {
-flags = arg;
+av_log_flags = arg;
 }
 
 int av_log_get_flags(void)
 {
-return flags;
+return av_log_flags;
 }
 
 void av_log_set_callback(void (*callback)(void*, int, const char*, va_list))
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v3 3/3] fftools/cmdutils: replace loglevel option parsing with av_log_set_opts

2018-03-28 Thread Tobias Rapp

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 fftools/cmdutils.c | 48 +++-
 1 file changed, 7 insertions(+), 41 deletions(-)

diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index c0ddf0b..2a0a995 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -870,51 +870,17 @@ int opt_cpuflags(void *optctx, const char *opt, const 
char *arg)
 
 int opt_loglevel(void *optctx, const char *opt, const char *arg)
 {
-const struct { const char *name; int level; } log_levels[] = {
-{ "quiet"  , AV_LOG_QUIET   },
-{ "panic"  , AV_LOG_PANIC   },
-{ "fatal"  , AV_LOG_FATAL   },
-{ "error"  , AV_LOG_ERROR   },
-{ "warning", AV_LOG_WARNING },
-{ "info"   , AV_LOG_INFO},
-{ "verbose", AV_LOG_VERBOSE },
-{ "debug"  , AV_LOG_DEBUG   },
-{ "trace"  , AV_LOG_TRACE   },
-};
-char *tail;
-int level;
-int flags;
-int i;
-
-flags = av_log_get_flags();
-tail = strstr(arg, "repeat");
-if (tail)
-flags &= ~AV_LOG_SKIP_REPEATED;
-else
-flags |= AV_LOG_SKIP_REPEATED;
-
-av_log_set_flags(flags);
-if (tail == arg)
-arg += 6 + (arg[6]=='+');
-if(tail && !*arg)
-return 0;
-
-for (i = 0; i < FF_ARRAY_ELEMS(log_levels); i++) {
-if (!strcmp(log_levels[i].name, arg)) {
-av_log_set_level(log_levels[i].level);
-return 0;
-}
-}
+int ret;
 
-level = strtol(arg, , 10);
-if (*tail) {
+ret = av_log_set_opts(arg);
+if (ret < 0) {
 av_log(NULL, AV_LOG_FATAL, "Invalid loglevel \"%s\". "
-   "Possible levels are numbers or:\n", arg);
-for (i = 0; i < FF_ARRAY_ELEMS(log_levels); i++)
-av_log(NULL, AV_LOG_FATAL, "\"%s\"\n", log_levels[i].name);
+   "Possible levels are numbers or:\n"
+   "\"quiet\", \"panic\", \"fatal\", \"error\", \"warning\", "
+   "\"info\", \"verbose\", \"debug\", \"trace\"\n"
+   "optionally prefixed by \"repeat\" or \"level\" flags\n", arg);
 exit_program(1);
 }
-av_log_set_level(level);
 return 0;
 }
 
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v3 2/3] avutil/log: add av_log_set_opts function

2018-03-28 Thread Tobias Rapp

Allows to set log level and flag values from string.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 doc/APIchanges  |  3 +++
 libavutil/log.c | 76 +
 libavutil/log.h | 16 +++
 libavutil/version.h |  2 +-
 4 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 83c7a40..2d14452 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21
 
 API changes, most recent first:
 
+2018-03-xx - xxx - lavu 56.13.100 - log.h
+  Add av_log_set_opts().
+
 2018-03-xx - xxx - lavc 58.16.100 - avcodec.h
   Add FF_SUB_CHARENC_MODE_IGNORE.
 
diff --git a/libavutil/log.c b/libavutil/log.c
index 0a99d01..af32cd6 100644
--- a/libavutil/log.c
+++ b/libavutil/log.c
@@ -34,6 +34,7 @@
 #endif
 #include 
 #include 
+#include "avassert.h"
 #include "avutil.h"
 #include "bprint.h"
 #include "common.h"
@@ -402,6 +403,81 @@ void av_log_set_callback(void (*callback)(void*, int, 
const char*, va_list))
 av_log_callback = callback;
 }
 
+int av_log_set_opts(const char *arg)
+{
+const struct { const char *name; int level; } log_levels[] = {
+{ "quiet"  , AV_LOG_QUIET   },
+{ "panic"  , AV_LOG_PANIC   },
+{ "fatal"  , AV_LOG_FATAL   },
+{ "error"  , AV_LOG_ERROR   },
+{ "warning", AV_LOG_WARNING },
+{ "info"   , AV_LOG_INFO},
+{ "verbose", AV_LOG_VERBOSE },
+{ "debug"  , AV_LOG_DEBUG   },
+{ "trace"  , AV_LOG_TRACE   },
+};
+const char *token;
+char *tail;
+int flags = av_log_get_flags();
+int level = av_log_get_level();
+int cmd, i = 0;
+
+av_assert0(arg);
+while (*arg) {
+token = arg;
+if (*token == '+' || *token == '-') {
+cmd = *token++;
+} else {
+cmd = 0;
+}
+if (!i && !cmd) {
+flags = 0;  /* missing relative prefix, build absolute value */
+}
+if (!strncmp(token, "repeat", 6)) {
+if (cmd == '-') {
+flags |= AV_LOG_SKIP_REPEATED;
+} else {
+flags &= ~AV_LOG_SKIP_REPEATED;
+}
+arg = token + 6;
+} else if (!strncmp(token, "level", 5)) {
+if (cmd == '-') {
+flags &= ~AV_LOG_PRINT_LEVEL;
+} else {
+flags |= AV_LOG_PRINT_LEVEL;
+}
+arg = token + 5;
+} else {
+break;
+}
+i++;
+}
+if (!*arg) {
+goto end;
+} else if (*arg == '+') {
+arg++;
+} else if (!i) {
+flags = av_log_get_flags();  /* level value without prefix, reset 
flags */
+}
+
+for (i = 0; i < FF_ARRAY_ELEMS(log_levels); i++) {
+if (!strcmp(arg, log_levels[i].name)) {
+level = log_levels[i].level;
+goto end;
+}
+}
+
+level = strtol(arg, , 10);
+if (*tail) {
+return -1;
+}
+
+end:
+av_log_set_flags(flags);
+av_log_set_level(level);
+return 0;
+}
+
 static void missing_feature_sample(int sample, void *avc, const char *msg,
va_list argument_list)
 {
diff --git a/libavutil/log.h b/libavutil/log.h
index d9554e6..97010f7 100644
--- a/libavutil/log.h
+++ b/libavutil/log.h
@@ -356,6 +356,22 @@ void av_log_set_flags(int arg);
 int av_log_get_flags(void);
 
 /**
+ * Set log flags and level as an option string. Accepts "repeat" and "level"
+ * flags mapped to AV_LOG_SKIP_REPEATED (inverted) and AV_LOG_PRINT_LEVEL,
+ * followed by the log level specified either by name ("warning", "info",
+ * "verbose", etc.) or by number.
+ *
+ * When flags are prefixed with "+" or "-" the change is relative to the
+ * current flags value. When both flags and level are present a "+" separator
+ * is expected between last flag and before level.
+ *
+ * @param  arg  log option string
+ * @return Returns a negative value if parsing the option string failed,
+ * otherwise returns 0.
+ */
+int av_log_set_opts(const char *arg);
+
+/**
  * @}
  */
 
diff --git a/libavutil/version.h b/libavutil/version.h
index d3dd2df..296c24b 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  56
-#define LIBAVUTIL_VERSION_MINOR  12
+#define LIBAVUTIL_VERSION_MINOR  13
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] fftools/cmdutils: add logflags option

2018-03-20 Thread Tobias Rapp


On 20.03.2018 02:44, Michael Niedermayer wrote:

On Mon, Mar 19, 2018 at 09:31:44AM +0100, Tobias Rapp wrote:

On 14.03.2018 09:55, Tobias Rapp wrote:

Allows to set the AV_LOG_PRINT_LEVEL and AV_LOG_SKIP_REPEATED flags
using a distinct command-line option, similar to other flag options.
Previously only the AV_LOG_SKIP_REPEATED flag was supported as a prefix
to the "loglevel" option value.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  doc/fftools-common-opts.texi | 13 +
  fftools/cmdutils.c   | 31 +++
  fftools/cmdutils.h   |  6 ++
  3 files changed, 50 insertions(+)

[...]


Any opinions? My motivation was adding support for AV_LOG_PRINT_LEVEL on the
CLI. Using the existing flags option string parsing functions seemed easier
and more consistent that extending the custom loglevel string parser.


this means the feature would require every user app to add similar code
if one wants to support it.

it would be better if apps do not need changes for added options


Not sure I understand. Do you mean user apps that link to libav* 
libraries and want to mimic the command-line options of 
ffmpeg/ffprobe/ffplay? What alternative would you suggest for managing 
AV_LOG_PRINT_LEVEL via command-line options?


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] OpenCV filter should be built as C++, and C builds fail since OpenCV 3.4.1

2018-03-20 Thread Tobias Rapp


On 20.03.2018 16:02, Nicolas George wrote:

Derek Buitenhuis (2018-03-19):

If you want to link with a C++ library that is indeed the 'proper' solution.
As far as I am aware "several languages all have that requirement" is simply
a theoretical scenario that doesn't actually exist. What else beside C++
requires this?


No, it is not a theoretical scenario: most languages that produce native
code have the requirement to produce the final binary using their
toolchain. It is true for OCaml: you need to use ocamlopt to produce a
binary, even if using a third-party C library; I believe it is true for
Rust too.


Not really. You can compile parts of a program in Rust (as a static 
library) and link it to other parts in C producing the final binary with 
e.g gcc.



The problem happens when you want to mix several of these languages,
because the requirements are incompatible.

Fortunately, most people who program in other languages than C are not
stupid enough to use them to produce libraries, except for use with that
language itself. Except c++ people, for some reason.


If the libraries don't depend on some run-time environment and provide a 
C-compatible ABI, mixing languages (like C, Delphi, Rust) is not a problem.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] fftools/cmdutils: add support for level flag in loglevel option parser

2018-03-22 Thread Tobias Rapp


On 22.03.2018 00:59, Michael Niedermayer wrote:

On Wed, Mar 21, 2018 at 03:20:30PM +0100, Tobias Rapp wrote:

Allows to manage the AV_LOG_PRINT_LEVEL flag as a prefix to the loglevel
option value, similar to the existing AV_LOG_SKIP_REPEATED flag.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  doc/fftools-common-opts.texi | 11 +++
  fftools/cmdutils.c   | 26 +++---
  2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi
index 185ec21..9b6bc44 100644
--- a/doc/fftools-common-opts.texi
+++ b/doc/fftools-common-opts.texi
@@ -168,14 +168,17 @@ The returned list cannot be assumed to be always complete.
  ffmpeg -sinks pulse,server=192.168.0.4
  @end example
  
-@item -loglevel [repeat+]@var{loglevel} | -v [repeat+]@var{loglevel}

+@item -loglevel [repeat+][level+]@var{loglevel} | -v 
[repeat+][level+]@var{loglevel}
  Set the logging level used by the library.
  Adding "repeat+" indicates that repeated log output should not be compressed
  to the first line and the "Last message repeated n times" line will be
  omitted. "repeat" can also be used alone.
-If "repeat" is used alone, and with no prior loglevel set, the default
-loglevel will be used. If multiple loglevel parameters are given, using
-'repeat' will not change the loglevel.
+Adding "level+" indicates that log output should add a @code{[level]} prefix to
+each message line. This can be used as an alternative to log coloring, e.g. 
when
+dumping the log to file.
+If "repeat" and/or "level" is used alone, and with no prior loglevel set, the
+default loglevel will be used. If multiple loglevel parameters are given, using
+'repeat'/'level' will not change the loglevel.
  @var{loglevel} is a string or a number containing one of the following values:
  @table @samp
  @item quiet, -8
diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index 708a849..51fa88c 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -888,16 +888,28 @@ int opt_loglevel(void *optctx, const char *opt, const 
char *arg)
  
  flags = av_log_get_flags();

  tail = strstr(arg, "repeat");
-if (tail)
+if (tail == arg) {
  flags &= ~AV_LOG_SKIP_REPEATED;
-else
+arg += 6 + (arg[6] == '+');
+if (!*arg) {
+av_log_set_flags(flags);
+return 0;
+}
+} else {
  flags |= AV_LOG_SKIP_REPEATED;
-
+}
+tail = strstr(arg, "level");
+if (tail == arg) {
+flags |= AV_LOG_PRINT_LEVEL;
+arg += 5 + (arg[5] == '+');
+if (!*arg) {
+av_log_set_flags(flags);
+return 0;
+}
+} else {
+flags &= ~AV_LOG_PRINT_LEVEL;
+}
  av_log_set_flags(flags);
-if (tail == arg)
-arg += 6 + (arg[6]=='+');
-if(tail && !*arg)
-return 0;


might be simpler to use av_strtok()

also this code should idealy be moved into libavutil so other applications
do not need to duplicate it


A useful helper function would allow to update one flag without 
affecting the other. Implementation would end up similar to parsing 
"normal" option flags with the level number handling on-top.


Do you have some suggestion on how to do this with least code 
duplication? Maybe checking the right-hand-side of the string for 
matching a level name or being a number string and then passing the rest 
to av_opt_eval_flags?


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] fftools/cmdutils: add logflags option

2018-03-21 Thread Tobias Rapp


On 21.03.2018 00:04, Michael Niedermayer wrote:

On Tue, Mar 20, 2018 at 09:36:51AM +0100, Tobias Rapp wrote:

On 20.03.2018 02:44, Michael Niedermayer wrote:

On Mon, Mar 19, 2018 at 09:31:44AM +0100, Tobias Rapp wrote:

On 14.03.2018 09:55, Tobias Rapp wrote:

Allows to set the AV_LOG_PRINT_LEVEL and AV_LOG_SKIP_REPEATED flags
using a distinct command-line option, similar to other flag options.
Previously only the AV_LOG_SKIP_REPEATED flag was supported as a prefix
to the "loglevel" option value.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  doc/fftools-common-opts.texi | 13 +
  fftools/cmdutils.c   | 31 +++
  fftools/cmdutils.h   |  6 ++
  3 files changed, 50 insertions(+)

[...]


Any opinions? My motivation was adding support for AV_LOG_PRINT_LEVEL on the
CLI. Using the existing flags option string parsing functions seemed easier
and more consistent that extending the custom loglevel string parser.


this means the feature would require every user app to add similar code
if one wants to support it.

it would be better if apps do not need changes for added options


Not sure I understand. Do you mean user apps that link to libav* libraries
and want to mimic the command-line options of ffmpeg/ffprobe/ffplay?


no, i mean that a user app doesnt want to support each option by specific code
most user apps just want things to work without changes

imagine each option for each codec would need a change to each user app
... unpractical ...



What
alternative would you suggest for managing AV_LOG_PRINT_LEVEL via
command-line options?


whatever parses the log level could also do all options
or am i missing something ?


No existing option is changed or removed, just some new option is added. 
So user apps (like scripts that call ffmpeg binaries, I assume) wouldn't 
need changes.


But it seems a solution that merges into the existing "loglevel" option 
is preferred, so will work on that instead.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v2] fftools/cmdutils: add support for level flag in loglevel option parser

2018-03-21 Thread Tobias Rapp

Allows to manage the AV_LOG_PRINT_LEVEL flag as a prefix to the loglevel
option value, similar to the existing AV_LOG_SKIP_REPEATED flag.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 doc/fftools-common-opts.texi | 11 +++
 fftools/cmdutils.c   | 26 +++---
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi
index 185ec21..9b6bc44 100644
--- a/doc/fftools-common-opts.texi
+++ b/doc/fftools-common-opts.texi
@@ -168,14 +168,17 @@ The returned list cannot be assumed to be always complete.
 ffmpeg -sinks pulse,server=192.168.0.4
 @end example
 
-@item -loglevel [repeat+]@var{loglevel} | -v [repeat+]@var{loglevel}
+@item -loglevel [repeat+][level+]@var{loglevel} | -v 
[repeat+][level+]@var{loglevel}
 Set the logging level used by the library.
 Adding "repeat+" indicates that repeated log output should not be compressed
 to the first line and the "Last message repeated n times" line will be
 omitted. "repeat" can also be used alone.
-If "repeat" is used alone, and with no prior loglevel set, the default
-loglevel will be used. If multiple loglevel parameters are given, using
-'repeat' will not change the loglevel.
+Adding "level+" indicates that log output should add a @code{[level]} prefix to
+each message line. This can be used as an alternative to log coloring, e.g. 
when
+dumping the log to file.
+If "repeat" and/or "level" is used alone, and with no prior loglevel set, the
+default loglevel will be used. If multiple loglevel parameters are given, using
+'repeat'/'level' will not change the loglevel.
 @var{loglevel} is a string or a number containing one of the following values:
 @table @samp
 @item quiet, -8
diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index 708a849..51fa88c 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -888,16 +888,28 @@ int opt_loglevel(void *optctx, const char *opt, const 
char *arg)
 
 flags = av_log_get_flags();
 tail = strstr(arg, "repeat");
-if (tail)
+if (tail == arg) {
 flags &= ~AV_LOG_SKIP_REPEATED;
-else
+arg += 6 + (arg[6] == '+');
+if (!*arg) {
+av_log_set_flags(flags);
+return 0;
+}
+} else {
 flags |= AV_LOG_SKIP_REPEATED;
-
+}
+tail = strstr(arg, "level");
+if (tail == arg) {
+flags |= AV_LOG_PRINT_LEVEL;
+arg += 5 + (arg[5] == '+');
+if (!*arg) {
+av_log_set_flags(flags);
+return 0;
+}
+} else {
+flags &= ~AV_LOG_PRINT_LEVEL;
+}
 av_log_set_flags(flags);
-if (tail == arg)
-arg += 6 + (arg[6]=='+');
-if(tail && !*arg)
-return 0;
 
 for (i = 0; i < FF_ARRAY_ELEMS(log_levels); i++) {
 if (!strcmp(log_levels[i].name, arg)) {
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] fftools/cmdutils: add logflags option

2018-03-19 Thread Tobias Rapp


On 14.03.2018 09:55, Tobias Rapp wrote:

Allows to set the AV_LOG_PRINT_LEVEL and AV_LOG_SKIP_REPEATED flags
using a distinct command-line option, similar to other flag options.
Previously only the AV_LOG_SKIP_REPEATED flag was supported as a prefix
to the "loglevel" option value.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  doc/fftools-common-opts.texi | 13 +
  fftools/cmdutils.c   | 31 +++
  fftools/cmdutils.h   |  6 ++
  3 files changed, 50 insertions(+)

[...]


Any opinions? My motivation was adding support for AV_LOG_PRINT_LEVEL on 
the CLI. Using the existing flags option string parsing functions seemed 
easier and more consistent that extending the custom loglevel string parser.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 2/2] fftools/ffmpeg: update print_report to use AVBPrint API

2018-03-05 Thread Tobias Rapp


On 26.02.2018 17:09, Tobias Rapp wrote:

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  fftools/ffmpeg.c | 44 
  1 file changed, 20 insertions(+), 24 deletions(-)

[...]


Rebased on v1 of the patch and pushed. Thanks for review.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 1/2] fftools/ffmpeg: fix progress log message in case pts is not available

2018-03-05 Thread Tobias Rapp


On 02.03.2018 23:14, wm4 wrote:

On Fri, 2 Mar 2018 22:48:07 +0100
Michael Niedermayer <mich...@niedermayer.cc> wrote:


On Fri, Mar 02, 2018 at 09:07:06AM +0100, Tobias Rapp wrote:

On 01.03.2018 22:08, Michael Niedermayer wrote:

On Wed, Feb 28, 2018 at 09:47:15AM +0100, Tobias Rapp wrote:

On 27.02.2018 19:03, Michael Niedermayer wrote:

On Tue, Feb 27, 2018 at 08:49:19AM +0100, Tobias Rapp wrote:

On 27.02.2018 01:12, Michael Niedermayer wrote:

On Mon, Feb 26, 2018 at 05:09:04PM +0100, Tobias Rapp wrote:

Move time string formatting into inline function. Also fixes out_time
sign prefix for progress report.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  fftools/ffmpeg.c | 48 +++-
  1 file changed, 31 insertions(+), 17 deletions(-)
  

[...]

+{
+const char *hours_sign;
+int hours, mins;
+double secs;
+
+if (pts == AV_NOPTS_VALUE) {
+snprintf(buf, AV_TS_MAX_STRING_SIZE, "N/A");
+} else {
+hours_sign = (pts < 0) ? "-" : "";
+secs = (double)FFABS(pts) / AV_TIME_BASE;
+mins = (int)secs / 60;
+secs = secs - mins * 60;
+hours = mins / 60;
+mins %= 60;


This is not the same code, also with double it can produce inexact
results and results differing between platforms


I changed secs to double to handle the cases with different number of
sub-second digits more easily. Would it be OK to output two digits after the
decimal point in both cases? The progress report contains the precise
out_time_ms value anyway.


iam not sure iam guessing correctly what you mean by "both cases"
you mean if its unneeded as in .00 ?
I guess that would be ok


There are two places within print_report() that output
hour/minute/seconds-formatted time. One is using HH:MM:SS.ZZ format and the
other one is using HH:MM:SS.ZZ format. Would it be OK to output
HH:MM:SS.ZZ (two digits after the decimal separator) in both places like in
the attached patch version?


iam not sure if the reduction of precission is a problem for some use case or 
not
But such a change doesnt belong in a patch factorizing the code.
It should be done seperately, if its changed


Factorizing the code *and* keeping the exact same behavior in both cases is
pointless in my eyes as it just increases the amount and complexity of code
for low benefit.


You could first make the formatting the same and then factorize it.
not saying that should be done, just that this should be the easy was to
factor it out



So please either consider this v3 patch or the original one posted in
https://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225775.html


iam not against this, but it seems others where


My comments were meant as minor suggestion for improvement (as I
stated, unfortunately only in my second reply), not a demand to rewrite
everything. If he just wants to push that first patch, fine with me.


I agree that factorization would make sense but it seems no agreement 
could be found on how exactly it should be implemented. So just pushed 
v1 of the patch which fixes the progress output strings followed by the 
AVBPrint clean-up patch.


Thanks,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v2 2/2] fftools/ffmpeg: update print_report to use AVBPrint API

2018-02-26 Thread Tobias Rapp

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 fftools/ffmpeg.c | 44 
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 0097a7d..9654e07 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1658,8 +1658,7 @@ static void print_final_stats(int64_t total_size)
 
 static void print_report(int is_last_report, int64_t timer_start, int64_t 
cur_time)
 {
-char buf[1024];
-AVBPrint buf_script;
+AVBPrint buf, buf_script;
 OutputStream *ost;
 AVFormatContext *oc;
 int64_t total_size;
@@ -1696,8 +1695,8 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 if (total_size <= 0) // FIXME improve avio_size() so it works with non 
seekable output too
 total_size = avio_tell(oc->pb);
 
-buf[0] = '\0';
 vid = 0;
+av_bprint_init(, 0, AV_BPRINT_SIZE_AUTOMATIC);
 av_bprint_init(_script, 0, 1);
 for (i = 0; i < nb_output_streams; i++) {
 float q = -1;
@@ -1707,7 +1706,7 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 q = ost->quality / (float) FF_QP2LAMBDA;
 
 if (vid && enc->codec_type == AVMEDIA_TYPE_VIDEO) {
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "q=%2.1f ", 
q);
+av_bprintf(, "q=%2.1f ", q);
 av_bprintf(_script, "stream_%d_%d_q=%.1f\n",
ost->file_index, ost->index, q);
 }
@@ -1716,21 +1715,21 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 
 frame_number = ost->frame_number;
 fps = t > 1 ? frame_number / t : 0;
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "frame=%5d 
fps=%3.*f q=%3.1f ",
+av_bprintf(, "frame=%5d fps=%3.*f q=%3.1f ",
  frame_number, fps < 9.95, fps, q);
 av_bprintf(_script, "frame=%d\n", frame_number);
 av_bprintf(_script, "fps=%.1f\n", fps);
 av_bprintf(_script, "stream_%d_%d_q=%.1f\n",
ost->file_index, ost->index, q);
 if (is_last_report)
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "L");
+av_bprintf(, "L");
 if (qp_hist) {
 int j;
 int qp = lrintf(q);
 if (qp >= 0 && qp < FF_ARRAY_ELEMS(qp_histogram))
 qp_histogram[qp]++;
 for (j = 0; j < 32; j++)
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), 
"%X", av_log2(qp_histogram[j] + 1));
+av_bprintf(, "%X", av_log2(qp_histogram[j] + 1));
 }
 
 if ((enc->flags & AV_CODEC_FLAG_PSNR) && (ost->pict_type != 
AV_PICTURE_TYPE_NONE || is_last_report)) {
@@ -1739,7 +1738,7 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 double scale, scale_sum = 0;
 double p;
 char type[3] = { 'Y','U','V' };
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), 
"PSNR=");
+av_bprintf(, "PSNR=");
 for (j = 0; j < 3; j++) {
 if (is_last_report) {
 error = enc->error[j];
@@ -1753,12 +1752,12 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 error_sum += error;
 scale_sum += scale;
 p = psnr(error / scale);
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), 
"%c:%2.2f ", type[j], p);
+av_bprintf(, "%c:%2.2f ", type[j], p);
 av_bprintf(_script, "stream_%d_%d_psnr_%c=%2.2f\n",
ost->file_index, ost->index, type[j] | 32, p);
 }
 p = psnr(error_sum / scale_sum);
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), 
"*:%2.2f ", psnr(error_sum / scale_sum));
+av_bprintf(, "*:%2.2f ", psnr(error_sum / scale_sum));
 av_bprintf(_script, "stream_%d_%d_psnr_all=%2.2f\n",
ost->file_index, ost->index, p);
 }
@@ -1775,18 +1774,15 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 bitrate = pts && total_size >= 0 ? total_size * 8 / (pts / 1000.0) : -1;
 speed = t != 0.0 ? (double)pts / AV_TIME_BASE / t : -1;
 
-if (total_size < 0) snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
-

[FFmpeg-devel] [PATCH v2 1/2] fftools/ffmpeg: fix progress log message in case pts is not available

2018-02-26 Thread Tobias Rapp

Move time string formatting into inline function. Also fixes out_time
sign prefix for progress report.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 fftools/ffmpeg.c | 48 +++-
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 32caa4b..0097a7d 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1518,6 +1518,27 @@ static int reap_filters(int flush)
 return 0;
 }
 
+static inline char *pts_to_hms_str(char *buf, int64_t pts, unsigned int digits)
+{
+const char *hours_sign;
+int hours, mins;
+double secs;
+
+if (pts == AV_NOPTS_VALUE) {
+snprintf(buf, AV_TS_MAX_STRING_SIZE, "N/A");
+} else {
+hours_sign = (pts < 0) ? "-" : "";
+secs = (double)FFABS(pts) / AV_TIME_BASE;
+mins = (int)secs / 60;
+secs = secs - mins * 60;
+hours = mins / 60;
+mins %= 60;
+snprintf(buf, AV_TS_MAX_STRING_SIZE, "%s%02d:%02d:%0*.*f",
+ hours_sign, hours, mins, digits+3, digits, secs);
+}
+return buf;
+}
+
 static void print_final_stats(int64_t total_size)
 {
 uint64_t video_size = 0, audio_size = 0, extra_size = 0, other_size = 0;
@@ -1649,7 +1670,7 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 int64_t pts = INT64_MIN + 1;
 static int64_t last_time = -1;
 static int qp_histogram[52];
-int hours, mins, secs, us;
+char buf_pts[AV_TS_MAX_STRING_SIZE] = {0};
 int ret;
 float t;
 
@@ -1751,25 +1772,15 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 nb_frames_drop += ost->last_dropped;
 }
 
-secs = FFABS(pts) / AV_TIME_BASE;
-us = FFABS(pts) % AV_TIME_BASE;
-mins = secs / 60;
-secs %= 60;
-hours = mins / 60;
-mins %= 60;
-
 bitrate = pts && total_size >= 0 ? total_size * 8 / (pts / 1000.0) : -1;
 speed = t != 0.0 ? (double)pts / AV_TIME_BASE / t : -1;
 
 if (total_size < 0) snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
- "size=N/A time=");
+ "size=N/A ");
 elsesnprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
- "size=%8.0fkB time=", total_size / 1024.0);
-if (pts < 0)
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "-");
+ "size=%8.0fkB ", total_size / 1024.0);
 snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
- "%02d:%02d:%02d.%02d ", hours, mins, secs,
- (100 * us) / AV_TIME_BASE);
+ "time=%s ", pts_to_hms_str(buf_pts, pts, 2));
 
 if (bitrate < 0) {
 snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),"bitrate=N/A");
@@ -1781,9 +1792,12 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 
 if (total_size < 0) av_bprintf(_script, "total_size=N/A\n");
 elseav_bprintf(_script, "total_size=%"PRId64"\n", 
total_size);
-av_bprintf(_script, "out_time_ms=%"PRId64"\n", pts);
-av_bprintf(_script, "out_time=%02d:%02d:%02d.%06d\n",
-   hours, mins, secs, us);
+if (pts == AV_NOPTS_VALUE) {
+av_bprintf(_script, "out_time_ms=N/A\n");
+} else {
+av_bprintf(_script, "out_time_ms=%"PRId64"\n", pts);
+}
+av_bprintf(_script, "out_time=%s\n", pts_to_hms_str(buf_pts, pts, 6));
 
 if (nb_frames_dup || nb_frames_drop)
 snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), " dup=%d 
drop=%d",
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg: fix progress log message in case pts is not available

2018-02-26 Thread Tobias Rapp


On 26.02.2018 14:02, wm4 wrote:

On Mon, 26 Feb 2018 13:14:58 +0100
Tobias Rapp <t.r...@noa-archive.com> wrote:


Also fixes sign prefix for progress report.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  fftools/ffmpeg.c | 25 +
  1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 32caa4b..51f27bf 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1650,6 +1650,7 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
  static int64_t last_time = -1;
  static int qp_histogram[52];
  int hours, mins, secs, us;
+const char *hours_sign;
  int ret;
  float t;
  
@@ -1757,6 +1758,7 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti

  secs %= 60;
  hours = mins / 60;
  mins %= 60;
+hours_sign = (pts < 0) ? "-" : "";
  
  bitrate = pts && total_size >= 0 ? total_size * 8 / (pts / 1000.0) : -1;

  speed = t != 0.0 ? (double)pts / AV_TIME_BASE / t : -1;
@@ -1765,11 +1767,13 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
   "size=N/A time=");
  elsesnprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
   "size=%8.0fkB time=", total_size / 1024.0);
-if (pts < 0)
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "-");
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
- "%02d:%02d:%02d.%02d ", hours, mins, secs,
- (100 * us) / AV_TIME_BASE);
+if (pts == AV_NOPTS_VALUE) {
+snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "N/A");
+} else {
+snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
+ "%s%02d:%02d:%02d.%02d ", hours_sign, hours, mins, secs,
+ (100 * us) / AV_TIME_BASE);
+}
  
  if (bitrate < 0) {

  snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),"bitrate=N/A");
@@ -1781,9 +1785,14 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
  
  if (total_size < 0) av_bprintf(_script, "total_size=N/A\n");

  elseav_bprintf(_script, "total_size=%"PRId64"\n", 
total_size);
-av_bprintf(_script, "out_time_ms=%"PRId64"\n", pts);
-av_bprintf(_script, "out_time=%02d:%02d:%02d.%06d\n",
-   hours, mins, secs, us);
+if (pts == AV_NOPTS_VALUE) {
+av_bprintf(_script, "out_time_ms=N/A\n");
+av_bprintf(_script, "out_time=N/A\n");
+} else {
+av_bprintf(_script, "out_time_ms=%"PRId64"\n", pts);
+av_bprintf(_script, "out_time=%s%02d:%02d:%02d.%06d\n",
+   hours_sign, hours, mins, secs, us);
+}
  
  if (nb_frames_dup || nb_frames_drop)

  snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), " dup=%d 
drop=%d",


Could use av_ts2str(), although that would return different formatting.


I would prefer to not change the formatting, av_ts2str just prints the 
number of seconds as a float while the current HH:MM:SS.ZZ format is 
more user friendly, IMHO.



Or maybe do something similar and put that code into a new function or
macro, so you don't have to repeat all those awful string buffer
management expressions in the first hunk.


If you refer to the "buf + strlen(buf), sizeof(buf) - strlen(buf)" 
expressions the print_report() function is full of those. I agree that 
switching buf to AVBPrint would improve the code -- this could be a 
follow-up patch, if desired.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] fftools/ffmpeg: fix progress log message in case pts is not available

2018-02-26 Thread Tobias Rapp

Also fixes sign prefix for progress report.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 fftools/ffmpeg.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 32caa4b..51f27bf 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1650,6 +1650,7 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 static int64_t last_time = -1;
 static int qp_histogram[52];
 int hours, mins, secs, us;
+const char *hours_sign;
 int ret;
 float t;
 
@@ -1757,6 +1758,7 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 secs %= 60;
 hours = mins / 60;
 mins %= 60;
+hours_sign = (pts < 0) ? "-" : "";
 
 bitrate = pts && total_size >= 0 ? total_size * 8 / (pts / 1000.0) : -1;
 speed = t != 0.0 ? (double)pts / AV_TIME_BASE / t : -1;
@@ -1765,11 +1767,13 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
  "size=N/A time=");
 elsesnprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
  "size=%8.0fkB time=", total_size / 1024.0);
-if (pts < 0)
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "-");
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
- "%02d:%02d:%02d.%02d ", hours, mins, secs,
- (100 * us) / AV_TIME_BASE);
+if (pts == AV_NOPTS_VALUE) {
+snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "N/A");
+} else {
+snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
+ "%s%02d:%02d:%02d.%02d ", hours_sign, hours, mins, secs,
+ (100 * us) / AV_TIME_BASE);
+}
 
 if (bitrate < 0) {
 snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),"bitrate=N/A");
@@ -1781,9 +1785,14 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 
 if (total_size < 0) av_bprintf(_script, "total_size=N/A\n");
 elseav_bprintf(_script, "total_size=%"PRId64"\n", 
total_size);
-av_bprintf(_script, "out_time_ms=%"PRId64"\n", pts);
-av_bprintf(_script, "out_time=%02d:%02d:%02d.%06d\n",
-   hours, mins, secs, us);
+if (pts == AV_NOPTS_VALUE) {
+av_bprintf(_script, "out_time_ms=N/A\n");
+av_bprintf(_script, "out_time=N/A\n");
+} else {
+av_bprintf(_script, "out_time_ms=%"PRId64"\n", pts);
+av_bprintf(_script, "out_time=%s%02d:%02d:%02d.%06d\n",
+   hours_sign, hours, mins, secs, us);
+}
 
 if (nb_frames_dup || nb_frames_drop)
 snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), " dup=%d 
drop=%d",
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 1/2] fftools/ffmpeg: fix progress log message in case pts is not available

2018-02-28 Thread Tobias Rapp


On 27.02.2018 19:03, Michael Niedermayer wrote:

On Tue, Feb 27, 2018 at 08:49:19AM +0100, Tobias Rapp wrote:

On 27.02.2018 01:12, Michael Niedermayer wrote:

On Mon, Feb 26, 2018 at 05:09:04PM +0100, Tobias Rapp wrote:

Move time string formatting into inline function. Also fixes out_time
sign prefix for progress report.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  fftools/ffmpeg.c | 48 +++-
  1 file changed, 31 insertions(+), 17 deletions(-)


[...]

+{
+const char *hours_sign;
+int hours, mins;
+double secs;
+
+if (pts == AV_NOPTS_VALUE) {
+snprintf(buf, AV_TS_MAX_STRING_SIZE, "N/A");
+} else {
+hours_sign = (pts < 0) ? "-" : "";
+secs = (double)FFABS(pts) / AV_TIME_BASE;
+mins = (int)secs / 60;
+secs = secs - mins * 60;
+hours = mins / 60;
+mins %= 60;


This is not the same code, also with double it can produce inexact
results and results differing between platforms


I changed secs to double to handle the cases with different number of
sub-second digits more easily. Would it be OK to output two digits after the
decimal point in both cases? The progress report contains the precise
out_time_ms value anyway.


iam not sure iam guessing correctly what you mean by "both cases"
you mean if its unneeded as in .00 ?
I guess that would be ok


There are two places within print_report() that output 
hour/minute/seconds-formatted time. One is using HH:MM:SS.ZZ format and 
the other one is using HH:MM:SS.ZZ format. Would it be OK to output 
HH:MM:SS.ZZ (two digits after the decimal separator) in both places like 
in the attached patch version?


Regards,
Tobias

From d2141a259d733e9ff8e76f0e33b3e2e449adde14 Mon Sep 17 00:00:00 2001
From: Tobias Rapp <t.r...@noa-archive.com>
Date: Mon, 26 Feb 2018 16:58:24 +0100
Subject: [PATCH v3] fftools/ffmpeg: fix progress log message in case pts is
 not available

Move time string formatting into inline function. Also fixes out_time
sign prefix for progress report.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 fftools/ffmpeg.c | 48 +++-
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 3a45f43..f3598ff 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1518,6 +1518,27 @@ static int reap_filters(int flush)
 return 0;
 }
 
+static inline char *pts_to_hms_str(char *buf, size_t buf_size, int64_t pts)
+{
+const char *hours_sign;
+int hours, mins, secs, usecs;
+
+if (pts == AV_NOPTS_VALUE) {
+snprintf(buf, buf_size, "N/A");
+} else {
+hours_sign = (pts < 0) ? "-" : "";
+secs = FFABS(pts) / AV_TIME_BASE;
+usecs = FFABS(pts) % AV_TIME_BASE;
+mins = secs / 60;
+secs %= 60;
+hours = mins / 60;
+mins %= 60;
+snprintf(buf, buf_size, "%s%02d:%02d:%02d.%02d",
+ hours_sign, hours, mins, secs, (100 * usecs) / AV_TIME_BASE);
+}
+return buf;
+}
+
 static void print_final_stats(int64_t total_size)
 {
 uint64_t video_size = 0, audio_size = 0, extra_size = 0, other_size = 0;
@@ -1649,7 +1670,7 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 int64_t pts = INT64_MIN + 1;
 static int64_t last_time = -1;
 static int qp_histogram[52];
-int hours, mins, secs, us;
+char buf_pts[AV_TS_MAX_STRING_SIZE] = {0};
 int ret;
 float t;
 
@@ -1751,25 +1772,15 @@ static void print_report(int is_last_report, int64_t 
timer_start, int64_t cur_ti
 nb_frames_drop += ost->last_dropped;
 }
 
-secs = FFABS(pts) / AV_TIME_BASE;
-us = FFABS(pts) % AV_TIME_BASE;
-mins = secs / 60;
-secs %= 60;
-hours = mins / 60;
-mins %= 60;
-
 bitrate = pts && total_size >= 0 ? total_size * 8 / (pts / 1000.0) : -1;
 speed = t != 0.0 ? (double)pts / AV_TIME_BASE / t : -1;
 
 if (total_size < 0) snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
- "size=N/A time=");
+ "size=N/A ");
 elsesnprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
- "size=%8.0fkB time=", total_size / 1024.0);
-if (pts < 0)
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "-");
+ "size=%8.0fkB ", total_size / 1024.0);
 snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
- "%02d:%02d:%02d.%02d ", hours, mins, secs,
- (100 * us) / AV_TIME_BASE);
+ "time=%s ", pts_to_hms_str(buf_pts, sizeof(buf_pts), pts));
 
 if (bitrate < 0) {
 snprintf(buf + strlen(b

Re: [FFmpeg-devel] [PATCH v2 1/2] fftools/ffmpeg: fix progress log message in case pts is not available

2018-03-02 Thread Tobias Rapp


On 01.03.2018 22:08, Michael Niedermayer wrote:

On Wed, Feb 28, 2018 at 09:47:15AM +0100, Tobias Rapp wrote:

On 27.02.2018 19:03, Michael Niedermayer wrote:

On Tue, Feb 27, 2018 at 08:49:19AM +0100, Tobias Rapp wrote:

On 27.02.2018 01:12, Michael Niedermayer wrote:

On Mon, Feb 26, 2018 at 05:09:04PM +0100, Tobias Rapp wrote:

Move time string formatting into inline function. Also fixes out_time
sign prefix for progress report.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
  fftools/ffmpeg.c | 48 +++-
  1 file changed, 31 insertions(+), 17 deletions(-)


[...]

+{
+const char *hours_sign;
+int hours, mins;
+double secs;
+
+if (pts == AV_NOPTS_VALUE) {
+snprintf(buf, AV_TS_MAX_STRING_SIZE, "N/A");
+} else {
+hours_sign = (pts < 0) ? "-" : "";
+secs = (double)FFABS(pts) / AV_TIME_BASE;
+mins = (int)secs / 60;
+secs = secs - mins * 60;
+hours = mins / 60;
+mins %= 60;


This is not the same code, also with double it can produce inexact
results and results differing between platforms


I changed secs to double to handle the cases with different number of
sub-second digits more easily. Would it be OK to output two digits after the
decimal point in both cases? The progress report contains the precise
out_time_ms value anyway.


iam not sure iam guessing correctly what you mean by "both cases"
you mean if its unneeded as in .00 ?
I guess that would be ok


There are two places within print_report() that output
hour/minute/seconds-formatted time. One is using HH:MM:SS.ZZ format and the
other one is using HH:MM:SS.ZZ format. Would it be OK to output
HH:MM:SS.ZZ (two digits after the decimal separator) in both places like in
the attached patch version?


iam not sure if the reduction of precission is a problem for some use case or 
not
But such a change doesnt belong in a patch factorizing the code.
It should be done seperately, if its changed


Factorizing the code *and* keeping the exact same behavior in both cases 
is pointless in my eyes as it just increases the amount and complexity 
of code for low benefit.


So please either consider this v3 patch or the original one posted in 
https://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225775.html


All I wanted to get fixed was the large negative value printed at the 
start of transcoding ... *sigh*


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg: properly initialize output stream field order

2018-04-26 Thread Tobias Rapp


On 26.04.2018 16:11, Derek Buitenhuis wrote:

On 4/26/2018 3:06 PM, Tobias Rapp wrote:

+if (ost->top_field_first == 0) {
+enc_ctx->field_order = AV_FIELD_BB;
+} else if (ost->top_field_first == 1) {
+enc_ctx->field_order = AV_FIELD_TT;
+}


This doesn't look correct; ost->top_field_first is only
valid if ost->interlaced_frame is set. Wouldn't this
incorrectly set field_order on progressive streams,
which should be set to AV_FIELD_PROGRESSIVE?


"ost" is of type OutputStream here, which only has top_field_first with 
values auto=-1/bff=0/tff=1. AVFrame has the 
interlaced_frame/top_field_first pair you mentioned, while 
AVCodecContext has field_order.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] fftools/ffmpeg: properly initialize output stream field order

2018-04-26 Thread Tobias Rapp

Fixes stream field order written by avformat_write_header when "top"
option is specified on the command-line.

Signed-off-by: Tobias Rapp <t.r...@noa-archive.com>
---
 fftools/ffmpeg.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 4dbe721..a28786a 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -3379,6 +3379,12 @@ static int init_output_stream_encode(OutputStream *ost)
 enc_ctx->bits_per_raw_sample = frame_bits_per_raw_sample;
 }
 
+if (ost->top_field_first == 0) {
+enc_ctx->field_order = AV_FIELD_BB;
+} else if (ost->top_field_first == 1) {
+enc_ctx->field_order = AV_FIELD_TT;
+}
+
 if (ost->forced_keyframes) {
 if (!strncmp(ost->forced_keyframes, "expr:", 5)) {
 ret = av_expr_parse(>forced_keyframes_pexpr, 
ost->forced_keyframes+5,
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/libopenh264enc: fix class_name

2018-11-12 Thread Tobias Rapp


On 12.11.2018 18:10, Jan Ekström wrote:

On Mon, Nov 12, 2018 at 6:11 PM Tobias Rapp  wrote:


Reverts some accidental change in commit 
e621b1ca646a2f268797adc3194b694a852548d2.

Signed-off-by: Tobias Rapp 
---
  libavcodec/libopenh264enc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index 5baa423..ae6d17c 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -75,7 +75,7 @@ static const AVOption options[] = {
  };

  static const AVClass class = {
-.class_name = "libvo_amrwbenc",
+.class_name = "libopenh264enc",
  .item_name  = av_default_item_name,
  .option = options,
  .version= LIBAVUTIL_VERSION_INT,
--


Looking at the file name, definitely LGTM (also checked the history
and this is what the string used to be).


Pushed, thanks for review.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avcodec/libopenh264enc: fix class_name

2018-11-12 Thread Tobias Rapp

Reverts some accidental change in commit 
e621b1ca646a2f268797adc3194b694a852548d2.

Signed-off-by: Tobias Rapp 
---
 libavcodec/libopenh264enc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index 5baa423..ae6d17c 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -75,7 +75,7 @@ static const AVOption options[] = {
 };
 
 static const AVClass class = {
-.class_name = "libvo_amrwbenc",
+.class_name = "libopenh264enc",
 .item_name  = av_default_item_name,
 .option = options,
 .version= LIBAVUTIL_VERSION_INT,
-- 
2.7.4


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] FFmpeg 4.1

2018-09-20 Thread Tobias Rapp


On 20.09.2018 12:26, Martin Vignali wrote:

Hello,

In current git, the qt faststart test doesn't pass for me (clang os 10.12),
maybe need to be fix before the release.

CCtools/qt-faststart.o
LDtools/qt-faststart
TESTmov-faststart-4gb-overflow
--- -2018-09-20 12:18:23.0 +0200
+++ tests/data/fate/mov-faststart-4gb-overflow2018-09-20
12:18:23.0 +0200
@@ -1 +0,0 @@
-bc875921f151871e787c4b4023269b29
Test mov-faststart-4gb-overflow failed. Look at
tests/data/fate/mov-faststart-4gb-overflow.err for details.
make: *** [fate-mov-faststart-4gb-overflow] Error 1

./tests/fate-run.sh: line 387: md5sum: command not found


The error message seems to indicate that not all required utilities are 
installed in your local environment, i.e. the "md5sum" binary is either 
not installed or not available in $PATH to the FATE script.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] doc/developer: require transparency about sponshorships.

2019-01-14 Thread Tobias Rapp


On 14.01.2019 17:20, Nicolas George wrote:

Tobias Rapp (12019-01-14):

Writing good code requires time. I don't see how being sponsored for
development should have a negative correlation (in general) to the time
invested on a specific topic/patch.


Let us say somebody worked one day on a sponsored patch. They now have
two choices:

- spend another day refactoring the code, designing functions API so
   that they can be shared with existing code;

- submit as is and start working on a new patch for a new sponsorship.

Which one will be more attractive?


I would assume that a sponsor's interest in sustainability is at least 
equal to the interest of somebody doing development in free time. At 
least I don't see an evident point why the interest should be different 
per se.


To me the more helpful discussion would be around how to resolve 
conflicts during code review and improve patch quality, rather than the 
influence of sponsorship.


Best regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter: add astretch filter

2019-01-21 Thread Tobias Rapp


On 21.01.2019 17:43, Paul B Mahol wrote:

On 11/10/18, Paul B Mahol  wrote:

Signed-off-by: Paul B Mahol 
---
  libavfilter/Makefile  |   1 +
  libavfilter/af_astretch.c | 330 ++
  libavfilter/allfilters.c  |   1 +
  3 files changed, 332 insertions(+)
  create mode 100644 libavfilter/af_astretch.c



will apply ASAP!


Please add some (short) documentation. Also FATE tests are welcome.

Best regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: Added an option to disable SIDX atom

2018-12-04 Thread Tobias Rapp


On 04.12.2018 09:41, Karthick J wrote:

---
  doc/muxers.texi  | 2 ++
  libavformat/movenc.c | 7 +--
  libavformat/movenc.h | 1 +
  3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index f1cc6f5fee..6ca27b04a3 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1325,6 +1325,8 @@ more efficient), but with this option set, the muxer 
writes one moof/mdat
  pair for each track, making it easier to separate tracks.
  
  This option is implicitly set when writing ismv (Smooth Streaming) files.

+@item -movflags no_sidx
+Don't write sidx atom.
  @item -movflags faststart
  Run a second pass moving the index (moov atom) to the beginning of the file.
  This operation can take a while, and will not work in various situations such


What about naming the option "skip_sidx" for symmetry with the existing 
"skip_trailer"? Just my personal thought.


Also it might be worth mentioning in the docs how global_sidx and the 
new option correlate (which one is preferred if both exists, or is it an 
error to specify both?).


>
> [...]
>

diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index fe605d1ad2..ee6749bce2 100644
--- a/libavformat/movenc.h
+++ b/libavformat/movenc.h
@@ -257,6 +257,7 @@ typedef struct MOVMuxContext {
  #define FF_MOV_FLAG_SKIP_TRAILER  (1 << 18)
  #define FF_MOV_FLAG_NEGATIVE_CTS_OFFSETS  (1 << 19)
  #define FF_MOV_FLAG_FRAG_EVERY_FRAME  (1 << 20)
+#define FF_MOV_FLAG_NO_SIDX   (1 << 21)
  
  int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt);
  



Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 1/2] avformat/movenc: Added an option to disable SIDX atom

2018-12-05 Thread Tobias Rapp


On 05.12.2018 07:18, Karthick J wrote:

---
  doc/muxers.texi  |  4 
  libavformat/movenc.c | 12 ++--
  libavformat/movenc.h |  1 +
  3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index f1cc6f5fee..ca10741900 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1325,6 +1325,10 @@ more efficient), but with this option set, the muxer 
writes one moof/mdat
  pair for each track, making it easier to separate tracks.
  
  This option is implicitly set when writing ismv (Smooth Streaming) files.

+@item -movflags skip_sidx
+Skip writing of sidx atom. When bitrate overhead due to sidx atom is high,
+this option could be used for cases where sidx atom is not mandatory.
+When global_sidx flag is enabled, this option will be ignored.
  @item -movflags faststart
  Run a second pass moving the index (moov atom) to the beginning of the file.
  This operation can take a while, and will not work in various situations such
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 6dab5193b0..3781a32895 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -75,6 +75,7 @@ static const AVOption options[] = {
  { "frag_discont", "Signal that the next fragment is discontinuous from earlier 
ones", 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_FRAG_DISCONT}, INT_MIN, INT_MAX, 
AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
  { "delay_moov", "Delay writing the initial moov until the first fragment is cut, or 
until the first fragment flush", 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_DELAY_MOOV}, INT_MIN, 
INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
  { "global_sidx", "Write a global sidx index at the start of the file", 0, 
AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_GLOBAL_SIDX}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, 
"movflags" },
+{ "skip_sidx", "Skip writing of sidx atom", 0, AV_OPT_TYPE_CONST, {.i64 = 
FF_MOV_FLAG_SKIP_SIDX}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
  { "write_colr", "Write colr atom (Experimental, may be renamed or changed, do not use 
from scripts)", 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_WRITE_COLR}, INT_MIN, INT_MAX, 
AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
  { "write_gama", "Write deprecated gama atom", 0, AV_OPT_TYPE_CONST, {.i64 = 
FF_MOV_FLAG_WRITE_GAMA}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
  { "use_metadata_tags", "Use mdta atom for metadata.", 0, AV_OPT_TYPE_CONST, {.i64 = 
FF_MOV_FLAG_USE_MDTA}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
@@ -4603,7 +4604,8 @@ static int mov_write_moof_tag(AVIOContext *pb, 
MOVMuxContext *mov, int tracks,
  mov_write_moof_tag_internal(avio_buf, mov, tracks, 0);
  moof_size = ffio_close_null_buf(avio_buf);
  
-if (mov->flags & FF_MOV_FLAG_DASH && !(mov->flags & FF_MOV_FLAG_GLOBAL_SIDX))

+if (mov->flags & FF_MOV_FLAG_DASH &&
+!(mov->flags & (FF_MOV_FLAG_GLOBAL_SIDX | FF_MOV_FLAG_SKIP_SIDX)))
  mov_write_sidx_tags(pb, mov, tracks, moof_size + 8 + mdat_size);
  
  if (mov->write_prft > MOV_PRFT_NONE && mov->write_prft < MOV_PRFT_NB)

@@ -5422,7 +5424,8 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
   * the next fragment. This means the cts of the first sample must
   * be the same in all fragments, unless end_pts was updated by
   * the packet causing the fragment to be written. */
-if ((mov->flags & FF_MOV_FLAG_DASH && !(mov->flags & 
FF_MOV_FLAG_GLOBAL_SIDX)) ||
+if ((mov->flags & FF_MOV_FLAG_DASH &&
+!(mov->flags & (FF_MOV_FLAG_GLOBAL_SIDX | 
FF_MOV_FLAG_SKIP_SIDX))) ||
  mov->mode == MODE_ISM)
  pkt->pts = pkt->dts + trk->end_pts - 
trk->cluster[trk->entry].dts;
  } else {
@@ -6067,6 +6070,11 @@ static int mov_init(AVFormatContext *s)
  s->flags &= ~AVFMT_FLAG_AUTO_BSF;
  }
  
+if (mov->flags & FF_MOV_FLAG_GLOBAL_SIDX && s->flags & FF_MOV_FLAG_SKIP_SIDX) {

+av_log(s, AV_LOG_WARNING, "Global SIDX enabled; Ignoring skip_sidx 
option\n");
+s->flags &= ~FF_MOV_FLAG_SKIP_SIDX;
+}
+


I guess this should use mov->flags instead of s->flags?


  if (mov->flags & FF_MOV_FLAG_FASTSTART) {
  mov->reserved_moov_size = -1;
  }
diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index fe605d1ad2..68d6f23a5a 100644
--- a/libavformat/movenc.h
+++ b/libavformat/movenc.h
@@ -257,6 +257,7 @@ typedef struct MOVMuxContext {
  #define FF_MOV_FLAG_SKIP_TRAILER  (1 << 18)
  #define FF_MOV_FLAG_NEGATIVE_CTS_OFFSETS  (1 << 19)
  #define FF_MOV_FLAG_FRAG_EVERY_FRAME  (1 << 20)
+#define FF_MOV_FLAG_SKIP_SIDX (1 << 21)
  
  int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt);
  



Otherwise changes look fine, but I am not the maintainer of this file.

Regards,
Tobias

___
ffmpeg-devel mailing list

Re: [FFmpeg-devel] [PATCH v3] avformat/movenc: Added an option to disable SIDX atom

2018-12-05 Thread Tobias Rapp


On 06.12.2018 06:07, Karthick J wrote:

---
  doc/muxers.texi  |  4 
  libavformat/movenc.c | 12 ++--
  libavformat/movenc.h |  1 +
  3 files changed, 15 insertions(+), 2 deletions(-)

[...]
  
@@ -6067,6 +6070,11 @@ static int mov_init(AVFormatContext *s)

  s->flags &= ~AVFMT_FLAG_AUTO_BSF;
  }
  
+if (mov->flags & FF_MOV_FLAG_GLOBAL_SIDX && s->flags & FF_MOV_FLAG_SKIP_SIDX) {

+av_log(s, AV_LOG_WARNING, "Global SIDX enabled; Ignoring skip_sidx 
option\n");
+mov->flags &= ~FF_MOV_FLAG_SKIP_SIDX;
+}
+


There is still one use of s->flags instead of mov->flags, that should be 
changed.



  if (mov->flags & FF_MOV_FLAG_FASTSTART) {
  mov->reserved_moov_size = -1;
  }
diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index fe605d1ad2..68d6f23a5a 100644
--- a/libavformat/movenc.h
+++ b/libavformat/movenc.h
@@ -257,6 +257,7 @@ typedef struct MOVMuxContext {
  #define FF_MOV_FLAG_SKIP_TRAILER  (1 << 18)
  #define FF_MOV_FLAG_NEGATIVE_CTS_OFFSETS  (1 << 19)
  #define FF_MOV_FLAG_FRAG_EVERY_FRAME  (1 << 20)
+#define FF_MOV_FLAG_SKIP_SIDX (1 << 21)
  
  int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt);
  



Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v4] avformat/movenc: Added an option to disable SIDX atom

2018-12-06 Thread Tobias Rapp


On 06.12.2018 08:28, Karthick J wrote:

---
  doc/muxers.texi  |  4 
  libavformat/movenc.c | 12 ++--
  libavformat/movenc.h |  1 +
  3 files changed, 15 insertions(+), 2 deletions(-)
  
[...]


Looks OK now, no more comments from my side.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] VDD FFmpeg session and community survey

2018-11-26 Thread Tobias Rapp


On 24.11.2018 16:32, Tomas Härdin wrote:

[...]

While I'm in here I have a small suggestion: talking is better than
writing when it comes to interpersonal conflicts. Using Mumble or
Jingle or whatever and getting the relevant people to talk can be a
good way to avoid more drastic measures. Unless of course someone's
just being an ass to be an ass, but I haven't noticed that yet on here.


Seems like a good idea. And the talking could be moderated by a person 
neutral to the conflict.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2019-01-08 Thread Tobias Rapp


On 07.01.2019 15:31, Dmitry Humeniuk wrote:

Signed-off-by: Dmitry Humeniuk 
---
  Changelog|1 +
  doc/filters.texi |   23 +
  libavfilter/Makefile |1 +
  libavfilter/af_dumpwave.c|  234 
  libavfilter/allfilters.c |1 +
  libavfilter/version.h|4 +-
  tests/fate-run.sh|9 +
  tests/fate/filter-audio.mak  |   12 +
  tests/ref/fate/filter-dumpwave   | 1800 ++
  tests/ref/fate/filter-dumpwave-24bit | 1800 ++
  tests/ref/fate/filter-dumpwave-fltp  | 1800 ++
  11 files changed, 5683 insertions(+), 2 deletions(-)
  create mode 100644 libavfilter/af_dumpwave.c
  create mode 100644 tests/ref/fate/filter-dumpwave
  create mode 100644 tests/ref/fate/filter-dumpwave-24bit
  create mode 100644 tests/ref/fate/filter-dumpwave-fltp

diff --git a/Changelog b/Changelog
index f135fa56b6..1eb337796b 100644
--- a/Changelog
+++ b/Changelog
@@ -13,6 +13,7 @@ version :
  - GIF parser
  - vividas demuxer
  - hymt decoder
+- dumpwave filter
  
  
  version 4.1:

diff --git a/doc/filters.texi b/doc/filters.texi
index 98858c5356..66c2961fd8 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -2912,6 +2912,29 @@ Set window length in seconds used to split audio into 
segments of equal length.
  Default is 3 seconds.
  @end table
  
+@section dumpwave

+Dump RMS envelope to a file.
+Convert samples to decibels and calculates RMS (Root-Mean-Square) audio power 
in 0 to 1.0 floats.
+
+@table @option
+@item w, width
+Number of data values.
+The default value is @var{1800}
+
+@item n, nb_samples
+Samples count per value per channel, default 128
+
+@item f, file
+Path to a file ``-'' is a shorthand
+for standard output.
+@end table
+
+For example, to generate RMS envelope for 44.1 kHz 6 seconds length audio
+with dimensions @var{1800x140}, samples count @code{44100*6/1800=147} and 
store it to @var{/tmp/out.csv}, you might use:
+@example
+dumpwave=w=1800:n=147:f=/tmp/out.csv
+@end example
+
  @section dynaudnorm
  Dynamic Audio Normalizer.
  
[...]


I appreciate the completeness of the proposed patch (documentation, 
tests, version bump) but it seems that the functionality overlaps with 
existing filters. Besides the different output format I think the same 
can be achieved by using a command like:


ffmpeg -i input.wav -f null -filter:a 
"asetnsamples=147,astats=metadata=on,ametadata=print:key=lavfi.astats.Overall.RMS_peak:file=/tmp/out.dat" 
/dev/null


So if it is only about the output format, it would be better to add CSV 
support to the ametadata filter, IMHO. That way other filters with frame 
metadata output will also gain CSV support.


Best regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] doc/developer: require transparency about sponshorships.

2019-01-13 Thread Tobias Rapp


On 13.01.2019 15:07, Gyan wrote:


On 13-01-2019 06:39 PM, Ronald S. Bultje wrote:

Hi,

On Sun, Jan 13, 2019 at 4:39 AM Gyan  wrote:


When someone submits a patch, it is implicit, unless stated otherwise,
that it is of their own initiative (and their own work), and thus they
are free to assign copyright. When work is performed for hire, the
copyright may belong to the employer. Such sponsored work cannot be
'donated' to the project


But we don't do copyright assignment.



No, the patch submitter (implicitly) does. Which is not a problem when 
the copyright holder and submitter are the same person. For sponsored 
code, they may not be.


Analogy:

Scenario 1

A 'vlogger' makes a video and uploads it as public to Youtube. Youtube 
then lets everyone see that video. No problem.


Scenario 2

Someone pays the vlogger to make a video. Vlogger uploads it to YT as 
public. There's a problem if the client did not allow that which makes 
it copyright infringement, Which is why YT has this clause in their T


"You affirm, represent, and warrant that you own or have the necessary 
licenses, rights, consents, and permissions to publish Content you 
submit; and you license to YouTube all patent, trademark, trade secret, 
copyright or other proprietary rights in and to such Content for 
publication on the Service pursuant to these Terms of Service."


So, we are YT in this case and the Content is the patch(es). The concern 
would be that the submitter doesn't have the right to license the code 
into ffmpeg, if the contract with the client doesn't allow them to do 
it. Only way to be sure is for the sponsor to affirm to it. And for 
that, we would have to know that there is a sponsor, to start with.


Isn't this what the "Signed-off-by" line in a commit already states? So 
to solve this (if actually deemed necessary) it would be better to 
request the commit author to signal that the copyright status has been 
clarified by always adding this Signed-off-by line instead.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] doc/developer: require transparency about sponshorships.

2019-01-14 Thread Tobias Rapp


On 13.01.2019 16:24, Nicolas George wrote:

James Almer (12019-01-13):

How is that related to sponsored work? If a patch was ignored, then the
extra line in the commit message would have been ignored as much as the
actual code.


Without sponsoring, most reasons for developing code are positively
correlated with code quality. Not perfectly, but at least some.

Sponsorship, on the other hand, is a motivation for developing code that
has little to do with code quality.

For that reason, sponsored code should be examined much more carefully.


Writing good code requires time. I don't see how being sponsored for 
development should have a negative correlation (in general) to the time 
invested on a specific topic/patch.


Patch review intensity should be based on the content of the patch 
itself (e.g. complexity and long-term maintenance factor) and not based 
on some disclosure requirement that has the potential to support prejudice.


Best regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] News: Removal of libndi

2019-03-26 Thread Tobias Rapp


On 25.03.2019 18:02, Jean-Baptiste Kempf wrote:

On Mon, 25 Mar 2019, at 08:32, Tobias Rapp wrote:

Most of those hardware libraries are glorified ioctls around the driver and 
shipped with the drivers.
And I see this with nVidia, Intel MFX and Decklink (lots of "acquire C++ interface, 
set param" there, release the C++ interface).

Matrox seems to do something else, though, introducing a special library for 
FFmpeg consumption, and I doubt that feels like a driver...


The GPL is mentioned a lot in this thread. Maybe it would make sense to
distinguish the two cases where FFmpeg is compiled with --enable-gpl and
without it -- as the LGPL applies in that case.


That does not change a thing, sorry.
The section 6 of the LGPLv2.1 is similar to the section 3 of the GPL, and 
mentions exactly the same limitations and exceptions for major components of 
the OS.

The fact that you can combine the library with a 3rd party library inside your 
program does not allow you to ship non-LGPL-compatible code inside the library. 
(The library must be changeable + redistributable by the user).

I know that means that you can do more or less the same feature, but that means 
the architecture must be different.


I thought that section 7 would allow to combine a 3rd party library with 
a LGPL library to create a new library but now when reading it again I 
stumble over the word "side-by-side" which indicates that the two parts 
should not interact with each other.


So yes, your interpretation looks correct to me (IANAL).

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] News: Removal of libndi

2019-03-25 Thread Tobias Rapp

On 24.03.2019 21:14, Jean-Baptiste Kempf wrote:

On Sun, 24 Mar 2019, at 20:10, Ronald S. Bultje wrote:

The GPL does not mention hardware (instead, they use the word "system
library"). Going from here, I don't consider enterprise-level hardware like
Matrox $$$ priced stuff to be a system library at all. My system certainly
has no hardware or drivers or system-level libraries that are
API+ABI+functionally compatible with Matrox' tools and wares - under any
license, not open-source and not closed-source. How can the system library
exception possibly apply here?

Drivers have always been considered part of the OS, whatever the price of the
hardware.
The Linux kernel has drivers for pieces of hardware that are way more expensive
than Matrox hardware.

So, if the library is part of the driver (installed at the same time), it is considered
part of the "major components of the OS", because if you don't install the
driver, you cannot use the hardware.
This is the opinion of the Linux Foundation, the FSF, FSFE and so many others.

Usually, the "major components" (as mentioned in the GPL and not "system libraries",
which is the shortcut) explicitly mention 3 parts: kernel, compiler and "others."
The common understanding is that everything that runs in Kernel-Land, aka kernel + drivers is the kernel part of the
"major components". libc, compiler and libraries linked by compilers, if distributed with the OS, are the
second part of the "major components"; and the last part, "others" cover the other core parts of
the OS (usually the "base" in linux distributions), and covers init and the shell, and the basic services
normally installed by default (at, cron, etc..).

Yes, this meaning is very dated, but GPLv2 is from 1991.

Most of those hardware libraries are glorified ioctls around the driver and
shipped with the drivers.
And I see this with nVidia, Intel MFX and Decklink (lots of "acquire C++ interface,
set param" there, release the C++ interface).

Matrox seems to do something else, though, introducing a special library for
FFmpeg consumption, and I doubt that feels like a driver...

The GPL is mentioned a lot in this thread. Maybe it would make sense to
distinguish the two cases where FFmpeg is compiled with --enable-gpl and
without it -- as the LGPL applies in that case.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH]download: Fix the release link

2019-03-28 Thread Tobias Rapp


On 28.03.2019 12:00, Carl Eugen Hoyos wrote:

Hi!

Attached patch also simplifies the release process.

Please comment, Carl Eugen


Personally I'd prefer to keep the link to the latest release. There is 
already a "Download Snapshot" button on the "Get the Sources" panel.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Fix loss of precision for silencedetecton large files

2019-03-28 Thread Tobias Rapp


On 27.03.2019 23:13, Allan Cady via ffmpeg-devel wrote:

  On Tue, Mar 26, 2019 at 10:07:10PM +, Allan Cady via ffmpeg-devel wrote:


When the silencedetect filter is run against very large files, the
output timestamps gradually lose precision as the scan proceeds
further into the file. This is because the output is formatted (in
libavutil/timestamp.h) as "%.6g", which limits the total field length.
Eventually, for offsets greater than 10 seconds (about 28 hours),
fractions of a second disappear altogether, and the timestamps
are logged as whole integers. This is insufficient precision for
my purposes.

I propose changing the format to "%.3f", which will give millisecond
precision for all timestamps regardless of offset.


I think it would be nice if some of this description would be included 
into the patch commit message. The header line is misleading in focusing 
on silencedetect when the change is actually done in (a)metadata filter.


Also usual commit header line convention is to start with the affected 
module (like "avutil/timestamp: ...").



First glance it looks like there are pre-built reference output files in the 
tests that may need to be re-generated, and that may be all that's required. 
I'll continue digging to try to understand what's going on.


You might want to look into "make fate GEN=1". See 
http://ffmpeg.org/fate.html#FATE-makefile-targets-and-variables for details.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [FFmpeg-cvslog] [ffmpeg-web] branch master updated. 2505968 web/download: Add FFmpeg 4.1.2

2019-03-28 Thread Tobias Rapp


On 23.03.2019 01:03, ffmpeg-...@ffmpeg.org wrote:

The branch, master has been updated
via  2505968f485fae32d7a68881eff0187f346adb61 (commit)
   from  b971570feed92970138b9234403d2ef213cf877e (commit)


- Log -
commit 2505968f485fae32d7a68881eff0187f346adb61
Author: Michael Niedermayer 
AuthorDate: Sat Mar 23 01:00:34 2019 +0100
Commit: Michael Niedermayer 
CommitDate: Sat Mar 23 01:01:26 2019 +0100

 web/download: Add FFmpeg 4.1.2
 
 Signed-off-by: Michael Niedermayer 


[...]


It looks like the big download button on http://ffmpeg.org/download.html 
still points to ffmpeg-4.1.tar.bz2 instead of ffmpeg-4.1.2.tar.bz2 .


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH]download: Fix the release link

2019-03-28 Thread Tobias Rapp


On 28.03.2019 15:02, Carl Eugen Hoyos wrote:

2019-03-28 15:00 GMT+01:00, Tobias Rapp :

On 28.03.2019 12:00, Carl Eugen Hoyos wrote:


Attached patch also simplifies the release process.



Personally I'd prefer to keep the link to the latest release.


Why? Such a link is already listed below.

I always found it extremely disturbing and I realize now
that it also was a (useless) maintenance burden.


I don't want to start a snapshot-vs-release discussion. Merging the 
prominent "Download" button with "Download Snapshot" is also fine for me.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/5] configure: Add an explicit check and option for nvcc

2019-02-26 Thread Tobias Rapp


On 26.02.2019 21:36, Soft Works wrote:



From: Jean-Baptiste Kempf
Sent: Dienstag, 26. Februar 2019 15:01

[...]

I don't think nvcc fit the"normally distributed with the operating system".


I'm not sure if the role of nvcc has been fully understood.
nvcc is some kind of 'pre-compiler' which is not distributed or linked to.

Distributing GPL code that was compiled with MSVC doesn't require the
MSVC compiler to be open source as well.


Yes, but there are alternative OSS compilers that implement the same 
programming language (at least to a very large extent). This doesn't 
seem to be the case here. So even if this scenario is not explicitly 
excluded in the GPL, if doesn't feel very free to me, too.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] doc: add missing hyphen prefix

2019-02-22 Thread Tobias Rapp


On 22.02.2019 12:43, Hendrik Leppkes wrote:

On Fri, Feb 22, 2019 at 12:29 PM Nicolas George  wrote:


Lou Logan (12019-02-21):

For consistency. Fixes #7740.

Signed-off-by: Lou Logan 


I do not think this is correct: the dash is not part of the option name,
it is part of the Unix command-line tradition. For consistency, it
should be removed when it is there, possibly replaced by the word
"option" if necessary.



I agree. You don't pass the dash when yo use avoptions through API,
for example, so this would only add to the confusion.


In my understanding the main source of documentation for API users are 
the Doxygen HTML output files while the Texinfo output is for both, 
command-line and API users -- with a slight bias towards command-line, 
at least from looking at all the ffmpeg/ffplay examples.


The current dash prefixes allow me to jump to specific options more 
easily (Ctrl+F "-b" for example). I agree that this is not a strong 
argument, but wanted to mention it anyway :-)


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] doc: add missing hyphen prefix

2019-02-22 Thread Tobias Rapp


On 22.02.2019 14:57, Nicolas George wrote:

Gyan (12019-02-22):

     '-key'  will be easier to search for these users as well. It's also a
low-cost arrangement. I trust adept API users will quickly suss out that the
hyphen represents CLI. GUI users won't be entering the key string, only the
values*, and casual CLI users will immediately recognize which entries
pertain to options.


That would be an argument for using something way more specific than a
dash.

Anyway, that kind of marking would belong in the definition of the macro
@option, not in the body of the documentation.


Yeah, that would be helpful.

Prior art for adding an invisible prefix to HTML markup for exactly this 
use-case: https://duktape.org/api.html


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] How to filter private folders from GIT patch

2019-03-08 Thread Tobias Rapp


On 08.03.2019 10:49, Ulf Zibis wrote:

[...]
Can some other developer please give me a practical hint how to deal
with private folders not to appear in GIT patches?


I'm using .git/info/exclude to ignore files that are only found within 
my private developing environment.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/af_asetnsamples: fix sample queuing.

2019-04-09 Thread Tobias Rapp


On 06.04.2019 02:11, Nikolas Bowe via ffmpeg-devel wrote:

When asetnsamples uses output samples < input samples, remaining samples build 
up in the fifo over time.
Fix this by marking the filter as ready again if there are enough samples.

Regression since ef3babb2c70f564dc1634b3f29c6e35a2b2dc239
[...]


Do you have a FFmpeg command-line at hand that triggers this bug? Maybe 
extending FATE tests would be a good idea.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/dvdec: add frame threads

2019-04-18 Thread Tobias Rapp


On 18.04.2019 11:40, Michael Niedermayer wrote:

On Thu, Apr 18, 2019 at 10:30:49AM +0200, Paul B Mahol wrote:

On 4/18/19, Michael Niedermayer  wrote:

On Thu, Apr 18, 2019 at 01:19:58AM +0200, Michael Niedermayer wrote:

On Wed, Apr 17, 2019 at 06:16:39PM +0200, Paul B Mahol wrote:

Signed-off-by: Paul B Mahol 
---
  libavcodec/dvdec.c | 19 ++-
  1 file changed, 10 insertions(+), 9 deletions(-)


Is this intended to be 100% same output ?

I have a few cases that produce differences. Dont have a good testcase
though
./ffmpeg -i ~/tickets/1589/A1590.dv avi-b.avi
produces different output.
oddly -f framecrc produces the same output
not sure this is a problem or not the input file may have issues


heres another testcase:
./ffmpeg -i ~/tickets/2390/dv_cut.dv aviavi2.avi

[...]


This is because audio is muxed differently.
This bug have nothing to do with this patch.


iam not sure thats the case. Looking at this again now in the morning
VPRP is different and the muxer receives a different par->field_order
(0 vs. 5). I dont think audio should afect the video field_order


That might be caused by frame-threading in general and not explicitly by 
the dv decoder. Have experienced field_order information not being 
reliably forwarded to the muxer in ffmpeg before. Seems sometimes the 
encoder settings are updated too late to be picked up by the muxer when 
writing header information.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavutil: add clean aperture (CLAP) side data.

2020-06-08 Thread Tobias Rapp


On 30.05.2020 12:41, Kieran O Leary wrote:

Hi,

On Fri 29 May 2020, 22:47 Neil Birkbeck,  wrote:


[...]
I've changed the side data to be PixelCrop (instead of CleanAperture) given
the intent to reuse as cropping elsewhere.
-For now, I've kept the rational representation--although CLAP seems to be
the only required case of it. Maybe Kieran could comment on the requirement
of having maintaining the rationals for CLAP (only works on mov to mov
transmuxing).


I'm no expert, but I think a lot of this just comes from video standards
that stipulate those rational numbers? I've cc'd tobias Rapp and Christoph
Gerstbauer of NOA just to bring this to their attention, as I'm pretty sure
that they use cropping values in AVI, so perhaps all of this could be
useful to them in some way.



Hi Kieran,

when digitizing SD video carriers we indeed store some offset 
information in case VBI is preverved (i.e. PAL 720x608). But these 
offset values are currently not stored in the AVI container itself. The 
OpenDML "vprp" chunk defines some offset values but for the purpose of 
compressed image data where the codec implies some multiple-of-N 
height/width dimension on the data. So it did not seem to match our 
use-case.


Besides AVI and the mentioned MKV and MOV formats I remember some 
display offset/cropping information being stored in MXF with the Display 
X/Y-Offset values.


Regardless whether the frame crop/offset values are stored as frame 
fields or side data: how would this information be affected by filters 
like "crop" or "scale"?


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] Bundling ffmpeg into windows application

2020-11-10 Thread Tobias Rapp


On 11.11.2020 06:50, Brian D. Pemberton wrote:

Hi,

I am thinking about writing an app that bundles or includes FFmpeg to do
some video processing. Is this allowed? If so, where can I read about the
constraints or requirements of doing this? Also, is there anything I should
know upfront to prepare my app licensing standpoint ahead of time?



You might want to take a look at http://ffmpeg.org/legal.html first. The 
page gives details about using FFmpeg under LGPL. Note that if you 
license your app under GPL you also have the possibility for linking 
FFmpeg libraries statically.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] Bundling ffmpeg into windows application

2020-11-12 Thread Tobias Rapp


On 11.11.2020 17:15, Carl Eugen Hoyos wrote:

Not necessarily related: In no way whatsoever does the LGPL mandate
dynamic linking (this was claimed several times lately).
The "License Compliance Checklist" on http://ffmpeg.org/legal.html lists 
dynamic linking under point 2.


I know that in theory you could link proprietary applications with LGPL 
libraries statically. But this is not very practical, neither for the 
application publisher nor for the user that wants to relink the library.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 3/3] avformat/mxfenc: prefer to use the existing metadata

2021-01-08 Thread Tobias Rapp


On 08.01.2021 11:01, lance.lmw...@gmail.com wrote:

On Fri, Jan 08, 2021 at 09:09:34AM +0100, Tobias Rapp wrote:

On 08.01.2021 07:32, lance.lmw...@gmail.com wrote:

From: Limin Wang 

Please check metadata with below command:
./ffmpeg -i ../fate-suite/mxf/Sony-1.mxf -c:v copy -c:a copy out.mxf
./ffmpeg -i out.mxf

  company_name: FFmpeg
  product_name: OP1a Muxer
  product_version : 58.65.101o
  =>
  company_name: SONY
  product_name: eVTR
  product_version : 1.00

So need to update fate-mxf fate test.

Signed-off-by: Limin Wang 
---
[...]


In my opinion the MXF identification set should contain data about the
application that wrote the current version of a file, not some previous file
version.


The example command shows what's change for the fate testing, if you want to
update to use  your own product version, please use -metadata 
product_version="x".


It looks wrong that a MXF file that is muxed by FFmpeg pretends to be 
written by a SONY application. I see that with "-codec copy" you can 
avoid re-encoding of the video and audio streams, and thus might want to 
indicate the A/V encoder software info somewhere separate from the 
container format writer software. But this patch overrides both information.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 3/3] avformat/mxfenc: prefer to use the existing metadata

2021-01-08 Thread Tobias Rapp


On 08.01.2021 07:32, lance.lmw...@gmail.com wrote:

From: Limin Wang 

Please check metadata with below command:
./ffmpeg -i ../fate-suite/mxf/Sony-1.mxf -c:v copy -c:a copy out.mxf
./ffmpeg -i out.mxf

 company_name: FFmpeg
 product_name: OP1a Muxer
 product_version : 58.65.101o
 =>
 company_name: SONY
 product_name: eVTR
 product_version : 1.00

So need to update fate-mxf fate test.

Signed-off-by: Limin Wang 
---
[...]


In my opinion the MXF identification set should contain data about the 
application that wrote the current version of a file, not some previous 
file version.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 3/3] avformat/mxfenc: prefer to use the configured metadta

2021-01-26 Thread Tobias Rapp


On 25.01.2021 20:40, emco...@ffastrans.com wrote:

[...]

What you can do instead is to push both identifications, the old one and 
the one from the current program into the identification array, this way 
the processing chain can be reconstructed. Unforutnately i have never 
seen anyone doing this besides Opencube.


This might work when your whole chain is built on a single container 
format (MXF). As soon as there is a mix of formats it is easier to store 
the processing chain information as a separate file.


PREMIS is some standard for doing this, but it has quite some overhead:
http://www.loc.gov/standards/premis/premis-mets.html
Not sure how popular it is.

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 3/3] avformat/mxfenc: prefer to use the configured metadta

2021-01-18 Thread Tobias Rapp


On 18.01.2021 23:53, Tomas Härdin wrote:

lör 2021-01-16 klockan 08:43 +0800 skrev lance.lmw...@gmail.com:

On Fri, Jan 15, 2021 at 09:43:58PM +0100, Marton Balint wrote:


On Fri, 15 Jan 2021, Tomas Härdin wrote:

Again, why? If you have a company that maintains a fork of FFmpeg then
compile that info in here instead. Compare with FFmbc which always puts
"FFmbc" as CompanyName.


And how can a product based on libavformat set the company name, product
name and product version? It seems a valid use case for me that these are
overridable. Also note that this product version is only the "user friendly"
version string, for the numeric version still LIBAVFORMAT_VERSION values are
used.


Yes, my use case is the product is using libavformat as library, so it's
prefer to have way to override these information as requirements.


What I'm worried about here is that we're going to get files which
claim to have been written by something other than libavformat. I've
had situations like this before, and it is a source of headache. For
example, if mxfenc writes some field incorrectly then this might cause
us to hack mxfdec to accept that field instead of fixing mxfenc.


I agree that especially for the MXF format with its flexible structure 
it is more relevant to know the muxing library rather than the hosting 
application. Have seen MXF output files of other commercial products 
that also contain library identifiers like "libMXF" or "MXFtk" here.


Other formats in FFmpeg use the "encoder" metadata key for embedding 
library information in the output file. A quick test with AVI output 
shows that this metadata is generated internally and cannot be 
overridden on the command-line.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

< 1 2 3 4 5 >

301 - 400 of 471 matches

Mail list logo