[FFmpeg-devel] [PATCH v3] avcodec/fft_template: improve performance of the ff_fft_init in fft_template

[Steven Liu]

Before patch:
init nbits = 17, get 1 samples, average cost: 16175 us
After patch:
init nbits = 17, get 1 samples, average cost: 14989 us

Signed-off-by: Steven Liu 
---
 libavcodec/fft_template.c | 46 +++---
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/libavcodec/fft_template.c b/libavcodec/fft_template.c
index 762c014bc8..20a62e4290 100644
--- a/libavcodec/fft_template.c
+++ b/libavcodec/fft_template.c
@@ -261,17 +261,41 @@ av_cold int ff_fft_init(FFTContext *s, int nbits, int 
inverse)
 if (s->fft_permutation == FF_FFT_PERM_AVX) {
 fft_perm_avx(s);
 } else {
-for(i=0; ifft_permutation == FF_FFT_PERM_SWAP_LSBS)
-j = (j&~3) | ((j>>1)&1) | ((j<<1)&2);
-k = -split_radix_permutation(i, n, s->inverse) & (n-1);
-if (s->revtab)
-s->revtab[k] = j;
-if (s->revtab32)
-s->revtab32[k] = j;
-}
+#define PROCESS_FFT_PERM_SWAP_LSBS(num) do {\
+for(i = 0; i < n; i++) {\
+int k;\
+j = i;\
+j = (j & ~3) | ((j >> 1) & 1) | ((j << 1) & 2);\
+k = -split_radix_permutation(i, n, s->inverse) & (n - 1);\
+s->revtab##num[k] = j;\
+} \
+} while(0);
+
+#define PROCESS_FFT_PERM_DEFAULT(num) do {\
+for(i = 0; i < n; i++) {\
+int k;\
+j = i;\
+k = -split_radix_permutation(i, n, s->inverse) & (n - 1);\
+s->revtab##num[k] = j;\
+} \
+} while(0);
+
+#define SPLIT_RADIX_PERMUTATION(num) do { \
+if (s->fft_permutation == FF_FFT_PERM_SWAP_LSBS) {\
+PROCESS_FFT_PERM_SWAP_LSBS(num) \
+} else {\
+PROCESS_FFT_PERM_DEFAULT(num) \
+}\
+} while(0);
+
+if (s->revtab)
+SPLIT_RADIX_PERMUTATION()
+if (s->revtab32)
+SPLIT_RADIX_PERMUTATION(32)
+
+#undef PROCESS_FFT_PERM_DEFAULT
+#undef PROCESS_FFT_PERM_SWAP_LSBS
+#undef SPLIT_RADIX_PERMUTATION
 }
 
 return 0;
-- 
2.15.2 (Apple Git-101.1)



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

Re: [FFmpeg-devel] [PATCH 2/3] avcodec/lagarith: Optimize case with singleton probability distribution

[Michael Niedermayer]

On Tue, Dec 25, 2018 at 04:43:24PM +0100, Paul B Mahol wrote:
> On 12/25/18, Michael Niedermayer  wrote:
> > On Mon, Dec 24, 2018 at 11:54:31PM +, Kieran Kunhya wrote:
> >> >
> >> > commit 0ca7a8deeffd33e05ae15a447259b32b6678c727 (HEAD -> master)
> >> > Author: Michael Niedermayer 
> >> > Date:   Mon Dec 24 01:14:50 2018 +0100
> >> >
> >> > avcodec/lagarith: Optimize case with singleton probability
> >> > distribution
> >> >
> >> > Fixes: Timeout
> >> > Fixes:
> >> > 10554/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_LAGARITH_fuzzer-5739938067251200
> >> >
> >> > In case of a Denial of Service attack, the attacker wants to
> >> > maximize
> >> > the load on the target
> >> > per byte transmitted from the attacker.
> >> > For such a DoS attack it is best for the attacker to setup the
> >> > probabilities so that the
> >> > arithmetic decoder does not advance in the bytestream that way the
> >> > attacker only needs to
> >> > transmit the initial bytes and header for an arbitrary large frame.
> >> > This patch here optimizes this codepath and avoids executing the
> >> > arithmetic decoder more than
> >> > once. It thus reduces the load causes by this codepath on the
> >> > target.
> >> > We also could completely disallow this codepath but it appears such
> >> > odd probability
> >> > distributions are not invalid.
> >> >
> >> > Found-by: continuous fuzzing process
> >> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >> > Signed-off-by: Michael Niedermayer 
> >> >
> >>
> >> This is a nonsense argument, a user could send a frame that was
> >> x in dimensions, would have the same effect.
> >
> > This suggested attack would not work, a user wanting to minimize these
> > DoS issues would have set AVCodecContext.max_pixels which would block this.
> >
> >
> >> The calling application should manage timeouts themselves in a sandbox or
> >> container or similar.
> >
> > Its always possible and also a very good idea to have a 2nd line of defense
> > like a sandbox / VM, ... as you suggest here, I did and do agree here.
> > And also a 3rd line of defense, ...
> >
> > But this doesnt mean we should not attempt to fix or mitigate
> > security (or other) issues directly in the code.
> >
> > I think the point you are raising has been raised previously, so let me
> > argue a little broader here and not specific to just what you suggest.
> >
> > If you compare a native fix in the code with a fix by a timeout, a
> > fix by a timeout causes:
> > * The whole process to be killed, so any application using libavcodec
> >   would basically "crash" and would not neccessarily save its state,
> >   flush out buffers, write any trailers or do proper protocol shutdowns
> >   or save any unsafed data. This is a outcome that should be minimized
> > * Using a timeout as the main way to block DoS is difficult as there is
> >   often no good timeout value. Its not unexpected that a system may need to
> >   support processing large videos taking several hours, thats a long
> >   time for a file of a hundread bytes in a DoS attack
> >   100bytes from an attacker could cause the same load as 10 bytes
> > from
> >   a user.
> > * Worst maybe is that a tight timeout likely makes the system more
> > vulnerable
> >   not less. because during an attack all the processes would likely slow
> >   down and real users would be pushed into the timeout even if the system
> >   without the user processes timeouting would still function correctly
> >
> > Iam sure there are more arguments but above are the ones that came to my
> > mind
> > ATM.
> >
> >>
> >> Merry Xmas.
> >
> > Merry Xmas as well!
> >
> > [...]
> > --
> > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > Those who would give up essential Liberty, to purchase a little
> > temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin
> >
> 
> This is still missing numbers/statistics.

Before: Executed 
clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_LAGARITH_fuzzer-5739938067251200
 in 27400 ms
After:  Executed 
clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_LAGARITH_fuzzer-5739938067251200
 in 6676 ms

(will add this to the commit message)


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/hls.c: Properly free prev_segments dynarray after playlist parsing

[Liu Steven]

> 在 2018年12月24日，下午5:24，Valery Kot  写道：
> 
> Updated commit message for hls.c memory leak fix
> <0001-avformat-hls.c-Properly-free-prev_segments-dynarray-.txt>___

Pushed

Thanks
Steven
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



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

Re: [FFmpeg-devel] [PATCH v2] lavc/libdavs2: enable multithread

[Steven Liu]

hwrenx  于2018年12月15日周六 上午11:41写道：
>
> From: hwrenx 
>
> Signed-off-by: hwrenx 
> ---
>  libavcodec/libdavs2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c
> index 37635bb..2846ecf 100644
> --- a/libavcodec/libdavs2.c
> +++ b/libavcodec/libdavs2.c
> @@ -205,7 +205,7 @@ AVCodec ff_libdavs2_decoder = {
>  .init   = davs2_init,
>  .close  = davs2_end,
>  .decode = davs2_decode_frame,
> -.capabilities   =  AV_CODEC_CAP_DELAY,//AV_CODEC_CAP_DR1 |
> +.capabilities   =  AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AUTO_THREADS,
>  .pix_fmts   = (const enum AVPixelFormat[]) { AV_PIX_FMT_YUV420P,
>   AV_PIX_FMT_NONE },
>  .wrapper_name   = "libdavs2",
> --
> 2.7.4
>
>
>

Pushed


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

Re: [FFmpeg-devel] [PATCH v3] Fix usage of temp_file flag in hls_flags option

[Liu Steven]

> 在 2018年12月20日，上午12:19，Aleksey Skripka  写道：
> 
> All my tests ok.
> Thank you, Adrian.
> 
>> On 19 Dec 2018, at 12:41, Adrian  wrote:
>> 
>> Okay, so given you two have some valid points, I've combined both and ended 
>> up with final version of the patch:
>> - it uses temp file for playlist when either a flag is specified or playlist 
>> type is anything other than VOD
>> - it does not use temp file for HLS_SINGLE_FILE in place mentioned by Aleksey
>> 
>> This is my final submission, I believe now everyone should be happy with the 
>> solution.
>> 
>> Regards
>> Adrian Guzowski
>> 
>> W dniu 18.12.2018 o 04:24, Ronak pisze:
 On Dec 17, 2018, at 4:35 PM, Aleksey Skripka  wrote:
 
 Evening!
 
 First of all, about playlist writeout:
 before 223d2bde22ce33dcbcb6f17f234b609cb98f1fb6 - playlist was always(!) 
 creating via .tmp file.
 after 223d2bde22ce33dcbcb6f17f234b609cb98f1fb6 - playlists .tmp logic 
 become dependent on +temp_file flag.
 
 I suggest to return to original logic.
 Non-atomic playlist writeout - is a disaster to just any hls player. This 
 way we will force everyone (except offline VOD) to start using +temp_file. 
 Always!
 Also, if somebody relying on this - they will get broken streams just 
 after upgrade (It seems to me, that it can be a lot).
 
 So, here maintainer should decide how to be.
 
 
 and about single_file:
 if it will be decided to stay with current way (playlist's .tmp logic 
 controlled by +temp_file flag), you are right - temp_file+single_file case 
 become unreal.
 if it will be decided to return original behaviour (playlist always via 
 .tmp), someone can choose how to writeout media file (despite i understand 
 temp+single is very-very rare case).
 
 Thanks!
 -- 
 Aleksey Skripk
 
>>> Hey Aleksey,
>>> 
>>> I'm not sure the best thing to do here is to switch to the original logic. 
>>> The temp file is only necessary if you are doing LIVE or EVENT streams; it 
>>> has nothing to do with VOD; where streams are pre-generated. I don't see 
>>> why you would ever want tmp files there.
>>> Before this commit; ffmpeg used to take more than a week to fragment a 2.2 
>>> GB Audio File in VOD mode. With this change; it takes less than 1 minute. 
>>> Other competing tools take less than 1 minute.
>>> 
>>> Also, if you want temp files, you should be supplying the temp_file option 
>>> - the documentation clearly says that: 
>>> https://www.ffmpeg.org/ffmpeg-all.html#hls-2. If you do not want temp 
>>> files; then you should leave this option out. If you are forcing folks to 
>>> always accept the temp file, then why have an option? You should delete the 
>>> option and do it based on the playlist type.
>>> 
>>> Ronak
>>> 
> On 17 Dec 2018, at 19:12, Adrian  > wrote:
> 
> Comments inline.
> 
> Regards
> Adrian Guzowski
> 
> W dniu 17.12.2018 o 16:56, Aleksey Skripka pisze:
>>> On 17 Dec 2018, at 18:45, Adrian >>  >> >> wrote:
>>> 
>>> IMO temp_file+single_file should be mutually exclusive - why would you 
>>> want to have temp file when you're using only one file anyway? In this 
>>> mode you would either pool for file changes or just get updated 
>>> playlist with new byte range (and that one should be updated after 
>>> video file). I believe this restriction was already in place anyway, 
>>> because there already were checks for HLS_SINGLE_FILE not being set, 
>>> before doing actual rename, so this was just a simplification of those 
>>> conditions.
>> not me to decide, please ;)
>> if playlist will always be done via .tmp, we will have a choice.
> Could you clarify what choice you're talking about? And the purpose of 
> this choice - frankly you got me lost with this. Regardless, I feel we're 
> getting off topic.
>>> As for playlist file - I'm not sure I follow, I've changed only places 
>>> where temp files were already used, so I don't think I broke something 
>>> in that matter.
>> not you, it was introduced by 223d2bde22ce33dcbcb6f17f234b609cb98f1fb6
> Not quite, temp files were already a feature, this commit narrowed 
> conditions where they are used. If you're referring to change in 
> hls_window about not checking TEMP flag - that is unrelated to current 
> state (before commit introducing regression, check was ignoring TEMP 
> flag, after commit it does take it into account - probably should be 
> discussed with person responsible for original change as to why it was 
> changed that way, so that's out of scope of this patch. Issue at hand is 
> not using TEMP files at all and that is fixed by this patch.
> 
>>> From repeat+level+trace I believe I saw that playlist file actually was 
>

Re: [FFmpeg-devel] [PATCH] avcodec: add AV1 frame split bitstream filter

[James Almer]

On 12/17/2018 9:56 PM, James Almer wrote:
> This will be needed by the eventual native AV1 decoder.
> 
> Signed-off-by: James Almer 
> ---
> Missing Changelog entry, version bump
> 
>  configure|   1 +
>  libavcodec/Makefile  |   1 +
>  libavcodec/av1_frame_split_bsf.c | 246 +++
>  libavcodec/bitstream_filters.c   |   1 +
>  4 files changed, 249 insertions(+)
>  create mode 100644 libavcodec/av1_frame_split_bsf.c

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

Re: [FFmpeg-devel] [PATCH] avcodec/mjpeg: don't override color information from demuxer

[Steinar H. Gunderson]

On Wed, Dec 05, 2018 at 09:31:48AM +0100, Steinar H. Gunderson wrote:
>> We don't have a solution for that, and its really something the user
>> application currently has to solve.
> The user application doesn't feel like the right place for this. How would it
> be in a better position than FFmpeg to know? And if it really is in the
> position to set a policy here, it can read out the information of the codec
> parameter struct and override it, no matter what libavcodec decided to do.

What's the status of this patch? Is it rejected on the basis of “this is the
wrong thing to do” and nedes a larger rework of the rest of FFmpeg, or could it
be merged with some changes?

/* Steinar */
-- 
Homepage: https://www.sesse.net/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Add a new channel layout API

[Nicolas George]

Paul B Mahol (2018-12-24):
> The new API is more extensible and allows for custom layouts.
> More accurate information is exported, eg for decoders that do not
> set a channel layout, lavc will not make one up for them.
> 
> Deprecate the old API working with just uint64_t bitmasks.
> 
> Original commit by Anton Khirnov .
> Expanded and completed by Vittorio Giovara .

> Adopted for FFmpeg by Paul B Mahol .

Adapted?

Reviewing only the visible API for now.

> diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
> index 50bb8f03c5..e6c8c58b9c 100644
> --- a/libavutil/channel_layout.h
> +++ b/libavutil/channel_layout.h
> @@ -24,6 +24,9 @@
>  
>  #include 
>  
> +#include "version.h"
> +#include "attributes.h"
> +
>  /**
>   * @file
>   * audio channel layout utility functions
> @@ -34,6 +37,60 @@
>   * @{
>   */
>  
> +enum AVChannel {
> +AV_CHAN_FRONT_LEFT,
> +AV_CHAN_FRONT_RIGHT,
> +AV_CHAN_FRONT_CENTER,
> +AV_CHAN_LOW_FREQUENCY,
> +AV_CHAN_BACK_LEFT,
> +AV_CHAN_BACK_RIGHT,
> +AV_CHAN_FRONT_LEFT_OF_CENTER,
> +AV_CHAN_FRONT_RIGHT_OF_CENTER,
> +AV_CHAN_BACK_CENTER,
> +AV_CHAN_SIDE_LEFT,
> +AV_CHAN_SIDE_RIGHT,
> +AV_CHAN_TOP_CENTER,
> +AV_CHAN_TOP_FRONT_LEFT,
> +AV_CHAN_TOP_FRONT_CENTER,
> +AV_CHAN_TOP_FRONT_RIGHT,
> +AV_CHAN_TOP_BACK_LEFT,
> +AV_CHAN_TOP_BACK_CENTER,
> +AV_CHAN_TOP_BACK_RIGHT,
> +/** Stereo downmix. */
> +AV_CHAN_STEREO_LEFT = 29,
> +/** See above. */
> +AV_CHAN_STEREO_RIGHT,
> +AV_CHAN_WIDE_LEFT,
> +AV_CHAN_WIDE_RIGHT,
> +AV_CHAN_SURROUND_DIRECT_LEFT,
> +AV_CHAN_SURROUND_DIRECT_RIGHT,
> +AV_CHAN_LOW_FREQUENCY_2,
> +
> +/** Channel is empty can be safely skipped. */
> +AV_CHAN_SILENCE = 64,
> +};
> +
> +enum AVChannelOrder {
> +/**
> + * The native channel order, i.e. the channels are in the same order in
> + * which they are defined in the AVChannel enum. This supports up to 63
> + * different channels.
> + */
> +AV_CHANNEL_ORDER_NATIVE,
> +/**
> + * The channel order does not correspond to any other predefined order 
> and
> + * is stored as an explicit map. For example, this could be used to 
> support
> + * layouts with 64 or more channels, or with channels that could be 
> skipped.
> + */
> +AV_CHANNEL_ORDER_CUSTOM,
> +/**
> + * Only the channel count is specified, without any further information
> + * about the channel order.
> + */
> +AV_CHANNEL_ORDER_UNSPEC,
> +};
> +
> +
>  /**
>   * @defgroup channel_masks Audio channel masks
>   *
> @@ -46,36 +103,41 @@
>   *
>   * @{
>   */
> -#define AV_CH_FRONT_LEFT 0x0001
> -#define AV_CH_FRONT_RIGHT0x0002
> -#define AV_CH_FRONT_CENTER   0x0004
> -#define AV_CH_LOW_FREQUENCY  0x0008
> -#define AV_CH_BACK_LEFT  0x0010
> -#define AV_CH_BACK_RIGHT 0x0020
> -#define AV_CH_FRONT_LEFT_OF_CENTER   0x0040
> -#define AV_CH_FRONT_RIGHT_OF_CENTER  0x0080
> -#define AV_CH_BACK_CENTER0x0100
> -#define AV_CH_SIDE_LEFT  0x0200
> -#define AV_CH_SIDE_RIGHT 0x0400
> -#define AV_CH_TOP_CENTER 0x0800
> -#define AV_CH_TOP_FRONT_LEFT 0x1000
> -#define AV_CH_TOP_FRONT_CENTER   0x2000
> -#define AV_CH_TOP_FRONT_RIGHT0x4000
> -#define AV_CH_TOP_BACK_LEFT  0x8000
> -#define AV_CH_TOP_BACK_CENTER0x0001
> -#define AV_CH_TOP_BACK_RIGHT 0x0002
> -#define AV_CH_STEREO_LEFT0x2000  ///< Stereo downmix.
> -#define AV_CH_STEREO_RIGHT   0x4000  ///< See AV_CH_STEREO_LEFT.
> -#define AV_CH_WIDE_LEFT  0x8000ULL
> -#define AV_CH_WIDE_RIGHT 0x0001ULL
> -#define AV_CH_SURROUND_DIRECT_LEFT   0x0002ULL
> -#define AV_CH_SURROUND_DIRECT_RIGHT  0x0004ULL
> -#define AV_CH_LOW_FREQUENCY_20x0008ULL
> +#define AV_CH_FRONT_LEFT (1ULL << AV_CHAN_FRONT_LEFT   )
> +#define AV_CH_FRONT_RIGHT(1ULL << AV_CHAN_FRONT_RIGHT  )
> +#define AV_CH_FRONT_CENTER   (1ULL << AV_CHAN_FRONT_CENTER )
> +#define AV_CH_LOW_FREQUENCY  (1ULL << AV_CHAN_LOW_FREQUENCY)
> +#define AV_CH_BACK_LEFT  (1ULL << AV_CHAN_BACK_LEFT)
> +#define AV_CH_BACK_RIGHT (1ULL << AV_CHAN_BACK_RIGHT   )
> +#define AV_CH_FRONT_LEFT_OF_CENTER   (1ULL << AV_CHAN_FRONT_LEFT_OF_CENTER )
> +#define AV_CH_FRONT_RIGHT_OF_CENTER  (1ULL << AV_CHAN_FRONT_RIGHT_OF_CENTER)
> +#define AV_CH_BACK_CENTER(1ULL << AV_CHAN_BACK_CENTER  )
> +#define AV_CH_SIDE_LEFT  (1ULL << AV_CHAN_SIDE_LEFT)
> +#define AV_CH_SIDE_RIGHT (1ULL << AV_CHAN_SIDE_RIGHT   )
> +#define AV_CH_TOP_CENTER (1ULL << AV_CHAN_TOP_CENTER   )
> +#define AV

[FFmpeg-devel] [PATCH] delogo filter: new "uglarm" interpolation mode added

[Uwe Freese]

Hello,

so now I've taken the time to integrate the alternative interpolation 
algorithm 'uglarm' into the delogo filter.


I took the code from ffdshow's logoaway, which took over the code from 
LogoAway for VirtualDub, where I originally added it 2001. :)


I tested the code with several different options and had temporarily 
added many debug outputs (to the console) to see the internally 
calculated values. So I'm pretty confident about the correct functioning 
of the code. I also made some improvements compared to the logoaway 
filter to minimize the lookup tables (which was also a reason for me to 
test it thoroughly).


Of course, it would be nice if the addition will now also find it's way 
into the normal development branch.


The current "xy" mode tends to flicker and show horizontal and vertical 
lines, which sometimes produces an "eye catcher" at the logo area. In 
contrast, the new UGLARM mode IMHO shows a much more non-distracting 
blurred area while keeping the same simple usage (only entering the 
coordinates of the logo).


Can someone please review and add the code? Who would be primarily 
responsible to add this to libavfilter?


You can test the filter (after compiling with the attached patch for 
vf_delogo.c) with the following command to show the old xy mode and the 
new uglarm mode next to each other by:


./ffmpeg -i "yourtestvideo.mkv" -map 0:v:0 -vframes 100 -filter_complex 
"crop=200:200:100:100,split=2[L][R];[L]delogo=x=50:y=50:w=100:h=100[L2];[R]delogo=x=50:y=50:w=100:h=100:mode=uglarm:power=15[R2];[L2][R2]hstack=inputs=2" 
-c:v huffyuv delogotest.mkv && ffplay delogotest.mkv


(power can be from 0 to 30, 15 is default and need not be specified)

Here are some details of what I added and how / why:

- a new parameter "mode" is used to select the mode
  Note: It defaults to the current xy mode.
  Note: I preferred using a string / enum parameter instead of a 
boolean so the parameter could stay backwards compatible in case another 
mode would be added some day.

- a "power" value can be used to set the strength of the effect
  Note: Named "power" (and not "strength") because the distance between 
logo pixels to the border is calculated by using the power of the 
diagonal distance.
  Note: The default value results in exponent 3 (cubic), which 
corresponds to how e.g. a magnetic field is behaving and what I used all 
the years for logoaway.
- Two lookup tables (per plane) are generated at startup (if the new 
mode is set).

- Memory is cleaned up in the new "uninit" function.
- The SAR (sample aspect ratio) is considered to calculate the 
interpolated inner pixels of the logo area.
- As I saw, the filter is applied to three planes, I think it's for Y U 
V, because of the YUV color model.
  Question: How many planes can there be? I used an array of pointers 
to 10 possible lookup tables (one per plane).
  Can there be more? Are 3 enough? Is there a constant to get the max. 
number of planes at compile time?
- Only a band / border of 1 pixel is used for interpolation. This is 
because the "band" option in the delogo filter is planned to be removed 
anyway. It's also not necessary for the new mode.
- I added comments and description as I thought would be useful to 
understand the code. I hope it's ok as it's done.
- I tried to use the same formatting etc. as in the original source 
code. Is there anything I have to change?


I would be really glad for any feedback, comments, improvement 
ideas,..., and of course a review and integration of the code. :)


Of course, I would support adding documentation about the filter as 
well. Just let me know what's needed.


Best Regards,
Uwe

Am 13.12.18 um 07:55 schrieb uwe.fre...@gmx.de:

Hello Kyle,

You should also try vf_removelogo. It is supposedly better than the
more simple vf_delogo.


I tried (with a box in a PNG mask as logo cover).

That is no alternative both quality wise and from usability point of 
view:


1. It gave me blurry borders with several pixels widths from outside 
to the inner. They were very symmetrical and showed a 45 degree 
"lines" from the edges to the middle. Hardly the nice blurred box I 
wanted.


2. If you want to cover the logo by simple coordinates (a box), it's 
unnecessary effort to create a PNG mask with that box in it instead of 
giving the coordinates as parameter.



Because removelogo can handle arbitrary masks and the calculation I 
posted is using a simple rectangular box (and this is my typical use 
case), I suggested to add it to the "delogo" filter.


So my offer stands, would be nice if someone can add this additional 
mode.

[...]
>From 9431911dbef4179663f1fc30efc4b37b696ed7d9 Mon Sep 17 00:00:00 2001
From: breaker27 
Date: Wed, 26 Dec 2018 14:37:35 +0100
Subject: [PATCH] Add new delogo interpolation mode uglarm.

---
 libavfilter/vf_delogo.c | 330 
 1 file changed, 247 insertions(+), 83 deletions(-)

diff --git a/libavfilter/vf_delogo.c b/libavfil

Re: [FFmpeg-devel] [PATCH] delogo filter: new "uglarm" interpolation mode added

[Carl Eugen Hoyos]

> Am 26.12.2018 um 15:23 schrieb Uwe Freese :

> so now I've taken the time to integrate the alternative interpolation 
> algorithm 'uglarm' into the delogo filter.

Please do not re-indent existing code in this patch as this makes reviewing the 
changes more difficult and please do not add trailing white space as it cannot 
be committed to our git repository.

(I did not check if you are possibly changing the license of code you did not 
write yourself, please be extra careful to make sure this does not happen.)

Thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Add a new channel layout API

[Paul B Mahol]

On 12/26/18, Nicolas George  wrote:
> Paul B Mahol (2018-12-24):
>> The new API is more extensible and allows for custom layouts.
>> More accurate information is exported, eg for decoders that do not
>> set a channel layout, lavc will not make one up for them.
>>
>> Deprecate the old API working with just uint64_t bitmasks.
>>
>> Original commit by Anton Khirnov .
>> Expanded and completed by Vittorio Giovara .
>
>> Adopted for FFmpeg by Paul B Mahol .
>
> Adapted?
>
> Reviewing only the visible API for now.
>
>> diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
>> index 50bb8f03c5..e6c8c58b9c 100644
>> --- a/libavutil/channel_layout.h
>> +++ b/libavutil/channel_layout.h
>> @@ -24,6 +24,9 @@
>>
>>  #include 
>>
>> +#include "version.h"
>> +#include "attributes.h"
>> +
>>  /**
>>   * @file
>>   * audio channel layout utility functions
>> @@ -34,6 +37,60 @@
>>   * @{
>>   */
>>
>> +enum AVChannel {
>> +AV_CHAN_FRONT_LEFT,
>> +AV_CHAN_FRONT_RIGHT,
>> +AV_CHAN_FRONT_CENTER,
>> +AV_CHAN_LOW_FREQUENCY,
>> +AV_CHAN_BACK_LEFT,
>> +AV_CHAN_BACK_RIGHT,
>> +AV_CHAN_FRONT_LEFT_OF_CENTER,
>> +AV_CHAN_FRONT_RIGHT_OF_CENTER,
>> +AV_CHAN_BACK_CENTER,
>> +AV_CHAN_SIDE_LEFT,
>> +AV_CHAN_SIDE_RIGHT,
>> +AV_CHAN_TOP_CENTER,
>> +AV_CHAN_TOP_FRONT_LEFT,
>> +AV_CHAN_TOP_FRONT_CENTER,
>> +AV_CHAN_TOP_FRONT_RIGHT,
>> +AV_CHAN_TOP_BACK_LEFT,
>> +AV_CHAN_TOP_BACK_CENTER,
>> +AV_CHAN_TOP_BACK_RIGHT,
>> +/** Stereo downmix. */
>> +AV_CHAN_STEREO_LEFT = 29,
>> +/** See above. */
>> +AV_CHAN_STEREO_RIGHT,
>> +AV_CHAN_WIDE_LEFT,
>> +AV_CHAN_WIDE_RIGHT,
>> +AV_CHAN_SURROUND_DIRECT_LEFT,
>> +AV_CHAN_SURROUND_DIRECT_RIGHT,
>> +AV_CHAN_LOW_FREQUENCY_2,
>> +
>> +/** Channel is empty can be safely skipped. */
>> +AV_CHAN_SILENCE = 64,
>> +};
>> +
>> +enum AVChannelOrder {
>> +/**
>> + * The native channel order, i.e. the channels are in the same order
>> in
>> + * which they are defined in the AVChannel enum. This supports up to
>> 63
>> + * different channels.
>> + */
>> +AV_CHANNEL_ORDER_NATIVE,
>> +/**
>> + * The channel order does not correspond to any other predefined
>> order and
>> + * is stored as an explicit map. For example, this could be used to
>> support
>> + * layouts with 64 or more channels, or with channels that could be
>> skipped.
>> + */
>> +AV_CHANNEL_ORDER_CUSTOM,
>> +/**
>> + * Only the channel count is specified, without any further
>> information
>> + * about the channel order.
>> + */
>> +AV_CHANNEL_ORDER_UNSPEC,
>> +};
>> +
>> +
>>  /**
>>   * @defgroup channel_masks Audio channel masks
>>   *
>> @@ -46,36 +103,41 @@
>>   *
>>   * @{
>>   */
>> -#define AV_CH_FRONT_LEFT 0x0001
>> -#define AV_CH_FRONT_RIGHT0x0002
>> -#define AV_CH_FRONT_CENTER   0x0004
>> -#define AV_CH_LOW_FREQUENCY  0x0008
>> -#define AV_CH_BACK_LEFT  0x0010
>> -#define AV_CH_BACK_RIGHT 0x0020
>> -#define AV_CH_FRONT_LEFT_OF_CENTER   0x0040
>> -#define AV_CH_FRONT_RIGHT_OF_CENTER  0x0080
>> -#define AV_CH_BACK_CENTER0x0100
>> -#define AV_CH_SIDE_LEFT  0x0200
>> -#define AV_CH_SIDE_RIGHT 0x0400
>> -#define AV_CH_TOP_CENTER 0x0800
>> -#define AV_CH_TOP_FRONT_LEFT 0x1000
>> -#define AV_CH_TOP_FRONT_CENTER   0x2000
>> -#define AV_CH_TOP_FRONT_RIGHT0x4000
>> -#define AV_CH_TOP_BACK_LEFT  0x8000
>> -#define AV_CH_TOP_BACK_CENTER0x0001
>> -#define AV_CH_TOP_BACK_RIGHT 0x0002
>> -#define AV_CH_STEREO_LEFT0x2000  ///< Stereo downmix.
>> -#define AV_CH_STEREO_RIGHT   0x4000  ///< See
>> AV_CH_STEREO_LEFT.
>> -#define AV_CH_WIDE_LEFT  0x8000ULL
>> -#define AV_CH_WIDE_RIGHT 0x0001ULL
>> -#define AV_CH_SURROUND_DIRECT_LEFT   0x0002ULL
>> -#define AV_CH_SURROUND_DIRECT_RIGHT  0x0004ULL
>> -#define AV_CH_LOW_FREQUENCY_20x0008ULL
>> +#define AV_CH_FRONT_LEFT (1ULL << AV_CHAN_FRONT_LEFT
>>  )
>> +#define AV_CH_FRONT_RIGHT(1ULL << AV_CHAN_FRONT_RIGHT
>>  )
>> +#define AV_CH_FRONT_CENTER   (1ULL << AV_CHAN_FRONT_CENTER
>>  )
>> +#define AV_CH_LOW_FREQUENCY  (1ULL << AV_CHAN_LOW_FREQUENCY
>>  )
>> +#define AV_CH_BACK_LEFT  (1ULL << AV_CHAN_BACK_LEFT
>>  )
>> +#define AV_CH_BACK_RIGHT (1ULL << AV_CHAN_BACK_RIGHT
>>  )
>> +#define AV_CH_FRONT_LEFT_OF_CENTER   (1ULL <<
>> AV_CHAN_FRONT_LEFT_OF_CENTER )
>> +#define AV_CH_FRONT_RIGHT_OF_CENTER  (1ULL <<
>> AV_CHAN_FRONT_RIGHT_OF_CENTER)
>> +#define AV_CH_BACK_CENTER(1ULL << AV_CHAN_BACK_CENTER
>>  )
>> +#define AV_CH_SIDE_LEFT  (1ULL << AV_CHAN_SIDE_LEFT
>>  )
>> +#define AV_CH_SIDE_RIGHT 

Re: [FFmpeg-devel] [PATCH] Add a new channel layout API

[Nicolas George]

Paul B Mahol (2018-12-26):
> Look folk, I'm not paid to do this nor I'm paid to read your "reviews"

Neither am I. What does it have to do with anything?

> so I will ignore this one.

You are not allowed to do that. FFmpeg is not your personal project, it
is a collective endeavour governed by consensus. This patch cannot go in
until consensus is reached, and consensus cannot be reached unless you
take reviews into account.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Add a new channel layout API

[James Almer]

On 12/26/2018 12:07 PM, Paul B Mahol wrote:
> On 12/26/18, Nicolas George  wrote:
>>> +
>>> +/**
>>> + * Initialize a native channel layout from a bitmask indicating which
>>> channels
>>> + * are present.
>>> + *
>>> + * @note channel_layout should be properly allocated as described above.
>>> + *
>>> + * @param channel_layout the layout structure to be initialized
>>> + * @param mask bitmask describing the channel layout
>>> + */
>>> +void av_channel_layout_from_mask(AVChannelLayout *channel_layout,
>>> uint64_t mask);
>>> +
>>> +/**
>>> + * Initialize a channel layout from a given string description.
>>
>>> + * The input string can be represented by:
>>> + *  - the formal channel layout name (returned by
>>> av_channel_layout_describe())
>>> + *  - single or multiple channel names (returned by av_channel_name()
>>> + *or concatenated with "|")
>>> + *  - a hexadecimal value of a channel layout (eg. "0x4")
>>> + *  - the number of channels with default layout (eg. "5")
>>> + *  - the number of unordered channels (eg. "4 channels")
>>
>> av_get_channel_layout() used to use '+' instead of '|', and I think it
>> is better. For once, '+' is not a special character for shells.
> 
> Look folk, I'm not paid to do this nor I'm paid to read your "reviews"
> so I will ignore this one.
What prompted you to reply this way? Was there a need to be this
aggressive with a review?

What do you or anyone wins with this?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avutil/imgutils: Optimize writing 4 bytes in memset_bytes()

[Paul B Mahol]

On 12/25/18, Michael Niedermayer  wrote:
> Fixes: Timeout
> Fixes:
> 11502/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> Before: Executed
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> in 11294 ms
> After : Executed
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> in 4249 ms
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavutil/imgutils.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> index 4938a7ef67..cc38f1e878 100644
> --- a/libavutil/imgutils.c
> +++ b/libavutil/imgutils.c
> @@ -529,6 +529,12 @@ static void memset_bytes(uint8_t *dst, size_t dst_size,
> uint8_t *clear,
>  }
>  } else if (clear_size == 4) {
>  uint32_t val = AV_RN32(clear);
> +uint64_t val8 = val * 0x10001ULL;
> +for (; dst_size >= 32; dst_size -= 32) {
> +AV_WN64(dst   , val8); AV_WN64(dst+ 8, val8);
> +AV_WN64(dst+16, val8); AV_WN64(dst+24, val8);
> +dst += 32;
> +}
>  for (; dst_size >= 4; dst_size -= 4) {
>  AV_WN32(dst, val);
>  dst += 4;
> --
> 2.20.1
>

NAK, implement special memset function instead.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] delogo filter: new "uglarm" interpolation mode added

[Uwe Freese]

Hello,

so now I've taken the time to integrate the alternative interpolation algorithm 
'uglarm' into the delogo filter.

Please do not re-indent existing code in this patch as this makes reviewing the 
changes more difficult


Parts of the previous code are now in an "if" statement block (function 
apply_delogo), so indentation is correct. Even it would maybe be better 
to read the diff, it wouldn't be correctly indented.


In the parameter lists, spaces are added to have the same layout of the 
descriptions in the right part for all lines.


So what should I do in this case?

I could create a diff of course with "wrong" indentation (not correctly 
indented and not good to read in the final file, but the diff is better 
to read...). Would this help for the first review?



  and please do not add trailing white space as it cannot be committed to our 
git repository.


OK, I'll remove them, no problem. (I'll send a new patch to this list 
after I know how to handle also the first point.)



(I did not check if you are possibly changing the license of code you did not 
write yourself, please be extra careful to make sure this does not happen.)


The TimgFilterLogoaway.cpp from ffdshow also has GPL 2 and up, the same 
as vf_delogo.c from ffmpeg. -> OK.


https://sourceforge.net/p/ffdshow-tryout/code/HEAD/tree/trunk/src/imgFilters/TimgFilterLogoaway.cpp

Regards,
Uwe

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

Re: [FFmpeg-devel] [PATCH] delogo filter: new "uglarm" interpolation mode added

[Nicolas George]

Uwe Freese (2018-12-26):
> Parts of the previous code are now in an "if" statement block (function
> apply_delogo), so indentation is correct. Even it would maybe be better to
> read the diff, it wouldn't be correctly indented.
> 
> In the parameter lists, spaces are added to have the same layout of the
> descriptions in the right part for all lines.
> 
> So what should I do in this case?
> 
> I could create a diff of course with "wrong" indentation (not correctly
> indented and not good to read in the final file, but the diff is better to
> read...). Would this help for the first review?

The practice in FFmpeg for this kind of situation is to first make a
commit without re-indenting (hence having the "wrong" indentation) and
then make a second commit with only the re-indent.

Since working on the real patch while juggling with the indentation
commit can be annoying, you can wait until the patch is in its final
form to even bother about the re-indent. You can flag the places where
it is needed with /* TODO reindent */ if you find it useful.

Regards,

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

Re: [FFmpeg-devel] [PATCH V2 1/2] add support for ROI-based encoding

[Michael Niedermayer]

On Wed, Dec 26, 2018 at 04:11:35AM +0800, Guo, Yejun wrote:
> This patchset contains two patches.
> - the first patch (this patch) finished the code and ask for upstream.
> - the second patch is just a quick example on how to generate ROI info.
> 
> The encoders such as libx264 support different QPs offset for different MBs,
> it makes possible for ROI-based encoding. It makes sense to add support
> within ffmpeg to generate/accept ROI infos and pass into encoders.
> 
> Typical usage: After AVFrame is decoded, a ffmpeg filter or user's code
> generates ROI info for that frame, and the encoder finally does the
> ROI-based encoding.
> 
> This patch just enabled the path from ffmpeg to libx264, the more encoders
> can be added later.
> 
> Signed-off-by: Guo, Yejun 
> ---
>  libavcodec/libx264.c | 40 
>  libavutil/frame.c|  1 +
>  libavutil/frame.h| 19 +++
>  3 files changed, 60 insertions(+)

The commit message talks about 2 patches but this commit is one patch


> 
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index a68d0a7..a4f8677 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -40,6 +40,10 @@
>  #include 
>  #include 
>  
> +// from x264.h, for quant_offsets, Macroblocks are 16x16
> +// blocks of pixels (with respect to the luma plane)
> +#define MB_SIZE 16
> +
>  typedef struct X264Context {
>  AVClass*class;
>  x264_param_tparams;
> @@ -345,6 +349,42 @@ static int X264_frame(AVCodecContext *ctx, AVPacket 
> *pkt, const AVFrame *frame,
>  }
>  }
>  }
> +
> +AVFrameSideData *sd = av_frame_get_side_data(frame, 
> AV_FRAME_DATA_ROIS);
> +if (sd != NULL) {
> +if (x4->params.rc.i_aq_mode == X264_AQ_NONE) {
> +av_log(ctx, AV_LOG_WARNING, "Adaptive quantization must be 
> enabled to use ROI encoding, skipping ROI.\n");
> +} else {
> +if (frame->interlaced_frame == 0) {
> +size_t mbx = (frame->width + MB_SIZE - 1) / MB_SIZE;
> +size_t mby = (frame->height + MB_SIZE - 1) / MB_SIZE;
> +float* qoffsets;
> +qoffsets = (float*)av_malloc(sizeof(*qoffsets) * mbx * 
> mby);
> +if (qoffsets == NULL)
> +return AVERROR(ENOMEM);
> +memset(qoffsets, 0, sizeof(*qoffsets) * mbx * mby);

av_mallocz_array()



> +
> +size_t nb_rois = sd->size / sizeof(AVROI);
> +AVROI* rois = (AVROI*)sd->data;
> +for (size_t roi = 0; roi < nb_rois; roi++) {
> +int starty = FFMIN(mby, rois[roi].top / MB_SIZE);
> +int endy = FFMIN(mby, (rois[roi].bottom + MB_SIZE - 
> 1)/ MB_SIZE);
> +int startx = FFMIN(mbx, rois[roi].left / MB_SIZE);
> +int endx = FFMIN(mbx, (rois[roi].right + MB_SIZE - 
> 1)/ MB_SIZE);
> +for (int y = starty; y < endy; y++) {
> +for (int x = startx; x < endx; x++) {
> +qoffsets[x + y*mbx] = rois[roi].qoffset;
> +}
> +}
> +}
> +
> +x4->pic.prop.quant_offsets = qoffsets;
> +x4->pic.prop.quant_offsets_free = av_free;
> +} else {
> +av_log(ctx, AV_LOG_WARNING, "interlaced_frame not 
> supported for ROI encoding yet, skipping ROI.\n");
> +}
> +}
> +}
>  }
>  
>  do {
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 34a6210..bebc50e 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -841,6 +841,7 @@ const char *av_frame_side_data_name(enum 
> AVFrameSideDataType type)
>  case AV_FRAME_DATA_QP_TABLE_DATA:   return "QP table data";
>  #endif
>  case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata 
> SMPTE2094-40 (HDR10+)";
> +case AV_FRAME_DATA_ROIS: return "Regions Of Interest";
>  }
>  return NULL;
>  }
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 582ac47..d18d235 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -173,6 +173,12 @@ enum AVFrameSideDataType {
>   * volume transform - application 4 of SMPTE 2094-40:2016 standard.
>   */
>  AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
> +
> +/**
> + * Regions Of Interest, the number of ROI area is implied
> + * in the size of buf.
> + */
> +AV_FRAME_DATA_ROIS,
>  };

the API addition to libavutil should be in a seperate patch from the
use of the API, it also must bump the version

The text describing AV_FRAME_DATA_ROIS also is inadequate to use it
it doesnt really describe what this side data looks like exactly

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC7

Re: [FFmpeg-devel] [PATCH] delogo filter: new "uglarm" interpolation mode added

[Moritz Barsnick]

On Wed, Dec 26, 2018 at 15:23:58 +0100, Uwe Freese wrote:

Just some initial remarks (no full review):

> Can someone please review and add the code? Who would be primarily 
> responsible to add this to libavfilter?

> ./ffmpeg -i "yourtestvideo.mkv" -map 0:v:0 -vframes 100 -filter_complex 
> "crop=200:200:100:100,split=2[L][R];[L]delogo=x=50:y=50:w=100:h=100[L2];[R]delogo=x=50:y=50:w=100:h=100:mode=uglarm:power=15[R2];[L2][R2]hstack=inputs=2"
>  
> -c:v huffyuv delogotest.mkv && ffplay delogotest.mkv

ffmpeg can display directly to screen as well, BTW.

>    Note: I preferred using a string / enum parameter instead of a 
> boolean so the parameter could stay backwards compatible in case another 
> mode would be added some day.

Perfect.

>    Question: How many planes can there be? I used an array of pointers 
> to 10 possible lookup tables (one per plane).

The "10" should be a #define, so you only need to change it in one
place if you figure out that the correct number of planes is different.
;-) (For your own ease, but also for understanding where the "10" comes
from.)

> - I tried to use the same formatting etc. as in the original source 
> code. Is there anything I have to change?

You almost did. You already heard about indentation and especially the
added whitespace. You became inconsistent in some places, such as here:

>  /* Actual default value for band/t is 1, set in init */
> -{ "band", "set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, { 
> .i64 =  0 },  0, INT_MAX, FLAGS },
> -{ "t","set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, { 
> .i64 =  0 },  0, INT_MAX, FLAGS },
> +{ "band",   "set delogo area band size",OFFSET(band),   
> AV_OPT_TYPE_INT, { .i64 =  0 },  0, INT_MAX, FLAGS },
> +{ "t",  "set delogo area band size",OFFSET(band),   
> AV_OPT_TYPE_INT, { .i64 =  0 },  0, INT_MAX, FLAGS },
>  #endif
> -{ "show", "show delogo area",  OFFSET(show), AV_OPT_TYPE_BOOL,{ 
> .i64 =  0 },  0, 1,   FLAGS },
> +{"mode","set the interpolation mode",   OFFSET(mode), 
> AV_OPT_TYPE_INT, { .i64 = MODE_XY},  0, 1, FLAGS, "mode"},
> +{"xy", "use pixels in straight x any y direction",  
> OFFSET(mode), AV_OPT_TYPE_CONST, { .i64 = MODE_XY}, 0, 0, FLAGS, "mode"},
> +{"uglarm", "UGLARM mode, use full border",  
> OFFSET(mode), AV_OPT_TYPE_CONST, { .i64 = MODE_UGLARM}, 0, 0, FLAGS, "mode"},
> +{ "power",  "power of UGLARM interpolation",OFFSET(power),  
> AV_OPT_TYPE_INT, { .i64 = 15 },  0,  30, FLAGS },

Note how the existing original code had '{ "string",...', and you chose
to drop some whitespace. That's just cosmetic, but inconsistent. You
also have mismatching indentation.

> Of course, I would support adding documentation about the filter as 
> well. Just let me know what's needed.

You can start writing it now already, because it needs to go into
doc/filters.texi.

> + * Copyright (c) 2019 Uwe Freese 

Considering you authored it in 2018, this is forward-looking. ;-)

>  uint8_t *xdst, *xsrc;
> -
> +
>  uint8_t *topleft, *botleft, *topright;

See, this, among others, is what we mean with whitespace. If you had
looked through your patch, you would have noticed this useless change.
(Some editors and some patch viewers point out this danglig
whitespace.)

> +left_sample = topleft[src_linesize*(y-logo_y1)]   +
> +topleft[src_linesize*(y-logo_y1-1)] +
> +topleft[src_linesize*(y-logo_y1+1)];

The "topleft"s and the '+'s used to be column-aligned. You should
ensure that stays that way (in the indentation patch of course, if it's
indentation related).

> +for (x = logo_x1+1,
> +xdst = dst+logo_x1+1,
> +xsrc = src+logo_x1+1; x < logo_x2; x++, xdst++, xsrc++) {

Spaces around operators: x = logo_x1 + 1
(Also everywhere else. Unless it's the original code, then leave it be.)

> + * @param *uglarmtable  Pointer to table containing weigths for each 
> possible

weigths -> weights

> +static void calcUGLARMTables(double *uglarmtable, double *uglarmweightsum,
> + AVRational sar, int logo_w, int logo_h, int 
> power)

No capital letters or CamelCase in function names.

> +double e = 0.2 * power;

Could power also be a double instead of an int? Would specifying a
power of e.g. 2.5 make sense?

> +/* uglarmtable will contain a weigth for each possible diagonal distance

weigth -> weight

> +double d = pow(sqrt((double)(x * x * aspect * aspect + y * 
> y)), e);

int * int * double * double + int * int
already results in a double, unless I am mistaken. No need to cast the
result.

> +/* uglarmweithsum will conatain the sum of all weigths which is used when

uglarmweithsum -> uglarmweightsum
conatain -> contain
weigths -> weights

> +{"mode","set the interp

Re: [FFmpeg-devel] [PATCH] delogo filter: new "uglarm" interpolation mode added

[Uwe Freese]

Hello,

attached you can find a new patch, with no trailing spaces and avoiding 
additional indentation in existing code.


Regards,
Uwe

Am 26.12.18 um 16:48 schrieb Uwe Freese:

Hello,
so now I've taken the time to integrate the alternative 
interpolation algorithm 'uglarm' into the delogo filter.
Please do not re-indent existing code in this patch as this makes 
reviewing the changes more difficult


Parts of the previous code are now in an "if" statement block 
(function apply_delogo), so indentation is correct. Even it would 
maybe be better to read the diff, it wouldn't be correctly indented.


In the parameter lists, spaces are added to have the same layout of 
the descriptions in the right part for all lines.


So what should I do in this case?

I could create a diff of course with "wrong" indentation (not 
correctly indented and not good to read in the final file, but the 
diff is better to read...). Would this help for the first review?


  and please do not add trailing white space as it cannot be 
committed to our git repository.


OK, I'll remove them, no problem. (I'll send a new patch to this list 
after I know how to handle also the first point.)


(I did not check if you are possibly changing the license of code you 
did not write yourself, please be extra careful to make sure this 
does not happen.)


The TimgFilterLogoaway.cpp from ffdshow also has GPL 2 and up, the 
same as vf_delogo.c from ffmpeg. -> OK.


https://sourceforge.net/p/ffdshow-tryout/code/HEAD/tree/trunk/src/imgFilters/TimgFilterLogoaway.cpp 



Regards,
Uwe

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>From 221ecf222eb855303055a32bdc27ca8885217a2c Mon Sep 17 00:00:00 2001
From: breaker27 
Date: Wed, 26 Dec 2018 18:16:48 +0100
Subject: [PATCH] Add new delogo interpolation mode uglarm.

---
 libavfilter/vf_delogo.c | 182 +---
 1 file changed, 173 insertions(+), 9 deletions(-)

diff --git a/libavfilter/vf_delogo.c b/libavfilter/vf_delogo.c
index 065d093641..608cff2e4d 100644
--- a/libavfilter/vf_delogo.c
+++ b/libavfilter/vf_delogo.c
@@ -2,6 +2,7 @@
  * Copyright (c) 2002 Jindrich Makovicka 
  * Copyright (c) 2011 Stefano Sabatini
  * Copyright (c) 2013, 2015 Jean Delvare 
+ * Copyright (c) 2019 Uwe Freese 
  *
  * This file is part of FFmpeg.
  *
@@ -25,6 +26,9 @@
  * A very simple tv station logo remover
  * Originally imported from MPlayer libmpcodecs/vf_delogo.c,
  * the algorithm was later improved.
+ * The "UGLARM" mode was first implemented 2001 by Uwe Freese for Virtual
+ * Dub's LogoAway filter (by Krzysztof Wojdon), taken over into ffdshow's
+ * logoaway filter (by Milan Cutka), from where it was ported to ffmpeg.
  */
 
 #include "libavutil/common.h"
@@ -50,6 +54,10 @@
  * @param logo_w width of the logo
  * @param logo_h height of the logo
  * @param band   the size of the band around the processed area
+ * @param *uglarmtable  pointer to weight table in UGLARM interpolation mode,
+ *  zero when x-y mode is used
+ * @param *uglarmweightsum  pointer to weight sum table in UGLARM interpolation mode,
+ *  zero when x-y mode is used
  * @param show   show a rectangle around the processed area, useful for
  *   parameters tweaking
  * @param direct if non-zero perform in-place processing
@@ -58,7 +66,8 @@ static void apply_delogo(uint8_t *dst, int dst_linesize,
  uint8_t *src, int src_linesize,
  int w, int h, AVRational sar,
  int logo_x, int logo_y, int logo_w, int logo_h,
- unsigned int band, int show, int direct)
+ unsigned int band, double *uglarmtable,
+ double *uglarmweightsum, int show, int direct)
 {
 int x, y;
 uint64_t interp, weightl, weightr, weightt, weightb, weight;
@@ -89,6 +98,7 @@ static void apply_delogo(uint8_t *dst, int dst_linesize,
 dst += (logo_y1 + 1) * dst_linesize;
 src += (logo_y1 + 1) * src_linesize;
 
+if (!uglarmtable) {
 for (y = logo_y1+1; y < logo_y2; y++) {
 left_sample = topleft[src_linesize*(y-logo_y1)]   +
   topleft[src_linesize*(y-logo_y1-1)] +
@@ -151,12 +161,121 @@ static void apply_delogo(uint8_t *dst, int dst_linesize,
 dst += dst_linesize;
 src += src_linesize;
 }
+} else {
+int bx, by;
+double interpd;
+
+for (y = logo_y1 + 1; y < logo_y2; y++) {
+for (x = logo_x1 + 1,
+xdst = dst + logo_x1 + 1,
+xsrc = src + logo_x1 + 1; x < logo_x2; x++, xdst++, xsrc++) {
+
+if (show && (y == logo_y1 + 1 || y == logo_y2 - 1 ||
+x == logo_x1 + 1 || x == logo_x2 - 1)) {
+*xdst = 0;
+continue;
+}
+
+   

Re: [FFmpeg-devel] [PATCH] avfilter/f_realtime: add option to scale speed

[Nicolas George]

Moritz Barsnick (2018-12-25):
> Signed-off-by: Moritz Barsnick 
> ---
> This adds two double precision divisions per frame, which seems acceptable.
> 
> I'm not sure scaling the limit by the factor is the correct idea.  It feels
> right regarding the discontinuities, but not according to the limit option's
> description.

I think it would be more logical to keep limit expressed as real time.
This option is a safety net, it is not nimble enough to select time gaps
to ignore in files that contain them.

> 
> 
>  doc/filters.texi | 4 
>  libavfilter/f_realtime.c | 7 +--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 65ce25bc18..60ebf2aa99 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -20829,6 +20829,10 @@ They accept the following options:
>  @item limit
>  Time limit for the pauses. Any pause longer than that will be considered
>  a timestamp discontinuity and reset the timer. Default is 2 seconds.
> +@item speed
> +Speed factor for processing. The value must be a float larger than zero.
> +Values larger than 1.0 will speed up processing, smaller will slow it down.
> +The @var{limit} is automatically adapted accordingly. Default is 1.0.
>  @end table
>  
>  @anchor{select}
> diff --git a/libavfilter/f_realtime.c b/libavfilter/f_realtime.c
> index 171c16..8d4fbf642b 100644
> --- a/libavfilter/f_realtime.c
> +++ b/libavfilter/f_realtime.c
> @@ -22,11 +22,13 @@
>  #include "libavutil/time.h"
>  #include "avfilter.h"
>  #include "internal.h"
> +#include 
>  
>  typedef struct RealtimeContext {
>  const AVClass *class;
>  int64_t delta;
>  int64_t limit;
> +double speed;
>  unsigned inited;
>  } RealtimeContext;
>  
> @@ -36,7 +38,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> *frame)
>  RealtimeContext *s = ctx->priv;
>  
>  if (frame->pts != AV_NOPTS_VALUE) {
> -int64_t pts = av_rescale_q(frame->pts, inlink->time_base, 
> AV_TIME_BASE_Q);
> +int64_t pts = av_rescale_q(frame->pts, inlink->time_base, 
> AV_TIME_BASE_Q) / s->speed;
>  int64_t now = av_gettime_relative();
>  int64_t sleep = pts - now + s->delta;
>  if (!s->inited) {
> @@ -44,7 +46,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> *frame)
>  sleep = 0;
>  s->delta = now - pts;
>  }
> -if (sleep > s->limit || sleep < -s->limit) {

> +if (sleep > s->limit / s->speed || sleep < -s->limit / s->speed) {

I should have used FFABS(sleep); if this hunk stays, then better make
the change to have only one division:

if (FFABS(sleep) > s->limit / s->speed)

>  av_log(ctx, AV_LOG_WARNING,
> "time discontinuity detected: %"PRIi64" us, resetting\n",
> sleep);
> @@ -65,6 +67,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> *frame)
>  #define FLAGS AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_AUDIO_PARAM | 
> AV_OPT_FLAG_FILTERING_PARAM
>  static const AVOption options[] = {
>  { "limit", "sleep time limit", OFFSET(limit), AV_OPT_TYPE_DURATION, { 
> .i64 = 200 }, 0, INT64_MAX, FLAGS },
> +{ "speed", "speed factor", OFFSET(speed), AV_OPT_TYPE_DOUBLE, { .dbl = 
> 1.0 }, DBL_MIN, DBL_MAX, FLAGS },
>  { NULL }
>  };
>  

LGTM apart from these nitpicks.

Regards,

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

Re: [FFmpeg-devel] [PATCH v2] swscale/output: Altivec-optimize float yuv2plane1

[Michael Niedermayer]

On Mon, Dec 24, 2018 at 07:39:18PM +0200, Lauri Kasanen wrote:
> On Sun, 16 Dec 2018 11:06:53 +0200
> Lauri Kasanen  wrote:
> 
> > This function wouldn't benefit from VSX instructions, so I put it
> > under altivec.
> > 
> > ./ffmpeg_g -f rawvideo -pix_fmt rgb24 -s hd1080 -i /dev/zero -pix_fmt 
> > grayf32le \
> > -f null -vframes 100 -v error -nostats -
> > 
> > 3743 UNITS in planar1,   65495 runs, 41 skips
> > 
> > -cpuflags 0
> > 
> > 23511 UNITS in planar1,   65530 runs,  6 skips
> > 
> > grayf32be
> > 
> > 4647 UNITS in planar1,   65449 runs, 87 skips
> > 
> > -cpuflags 0
> > 
> > 28608 UNITS in planar1,   65530 runs,  6 skips
> > 
> > The native speedup is 6.28133, and the bswapping one 6.15623.
> > Fate passes, each format tested with an image to video conversion.
> > 
> > Signed-off-by: Lauri Kasanen 
> > ---
> > 
> > Tested on POWER8 LE. Testing on earlier ppc and/or BE appreciated.
> > 
> > v2: Added #undef vzero, that define broke the build on older gcc. Thanks 
> > Michael
> 
> Ping. And of course it's not gcc version dependant, but rather it was
> the BE ifdef; it was too early in the morning.

seems working, will apply

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Add a new channel layout API

[Paul B Mahol]

On 12/26/18, James Almer  wrote:
> On 12/26/2018 12:07 PM, Paul B Mahol wrote:
>> On 12/26/18, Nicolas George  wrote:
 +
 +/**
 + * Initialize a native channel layout from a bitmask indicating which
 channels
 + * are present.
 + *
 + * @note channel_layout should be properly allocated as described
 above.
 + *
 + * @param channel_layout the layout structure to be initialized
 + * @param mask bitmask describing the channel layout
 + */
 +void av_channel_layout_from_mask(AVChannelLayout *channel_layout,
 uint64_t mask);
 +
 +/**
 + * Initialize a channel layout from a given string description.
>>>
 + * The input string can be represented by:
 + *  - the formal channel layout name (returned by
 av_channel_layout_describe())
 + *  - single or multiple channel names (returned by av_channel_name()
 + *or concatenated with "|")
 + *  - a hexadecimal value of a channel layout (eg. "0x4")
 + *  - the number of channels with default layout (eg. "5")
 + *  - the number of unordered channels (eg. "4 channels")
>>>
>>> av_get_channel_layout() used to use '+' instead of '|', and I think it
>>> is better. For once, '+' is not a special character for shells.
>>
>> Look folk, I'm not paid to do this nor I'm paid to read your "reviews"
>> so I will ignore this one.
> What prompted you to reply this way? Was there a need to be this
> aggressive with a review?
>
> What do you or anyone wins with this?

You called for this, I'm not gonna continue working on this.

All thanks to very "nice" reviewers like all of you.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avutil/imgutils: Optimize writing 4 bytes in memset_bytes()

[Michael Niedermayer]

On Wed, Dec 26, 2018 at 04:32:17PM +0100, Paul B Mahol wrote:
> On 12/25/18, Michael Niedermayer  wrote:
> > Fixes: Timeout
> > Fixes:
> > 11502/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> > Before: Executed
> > clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> > in 11294 ms
> > After : Executed
> > clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> > in 4249 ms
> >
> > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavutil/imgutils.c | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> > index 4938a7ef67..cc38f1e878 100644
> > --- a/libavutil/imgutils.c
> > +++ b/libavutil/imgutils.c
> > @@ -529,6 +529,12 @@ static void memset_bytes(uint8_t *dst, size_t dst_size,
> > uint8_t *clear,
> >  }
> >  } else if (clear_size == 4) {
> >  uint32_t val = AV_RN32(clear);
> > +uint64_t val8 = val * 0x10001ULL;
> > +for (; dst_size >= 32; dst_size -= 32) {
> > +AV_WN64(dst   , val8); AV_WN64(dst+ 8, val8);
> > +AV_WN64(dst+16, val8); AV_WN64(dst+24, val8);
> > +dst += 32;
> > +}
> >  for (; dst_size >= 4; dst_size -= 4) {
> >  AV_WN32(dst, val);
> >  dst += 4;
> > --
> > 2.20.1
> >
> 
> NAK, implement special memset function instead.

I can move the added loop into a seperate function, if thats what you
suggest ?
All the code is already in a "special" memset though, this is
memset_bytes()

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"I am not trying to be anyone's saviour, I'm trying to think about the
 future and not be sad" - Elon Musk



signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avutil/imgutils: Optimize writing 4 bytes in memset_bytes()

[Michael Niedermayer]

On Tue, Dec 25, 2018 at 10:12:13PM -0300, James Almer wrote:
> On 12/25/2018 7:15 PM, Michael Niedermayer wrote:
> > Fixes: Timeout
> > Fixes: 
> > 11502/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> > Before: Executed 
> > clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> >  in 11294 ms
> > After : Executed 
> > clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> >  in 4249 ms
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavutil/imgutils.c | 6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> > index 4938a7ef67..cc38f1e878 100644
> > --- a/libavutil/imgutils.c
> > +++ b/libavutil/imgutils.c
> > @@ -529,6 +529,12 @@ static void memset_bytes(uint8_t *dst, size_t 
> > dst_size, uint8_t *clear,
> >  }
> >  } else if (clear_size == 4) {
> >  uint32_t val = AV_RN32(clear);
> > +uint64_t val8 = val * 0x10001ULL;
> > +for (; dst_size >= 32; dst_size -= 32) {
> > +AV_WN64(dst   , val8); AV_WN64(dst+ 8, val8);
> > +AV_WN64(dst+16, val8); AV_WN64(dst+24, val8);
> > +dst += 32;
> > +}
> 
> This should be wrapped with a HAVE_FAST_64BIT preprocessor check.

will do so


> 
> Also, is it much slower if you also write one per loop like everywhere
> else in the function? I'd prefer if things are consistent.

as in the patch:
 3955 ms  3954 ms  3954 ms
 
with one write per iteration:
 5705 ms  5635 ms  5629 ms

 
> Similarly, you could add four and eight bytes loops to the clear_size ==
> 2 case above.

yes i can if you want me to?, but i have no testcase for that so it would be 
untested

thx


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] Support HDR dynamic metdata (HDR10+) in HEVC decoder.

[Mohammad Izadi]

Decode HDR10+ metadata from SEI message and propagate it to side data.
---
 libavcodec/avcodec.h  |  10 +-
 libavcodec/avpacket.c |   1 +
 libavcodec/decode.c   |   2 +-
 libavcodec/hevc_sei.c | 234 --
 libavcodec/hevc_sei.h |   7 ++
 libavcodec/hevcdec.c  |  79 ++
 6 files changed, 322 insertions(+), 11 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index fd7f60bf4a..044aa447ab 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1328,7 +1328,7 @@ enum AVPacketSideDataType {
 AV_PKT_DATA_METADATA_UPDATE,
 
 /**
- * MPEGTS stream ID as uint8_t, this is required to pass the stream ID
+ * MPEGTS stream ID, this is required to pass the stream ID
  * information from the demuxer to the corresponding muxer.
  */
 AV_PKT_DATA_MPEGTS_STREAM_ID,
@@ -1360,6 +1360,14 @@ enum AVPacketSideDataType {
  */
 AV_PKT_DATA_A53_CC,
 
+/**
+ * HDR10+ dynamic metadata associated with a video frame. The metadata is 
in
+ * the form of the AVDynamicHDRPlus struct and contains
+ * information for color volume transform - application 4 of
+ * SPMTE 2094-40:2016 standard.
+ */
+AV_PKT_DATA_HDR_DYNAMIC_HDR_PLUS,
+
 /**
  * This side data is encryption initialization data.
  * The format is not part of ABI, use av_encryption_init_info_* methods to
diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index e160ad3033..137a0489d4 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -391,6 +391,7 @@ const char *av_packet_side_data_name(enum 
AVPacketSideDataType type)
 case AV_PKT_DATA_CONTENT_LIGHT_LEVEL:return "Content light level 
metadata";
 case AV_PKT_DATA_SPHERICAL:  return "Spherical Mapping";
 case AV_PKT_DATA_A53_CC: return "A53 Closed Captions";
+case AV_PKT_DATA_HDR_DYNAMIC_HDR_PLUS: return "HDR10+ Dynamic Metadata 
(SMPTE 2094-40)";
 case AV_PKT_DATA_ENCRYPTION_INIT_INFO:   return "Encryption 
initialization data";
 case AV_PKT_DATA_ENCRYPTION_INFO:return "Encryption info";
 case AV_PKT_DATA_AFD:return "Active Format 
Description data";
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index a32ff2fcd3..a2d6ec4f18 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1378,7 +1378,6 @@ int ff_get_format(AVCodecContext *avctx, const enum 
AVPixelFormat *fmt)
 if (i == n) {
 av_log(avctx, AV_LOG_ERROR, "Invalid return from get_format(): "
"%s not in possible list.\n", desc->name);
-ret = AV_PIX_FMT_NONE;
 break;
 }
 
@@ -1706,6 +1705,7 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame 
*frame)
 { AV_PKT_DATA_MASTERING_DISPLAY_METADATA, 
AV_FRAME_DATA_MASTERING_DISPLAY_METADATA },
 { AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
AV_FRAME_DATA_CONTENT_LIGHT_LEVEL },
 { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC },
+{ AV_PKT_DATA_HDR_DYNAMIC_HDR_PLUS, AV_FRAME_DATA_DYNAMIC_HDR_PLUS },
 };
 
 if (pkt) {
diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
index c59bd4321e..e261c038c3 100644
--- a/libavcodec/hevc_sei.c
+++ b/libavcodec/hevc_sei.c
@@ -206,10 +206,209 @@ static int 
decode_registered_user_data_closed_caption(HEVCSEIA53Caption *s, GetB
 return 0;
 }
 
-static int decode_nal_sei_user_data_registered_itu_t_t35(HEVCSEI *s, 
GetBitContext *gb,
+static int decode_registered_user_data_dynamic_hdr_plus(
+HEVCSEIDynamicHDRPlus *s, GetBitContext *gb,
+void *logctx, int size)
+{
+const int luminance_den = 1;
+const int peak_luminance_den = 15;
+const int rgb_den = 10;
+const int fraction_pixel_den = 1000;
+const int knee_point_den = 4095;
+const int bezier_anchor_den = 1023;
+const int saturation_weight_den = 8;
+
+AVDynamicHDRPlus* info = s->info;
+int bits_left = size * 8;
+int w, i, j;
+
+if (bits_left < 2)
+return AVERROR(EINVAL);
+
+info->num_windows = get_bits(gb, 2);
+bits_left -= 2;
+if (info->num_windows < 1 || info->num_windows > 3) {
+av_log(logctx, AV_LOG_ERROR, "num_windows=%d, must be in [1, 3]\n",
+   info->num_windows);
+return AVERROR_INVALIDDATA;
+}
+
+if (bits_left < ((19 * 8 + 1) * (info->num_windows - 1)))
+return AVERROR(EINVAL);
+for (w = 1; w < info->num_windows; w++) {
+info->params[w].window_upper_left_corner_x.num = get_bits(gb, 16);
+info->params[w].window_upper_left_corner_y.num = get_bits(gb, 16);
+info->params[w].window_lower_right_corner_x.num = get_bits(gb, 16);
+info->params[w].window_lower_right_corner_y.num = get_bits(gb, 16);
+// The corners are set to absolute coordinates here. They should be
+// converted to the relative coordinates (in [0, 1]) in the decoder.
+in

[FFmpeg-devel] [PATCH] avcodec/dxva2_mpeg2.c: don't try to get surface index for absent frame

[Anton Fedchin]

From: Anton Fedchin 

after 153b36f there is a possibility to crash when trying to get index of
a surface which points to nirvana. it may occurs when a mpeg2 stream starts
with non i-frame.
---
 libavcodec/dxva2_mpeg2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/dxva2_mpeg2.c b/libavcodec/dxva2_mpeg2.c
index 8cc21bf199..c862e6e5e4 100644
--- a/libavcodec/dxva2_mpeg2.c
+++ b/libavcodec/dxva2_mpeg2.c
@@ -48,11 +48,11 @@ static void fill_picture_parameters(AVCodecContext *avctx,
 memset(pp, 0, sizeof(*pp));
 pp->wDecodedPictureIndex = ff_dxva2_get_surface_index(avctx, ctx, 
current_picture->f);
 pp->wDeblockedPictureIndex   = 0;
-if (s->pict_type != AV_PICTURE_TYPE_I)
+if (s->pict_type != AV_PICTURE_TYPE_I && s->last_picture_ptr)
 pp->wForwardRefPictureIndex  = ff_dxva2_get_surface_index(avctx, ctx, 
s->last_picture.f);
 else
 pp->wForwardRefPictureIndex  = 0x;
-if (s->pict_type == AV_PICTURE_TYPE_B)
+if (s->pict_type == AV_PICTURE_TYPE_B && s->next_picture_ptr)
 pp->wBackwardRefPictureIndex = ff_dxva2_get_surface_index(avctx, ctx, 
s->next_picture.f);
 else
 pp->wBackwardRefPictureIndex = 0x;
-- 
2.17.1.windows.2

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

Re: [FFmpeg-devel] [PATCH] Support HDR dynamic metdata (HDR10+) in HEVC decoder.

[James Almer]

On 12/26/2018 4:40 PM, Mohammad Izadi wrote:
> Decode HDR10+ metadata from SEI message and propagate it to side data.
> ---
>  libavcodec/avcodec.h  |  10 +-
>  libavcodec/avpacket.c |   1 +
>  libavcodec/decode.c   |   2 +-
>  libavcodec/hevc_sei.c | 234 --
>  libavcodec/hevc_sei.h |   7 ++
>  libavcodec/hevcdec.c  |  79 ++
>  6 files changed, 322 insertions(+), 11 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index fd7f60bf4a..044aa447ab 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -1328,7 +1328,7 @@ enum AVPacketSideDataType {
>  AV_PKT_DATA_METADATA_UPDATE,
>  
>  /**
> - * MPEGTS stream ID as uint8_t, this is required to pass the stream ID
> + * MPEGTS stream ID, this is required to pass the stream ID

This looks like an unrelated change.

>   * information from the demuxer to the corresponding muxer.
>   */
>  AV_PKT_DATA_MPEGTS_STREAM_ID,
> @@ -1360,6 +1360,14 @@ enum AVPacketSideDataType {
>   */
>  AV_PKT_DATA_A53_CC,
>  
> +/**
> + * HDR10+ dynamic metadata associated with a video frame. The metadata 
> is in
> + * the form of the AVDynamicHDRPlus struct and contains
> + * information for color volume transform - application 4 of
> + * SPMTE 2094-40:2016 standard.
> + */
> +AV_PKT_DATA_HDR_DYNAMIC_HDR_PLUS,

Adding this value should be its own commit, with a minor avcodec version
bump.

> +
>  /**
>   * This side data is encryption initialization data.
>   * The format is not part of ABI, use av_encryption_init_info_* methods 
> to
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index e160ad3033..137a0489d4 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -391,6 +391,7 @@ const char *av_packet_side_data_name(enum 
> AVPacketSideDataType type)
>  case AV_PKT_DATA_CONTENT_LIGHT_LEVEL:return "Content light level 
> metadata";
>  case AV_PKT_DATA_SPHERICAL:  return "Spherical Mapping";
>  case AV_PKT_DATA_A53_CC: return "A53 Closed 
> Captions";
> +case AV_PKT_DATA_HDR_DYNAMIC_HDR_PLUS: return "HDR10+ Dynamic Metadata 
> (SMPTE 2094-40)";

Vertical alignment.

>  case AV_PKT_DATA_ENCRYPTION_INIT_INFO:   return "Encryption 
> initialization data";
>  case AV_PKT_DATA_ENCRYPTION_INFO:return "Encryption info";
>  case AV_PKT_DATA_AFD:return "Active Format 
> Description data";
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index a32ff2fcd3..a2d6ec4f18 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1378,7 +1378,6 @@ int ff_get_format(AVCodecContext *avctx, const enum 
> AVPixelFormat *fmt)
>  if (i == n) {
>  av_log(avctx, AV_LOG_ERROR, "Invalid return from get_format(): "
> "%s not in possible list.\n", desc->name);
> -ret = AV_PIX_FMT_NONE;

Also unrelated.

>  break;
>  }
>  
> @@ -1706,6 +1705,7 @@ int ff_decode_frame_props(AVCodecContext *avctx, 
> AVFrame *frame)
>  { AV_PKT_DATA_MASTERING_DISPLAY_METADATA, 
> AV_FRAME_DATA_MASTERING_DISPLAY_METADATA },
>  { AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
> AV_FRAME_DATA_CONTENT_LIGHT_LEVEL },
>  { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC },
> +{ AV_PKT_DATA_HDR_DYNAMIC_HDR_PLUS, AV_FRAME_DATA_DYNAMIC_HDR_PLUS },

Vertical alignment again.

>  };
>  
>  if (pkt) {
> diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
> index c59bd4321e..e261c038c3 100644
> --- a/libavcodec/hevc_sei.c
> +++ b/libavcodec/hevc_sei.c
> @@ -206,10 +206,209 @@ static int 
> decode_registered_user_data_closed_caption(HEVCSEIA53Caption *s, GetB
>  return 0;
>  }
>  
> -static int decode_nal_sei_user_data_registered_itu_t_t35(HEVCSEI *s, 
> GetBitContext *gb,
> +static int decode_registered_user_data_dynamic_hdr_plus(
> +HEVCSEIDynamicHDRPlus *s, GetBitContext *gb,
> +void *logctx, int size)

Same.

> +{
> +const int luminance_den = 1;
> +const int peak_luminance_den = 15;
> +const int rgb_den = 10;
> +const int fraction_pixel_den = 1000;
> +const int knee_point_den = 4095;
> +const int bezier_anchor_den = 1023;
> +const int saturation_weight_den = 8;
> +
> +AVDynamicHDRPlus* info = s->info;
> +int bits_left = size * 8;
> +int w, i, j;
> +
> +if (bits_left < 2)
> +return AVERROR(EINVAL);

AVERROR_INVALIDDATA. We use EINVAL only for invalid arguments and not
for invalid bitstream data. Same for other cases below.

> +
> +info->num_windows = get_bits(gb, 2);
> +bits_left -= 2;
> +if (info->num_windows < 1 || info->num_windows > 3) {
> +av_log(logctx, AV_LOG_ERROR, "num_windows=%d, must be in [1, 3]\n",
> +   info->num_windows);
> +return AVERROR_INVALIDDATA;
> +}
> +
> + 

Re: [FFmpeg-devel] [PATCH] Add a new channel layout API

[James Almer]

On 12/26/2018 4:35 PM, Paul B Mahol wrote:
> On 12/26/18, James Almer  wrote:
>> On 12/26/2018 12:07 PM, Paul B Mahol wrote:
>>> On 12/26/18, Nicolas George  wrote:
> +
> +/**
> + * Initialize a native channel layout from a bitmask indicating which
> channels
> + * are present.
> + *
> + * @note channel_layout should be properly allocated as described
> above.
> + *
> + * @param channel_layout the layout structure to be initialized
> + * @param mask bitmask describing the channel layout
> + */
> +void av_channel_layout_from_mask(AVChannelLayout *channel_layout,
> uint64_t mask);
> +
> +/**
> + * Initialize a channel layout from a given string description.

> + * The input string can be represented by:
> + *  - the formal channel layout name (returned by
> av_channel_layout_describe())
> + *  - single or multiple channel names (returned by av_channel_name()
> + *or concatenated with "|")
> + *  - a hexadecimal value of a channel layout (eg. "0x4")
> + *  - the number of channels with default layout (eg. "5")
> + *  - the number of unordered channels (eg. "4 channels")

 av_get_channel_layout() used to use '+' instead of '|', and I think it
 is better. For once, '+' is not a special character for shells.
>>>
>>> Look folk, I'm not paid to do this nor I'm paid to read your "reviews"
>>> so I will ignore this one.
>> What prompted you to reply this way? Was there a need to be this
>> aggressive with a review?
>>
>> What do you or anyone wins with this?
> 
> You called for this, I'm not gonna continue working on this.
> 
> All thanks to very "nice" reviewers like all of you.

You're making no sense...
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avutil/imgutils: Optimize writing 4 bytes in memset_bytes()

[Paul B Mahol]

On 12/26/18, Michael Niedermayer  wrote:
> On Wed, Dec 26, 2018 at 04:32:17PM +0100, Paul B Mahol wrote:
>> On 12/25/18, Michael Niedermayer  wrote:
>> > Fixes: Timeout
>> > Fixes:
>> > 11502/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
>> > Before: Executed
>> > clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
>> > in 11294 ms
>> > After : Executed
>> > clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
>> > in 4249 ms
>> >
>> > Found-by: continuous fuzzing process
>> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>> > Signed-off-by: Michael Niedermayer 
>> > ---
>> >  libavutil/imgutils.c | 6 ++
>> >  1 file changed, 6 insertions(+)
>> >
>> > diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
>> > index 4938a7ef67..cc38f1e878 100644
>> > --- a/libavutil/imgutils.c
>> > +++ b/libavutil/imgutils.c
>> > @@ -529,6 +529,12 @@ static void memset_bytes(uint8_t *dst, size_t
>> > dst_size,
>> > uint8_t *clear,
>> >  }
>> >  } else if (clear_size == 4) {
>> >  uint32_t val = AV_RN32(clear);
>> > +uint64_t val8 = val * 0x10001ULL;
>> > +for (; dst_size >= 32; dst_size -= 32) {
>> > +AV_WN64(dst   , val8); AV_WN64(dst+ 8, val8);
>> > +AV_WN64(dst+16, val8); AV_WN64(dst+24, val8);
>> > +dst += 32;
>> > +}
>> >  for (; dst_size >= 4; dst_size -= 4) {
>> >  AV_WN32(dst, val);
>> >  dst += 4;
>> > --
>> > 2.20.1
>> >
>>
>> NAK, implement special memset function instead.
>
> I can move the added loop into a seperate function, if thats what you
> suggest ?

No, don't do that.

> All the code is already in a "special" memset though, this is
> memset_bytes()
>

I guess function is less useful if its static. So any duplicate should
be avoided in codebase.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avutil/imgutils: Optimize writing 4 bytes in memset_bytes()

[Michael Niedermayer]

On Wed, Dec 26, 2018 at 10:02:15PM +0100, Paul B Mahol wrote:
> On 12/26/18, Michael Niedermayer  wrote:
> > On Wed, Dec 26, 2018 at 04:32:17PM +0100, Paul B Mahol wrote:
> >> On 12/25/18, Michael Niedermayer  wrote:
> >> > Fixes: Timeout
> >> > Fixes:
> >> > 11502/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> >> > Before: Executed
> >> > clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> >> > in 11294 ms
> >> > After : Executed
> >> > clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> >> > in 4249 ms
> >> >
> >> > Found-by: continuous fuzzing process
> >> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >> > Signed-off-by: Michael Niedermayer 
> >> > ---
> >> >  libavutil/imgutils.c | 6 ++
> >> >  1 file changed, 6 insertions(+)
> >> >
> >> > diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> >> > index 4938a7ef67..cc38f1e878 100644
> >> > --- a/libavutil/imgutils.c
> >> > +++ b/libavutil/imgutils.c
> >> > @@ -529,6 +529,12 @@ static void memset_bytes(uint8_t *dst, size_t
> >> > dst_size,
> >> > uint8_t *clear,
> >> >  }
> >> >  } else if (clear_size == 4) {
> >> >  uint32_t val = AV_RN32(clear);
> >> > +uint64_t val8 = val * 0x10001ULL;
> >> > +for (; dst_size >= 32; dst_size -= 32) {
> >> > +AV_WN64(dst   , val8); AV_WN64(dst+ 8, val8);
> >> > +AV_WN64(dst+16, val8); AV_WN64(dst+24, val8);
> >> > +dst += 32;
> >> > +}
> >> >  for (; dst_size >= 4; dst_size -= 4) {
> >> >  AV_WN32(dst, val);
> >> >  dst += 4;
> >> > --
> >> > 2.20.1
> >> >
> >>
> >> NAK, implement special memset function instead.
> >
> > I can move the added loop into a seperate function, if thats what you
> > suggest ?
> 
> No, don't do that.
> 
> > All the code is already in a "special" memset though, this is
> > memset_bytes()
> >
> 
> I guess function is less useful if its static. So any duplicate should
> be avoided in codebase.

i can make it non static and export it if you want ?

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avutil/imgutils: Optimize writing 4 bytes in memset_bytes()

[Marton Balint]

On Wed, 26 Dec 2018, Paul B Mahol wrote:


On 12/26/18, Michael Niedermayer  wrote:

On Wed, Dec 26, 2018 at 04:32:17PM +0100, Paul B Mahol wrote:

On 12/25/18, Michael Niedermayer  wrote:
> Fixes: Timeout
> Fixes:
> 
11502/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> Before: Executed
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> in 11294 ms
> After : Executed
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> in 4249 ms
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavutil/imgutils.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> index 4938a7ef67..cc38f1e878 100644
> --- a/libavutil/imgutils.c
> +++ b/libavutil/imgutils.c
> @@ -529,6 +529,12 @@ static void memset_bytes(uint8_t *dst, size_t
> dst_size,
> uint8_t *clear,
>  }
>  } else if (clear_size == 4) {
>  uint32_t val = AV_RN32(clear);
> +uint64_t val8 = val * 0x10001ULL;
> +for (; dst_size >= 32; dst_size -= 32) {
> +AV_WN64(dst   , val8); AV_WN64(dst+ 8, val8);
> +AV_WN64(dst+16, val8); AV_WN64(dst+24, val8);
> +dst += 32;
> +}
>  for (; dst_size >= 4; dst_size -= 4) {
>  AV_WN32(dst, val);
>  dst += 4;
> --
> 2.20.1
>

NAK, implement special memset function instead.


I can move the added loop into a seperate function, if thats what you
suggest ?


No, don't do that.


All the code is already in a "special" memset though, this is
memset_bytes()



I guess function is less useful if its static. So any duplicate should
be avoided in codebase.


Isn't av_memcpy_backptr does almost exactly what is needed here? That can 
also be optimized further if needed.


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

Re: [FFmpeg-devel] [PATCH 1/3] avutil/imgutils: Optimize writing 4 bytes in memset_bytes()

[Paul B Mahol]

On 12/26/18, Michael Niedermayer  wrote:
> On Wed, Dec 26, 2018 at 10:02:15PM +0100, Paul B Mahol wrote:
>> On 12/26/18, Michael Niedermayer  wrote:
>> > On Wed, Dec 26, 2018 at 04:32:17PM +0100, Paul B Mahol wrote:
>> >> On 12/25/18, Michael Niedermayer  wrote:
>> >> > Fixes: Timeout
>> >> > Fixes:
>> >> > 11502/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
>> >> > Before: Executed
>> >> > clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
>> >> > in 11294 ms
>> >> > After : Executed
>> >> > clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
>> >> > in 4249 ms
>> >> >
>> >> > Found-by: continuous fuzzing process
>> >> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>> >> > Signed-off-by: Michael Niedermayer 
>> >> > ---
>> >> >  libavutil/imgutils.c | 6 ++
>> >> >  1 file changed, 6 insertions(+)
>> >> >
>> >> > diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
>> >> > index 4938a7ef67..cc38f1e878 100644
>> >> > --- a/libavutil/imgutils.c
>> >> > +++ b/libavutil/imgutils.c
>> >> > @@ -529,6 +529,12 @@ static void memset_bytes(uint8_t *dst, size_t
>> >> > dst_size,
>> >> > uint8_t *clear,
>> >> >  }
>> >> >  } else if (clear_size == 4) {
>> >> >  uint32_t val = AV_RN32(clear);
>> >> > +uint64_t val8 = val * 0x10001ULL;
>> >> > +for (; dst_size >= 32; dst_size -= 32) {
>> >> > +AV_WN64(dst   , val8); AV_WN64(dst+ 8, val8);
>> >> > +AV_WN64(dst+16, val8); AV_WN64(dst+24, val8);
>> >> > +dst += 32;
>> >> > +}
>> >> >  for (; dst_size >= 4; dst_size -= 4) {
>> >> >  AV_WN32(dst, val);
>> >> >  dst += 4;
>> >> > --
>> >> > 2.20.1
>> >> >
>> >>
>> >> NAK, implement special memset function instead.
>> >
>> > I can move the added loop into a seperate function, if thats what you
>> > suggest ?
>>
>> No, don't do that.
>>
>> > All the code is already in a "special" memset though, this is
>> > memset_bytes()
>> >
>>
>> I guess function is less useful if its static. So any duplicate should
>> be avoided in codebase.
>
> i can make it non static and export it if you want ?
>

Yes. Also make it used where there is already similar code doing same.

> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> If you fake or manipulate statistics in a paper in physics you will never
> get a job again.
> If you fake or manipulate statistics in a paper in medicin you will get
> a job for life at the pharma industry.
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/f_realtime: add option to scale speed

[Moritz Barsnick]

On Wed, Dec 26, 2018 at 18:37:42 +0100, Nicolas George wrote:
> > I'm not sure scaling the limit by the factor is the correct idea.  It feels
> > right regarding the discontinuities, but not according to the limit option's
> > description.
> 
> I think it would be more logical to keep limit expressed as real time.
> This option is a safety net, it is not nimble enough to select time gaps
> to ignore in files that contain them.

So the limit shall be in "real" realtime? In other words, if the max
sleep time limit is 2 seconds (default), it shall stay there, even if a
speed < 1 slowdown extends all sleep times?

(The use case is, for example: Slowdown a 25 fps stream with 0.01, all
sleep times will be around 4 seconds, and therefore be skipped, which
is not quite intended. The user would need to adapt the limit
explicitly.)

> I should have used FFABS(sleep); if this hunk stays, then better make
> the change to have only one division:
> 
>   if (FFABS(sleep) > s->limit / s->speed)

I can do so. Who decides whether the hunks stays or goes? I'm
indecisive.

> LGTM apart from these nitpicks.

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

Re: [FFmpeg-devel] [PATCH V2 1/2] add support for ROI-based encoding

[Guo, Yejun]

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Michael Niedermayer
> Sent: Thursday, December 27, 2018 12:44 AM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH V2 1/2] add support for ROI-based
> encoding
> 
> On Wed, Dec 26, 2018 at 04:11:35AM +0800, Guo, Yejun wrote:
> > This patchset contains two patches.
> > - the first patch (this patch) finished the code and ask for upstream.
> > - the second patch is just a quick example on how to generate ROI info.
> >
> > The encoders such as libx264 support different QPs offset for
> > different MBs, it makes possible for ROI-based encoding. It makes
> > sense to add support within ffmpeg to generate/accept ROI infos and pass
> into encoders.
> >
> > Typical usage: After AVFrame is decoded, a ffmpeg filter or user's
> > code generates ROI info for that frame, and the encoder finally does
> > the ROI-based encoding.
> >
> > This patch just enabled the path from ffmpeg to libx264, the more
> > encoders can be added later.
> >
> > Signed-off-by: Guo, Yejun 
> > ---
> >  libavcodec/libx264.c | 40
> 
> >  libavutil/frame.c|  1 +
> >  libavutil/frame.h| 19 +++
> >  3 files changed, 60 insertions(+)
> 
> The commit message talks about 2 patches but this commit is one patch
> 

This patch is:  [FFmpeg-devel] [PATCH V2 1/2] add support for ROI-based 
encoding, see 
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-December/238135.html, 
and the other patch is:  [FFmpeg-devel] [PATCH V2 2/2] add an example to show 
how to fill   the ROI info , see 
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-December/238136.html. 

I sent out the two patches almost at the same time.

Thanks for pointing out the possible confusion, to make it clear, I will remove 
the relative descriptions in this patch, and keep the relative descriptions in 
the last patch (the patch that just for a reference, not for upstream).

> 
> >
> > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index
> > a68d0a7..a4f8677 100644
> > --- a/libavcodec/libx264.c
> > +++ b/libavcodec/libx264.c
> > @@ -40,6 +40,10 @@
> >  #include 
> >  #include 
> >
> > +// from x264.h, for quant_offsets, Macroblocks are 16x16 // blocks of
> > +pixels (with respect to the luma plane) #define MB_SIZE 16
> > +
> >  typedef struct X264Context {
> >  AVClass*class;
> >  x264_param_tparams;
> > @@ -345,6 +349,42 @@ static int X264_frame(AVCodecContext *ctx,
> AVPacket *pkt, const AVFrame *frame,
> >  }
> >  }
> >  }
> > +
> > +AVFrameSideData *sd = av_frame_get_side_data(frame,
> AV_FRAME_DATA_ROIS);
> > +if (sd != NULL) {
> > +if (x4->params.rc.i_aq_mode == X264_AQ_NONE) {
> > +av_log(ctx, AV_LOG_WARNING, "Adaptive quantization must be
> enabled to use ROI encoding, skipping ROI.\n");
> > +} else {
> > +if (frame->interlaced_frame == 0) {
> > +size_t mbx = (frame->width + MB_SIZE - 1) / MB_SIZE;
> > +size_t mby = (frame->height + MB_SIZE - 1) / MB_SIZE;
> > +float* qoffsets;
> > +qoffsets = (float*)av_malloc(sizeof(*qoffsets) * mbx * 
> > mby);
> > +if (qoffsets == NULL)
> > +return AVERROR(ENOMEM);
> > +memset(qoffsets, 0, sizeof(*qoffsets) * mbx *
> > + mby);
> 
> av_mallocz_array()
> 

thanks, will change to it.

> 
> 
> > +
> > +size_t nb_rois = sd->size / sizeof(AVROI);
> > +AVROI* rois = (AVROI*)sd->data;
> > +for (size_t roi = 0; roi < nb_rois; roi++) {
> > +int starty = FFMIN(mby, rois[roi].top / MB_SIZE);
> > +int endy = FFMIN(mby, (rois[roi].bottom + MB_SIZE 
> > - 1)/
> MB_SIZE);
> > +int startx = FFMIN(mbx, rois[roi].left / MB_SIZE);
> > +int endx = FFMIN(mbx, (rois[roi].right + MB_SIZE - 
> > 1)/
> MB_SIZE);
> > +for (int y = starty; y < endy; y++) {
> > +for (int x = startx; x < endx; x++) {
> > +qoffsets[x + y*mbx] = rois[roi].qoffset;
> > +}
> > +}
> > +}
> > +
> > +x4->pic.prop.quant_offsets = qoffsets;
> > +x4->pic.prop.quant_offsets_free = av_free;
> > +} else {
> > +av_log(ctx, AV_LOG_WARNING, "interlaced_frame not
> supported for ROI encoding yet, skipping ROI.\n");
> > +}
> > +}
> > +}
> >  }
> >
> >  do {
> > diff --git a/libavutil/frame.c b/libavutil/frame.c index
> > 34a6210..bebc50e 100644
> > --- a/libavutil/frame.c
> > +++ b/libavu

[FFmpeg-devel] [PATCH V3 1/3] avutil: add ROI data struct and bump version

[Guo, Yejun]

The encoders such as libx264 support different QPs offset for different MBs,
it makes possible for ROI-based encoding. It makes sense to add support
within ffmpeg to generate/accept ROI infos and pass into encoders.

Typical usage: After AVFrame is decoded, a ffmpeg filter or user's code
generates ROI info for that frame, and the encoder finally does the
ROI-based encoding.

The ROI info is maintained as side data of AVFrame.

Signed-off-by: Guo, Yejun 
---
 libavutil/frame.c   |  1 +
 libavutil/frame.h   | 19 +++
 libavutil/version.h |  2 +-
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index 34a6210..bebc50e 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -841,6 +841,7 @@ const char *av_frame_side_data_name(enum 
AVFrameSideDataType type)
 case AV_FRAME_DATA_QP_TABLE_DATA:   return "QP table data";
 #endif
 case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata 
SMPTE2094-40 (HDR10+)";
+case AV_FRAME_DATA_ROIS: return "Regions Of Interest";
 }
 return NULL;
 }
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 582ac47..f9e154c 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -173,6 +173,12 @@ enum AVFrameSideDataType {
  * volume transform - application 4 of SMPTE 2094-40:2016 standard.
  */
 AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
+
+/**
+ * Regions Of Interest, the data is an array of AVROI type, the array size
+ * is implied by AVFrameSideData::size / sizeof(AVROI).
+ */
+AV_FRAME_DATA_ROIS,
 };
 
 enum AVActiveFormatDescription {
@@ -200,6 +206,19 @@ typedef struct AVFrameSideData {
 AVBufferRef *buf;
 } AVFrameSideData;
 
+typedef struct AVROI {
+/* coordinates at frame pixel level.
+ * It will be extended internally if the codec requires an alignment.
+ * If the regions overlap, the last value in the list will be used.
+ */
+size_t top;
+size_t bottom;
+size_t left;
+size_t right;
+// quant offset is encoder dependent
+int qoffset;
+} AVROI;
+
 /**
  * This structure describes decoded (raw) audio or video data.
  *
diff --git a/libavutil/version.h b/libavutil/version.h
index f997615..1fcdea9 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  56
-#define LIBAVUTIL_VERSION_MINOR  25
+#define LIBAVUTIL_VERSION_MINOR  26
 #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

[FFmpeg-devel] [PATCH V3 2/3] avcodec/libx264: add support for ROI-based encoding

[Guo, Yejun]

This patch just enables the path from ffmpeg to libx264,
the more encoders can be added later.

Signed-off-by: Guo, Yejun 
---
 libavcodec/libx264.c | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index a68d0a7..a411f17 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -40,6 +40,10 @@
 #include 
 #include 
 
+// from x264.h, for quant_offsets, Macroblocks are 16x16
+// blocks of pixels (with respect to the luma plane)
+#define MB_SIZE 16
+
 typedef struct X264Context {
 AVClass*class;
 x264_param_tparams;
@@ -345,6 +349,41 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, 
const AVFrame *frame,
 }
 }
 }
+
+AVFrameSideData *sd = av_frame_get_side_data(frame, 
AV_FRAME_DATA_ROIS);
+if (sd != NULL) {
+if (x4->params.rc.i_aq_mode == X264_AQ_NONE) {
+av_log(ctx, AV_LOG_WARNING, "Adaptive quantization must be 
enabled to use ROI encoding, skipping ROI.\n");
+} else {
+if (frame->interlaced_frame == 0) {
+size_t mbx = (frame->width + MB_SIZE - 1) / MB_SIZE;
+size_t mby = (frame->height + MB_SIZE - 1) / MB_SIZE;
+float* qoffsets;
+qoffsets = (float*)av_mallocz_array(mbx * mby, 
sizeof(*qoffsets));
+if (qoffsets == NULL)
+return AVERROR(ENOMEM);
+
+size_t nb_rois = sd->size / sizeof(AVROI);
+AVROI* rois = (AVROI*)sd->data;
+for (size_t roi = 0; roi < nb_rois; roi++) {
+int starty = FFMIN(mby, rois[roi].top / MB_SIZE);
+int endy = FFMIN(mby, (rois[roi].bottom + MB_SIZE - 
1)/ MB_SIZE);
+int startx = FFMIN(mbx, rois[roi].left / MB_SIZE);
+int endx = FFMIN(mbx, (rois[roi].right + MB_SIZE - 1)/ 
MB_SIZE);
+for (int y = starty; y < endy; y++) {
+for (int x = startx; x < endx; x++) {
+qoffsets[x + y*mbx] = rois[roi].qoffset;
+}
+}
+}
+
+x4->pic.prop.quant_offsets = qoffsets;
+x4->pic.prop.quant_offsets_free = av_free;
+} else {
+av_log(ctx, AV_LOG_WARNING, "interlaced_frame not 
supported for ROI encoding yet, skipping ROI.\n");
+}
+}
+}
 }
 
 do {
-- 
2.7.4

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

[FFmpeg-devel] [PATCH V3 3/3] add an example to show how to fill the ROI info

[Guo, Yejun]

This patch is just a quick example to show how to fill the ROI info,
it does not ask for upstreaming, just for your reference.

to verify the ROI feature with this example, the command line looks like:
./ffmpeg -i .../path_to_1920x1080_video_file -vf scale=1920:1080 -c:v libx264 
-b:v 2000k -y tmp.264

Signed-off-by: Guo, Yejun 
---
 libavfilter/vf_scale.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index f741419..561391c 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -437,6 +437,20 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
 return ret;
 }
 
+// just to show how a filter fills the ROI info
+size_t nb_rois = 1;
+AVFrameSideData *sd = av_frame_new_side_data(in, AV_FRAME_DATA_ROIS, 
nb_rois * sizeof(AVROI));
+if (!sd) {
+av_frame_free(&in);
+return AVERROR(ENOMEM);
+}
+AVROI* rois = (AVROI*)sd->data;
+rois[0].top = 0;
+rois[0].left = 0;
+rois[0].bottom = in->height;
+rois[0].right = in->width/2;
+rois[0].qoffset = -15;
+
 if (!scale->sws)
 return ff_filter_frame(outlink, in);
 
-- 
2.7.4

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

[FFmpeg-devel] [PATCH] avcodec/mips: [loongson] optimize theora decoding in vp3dsp.

[gxw]

Optimize theora decoding with msa in functions:
1. ff_vp3_idct_add_msa
2. ff_vp3_idct_put_msa
3. ff_vp3_idct_dc_add_msa
4. ff_vp3_v_loop_filter_msa
5. ff_vp3_h_loop_filter_msa
6. ff_put_no_rnd_pixels_l2_msa

Theora decoding speed improved about 36%(from 22fps to 30fps, Tested on 
loongson 2K1000).
---
 libavcodec/mips/Makefile   |   2 +
 libavcodec/mips/vp3dsp_idct_msa.c  | 662 +
 libavcodec/mips/vp3dsp_init_mips.c |  46 +++
 libavcodec/mips/vp3dsp_mips.h  |  37 +++
 libavcodec/vp3dsp.c|   2 +
 libavcodec/vp3dsp.h|   1 +
 6 files changed, 750 insertions(+)
 create mode 100644 libavcodec/mips/vp3dsp_idct_msa.c
 create mode 100644 libavcodec/mips/vp3dsp_init_mips.c
 create mode 100644 libavcodec/mips/vp3dsp_mips.h

diff --git a/libavcodec/mips/Makefile b/libavcodec/mips/Makefile
index 1f659a0..3571207 100644
--- a/libavcodec/mips/Makefile
+++ b/libavcodec/mips/Makefile
@@ -22,6 +22,7 @@ OBJS-$(CONFIG_HEVC_DECODER)   += 
mips/hevcdsp_init_mips.o  \
  mips/hevcpred_init_mips.o
 OBJS-$(CONFIG_VP9_DECODER)+= mips/vp9dsp_init_mips.o
 OBJS-$(CONFIG_VP8_DECODER)+= mips/vp8dsp_init_mips.o
+OBJS-$(CONFIG_VP3DSP) += mips/vp3dsp_init_mips.o
 OBJS-$(CONFIG_H264DSP)+= mips/h264dsp_init_mips.o
 OBJS-$(CONFIG_H264QPEL)   += mips/h264qpel_init_mips.o
 OBJS-$(CONFIG_H264CHROMA) += mips/h264chroma_init_mips.o
@@ -54,6 +55,7 @@ MSA-OBJS-$(CONFIG_VP9_DECODER)+= 
mips/vp9_mc_msa.o \
 MSA-OBJS-$(CONFIG_VP8_DECODER)+= mips/vp8_mc_msa.o \
  mips/vp8_idct_msa.o   \
  mips/vp8_lpf_msa.o
+MSA-OBJS-$(CONFIG_VP3DSP) += mips/vp3dsp_idct_msa.o
 MSA-OBJS-$(CONFIG_H264DSP)+= mips/h264dsp_msa.o\
  mips/h264idct_msa.o
 MSA-OBJS-$(CONFIG_H264QPEL)   += mips/h264qpel_msa.o
diff --git a/libavcodec/mips/vp3dsp_idct_msa.c 
b/libavcodec/mips/vp3dsp_idct_msa.c
new file mode 100644
index 000..5427ac5
--- /dev/null
+++ b/libavcodec/mips/vp3dsp_idct_msa.c
@@ -0,0 +1,662 @@
+/*
+ * Copyright (c) 2018 gxw 
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "vp3dsp_mips.h"
+#include "libavutil/mips/generic_macros_msa.h"
+#include "libavutil/intreadwrite.h"
+#include "libavcodec/rnd_avg.h"
+
+static void idct_msa(uint8_t *dst, int stride, int16_t *input, int type)
+{
+v8i16 r0, r1, r2, r3, r4, r5, r6, r7, sign;
+v4i32 r0_r, r0_l, r1_r, r1_l, r2_r, r2_l, r3_r, r3_l,
+  r4_r, r4_l, r5_r, r5_l, r6_r, r6_l, r7_r, r7_l;
+v4i32 A, B, C, D, Ad, Bd, Cd, Dd, E, F, G, H;
+v4i32 Ed, Gd, Add, Bdd, Fd, Hd;
+v16u8 sign_l;
+v16i8 d0, d1, d2, d3, d4, d5, d6, d7;
+v4i32 c0, c1, c2, c3, c4, c5, c6, c7;
+v4i32 f0, f1, f2, f3, f4, f5, f6, f7;
+v4i32 sign_t;
+v16i8 zero = {0};
+v16i8 mask = {0, 4, 8, 12, 16, 20, 24, 28, 0, 0, 0, 0, 0, 0, 0, 0};
+v4i32 cnst64277w = {64277, 64277, 64277, 64277};
+v4i32 cnst60547w = {60547, 60547, 60547, 60547};
+v4i32 cnst54491w = {54491, 54491, 54491, 54491};
+v4i32 cnst46341w = {46341, 46341, 46341, 46341};
+v4i32 cnst36410w = {36410, 36410, 36410, 36410};
+v4i32 cnst25080w = {25080, 25080, 25080, 25080};
+v4i32 cnst12785w = {12785, 12785, 12785, 12785};
+v4i32 cnst8w = {8, 8, 8, 8};
+v4i32 cnst2048w = {2048, 2048, 2048, 2048};
+v4i32 cnst128w = {128, 128, 128, 128};
+int nstride = stride;
+
+/* Extended input data */
+LD_SH8(input, 8, r0, r1, r2, r3, r4, r5, r6, r7);
+sign = __msa_clti_s_h(r0, 0);
+r0_r = (v4i32) __msa_ilvr_h(sign, r0);
+r0_l = (v4i32) __msa_ilvl_h(sign, r0);
+sign = __msa_clti_s_h(r1, 0);
+r1_r = (v4i32) __msa_ilvr_h(sign, r1);
+r1_l = (v4i32) __msa_ilvl_h(sign, r1);
+sign = __msa_clti_s_h(r2, 0);
+r2_r = (v4i32) __msa_ilvr_h(sign, r2);
+r2_l = (v4i32) __msa_ilvl_h(sign, r2);
+sign = __msa_clti_s_h(r3, 0);
+r3_r = (v4i32) __msa_ilvr_h(sign, r3);
+r3_l = (v4i32) __msa