Re: [FFmpeg-devel] [RFC] avpriv cleanup

[wm4]

On Sun, 25 Mar 2018 23:46:25 +0200
Michael Niedermayer  wrote:

> On Sun, Mar 25, 2018 at 05:55:37PM +0200, wm4 wrote:
> > On Sun, 25 Mar 2018 17:34:51 +0200
> > Michael Niedermayer  wrote:
> >   
> > > On Sun, Mar 25, 2018 at 05:07:33PM +0200, wm4 wrote:  
> > > > On Sun, 25 Mar 2018 17:00:32 +0200
> > > > Michael Niedermayer  wrote:
> > > > 
> > > > > Hi all
> > > > > 
> > > > > Nicolas wrote an interresting comment about avpriv, and as this better
> > > > > belongs into a new thread, here it is in a new thread
> > > > > 
> > > > > On Sun, Mar 25, 2018 at 03:32:33PM +0200, Nicolas George wrote:
> > > > > > Josh de Kock (2018-03-23):  
> > > > > [...]
> > > > >  
> > > > > > You can observe just that by the fact that you needed to add an 
> > > > > > avpriv
> > > > > > function to let lavd communicate with lavf. Each time an avpriv 
> > > > > > function
> > > > > > appears, it shows there is something flawed in the design.  
> > > > > 
> > > > > If this is true, in general, then we can improve designs by removing
> > > > > avpriv_* functions ...
> > > > > 
> > > > > looking at what we have as avprivs, in the tree, i think some of these
> > > > > could be indeed usefull as public APIs. And we need to already keep 
> > > > > them
> > > > > compatible as they are exported as avpriv too, so making such changes 
> > > > > does
> > > > > indeed in some cases look like a good idea to me
> > > > > 
> > > > > There are some avprivs we have currently though that are quite 
> > > > > specific
> > > > > to format intgernals, i dont think that its always a flaw to use 
> > > > > avpriv
> > > > > functions thus
> > > > > 
> > > > > but i think we should evaluate each of the currently existing avpriv
> > > > > functions and check if the API wouldnt be usefull to user 
> > > > > applications.
> > > > > And if it wouldnt be better to make it properly public
> > > > 
> > >   
> > > > This might apply to some cases, but in general: just no.
> > > 
> > > That is what i meant :)
> > > 
> > >   
> > > > 
> > > > We make things avpriv_ *because* we don't want them to be public API,
> > > > but the odd multiple-library design forces us to. Not more, not less.
> > > > 
> > >   
> > > > What is this thread even for? If you want to make certain functions
> > > > public, post specific RFCs or patches. In general, we probably all
> > > > agree that avpriv functions are necessary hacks to deal with the split
> > > > library nonsense.
> > > 
> > > The thread is for finding out peoples oppinon before anyone spends time
> > > writing patches.
> > > For example it takes time to go over the functions and time to write 
> > > patches
> > > that turn functions with proper documention into public functions.
> > > 
> > > If people object to it, no point in anyone doing any of this work
> > > 
> > > Also there have been people looking for things to work on. If we agree
> > > that looking over avpriv functions for more generally usefull ones 
> > > makes sense then we need to discuss this in some thread for anyone
> > > to be able to see that suggestion.  
> > 
> > Well, just generally suggesting to remove avpriv functions is not going
> > to work, because they exist for a reason (see e.g. Nicolas George or my
> > post). Every specific case likely needs a separate discussion, so it
> > doesn't make sense to have some sort of summary discussion.  
> 
> well, the summary discussion certainly is very effective to get a feeling
> for peoples oppinion on this in general.
> 
> 
> >   
> > > For a specific function
> > > As a random example, lets take avpriv_set_systematic_pal2()
> > > It provides a palette for 8bit per pixel formats which have a constant
> > > palette. It sounds usefull to have this public to me ...  
> > 
> > For that specific example I'd rather argue we should get rid of
> > AV_PIX_FMT_FLAG_PSEUDOPAL. It's an awful hack to allow code to treat 1
> > byte formats as paletted, such as AV_PIXFMT_GRAY8. Basically AVFrames
> > with this pixfmt must come with a palette that acts as lookup table,
> > that treats the 1 byte value as index into the table to convert it to
> > RGB. The avpriv_set_systematic_pal2() just sets the palette for those
> > formats.
> > 
> > This is broken and bad for the following reasons:
> >   
> 
> > 1. The affected pixfmts are NOT paletted. So they shouldn't have a
> >palette. It's against the principle of least astonishment (basically
> >surprising the user with some broken obscure bullshit, instead of
> >things just working). The only reason why I know about this pseudi
> >PAL feature is because once I got a segfault by passing a self
> >allocated GRAY8 frame to libswscale.  
> 
> avpriv_set_systematic_pal2() sets the palette you where missing, that
> caused the issue.
> how is that an argument to not make it public !?

I already described how the function you want to make 

[FFmpeg-devel] [V2 2/2] lavc/vaapi_encode: fix to set the default max bitrate for VBR

[Pengfei Qu]

The current RC(CBR/VBR) mode in ffmpeg-vaapi depends on the target bitrate
and max bitrate. CBR when target bitrate equal to max bitrate, or VBR.

Here is the background to have this solution. One issue I saw when doing
transcoding Mpeg2(interlace)->AVC on VBR mode, the real output bitrate go
away too much compared to target bitrate when there is no max bitrate
parameter in the command line. It work well when set the correct max
bitrate parameter in the command line.

And for VBR mode in the codec, generally the max bit rate is bigger than the 
taraget
bitrate. For CBR mode, the max bitrate is same as the target bitrate.
When there is no specfic setting for the max bit rate parameter in the command,
here the default value 95% is used to caculate the default max bitrate
accordingly. The original code set max bitrate same as the target bitrate
with the rc_target_percentage=100. The value "95" is also the experienced
value and here give an initial max bitrate closer to the target bitrate
and avoid bigger fluctuation.

Signed-off-by: Pengfei Qu 
---
 libavcodec/vaapi_encode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 78347d4..47110cf 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1164,8 +1164,8 @@ static av_cold int 
vaapi_encode_init_rate_control(AVCodecContext *avctx)
 } else {
 if (avctx->rc_max_rate < avctx->bit_rate) {
 // Max rate is unset or invalid, just use the normal bitrate.
-rc_bits_per_second   = avctx->bit_rate;
-rc_target_percentage = 100;
+rc_target_percentage = 95;
+rc_bits_per_second   = (unsigned long)(avctx->bit_rate * 100.0 / 
rc_target_percentage);
 } else {
 rc_bits_per_second   = avctx->rc_max_rate;
 rc_target_percentage = (unsigned long)(avctx->bit_rate * 100) / 
rc_bits_per_second;
-- 
2.9.3

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

[FFmpeg-devel] [V2 1/2] lavc/vaapi_encode: fix the caculation overflow

[Pengfei Qu]

this fix the overflow during the caculation before value assignment.

Signed-off-by: Pengfei Qu 
---
 libavcodec/vaapi_encode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 36c85a3..78347d4 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1168,9 +1168,9 @@ static av_cold int 
vaapi_encode_init_rate_control(AVCodecContext *avctx)
 rc_target_percentage = 100;
 } else {
 rc_bits_per_second   = avctx->rc_max_rate;
-rc_target_percentage = (avctx->bit_rate * 100) / 
rc_bits_per_second;
+rc_target_percentage = (unsigned long)(avctx->bit_rate * 100) / 
rc_bits_per_second;
 }
-rc_window_size = (hrd_buffer_size * 1000) / avctx->bit_rate;
+rc_window_size = (unsigned long)(hrd_buffer_size * 1000) / 
avctx->bit_rate;
 }
 
 ctx->rc_params.misc.type = VAEncMiscParameterTypeRateControl;
-- 
2.9.3

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

Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: initialize saveptrs

[Jeyapal, Karthick]

On 3/25/18 1:43 PM, Timo Rothenpieler wrote:
> Am 21.03.2018 um 20:37 schrieb Timo Rothenpieler:
>> Am 21.03.2018 um 20:33 schrieb Timo Rothenpieler:
>>> av_strtok calls strspn on a non-NULL *saveptr, so not NULL 
>>> initializing it is an issue.
>>>
>>> Fixes CID #1428568
>>> ---
>>>   libavformat/hlsenc.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>> index b7c6fbde6a..fa17776efe 100644
>>> --- a/libavformat/hlsenc.c
>>> +++ b/libavformat/hlsenc.c
>>> @@ -1873,7 +1873,8 @@ static int 
>>> parse_cc_stream_mapstring(AVFormatContext *s)
>>>   {
>>>   HLSContext *hls = s->priv_data;
>>>   int nb_ccstreams;
>>> -char *p, *q, *saveptr1, *saveptr2, *ccstr, *keyval;
>>> +char *p, *q, *ccstr, *keyval;
>>> +char *saveptr1 = NULL, *saveptr2 = NULL;
>>>   const char *val;
>>>   ClosedCaptionsStream *ccs;
>>
>> Just realized, the more correct approach is probably to check the 
>> av_strdup below this for ENOMEM. Not sure about the exact semantics 
>> there, initializing these still seems like a good safety measure.
>>
I agree with you that checking the return value of av_strdup for NULL is a 
better approach.
But I am fine with this initialization fix as well.
>
> ping
>



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

[FFmpeg-devel] [PATCH V2] ffmpeg_filter: enable stream_loop in HWAccel transcoding.

[Jun Zhao]

V2: simplified the logic as Michael's review.

From 6c332c65d64977c4d6220acfb7e9db3505281f87 Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Wed, 14 Mar 2018 16:13:39 +0800
Subject: [PATCH V2] ffmpeg_filter: enable stream_loop in HWAccel transcoding.

use the cmd: ffmpeg -y -stream_loop 1 -hwaccel vaapi -hwaccel_device
/dev/dri/renderD128 -hwaccel_output_format vaapi -i
input.mp4 -c:v h264_vaapi output.mp4 can get the error like:

Error while decoding stream #0:1: Invalid data found when processing
input
Impossible to convert between the formats supported by the filter
'Parsed_null_0' and the filter 'auto_scaler_0'
Error reinitializing filters!
Failed to inject frame into filter network: Function not implemented

the root cause is can't insert software scale filter in the hwaccel
transcoding pipeline.

Signed-off-by: Jun Zhao 
---
 fftools/ffmpeg_filter.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 877fd670e6..4e5d8fa24e 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -452,6 +452,7 @@ static int configure_output_video_filter(FilterGraph *fg, 
OutputFilter *ofilter,
 int pad_idx = out->pad_idx;
 int ret;
 char name[255];
+const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(ofilter->format);
 
 snprintf(name, sizeof(name), "out_%d_%d", ost->file_index, ost->index);
 ret = avfilter_graph_create_filter(>filter,
@@ -461,7 +462,8 @@ static int configure_output_video_filter(FilterGraph *fg, 
OutputFilter *ofilter,
 if (ret < 0)
 return ret;
 
-if (ofilter->width || ofilter->height) {
+if ((ofilter->width || ofilter->height) &&
+(!desc || !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL))) {
 char args[255];
 AVFilterContext *filter;
 AVDictionaryEntry *e = NULL;
-- 
2.14.1

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

[FFmpeg-devel] [PATCH] avcodec/avpacket: remove unnecessary check in av_packet_make_writable()

[James Almer]

Zero sized packets are already handled below in the function.
This is more in line with av_packet_ref().

Signed-off-by: James Almer 
---
 libavcodec/avpacket.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 0693ca6f62..0993481961 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -660,9 +660,6 @@ int av_packet_make_writable(AVPacket *pkt)
 if (pkt->buf && av_buffer_is_writable(pkt->buf))
 return 0;
 
-if (!pkt->data)
-return AVERROR(EINVAL);
-
 ret = packet_alloc(, pkt->size);
 if (ret < 0)
 return ret;
-- 
2.16.2

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

[FFmpeg-devel] [PATCH] kmsgrab: add category for kmsgrab

[Jun Zhao]

From fc9b04d95b1e21fa9985dc611fcf43d2a4c635c0 Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Fri, 23 Mar 2018 21:32:03 +0800
Subject: [PATCH] kmsgrab: add category for kmsgrab

add category for kmsgrab, then we can display kmsgrab in
"ffmpeg -devices".

Signed-off-by: Jun Zhao 
---
 libavdevice/kmsgrab.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
index 6a6de09c37..d0de774871 100644
--- a/libavdevice/kmsgrab.c
+++ b/libavdevice/kmsgrab.c
@@ -451,6 +451,7 @@ static const AVClass kmsgrab_class = {
 .item_name  = av_default_item_name,
 .option = options,
 .version= LIBAVUTIL_VERSION_INT,
+.category   = AV_CLASS_CATEGORY_DEVICE_VIDEO_INPUT,
 };
 
 AVInputFormat ff_kmsgrab_demuxer = {
-- 
2.14.1

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

Re: [FFmpeg-devel] [PATCH 2/2] doc/examples/hw_decode: Remove logically dead code in decode_write()

[Jun Zhao]

On 2018/3/26 2:21, Michael Niedermayer wrote:
> Fixes CID1415951
>
> Signed-off-by: Michael Niedermayer 
> ---
>  doc/examples/hw_decode.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/doc/examples/hw_decode.c b/doc/examples/hw_decode.c
> index 74a0ca32db..77ae8df35b 100644
> --- a/doc/examples/hw_decode.c
> +++ b/doc/examples/hw_decode.c
> @@ -86,7 +86,7 @@ static int decode_write(AVCodecContext *avctx, AVPacket 
> *packet)
>  return ret;
>  }
>  
> -while (ret >= 0) {
> +while (1) {
>  if (!(frame = av_frame_alloc()) || !(sw_frame = av_frame_alloc())) {
>  fprintf(stderr, "Can not alloc frame\n");
>  ret = AVERROR(ENOMEM);
> @@ -142,8 +142,6 @@ static int decode_write(AVCodecContext *avctx, AVPacket 
> *packet)
>  if (ret < 0)
>  return ret;
>  }
> -
> -return 0;
>  }
>  
>  int main(int argc, char *argv[])
LGTM both patches
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/mpeg4videodec: Use more specific error codes

[Michael Niedermayer]

On Sat, Mar 10, 2018 at 10:17:05PM +0100, Michael Niedermayer wrote:
> Forward error codes where possible.
> 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/mpeg4video.h|   4 +-
>  libavcodec/mpeg4videodec.c | 100 
> +++--
>  2 files changed, 53 insertions(+), 51 deletions(-)

will apply

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

What does censorship reveal? It reveals fear. -- Julian Assange


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

Re: [FFmpeg-devel] [PATCH 2/5] h264_metadata: Add support for A/53 closed captions

[James Almer]

On 3/25/2018 2:41 PM, Mark Thompson wrote:
> Allows insertion (from side data), extraction (to side data), and removal
> of closed captions in SEI messages.
> ---
>  libavcodec/Makefile|   2 +-
>  libavcodec/h264_metadata_bsf.c | 138 
> +
>  2 files changed, 139 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index aaef6c3ab8..cfde104055 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -1044,7 +1044,7 @@ OBJS-$(CONFIG_DCA_CORE_BSF)   += 
> dca_core_bsf.o
>  OBJS-$(CONFIG_EXTRACT_EXTRADATA_BSF)  += extract_extradata_bsf.o\
>   h2645_parse.o
>  OBJS-$(CONFIG_FILTER_UNITS_BSF)   += filter_units_bsf.o
> -OBJS-$(CONFIG_H264_METADATA_BSF)  += h264_metadata_bsf.o
> +OBJS-$(CONFIG_H264_METADATA_BSF)  += h264_metadata_bsf.o cbs_misc.o
>  OBJS-$(CONFIG_H264_MP4TOANNEXB_BSF)   += h264_mp4toannexb_bsf.o
>  OBJS-$(CONFIG_H264_REDUNDANT_PPS_BSF) += h264_redundant_pps_bsf.o
>  OBJS-$(CONFIG_HAPQA_EXTRACT_BSF)  += hapqa_extract_bsf.o hap.o
> diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
> index 27053dbdcf..d79e6c0c87 100644
> --- a/libavcodec/h264_metadata_bsf.c
> +++ b/libavcodec/h264_metadata_bsf.c
> @@ -24,6 +24,7 @@
>  #include "bsf.h"
>  #include "cbs.h"
>  #include "cbs_h264.h"
> +#include "cbs_misc.h"
>  #include "h264.h"
>  #include "h264_sei.h"
>  
> @@ -74,6 +75,8 @@ typedef struct H264MetadataContext {
>  int display_orientation;
>  double rotate;
>  int flip;
> +
> +int a53_cc;
>  } H264MetadataContext;
>  
>  
> @@ -222,6 +225,8 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
> AVPacket *out)
>  int err, i, j, has_sps;
>  uint8_t *displaymatrix_side_data = NULL;
>  size_t displaymatrix_side_data_size = 0;
> +uint8_t *a53_side_data = NULL;
> +size_t a53_side_data_size = 0;
>  
>  err = ff_bsf_get_packet(bsf, );
>  if (err < 0)
> @@ -516,6 +521,116 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
> AVPacket *out)
>  }
>  }
>  
> +if (ctx->a53_cc == INSERT) {

This function is becoming pretty big. Could you split it, either before
or after this patch, for readability sake? One function per each
AVOption with pass/insert/remove, basically.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/mov: Move +1 in check to avoid hypothetical overflow in add_ctts_entry()

[Michael Niedermayer]

On Sat, Feb 03, 2018 at 10:45:24PM +0100, Michael Niedermayer wrote:
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/mov.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

will apply

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

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA


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

Re: [FFmpeg-devel] [PATCH] ffmpeg_filter: enable stream_loop in HWAccel transcoding.

[Jun Zhao]

On 2018/3/24 8:14, Michael Niedermayer wrote:
> On Wed, Mar 14, 2018 at 04:26:54PM +0800, Jun Zhao wrote:
>>  ffmpeg_filter.c |4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 170327a7137d3ce26124c86525566d32c523a948  
>> 0001-ffmpeg_filter-enable-stream_loop-in-HWAccel-transcod.patch
>> From 731b6cb1f3a13fa18cfe39c1ddba92050b999668 Mon Sep 17 00:00:00 2001
>> From: Jun Zhao 
>> Date: Wed, 14 Mar 2018 16:13:39 +0800
>> Subject: [PATCH] ffmpeg_filter: enable stream_loop in HWAccel transcoding.
>>
>> use the cmd: ffmpeg -y -stream_loop 1 -hwaccel vaapi -hwaccel_device
>> /dev/dri/renderD128 -hwaccel_output_format vaapi -i
>> input.mp4 -c:v h264_vaapi output.mp4 can get the error like:
>>
>> Error while decoding stream #0:1: Invalid data found when processing
>> input
>> Impossible to convert between the formats supported by the filter
>> 'Parsed_null_0' and the filter 'auto_scaler_0'
>> Error reinitializing filters!
>> Failed to inject frame into filter network: Function not implemented
>>
>> the root cause is can't insert software scale filter in the hwaccel
>> transcoding pipeline.
>>
>> Signed-off-by: Jun Zhao 
>> ---
>>  fftools/ffmpeg_filter.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
>> index 877fd670e6..c85dd7ae8d 100644
>> --- a/fftools/ffmpeg_filter.c
>> +++ b/fftools/ffmpeg_filter.c
>> @@ -452,6 +452,7 @@ static int configure_output_video_filter(FilterGraph 
>> *fg, OutputFilter *ofilter,
>>  int pad_idx = out->pad_idx;
>>  int ret;
>>  char name[255];
>> +const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(ofilter->format);
>>  
>>  snprintf(name, sizeof(name), "out_%d_%d", ost->file_index, ost->index);
>>  ret = avfilter_graph_create_filter(>filter,
>> @@ -461,7 +462,8 @@ static int configure_output_video_filter(FilterGraph 
>> *fg, OutputFilter *ofilter,
>>  if (ret < 0)
>>  return ret;
>>  
>> -if (ofilter->width || ofilter->height) {
>> +if ((ofilter->width || ofilter->height) &&
>> +(!desc || (desc && !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL {
> this can be simplified because after "!desc ||" desc is not NULL
>
> about the patch itself, ill leave this to the people activly working on 
> hwaccel
Yes, local have simplified the logic, will submit V2, Tks.
>
> [...]
>
>
>
> ___
> 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

[FFmpeg-devel] (no subject)

[dylanf123]

Thanks, fixed

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

Re: [FFmpeg-devel] [PATCH] avcodec/get_bits: Make sure the input bitstream with padding can be addressed

[Michael Niedermayer]

On Sat, Mar 24, 2018 at 01:56:26AM +0100, Michael Niedermayer wrote:
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/get_bits.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

will apply

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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.


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

Re: [FFmpeg-devel] [PATCH] avcodec/get_bits: Document skip_bits_long()

[Michael Niedermayer]

apparently i failed to send this reply after writing it

On Fri, Mar 23, 2018 at 10:02:39PM +0100, Thilo Borgmann wrote:
> Am 23.03.18 um 20:20 schrieb Michael Niedermayer:
> > Found-by: Kieran
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/get_bits.h | 7 +++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
> > index 0c7f5ff0c6..3ec45e7ab6 100644
> > --- a/libavcodec/get_bits.h
> > +++ b/libavcodec/get_bits.h
> > @@ -201,6 +201,13 @@ static inline int get_bits_count(const GetBitContext 
> > *s)
> >  return s->index;
> >  }
> >  
> > +/**
> > + * Skips the specified number of bits.
> > + * @param n the number of bits to skip,
> > + *  For the UNCHECKED_BITSTREAM_READER this must not cause the 
> > distance
> > + *  from the start to overflow int32_t. Staying within the 
> > bitstream + padding
> > + *  is sufficient too.
>   ^^
> Shouldn't this be "required" or "necessary"?

The bitstream reader must be able to index the whole bitstream and the padding
otherwise it would have some issues, actually it possibly does have a bug there
i need to double check this

currently it uses int32, so if the input is bigger the reader will reject this
so just staying within that size should be fine
that is unless iam missing something

[...]


> And nit: "something, too."

will fix

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

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.


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

[FFmpeg-devel] [PATCH] avfilter/vf_avgblur_opencl: fix error when clSetKernelArg fails

[dylanf123]

From: drfer3 

Fixes Coverity CID 1430382
---
 libavfilter/vf_avgblur_opencl.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavfilter/vf_avgblur_opencl.c b/libavfilter/vf_avgblur_opencl.c
index 5ee66c0ba2..53db83c21f 100644
--- a/libavfilter/vf_avgblur_opencl.c
+++ b/libavfilter/vf_avgblur_opencl.c
@@ -155,18 +155,21 @@ static int avgblur_opencl_filter_frame(AVFilterLink 
*inlink, AVFrame *input)
 if (cle != CL_SUCCESS) {
 av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
"destination image argument: %d.\n", cle);
+err = AVERROR_UNKNOWN;
 goto fail;
 }
 cle = clSetKernelArg(ctx->kernel_horiz, 1, sizeof(cl_mem), );
 if (cle != CL_SUCCESS) {
 av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
"source image argument: %d.\n", cle);
+err = AVERROR_UNKNOWN;
 goto fail;
 }
 cle = clSetKernelArg(ctx->kernel_horiz, 2, sizeof(cl_int), _x);
 if (cle != CL_SUCCESS) {
 av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
"sizeX argument: %d.\n", cle);
+err = AVERROR_UNKNOWN;
 goto fail;
 }
 
@@ -191,18 +194,21 @@ static int avgblur_opencl_filter_frame(AVFilterLink 
*inlink, AVFrame *input)
 if (cle != CL_SUCCESS) {
 av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
"destination image argument: %d.\n", cle);
+err = AVERROR_UNKNOWN;
 goto fail;
 }
 cle = clSetKernelArg(ctx->kernel_vert, 1, sizeof(cl_mem), );
 if (cle != CL_SUCCESS) {
 av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
"source image argument: %d.\n", cle);
+err = AVERROR_UNKNOWN;
 goto fail;
 }
 cle = clSetKernelArg(ctx->kernel_vert, 2, sizeof(cl_int), _y);
 if (cle != CL_SUCCESS) {
 av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
"sizeY argument: %d.\n", cle);
+err = AVERROR_UNKNOWN;
 goto fail;
 }
 
-- 
2.14.3 (Apple Git-98)

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

Re: [FFmpeg-devel] [PATCH] avfilter/vf_scale: Do not set YUV color range for RGB formats

[Michael Niedermayer]

On Wed, Mar 21, 2018 at 09:18:21AM +0100, Paul B Mahol wrote:
> On 3/20/18, Michael Niedermayer  wrote:
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavfilter/vf_scale.c | 7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> > index 9f45032e85..2f6fa4791d 100644
> > --- a/libavfilter/vf_scale.c
> > +++ b/libavfilter/vf_scale.c
> > @@ -407,6 +407,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
> >  AVFilterLink *outlink = link->dst->outputs[0];
> >  AVFrame *out;
> >  const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
> > +const AVPixFmtDescriptor *out_desc =
> > av_pix_fmt_desc_get(outlink->format);
> >  char buf[32];
> >  int in_range;
> >
> > @@ -497,7 +498,11 @@ static int filter_frame(AVFilterLink *link, AVFrame
> > *in)
> >   table, out_full,
> >   brightness, contrast, saturation);
> >
> > -out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG;
> > +// color_range describes YUV, it is undefined for RGB
> > +if ((out_desc->flags & AV_PIX_FMT_FLAG_RGB) &&
> > out_desc->nb_components != 1) {
> > +out->color_range = AVCOL_RANGE_UNSPECIFIED;
> > +} else
> > +out->color_range = out_full ? AVCOL_RANGE_JPEG :
> > AVCOL_RANGE_MPEG;
> >  }
> >
> >  av_reduce(>sample_aspect_ratio.num, >sample_aspect_ratio.den,
> > --
> > 2.16.2
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> 
> This is not optimal, as full color_range should remain full when not changed.

there is no range for rgb formats. The range is specific to YUV based
formats. 
Thats, if iam guessing correctly what you meant. You did not really say
which case you meant here. So maybe there is an issue and i misunderstand
what you refer to

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.


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

Re: [FFmpeg-devel] [PATCH] avfilter/vf_scale: Do not set YUV color range for RGB formats

[Michael Niedermayer]

On Wed, Mar 21, 2018 at 03:04:35PM +0100, wm4 wrote:
> On Tue, 20 Mar 2018 23:59:46 +0100
> Michael Niedermayer  wrote:
> 
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavfilter/vf_scale.c | 7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> > index 9f45032e85..2f6fa4791d 100644
> > --- a/libavfilter/vf_scale.c
> > +++ b/libavfilter/vf_scale.c
> > @@ -407,6 +407,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
> >  AVFilterLink *outlink = link->dst->outputs[0];
> >  AVFrame *out;
> >  const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
> > +const AVPixFmtDescriptor *out_desc = 
> > av_pix_fmt_desc_get(outlink->format);
> >  char buf[32];
> >  int in_range;
> >  
> > @@ -497,7 +498,11 @@ static int filter_frame(AVFilterLink *link, AVFrame 
> > *in)
> >   table, out_full,
> >   brightness, contrast, saturation);
> >  
> > -out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG;
> > +// color_range describes YUV, it is undefined for RGB
> > +if ((out_desc->flags & AV_PIX_FMT_FLAG_RGB) && 
> > out_desc->nb_components != 1) {
> > +out->color_range = AVCOL_RANGE_UNSPECIFIED;
> > +} else
> > +out->color_range = out_full ? AVCOL_RANGE_JPEG : 
> > AVCOL_RANGE_MPEG;
> >  }
> >  
> >  av_reduce(>sample_aspect_ratio.num, >sample_aspect_ratio.den,
> 
> Why is it not setting full range for gray? PNGs can use this for
> grayscale images, so they would be affected by this too.

I do not understand what you describe here. Please describe the issue
clearer and or provide a testcase.

there is no unique gray pixel format.
gray8 is not a AV_PIX_FMT_FLAG_RGB format, thus its treated the same as
before this patch



[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf


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

Re: [FFmpeg-devel] [RFC] avpriv cleanup

[Michael Niedermayer]

On Sun, Mar 25, 2018 at 10:24:31PM +0200, Nicolas George wrote:
[...]
> > If they used distro packages then they wouldn't have any power over what
> > gets built or shipped. Distro packages are the "Enable everything" kind
> > of build, so I'm of course talking about projects shipping their own
> > builds of the libraries.
> 
> I have nothing to add to that. But you only answered half the question.
> The other half was: do they use static or dynamic linking?
> 
> I am not sure how familiar you are with the precise workings of both
> solutions, so let me summarize a bit.
> 
> With static linking, libraries are just archives or object files (*.o),
> with an index of symbols. The linker will examine each archive in turn
> and select the object files that contains required symbols.
> 
> With dynamic linking, the linker takes notes of all the libraries,
> checks that all symbols are resolved and registers as dependencies of
> the executable file.
> 

> What is remarkable about static linking is that it only takes the object
> files that are necessary, nothing more. It does not matter whether there
> are one library, eight or two thousands (one for each object file in the
> project), the linker will take what it needs in all of that.

at the risk of stating the obvious, and talking about something you
assumed would be reverted before all this ...

This largly stoped working with the new iteration APIs which has arrays of
every component in each lib which is referened by mandatory functions. 
thus referencing everything and pulling every symbol in.

It still works kind of i guess if you never use any codec, any filter and
any muxer or demuxer and just some component from libavutil. But thats a
far way from well working


> 
> Therefore, these project who insist on small libraries could achieve
> their goal with the simplest possible solution: they should be using
> static linking. It would work automatically, because linkers have been
> designed that way.
> 

> It is possible that for some reason, they need a shared library. In that
> case, the best solution is to make their own: put all the functions that
> use lav* together somewhere (most projects will abstract the lav* APIs
> to suit their own design choices anyway), and link that statically with
> FFmpeg, making a shared library perfectly tailored to the needs of the
> project.
> 
> And if you think about it, you will realize that what I propose has the
> consequence of making that simpler for them.
> 
> > Steam, Foobar2000, Firefox, Chromium, only four examples of projects
> > where the av*.dll files they ship have long configure lines where they
> > manually disable all kinds of things beyond the standard
> > "--disable-everything --enable-decoder=foo" use case, including entire
> > libraries and frameworks, because they only need a handful of software
> > decoders.
> 
> This is true, but misleading. First of all, I would like to emphasize
> that the framework code is very small compared to the code of the
> components.
> 
> But most importantly, the granularity of the modules is not the correct
> one. Consider an application that needs to decode from a few files. It
> requires the lavc decoder framework and a few decoders, and the lavf
> demuxing framework and a few demuxers. All encoders and all muxers are
> disabled. Yet, it will include the encoding framework and the muxing
> framework.
> 
> Now, you may say that it calls for more modularity, like you advocate
> below, and you would be right. But this is missing something, see below.
> 
> > And Chromium as I said even goes the extra mile by manually striping
> > libavutil of virtually everything except the core modules, something our
> > build system currently doesn't support.
> 
> And with my full proposal, our build system should actually support it.
> 
> > Building a single monolithic library will force the presence of a lot of
> > public symbols that currently can be avoided by simply building ffmpeg
> > for one decoder and effectively require just avcodec and avutil.
> > The direction we should head towards is making the libraries even more
> > modular and independent, starting with libavutil.
> 
> You are entirely right here, but it is actually not really related to
> the number of libraries.
> 
> Modularity has a cost: tracking the dependencies between components. If
> you implement modularity with many libraries, you push that complexity
> into the build system. This is a waste, since the linker already has all
> the algorithms to track dependencies.
> 

> As I have explained above, static linking already achieves maximum
> modularity automatically, and irregardless of whether there is one
> library or many.

It still needs some care to not introduce unneeded references.
no matter if one lib or 10 libs, if symbols are referenced they and
everything they reference are pulled in.
you just need a function that has a conditional call to some 
help printout which iterates over all components (like 

Re: [FFmpeg-devel] [RFC] avpriv cleanup

[Michael Niedermayer]

On Sun, Mar 25, 2018 at 05:55:37PM +0200, wm4 wrote:
> On Sun, 25 Mar 2018 17:34:51 +0200
> Michael Niedermayer  wrote:
> 
> > On Sun, Mar 25, 2018 at 05:07:33PM +0200, wm4 wrote:
> > > On Sun, 25 Mar 2018 17:00:32 +0200
> > > Michael Niedermayer  wrote:
> > >   
> > > > Hi all
> > > > 
> > > > Nicolas wrote an interresting comment about avpriv, and as this better
> > > > belongs into a new thread, here it is in a new thread
> > > > 
> > > > On Sun, Mar 25, 2018 at 03:32:33PM +0200, Nicolas George wrote:  
> > > > > Josh de Kock (2018-03-23):
> > > > [...]
> > > >
> > > > > You can observe just that by the fact that you needed to add an avpriv
> > > > > function to let lavd communicate with lavf. Each time an avpriv 
> > > > > function
> > > > > appears, it shows there is something flawed in the design.
> > > > 
> > > > If this is true, in general, then we can improve designs by removing
> > > > avpriv_* functions ...
> > > > 
> > > > looking at what we have as avprivs, in the tree, i think some of these
> > > > could be indeed usefull as public APIs. And we need to already keep them
> > > > compatible as they are exported as avpriv too, so making such changes 
> > > > does
> > > > indeed in some cases look like a good idea to me
> > > > 
> > > > There are some avprivs we have currently though that are quite specific
> > > > to format intgernals, i dont think that its always a flaw to use avpriv
> > > > functions thus
> > > > 
> > > > but i think we should evaluate each of the currently existing avpriv
> > > > functions and check if the API wouldnt be usefull to user applications.
> > > > And if it wouldnt be better to make it properly public  
> > >   
> > 
> > > This might apply to some cases, but in general: just no.  
> > 
> > That is what i meant :)
> > 
> > 
> > > 
> > > We make things avpriv_ *because* we don't want them to be public API,
> > > but the odd multiple-library design forces us to. Not more, not less.
> > >   
> > 
> > > What is this thread even for? If you want to make certain functions
> > > public, post specific RFCs or patches. In general, we probably all
> > > agree that avpriv functions are necessary hacks to deal with the split
> > > library nonsense.  
> > 
> > The thread is for finding out peoples oppinon before anyone spends time
> > writing patches.
> > For example it takes time to go over the functions and time to write patches
> > that turn functions with proper documention into public functions.
> > 
> > If people object to it, no point in anyone doing any of this work
> > 
> > Also there have been people looking for things to work on. If we agree
> > that looking over avpriv functions for more generally usefull ones 
> > makes sense then we need to discuss this in some thread for anyone
> > to be able to see that suggestion.
> 
> Well, just generally suggesting to remove avpriv functions is not going
> to work, because they exist for a reason (see e.g. Nicolas George or my
> post). Every specific case likely needs a separate discussion, so it
> doesn't make sense to have some sort of summary discussion.

well, the summary discussion certainly is very effective to get a feeling
for peoples oppinion on this in general.


> 
> > For a specific function
> > As a random example, lets take avpriv_set_systematic_pal2()
> > It provides a palette for 8bit per pixel formats which have a constant
> > palette. It sounds usefull to have this public to me ...
> 
> For that specific example I'd rather argue we should get rid of
> AV_PIX_FMT_FLAG_PSEUDOPAL. It's an awful hack to allow code to treat 1
> byte formats as paletted, such as AV_PIXFMT_GRAY8. Basically AVFrames
> with this pixfmt must come with a palette that acts as lookup table,
> that treats the 1 byte value as index into the table to convert it to
> RGB. The avpriv_set_systematic_pal2() just sets the palette for those
> formats.
> 
> This is broken and bad for the following reasons:
> 

> 1. The affected pixfmts are NOT paletted. So they shouldn't have a
>palette. It's against the principle of least astonishment (basically
>surprising the user with some broken obscure bullshit, instead of
>things just working). The only reason why I know about this pseudi
>PAL feature is because once I got a segfault by passing a self
>allocated GRAY8 frame to libswscale.

avpriv_set_systematic_pal2() sets the palette you where missing, that
caused the issue.
how is that an argument to not make it public !?


> 2. I suspect it's a hack only used by libswscale to speed up some
>trivial conversions. But libswscale can just create and keep such
>lookup table internally. Not a problem. This mechanism is not needed
>in the public API.


> 3. Thus why clutter public API and burden the API user with this?

you already provided the arguments for this above.
A. You tried to allocate a AVFrame manually, you need to setup things
   correctly. And iam not 

[FFmpeg-devel] [PATCH] avformat/concatdec: only set output stream index before returning packet

[Marton Balint]

Fixes ticket #6434.

Signed-off-by: Marton Balint 
---
 libavformat/concatdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
index 8fff9cc2cb..bbe13136fa 100644
--- a/libavformat/concatdec.c
+++ b/libavformat/concatdec.c
@@ -603,7 +603,6 @@ static int concat_read_packet(AVFormatContext *avf, 
AVPacket *pkt)
 av_packet_unref(pkt);
 continue;
 }
-pkt->stream_index = cs->out_stream_index;
 break;
 }
 if ((ret = filter_packet(avf, cs, pkt)))
@@ -646,6 +645,7 @@ static int concat_read_packet(AVFormatContext *avf, 
AVPacket *pkt)
 }
 }
 
+pkt->stream_index = cs->out_stream_index;
 return ret;
 }
 
-- 
2.13.6

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

Re: [FFmpeg-devel] [RFC] avpriv cleanup

[Nicolas George]

First of all, thanks for taking the time to discuss this. It has been
too long, and the arguments need to be sharpened.


James Almer (2018-03-25):
> Considering most are pretty small but not small enough to be dumped into
> a header, no, I'm not joking.
> Maybe avpriv_dnxhd_get_frame_size() should stay as is since it requires
> a huge table and duplicating it would be a waste of space, but
> avpriv_dca_convert_bitstream() seems simple enough that i don't see the
> benefits from having it like this, when it could be in both avcodec and
> avformat and weight maybe 1kb on each of them.

The weight in code size is far from being the most important criterion.
The case against code duplication is maintenance: when a bug is found,
it requires fixing in all copies of the code. You can be sure it will
not happen: only the copy where the bug was diagnosed will be fixed. The
same goes for optimizations, evolutions, etc. This in turn causes
another nightmare, when results are expected to be identical but are not
because they come from two copies of the same code that evolved
differently.

> If they used distro packages then they wouldn't have any power over what
> gets built or shipped. Distro packages are the "Enable everything" kind
> of build, so I'm of course talking about projects shipping their own
> builds of the libraries.

I have nothing to add to that. But you only answered half the question.
The other half was: do they use static or dynamic linking?

I am not sure how familiar you are with the precise workings of both
solutions, so let me summarize a bit.

With static linking, libraries are just archives or object files (*.o),
with an index of symbols. The linker will examine each archive in turn
and select the object files that contains required symbols.

With dynamic linking, the linker takes notes of all the libraries,
checks that all symbols are resolved and registers as dependencies of
the executable file.

What is remarkable about static linking is that it only takes the object
files that are necessary, nothing more. It does not matter whether there
are one library, eight or two thousands (one for each object file in the
project), the linker will take what it needs in all of that.

Therefore, these project who insist on small libraries could achieve
their goal with the simplest possible solution: they should be using
static linking. It would work automatically, because linkers have been
designed that way.

It is possible that for some reason, they need a shared library. In that
case, the best solution is to make their own: put all the functions that
use lav* together somewhere (most projects will abstract the lav* APIs
to suit their own design choices anyway), and link that statically with
FFmpeg, making a shared library perfectly tailored to the needs of the
project.

And if you think about it, you will realize that what I propose has the
consequence of making that simpler for them.

> Steam, Foobar2000, Firefox, Chromium, only four examples of projects
> where the av*.dll files they ship have long configure lines where they
> manually disable all kinds of things beyond the standard
> "--disable-everything --enable-decoder=foo" use case, including entire
> libraries and frameworks, because they only need a handful of software
> decoders.

This is true, but misleading. First of all, I would like to emphasize
that the framework code is very small compared to the code of the
components.

But most importantly, the granularity of the modules is not the correct
one. Consider an application that needs to decode from a few files. It
requires the lavc decoder framework and a few decoders, and the lavf
demuxing framework and a few demuxers. All encoders and all muxers are
disabled. Yet, it will include the encoding framework and the muxing
framework.

Now, you may say that it calls for more modularity, like you advocate
below, and you would be right. But this is missing something, see below.

> And Chromium as I said even goes the extra mile by manually striping
> libavutil of virtually everything except the core modules, something our
> build system currently doesn't support.

And with my full proposal, our build system should actually support it.

> Building a single monolithic library will force the presence of a lot of
> public symbols that currently can be avoided by simply building ffmpeg
> for one decoder and effectively require just avcodec and avutil.
> The direction we should head towards is making the libraries even more
> modular and independent, starting with libavutil.

You are entirely right here, but it is actually not really related to
the number of libraries.

Modularity has a cost: tracking the dependencies between components. If
you implement modularity with many libraries, you push that complexity
into the build system. This is a waste, since the linker already has all
the algorithms to track dependencies.

As I have explained above, static linking already achieves maximum
modularity 

Re: [FFmpeg-devel] [PATCH] Support signaling of last segment number

[sanilraut]

Thanks.

@Jan: I will look into the FATE tests. I have emailed you a few questions
that I have about FATE. Looking forward to your reply.
@Michael: I have added the prefix and resubmitted the patch.

Regards,
Sanil


On Fri, Mar 23, 2018 at 5:24 PM, Michael Niedermayer  wrote:

> On Thu, Mar 22, 2018 at 10:14:36PM +0200, Jan Ekström wrote:
> > On Thu, Mar 22, 2018 at 9:51 PM, sanilraut  wrote:
> > > Last segment indicated by mpd is not parsed.
> > > Example stream: http://dash.akamaized.net/dash264/TestCasesIOP41/
> LastSegmentNumber/1/manifest_last_segment_num.mpd
> > >
> > > This patch supports parsing of Supplemental Descriptor with
> @schemeIdUri set to http://dashif.org/guide-
> > > lines/last-segment-number with the @value set to the last segment
> number.
> > >
> >
> > Hi,
> >
> > Just seeing the commit message,
>
> ... i realize its missing the avformat / libavformat/dashdec prefix :)
> very minor issue but it seems a common mistake
> maybe something could test for this automatically so new people would
> notice before git send mail
>
>
> --
> 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
>
> ___
> 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

[FFmpeg-devel] [PATCH v2] avformat/dashdec: Support signaling of last segment number

[sanilraut]

Last segment indicated by mpd is not parsed.
Example stream: 
http://dash.akamaized.net/dash264/TestCasesIOP41/LastSegmentNumber/1/manifest_last_segment_num.mpd

This patch supports parsing of Supplemental Descriptor with @schemeIdUri set to 
http://dashif.org/guide-
lines/last-segment-number with the @value set to the last segment number.

---
 libavformat/dashdec.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 7b79b93..db63a99 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -805,7 +805,8 @@ static int parse_manifest_representation(AVFormatContext 
*s, const char *url,
  xmlNodePtr fragment_template_node,
  xmlNodePtr content_component_node,
  xmlNodePtr adaptionset_baseurl_node,
- xmlNodePtr 
adaptionset_segmentlist_node)
+ xmlNodePtr 
adaptionset_segmentlist_node,
+ xmlNodePtr 
adaptionset_supplementalproperty_node)
 {
 int32_t ret = 0;
 int32_t audio_rep_idx = 0;
@@ -825,6 +826,7 @@ static int parse_manifest_representation(AVFormatContext 
*s, const char *url,
 char *timescale_val = NULL;
 char *initialization_val = NULL;
 char *media_val = NULL;
+char *val = NULL;
 xmlNodePtr baseurl_nodes[4];
 xmlNodePtr representation_node = node;
 char *rep_id_val = xmlGetProp(representation_node, "id");
@@ -920,6 +922,13 @@ static int parse_manifest_representation(AVFormatContext 
*s, const char *url,
 rep->first_seq_no = (int64_t) strtoll(startnumber_val, NULL, 
10);
 xmlFree(startnumber_val);
 }
+if (adaptionset_supplementalproperty_node) {
+if 
(!strcmp(xmlGetProp(adaptionset_supplementalproperty_node,"schemeIdUri"), 
"http://dashif.org/guidelines/last-segment-number;)) {
+val = 
xmlGetProp(adaptionset_supplementalproperty_node,"value");
+rep->last_seq_no =(int64_t) strtoll(val, NULL, 10) - 1;
+xmlFree(val);
+ }
+}
 
 fragment_timeline_node = 
find_child_node_by_name(representation_segmenttemplate_node, "SegmentTimeline");
 
@@ -1054,6 +1063,7 @@ static int parse_manifest_adaptationset(AVFormatContext 
*s, const char *url,
 xmlNodePtr content_component_node = NULL;
 xmlNodePtr adaptionset_baseurl_node = NULL;
 xmlNodePtr adaptionset_segmentlist_node = NULL;
+xmlNodePtr adaptionset_supplementalproperty_node = NULL;
 xmlNodePtr node = NULL;
 
 node = xmlFirstElementChild(adaptionset_node);
@@ -1066,6 +1076,8 @@ static int parse_manifest_adaptationset(AVFormatContext 
*s, const char *url,
 adaptionset_baseurl_node = node;
 } else if (!av_strcasecmp(node->name, (const char *)"SegmentList")) {
 adaptionset_segmentlist_node = node;
+} else if (!av_strcasecmp(node->name, (const char 
*)"SupplementalProperty")) {
+adaptionset_supplementalproperty_node = node;
 } else if (!av_strcasecmp(node->name, (const char *)"Representation")) 
{
 ret = parse_manifest_representation(s, url, node,
 adaptionset_node,
@@ -1076,7 +1088,8 @@ static int parse_manifest_adaptationset(AVFormatContext 
*s, const char *url,
 fragment_template_node,
 content_component_node,
 adaptionset_baseurl_node,
-adaptionset_segmentlist_node);
+adaptionset_segmentlist_node,
+
adaptionset_supplementalproperty_node);
 if (ret < 0) {
 return ret;
 }
@@ -1819,7 +1832,10 @@ static int open_demux_for_component(AVFormatContext *s, 
struct representation *p
 
 pls->parent = s;
 pls->cur_seq_no  = calc_cur_seg_no(s, pls);
-pls->last_seq_no = calc_max_seg_no(pls, s->priv_data);
+
+if (!pls->last_seq_no) {
+pls->last_seq_no = calc_max_seg_no(pls, s->priv_data);
+}
 
 ret = reopen_demux_for_component(s, pls);
 if (ret < 0) {
-- 

Thanks

2.7.4

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

Re: [FFmpeg-devel] [PATCH] lavc/cfhd: error due to improper allocation of height in buffers

[Paul B Mahol]

On 3/25/18, Gagandeep Singh  wrote:
> ticket #6675 the distortion in the bottom 8 pixels fixed
> ---
>  libavcodec/cfhd.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
> index e35732df45..f10742f4fa 100644
> --- a/libavcodec/cfhd.c
> +++ b/libavcodec/cfhd.c
> @@ -213,13 +213,14 @@ static int alloc_buffers(AVCodecContext *avctx)
>  int width  = i ? avctx->width  >> chroma_x_shift : avctx->width;
>  int height = i ? avctx->height >> chroma_y_shift : avctx->height;
>  ptrdiff_t stride = FFALIGN(width  / 8, 8) * 8;
> -height   = FFALIGN(height / 8, 2) * 8;
> +if (chroma_y_shift)
> +height = FFALIGN(height / 8, 2) * 8;
>  s->plane[i].width  = width;
>  s->plane[i].height = height;
>  s->plane[i].stride = stride;
>
>  w8 = FFALIGN(s->plane[i].width  / 8, 8);
> -h8 = FFALIGN(s->plane[i].height / 8, 2);
> +h8 = height / 8;
>  w4 = w8 * 2;
>  h4 = h8 * 2;
>  w2 = w4 * 2;
> --
> 2.14.1
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Breaks cfhd fate test, and can not be applied as is.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 1/2] doc/examples/hw_decode: Remove useless NULL check

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer 
---
 doc/examples/hw_decode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/doc/examples/hw_decode.c b/doc/examples/hw_decode.c
index 14fe08b374..74a0ca32db 100644
--- a/doc/examples/hw_decode.c
+++ b/doc/examples/hw_decode.c
@@ -138,8 +138,7 @@ static int decode_write(AVCodecContext *avctx, AVPacket 
*packet)
 fail:
 av_frame_free();
 av_frame_free(_frame);
-if (buffer)
-av_freep();
+av_freep();
 if (ret < 0)
 return ret;
 }
-- 
2.16.2

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

[FFmpeg-devel] [PATCH 2/2] doc/examples/hw_decode: Remove logically dead code in decode_write()

[Michael Niedermayer]

Fixes CID1415951

Signed-off-by: Michael Niedermayer 
---
 doc/examples/hw_decode.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/doc/examples/hw_decode.c b/doc/examples/hw_decode.c
index 74a0ca32db..77ae8df35b 100644
--- a/doc/examples/hw_decode.c
+++ b/doc/examples/hw_decode.c
@@ -86,7 +86,7 @@ static int decode_write(AVCodecContext *avctx, AVPacket 
*packet)
 return ret;
 }
 
-while (ret >= 0) {
+while (1) {
 if (!(frame = av_frame_alloc()) || !(sw_frame = av_frame_alloc())) {
 fprintf(stderr, "Can not alloc frame\n");
 ret = AVERROR(ENOMEM);
@@ -142,8 +142,6 @@ static int decode_write(AVCodecContext *avctx, AVPacket 
*packet)
 if (ret < 0)
 return ret;
 }
-
-return 0;
 }
 
 int main(int argc, char *argv[])
-- 
2.16.2

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

Re: [FFmpeg-devel] [RFC] avpriv cleanup

[James Almer]

On 3/25/2018 1:58 PM, Nicolas George wrote:
> James Almer (2018-03-25):
>> Most avpriv_ functions exist solely to avoid code duplication. If it's
> 
> Most functions exist solely to avoid code duplication. Functions,
> unqualified, all of them: static, ff_, etc. The avpriv_ prefix only
> exists because of library boundaries.
> 
>> so much of an issue we could effectively duplicate them all on each
>> library as required by the different modules.
> 
> I hope this was a joke.

Considering most are pretty small but not small enough to be dumped into
a header, no, I'm not joking.
Maybe avpriv_dnxhd_get_frame_size() should stay as is since it requires
a huge table and duplicating it would be a waste of space, but
avpriv_dca_convert_bitstream() seems simple enough that i don't see the
benefits from having it like this, when it could be in both avcodec and
avformat and weight maybe 1kb on each of them.

> 
>> No, because a *lot* of people only want a decoder or two, and don't want
>> to deal with a lot of non optional filter/device/format/resample/scale
>> framework they don't need bloating their applications.
> 
> FINALLY! Somebody giving the beginning of actual arguments.
> 
> Now we can discuss. Can you tell us more about these many people who
> want only a decoder or two? Do they use static linking or dynamic
> linking? Do they ship their own lav* or do they use distros?

If they used distro packages then they wouldn't have any power over what
gets built or shipped. Distro packages are the "Enable everything" kind
of build, so I'm of course talking about projects shipping their own
builds of the libraries.

Steam, Foobar2000, Firefox, Chromium, only four examples of projects
where the av*.dll files they ship have long configure lines where they
manually disable all kinds of things beyond the standard
"--disable-everything --enable-decoder=foo" use case, including entire
libraries and frameworks, because they only need a handful of software
decoders.
And Chromium as I said even goes the extra mile by manually striping
libavutil of virtually everything except the core modules, something our
build system currently doesn't support.

Ronald i remember some time ago tried to remove a bunch of unnecessary
objects from libavutil in a minimalist build because he didn't need 1kb
of binary code for a duplicate asm function that would never see any
use. He also wondered why his VP9-only build of libavcodec needed to
compile Vorbis related objects.

Building a single monolithic library will force the presence of a lot of
public symbols that currently can be avoided by simply building ffmpeg
for one decoder and effectively require just avcodec and avutil.
The direction we should head towards is making the libraries even more
modular and independent, starting with libavutil.

> 
> These are very important questions, because different cases warrant
> different responses, and only a few cases are actually relevant for this
> discussion.
> 
>> Even the most minimalist build today still builds a lot of non optional
>> stuff in libavcodec that was made public for the purpose of getting rid
>> of avpriv_ symbols, like all those vorbis and aac header parsing
>> functions that i doubt anyone or anything outside of libav* ever uses.
> 
> I think you are missing a very important fact here: all that
> non-optional stuff exists precisely because it cannot be made really
> internal due to the library boundaries. If the project built into a
> single library, then it would be much easier to ensure that
> --disable-everything --enable=a,few,things actually builds only the
> necessary.

Not the core framework and public API for each library. Those are
unconditional and would remain so in a monolithic library, but with the
added drawback that instead of only needing those from avcodec/avutil in
a single decoder build, you'd be forced to also compile and ship those
from avfilter/swscale/swresample/avdevice.

> 
> Regards,
> 
> 
> 
> ___
> 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 1/6] avcodec/avpacket: add av_packet_make_ref()

[James Almer]

On 3/25/2018 2:54 PM, Mark Thompson wrote:
> On 25/03/18 05:03, James Almer wrote:
>> It works as a drop in replacement for the deprecated av_dup_packet(),
>> to ensure a packet is reference counted.
>>
>> Signed-off-by: James Almer 
>> ---
>> Better name welcome.
>>
>>  libavcodec/avcodec.h  | 18 +-
>>  libavcodec/avpacket.c | 18 ++
>>  2 files changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index 495242faf0..eb3fe4e428 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -4360,7 +4360,7 @@ int av_packet_from_data(AVPacket *pkt, uint8_t *data, 
>> int size);
>>   * @warning This is a hack - the packet memory allocation stuff is broken. 
>> The
>>   * packet is allocated if it was not really allocated.
>>   *
>> - * @deprecated Use av_packet_ref
>> + * @deprecated Use av_packet_ref or av_packet_make_ref
>>   */
>>  attribute_deprecated
>>  int av_dup_packet(AVPacket *pkt);
>> @@ -4531,6 +4531,22 @@ void av_packet_move_ref(AVPacket *dst, AVPacket *src);
>>   */
>>  int av_packet_copy_props(AVPacket *dst, const AVPacket *src);
>>  
>> +/**
>> + * Ensure the data described by a given packet is reference counted.
>> + *
>> + * @note This function does no ensure that the reference will be writable.
> 
> "does not ensure"

Fixed.

> 
>> + *   Use av_packet_make_writable instead for that purpose.
>> + *
>> + * @see av_packet_ref
>> + * @see av_packet_make_writable
>> + *
>> + * @param pkt packet whose data should be made reference counted.
>> + *
>> + * @return 0 on success, a negative AVERROR on error. On failure, the
>> + * packet is unchanged.
>> + */
>> +int av_packet_make_ref(AVPacket *pkt);
> 
> "make_ref" seems pretty confusing as a name - it sounds like it will always 
> make a new reference somehow.
> 
> Given the first line of the description, "ensure_refcounted"?

av_packet_ensure_refcounted() seems a tad too long, IMO, especially for
a function doing something as simple as this one as replacement for one
called av_dup_packet().

But if others agree I'll go with it.

> 
>> +
>>  /**
>>   * Create a writable reference for the data described by a given packet,
>>   * avoiding data copy if possible.
>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
>> index 7faa082395..a6d2e6eb74 100644
>> --- a/libavcodec/avpacket.c
>> +++ b/libavcodec/avpacket.c
>> @@ -655,6 +655,24 @@ void av_packet_move_ref(AVPacket *dst, AVPacket *src)
>>  src->size = 0;
>>  }
>>  
>> +int av_packet_make_ref(AVPacket *pkt)
>> +{
>> +int ret;
>> +
>> +if (pkt->buf)
>> +return 0;
>> +
>> +ret = packet_alloc(>buf, pkt->size);
>> +if (ret < 0)
>> +return ret;
>> +if (pkt->size)
>> +memcpy(pkt->buf->data, pkt->data, pkt->size);
>> +
>> +pkt->data = pkt->buf->data;
>> +
>> +return 0;
>> +}
>> +
>>  int av_packet_make_writable(AVPacket *pkt)
>>  {
>>  AVBufferRef *buf = NULL;
>>
> 
> Looks like sensible thing to have.
> 
> - Mark
> ___
> 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] avfilter/vf_avgblur_opencl: fix error when clSetKernelArg fails

[Mark Thompson]

On 25/03/18 13:42, dylanf...@gmail.com wrote:
> From: drfer3 
> 
> Fixes Coverity CID 1430382
> ---> Following is a patch attempting to fix the err issue. It returns -1 if 
> any
> clSetKernelArg() fails. Is this good, or should I be using a different
> return value for this error?

-1 shouldn't be used (since it maps to errno values, it looks like EPERM on 
Linux, and different things on other systems).

Given that would be quite unexpected if it failed, I think AVERROR_UNKNOWN is 
appropriate here - see 
.

Patch looks fine with that changed.

Thanks,

- Mark


>  libavfilter/vf_avgblur_opencl.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libavfilter/vf_avgblur_opencl.c b/libavfilter/vf_avgblur_opencl.c
> index 5ee66c0ba2..09caa1fd4f 100644
> --- a/libavfilter/vf_avgblur_opencl.c
> +++ b/libavfilter/vf_avgblur_opencl.c
> @@ -155,18 +155,21 @@ static int avgblur_opencl_filter_frame(AVFilterLink 
> *inlink, AVFrame *input)
>  if (cle != CL_SUCCESS) {
>  av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
> "destination image argument: %d.\n", cle);
> +err = -1;
>  goto fail;
>  }
>  cle = clSetKernelArg(ctx->kernel_horiz, 1, sizeof(cl_mem), );
>  if (cle != CL_SUCCESS) {
>  av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
> "source image argument: %d.\n", cle);
> +err = -1;
>  goto fail;
>  }
>  cle = clSetKernelArg(ctx->kernel_horiz, 2, sizeof(cl_int), 
> _x);
>  if (cle != CL_SUCCESS) {
>  av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
> "sizeX argument: %d.\n", cle);
> +err = -1;
>  goto fail;
>  }
>  
> @@ -191,18 +194,21 @@ static int avgblur_opencl_filter_frame(AVFilterLink 
> *inlink, AVFrame *input)
>  if (cle != CL_SUCCESS) {
>  av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
> "destination image argument: %d.\n", cle);
> +err = -1;
>  goto fail;
>  }
>  cle = clSetKernelArg(ctx->kernel_vert, 1, sizeof(cl_mem), );
>  if (cle != CL_SUCCESS) {
>  av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
> "source image argument: %d.\n", cle);
> +err = -1;
>  goto fail;
>  }
>  cle = clSetKernelArg(ctx->kernel_vert, 2, sizeof(cl_int), _y);
>  if (cle != CL_SUCCESS) {
>  av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
> "sizeY argument: %d.\n", cle);
> +err = -1;
>  goto fail;
>  }
>  
> 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc: Add filter_units bitstream filter

[Mark Thompson]

On 25/03/18 13:40, Eran Kornblau wrote:
>> -Original Message-
>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of 
>> Mark Thompson
>> Sent: Sunday, March 11, 2018 8:38 PM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH] lavc: Add filter_units bitstream filter
>>
>> On 08/03/18 04:01, James Almer wrote:
>>> On 3/6/2018 3:49 PM, Mark Thompson wrote:
 This can remove units with types in or not in a given set from a stream.
 For example, it can be used to remove all non-VCL NAL units from an 
 H.264 or
 H.265 stream.
 ---
 On 06/03/18 17:27, Hendrik Leppkes wrote:
> On Tue, Mar 6, 2018 at 3:51 PM, Eran Kornblau  
> wrote:
>> Hi all,
>>
>> The attached patch adds a parameter that enables the user to choose 
>> which AVC/HEVC NAL units to include in the output.
>> The parameter is supplied as a bitmask in order to keep things simple.
>>
>> A short background on why we need it - in our transcoding process, 
>> we partition the video in chunks, the chunks are transcoded in 
>> parallel and packaged in MPEG-TS container. The transcoded TS 
>> chunks are then concatenated and packaged in MP4. These MP4 files are 
>> later repackaged on-the-fly to various protocols (HLS/DASH etc.) using 
>> our JIT packager.
>> For performance reasons (can get into more detail if anyone's 
>> interested...), when packaging the MP4 to DASH/CENC, we configure the 
>> packager to assume that each AVC frame contains exactly one NAL unit.
>> The problem is that the transition through MPEG-TS adds additional 
>> NAL units (NAL AUD before each frame + SPS/PPS before each key frame), 
>> and this assumption fails.
>> Using the attached patch we can pass '-nal_types_mask 0x3e' which will 
>> make ffmpeg output only VCL NALs in the stream.
>>
>
> Having such logic in one single muxer is not something we really 
> like around here. Next time someone needs something similar for 
> another codec, you're stuck re-implementing it.
>
> To achieve the same effect, Mark Thompson quickly wipped up a 
> Bitstream Filter using his CBS framework which achieves the same 
> result. He'll be sending that patch to the mailing list in a while.
 The suggested use-case would be '-bsf:v filter_units=pass_types=1-5' for 
 H.264 or '-bsf:v filter_units=pass_types=0-31' for H.265.

 (Also note that filters already exist for some individual parts of 
 this: h264_metadata can remove AUDs, extract_extradata can remove 
 parameter sets.)

 - Mark


  libavcodec/Makefile|   1 +
  libavcodec/bitstream_filters.c |   1 +
  libavcodec/filter_units_bsf.c  | 250 
 +
  3 files changed, 252 insertions(+)
  create mode 100644 libavcodec/filter_units_bsf.c

>>>
>>> Can you write some minimal documentation with the two above examples?
>>
>> Done.
>>
 diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 
 b496f0d..b99bdce 100644
 --- a/libavcodec/Makefile
 +++ b/libavcodec/Makefile
 @@ -1037,6 +1037,7 @@ OBJS-$(CONFIG_DUMP_EXTRADATA_BSF) += 
 dump_extradata_bsf.o
  OBJS-$(CONFIG_DCA_CORE_BSF)   += dca_core_bsf.o
  OBJS-$(CONFIG_EXTRACT_EXTRADATA_BSF)  += extract_extradata_bsf.o\
   h2645_parse.o
 +OBJS-$(CONFIG_FILTER_UNITS_BSF)   += filter_units_bsf.o
  OBJS-$(CONFIG_H264_METADATA_BSF)  += h264_metadata_bsf.o
  OBJS-$(CONFIG_H264_MP4TOANNEXB_BSF)   += h264_mp4toannexb_bsf.o
  OBJS-$(CONFIG_H264_REDUNDANT_PPS_BSF) += h264_redundant_pps_bsf.o
 diff --git a/libavcodec/bitstream_filters.c 
 b/libavcodec/bitstream_filters.c index 338ef82..e1dc198 100644
 --- a/libavcodec/bitstream_filters.c
 +++ b/libavcodec/bitstream_filters.c
 @@ -29,6 +29,7 @@ extern const AVBitStreamFilter ff_chomp_bsf;  
 extern const AVBitStreamFilter ff_dump_extradata_bsf;  extern const 
 AVBitStreamFilter ff_dca_core_bsf;  extern const AVBitStreamFilter 
 ff_extract_extradata_bsf;
 +extern const AVBitStreamFilter ff_filter_units_bsf;
  extern const AVBitStreamFilter ff_h264_metadata_bsf;  extern const 
 AVBitStreamFilter ff_h264_mp4toannexb_bsf;  extern const 
 AVBitStreamFilter ff_h264_redundant_pps_bsf; diff --git 
 a/libavcodec/filter_units_bsf.c b/libavcodec/filter_units_bsf.c new 
 file mode 100644 index 000..3126f17
 --- /dev/null
 +++ b/libavcodec/filter_units_bsf.c
 @@ -0,0 +1,250 @@
 +/*
 + * 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

Re: [FFmpeg-devel] [PATCH 1/6] avcodec/avpacket: add av_packet_make_ref()

[Mark Thompson]

On 25/03/18 05:03, James Almer wrote:
> It works as a drop in replacement for the deprecated av_dup_packet(),
> to ensure a packet is reference counted.
> 
> Signed-off-by: James Almer 
> ---
> Better name welcome.
> 
>  libavcodec/avcodec.h  | 18 +-
>  libavcodec/avpacket.c | 18 ++
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 495242faf0..eb3fe4e428 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -4360,7 +4360,7 @@ int av_packet_from_data(AVPacket *pkt, uint8_t *data, 
> int size);
>   * @warning This is a hack - the packet memory allocation stuff is broken. 
> The
>   * packet is allocated if it was not really allocated.
>   *
> - * @deprecated Use av_packet_ref
> + * @deprecated Use av_packet_ref or av_packet_make_ref
>   */
>  attribute_deprecated
>  int av_dup_packet(AVPacket *pkt);
> @@ -4531,6 +4531,22 @@ void av_packet_move_ref(AVPacket *dst, AVPacket *src);
>   */
>  int av_packet_copy_props(AVPacket *dst, const AVPacket *src);
>  
> +/**
> + * Ensure the data described by a given packet is reference counted.
> + *
> + * @note This function does no ensure that the reference will be writable.

"does not ensure"

> + *   Use av_packet_make_writable instead for that purpose.
> + *
> + * @see av_packet_ref
> + * @see av_packet_make_writable
> + *
> + * @param pkt packet whose data should be made reference counted.
> + *
> + * @return 0 on success, a negative AVERROR on error. On failure, the
> + * packet is unchanged.
> + */
> +int av_packet_make_ref(AVPacket *pkt);

"make_ref" seems pretty confusing as a name - it sounds like it will always 
make a new reference somehow.

Given the first line of the description, "ensure_refcounted"?

> +
>  /**
>   * Create a writable reference for the data described by a given packet,
>   * avoiding data copy if possible.
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 7faa082395..a6d2e6eb74 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -655,6 +655,24 @@ void av_packet_move_ref(AVPacket *dst, AVPacket *src)
>  src->size = 0;
>  }
>  
> +int av_packet_make_ref(AVPacket *pkt)
> +{
> +int ret;
> +
> +if (pkt->buf)
> +return 0;
> +
> +ret = packet_alloc(>buf, pkt->size);
> +if (ret < 0)
> +return ret;
> +if (pkt->size)
> +memcpy(pkt->buf->data, pkt->data, pkt->size);
> +
> +pkt->data = pkt->buf->data;
> +
> +return 0;
> +}
> +
>  int av_packet_make_writable(AVPacket *pkt)
>  {
>  AVBufferRef *buf = NULL;
> 

Looks like sensible thing to have.

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

[FFmpeg-devel] [PATCH 3/5] h264_metadata: Update documentation

[Mark Thompson]

Improve documentation for the delete_filler option, and add the
display_orientation and a53_cc options.
---
 doc/bitstream_filters.texi | 51 +-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
index 982e3edac8..41424cf42f 100644
--- a/doc/bitstream_filters.texi
+++ b/doc/bitstream_filters.texi
@@ -209,7 +209,56 @@ For example, 
@samp{086f3693-b7b3-4f2c-9653-21492feee5b8+hello} will
 insert the string ``hello'' associated with the given UUID.
 
 @item delete_filler
-Deletes both filler NAL units and filler SEI messages.
+Delete all filler in the stream: both filler data NAL units and
+filler payload SEI messages.
+
+Filler zero bytes between NAL units in Annex B format (leading_zero_8bits
+and trailing_zero_8bits) will be discarded from the stream with or
+without this option - as such, HRD parameters are not expected to be
+accurate after any rewriting transformation made by this filter.
+
+@item display_orientation
+Modify display orientation SEI messages.
+
+@table @samp
+@item insert
+Insert new display orientation messages, overwriting any currently in
+the stream.  The new value is taken from the packet side data, modified
+by the @option{rotate} and @option{flip} options.
+
+@item remove
+Remove all display orientation messages from the stream.
+
+@item extract
+Extract display orientation messages so that they are available as
+packet side data.
+@end table
+
+@item rotate
+Set the rotation angle in the display orientation data.  This is an
+anticlockwise rotation angle in degrees.
+@item flip
+Set the flip fields in the display orientation data.  This can be any
+combination of the flags:
+@table @samp
+@item horizontal
+@item vertical
+@end table
+
+@item a53_cc
+Modify A/53 closed caption data in SEI messages.
+
+@table @samp
+@item insert
+Insert closed captions taken from packet side data into the stream.
+
+@item remove
+Remove all closed caption data from the stream.
+
+@item extract
+Extract closed captions from the stream so that they are available as
+as packet side data.
+@end table
 
 @end table
 
-- 
2.16.1

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

[FFmpeg-devel] [PATCH 5/5] lavc/Makefile: Fix ordering of bitstream filter components

[Mark Thompson]

---
 libavcodec/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index e5430ab10b..02f778231c 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1039,8 +1039,8 @@ OBJS-$(CONFIG_XMA_PARSER)  += xma_parser.o
 # bitstream filters
 OBJS-$(CONFIG_AAC_ADTSTOASC_BSF)  += aac_adtstoasc_bsf.o mpeg4audio.o
 OBJS-$(CONFIG_CHOMP_BSF)  += chomp_bsf.o
-OBJS-$(CONFIG_DUMP_EXTRADATA_BSF) += dump_extradata_bsf.o
 OBJS-$(CONFIG_DCA_CORE_BSF)   += dca_core_bsf.o
+OBJS-$(CONFIG_DUMP_EXTRADATA_BSF) += dump_extradata_bsf.o
 OBJS-$(CONFIG_EXTRACT_EXTRADATA_BSF)  += extract_extradata_bsf.o\
  h2645_parse.o
 OBJS-$(CONFIG_FILTER_UNITS_BSF)   += filter_units_bsf.o
@@ -1053,11 +1053,11 @@ OBJS-$(CONFIG_HEVC_MP4TOANNEXB_BSF)   += 
hevc_mp4toannexb_bsf.o
 OBJS-$(CONFIG_IMX_DUMP_HEADER_BSF)+= imx_dump_header_bsf.o
 OBJS-$(CONFIG_MJPEG2JPEG_BSF) += mjpeg2jpeg_bsf.o
 OBJS-$(CONFIG_MJPEGA_DUMP_HEADER_BSF) += mjpega_dump_header_bsf.o
-OBJS-$(CONFIG_MPEG4_UNPACK_BFRAMES_BSF)   += mpeg4_unpack_bframes_bsf.o
 OBJS-$(CONFIG_MOV2TEXTSUB_BSF)+= movsub_bsf.o
 OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  += mp3_header_decompress_bsf.o \
  mpegaudiodata.o
 OBJS-$(CONFIG_MPEG2_METADATA_BSF) += mpeg2_metadata_bsf.o cbs_misc.o
+OBJS-$(CONFIG_MPEG4_UNPACK_BFRAMES_BSF)   += mpeg4_unpack_bframes_bsf.o
 OBJS-$(CONFIG_NOISE_BSF)  += noise_bsf.o
 OBJS-$(CONFIG_NULL_BSF)   += null_bsf.o
 OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)   += remove_extradata_bsf.o
-- 
2.16.1

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

[FFmpeg-devel] [PATCH 4/5] mpeg2_metadata: Add support for A/53 closed captions

[Mark Thompson]

Allows extraction (to side data) and removal of closed captions in
user data blocks.
---
 doc/bitstream_filters.texi  | 12 ++
 libavcodec/Makefile |  2 +-
 libavcodec/mpeg2_metadata_bsf.c | 81 -
 3 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
index 41424cf42f..f115f7b0c5 100644
--- a/doc/bitstream_filters.texi
+++ b/doc/bitstream_filters.texi
@@ -465,6 +465,18 @@ table 6-6).
 Set the colour description in the stream (see H.262 section 6.3.6
 and tables 6-7, 6-8 and 6-9).
 
+@item a53_cc
+Modify A/53 closed captions in user data blocks.
+
+@table @samp
+@item remove
+Remove all closed caption data from the stream.
+
+@item extract
+Extract closed captions from the stream so that they are available as
+as packet side data.
+@end table
+
 @end table
 
 @section mpeg4_unpack_bframes
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index cfde104055..e5430ab10b 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1057,7 +1057,7 @@ OBJS-$(CONFIG_MPEG4_UNPACK_BFRAMES_BSF)   += 
mpeg4_unpack_bframes_bsf.o
 OBJS-$(CONFIG_MOV2TEXTSUB_BSF)+= movsub_bsf.o
 OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  += mp3_header_decompress_bsf.o \
  mpegaudiodata.o
-OBJS-$(CONFIG_MPEG2_METADATA_BSF) += mpeg2_metadata_bsf.o
+OBJS-$(CONFIG_MPEG2_METADATA_BSF) += mpeg2_metadata_bsf.o cbs_misc.o
 OBJS-$(CONFIG_NOISE_BSF)  += noise_bsf.o
 OBJS-$(CONFIG_NULL_BSF)   += null_bsf.o
 OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)   += remove_extradata_bsf.o
diff --git a/libavcodec/mpeg2_metadata_bsf.c b/libavcodec/mpeg2_metadata_bsf.c
index e787cb3782..49335d7fcb 100644
--- a/libavcodec/mpeg2_metadata_bsf.c
+++ b/libavcodec/mpeg2_metadata_bsf.c
@@ -22,9 +22,17 @@
 
 #include "bsf.h"
 #include "cbs.h"
+#include "cbs_misc.h"
 #include "cbs_mpeg2.h"
 #include "mpeg12.h"
 
+enum {
+PASS,
+INSERT,
+REMOVE,
+EXTRACT,
+};
+
 typedef struct MPEG2MetadataContext {
 const AVClass *class;
 
@@ -42,6 +50,8 @@ typedef struct MPEG2MetadataContext {
 int transfer_characteristics;
 int matrix_coefficients;
 
+int a53_cc;
+
 int mpeg1_warned;
 } MPEG2MetadataContext;
 
@@ -184,7 +194,9 @@ static int mpeg2_metadata_filter(AVBSFContext *bsf, 
AVPacket *out)
 MPEG2MetadataContext *ctx = bsf->priv_data;
 AVPacket *in = NULL;
 CodedBitstreamFragment *frag = >fragment;
-int err;
+int err, i;
+uint8_t *a53_side_data = NULL;
+size_t a53_side_data_size = 0;
 
 err = ff_bsf_get_packet(bsf, );
 if (err < 0)
@@ -202,6 +214,51 @@ static int mpeg2_metadata_filter(AVBSFContext *bsf, 
AVPacket *out)
 goto fail;
 }
 
+if (ctx->a53_cc == REMOVE || ctx->a53_cc == EXTRACT) {
+for (i = 0; i < frag->nb_units; i++) {
+MPEG2RawUserData *ud;
+A53UserData a53_ud;
+
+if (frag->units[i].type != MPEG2_START_USER_DATA)
+continue;
+ud = frag->units[i].content;
+
+err = ff_cbs_read_a53_user_data(ctx->cbc, _ud, ud->user_data,
+ud->user_data_length);
+if (err < 0) {
+// Invalid or something completely different.
+continue;
+}
+if (a53_ud.user_identifier != A53_USER_IDENTIFIER_ATSC ||
+a53_ud.atsc.user_data_type_code !=
+A53_USER_DATA_TYPE_CODE_CC_DATA) {
+// Valid but something else (e.g. AFD).
+continue;
+}
+
+if (ctx->a53_cc == REMOVE) {
+err = ff_cbs_delete_unit(ctx->cbc, frag, i);
+if (err < 0) {
+av_log(bsf, AV_LOG_ERROR, "Failed to delete "
+   "A/53 CC user data.\n");
+goto fail;
+}
+--i;
+break;
+} else if(ctx->a53_cc == EXTRACT) {
+err = ff_cbs_write_a53_cc_side_data(ctx->cbc,
+_side_data,
+_side_data_size,
+_ud);
+if (err < 0) {
+av_log(bsf, AV_LOG_ERROR, "Failed to write "
+   "A/53 user data for packet side data.\n");
+goto fail;
+}
+}
+}
+}
+
 err = ff_cbs_write_packet(ctx->cbc, out, frag);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n");
@@ -212,9 +269,21 @@ static int mpeg2_metadata_filter(AVBSFContext *bsf, 
AVPacket *out)
 if (err < 0)
 goto fail;
 
+if (a53_side_data) {
+err = av_packet_add_side_data(out, AV_PKT_DATA_A53_CC,
+  

[FFmpeg-devel] [PATCH 2/5] h264_metadata: Add support for A/53 closed captions

[Mark Thompson]

Allows insertion (from side data), extraction (to side data), and removal
of closed captions in SEI messages.
---
 libavcodec/Makefile|   2 +-
 libavcodec/h264_metadata_bsf.c | 138 +
 2 files changed, 139 insertions(+), 1 deletion(-)

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index aaef6c3ab8..cfde104055 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1044,7 +1044,7 @@ OBJS-$(CONFIG_DCA_CORE_BSF)   += 
dca_core_bsf.o
 OBJS-$(CONFIG_EXTRACT_EXTRADATA_BSF)  += extract_extradata_bsf.o\
  h2645_parse.o
 OBJS-$(CONFIG_FILTER_UNITS_BSF)   += filter_units_bsf.o
-OBJS-$(CONFIG_H264_METADATA_BSF)  += h264_metadata_bsf.o
+OBJS-$(CONFIG_H264_METADATA_BSF)  += h264_metadata_bsf.o cbs_misc.o
 OBJS-$(CONFIG_H264_MP4TOANNEXB_BSF)   += h264_mp4toannexb_bsf.o
 OBJS-$(CONFIG_H264_REDUNDANT_PPS_BSF) += h264_redundant_pps_bsf.o
 OBJS-$(CONFIG_HAPQA_EXTRACT_BSF)  += hapqa_extract_bsf.o hap.o
diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index 27053dbdcf..d79e6c0c87 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -24,6 +24,7 @@
 #include "bsf.h"
 #include "cbs.h"
 #include "cbs_h264.h"
+#include "cbs_misc.h"
 #include "h264.h"
 #include "h264_sei.h"
 
@@ -74,6 +75,8 @@ typedef struct H264MetadataContext {
 int display_orientation;
 double rotate;
 int flip;
+
+int a53_cc;
 } H264MetadataContext;
 
 
@@ -222,6 +225,8 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket 
*out)
 int err, i, j, has_sps;
 uint8_t *displaymatrix_side_data = NULL;
 size_t displaymatrix_side_data_size = 0;
+uint8_t *a53_side_data = NULL;
+size_t a53_side_data_size = 0;
 
 err = ff_bsf_get_packet(bsf, );
 if (err < 0)
@@ -516,6 +521,116 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
AVPacket *out)
 }
 }
 
+if (ctx->a53_cc == INSERT) {
+uint8_t *data;
+int size;
+
+data = av_packet_get_side_data(in, AV_PKT_DATA_A53_CC, );
+if (data) {
+A53UserData a53_ud;
+
+err = ff_cbs_read_a53_cc_side_data(ctx->cbc, _ud,
+   data, size);
+if (err < 0) {
+av_log(bsf, AV_LOG_WARNING, "Invalid A/53 closed captions "
+   "in packet side data dropped.\n");
+} else {
+H264RawSEIPayload payload = {
+.payload_type = H264_SEI_TYPE_USER_DATA_REGISTERED,
+};
+H264RawSEIUserDataRegistered *udr =
+_data_registered;
+size_t size = 9 + 3 * a53_ud.atsc.cc_data.cc_count;
+
+udr->data_ref = av_buffer_alloc(2 + size);
+if (!udr->data_ref) {
+err = AVERROR(ENOMEM);
+goto fail;
+}
+udr->data = udr->data_ref->data;
+
+udr->itu_t_t35_country_code = 181;
+AV_WB16(udr->data, 49); // provider_code
+
+err = ff_cbs_write_a53_user_data(ctx->cbc, udr->data + 2,
+ , _ud);
+if (err < 0) {
+av_log(bsf, AV_LOG_ERROR, "Failed to write "
+   "A/53 user data.\n");
+av_buffer_unref(>data_ref);
+goto fail;
+}
+udr->data_length = size + 2;
+
+err = ff_cbs_h264_add_sei_message(ctx->cbc, au, );
+if (err < 0) {
+av_log(bsf, AV_LOG_ERROR, "Failed to add A/53 user data "
+   "SEI message to access unit.\n");
+av_buffer_unref(>data_ref);
+goto fail;
+}
+}
+}
+
+} else if (ctx->a53_cc == REMOVE || ctx->a53_cc == EXTRACT) {
+for (i = 0; i < au->nb_units; i++) {
+H264RawSEI *sei;
+if (au->units[i].type != H264_NAL_SEI)
+continue;
+sei = au->units[i].content;
+
+for (j = 0; j < sei->payload_count; j++) {
+H264RawSEIUserDataRegistered *udr;
+A53UserData a53_ud;
+
+if (sei->payload[j].payload_type !=
+H264_SEI_TYPE_USER_DATA_REGISTERED)
+continue;
+udr = >payload[j].payload.user_data_registered;
+if (udr->data_length < 6) {
+// Can't be relevant.
+continue;
+}
+
+err = ff_cbs_read_a53_user_data(ctx->cbc, _ud,
+udr->data + 2,
+udr->data_length - 2);
+if (err < 0) {
+// 

[FFmpeg-devel] [PATCH 1/5] cbs: Add some common code for read/write of miscellaneous user data

[Mark Thompson]

Supports closed captions, active format and bar data as defined by
SCTE 128 part 1 or A/53 part 4, suitable for use with both MPEG-2
and H.264.
---
 libavcodec/cbs_misc.c | 216 ++
 libavcodec/cbs_misc.h | 109 +
 libavcodec/cbs_misc_syntax_template.c | 150 +++
 3 files changed, 475 insertions(+)
 create mode 100644 libavcodec/cbs_misc.c
 create mode 100644 libavcodec/cbs_misc.h
 create mode 100644 libavcodec/cbs_misc_syntax_template.c

diff --git a/libavcodec/cbs_misc.c b/libavcodec/cbs_misc.c
new file mode 100644
index 00..cdf01fe229
--- /dev/null
+++ b/libavcodec/cbs_misc.c
@@ -0,0 +1,216 @@
+/*
+ * 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 "libavutil/attributes.h"
+#include "libavutil/avassert.h"
+
+#include "cbs.h"
+#include "cbs_internal.h"
+#include "cbs_misc.h"
+
+#define CHECK(call) do { \
+err = (call); \
+if (err < 0) \
+return err; \
+} while (0)
+
+#define FUNC_NAME(rw, codec, name) cbs_ ## codec ## _ ## rw ## _ ## name
+#define FUNC_MISC(rw, name) FUNC_NAME(rw, misc, name)
+#define FUNC(name) FUNC_MISC(READWRITE, name)
+
+
+#define READWRITE read
+#define RWContext GetBitContext
+
+#define xui(width, name, var) do { \
+uint32_t value = 0; \
+CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, \
+   , 0, MAX_UINT_BITS(width))); \
+var = value; \
+} while (0)
+
+#define ui(width, name) \
+xui(width, name, current->name)
+
+#define fixed(width, name, expected) do { \
+av_unused uint32_t value; \
+CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, , \
+   expected, expected)); \
+} while (0)
+
+#include "cbs_misc_syntax_template.c"
+
+#undef READWRITE
+#undef RWContext
+#undef xui
+#undef ui
+#undef fixed
+
+
+#define READWRITE write
+#define RWContext PutBitContext
+
+#define xui(width, name, var) do { \
+CHECK(ff_cbs_write_unsigned(ctx, rw, width, #name, \
+var, 0, MAX_UINT_BITS(width))); \
+} while (0)
+
+#define ui(width, name) \
+xui(width, name, current->name)
+
+#define fixed(width, name, value) do { \
+CHECK(ff_cbs_write_unsigned(ctx, rw, width, #name, value, \
+value, value)); \
+} while (0)
+
+#include "cbs_misc_syntax_template.c"
+
+#undef READWRITE
+#undef RWContext
+#undef xui
+#undef ui
+#undef fixed
+
+
+int ff_cbs_read_a53_user_data(CodedBitstreamContext *ctx,
+  A53UserData *data,
+  const uint8_t *read_buffer, size_t length)
+{
+GetBitContext gbc;
+int err;
+
+err = init_get_bits(, read_buffer, 8 * length);
+if (err < 0)
+return err;
+
+return cbs_misc_read_a53_user_data(ctx, , data);
+}
+
+int ff_cbs_write_a53_user_data(CodedBitstreamContext *ctx,
+   uint8_t *write_buffer, size_t *length,
+   A53UserData *data)
+{
+PutBitContext pbc;
+int err;
+
+init_put_bits(, write_buffer, *length);
+
+err = cbs_misc_write_a53_user_data(ctx, , data);
+if (err < 0) {
+// Includes AVERROR(ENOSPC).
+return err;
+}
+
+// That output must be aligned.
+av_assert0(put_bits_count() % 8 == 0);
+
+*length = put_bits_count() / 8;
+
+flush_put_bits();
+
+return 0;
+}
+
+int ff_cbs_read_a53_cc_side_data(CodedBitstreamContext *ctx,
+ A53UserData *data,
+ const uint8_t *side_data,
+ size_t side_data_size)
+{
+GetBitContext gbc;
+CEA708CCData *cc;
+int err, i, cc_count;
+
+if (side_data_size % 3) {
+av_log(ctx->log_ctx, AV_LOG_ERROR, "A53 CC side data length must "
+   "be a multiple of 3 (got %zu).\n", side_data_size);
+return AVERROR(EINVAL);
+}
+cc_count = side_data_size / 3;
+if (cc_count > 31) {
+av_log(ctx->log_ctx, AV_LOG_ERROR, "A53 CC can only fit 31 packets "
+   "in a single user data block (got %d).\n", cc_count);
+return AVERROR(EINVAL);
+  

[FFmpeg-devel] [PATCHv2] avfilter/af_pan: reject expressions referencing the same channel multiple times

[Marton Balint]

Fixes parsing of expressions like c0=c0+c0 or c0=c0|c0=c1.  Previously no
error was thrown and for input channels, only the last gain factor was used,
for output channels the source channel gains were combined.

Signed-off-by: Marton Balint 
---
 libavfilter/af_pan.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/libavfilter/af_pan.c b/libavfilter/af_pan.c
index d8a63a7952..34e522c9d4 100644
--- a/libavfilter/af_pan.c
+++ b/libavfilter/af_pan.c
@@ -104,6 +104,7 @@ static av_cold int init(AVFilterContext *ctx)
 char *arg, *arg0, *tokenizer, *args = av_strdup(pan->args);
 int out_ch_id, in_ch_id, len, named, ret, sign = 1;
 int nb_in_channels[2] = { 0, 0 }; // number of unnamed and named input 
channels
+int used_out_ch[MAX_CHANNELS] = {0};
 double gain;
 
 if (!pan->args) {
@@ -127,6 +128,7 @@ static av_cold int init(AVFilterContext *ctx)
 
 /* parse channel specifications */
 while ((arg = arg0 = av_strtok(NULL, "|", ))) {
+int used_in_ch[MAX_CHANNELS] = {0};
 /* channel name */
 if (parse_channel_name(, _ch_id, )) {
 av_log(ctx, AV_LOG_ERROR,
@@ -153,6 +155,13 @@ static av_cold int init(AVFilterContext *ctx)
 ret = AVERROR(EINVAL);
 goto fail;
 }
+if (used_out_ch[out_ch_id]) {
+av_log(ctx, AV_LOG_ERROR,
+   "Can not reference out channel %d twice\n", out_ch_id);
+ret = AVERROR(EINVAL);
+goto fail;
+}
+used_out_ch[out_ch_id] = 1;
 skip_spaces();
 if (*arg == '=') {
 arg++;
@@ -184,6 +193,13 @@ static av_cold int init(AVFilterContext *ctx)
 ret = AVERROR(EINVAL);
 goto fail;
 }
+if (used_in_ch[in_ch_id]) {
+av_log(ctx, AV_LOG_ERROR,
+   "Can not reference in channel %d twice\n", in_ch_id);
+ret = AVERROR(EINVAL);
+goto fail;
+}
+used_in_ch[in_ch_id] = 1;
 pan->gain[out_ch_id][in_ch_id] = sign * gain;
 skip_spaces();
 if (!*arg)
-- 
2.13.6

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

Re: [FFmpeg-devel] [PATCH] avcodec: add a subcharenc mode that disables UTF-8 check

[wm4]

On Sun, 25 Mar 2018 09:00:45 -0700
Philip Langdale  wrote:

> On Sat, 24 Mar 2018 13:43:21 +0100
> wm4  wrote:
> 
> > This is for applications which want to explicitly check for invalid
> > UTF-8 manually, and take actions that are better than dropping invalid
> > subtitles silently. (It's pretty much silent because sporadic avcodec
> > error messages are so common that you can't reasonably display them
> > in a prominent and meaningful way in a application GUI.)
> > ---
> >  doc/APIchanges | 3 +++
> >  libavcodec/avcodec.h   | 1 +
> >  libavcodec/decode.c| 3 ++-
> >  libavcodec/options_table.h | 1 +
> >  libavcodec/version.h   | 2 +-
> >  5 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index a099afd9bc..95b5cd772f 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -15,6 +15,9 @@ libavutil: 2017-10-21
> >  
> >  API changes, most recent first:
> >  
> > +2018-03-xx - xxx - lavc 58.16.100 - avcodec.h
> > +  Add FF_SUB_CHARENC_MODE_IGNORE.
> > +
> >  2018-xx-xx - xxx - lavu 56.8.100 - encryption_info.h
> >Add AVEncryptionInitInfo and AVEncryptionInfo structures to hold
> > new side-data for encryption info.
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index 495242faf0..50c34dbff9 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -3092,6 +3092,7 @@ typedef struct AVCodecContext {
> >  #define FF_SUB_CHARENC_MODE_DO_NOTHING  -1  ///< do nothing (demuxer
> > outputs a stream supposed to be already in UTF-8, or the codec is
> > bitmap for instance) #define FF_SUB_CHARENC_MODE_AUTOMATIC0  ///<
> > libavcodec will select the mode itself #define
> > FF_SUB_CHARENC_MODE_PRE_DECODER  1  ///< the AVPacket data needs to
> > be recoded to UTF-8 before being fed to the decoder, requires iconv
> > +#define FF_SUB_CHARENC_MODE_IGNORE   2  ///< neither convert the
> > subtitles, nor check them for valid UTF-8 /**
> >   * Skip processing alpha if supported by codec.
> > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > index ea2168ad0c..40c8a8855c 100644
> > --- a/libavcodec/decode.c
> > +++ b/libavcodec/decode.c
> > @@ -1057,7 +1057,8 @@ int avcodec_decode_subtitle2(AVCodecContext
> > *avctx, AVSubtitle *sub, sub->format = 1;
> >  
> >  for (i = 0; i < sub->num_rects; i++) {
> > -if (sub->rects[i]->ass
> > && !utf8_check(sub->rects[i]->ass)) {
> > +if (avctx->sub_charenc_mode !=
> > FF_SUB_CHARENC_MODE_IGNORE &&
> > +sub->rects[i]->ass
> > && !utf8_check(sub->rects[i]->ass)) { av_log(avctx, AV_LOG_ERROR,
> > "Invalid UTF-8 in decoded subtitles text;
> > " "maybe missing -sub_charenc option\n");
> > diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> > index 5a5eae65fb..099261e168 100644
> > --- a/libavcodec/options_table.h
> > +++ b/libavcodec/options_table.h
> > @@ -447,6 +447,7 @@ static const AVOption avcodec_options[] = {
> >  {"do_nothing",  NULL, 0, AV_OPT_TYPE_CONST, {.i64 =
> > FF_SUB_CHARENC_MODE_DO_NOTHING},  INT_MIN, INT_MAX, S|D,
> > "sub_charenc_mode"}, {"auto",NULL, 0, AV_OPT_TYPE_CONST,
> > {.i64 = FF_SUB_CHARENC_MODE_AUTOMATIC},   INT_MIN, INT_MAX, S|D,
> > "sub_charenc_mode"}, {"pre_decoder", NULL, 0, AV_OPT_TYPE_CONST,
> > {.i64 = FF_SUB_CHARENC_MODE_PRE_DECODER}, INT_MIN, INT_MAX, S|D,
> > "sub_charenc_mode"}, +{"ignore",  NULL, 0, AV_OPT_TYPE_CONST,
> > {.i64 = FF_SUB_CHARENC_MODE_IGNORE},  INT_MIN, INT_MAX, S|D,
> > "sub_charenc_mode"}, #if FF_API_ASS_TIMING {"sub_text_format", "set
> > decoded text subtitle format", OFFSET(sub_text_format),
> > AV_OPT_TYPE_INT, {.i64 = FF_SUB_TEXT_FMT_ASS_WITH_TIMINGS}, 0, 1,
> > S|D, "sub_text_format"}, #else diff --git a/libavcodec/version.h
> > b/libavcodec/version.h index a5b7f752d1..8ac4626da7 100644 ---
> > a/libavcodec/version.h +++ b/libavcodec/version.h @@ -28,7 +28,7 @@
> > #include "libavutil/version.h" #define LIBAVCODEC_VERSION_MAJOR  58
> > -#define LIBAVCODEC_VERSION_MINOR  15
> > +#define LIBAVCODEC_VERSION_MINOR  16
> >  #define LIBAVCODEC_VERSION_MICRO 100
> >  
> >  #define LIBAVCODEC_VERSION_INT
> > AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \  
> 
> LGTM.

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

Re: [FFmpeg-devel] [PATCH] movtextdec: fix handling of UTF-8 subtitles

[wm4]

On Sat, 24 Mar 2018 15:48:36 +0100
wm4  wrote:

> Subtitles which contained styled UTF-8 subtitles (i.e. not just 7 bit
> ASCII characters) were not handled correctly. The spec mandates that
> styling start/end ranges are in "characters". It's not quite clear what
> a "character" is supposed to be, but maybe they mean unicode codepoints.
> 
> FFmpeg's decoder treated the style ranges as byte idexes, which could
> lead to UTF-8 sequences being broken, and the common code dropping the
> whole subtitle line.
> 
> Change this and count the codepoint instead. This also means that even
> if this is somehow wrong, the decoder won't break UTF-8 sequences
> anymore. The sample which led me to investigate this now appears to work
> correctly.
> ---
> https://github.com/mpv-player/mpv/issues/5675
> ---
>  libavcodec/movtextdec.c | 50 
> -
>  1 file changed, 37 insertions(+), 13 deletions(-)
> 
> diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c
> index bd19577724..89ac791602 100644
> --- a/libavcodec/movtextdec.c
> +++ b/libavcodec/movtextdec.c
> @@ -326,9 +326,24 @@ static const Box box_types[] = {
>  
>  const static size_t box_count = FF_ARRAY_ELEMS(box_types);
>  
> +// Return byte length of the UTF-8 sequence starting at text[0]. 0 on error.
> +static int get_utf8_length_at(const char *text, const char *text_end)
> +{
> +const char *start = text;
> +int err = 0;
> +uint32_t c;
> +GET_UTF8(c, text < text_end ? (uint8_t)*text++ : (err = 1, 0), goto 
> error;);
> +if (err)
> +goto error;
> +return text - start;
> +error:
> +return 0;
> +}
> +
>  static int text_to_ass(AVBPrint *buf, const char *text, const char *text_end,
> -MovTextContext *m)
> +   AVCodecContext *avctx)
>  {
> +MovTextContext *m = avctx->priv_data;
>  int i = 0;
>  int j = 0;
>  int text_pos = 0;
> @@ -342,6 +357,8 @@ static int text_to_ass(AVBPrint *buf, const char *text, 
> const char *text_end,
>  }
>  
>  while (text < text_end) {
> +int len;
> +
>  if (m->box_flags & STYL_BOX) {
>  for (i = 0; i < m->style_entries; i++) {
>  if (m->s[i]->style_flag && text_pos == m->s[i]->style_end) {
> @@ -388,17 +405,24 @@ static int text_to_ass(AVBPrint *buf, const char *text, 
> const char *text_end,
>  }
>  }
>  
> -switch (*text) {
> -case '\r':
> -break;
> -case '\n':
> -av_bprintf(buf, "\\N");
> -break;
> -default:
> -av_bprint_chars(buf, *text, 1);
> -break;
> +len = get_utf8_length_at(text, text_end);
> +if (len < 1) {
> +av_log(avctx, AV_LOG_ERROR, "invalid UTF-8 byte in subtitle\n");
> +len = 1;
> +}
> +for (i = 0; i < len; i++) {
> +switch (*text) {
> +case '\r':
> +break;
> +case '\n':
> +av_bprintf(buf, "\\N");
> +break;
> +default:
> +av_bprint_chars(buf, *text, 1);
> +break;
> +}
> +text++;
>  }
> -text++;
>  text_pos++;
>  }
>  
> @@ -507,10 +531,10 @@ static int mov_text_decode_frame(AVCodecContext *avctx,
>  }
>  m->tracksize = m->tracksize + tsmb_size;
>  }
> -text_to_ass(, ptr, end, m);
> +text_to_ass(, ptr, end, avctx);
>  mov_text_cleanup(m);
>  } else
> -text_to_ass(, ptr, end, m);
> +text_to_ass(, ptr, end, avctx);
>  
>  ret = ff_ass_add_rect(sub, buf.str, m->readorder++, 0, NULL, NULL);
>  av_bprint_finalize(, NULL);

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

Re: [FFmpeg-devel] [PATCHv2] avfilter/af_pan: reject expressions referencing the same channel multiple times

[Nicolas George]

Marton Balint (2018-03-25):
> Fixes parsing of expressions like c0=c0+c0 or c0=c0|c0=c1.  Previously no
> error was thrown and for input channels, only the last gain factor was used,
> for output channels the source channel gains were combined.
> 
> Signed-off-by: Marton Balint 
> ---
>  libavfilter/af_pan.c | 16 
>  1 file changed, 16 insertions(+)

LGTM, thanks.

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] [RFC] avpriv cleanup

[Nicolas George]

James Almer (2018-03-25):
> Most avpriv_ functions exist solely to avoid code duplication. If it's

Most functions exist solely to avoid code duplication. Functions,
unqualified, all of them: static, ff_, etc. The avpriv_ prefix only
exists because of library boundaries.

> so much of an issue we could effectively duplicate them all on each
> library as required by the different modules.

I hope this was a joke.

> No, because a *lot* of people only want a decoder or two, and don't want
> to deal with a lot of non optional filter/device/format/resample/scale
> framework they don't need bloating their applications.

FINALLY! Somebody giving the beginning of actual arguments.

Now we can discuss. Can you tell us more about these many people who
want only a decoder or two? Do they use static linking or dynamic
linking? Do they ship their own lav* or do they use distros?

These are very important questions, because different cases warrant
different responses, and only a few cases are actually relevant for this
discussion.

> Even the most minimalist build today still builds a lot of non optional
> stuff in libavcodec that was made public for the purpose of getting rid
> of avpriv_ symbols, like all those vorbis and aac header parsing
> functions that i doubt anyone or anything outside of libav* ever uses.

I think you are missing a very important fact here: all that
non-optional stuff exists precisely because it cannot be made really
internal due to the library boundaries. If the project built into a
single library, then it would be much easier to ensure that
--disable-everything --enable=a,few,things actually builds only the
necessary.

Regards,

-- 
  Nicolas George


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

[FFmpeg-devel] [PATCH] vf_libvmaf: Fix memory leak

[enctac]

Fixes ticket #6967
---
 libavfilter/vf_libvmaf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
index dfe474c40c..42c6b66b69 100644
--- a/libavfilter/vf_libvmaf.c
+++ b/libavfilter/vf_libvmaf.c
@@ -130,6 +130,8 @@ FRAMESYNC_DEFINE_CLASS(libvmaf, LIBVMAFContext, fs);
 \
 ret = !s->frame_set;   
 \
 \
+av_frame_unref(s->gref);   
 \
+av_frame_unref(s->gmain);  
 \
 s->frame_set = 0;  
 \
 \
 pthread_cond_signal(>cond); 
 \
-- 
2.16.2

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

[FFmpeg-devel] [PATCH 2/2] avfilter: add declip audio filter

[Paul B Mahol]

Signed-off-by: Paul B Mahol 
---
Depends on declick filter.
---
 doc/filters.texi | 28 +++
 libavfilter/Makefile |  1 +
 libavfilter/af_declick.c | 73 +---
 libavfilter/allfilters.c |  1 +
 4 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 9a067ba9ea..86cf570ab9 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -2597,6 +2597,34 @@ This controls between how much samples, which are 
detected as impulsive noise,
 any sample between 2 detected noise samples is considered also as noise sample.
 @end table
 
+@section declip
+Remove clipped samples from input audio.
+
+Samples detected as clipped are replaced by interpolated samples using
+autoregressive modeling.
+
+@table @option
+@item w
+Set window size, in milliseconds. Allowed range is from @code{10} to 
@code{100}.
+Default value is @code{50} milliseconds.
+This sets size of window which will be processed at once.
+
+@item o
+Set window overlap, in percentage of window size. Allowed range is from 
@code{50}
+to @code{95}. Default value is @code{75} percent.
+
+@item a
+Set autoregression order, in percentage of window size. Allowed range is from
+@code{1} to @code{50}. Default value is @code{2} percent. This option also 
controls
+quality of interpolated samples using neighbour good samples.
+
+@item t
+Set threshold value. Allowed range is from @code{0.2} to @code{1.0}.
+Default value is @code{0.98}.
+Any sample which absolute value is equal or higher of this value will be
+detected as clipped and be replaced with interpolated value.
+@end table
+
 @section drmeter
 Measure audio dynamic range.
 
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 978751d2a0..505287b5b4 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -88,6 +88,7 @@ OBJS-$(CONFIG_CROSSFEED_FILTER)  += af_crossfeed.o
 OBJS-$(CONFIG_CRYSTALIZER_FILTER)+= af_crystalizer.o
 OBJS-$(CONFIG_DCSHIFT_FILTER)+= af_dcshift.o
 OBJS-$(CONFIG_DECLICK_FILTER)+= af_declick.o
+OBJS-$(CONFIG_DECLIP_FILTER) += af_declick.o
 OBJS-$(CONFIG_DRMETER_FILTER)+= af_drmeter.o
 OBJS-$(CONFIG_DYNAUDNORM_FILTER) += af_dynaudnorm.o
 OBJS-$(CONFIG_EARWAX_FILTER) += af_earwax.o
diff --git a/libavfilter/af_declick.c b/libavfilter/af_declick.c
index 0de4c35c95..658dae420b 100644
--- a/libavfilter/af_declick.c
+++ b/libavfilter/af_declick.c
@@ -29,9 +29,11 @@ typedef struct DeclickContext {
 double w;
 double overlap;
 double threshold;
+double clip_threshold;
 double ar;
 double burst;
 
+int is_declip;
 int ar_order;
 int nb_burst_samples;
 int window_size;
@@ -68,6 +70,10 @@ typedef struct DeclickContext {
 
 AVAudioFifo *fifo;
 double *window_func_lut;
+
+int (*detector)(struct DeclickContext *s, double sigmae, double *detection,
+double *acoefficients, uint8_t *click, int *index,
+const double *src, double *dst);
 } DeclickContext;
 
 #define OFFSET(x) offsetof(DeclickContext, x)
@@ -354,6 +360,28 @@ static int interpolation(DeclickContext *s, const double 
*src, int ar_order,
 return cholesky_decomposition(s, matrix, vector, nb_clicks, interpolated);
 }
 
+static int detect_clips(DeclickContext *s, double unused0, double *unused1, 
double *unused2,
+uint8_t *clips, int *index,
+const double *src, double *dst)
+{
+const double threshold = s->clip_threshold;
+int i, nb_clips = 0;
+
+for (i = 0; i < s->window_size; i++) {
+clips[i] = fabs(src[i]) >= threshold;
+dst[i] = src[i];
+}
+
+memset(clips, 0, s->ar_order * sizeof(*clips));
+memset(clips + (s->window_size - s->ar_order), 0, s->ar_order * 
sizeof(*clips));
+
+for (i = s->ar_order; i < s->window_size - s->ar_order; i++)
+if (clips[i])
+index[nb_clips++] = i;
+
+return nb_clips;
+}
+
 static int detect_clicks(DeclickContext *s, double sigmae, double *detection, 
double *acoefficients,
  uint8_t *click, int *index,
  const double *src, double *dst)
@@ -441,8 +469,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 int *index = s->index;
 int nb_clicks;
 
-nb_clicks = detect_clicks(s, sigmae, s->detection, 
s->acoefficients,
-  s->click, index, src, dst);
+nb_clicks = s->detector(s, sigmae, s->detection, 
s->acoefficients,
+s->click, index, src, dst);
 if (nb_clicks > 0) {
 ret = interpolation(s, src, s->ar_order, s->acoefficients, 
index,
 nb_clicks, s->auxiliary, interpolated);
@@ -521,13 

Re: [FFmpeg-devel] [RFC] avpriv cleanup

[James Almer]

On 3/25/2018 12:56 PM, wm4 wrote:
> On Sun, 25 Mar 2018 12:47:41 -0300
> James Almer  wrote:
> 
>> On 3/25/2018 12:19 PM, Nicolas George wrote:
>>> Michael Niedermayer (2018-03-25):  
 looking at what we have as avprivs, in the tree, i think some of these
 could be indeed usefull as public APIs. And we need to already keep them
 compatible as they are exported as avpriv too, so making such changes does
 indeed in some cases look like a good idea to me

 There are some avprivs we have currently though that are quite specific
 to format intgernals, i dont think that its always a flaw to use avpriv
 functions thus  
>>>
>>> The reason some functions are avpriv instead of public is quite simple:
>>> it was considered too much effort to keep them stable enough to make
>>> them public.
>>>
>>> For these cases, we have the ff_ prefix, it works perfectly. But for
>>> some reason, somebody decided to make several separate libraries for
>>> this single project. Hence the avpriv prefix, which is exported but not
>>> public, with functions that are supposed to be... we don't know how
>>> stable. There is an implicit policy that shared lav* must all have the
>>> same version, but nothing ensures that technically.  
>>
>> Most avpriv_ functions exist solely to avoid code duplication. If it's
>> so much of an issue we could effectively duplicate them all on each
>> library as required by the different modules.
>>
>>>
>>> The whole mess stems from a huge core flaw: having separate libraries.
>>> It does not bring any benefit (at least, any benefit that could not be
>>> achieved otherwise without the drawbacks) and makes everything more
>>> complicated.
>>>
>>> So could we please stop beating around the bush and possibly address the
>>> actual issue?  
>>
>> No, because a *lot* of people only want a decoder or two, and don't want
>> to deal with a lot of non optional filter/device/format/resample/scale
>> framework they don't need bloating their applications.
>> Even the most minimalist build today still builds a lot of non optional
>> stuff in libavcodec that was made public for the purpose of getting rid
>> of avpriv_ symbols, like all those vorbis and aac header parsing
>> functions that i doubt anyone or anything outside of libav* ever uses.
>>
>> Hell, many downstream projects even add custom changes to remove all the
>> crap they don't need from libavutil because we still haven't made it a
>> modular library like the rest. See Chromium stripping everything but
>> md5, mem, buffer, pixfmt and such core functionality.
> 
> Could be solved with --disable-everything -.-enable-codec=whatyouwant.

That's what i called a minimalist build, and it still builds the entire
libavutil library plus a lot of unconditional objects in libavcodec
because of pointless public API that used to be avpriv_.

And that's what Chromium and such manually get rid of.

> 
> In addition we could add a flag to disable entire unneeded sub
> libraries. E.g. using only a decoder -> disable build of libswscale etc.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec: add a subcharenc mode that disables UTF-8 check

[Philip Langdale]

On Sat, 24 Mar 2018 13:43:21 +0100
wm4  wrote:

> This is for applications which want to explicitly check for invalid
> UTF-8 manually, and take actions that are better than dropping invalid
> subtitles silently. (It's pretty much silent because sporadic avcodec
> error messages are so common that you can't reasonably display them
> in a prominent and meaningful way in a application GUI.)
> ---
>  doc/APIchanges | 3 +++
>  libavcodec/avcodec.h   | 1 +
>  libavcodec/decode.c| 3 ++-
>  libavcodec/options_table.h | 1 +
>  libavcodec/version.h   | 2 +-
>  5 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index a099afd9bc..95b5cd772f 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil: 2017-10-21
>  
>  API changes, most recent first:
>  
> +2018-03-xx - xxx - lavc 58.16.100 - avcodec.h
> +  Add FF_SUB_CHARENC_MODE_IGNORE.
> +
>  2018-xx-xx - xxx - lavu 56.8.100 - encryption_info.h
>Add AVEncryptionInitInfo and AVEncryptionInfo structures to hold
> new side-data for encryption info.
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 495242faf0..50c34dbff9 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -3092,6 +3092,7 @@ typedef struct AVCodecContext {
>  #define FF_SUB_CHARENC_MODE_DO_NOTHING  -1  ///< do nothing (demuxer
> outputs a stream supposed to be already in UTF-8, or the codec is
> bitmap for instance) #define FF_SUB_CHARENC_MODE_AUTOMATIC0  ///<
> libavcodec will select the mode itself #define
> FF_SUB_CHARENC_MODE_PRE_DECODER  1  ///< the AVPacket data needs to
> be recoded to UTF-8 before being fed to the decoder, requires iconv
> +#define FF_SUB_CHARENC_MODE_IGNORE   2  ///< neither convert the
> subtitles, nor check them for valid UTF-8 /**
>   * Skip processing alpha if supported by codec.
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index ea2168ad0c..40c8a8855c 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1057,7 +1057,8 @@ int avcodec_decode_subtitle2(AVCodecContext
> *avctx, AVSubtitle *sub, sub->format = 1;
>  
>  for (i = 0; i < sub->num_rects; i++) {
> -if (sub->rects[i]->ass
> && !utf8_check(sub->rects[i]->ass)) {
> +if (avctx->sub_charenc_mode !=
> FF_SUB_CHARENC_MODE_IGNORE &&
> +sub->rects[i]->ass
> && !utf8_check(sub->rects[i]->ass)) { av_log(avctx, AV_LOG_ERROR,
> "Invalid UTF-8 in decoded subtitles text;
> " "maybe missing -sub_charenc option\n");
> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> index 5a5eae65fb..099261e168 100644
> --- a/libavcodec/options_table.h
> +++ b/libavcodec/options_table.h
> @@ -447,6 +447,7 @@ static const AVOption avcodec_options[] = {
>  {"do_nothing",  NULL, 0, AV_OPT_TYPE_CONST, {.i64 =
> FF_SUB_CHARENC_MODE_DO_NOTHING},  INT_MIN, INT_MAX, S|D,
> "sub_charenc_mode"}, {"auto",NULL, 0, AV_OPT_TYPE_CONST,
> {.i64 = FF_SUB_CHARENC_MODE_AUTOMATIC},   INT_MIN, INT_MAX, S|D,
> "sub_charenc_mode"}, {"pre_decoder", NULL, 0, AV_OPT_TYPE_CONST,
> {.i64 = FF_SUB_CHARENC_MODE_PRE_DECODER}, INT_MIN, INT_MAX, S|D,
> "sub_charenc_mode"}, +{"ignore",  NULL, 0, AV_OPT_TYPE_CONST,
> {.i64 = FF_SUB_CHARENC_MODE_IGNORE},  INT_MIN, INT_MAX, S|D,
> "sub_charenc_mode"}, #if FF_API_ASS_TIMING {"sub_text_format", "set
> decoded text subtitle format", OFFSET(sub_text_format),
> AV_OPT_TYPE_INT, {.i64 = FF_SUB_TEXT_FMT_ASS_WITH_TIMINGS}, 0, 1,
> S|D, "sub_text_format"}, #else diff --git a/libavcodec/version.h
> b/libavcodec/version.h index a5b7f752d1..8ac4626da7 100644 ---
> a/libavcodec/version.h +++ b/libavcodec/version.h @@ -28,7 +28,7 @@
> #include "libavutil/version.h" #define LIBAVCODEC_VERSION_MAJOR  58
> -#define LIBAVCODEC_VERSION_MINOR  15
> +#define LIBAVCODEC_VERSION_MINOR  16
>  #define LIBAVCODEC_VERSION_MICRO 100
>  
>  #define LIBAVCODEC_VERSION_INT
> AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \

LGTM.


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

Re: [FFmpeg-devel] [PATCH v4 3/7] cmdutils: use new iteration APIs

[wm4]

On Sun, 25 Mar 2018 17:46:50 +0200
Michael Niedermayer  wrote:

> On Sun, Mar 25, 2018 at 04:35:12PM +0100, Josh de Kock wrote:
> > On 2018/03/25 16:21, Michael Niedermayer wrote:  
> > >On Fri, Mar 23, 2018 at 11:05:22AM +0100, Nicolas George wrote:  
> > >>Josh de Kock (2018-03-22):  
> [...]
> >   
> > >People would just start to switch to the current API only to have it
> > >deprecated in the release after that and having to replace it again
> > >
> > >If the new API stays then I will most likely have to submit some ugly
> > >hack to workaround
> > >the size explosion issue for static linking with the current API.  
> >   
> 
> > What size explosion?  
> 
> the one for tools/target_dec_fuzzer
> 
> It links everything instead of just one codec after the changes, that makes
> it substantially bigger
> 

Michael. Haven't we discussed this to DEATH. Seriously.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avpriv cleanup

[wm4]

On Sun, 25 Mar 2018 12:47:41 -0300
James Almer  wrote:

> On 3/25/2018 12:19 PM, Nicolas George wrote:
> > Michael Niedermayer (2018-03-25):  
> >> looking at what we have as avprivs, in the tree, i think some of these
> >> could be indeed usefull as public APIs. And we need to already keep them
> >> compatible as they are exported as avpriv too, so making such changes does
> >> indeed in some cases look like a good idea to me
> >>
> >> There are some avprivs we have currently though that are quite specific
> >> to format intgernals, i dont think that its always a flaw to use avpriv
> >> functions thus  
> > 
> > The reason some functions are avpriv instead of public is quite simple:
> > it was considered too much effort to keep them stable enough to make
> > them public.
> > 
> > For these cases, we have the ff_ prefix, it works perfectly. But for
> > some reason, somebody decided to make several separate libraries for
> > this single project. Hence the avpriv prefix, which is exported but not
> > public, with functions that are supposed to be... we don't know how
> > stable. There is an implicit policy that shared lav* must all have the
> > same version, but nothing ensures that technically.  
> 
> Most avpriv_ functions exist solely to avoid code duplication. If it's
> so much of an issue we could effectively duplicate them all on each
> library as required by the different modules.
> 
> > 
> > The whole mess stems from a huge core flaw: having separate libraries.
> > It does not bring any benefit (at least, any benefit that could not be
> > achieved otherwise without the drawbacks) and makes everything more
> > complicated.
> > 
> > So could we please stop beating around the bush and possibly address the
> > actual issue?  
> 
> No, because a *lot* of people only want a decoder or two, and don't want
> to deal with a lot of non optional filter/device/format/resample/scale
> framework they don't need bloating their applications.
> Even the most minimalist build today still builds a lot of non optional
> stuff in libavcodec that was made public for the purpose of getting rid
> of avpriv_ symbols, like all those vorbis and aac header parsing
> functions that i doubt anyone or anything outside of libav* ever uses.
> 
> Hell, many downstream projects even add custom changes to remove all the
> crap they don't need from libavutil because we still haven't made it a
> modular library like the rest. See Chromium stripping everything but
> md5, mem, buffer, pixfmt and such core functionality.

Could be solved with --disable-everything -.-enable-codec=whatyouwant.

In addition we could add a flag to disable entire unneeded sub
libraries. E.g. using only a decoder -> disable build of libswscale etc.

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

Re: [FFmpeg-devel] [RFC] avpriv cleanup

[wm4]

On Sun, 25 Mar 2018 17:34:51 +0200
Michael Niedermayer  wrote:

> On Sun, Mar 25, 2018 at 05:07:33PM +0200, wm4 wrote:
> > On Sun, 25 Mar 2018 17:00:32 +0200
> > Michael Niedermayer  wrote:
> >   
> > > Hi all
> > > 
> > > Nicolas wrote an interresting comment about avpriv, and as this better
> > > belongs into a new thread, here it is in a new thread
> > > 
> > > On Sun, Mar 25, 2018 at 03:32:33PM +0200, Nicolas George wrote:  
> > > > Josh de Kock (2018-03-23):
> > > [...]
> > >
> > > > You can observe just that by the fact that you needed to add an avpriv
> > > > function to let lavd communicate with lavf. Each time an avpriv function
> > > > appears, it shows there is something flawed in the design.
> > > 
> > > If this is true, in general, then we can improve designs by removing
> > > avpriv_* functions ...
> > > 
> > > looking at what we have as avprivs, in the tree, i think some of these
> > > could be indeed usefull as public APIs. And we need to already keep them
> > > compatible as they are exported as avpriv too, so making such changes does
> > > indeed in some cases look like a good idea to me
> > > 
> > > There are some avprivs we have currently though that are quite specific
> > > to format intgernals, i dont think that its always a flaw to use avpriv
> > > functions thus
> > > 
> > > but i think we should evaluate each of the currently existing avpriv
> > > functions and check if the API wouldnt be usefull to user applications.
> > > And if it wouldnt be better to make it properly public  
> >   
> 
> > This might apply to some cases, but in general: just no.  
> 
> That is what i meant :)
> 
> 
> > 
> > We make things avpriv_ *because* we don't want them to be public API,
> > but the odd multiple-library design forces us to. Not more, not less.
> >   
> 
> > What is this thread even for? If you want to make certain functions
> > public, post specific RFCs or patches. In general, we probably all
> > agree that avpriv functions are necessary hacks to deal with the split
> > library nonsense.  
> 
> The thread is for finding out peoples oppinon before anyone spends time
> writing patches.
> For example it takes time to go over the functions and time to write patches
> that turn functions with proper documention into public functions.
> 
> If people object to it, no point in anyone doing any of this work
> 
> Also there have been people looking for things to work on. If we agree
> that looking over avpriv functions for more generally usefull ones 
> makes sense then we need to discuss this in some thread for anyone
> to be able to see that suggestion.

Well, just generally suggesting to remove avpriv functions is not going
to work, because they exist for a reason (see e.g. Nicolas George or my
post). Every specific case likely needs a separate discussion, so it
doesn't make sense to have some sort of summary discussion.

> For a specific function
> As a random example, lets take avpriv_set_systematic_pal2()
> It provides a palette for 8bit per pixel formats which have a constant
> palette. It sounds usefull to have this public to me ...

For that specific example I'd rather argue we should get rid of
AV_PIX_FMT_FLAG_PSEUDOPAL. It's an awful hack to allow code to treat 1
byte formats as paletted, such as AV_PIXFMT_GRAY8. Basically AVFrames
with this pixfmt must come with a palette that acts as lookup table,
that treats the 1 byte value as index into the table to convert it to
RGB. The avpriv_set_systematic_pal2() just sets the palette for those
formats.

This is broken and bad for the following reasons:

1. The affected pixfmts are NOT paletted. So they shouldn't have a
   palette. It's against the principle of least astonishment (basically
   surprising the user with some broken obscure bullshit, instead of
   things just working). The only reason why I know about this pseudi
   PAL feature is because once I got a segfault by passing a self
   allocated GRAY8 frame to libswscale.
2. I suspect it's a hack only used by libswscale to speed up some
   trivial conversions. But libswscale can just create and keep such
   lookup table internally. Not a problem. This mechanism is not needed
   in the public API.
3. Thus why clutter public API and burden the API user with this?
4. It's actually broken. The actual RGB mapping depends on the color
   levels for GRAY8 (because GRAY8 is essentially a Y plane). The
   lookup table is valid only for full range GRAY8, for other values
   it's broken. The API function you suggest to make public is broken
   in this way too, because it takes only the pixfmt, not levels or
   other colorimetry info.
5. Even if the things before gets fixed, nobody is going to call the
   function after updating color levels on the AVFrame. Thus it's
   fragile and error prone.

Do you still want to make that function public?

This is something I always wanted to fix though. I might send a patch
to deprecate PSEUDOPAL 

Re: [FFmpeg-devel] [RFC] avpriv cleanup

[wm4]

On Sun, 25 Mar 2018 17:39:17 +0200
Nicolas George  wrote:

> Michael Niedermayer (2018-03-25):
> > The thread is for finding out peoples oppinon before anyone spends time
> > writing patches.  
> 
> If you have time to spend on the avpriv issue, I suggest you spend it
> trying to get this working. It would be most beneficial.
> 

I'm not against this patch in general, but...

Do you want to block the new FFmpeg release and the new API over this?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC] avpriv cleanup

[James Almer]

On 3/25/2018 12:19 PM, Nicolas George wrote:
> Michael Niedermayer (2018-03-25):
>> looking at what we have as avprivs, in the tree, i think some of these
>> could be indeed usefull as public APIs. And we need to already keep them
>> compatible as they are exported as avpriv too, so making such changes does
>> indeed in some cases look like a good idea to me
>>
>> There are some avprivs we have currently though that are quite specific
>> to format intgernals, i dont think that its always a flaw to use avpriv
>> functions thus
> 
> The reason some functions are avpriv instead of public is quite simple:
> it was considered too much effort to keep them stable enough to make
> them public.
> 
> For these cases, we have the ff_ prefix, it works perfectly. But for
> some reason, somebody decided to make several separate libraries for
> this single project. Hence the avpriv prefix, which is exported but not
> public, with functions that are supposed to be... we don't know how
> stable. There is an implicit policy that shared lav* must all have the
> same version, but nothing ensures that technically.

Most avpriv_ functions exist solely to avoid code duplication. If it's
so much of an issue we could effectively duplicate them all on each
library as required by the different modules.

> 
> The whole mess stems from a huge core flaw: having separate libraries.
> It does not bring any benefit (at least, any benefit that could not be
> achieved otherwise without the drawbacks) and makes everything more
> complicated.
> 
> So could we please stop beating around the bush and possibly address the
> actual issue?

No, because a *lot* of people only want a decoder or two, and don't want
to deal with a lot of non optional filter/device/format/resample/scale
framework they don't need bloating their applications.
Even the most minimalist build today still builds a lot of non optional
stuff in libavcodec that was made public for the purpose of getting rid
of avpriv_ symbols, like all those vorbis and aac header parsing
functions that i doubt anyone or anything outside of libav* ever uses.

Hell, many downstream projects even add custom changes to remove all the
crap they don't need from libavutil because we still haven't made it a
modular library like the rest. See Chromium stripping everything but
md5, mem, buffer, pixfmt and such core functionality.

> 
> Regards,
> 
> 
> 
> ___
> 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 v4 3/7] cmdutils: use new iteration APIs

[Michael Niedermayer]

On Sun, Mar 25, 2018 at 04:35:12PM +0100, Josh de Kock wrote:
> On 2018/03/25 16:21, Michael Niedermayer wrote:
> >On Fri, Mar 23, 2018 at 11:05:22AM +0100, Nicolas George wrote:
> >>Josh de Kock (2018-03-22):
[...]
> 
> >People would just start to switch to the current API only to have it
> >deprecated in the release after that and having to replace it again
> >
> >If the new API stays then I will most likely have to submit some ugly
> >hack to workaround
> >the size explosion issue for static linking with the current API.
> 

> What size explosion?

the one for tools/target_dec_fuzzer

It links everything instead of just one codec after the changes, that makes
it substantially bigger


> 
> >And that for each lib not just avcodec.
> >Thats to allow the ffmpeg ossfuzz code to grow and test more things
> >quickly within the available resources.
> I honestly would like to be an idealist here, but it's much more practical
> to just be real here. Nothing happens in this project unless the
> conversation begins with a patch (and even then, stuff barely happens). So
> in the words of others on the list:
> 
> You forgot to attach your patch.

yes, ill likely write that in case the API is not reverted. 

thx

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

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin


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

Re: [FFmpeg-devel] [RFC] avpriv cleanup

[Nicolas George]

Michael Niedermayer (2018-03-25):
> The thread is for finding out peoples oppinon before anyone spends time
> writing patches.

If you have time to spend on the avpriv issue, I suggest you spend it
trying to get this working. It would be most beneficial.

Regards,

-- 
  Nicolas George
From fcf7b622135a845e66bfa164447adb5fecacb1f6 Mon Sep 17 00:00:00 2001
From: Nicolas George 
Date: Mon, 31 Jul 2017 16:41:08 +0200
Subject: [PATCH] all: merge all libraries.

Signed-off-by: Nicolas George 
---
 Makefile |  6 +++-
 configure|  2 +-
 ffbuild/common.mak   | 13 ---
 ffbuild/library.mak  | 63 +-
 libavcodec/Makefile  |  1 -
 libavcodec/log2_tab.c|  1 -
 libavcodec/reverse.c |  1 -
 libavdevice/Makefile |  1 -
 libavdevice/reverse.c|  1 -
 libavfilter/Makefile |  2 --
 libavfilter/log2_tab.c   |  1 -
 libavformat/Makefile |  1 -
 libavformat/golomb_tab.c |  1 -
 libavformat/log2_tab.c   |  1 -
 libffmpeg/Makefile   | 88 
 libffmpeg/libffmpeg.v| 23 +
 libswresample/Makefile   |  1 -
 libswresample/log2_tab.c |  1 -
 libswscale/Makefile  |  2 --
 libswscale/log2_tab.c|  1 -
 20 files changed, 126 insertions(+), 85 deletions(-)
 delete mode 100644 libavcodec/log2_tab.c
 delete mode 100644 libavcodec/reverse.c
 delete mode 100644 libavdevice/reverse.c
 delete mode 100644 libavfilter/log2_tab.c
 delete mode 100644 libavformat/golomb_tab.c
 delete mode 100644 libavformat/log2_tab.c
 create mode 100644 libffmpeg/Makefile
 create mode 100644 libffmpeg/libffmpeg.v
 delete mode 100644 libswresample/log2_tab.c
 delete mode 100644 libswscale/log2_tab.c

diff --git a/Makefile b/Makefile
index 29870d7710..4aaa966f4a 100644
--- a/Makefile
+++ b/Makefile
@@ -118,6 +118,10 @@ endef
 
 $(foreach D,$(FFLIBS),$(eval $(call DOSUBDIR,lib$(D
 
+SUBDIR := libffmpeg/
+include $(SRC_PATH)/$(SUBDIR)Makefile
+SUBDIR := diediedie
+
 include $(SRC_PATH)/doc/Makefile
 
 define DOPROG
@@ -208,7 +212,7 @@ check: all alltools examples testprogs fate
 
 include $(SRC_PATH)/tests/Makefile
 
-$(sort $(OBJDIRS)):
+$(sort $(OBJDIRS)) libffmpeg/:
 	$(Q)mkdir -p $@
 
 # Dummy rule to stop make trying to rebuild removed or renamed headers
diff --git a/configure b/configure
index 66c7b948e4..e3d18ff226 100755
--- a/configure
+++ b/configure
@@ -6226,7 +6226,7 @@ EOF
 
 # add some linker flags
 check_ldflags -Wl,--warn-common
-check_ldflags -Wl,-rpath-link=libpostproc:libswresample:libswscale:libavfilter:libavdevice:libavformat:libavcodec:libavutil:libavresample
+check_ldflags -Wl,-rpath-link=libffmpeg
 enabled rpath && add_ldexeflags -Wl,-rpath,$libdir
 enabled rpath && add_ldlibflags -Wl,-rpath,$libdir
 test_ldflags -Wl,-Bsymbolic && append SHFLAGS -Wl,-Bsymbolic
diff --git a/ffbuild/common.mak b/ffbuild/common.mak
index e168fb2cfd..7499db0865 100644
--- a/ffbuild/common.mak
+++ b/ffbuild/common.mak
@@ -41,7 +41,7 @@ X86ASMFLAGS += $(IFLAGS:%=%/) -I$(

Re: [FFmpeg-devel] [PATCH v4 3/7] cmdutils: use new iteration APIs

[Josh de Kock]

On 2018/03/25 16:21, Michael Niedermayer wrote:

On Fri, Mar 23, 2018 at 11:05:22AM +0100, Nicolas George wrote:

Josh de Kock (2018-03-22):

move lavd avinputformats and avoutputformats into lavf

delete lavd


Possibly ok in principle for me, but see below.


write new lavd aimed at actual devices


There are already such libraries, we do not need another. The basic
devices with a (de)muxer API are quite right to give many extra features
with little extra cost.

But why are we discussing this? It seems to me that the discussion went
approximately like this:

"Darn, the faucet I just bought to fix the leaky one does not fit the
pipes. Well, I guess I will have to redo the whole plumbing to make it
fit."

The correct way of addressing the problem is to buy a new faucet with
the correct size. And cut the losses if the first one cannot be
refunded. I feel like the discussion is largely fueled by the cognitive
bias known as "sunk cost fallacy": due to efforts invested in a
solution, become attached emotionally to it and fail to see when it
proves to cause more costs than benefits.

Can we at least REALLY CONSIDER this:

1. Acknowledge that this issue about lavd, on top of Michael's early
concerns about registering external components, has proven that the
all-static approach, while elegant in many ways, is not practical.




2. Agree to revert the API as it is and discuss a better solution.


iam in favor of reverting the API, there is apparently discussion going
on now here to design a different, better API and IMHO its better not to
introduce a new API now if there is active work going on to change it.


Sure, if someone else reverts, designs, write, integrates, deals with 
the never-ending bikeshedding, fixes issues around lavf/lavd's broken 
shit, why not? But that's not going to happen, let's be real. If anyone 
actually cared enough, then it would be fixed already. It's been broken 
since version 0.5, the only reason people care now is because they're 
scared of change. It may not be the 'best' API, but if everything was 
designed the 'best' then we wouldn't have to deal with stuff like this.



People would just start to switch to the current API only to have it
deprecated in the release after that and having to replace it again

If the new API stays then I will most likely have to submit some ugly
hack to workaround
the size explosion issue for static linking with the current API.


What size explosion?


And that for each lib not just avcodec.
Thats to allow the ffmpeg ossfuzz code to grow and test more things
quickly within the available resources.
I honestly would like to be an idealist here, but it's much more 
practical to just be real here. Nothing happens in this project unless 
the conversation begins with a patch (and even then, stuff barely 
happens). So in the words of others on the list:


You forgot to attach your patch.

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

Re: [FFmpeg-devel] [RFC] avpriv cleanup

[Michael Niedermayer]

On Sun, Mar 25, 2018 at 05:07:33PM +0200, wm4 wrote:
> On Sun, 25 Mar 2018 17:00:32 +0200
> Michael Niedermayer  wrote:
> 
> > Hi all
> > 
> > Nicolas wrote an interresting comment about avpriv, and as this better
> > belongs into a new thread, here it is in a new thread
> > 
> > On Sun, Mar 25, 2018 at 03:32:33PM +0200, Nicolas George wrote:
> > > Josh de Kock (2018-03-23):  
> > [...]
> >  
> > > You can observe just that by the fact that you needed to add an avpriv
> > > function to let lavd communicate with lavf. Each time an avpriv function
> > > appears, it shows there is something flawed in the design.  
> > 
> > If this is true, in general, then we can improve designs by removing
> > avpriv_* functions ...
> > 
> > looking at what we have as avprivs, in the tree, i think some of these
> > could be indeed usefull as public APIs. And we need to already keep them
> > compatible as they are exported as avpriv too, so making such changes does
> > indeed in some cases look like a good idea to me
> > 
> > There are some avprivs we have currently though that are quite specific
> > to format intgernals, i dont think that its always a flaw to use avpriv
> > functions thus
> > 
> > but i think we should evaluate each of the currently existing avpriv
> > functions and check if the API wouldnt be usefull to user applications.
> > And if it wouldnt be better to make it properly public
> 

> This might apply to some cases, but in general: just no.

That is what i meant :)


> 
> We make things avpriv_ *because* we don't want them to be public API,
> but the odd multiple-library design forces us to. Not more, not less.
> 

> What is this thread even for? If you want to make certain functions
> public, post specific RFCs or patches. In general, we probably all
> agree that avpriv functions are necessary hacks to deal with the split
> library nonsense.

The thread is for finding out peoples oppinon before anyone spends time
writing patches.
For example it takes time to go over the functions and time to write patches
that turn functions with proper documention into public functions.

If people object to it, no point in anyone doing any of this work

Also there have been people looking for things to work on. If we agree
that looking over avpriv functions for more generally usefull ones 
makes sense then we need to discuss this in some thread for anyone
to be able to see that suggestion.

For a specific function
As a random example, lets take avpriv_set_systematic_pal2()
It provides a palette for 8bit per pixel formats which have a constant
palette. It sounds usefull to have this public to me ...


[...]
-- 
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 v4 3/7] cmdutils: use new iteration APIs

[Michael Niedermayer]

On Fri, Mar 23, 2018 at 11:05:22AM +0100, Nicolas George wrote:
> Josh de Kock (2018-03-22):
> > move lavd avinputformats and avoutputformats into lavf
> > 
> > delete lavd
> 
> Possibly ok in principle for me, but see below.
> 
> > write new lavd aimed at actual devices
> 
> There are already such libraries, we do not need another. The basic
> devices with a (de)muxer API are quite right to give many extra features
> with little extra cost.
> 
> But why are we discussing this? It seems to me that the discussion went
> approximately like this:
> 
> "Darn, the faucet I just bought to fix the leaky one does not fit the
> pipes. Well, I guess I will have to redo the whole plumbing to make it
> fit."
> 
> The correct way of addressing the problem is to buy a new faucet with
> the correct size. And cut the losses if the first one cannot be
> refunded. I feel like the discussion is largely fueled by the cognitive
> bias known as "sunk cost fallacy": due to efforts invested in a
> solution, become attached emotionally to it and fail to see when it
> proves to cause more costs than benefits.
> 
> Can we at least REALLY CONSIDER this:
> 
> 1. Acknowledge that this issue about lavd, on top of Michael's early
>concerns about registering external components, has proven that the
>all-static approach, while elegant in many ways, is not practical.
> 

> 2. Agree to revert the API as it is and discuss a better solution.

iam in favor of reverting the API, there is apparently discussion going
on now here to design a different, better API and IMHO its better not to
introduce a new API now if there is active work going on to change it.

People would just start to switch to the current API only to have it
deprecated in the release after that and having to replace it again

If the new API stays then I will most likely have to submit some ugly
hack to workaround
the size explosion issue for static linking with the current API.
And that for each lib not just avcodec. 
Thats to allow the ffmpeg ossfuzz code to grow and test more things
quickly within the available resources.

thx


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

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire


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

Re: [FFmpeg-devel] [RFC] avpriv cleanup

[Nicolas George]

Michael Niedermayer (2018-03-25):
> looking at what we have as avprivs, in the tree, i think some of these
> could be indeed usefull as public APIs. And we need to already keep them
> compatible as they are exported as avpriv too, so making such changes does
> indeed in some cases look like a good idea to me
> 
> There are some avprivs we have currently though that are quite specific
> to format intgernals, i dont think that its always a flaw to use avpriv
> functions thus

The reason some functions are avpriv instead of public is quite simple:
it was considered too much effort to keep them stable enough to make
them public.

For these cases, we have the ff_ prefix, it works perfectly. But for
some reason, somebody decided to make several separate libraries for
this single project. Hence the avpriv prefix, which is exported but not
public, with functions that are supposed to be... we don't know how
stable. There is an implicit policy that shared lav* must all have the
same version, but nothing ensures that technically.

The whole mess stems from a huge core flaw: having separate libraries.
It does not bring any benefit (at least, any benefit that could not be
achieved otherwise without the drawbacks) and makes everything more
complicated.

So could we please stop beating around the bush and possibly address the
actual issue?

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] [RFC] avpriv cleanup

[wm4]

On Sun, 25 Mar 2018 17:00:32 +0200
Michael Niedermayer  wrote:

> Hi all
> 
> Nicolas wrote an interresting comment about avpriv, and as this better
> belongs into a new thread, here it is in a new thread
> 
> On Sun, Mar 25, 2018 at 03:32:33PM +0200, Nicolas George wrote:
> > Josh de Kock (2018-03-23):  
> [...]
>  
> > You can observe just that by the fact that you needed to add an avpriv
> > function to let lavd communicate with lavf. Each time an avpriv function
> > appears, it shows there is something flawed in the design.  
> 
> If this is true, in general, then we can improve designs by removing
> avpriv_* functions ...
> 
> looking at what we have as avprivs, in the tree, i think some of these
> could be indeed usefull as public APIs. And we need to already keep them
> compatible as they are exported as avpriv too, so making such changes does
> indeed in some cases look like a good idea to me
> 
> There are some avprivs we have currently though that are quite specific
> to format intgernals, i dont think that its always a flaw to use avpriv
> functions thus
> 
> but i think we should evaluate each of the currently existing avpriv
> functions and check if the API wouldnt be usefull to user applications.
> And if it wouldnt be better to make it properly public

This might apply to some cases, but in general: just no.

We make things avpriv_ *because* we don't want them to be public API,
but the odd multiple-library design forces us to. Not more, not less.

What is this thread even for? If you want to make certain functions
public, post specific RFCs or patches. In general, we probably all
agree that avpriv functions are necessary hacks to deal with the split
library nonsense.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [RFC] avpriv cleanup

[Michael Niedermayer]

Hi all

Nicolas wrote an interresting comment about avpriv, and as this better
belongs into a new thread, here it is in a new thread

On Sun, Mar 25, 2018 at 03:32:33PM +0200, Nicolas George wrote:
> Josh de Kock (2018-03-23):
[...]
 
> You can observe just that by the fact that you needed to add an avpriv
> function to let lavd communicate with lavf. Each time an avpriv function
> appears, it shows there is something flawed in the design.

If this is true, in general, then we can improve designs by removing
avpriv_* functions ...

looking at what we have as avprivs, in the tree, i think some of these
could be indeed usefull as public APIs. And we need to already keep them
compatible as they are exported as avpriv too, so making such changes does
indeed in some cases look like a good idea to me

There are some avprivs we have currently though that are quite specific
to format intgernals, i dont think that its always a flaw to use avpriv
functions thus

but i think we should evaluate each of the currently existing avpriv
functions and check if the API wouldnt be usefull to user applications.
And if it wouldnt be better to make it properly public

thx


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus


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

Re: [FFmpeg-devel] [PATCH] avcodec/dcaenc: Use aac psychoacoustic model for dcaenc

[Michael Niedermayer]

On Sun, Mar 25, 2018 at 01:55:42PM +0300, Даниил Чередник wrote:
[...]

>  libavcodec/dcaenc.c   |  369 
> --
>  libavcodec/psymodel.c |1 
>  tests/fate/acodec.mak |4 
>  3 files changed, 156 insertions(+), 218 deletions(-)
> fd146632a7f29530a59d35abd63149c81b4dfcc6  
> 0002-avcodec-dcaenc-Use-aac-psychoacoustic-model-for-DCA-.patch
> From 99d937a0828bbd60aef52d7979c75f8c21989145 Mon Sep 17 00:00:00 2001
> From: Daniil Cherednik 
> Date: Sun, 4 Mar 2018 13:14:17 +
> Subject: [PATCH 2/2] avcodec/dcaenc: Use aac psychoacoustic model for DCA
>  encoder
> 
> There are several reasons to replace dca psychoacoustic to common model:
>  - dca psychoacoustic has some quality problems especially at high frequency 
> bands
>  - unclear implementation
>  - aac implementation allows to use tonality measurement for future 
> improvements
>  - a bit faster

This breaks 
make -j12 fate-acodec-dca
(segfaults)

tell me if you can reproduce? If not ill rebuild with debug symbols and get a
backtrace

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

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.


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

Re: [FFmpeg-devel] [PATCH v4 3/7] cmdutils: use new iteration APIs

[wm4]

On Sun, 25 Mar 2018 16:41:12 +0200
Michael Niedermayer  wrote:

> On Sun, Mar 25, 2018 at 03:32:33PM +0200, Nicolas George wrote:
> > Josh de Kock (2018-03-23):  
> [...]
> 
> > You can observe just that by the fact that you needed to add an avpriv
> > function to let lavd communicate with lavf. Each time an avpriv function
> > appears, it shows there is something flawed in the design.  

Yes, libavdevice is flawed. It's a "separate" library, but uses a lot
of internal libavformat functionality, against all API and ABI rules.
This can't be fixed with a more fancy component registration scheme.

> If this is true, in general, then we can improve designs by removing 
> avpriv_* functions ...

Not necessarily. avpriv_ functions exist in the first place to avoid
worse things. Like making things public that shouldn't be public.

> looking at what we have as avprivs, in the tree, i think some of these
> could be indeed usefull as public APIs. And we need to already keep them 
> compatible as they are exported as avpriv too, so making such changes does 
> indeed in some cases look like a good idea to me
> 
> There are some avprivs we have currently though that are quite specific
> to format intgernals, i dont think that its always a flaw to use avpriv
> functions thus
> 
> but i think we should evaluate each of the currently existing avpriv
> functions and check if the API wouldnt be usefull to user applications.
> And if it wouldnt be better to make it properly public

This is 100% offtopic bikeshedding. Please don't make this discussion
harder than it is.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v4 3/7] cmdutils: use new iteration APIs

[Michael Niedermayer]

On Sun, Mar 25, 2018 at 03:32:33PM +0200, Nicolas George wrote:
> Josh de Kock (2018-03-23):
[...]

> You can observe just that by the fact that you needed to add an avpriv
> function to let lavd communicate with lavf. Each time an avpriv function
> appears, it shows there is something flawed in the design.

If this is true, in general, then we can improve designs by removing 
avpriv_* functions ...

looking at what we have as avprivs, in the tree, i think some of these
could be indeed usefull as public APIs. And we need to already keep them 
compatible as they are exported as avpriv too, so making such changes does 
indeed in some cases look like a good idea to me

There are some avprivs we have currently though that are quite specific
to format intgernals, i dont think that its always a flaw to use avpriv
functions thus

but i think we should evaluate each of the currently existing avpriv
functions and check if the API wouldnt be usefull to user applications.
And if it wouldnt be better to make it properly public

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato


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

[FFmpeg-devel] [PATCH] lavc/cfhd: error due to improper allocation of height in buffers

[Gagandeep Singh]

ticket #6675 the distortion in the bottom 8 pixels fixed
---
 libavcodec/cfhd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
index e35732df45..f10742f4fa 100644
--- a/libavcodec/cfhd.c
+++ b/libavcodec/cfhd.c
@@ -213,13 +213,14 @@ static int alloc_buffers(AVCodecContext *avctx)
 int width  = i ? avctx->width  >> chroma_x_shift : avctx->width;
 int height = i ? avctx->height >> chroma_y_shift : avctx->height;
 ptrdiff_t stride = FFALIGN(width  / 8, 8) * 8;
-height   = FFALIGN(height / 8, 2) * 8;
+if (chroma_y_shift)
+height = FFALIGN(height / 8, 2) * 8;
 s->plane[i].width  = width;
 s->plane[i].height = height;
 s->plane[i].stride = stride;
 
 w8 = FFALIGN(s->plane[i].width  / 8, 8);
-h8 = FFALIGN(s->plane[i].height / 8, 2);
+h8 = height / 8;
 w4 = w8 * 2;
 h4 = h8 * 2;
 w2 = w4 * 2;
-- 
2.14.1

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

Re: [FFmpeg-devel] [PATCH] avcodec/avpacket: ensure the packet is writable in av_shrink_packet()

[James Almer]

On 3/25/2018 9:14 AM, wm4 wrote:
> On Sat, 24 Mar 2018 22:39:45 -0300
> James Almer  wrote:
> 
>> On 3/24/2018 6:46 PM, wm4 wrote:
>>> On Sat, 24 Mar 2018 18:11:53 -0300
>>> James Almer  wrote:
>>>   
 Signed-off-by: James Almer 
 ---
 This is a good time to deprecate this function and introduce a
 replacement using the correct av_packet namespace and this time
 returning an int.
 What would be better

 int av_packet_shrink(AVPacket *pkt, int size);

 Or
  
>>>   
 int av_packet_resize(AVPacket *pkt, int size);  
>>>
>>> Seems better.
>>>   

 The latter would be a combination of both the current shrink and grow
 functions.

  libavcodec/avpacket.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
 index 0693ca6f62..7faa082395 100644
 --- a/libavcodec/avpacket.c
 +++ b/libavcodec/avpacket.c
 @@ -100,9 +100,12 @@ int av_new_packet(AVPacket *pkt, int size)
  
  void av_shrink_packet(AVPacket *pkt, int size)
  {
 +int packet_is_writable;
  if (pkt->size <= size)
  return;
  pkt->size = size;
 +packet_is_writable = !av_packet_make_writable(pkt);
 +av_assert0(packet_is_writable);
  memset(pkt->data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
  }

>>>
>>> LGTM  
>>
>> Ok, but i just realized that i can't apply this without first writing an
>> internal variant specifically for the encoders that makes sure pkt->data
>> != avctx->internal->byte_buffer before trying to do anything, otherwise
>> the supposed benefits of that weird internal buffer code in
>> ff_alloc_packet2() would be lost.
>>
>> Will look at that later.
> 
> Didn't look at it again, but this mechanism might be broken anyway
> since the new encode API must return refcounted packets.
> 
> Maybe the new dynamic sized buffer pool would help.

Good point. Still waiting for suggestions about how to introduce that in
the relevant thread, for that matter :p
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/af_pan: parse properly expressions referencing the same channel multiple times

[Nicolas George]

Marton Balint (2018-03-24):
> Fixes parsing of expressions like c0=c0+c0. Previously no error was thrown and
> only the last gain factor was used.
> 
> Signed-off-by: Marton Balint 
> ---
>  libavfilter/af_pan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks for finding that misfeature. But I think your fixes goes in the
wrong direction. I think that "c1=c1+c1" is much more likely to be a
typo for "c1=c1+c4" than an intentional way of writing "c1=2*c1".
Therefore I think it would be better to return an error, or at least
print a warning, than accepting it silently.

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 v4 3/7] cmdutils: use new iteration APIs

[Nicolas George]

Josh de Kock (2018-03-23):
> I personally think this will fix a lot of the weird interactions between the
> two libraries, and should be considered a serious option independent of the
> new API.

Maybe. But that is for another discussion.

> Sure, I guess but I still think it's more involved than just 'one faucet not
> fitting'. There's still an entire pipe which doesn't fit but was still
> installed with duct tape.

Possibly, but that still does not warrant redoing the whole plumbing.

> I mean it's so far gone that I don't think it matters how long it takes
> anymore as long as it gets done, and gets done 'right'. This is a release
> blocker,

This is one of the reasons I am against releases. Still, we can revert
and take all the time we need.

>  and yes that's my bad but I do think that some earlier help (when I
> asked even before starting the new API) from others would have maybe avoided
> the current situation.

> I hope that in the future it will be less of a 'send a patch and we'll
> ACK/NACK', because some things really benefit from discussion.

I completely agree that this project is awful in that matter. We should
strive to change that and find better practices.

Maybe post a mail tagged [METAPATCH] to discuss the principle of the
changes, treat it like a patch in that it needs to be approved before
being adopted. But once it is adopted, the discussion can move forward
with the patches themselves, and objections about the principle can be
disregarded.

Note that this would not have helped much here, since nobody foresaw the
complication about lavd.

> If you submit a set to revert it (my git skills suck), I won't block it,
> provided there will actually be discussion--from what I've seen, discussion
> only occurs after things happen, which isn't very helpful for larger more
> impactful changes.

I will try to do just that, if time permits. But time is very short for
me this week.

> So the reason I liked having an iteration-style function was that it allowed
> an API user to get an array of all the components *but it didn't force
> them*.

If an array is available, you can always look at only one cell.

> Your suggestion here is interesting, from what I understand it seems to be a
> thread-local, user-managed state of the loaded components in each library?
> Coupled with a new and proper registration API (the previous one wasn't
> ideal for registering external components. The downside of this approach is
> then you go in completely the opposite direction: the library requires even
> more 'bootstrap'. The reason for the new API wasn't just to use arrays
> internally, but to have no initialisation required.

Some of your comments show that I did not explain what I propose well
enough, causing you to completely misunderstand it. Let me explain it
more precisely. There is nothing thread-local about it. What I propose
looks like this:

AVFormatLibrary *lavf = avformat_library_alloc();
avformat_register_all(lavf);
avdevice_register_all(lavf);
...
avformat_open_input2(lavf, , url, NULL, );

The benefit is that the global state becomes explicit, and not global:
it can exist in several instances.

Another benefit is that is provides a much cleaner and more reliable
black-/white-listing solution that the one we currently have.

I acknowledge your efforts to make things more static, but I think this
discussion has proven it was a mistake. We cannot eradicate all needs
for dynamic initialization, and a design that is meant for almost all
static will make the few cases where static is not possible very
hackish.

You can observe just that by the fact that you needed to add an avpriv
function to let lavd communicate with lavf. Each time an avpriv function
appears, it shows there is something flawed in the design.

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 0/5] Add lavfi api & remove device iteration

[Josh de Kock]

On 2018/03/23 20:48, Josh de Kock wrote:

This set is an alternative to the previous set I posted, it makes
it so that the current behaviour is kept (devices are returned in
by the iteration functions but must be loaded in manually). It
is a compromise between what Nicolas George suggested and a full
rewrite.

I personally would like to just 'get this done', and I feel other
than completely rewriting the API from scratch (and the previous
set I posted), merging lavf and lavd is the only other option.



Just as a side-note, I am planning to push this on the 1st of April. If 
you have any objections, comments, or related proposals please make them 
before then so that I can either further delay it or we can resolve them 
beforehand.


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

Re: [FFmpeg-devel] [PATCH] vf_avgblur_opencl: Don't run kernel on pixels outside the image

[Dylan Fernando]

On Sun, Mar 25, 2018 at 1:06 AM, Mark Thompson  wrote:

> The output frame size is larger than the image containing a subsampled
> plane - use the actual size of the image being written rather than the
> dimensions of the intended output frame.
> ---
>  libavfilter/vf_avgblur_opencl.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/libavfilter/vf_avgblur_opencl.c
> b/libavfilter/vf_avgblur_opencl.c
> index 5ee66c0ba2..3a5b4a28ca 100644
> --- a/libavfilter/vf_avgblur_opencl.c
> +++ b/libavfilter/vf_avgblur_opencl.c
> @@ -170,8 +170,10 @@ static int avgblur_opencl_filter_frame(AVFilterLink
> *inlink, AVFrame *input)
>  goto fail;
>  }
>
> -global_work[0] = output->width;
> -global_work[1] = output->height;
> +err = ff_opencl_filter_work_size_from_image(avctx, global_work,
> +intermediate, p, 0);
> +if (err < 0)
> +goto fail;
>
>  av_log(avctx, AV_LOG_DEBUG, "Run kernel on plane %d "
> "(%"SIZE_SPECIFIER"x%"SIZE_SPECIFIER").\n",
> @@ -206,8 +208,10 @@ static int avgblur_opencl_filter_frame(AVFilterLink
> *inlink, AVFrame *input)
>  goto fail;
>  }
>
> -global_work[0] = output->width;
> -global_work[1] = output->height;
> +err = ff_opencl_filter_work_size_from_image(avctx, global_work,
> +output, p, 0);
> +if (err < 0)
> +goto fail;
>
>  av_log(avctx, AV_LOG_DEBUG, "Run kernel on plane %d "
> "(%"SIZE_SPECIFIER"x%"SIZE_SPECIFIER").\n",
> --
> 2.16.1
>
Thanks. I tried the patch, it works correctly.

Following is a patch attempting to fix the err issue. It returns -1 if any
clSetKernelArg() fails. Is this good, or should I be using a different
return value for this error?

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

[FFmpeg-devel] [PATCH] avfilter/vf_avgblur_opencl: fix error when clSetKernelArg fails

[dylanf123]

From: drfer3 

Fixes Coverity CID 1430382
---
 libavfilter/vf_avgblur_opencl.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavfilter/vf_avgblur_opencl.c b/libavfilter/vf_avgblur_opencl.c
index 5ee66c0ba2..09caa1fd4f 100644
--- a/libavfilter/vf_avgblur_opencl.c
+++ b/libavfilter/vf_avgblur_opencl.c
@@ -155,18 +155,21 @@ static int avgblur_opencl_filter_frame(AVFilterLink 
*inlink, AVFrame *input)
 if (cle != CL_SUCCESS) {
 av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
"destination image argument: %d.\n", cle);
+err = -1;
 goto fail;
 }
 cle = clSetKernelArg(ctx->kernel_horiz, 1, sizeof(cl_mem), );
 if (cle != CL_SUCCESS) {
 av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
"source image argument: %d.\n", cle);
+err = -1;
 goto fail;
 }
 cle = clSetKernelArg(ctx->kernel_horiz, 2, sizeof(cl_int), _x);
 if (cle != CL_SUCCESS) {
 av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
"sizeX argument: %d.\n", cle);
+err = -1;
 goto fail;
 }
 
@@ -191,18 +194,21 @@ static int avgblur_opencl_filter_frame(AVFilterLink 
*inlink, AVFrame *input)
 if (cle != CL_SUCCESS) {
 av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
"destination image argument: %d.\n", cle);
+err = -1;
 goto fail;
 }
 cle = clSetKernelArg(ctx->kernel_vert, 1, sizeof(cl_mem), );
 if (cle != CL_SUCCESS) {
 av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
"source image argument: %d.\n", cle);
+err = -1;
 goto fail;
 }
 cle = clSetKernelArg(ctx->kernel_vert, 2, sizeof(cl_int), _y);
 if (cle != CL_SUCCESS) {
 av_log(avctx, AV_LOG_ERROR, "Failed to set kernel "
"sizeY argument: %d.\n", cle);
+err = -1;
 goto fail;
 }
 
-- 
2.14.3 (Apple Git-98)

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

Re: [FFmpeg-devel] [PATCH] lavc: Add filter_units bitstream filter

[Eran Kornblau]

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of Mark 
> Thompson
> Sent: Sunday, March 11, 2018 8:38 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavc: Add filter_units bitstream filter
> 
> On 08/03/18 04:01, James Almer wrote:
> > On 3/6/2018 3:49 PM, Mark Thompson wrote:
> >> This can remove units with types in or not in a given set from a stream.
> >> For example, it can be used to remove all non-VCL NAL units from an 
> >> H.264 or
> >> H.265 stream.
> >> ---
> >> On 06/03/18 17:27, Hendrik Leppkes wrote:
> >>> On Tue, Mar 6, 2018 at 3:51 PM, Eran Kornblau  
> >>> wrote:
>  Hi all,
> 
>  The attached patch adds a parameter that enables the user to choose 
>  which AVC/HEVC NAL units to include in the output.
>  The parameter is supplied as a bitmask in order to keep things simple.
> 
>  A short background on why we need it - in our transcoding process, 
>  we partition the video in chunks, the chunks are transcoded in 
>  parallel and packaged in MPEG-TS container. The transcoded TS 
>  chunks are then concatenated and packaged in MP4. These MP4 files are 
>  later repackaged on-the-fly to various protocols (HLS/DASH etc.) using 
>  our JIT packager.
>  For performance reasons (can get into more detail if anyone's 
>  interested...), when packaging the MP4 to DASH/CENC, we configure the 
>  packager to assume that each AVC frame contains exactly one NAL unit.
>  The problem is that the transition through MPEG-TS adds additional 
>  NAL units (NAL AUD before each frame + SPS/PPS before each key frame), 
>  and this assumption fails.
>  Using the attached patch we can pass '-nal_types_mask 0x3e' which will 
>  make ffmpeg output only VCL NALs in the stream.
> 
> >>>
> >>> Having such logic in one single muxer is not something we really 
> >>> like around here. Next time someone needs something similar for 
> >>> another codec, you're stuck re-implementing it.
> >>>
> >>> To achieve the same effect, Mark Thompson quickly wipped up a 
> >>> Bitstream Filter using his CBS framework which achieves the same 
> >>> result. He'll be sending that patch to the mailing list in a while.
> >> The suggested use-case would be '-bsf:v filter_units=pass_types=1-5' for 
> >> H.264 or '-bsf:v filter_units=pass_types=0-31' for H.265.
> >>
> >> (Also note that filters already exist for some individual parts of 
> >> this: h264_metadata can remove AUDs, extract_extradata can remove 
> >> parameter sets.)
> >>
> >> - Mark
> >>
> >>
> >>  libavcodec/Makefile|   1 +
> >>  libavcodec/bitstream_filters.c |   1 +
> >>  libavcodec/filter_units_bsf.c  | 250 
> >> +
> >>  3 files changed, 252 insertions(+)
> >>  create mode 100644 libavcodec/filter_units_bsf.c
> >>
> > 
> > Can you write some minimal documentation with the two above examples?
> 
> Done.
> 
> >> diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 
> >> b496f0d..b99bdce 100644
> >> --- a/libavcodec/Makefile
> >> +++ b/libavcodec/Makefile
> >> @@ -1037,6 +1037,7 @@ OBJS-$(CONFIG_DUMP_EXTRADATA_BSF) += 
> >> dump_extradata_bsf.o
> >>  OBJS-$(CONFIG_DCA_CORE_BSF)   += dca_core_bsf.o
> >>  OBJS-$(CONFIG_EXTRACT_EXTRADATA_BSF)  += extract_extradata_bsf.o\
> >>   h2645_parse.o
> >> +OBJS-$(CONFIG_FILTER_UNITS_BSF)   += filter_units_bsf.o
> >>  OBJS-$(CONFIG_H264_METADATA_BSF)  += h264_metadata_bsf.o
> >>  OBJS-$(CONFIG_H264_MP4TOANNEXB_BSF)   += h264_mp4toannexb_bsf.o
> >>  OBJS-$(CONFIG_H264_REDUNDANT_PPS_BSF) += h264_redundant_pps_bsf.o
> >> diff --git a/libavcodec/bitstream_filters.c 
> >> b/libavcodec/bitstream_filters.c index 338ef82..e1dc198 100644
> >> --- a/libavcodec/bitstream_filters.c
> >> +++ b/libavcodec/bitstream_filters.c
> >> @@ -29,6 +29,7 @@ extern const AVBitStreamFilter ff_chomp_bsf;  
> >> extern const AVBitStreamFilter ff_dump_extradata_bsf;  extern const 
> >> AVBitStreamFilter ff_dca_core_bsf;  extern const AVBitStreamFilter 
> >> ff_extract_extradata_bsf;
> >> +extern const AVBitStreamFilter ff_filter_units_bsf;
> >>  extern const AVBitStreamFilter ff_h264_metadata_bsf;  extern const 
> >> AVBitStreamFilter ff_h264_mp4toannexb_bsf;  extern const 
> >> AVBitStreamFilter ff_h264_redundant_pps_bsf; diff --git 
> >> a/libavcodec/filter_units_bsf.c b/libavcodec/filter_units_bsf.c new 
> >> file mode 100644 index 000..3126f17
> >> --- /dev/null
> >> +++ b/libavcodec/filter_units_bsf.c
> >> @@ -0,0 +1,250 @@
> >> +/*
> >> + * 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 

Re: [FFmpeg-devel] [PATCH] avcodec/avpacket: ensure the packet is writable in av_shrink_packet()

[wm4]

On Sat, 24 Mar 2018 22:39:45 -0300
James Almer  wrote:

> On 3/24/2018 6:46 PM, wm4 wrote:
> > On Sat, 24 Mar 2018 18:11:53 -0300
> > James Almer  wrote:
> >   
> >> Signed-off-by: James Almer 
> >> ---
> >> This is a good time to deprecate this function and introduce a
> >> replacement using the correct av_packet namespace and this time
> >> returning an int.
> >> What would be better
> >>
> >> int av_packet_shrink(AVPacket *pkt, int size);
> >>
> >> Or
> >>  
> >   
> >> int av_packet_resize(AVPacket *pkt, int size);  
> > 
> > Seems better.
> >   
> >>
> >> The latter would be a combination of both the current shrink and grow
> >> functions.
> >>
> >>  libavcodec/avpacket.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> >> index 0693ca6f62..7faa082395 100644
> >> --- a/libavcodec/avpacket.c
> >> +++ b/libavcodec/avpacket.c
> >> @@ -100,9 +100,12 @@ int av_new_packet(AVPacket *pkt, int size)
> >>  
> >>  void av_shrink_packet(AVPacket *pkt, int size)
> >>  {
> >> +int packet_is_writable;
> >>  if (pkt->size <= size)
> >>  return;
> >>  pkt->size = size;
> >> +packet_is_writable = !av_packet_make_writable(pkt);
> >> +av_assert0(packet_is_writable);
> >>  memset(pkt->data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
> >>  }
> >>
> > 
> > LGTM  
> 
> Ok, but i just realized that i can't apply this without first writing an
> internal variant specifically for the encoders that makes sure pkt->data
> != avctx->internal->byte_buffer before trying to do anything, otherwise
> the supposed benefits of that weird internal buffer code in
> ff_alloc_packet2() would be lost.
> 
> Will look at that later.

Didn't look at it again, but this mechanism might be broken anyway
since the new encode API must return refcounted packets.

Maybe the new dynamic sized buffer pool would help.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avcodec/dcaenc: Use aac psychoacoustic model for dcaenc

[Даниил Чередник]

There are several reasons to replace dca psychoacoustic to common model:
 - dca psychoacoustic has some quality problems especially at high
frequency bands
 - unclear implementation
 - aac implementation allows to use tonality measurement for future
improvements
 - a bit faster

AAC psychoacoustic uses float sample format, so first patch changes dcaenc
input format to AV_SAMPLE_FMT_FLTP.
Second patch replaces psychoacoustic model.

-- 
Daniil Cherednik


0001-avcodec-dcaenc-Use-planar-float-frame-format.patch
Description: Binary data


0002-avcodec-dcaenc-Use-aac-psychoacoustic-model-for-DCA-.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [Patch][GSoC] srcnn - a super resolution filter using CNN

[Mina]

Hi,

  This patch is introduced as a qualification task required by Super 
Resolution project for GSoC. It passes patchcheck and make fate, doesn't 
introduce new warnings and gives expected results for all tested images. 
It's implemented by the help of the mentor: Pedro Arthur.


Best regards,
Mina Sami.

>From bf8ac38cee36886f6c317681a16aecfa5dee0752 Mon Sep 17 00:00:00 2001
From: MinaBombo 
Date: Sat, 24 Mar 2018 21:15:08 +0200
Subject: [PATCH] Added srcnn filter

---
 Changelog   |   1 +
 libavfilter/Makefile|   1 +
 libavfilter/allfilters.c|   1 +
 libavfilter/srcnn_weights.h | 290 
 libavfilter/vf_srcnn.c  | 264 
 5 files changed, 557 insertions(+)
 create mode 100644 libavfilter/srcnn_weights.h
 create mode 100644 libavfilter/vf_srcnn.c

diff --git a/Changelog b/Changelog
index c1b9df4..e153f12 100644
--- a/Changelog
+++ b/Changelog
@@ -46,6 +46,7 @@ version :
   They can be found at http://git.videolan.org/?p=ffmpeg/nv-codec-headers.git
 - native SBC encoder and decoder
 - drmeter audio filter
+- Added image SRCNN(Super Resolution Convolutional Neural Network) filter. No training yet.
 
 
 version 3.4:
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index fc16512..b37a956 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -323,6 +323,7 @@ OBJS-$(CONFIG_SMARTBLUR_FILTER)  += vf_smartblur.o
 OBJS-$(CONFIG_SOBEL_FILTER)  += vf_convolution.o
 OBJS-$(CONFIG_SPLIT_FILTER)  += split.o
 OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o
+OBJS-$(CONFIG_SRCNN_FILTER)  += vf_srcnn.o
 OBJS-$(CONFIG_SSIM_FILTER)   += vf_ssim.o framesync.o
 OBJS-$(CONFIG_STEREO3D_FILTER)   += vf_stereo3d.o
 OBJS-$(CONFIG_STREAMSELECT_FILTER)   += f_streamselect.o framesync.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index cc423af..42f409c 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -332,6 +332,7 @@ static void register_all(void)
 REGISTER_FILTER(SOBEL,  sobel,  vf);
 REGISTER_FILTER(SPLIT,  split,  vf);
 REGISTER_FILTER(SPP,spp,vf);
+REGISTER_FILTER(SRCNN,  srcnn,  vf);
 REGISTER_FILTER(SSIM,   ssim,   vf);
 REGISTER_FILTER(STEREO3D,   stereo3d,   vf);
 REGISTER_FILTER(STREAMSELECT,   streamselect,   vf);
diff --git a/libavfilter/srcnn_weights.h b/libavfilter/srcnn_weights.h
new file mode 100644
index 000..d6ac79c
--- /dev/null
+++ b/libavfilter/srcnn_weights.h
@@ -0,0 +1,290 @@
+/*
+ * Copyright (c) 2018 Mina Sami
+ *
+ * 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
+ */
+
+/**
+ * @file
+ * Super resolution filter pretrained weights
+ *
+ * @see https://arxiv.org/abs/1501.00092
+ */
+
+#ifndef AVFILTER_SRCNN_WEIGHTS_H
+#define AVFILTER_SRCNN_WEIGHTS_H
+
+const int ff_cnn_num_filters[] = {
+1, 64, 32, 1
+};
+
+const int ff_cnn_size_filters[] = {
+1, 9, 1, 5
+};
+
+static const float weights1_64x81[] = {
+-0.088663,0.05541,0.037197,-0.11961,-0.12342,0.29963,-0.091182,-0.00013614,-0.049024,0.038421,-0.077268,0.027273,0.45762,0.023582,-0.20364,-0.077384,0.072884,0.066799,-0.073151,0.26653,0.078364,-0.28985,-0.50691,0.044665,-0.062469,-0.024425,-0.029073,-0.16885,0.20686,-0.25367,-0.385,-0.12602,0.72579,0.095884,-0.044873,-0.011252,-0.029559,0.22117,-0.38453,-0.05351,0.10956,0.37257,-0.28968,-0.10884,0.19629,0.097468,-0.0056471,-0.13036,0.62543,0.19104,-0.011692,-0.27068,-0.030754,0.029876,0.016809,-0.18318,-0.16016,0.35983,-0.1889,-0.22343,-0.2,0.32211,-0.18514,-0.06647,0.18006,-0.03566,-0.25352,-0.1837,0.28395,0.0030237,0.24073,-0.14355,0.047419,-0.032212,-0.088013,0.29592,0.041087,-0.10354,-0.11681,-0.010267,0.023704,
+

Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: initialize saveptrs

[Timo Rothenpieler]

Am 21.03.2018 um 20:37 schrieb Timo Rothenpieler:

Am 21.03.2018 um 20:33 schrieb Timo Rothenpieler:
av_strtok calls strspn on a non-NULL *saveptr, so not NULL 
initializing it is an issue.


Fixes CID #1428568
---
  libavformat/hlsenc.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index b7c6fbde6a..fa17776efe 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -1873,7 +1873,8 @@ static int 
parse_cc_stream_mapstring(AVFormatContext *s)

  {
  HLSContext *hls = s->priv_data;
  int nb_ccstreams;
-    char *p, *q, *saveptr1, *saveptr2, *ccstr, *keyval;
+    char *p, *q, *ccstr, *keyval;
+    char *saveptr1 = NULL, *saveptr2 = NULL;
  const char *val;
  ClosedCaptionsStream *ccs;


Just realized, the more correct approach is probably to check the 
av_strdup below this for ENOMEM. Not sure about the exact semantics 
there, initializing these still seems like a good safety measure.




ping



smime.p7s
Description: S/MIME Cryptographic Signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] avcodec/arm/hevcdsp_sao : add NEON optimization for sao

[Shengbin Meng]

> On 22 Mar 2018, at 20:51, Yingming Fan  wrote:
> 
> From: Meng Wang 
> 
> Signed-off-by: Meng Wang 
> ---
> This v2 patch remove unused codes 'stride_dst /= sizeof(uint8_t);' compared 
> to v1. V1 have this codes because we referred to hevc dsp template codes.
> 
> As FFmpeg hevc decoder have no SAO neon optimization, we add sao_band and 
> sao_edge neon codes in this patch.
> I have already submit a patch called 'checkasm/hevc_sao : add hevc_sao for 
> checkasm' several days ago.
> Results below was printed by hevc_sao checkasm on an armv7 device Nexus 5. 
> From the results we can see: hevc_sao_band speed up ~2x, hevc_sao_edge speed 
> up ~4x. 
> Also test FATE under armv7 device and MacOS.
> 
> hevc_sao_band_8x8_8_c: 804.9
> hevc_sao_band_8x8_8_neon: 452.4
> hevc_sao_band_16x16_8_c: 2638.1
> hevc_sao_band_16x16_8_neon: 1169.9
> hevc_sao_band_32x32_8_c: 9259.9
> hevc_sao_band_32x32_8_neon: 3956.1
> hevc_sao_band_48x48_8_c: 20344.6
> hevc_sao_band_48x48_8_neon: 8649.6
> hevc_sao_band_64x64_8_c: 35684.6
> hevc_sao_band_64x64_8_neon: 15213.1
> hevc_sao_edge_8x8_8_c: 1761.6
> hevc_sao_edge_8x8_8_neon: 414.6
> hevc_sao_edge_16x16_8_c: 6844.4
> hevc_sao_edge_16x16_8_neon: 1589.9
> hevc_sao_edge_32x32_8_c: 27156.4
> hevc_sao_edge_32x32_8_neon: 6116.6
> hevc_sao_edge_48x48_8_c: 60004.6
> hevc_sao_edge_48x48_8_neon: 13686.4
> hevc_sao_edge_64x64_8_c: 106708.1
> hevc_sao_edge_64x64_8_neon: 24240.1
> 
> libavcodec/arm/Makefile|   3 +-
> libavcodec/arm/hevcdsp_init_neon.c |  59 
> libavcodec/arm/hevcdsp_sao_neon.S  | 181 +
> 3 files changed, 242 insertions(+), 1 deletion(-)
> create mode 100644 libavcodec/arm/hevcdsp_sao_neon.S
> 
> diff --git a/libavcodec/arm/Makefile b/libavcodec/arm/Makefile
> index 1eeac5449e..9c164f82ae 100644
> --- a/libavcodec/arm/Makefile
> +++ b/libavcodec/arm/Makefile
> @@ -136,7 +136,8 @@ NEON-OBJS-$(CONFIG_DCA_DECODER)+= 
> arm/synth_filter_neon.o
> NEON-OBJS-$(CONFIG_HEVC_DECODER)   += arm/hevcdsp_init_neon.o   \
>   arm/hevcdsp_deblock_neon.o\
>   arm/hevcdsp_idct_neon.o   \
> -  arm/hevcdsp_qpel_neon.o
> +  arm/hevcdsp_qpel_neon.o   \
> +  arm/hevcdsp_sao_neon.o
> NEON-OBJS-$(CONFIG_RV30_DECODER)   += arm/rv34dsp_neon.o
> NEON-OBJS-$(CONFIG_RV40_DECODER)   += arm/rv34dsp_neon.o\
>   arm/rv40dsp_neon.o
> diff --git a/libavcodec/arm/hevcdsp_init_neon.c 
> b/libavcodec/arm/hevcdsp_init_neon.c
> index a4628d2a93..af68e24f93 100644
> --- a/libavcodec/arm/hevcdsp_init_neon.c
> +++ b/libavcodec/arm/hevcdsp_init_neon.c
> @@ -21,8 +21,16 @@
> #include "libavutil/attributes.h"
> #include "libavutil/arm/cpu.h"
> #include "libavcodec/hevcdsp.h"
> +#include "libavcodec/avcodec.h"
> #include "hevcdsp_arm.h"
> 
> +void ff_hevc_sao_band_filter_neon_8_wrapper(uint8_t *_dst, uint8_t *_src,
> +  ptrdiff_t stride_dst, ptrdiff_t stride_src,
> +  int16_t *sao_offset_val, int 
> sao_left_class,
> +  int width, int height);
> +void ff_hevc_sao_edge_filter_neon_8_wrapper(uint8_t *_dst, uint8_t *_src, 
> ptrdiff_t stride_dst, int16_t *sao_offset_val,
> +  int eo, int width, int height);
> +
> void ff_hevc_v_loop_filter_luma_neon(uint8_t *_pix, ptrdiff_t _stride, int 
> _beta, int *_tc, uint8_t *_no_p, uint8_t *_no_q);
> void ff_hevc_h_loop_filter_luma_neon(uint8_t *_pix, ptrdiff_t _stride, int 
> _beta, int *_tc, uint8_t *_no_p, uint8_t *_no_q);
> void ff_hevc_v_loop_filter_chroma_neon(uint8_t *_pix, ptrdiff_t _stride, int 
> *_tc, uint8_t *_no_p, uint8_t *_no_q);
> @@ -142,6 +150,47 @@ QPEL_FUNC_UW(ff_hevc_put_qpel_uw_h3v2_neon_8);
> QPEL_FUNC_UW(ff_hevc_put_qpel_uw_h3v3_neon_8);
> #undef QPEL_FUNC_UW
> 
> +void ff_hevc_sao_band_filter_neon_8(uint8_t *dst, uint8_t *src, ptrdiff_t 
> stride_dst, ptrdiff_t stride_src, int width, int height, int16_t 
> *offset_table);
> +
> +void ff_hevc_sao_band_filter_neon_8_wrapper(uint8_t *_dst, uint8_t *_src,
> +  ptrdiff_t stride_dst, ptrdiff_t stride_src,
> +  int16_t *sao_offset_val, int 
> sao_left_class,
> +  int width, int height) {
> +uint8_t *dst = (uint8_t *)_dst;
> +uint8_t *src = (uint8_t *)_src;
This conversion is also not needed since we are only handling 8-bit pixels here.

> +int16_t offset_table[32] = {0};
> +int k;
> +
> +for (k = 0; k < 4; k++) {
> +offset_table[(k + sao_left_class) & 31] = sao_offset_val[k + 1];
> +}
> +
> +ff_hevc_sao_band_filter_neon_8(dst, src,