[FFmpeg-devel] [PATCH 1/3] lavc/vaapi_encode: Change the encode common code to support mutil-refs.

[Jun Zhao]

This is a early version for RFC, and will refactoring if have some comments.
Test with i965 mainline master branch, test command like this:

  ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128
-hwaccel_output_format vaapi -i input.mp4 -c:v h264_vaapi -refs 4
output_refs4.mp4

  ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128
-hwaccel_output_format vaapi -i input.mp4 -c:v hevc_vaapi -refs 4
output_refs4.mp4



From 703e4425942eb51cfddda578bd21d1662cc50be7 Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Tue, 7 Nov 2017 14:30:57 +0800
Subject: [PATCH 1/3] lavc/vaapi_encode: Change the encode common code to
 support mutil-refs.

Move vaapi_encode_alloc/free/step/output to vaapi_encode.h and change
the max reference frames number.

Signed-off-by: Jun Zhao 
Signed-off-by: Wang, Yi A 
---
 libavcodec/vaapi_encode.c | 21 ++---
 libavcodec/vaapi_encode.h | 19 ++-
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 590f4be4ed..74bb02dc29 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -108,6 +108,7 @@ static int vaapi_encode_wait(AVCodecContext *avctx,
 {
 VAAPIEncodeContext *ctx = avctx->priv_data;
 VAStatus vas;
+int i;
 
 av_assert0(pic->encode_issued);
 
@@ -131,6 +132,9 @@ static int vaapi_encode_wait(AVCodecContext *avctx,
 av_frame_free(&pic->input_image);
 
 pic->encode_complete = 1;
+for (i = 0; i < pic->nb_refs; i++) {
+pic->refs[i]->ref_count --;
+}
 return 0;
 }
 
@@ -448,8 +452,8 @@ fail_at_end:
 return err;
 }
 
-static int vaapi_encode_output(AVCodecContext *avctx,
-   VAAPIEncodePicture *pic, AVPacket *pkt)
+int vaapi_encode_output(AVCodecContext *avctx,
+   VAAPIEncodePicture *pic, AVPacket *pkt)
 {
 VAAPIEncodeContext *ctx = avctx->priv_data;
 VACodedBufferSegment *buf_list, *buf;
@@ -526,7 +530,7 @@ static int vaapi_encode_discard(AVCodecContext *avctx,
 return 0;
 }
 
-static VAAPIEncodePicture *vaapi_encode_alloc(void)
+VAAPIEncodePicture *vaapi_encode_alloc(void)
 {
 VAAPIEncodePicture *pic;
 
@@ -541,8 +545,8 @@ static VAAPIEncodePicture *vaapi_encode_alloc(void)
 return pic;
 }
 
-static int vaapi_encode_free(AVCodecContext *avctx,
- VAAPIEncodePicture *pic)
+int vaapi_encode_free(AVCodecContext *avctx,
+  VAAPIEncodePicture *pic)
 {
 int i;
 
@@ -573,8 +577,8 @@ static int vaapi_encode_free(AVCodecContext *avctx,
 return 0;
 }
 
-static int vaapi_encode_step(AVCodecContext *avctx,
- VAAPIEncodePicture *target)
+int vaapi_encode_step(AVCodecContext *avctx,
+  VAAPIEncodePicture *target)
 {
 VAAPIEncodeContext *ctx = avctx->priv_data;
 VAAPIEncodePicture *pic;
@@ -1097,6 +1101,8 @@ static av_cold int 
vaapi_encode_config_attributes(AVCodecContext *avctx)
 err = AVERROR(EINVAL);
 goto fail;
 }
+   ctx->max_ref_l0 = ref_l0;
+   ctx->max_ref_l1 = ref_l1;
 }
 break;
 case VAConfigAttribEncPackedHeaders:
@@ -1340,6 +1346,7 @@ static av_cold int 
vaapi_encode_create_recon_frames(AVCodecContext *avctx)
 // At most three IDR/I/P frames and two runs of B frames can be in
 // flight at any one time.
 ctx->recon_frames->initial_pool_size = 3 + 2 * avctx->max_b_frames;
+ctx->recon_frames->initial_pool_size += ctx->max_ref_l0 + ctx->max_ref_l1;
 
 err = av_hwframe_ctx_init(ctx->recon_frames_ref);
 if (err < 0) {
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index bcb9d57371..53950cc0a1 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -34,7 +34,7 @@ struct VAAPIEncodePicture;
 enum {
 MAX_CONFIG_ATTRIBUTES  = 4,
 MAX_GLOBAL_PARAMS  = 4,
-MAX_PICTURE_REFERENCES = 2,
+MAX_PICTURE_REFERENCES = 12,
 MAX_REORDER_DELAY  = 16,
 MAX_PARAM_BUFFER_SIZE  = 1024,
 };
@@ -79,9 +79,12 @@ typedef struct VAAPIEncodePicture {
 void   *priv_data;
 void   *codec_picture_params;
 
+int  ref_count;
+
 int  nb_refs;
 struct VAAPIEncodePicture *refs[MAX_PICTURE_REFERENCES];
 
+
 int  nb_slices;
 VAAPIEncodeSlice *slices;
 } VAAPIEncodePicture;
@@ -206,6 +209,10 @@ typedef struct VAAPIEncodeContext {
 int p_counter;
 int end_of_stream;
 
+// max reference frame number
+int max_ref_l0;
+int max_ref_l1;
+
 // Codec-local options are allocated to follow this structure in
 // memory (in the AVCodec definition, set priv_data_size to
 // sizeof(VAAPIEncodeContext) + sizeof(VAAPIEncodeFooOptions)).
@@ -279,5 +286,15 @@ int ff_vaapi_encode2(AVCodecContext *avctx, AVPacket *pkt,
 
 int ff_vaapi_encode_init(AVCodecContext *avctx);
 int ff_vaapi_encode_close(AVCodecContext *avctx);
+VAAPIEnco

[FFmpeg-devel] [PATCH 2/3] lavc/vaapi_encode_h264: enable mutil-reference frames.

[Jun Zhao]

From ed976e6cde34521ffd59269100d49526e68a301e Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Tue, 7 Nov 2017 14:32:42 +0800
Subject: [PATCH 2/3] lavc/vaapi_encode_h264: enable mutil-reference frames.

Add mutil-reference frames support and respect "refs" option
in h264_vaapi encoder.

Signed-off-by: Jun Zhao 
Signed-off-by: Wang, Yi A 
---
 libavcodec/vaapi_encode_h264.c | 435 +++--
 1 file changed, 421 insertions(+), 14 deletions(-)

diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
index 1d43e934ef..479783bdf4 100644
--- a/libavcodec/vaapi_encode_h264.c
+++ b/libavcodec/vaapi_encode_h264.c
@@ -40,6 +40,8 @@ enum {
 SEI_RECOVERY_POINT = 0x04,
 };
 
+static const char *picture_type_name[] = { "IDR", "I", "P", "B" };
+
 // Random (version 4) ISO 11578 UUID.
 static const uint8_t vaapi_encode_h264_sei_identifier_uuid[16] = {
 0x59, 0x94, 0x8b, 0x28, 0x11, 0xec, 0x45, 0xaf,
@@ -82,6 +84,11 @@ typedef struct VAAPIEncodeH264Context {
 CodedBitstreamFragment current_access_unit;
 int aud_needed;
 int sei_needed;
+
+// reference frames param
+struct VAAPIEncodePicture *references[MAX_PICTURE_REFERENCES];
+int ref_nr;
+int max_ref_nr;
 } VAAPIEncodeH264Context;
 
 typedef struct VAAPIEncodeH264Options {
@@ -302,9 +309,7 @@ static int 
vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
 sps->log2_max_pic_order_cnt_lsb_minus4 =
 av_clip(av_log2(ctx->b_per_p + 1) - 2, 0, 12);
 
-sps->max_num_ref_frames =
-(avctx->profile & FF_PROFILE_H264_INTRA) ? 0 :
-1 + (ctx->b_per_p > 0);
+sps->max_num_ref_frames = (avctx->profile & FF_PROFILE_H264_INTRA) ? 0 : 
priv->max_ref_nr;
 
 sps->pic_width_in_mbs_minus1= priv->mb_width  - 1;
 sps->pic_height_in_map_units_minus1 = priv->mb_height - 1;
@@ -737,19 +742,23 @@ static int 
vaapi_encode_h264_init_slice_params(AVCodecContext *avctx,
 vslice->RefPicList1[i].flags  = VA_PICTURE_H264_INVALID;
 }
 
-av_assert0(pic->nb_refs <= 2);
-if (pic->nb_refs >= 1) {
-// Backward reference for P- or B-frame.
-av_assert0(pic->type == PICTURE_TYPE_P ||
-   pic->type == PICTURE_TYPE_B);
-vslice->RefPicList0[0] = vpic->ReferenceFrames[0];
+sh->num_ref_idx_active_override_flag = 1;
+
+if (pic->type == PICTURE_TYPE_P) {
+for (i = 0; i < pic->nb_refs; i++)
+vslice->RefPicList0[i] = vpic->ReferenceFrames[pic->nb_refs - 1 - 
i];
+   sh->num_ref_idx_l0_active_minus1 = pic->nb_refs - 1;
 }
-if (pic->nb_refs >= 2) {
-// Forward reference for B-frame.
-av_assert0(pic->type == PICTURE_TYPE_B);
-vslice->RefPicList1[0] = vpic->ReferenceFrames[1];
+
+if (pic->type == PICTURE_TYPE_B) {
+for (i = 0; i < pic->nb_refs - 1; i++)
+vslice->RefPicList0[i] = vpic->ReferenceFrames[pic->nb_refs - 2 - 
i];
+vslice->RefPicList1[0] = vpic->ReferenceFrames[pic->nb_refs - 1];
+sh->num_ref_idx_l0_active_minus1 = pic->nb_refs - 2;
 }
 
+vslice->num_ref_idx_active_override_flag = 
sh->num_ref_idx_active_override_flag;
+vslice->num_ref_idx_l0_active_minus1 = sh->num_ref_idx_l0_active_minus1;
 vslice->slice_qp_delta = sh->slice_qp_delta;
 
 return 0;
@@ -834,6 +843,34 @@ static av_cold int 
vaapi_encode_h264_configure(AVCodecContext *avctx)
 }
 }
 
+priv->max_ref_nr = avctx->refs;
+
+if (priv->max_ref_nr > ctx->max_ref_l0 + ctx->max_ref_l1) {
+av_log(avctx, AV_LOG_WARNING, "Warning: " \
+   "reference frame number exceeds %d" \
+   "correct to %d\n", \
+   ctx->max_ref_l0 + ctx->max_ref_l1,
+   ctx->max_ref_l0 + ctx->max_ref_l1);
+priv->max_ref_nr = ctx->max_ref_l0 + ctx->max_ref_l1;
+}
+if (avctx->max_b_frames && priv->max_ref_nr < 2) {
+av_log(avctx, AV_LOG_WARNING, "Warning: " \
+   "reference frame number is 1 but b frame encoding is setted," \
+   "correct to 2\n");
+priv->max_ref_nr = 2;
+}
+if (!avctx->max_b_frames && priv->max_ref_nr > ctx->max_ref_l0) {
+av_log(avctx, AV_LOG_WARNING, "Warning: " \
+   "no b frame, but ref_nr > max_ref_l0" \
+   "correct to %d\n", ctx->max_ref_l0);
+priv->max_ref_nr = ctx->max_ref_l0;
+}
+if (priv->max_ref_nr < 1 && avctx->gop_size) {
+av_log(avctx, AV_LOG_WARNING, "Warning: " \
+   "reference frame number is %d but gop_size > 0," \
+   "correct to 1\n", priv->max_ref_nr);
+priv->max_ref_nr = 1;
+}
 return 0;
 }
 
@@ -958,6 +995,376 @@ static av_cold int vaapi_encode_h264_close(AVCodecContext 
*avctx)
 return ff_vaapi_encode_close(avctx);
 }
 
+static void vaapi_encode_h264_add_reference (AVCodecContext *avctx,
+ VAAPIEncodePicture *pic)
+{
+VAAPIEncodeContext *ctx

[FFmpeg-devel] [PATCH 3/3] lavc/vaapi_encode_h265: enable multi-reference frames.

[Jun Zhao]

From f7e5e56c5a64d0414a7e5fceb22cf9d962e09dc1 Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Tue, 7 Nov 2017 14:33:39 +0800
Subject: [PATCH 3/3] lavc/vaapi_encode_h265: enable multi-reference frames.

Enable the multi-reference frames and respect "refs" option
in hevc_vaapi encoder.

Signed-off-by: Jun Zhao 
Signed-off-by: Wang, Yi A 
---
 libavcodec/vaapi_encode_h265.c | 499 -
 1 file changed, 449 insertions(+), 50 deletions(-)

diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
index 3ae92a7a09..85350380ca 100644
--- a/libavcodec/vaapi_encode_h265.c
+++ b/libavcodec/vaapi_encode_h265.c
@@ -34,6 +34,8 @@
 #include "vaapi_encode.h"
 
 
+static const char *picture_type_name[] = { "IDR", "I", "P", "B" };
+
 typedef struct VAAPIEncodeH265Context {
 unsigned int ctu_width;
 unsigned int ctu_height;
@@ -58,6 +60,11 @@ typedef struct VAAPIEncodeH265Context {
 CodedBitstreamContext *cbc;
 CodedBitstreamFragment current_access_unit;
 int aud_needed;
+
+// reference frames param
+struct VAAPIEncodePicture *references[MAX_PICTURE_REFERENCES];
+int ref_nr;
+int max_ref_nr;
 } VAAPIEncodeH265Context;
 
 typedef struct VAAPIEncodeH265Options {
@@ -224,7 +231,7 @@ static int 
vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
 vps->profile_tier_level.general_profile_compatibility_flag[avctx->profile 
& 31] = 1;
 
 vps->vps_sub_layer_ordering_info_present_flag = 0;
-vps->vps_max_dec_pic_buffering_minus1[0]  = (ctx->b_per_p > 0) + 1;
+vps->vps_max_dec_pic_buffering_minus1[0]  = priv->max_ref_nr;
 vps->vps_max_num_reorder_pics[0]  = (ctx->b_per_p > 0);
 vps->vps_max_latency_increase_plus1[0]= 0;
 
@@ -677,59 +684,44 @@ static int 
vaapi_encode_h265_init_slice_params(AVCodecContext *avctx,
 
 if (pic->type != PICTURE_TYPE_IDR) {
 H265RawSTRefPicSet *rps;
-VAAPIEncodePicture *st;
-int used;
 
 sh->short_term_ref_pic_set_sps_flag = 0;
 
 rps = &sh->short_term_ref_pic_set;
 memset(rps, 0, sizeof(*rps));
 
-for (st = ctx->pic_start; st; st = st->next) {
-if (st->encode_order >= pic->encode_order) {
-// Not yet in DPB.
-continue;
+if (pic->type != PICTURE_TYPE_B) {
+rps->num_negative_pics = pic->nb_refs;
+for (i = 0 ; i < rps->num_negative_pics; i++) {
+rps->used_by_curr_pic_s0_flag[i] = 1;
+if (i == 0) {
+rps->delta_poc_s0_minus1[i] =  priv->pic_order_cnt -
+  vpic->reference_frames[pic->nb_refs - 1 
- i].pic_order_cnt - 1;
+} else {
+rps->delta_poc_s0_minus1[i] = 
vpic->reference_frames[pic->nb_refs - i].pic_order_cnt -
+vpic->reference_frames[pic->nb_refs - 
1 - i].pic_order_cnt - 1;
+}
 }
-used = 0;
-for (i = 0; i < pic->nb_refs; i++) {
-if (pic->refs[i] == st)
-used = 1;
+} else {
+rps->num_positive_pics = 1;
+for (i = 0 ; i < rps->num_positive_pics; i++) {
+rps->used_by_curr_pic_s1_flag[i] = 1;
+rps->delta_poc_s1_minus1[i] =
+vpic->reference_frames[pic->nb_refs- 1 - i].pic_order_cnt - 
priv->pic_order_cnt - 1;
 }
-if (!used) {
-// Usually each picture always uses all of the others in the
-// DPB as references.  The one case we have to treat here is
-// a non-IDR IRAP picture, which may need to hold unused
-// references across itself to be used for the decoding of
-// following RASL pictures.  This looks for such an RASL
-// picture, and keeps the reference if there is one.
-VAAPIEncodePicture *rp;
-for (rp = ctx->pic_start; rp; rp = rp->next) {
-if (rp->encode_order < pic->encode_order)
-continue;
-if (rp->type != PICTURE_TYPE_B)
-continue;
-if (rp->refs[0] == st && rp->refs[1] == pic)
-break;
+rps->num_negative_pics = pic->nb_refs - 1 ;
+for (i = 0 ; i < rps->num_negative_pics; i++) {
+rps->used_by_curr_pic_s0_flag[i] = 1;
+if (i == 0) {
+rps->delta_poc_s0_minus1[i] =
+priv->pic_order_cnt - 
vpic->reference_frames[pic->nb_refs - 2 - i].pic_order_cnt - 1;
+} else {
+rps->delta_poc_s0_minus1[i] =
+ vpic->reference_frames[pic->nb_refs - 1 - 
i].pic_order_cnt -
+ vpic->reference_frames[pic->nb_refs - 2 - 
i].pic_order_cnt - 1;
 

[FFmpeg-devel] [PATCH]lavfi/scale2ref: Set output frame rate to main input frame rate

[Carl Eugen Hoyos]

Hi!

I believe attached patch fixes ticket #6817.

Please comment, Carl Eugen
From deee64102ef898b5f99974cf11bb05ec09963e90 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Wed, 8 Nov 2017 09:15:29 +0100
Subject: [PATCH] lavfi/scale2ref: Set output frame rate to main input frame
 rate.

Fixes ticket #6817.
---
 libavfilter/vf_scale.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 3329c12..2fd9c90 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -362,6 +362,7 @@ static int config_props_ref(AVFilterLink *outlink)
 outlink->h = inlink->h;
 outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
 outlink->time_base = inlink->time_base;
+outlink->frame_rate = inlink->frame_rate;
 
 return 0;
 }
-- 
1.7.10.4

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

Re: [FFmpeg-devel] [PATCH]lavc/dnxhddata: Do not print frame rates with supported profiles

[Carl Eugen Hoyos]

2017-11-04 1:19 GMT+01:00 Carl Eugen Hoyos :
> 2017-10-31 3:05 GMT+01:00 Carl Eugen Hoyos :
>> Hi!
>>
>> Attached patch is meant to fix ticket #4815: Nobody can maintain the
>> list of frame-rates, the current output primarily leads to confusion
>> instead of helping users.
>
> I will commit this patch if there are no objections.

Patch applied.

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

Re: [FFmpeg-devel] Must I subscribe to ffmpeg-devel in order to submit a patch?

[Jim DeLaHunt]

Lou, Thilo, Nicolas:

Thank you for the replies.

I'm subscribed to ffmpeg-devel now. I'll be back with documentation 
patches (one FAQ, one about submitting patches and subscribing to the 
ffmpeg-devel list) once I have a chance to clone the repo and see how 
building the docs works.


Your replies have done a good job of sketching out what an improvement 
to the docs on submitting patches and subscribing to the ffmpeg-devel 
list might say.


On 2017-11-05 12:37, Nicolas George wrote:

The comments on your patch will be posted to the mailing-list. You need
to read them.


On 2017-11-05 15:18, Nicolas George wrote:

Thanks for pointing it out, Jim. We'd appreciate it and welcome you to propose 
a patch for updating the corresponding docs.

Fact is, that it is in most cases of the author's best interest do participate 
in a following review and therefore subscription to the mailing list is highly 
recommended for that purpose like Nicolas pointed out.

However, if it really is in the authors intention, it is better to post a 
possible patch without caring about its future instead of not to post that 
patch at all. I think it should already be documented that it is appreciated 
but not necessary to use a real name for contributing.


On 2017-11-06 15:13, Lou Logan wrote:

Hi,

On Sat, 4 Nov 2017 21:52:28 -0700
Jim DeLaHunt  wrote:


Hello, ffmpeg developers:

I'm gearing up to submit a patch, adding an FAQ to the documentation.
According to ,
"Patches should be posted to the ffmpeg-devel
 mailing list."
But neither that section, nor the ffmpeg-devel list information at
 is clear about
whether one must be subscribed to the list in order to post to it.

No, you do not need to be subscribed to any mailing list to send a
message or a patch.

However:

* You will not directly receive replies unless you ask to be CCd (and
   replying users will occasionally forget to do so).

* Your messages will remain in the moderation queue until a mailing
   list admin manually approves it which usually occurs once or twice a
   day but sometimes longer.

Although it is not necessary to do so it is recommended to subscribe
when submitting patches for these reasons. The volume of messages may be
considered to be high, but filtering the messages by List-Id is
effective in filtering them to a dedicated location. You can always
unsubscribe or disable receiving messages once your patch has been
accepted.

If you still do not want to subscribe you can always view messages and
reply via the archives:



Speaking of which, I CCd you but others who have replied may not have
so see the archives for their replies:


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


--
--Jim DeLaHunt, j...@jdlh.com http://blog.jdlh.com/ (http://jdlh.com/)
  multilingual websites consultant

  355-1027 Davie St, Vancouver BC V6E 4L2, Canada
 Canada mobile +1-604-376-8953

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

[FFmpeg-devel] [PATCH 1/1] avformat/dashenc: Added configuration to override HTTP User-Agent

[Karthick J]

---
 doc/muxers.texi   |  2 ++
 libavformat/dashenc.c | 22 +++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 91bbe67..412fede 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -247,6 +247,8 @@ DASH-templated name to used for the initialization segment. 
Default is "init-str
 DASH-templated name to used for the media segments. Default is 
"chunk-stream$RepresentationID$-$Number%05d$.m4s"
 @item -utc_timing_url @var{utc_url}
 URL of the page that will return the UTC timestamp in ISO format. Example: 
"https://time.akamai.com/?iso";
+@item -http_user_agent @var{user_agent}
+Override User-Agent field in HTTP header. Applicable only for HTTP output.
 @item -adaptation_sets @var{adaptation_sets}
 Assign streams to AdaptationSets. Syntax is "id=x,streams=a,b,c 
id=y,streams=d,e" with x and y being the IDs
 of the adaptation sets and a,b,c,d and e are the indices of the mapped streams.
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 7813f44..a68f7fb 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -100,6 +100,7 @@ typedef struct DASHContext {
 const char *init_seg_name;
 const char *media_seg_name;
 const char *utc_timing_url;
+const char *user_agent;
 } DASHContext;
 
 static struct codec_string {
@@ -210,6 +211,12 @@ static int flush_dynbuf(OutputStream *os, int 
*range_length)
 return avio_open_dyn_buf(&os->ctx->pb);
 }
 
+static void set_http_options(AVDictionary **options, DASHContext *c)
+{
+if (c->user_agent)
+av_dict_set(options, "user_agent", c->user_agent, 0);
+}
+
 static int flush_init_segment(AVFormatContext *s, OutputStream *os)
 {
 DASHContext *c = s->priv_data;
@@ -575,16 +582,19 @@ static int write_manifest(AVFormatContext *s, int final)
 int use_rename = proto && !strcmp(proto, "file");
 static unsigned int warned_non_file = 0;
 AVDictionaryEntry *title = av_dict_get(s->metadata, "title", NULL, 0);
+AVDictionary *opts = NULL;
 
 if (!use_rename && !warned_non_file++)
 av_log(s, AV_LOG_ERROR, "Cannot use rename on non file protocol, this 
may lead to races and temporary partial files\n");
 
 snprintf(temp_filename, sizeof(temp_filename), use_rename ? "%s.tmp" : 
"%s", s->filename);
-ret = s->io_open(s, &out, temp_filename, AVIO_FLAG_WRITE, NULL);
+set_http_options(&opts, c);
+ret = s->io_open(s, &out, temp_filename, AVIO_FLAG_WRITE, &opts);
 if (ret < 0) {
 av_log(s, AV_LOG_ERROR, "Unable to open %s for writing\n", 
temp_filename);
 return ret;
 }
+av_dict_free(&opts);
 avio_printf(out, "\n");
 avio_printf(out, "http://www.w3.org/2001/XMLSchema-instance\"\n";
 "\txmlns=\"urn:mpeg:dash:schema:mpd:2011\"\n"
@@ -768,9 +778,11 @@ static int dash_init(AVFormatContext *s)
 ff_dash_fill_tmpl_params(os->initfile, sizeof(os->initfile), 
c->init_seg_name, i, 0, os->bit_rate, 0);
 }
 snprintf(filename, sizeof(filename), "%s%s", c->dirname, os->initfile);
-ret = s->io_open(s, &os->out, filename, AVIO_FLAG_WRITE, NULL);
+set_http_options(&opts, c);
+ret = s->io_open(s, &os->out, filename, AVIO_FLAG_WRITE, &opts);
 if (ret < 0)
 return ret;
+av_dict_free(&opts);
 os->init_start_pos = 0;
 
 if (!strcmp(os->format_name, "mp4")) {
@@ -974,12 +986,15 @@ static int dash_flush(AVFormatContext *s, int final, int 
stream)
 }
 
 if (!c->single_file) {
+AVDictionary *opts = NULL;
 ff_dash_fill_tmpl_params(filename, sizeof(filename), 
c->media_seg_name, i, os->segment_index, os->bit_rate, os->start_pts);
 snprintf(full_path, sizeof(full_path), "%s%s", c->dirname, 
filename);
 snprintf(temp_path, sizeof(temp_path), use_rename ? "%s.tmp" : 
"%s", full_path);
-ret = s->io_open(s, &os->out, temp_path, AVIO_FLAG_WRITE, NULL);
+set_http_options(&opts, c);
+ret = s->io_open(s, &os->out, temp_path, AVIO_FLAG_WRITE, &opts);
 if (ret < 0)
 break;
+av_dict_free(&opts);
 if (!strcmp(os->format_name, "mp4"))
 write_styp(os->ctx->pb);
 } else {
@@ -1188,6 +1203,7 @@ static const AVOption options[] = {
 { "init_seg_name", "DASH-templated name to used for the initialization 
segment", OFFSET(init_seg_name), AV_OPT_TYPE_STRING, {.str = 
"init-stream$RepresentationID$.m4s"}, 0, 0, E },
 { "media_seg_name", "DASH-templated name to used for the media segments", 
OFFSET(media_seg_name), AV_OPT_TYPE_STRING, {.str = 
"chunk-stream$RepresentationID$-$Number%05d$.m4s"}, 0, 0, E },
 { "utc_timing_url", "URL of the page that will return the UTC timestamp in 
ISO format", OFFSET(utc_timing_url), AV_OPT_TYPE_STRING, { 0 }, 0, 0, E },
+{ "http_user_agent", "override User-Agent field in HTTP header", 
OFFSET(user_agent), A

Re: [FFmpeg-devel] [PATCH]lavfi/scale2ref: Set output frame rate to main input frame rate

[Paul B Mahol]

On 11/8/17, Carl Eugen Hoyos  wrote:
> Hi!
>
> I believe attached patch fixes ticket #6817.
>
> Please comment, Carl Eugen
>

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

Re: [FFmpeg-devel] [PATCH 1/2] aptx: implement the aptX bluetooth codec

[Paul B Mahol]

On 11/7/17, Aurelien Jacobs  wrote:
> The encoder was reverse engineered from binary library and from
> EP0398973B1 patent (long expired).
> The decoder was simply deduced from the encoder.
> ---
>  doc/general.texi|   2 +
>  libavcodec/Makefile |   2 +
>  libavcodec/allcodecs.c  |   1 +
>  libavcodec/aptx.c   | 826
> 
>  libavcodec/avcodec.h|   1 +
>  libavcodec/codec_desc.c |   7 +
>  6 files changed, 839 insertions(+)
>  create mode 100644 libavcodec/aptx.c
>

[...]

> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index c4134424f0..36a99f4162 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -614,6 +614,7 @@ enum AVCodecID {
>  AV_CODEC_ID_PAF_AUDIO,
>  AV_CODEC_ID_ON2AVC,
>  AV_CODEC_ID_DSS_SP,
> +AV_CODEC_ID_APTX,

Wrong place, it should be put bellow DOLBY_E.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: creation of hls variant streams with master playlist in a single hlsenc instance

[刘歧]

> 在 2017年11月8日，18:02，Dixit, Vishwanath  写道：
> 
> I have added the copyright statement as per our legal team requirements. I 
> have updated the patches again. Please consider the patches in this mail 
> attachment as latest. Sorry for any inconvenience.
Add  Aman Gupta, DeHackEd, Hendrik Leppkes, Bela Bodecs, Nicolas George etc. 
and me too :D 

Thanks
> 
> Regards,
> Vishwanath
> 
> <0001-avformat-hlsenc-creation-of-hls-variant-streams-in-a.patch><0002-avformat-hlsenc-creation-of-hls-master-playlist-file.patch><0003-tests-fate-addition-of-test-case-for-hls-variant-str.patch>___
> 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] avcodec/mips: Improve hevc bi wgt 4 tap hv mc msa functions

[Michael Niedermayer]

On Tue, Nov 07, 2017 at 05:29:57AM +, Manojkumar Bhosale wrote:
> LGTM

will apply

thx

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

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


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

Re: [FFmpeg-devel] [PATCH] avcodec/mips: Improve hevc uni 4 tap hv mc msa functions

[Michael Niedermayer]

On Tue, Nov 07, 2017 at 05:30:02AM +, Manojkumar Bhosale wrote:
> LGTM

will apply
thx

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

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable


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

Re: [FFmpeg-devel] [PATCH] avcodec/mips: Improve hevc non-uni hv mc msa functions

[Michael Niedermayer]

On Tue, Nov 07, 2017 at 07:33:21AM +, Manojkumar Bhosale wrote:
> LGTM

will apply

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

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin


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

Re: [FFmpeg-devel] [PATCH] avcodec/mips: Improve hevc uni weighted 4 tap vt mc msa functions

[Michael Niedermayer]

On Tue, Nov 07, 2017 at 07:33:15AM +, Manojkumar Bhosale wrote:
> LGTM

will apply

thx

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

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus


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

Re: [FFmpeg-devel] [PATCH]lavf/tls_openssl: Fix compilation with current libressl

[Michael Niedermayer]

On Tue, Nov 07, 2017 at 12:19:02PM -0300, James Almer wrote:
> On 11/7/2017 12:15 PM, Hendrik Leppkes wrote:
> > On Tue, Nov 7, 2017 at 2:57 PM, James Almer  wrote:
> >> On 11/7/2017 10:48 AM, Hendrik Leppkes wrote:
> >>> On Tue, Nov 7, 2017 at 1:28 PM, Carl Eugen Hoyos  
> >>> wrote:
>  Hi!
> 
>  Attached patch fixes compilation with current libressl, related to 
>  ticket #6801.
> 
> >>>
> >>> I believe we basically had that exact same patch on the ML before, and
> >>> the consensus was to not have a jungle of #ifdefs in the code, and
> >>> instead have configure figure it out.
> >>
> >> configure could at most refuse to compile tls_openssl with libressl, but
> >> nothing to make it build successfully without ifdeffery.
> >>
> >> Is that preferred?
> > 
> > I meant to have configure figure out properly if OpenSSL 1.1 API is
> > available, and not repeat that check in the code several times, which
> > is growing more complex now.
> > 
> > - Hendrik
> 
> So you mean replacing "#if OPENSSL_VERSION_NUMBER >= 0x101fL &&
> !defined(LIBRESSL_VERSION_NUMBER)" with "#if CONFIG_OPENSSL_1_1" or
> similar? Should be easy and agree it's slightly cleaner.

+1
this seems the cleanest solution to me too

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

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope


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

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Michael Niedermayer]

On Tue, Nov 07, 2017 at 08:13:57PM +, Rostislav Pehlivanov wrote:
> On 6 November 2017 at 18:03, Nicolas George  wrote:
> 
> > Le sextidi 16 brumaire, an CCXXVI, Rostislav Pehlivanov a écrit :
> > > Signed-off-by: Rostislav Pehlivanov 
> > > ---
> > >  doc/APIchanges  | 3 +++
> > >  libavutil/frame.h   | 6 ++
> > >  libavutil/version.h | 2 +-
> > >  3 files changed, 10 insertions(+), 1 deletion(-)
> >
> > Sorry if it has come up before, but why not just add
> >
> > AVRational gamma;
> >
> > with 0/0 meaning unspecified? It seems like a relevant information, at
> > least as much as AVColor*. And overall much simpler.
> >
> > Regards,
> >
> > --
> >   Nicolas George
> >
> 
> Gamma info is related to mastering info so API users expect to get both
> using the same API rather than look for new fields in avframe and evaluate
> whether they're different on a per-frame basis.
> Also, it prevents from adding more fields to avframe and making a bigger
> mess.

If a filter changes gamma, it will then have to update the side data
if its in side data

I think i understand that you may see gamma more as a input device
characteristic. But really its part of the mapping of the physical
worlds light spectrum shape for each pixel to the 3 element vector
used to represent pixels.

As such if the representation of pixels change, gamma may change

Take a very simple example, simple rescaling or similar filtering,
all these filters should be performed with a flat 1.0 gamma to be
physically correct. So often a whole filter chain would be more
correct by converting the gamma != 1.0 8bit per sample to a gamma = 1.0
bps > 8bit first and at the end of the chain convert back to a more
commonly used gamma != 1.0.
We dont do this but we should support it at least optionally and
having gamma only in side data would make it a bit unwieldy


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope


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

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Michael Niedermayer]

On Wed, Nov 08, 2017 at 11:50:34AM +0100, Michael Niedermayer wrote:
> On Tue, Nov 07, 2017 at 08:13:57PM +, Rostislav Pehlivanov wrote:
> > On 6 November 2017 at 18:03, Nicolas George  wrote:
> > 
> > > Le sextidi 16 brumaire, an CCXXVI, Rostislav Pehlivanov a écrit :
> > > > Signed-off-by: Rostislav Pehlivanov 
> > > > ---
> > > >  doc/APIchanges  | 3 +++
> > > >  libavutil/frame.h   | 6 ++
> > > >  libavutil/version.h | 2 +-
> > > >  3 files changed, 10 insertions(+), 1 deletion(-)
> > >
> > > Sorry if it has come up before, but why not just add
> > >
> > > AVRational gamma;
> > >
> > > with 0/0 meaning unspecified? It seems like a relevant information, at
> > > least as much as AVColor*. And overall much simpler.
> > >
> > > Regards,
> > >
> > > --
> > >   Nicolas George
> > >
> > 
> > Gamma info is related to mastering info so API users expect to get both
> > using the same API rather than look for new fields in avframe and evaluate
> > whether they're different on a per-frame basis.
> > Also, it prevents from adding more fields to avframe and making a bigger
> > mess.
> 
> If a filter changes gamma, it will then have to update the side data
> if its in side data
> 
> I think i understand that you may see gamma more as a input device
> characteristic. But really its part of the mapping of the physical
> worlds light spectrum shape for each pixel to the 3 element vector
> used to represent pixels.
> 
> As such if the representation of pixels change, gamma may change
> 
> Take a very simple example, simple rescaling or similar filtering,
> all these filters should be performed with a flat 1.0 gamma to be
> physically correct. So often a whole filter chain would be more
> correct by converting the gamma != 1.0 8bit per sample to a gamma = 1.0
> bps > 8bit first and at the end of the chain convert back to a more
> commonly used gamma != 1.0.
> We dont do this but we should support it at least optionally and
> having gamma only in side data would make it a bit unwieldy

we also could have a set of recoding device parameters in side data
(mastering info)

and the current frames data[] gamma in avframe.
not sure thats a good idea or too confusing but it would allow
a filter to convert back to the input devices gamma without its value
being given to the filter as its in side data still

[...]


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus


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

Re: [FFmpeg-devel] [PATCH] avfilter: initial macroblock types export and visualization

[Michael Niedermayer]

On Thu, Nov 02, 2017 at 12:04:17PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Thu, Nov 2, 2017 at 7:52 AM, Paul B Mahol  wrote:
> 
> > On 11/2/17, Michael Niedermayer  wrote:
> > > Hi
> > >
> > > On Sat, Oct 28, 2017 at 07:43:05AM -0400, Ronald S. Bultje wrote:
> > >> Hi,
> > >>
> > >> On Fri, Oct 27, 2017 at 10:14 PM, Michael Niedermayer <
> > >> mich...@niedermayer.cc> wrote:
> > >>
> > >> > On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote:
> > >> > > Signed-off-by: Paul B Mahol 
> > >> > > ---
> > >> > >  libavcodec/mpegvideo.c |  10 +
> > >> > >  libavfilter/vf_codecview.c | 105 ++
> > >> > +++
> > >> > >  libavutil/frame.h  |   4 ++
> > >> > >  3 files changed, 119 insertions(+)
> > >> >
> > >> > First, thanks for working on this.
> > >> >
> > >> >
> > >> > [...]
> > >> >
> > >> > > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > >> > > index fef558ea2f..8481dc080b 100644
> > >> > > --- a/libavutil/frame.h
> > >> > > +++ b/libavutil/frame.h
> > >> > > @@ -141,6 +141,10 @@ enum AVFrameSideDataType {
> > >> > >   * metadata key entry "name".
> > >> > >   */
> > >> > >  AV_FRAME_DATA_ICC_PROFILE,
> > >> > > +/**
> > >> > > + * Macroblock types exported by some codecs.
> > >> > > + */
> > >> > > +AV_FRAME_DATA_MACROBLOCK_TYPES,
> > >> > >  };
> > >> > >
> > >> >
> > >> > This makes the internal data of the decoder part of the ABI and API of
> > >> > libavcodec and libavfilter
> > >> > and its undocumented
> > >> >
> > >> > do people prefer to make the internal data part of the ABI, document
> > >> > it and ensure it does not change till the next bump
> > >> >
> > >> [..]
> > >>
> > >> > or is there some other option iam missing ?
> > >> >
> > >>
> > >> I noted this on IRC also. A third option is to not document it and
> > >> consider
> > >> it codec-specific.
> > >>
> > >> (Codec-specific implies "ABI/API unstable" and subject to change -
> > "don't
> > >> mix versions".)
> > >
> > > If you say the interface is "ABI/API unstable" then we cannot use it
> > > from another lib like libavfilter. So this would not work.
> >
> > I disagree.
> 
> 
> I'd like to speak in solutions, not problems. We can simply prepend a
> version number to the flags and ensure versions match. That makes it easy
> to break "ABI" (and if versions don't match, simply display an error
> message and tell the user to not mix versions since this is considered an
> unstable feature).
> 
> By no means can we set the current ABI of this feature in stone, we're
> trying to fix it, not keep it. I appreciate Paul's effort to moving this
> forward and think his work pushes it in the right direction.

How does this "versioned", "codec specific" API/ABI fit together with the
carefully designed strict API used by the same filter to access motion
vectors (libavutil/motion_vector.h)
?

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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle


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

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Paul B Mahol]

On 11/8/17, Michael Niedermayer  wrote:
> On Wed, Nov 08, 2017 at 11:50:34AM +0100, Michael Niedermayer wrote:
>> On Tue, Nov 07, 2017 at 08:13:57PM +, Rostislav Pehlivanov wrote:
>> > On 6 November 2017 at 18:03, Nicolas George  wrote:
>> >
>> > > Le sextidi 16 brumaire, an CCXXVI, Rostislav Pehlivanov a ecrit :
>> > > > Signed-off-by: Rostislav Pehlivanov 
>> > > > ---
>> > > >  doc/APIchanges  | 3 +++
>> > > >  libavutil/frame.h   | 6 ++
>> > > >  libavutil/version.h | 2 +-
>> > > >  3 files changed, 10 insertions(+), 1 deletion(-)
>> > >
>> > > Sorry if it has come up before, but why not just add
>> > >
>> > > AVRational gamma;
>> > >
>> > > with 0/0 meaning unspecified? It seems like a relevant information,
>> > > at
>> > > least as much as AVColor*. And overall much simpler.
>> > >
>> > > Regards,
>> > >
>> > > --
>> > >   Nicolas George
>> > >
>> >
>> > Gamma info is related to mastering info so API users expect to get both
>> > using the same API rather than look for new fields in avframe and
>> > evaluate
>> > whether they're different on a per-frame basis.
>> > Also, it prevents from adding more fields to avframe and making a
>> > bigger
>> > mess.
>>
>> If a filter changes gamma, it will then have to update the side data
>> if its in side data
>>
>> I think i understand that you may see gamma more as a input device
>> characteristic. But really its part of the mapping of the physical
>> worlds light spectrum shape for each pixel to the 3 element vector
>> used to represent pixels.
>>
>> As such if the representation of pixels change, gamma may change
>>
>> Take a very simple example, simple rescaling or similar filtering,
>> all these filters should be performed with a flat 1.0 gamma to be
>> physically correct. So often a whole filter chain would be more
>> correct by converting the gamma != 1.0 8bit per sample to a gamma = 1.0
>> bps > 8bit first and at the end of the chain convert back to a more
>> commonly used gamma != 1.0.
>> We dont do this but we should support it at least optionally and
>> having gamma only in side data would make it a bit unwieldy
>
> we also could have a set of recoding device parameters in side data
> (mastering info)
>
> and the current frames data[] gamma in avframe.
> not sure thats a good idea or too confusing but it would allow
> a filter to convert back to the input devices gamma without its value
> being given to the filter as its in side data still

No, no and no. Gamma is not always available and polluting AVFrame is big no.
I vote and veto against your and Nicolas idea.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter: initial macroblock types export and visualization

[Paul B Mahol]

On 11/8/17, Michael Niedermayer  wrote:
> On Thu, Nov 02, 2017 at 12:04:17PM -0400, Ronald S. Bultje wrote:
>> Hi,
>>
>> On Thu, Nov 2, 2017 at 7:52 AM, Paul B Mahol  wrote:
>>
>> > On 11/2/17, Michael Niedermayer  wrote:
>> > > Hi
>> > >
>> > > On Sat, Oct 28, 2017 at 07:43:05AM -0400, Ronald S. Bultje wrote:
>> > >> Hi,
>> > >>
>> > >> On Fri, Oct 27, 2017 at 10:14 PM, Michael Niedermayer <
>> > >> mich...@niedermayer.cc> wrote:
>> > >>
>> > >> > On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote:
>> > >> > > Signed-off-by: Paul B Mahol 
>> > >> > > ---
>> > >> > >  libavcodec/mpegvideo.c |  10 +
>> > >> > >  libavfilter/vf_codecview.c | 105 ++
>> > >> > +++
>> > >> > >  libavutil/frame.h  |   4 ++
>> > >> > >  3 files changed, 119 insertions(+)
>> > >> >
>> > >> > First, thanks for working on this.
>> > >> >
>> > >> >
>> > >> > [...]
>> > >> >
>> > >> > > diff --git a/libavutil/frame.h b/libavutil/frame.h
>> > >> > > index fef558ea2f..8481dc080b 100644
>> > >> > > --- a/libavutil/frame.h
>> > >> > > +++ b/libavutil/frame.h
>> > >> > > @@ -141,6 +141,10 @@ enum AVFrameSideDataType {
>> > >> > >   * metadata key entry "name".
>> > >> > >   */
>> > >> > >  AV_FRAME_DATA_ICC_PROFILE,
>> > >> > > +/**
>> > >> > > + * Macroblock types exported by some codecs.
>> > >> > > + */
>> > >> > > +AV_FRAME_DATA_MACROBLOCK_TYPES,
>> > >> > >  };
>> > >> > >
>> > >> >
>> > >> > This makes the internal data of the decoder part of the ABI and API
>> > >> > of
>> > >> > libavcodec and libavfilter
>> > >> > and its undocumented
>> > >> >
>> > >> > do people prefer to make the internal data part of the ABI,
>> > >> > document
>> > >> > it and ensure it does not change till the next bump
>> > >> >
>> > >> [..]
>> > >>
>> > >> > or is there some other option iam missing ?
>> > >> >
>> > >>
>> > >> I noted this on IRC also. A third option is to not document it and
>> > >> consider
>> > >> it codec-specific.
>> > >>
>> > >> (Codec-specific implies "ABI/API unstable" and subject to change -
>> > "don't
>> > >> mix versions".)
>> > >
>> > > If you say the interface is "ABI/API unstable" then we cannot use it
>> > > from another lib like libavfilter. So this would not work.
>> >
>> > I disagree.
>>
>>
>> I'd like to speak in solutions, not problems. We can simply prepend a
>> version number to the flags and ensure versions match. That makes it easy
>> to break "ABI" (and if versions don't match, simply display an error
>> message and tell the user to not mix versions since this is considered an
>> unstable feature).
>>
>> By no means can we set the current ABI of this feature in stone, we're
>> trying to fix it, not keep it. I appreciate Paul's effort to moving this
>> forward and think his work pushes it in the right direction.
>
> How does this "versioned", "codec specific" API/ABI fit together with the
> carefully designed strict API used by the same filter to access motion
> vectors (libavutil/motion_vector.h)
> ?

If you do not like it as it is, write specification which I will follow.
Otherwise this code gonna be removed from decoder soonTM.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] aptx: implement the aptX bluetooth codec

[Aurelien Jacobs]

On Wed, Nov 08, 2017 at 10:54:18AM +0100, Paul B Mahol wrote:
> On 11/7/17, Aurelien Jacobs  wrote:
> > The encoder was reverse engineered from binary library and from
> > EP0398973B1 patent (long expired).
> > The decoder was simply deduced from the encoder.
> > ---
> >  doc/general.texi|   2 +
> >  libavcodec/Makefile |   2 +
> >  libavcodec/allcodecs.c  |   1 +
> >  libavcodec/aptx.c   | 826
> > 
> >  libavcodec/avcodec.h|   1 +
> >  libavcodec/codec_desc.c |   7 +
> >  6 files changed, 839 insertions(+)
> >  create mode 100644 libavcodec/aptx.c
> >
> 
> [...]
> 
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index c4134424f0..36a99f4162 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -614,6 +614,7 @@ enum AVCodecID {
> >  AV_CODEC_ID_PAF_AUDIO,
> >  AV_CODEC_ID_ON2AVC,
> >  AV_CODEC_ID_DSS_SP,
> > +AV_CODEC_ID_APTX,
> 
> Wrong place, it should be put bellow DOLBY_E.

Humm... Not sure why there are 2 separate lists for audio codecs but OK,
here is the updated patch.
>From 3d3a1afe678edfbd3b00079b59b84add3461a7b0 Mon Sep 17 00:00:00 2001
From: Aurelien Jacobs 
Date: Thu, 31 Aug 2017 20:12:54 +0200
Subject: [PATCH 1/2] aptx: implement the aptX bluetooth codec

The encoder was reverse engineered from binary library and from
EP0398973B1 patent (long expired).
The decoder was simply deduced from the encoder.
---
 doc/general.texi|   2 +
 libavcodec/Makefile |   2 +
 libavcodec/allcodecs.c  |   1 +
 libavcodec/aptx.c   | 826 
 libavcodec/avcodec.h|   1 +
 libavcodec/codec_desc.c |   7 +
 6 files changed, 839 insertions(+)
 create mode 100644 libavcodec/aptx.c

diff --git a/doc/general.texi b/doc/general.texi
index 9e6ae13435..4a89531c47 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -991,6 +991,8 @@ following image formats are supported:
 @item Amazing Studio PAF Audio @tab @tab  X
 @item Apple lossless audio   @tab  X  @tab  X
 @tab QuickTime fourcc 'alac'
+@item aptX   @tab  X  @tab  X
+@tab Used in Bluetooth A2DP
 @item ATRAC1 @tab @tab  X
 @item ATRAC3 @tab @tab  X
 @item ATRAC3+@tab @tab  X
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 3a33361f33..25706a263d 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -188,6 +188,8 @@ OBJS-$(CONFIG_AMV_ENCODER) += mjpegenc.o mjpegenc_common.o \
 OBJS-$(CONFIG_ANM_DECODER) += anm.o
 OBJS-$(CONFIG_ANSI_DECODER)+= ansi.o cga_data.o
 OBJS-$(CONFIG_APE_DECODER) += apedec.o
+OBJS-$(CONFIG_APTX_DECODER)+= aptx.o
+OBJS-$(CONFIG_APTX_ENCODER)+= aptx.o
 OBJS-$(CONFIG_APNG_DECODER)+= png.o pngdec.o pngdsp.o
 OBJS-$(CONFIG_APNG_ENCODER)+= png.o pngenc.o
 OBJS-$(CONFIG_SSA_DECODER) += assdec.o ass.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 98655ddd7c..61abe9939c 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -406,6 +406,7 @@ static void register_all(void)
 REGISTER_DECODER(AMRNB, amrnb);
 REGISTER_DECODER(AMRWB, amrwb);
 REGISTER_DECODER(APE,   ape);
+REGISTER_ENCDEC (APTX,  aptx);
 REGISTER_DECODER(ATRAC1,atrac1);
 REGISTER_DECODER(ATRAC3,atrac3);
 REGISTER_DECODER(ATRAC3AL,  atrac3al);
diff --git a/libavcodec/aptx.c b/libavcodec/aptx.c
new file mode 100644
index 00..b2879ea370
--- /dev/null
+++ b/libavcodec/aptx.c
@@ -0,0 +1,826 @@
+/*
+ * Audio Processing Technology codec for Bluetooth (aptX)
+ *
+ * Copyright (C) 2017  Aurelien Jacobs 
+ *
+ * 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/intreadwrite.h"
+#include "avcodec.h"
+#include "internal.h"
+#include "mathops.h"
+
+
+enum channels {
+LEFT,
+RIGHT,
+NB_CHANNELS
+};
+
+enum subbands {
+LF,  // Low Frequency (0-5.5 kHz)
+MLF, // Medium-Low Frequency (5.5-11kHz)
+MHF, // Medium-High Frequency (11-16.5kHz)
+HF,  // High Frequency (16.5-22kHz)
+NB_SUBBANDS
+};
+
+#define NB_

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Nicolas George]

L'octidi 18 brumaire, an CCXXVI, Paul B Mahol a écrit :
> No, no and no. Gamma is not always available and polluting AVFrame is big no.

The same arguments would apply for pict_type, channel_layout, color_*,
crop_*. This is no pollution, it is a minor but relevant information
about the frame.

> I vote and veto against your and Nicolas idea.

What a nice way of conducing a discussion. Consider I vote and veto
against adding side data.

Regards,

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

Re: [FFmpeg-devel] [PATCH] avfilter: slice processing for geq

[Marc-Antoine ARNAUD]

This patch will fix the stride issue.
Is it valid for you ?

Does it required to merge these 2 patches ? (and remove base64 encoding on
the first one)

Thx
Marc-Antoine

Le ven. 3 nov. 2017 à 03:13, Michael Niedermayer  a
écrit :

> On Thu, Nov 02, 2017 at 02:04:33PM +, Marc-Antoine ARNAUD wrote:
> >
>
> >  vf_geq.c |  126
> ---
> >  1 file changed, 89 insertions(+), 37 deletions(-)
> > b0379f3d7f1d9660e209fe491f48fd7f70113615
> 0001-avfilter-slice-processing-for-geq.patch
> > From c4bdf956e7f8b91b03f16bdf80b30058a117aae2 Mon Sep 17 00:00:00 2001
> > From: Marc-Antoine Arnaud 
> > Date: Thu, 2 Nov 2017 12:25:46 +0100
> > Subject: [PATCH] avfilter: slice processing for geq
>
> > Content-Type: text/x-patch; charset="utf-8"
> > Content-Transfer-Encoding: base64
>
> hmm
>
>
> >
> > ---
> >  libavfilter/vf_geq.c | 126
> ---
> >  1 file changed, 89 insertions(+), 37 deletions(-)
>
> breaks:
> ./ffplay  matrixbench_mpeg2.mpg  -vf 'geq=b=b(X\,Y)-r(X\,Y)'
>
> looks like possibly something related to strides
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Its not that you shouldnt use gotos but rather that you should write
> readable code and code with gotos often but not always is less readable
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


0001-avfilter-fix-stride-for-g_eq.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Paul B Mahol]

On 11/8/17, Nicolas George  wrote:
> L'octidi 18 brumaire, an CCXXVI, Paul B Mahol a ecrit :
>> No, no and no. Gamma is not always available and polluting AVFrame is big
>> no.
>
> The same arguments would apply for pict_type, channel_layout, color_*,
> crop_*. This is no pollution, it is a minor but relevant information
> about the frame.

Why are you doing this? Your bikesheds do not apply anymore.
You just like to make your opinions important to block other devs hard work.
What's next? Adding more entries like this?
Only PNG appears to set gamma and others simply do not care.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Added HW H.264 and HEVC encoding for AMD GPUs based on AMF SDK

[Mark Thompson]

On 06/11/17 22:46, Michael Niedermayer wrote:
> ...
> In file included from src/libavcodec/amfenc.h:24:0,
>  from src/libavcodec/amfenc.c:27:
> src/compat/amd/amfsdkenc.h:191:23: error: no previous prototype for 
> ‘AMFConstructRect’ [-Werror=missing-prototypes]
>  AMF_INLINE struct AMFRect AMFConstructRect(amf_int32 left, amf_int32 top, 
> amf_int32 right, amf_int32 bottom)
>^
> src/compat/amd/amfsdkenc.h:203:23: error: no previous prototype for 
> ‘AMFConstructSize’ [-Werror=missing-prototypes]
>  AMF_INLINE struct AMFSize AMFConstructSize(amf_int32 width, amf_int32 height)
>^
> src/compat/amd/amfsdkenc.h:215:24: error: no previous prototype for 
> ‘AMFConstructPoint’ [-Werror=missing-prototypes]
>  AMF_INLINE struct AMFPoint AMFConstructPoint(amf_int32 x, amf_int32 y)
> ^
> src/compat/amd/amfsdkenc.h:227:23: error: no previous prototype for 
> ‘AMFConstructRate’ [-Werror=missing-prototypes]
>  AMF_INLINE struct AMFRate AMFConstructRate(amf_uint32 num, amf_uint32 den)
>^
> src/compat/amd/amfsdkenc.h:239:24: error: no previous prototype for 
> ‘AMFConstructRatio’ [-Werror=missing-prototypes]
>  AMF_INLINE struct AMFRatio AMFConstructRatio(amf_uint32 num, amf_uint32 den)
> ^
> In file included from src/libavcodec/amfenc.h:24:0,
>  from src/libavcodec/amfenc.c:27:
> src/compat/amd/amfsdkenc.h:275:24: error: no previous prototype for 
> ‘AMFConstructColor’ [-Werror=missing-prototypes]
>  AMF_INLINE struct AMFColor AMFConstructColor(amf_uint8 r, amf_uint8 g, 
> amf_uint8 b, amf_uint8 a)
> ^
> In file included from src/libavcodec/amfenc.h:24:0,
>  from src/libavcodec/amfenc.c:27:
> src/compat/amd/amfsdkenc.h:293:45: error: no previous prototype for 
> ‘amf_variant_alloc’ [-Werror=missing-prototypes]
>  AMF_INLINE void* AMF_CDECL_CALL amf_variant_alloc(amf_size count)
>  ^
> src/compat/amd/amfsdkenc.h:297:44: error: no previous prototype for 
> ‘amf_variant_free’ [-Werror=missing-prototypes]
>  AMF_INLINE void AMF_CDECL_CALL amf_variant_free(void* ptr)
> ^
> cc1: some warnings being treated as errors
> make: *** [libavcodec/amfenc.o] Error 1

This is showing an error in the AMF C API.

AMF_INLINE is defined at 

 as just "__inline__" (i.e. "inline"), so these functions are all inline 
without a storage class specifier.  But, no external definition actually exists 
anywhere, so if the compiler doesn't inline all instances you will fail to link.

(The behaviour is not the same in C++, where external versions are generated 
anyway in this case and then the multiple instances collapsed together later by 
invoking the ODR.)

Trivial test case (gcc defaults to not inlining anything when no optimisation 
options are given):

$ cat amf-inline.c
#include "Platform.h"

int main(void)
{
AMFSize size;
size = AMFConstructSize(123, 456);
return 0;
}
$ gcc -std=c99 amf-inline.c
In file included from amf-inline.c:1:0:
Platform.h: In function ‘AMFCompareGUIDs’:
Platform.h:421:16: warning: implicit declaration of function ‘memcmp’ 
[-Wimplicit-function-declaration]
 return memcmp(&guid1, &guid2, sizeof(guid1)) == 0;
^
/tmp/ccMCUBTW.o: In function `main':
amf-inline.c:(.text+0x13): undefined reference to `AMFConstructSize'
collect2: error: ld returned 1 exit status
$ gcc -c -std=c99 amf-inline.c
In file included from amf-inline.c:1:0:
Platform.h: In function ‘AMFCompareGUIDs’:
Platform.h:421:16: warning: implicit declaration of function ‘memcmp’ 
[-Wimplicit-function-declaration]
 return memcmp(&guid1, &guid2, sizeof(guid1)) == 0;
^
$ nm amf-inline.o 
 U AMFConstructSize
 T main
$ gcc -O1 -std=c99 amf-inline.c
In file included from amf-inline.c:1:0:
Platform.h: In function ‘AMFCompareGUIDs’:
Platform.h:421:16: warning: implicit declaration of function ‘memcmp’ 
[-Wimplicit-function-declaration]
 return memcmp(&guid1, &guid2, sizeof(guid1)) == 0;
^
$ 

I think AMF_INLINE should for these functions instead be "static inline" 
(though I didn't look at all uses, so there may be others where this isn't 
true).

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

Re: [FFmpeg-devel] [PATCH] avfilter: slice processing for geq

[Paul B Mahol]

On 11/8/17, Marc-Antoine ARNAUD  wrote:
> This patch will fix the stride issue.
> Is it valid for you ?
>
> Does it required to merge these 2 patches ? (and remove base64 encoding on
> the first one)

Please merge those two patches, base64 encoding should not be needed
(it helps to faster review patches if they are not encoded).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] aptx: add raw muxer and demuxer for aptX

[Rostislav Pehlivanov]

On 7 November 2017 at 22:26, Aurelien Jacobs  wrote:

> ---
>  doc/general.texi |  1 +
>  libavformat/Makefile |  2 ++
>  libavformat/allformats.c |  1 +
>  libavformat/aptxdec.c| 58 ++
> ++
>  libavformat/rawenc.c | 13 +++
>  libavformat/utils.c  |  1 +
>  6 files changed, 76 insertions(+)
>  create mode 100644 libavformat/aptxdec.c
>
> diff --git a/doc/general.texi b/doc/general.texi
> index 4a89531c47..79e0bd0993 100644
> --- a/doc/general.texi
> +++ b/doc/general.texi
> @@ -439,6 +439,7 @@ library:
>  @item QCP   @tab   @tab X
>  @item raw ADTS (AAC)@tab X @tab X
>  @item raw AC-3  @tab X @tab X
> +@item raw aptX  @tab X @tab X
>  @item raw Chinese AVS video @tab X @tab X
>  @item raw CRI ADX   @tab X @tab X
>  @item raw Dirac @tab X @tab X
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index caebe5b146..21fb892a81 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -92,6 +92,8 @@ OBJS-$(CONFIG_APC_DEMUXER)   += apc.o
>  OBJS-$(CONFIG_APE_DEMUXER)   += ape.o apetag.o img2.o
>  OBJS-$(CONFIG_APNG_DEMUXER)  += apngdec.o
>  OBJS-$(CONFIG_APNG_MUXER)+= apngenc.o
> +OBJS-$(CONFIG_APTX_DEMUXER)  += aptxdec.o rawdec.o
> +OBJS-$(CONFIG_APTX_MUXER)+= rawenc.o
>  OBJS-$(CONFIG_AQTITLE_DEMUXER)   += aqtitledec.o subtitles.o
>  OBJS-$(CONFIG_ASF_DEMUXER)   += asfdec_f.o asf.o asfcrypt.o \
>  avlanguage.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 405ddb5ad9..40964a0df0 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -67,6 +67,7 @@ static void register_all(void)
>  REGISTER_DEMUXER (APC,  apc);
>  REGISTER_DEMUXER (APE,  ape);
>  REGISTER_MUXDEMUX(APNG, apng);
> +REGISTER_MUXDEMUX(APTX, aptx);
>  REGISTER_DEMUXER (AQTITLE,  aqtitle);
>  REGISTER_MUXDEMUX(ASF,  asf);
>  REGISTER_DEMUXER (ASF_O,asf_o);
> diff --git a/libavformat/aptxdec.c b/libavformat/aptxdec.c
> new file mode 100644
> index 00..90ce789454
> --- /dev/null
> +++ b/libavformat/aptxdec.c
> @@ -0,0 +1,58 @@
> +/*
> + * RAW aptX demuxer
> + *
> + * Copyright (C) 2017  Aurelien Jacobs 
> + *
> + * 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 "avformat.h"
> +#include "rawdec.h"
> +
> +#define APTX_BLOCK_SIZE   4
> +#define APTX_PACKET_SIZE  (256*APTX_BLOCK_SIZE)
> +
> +static int aptx_read_header(AVFormatContext *s)
> +{
> +AVStream *st = avformat_new_stream(s, NULL);
> +if (!st)
> +return AVERROR(ENOMEM);
> +st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
> +st->codecpar->codec_id = AV_CODEC_ID_APTX;
> +st->codecpar->format = AV_SAMPLE_FMT_S32P;
> +st->codecpar->channels = 2;
> +st->codecpar->sample_rate = 44100;
> +st->codecpar->bits_per_coded_sample = 4;
> +st->codecpar->block_align = APTX_BLOCK_SIZE;
> +st->codecpar->frame_size = APTX_PACKET_SIZE;
> +st->start_time = 0;
> +return 0;
> +}
> +
> +static int aptx_read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +return av_get_packet(s->pb, pkt, APTX_PACKET_SIZE);
> +}
> +
> +AVInputFormat ff_aptx_demuxer = {
> +.name   = "aptx",
> +.long_name  = NULL_IF_CONFIG_SMALL("raw aptX"),
> +.extensions = "aptx",
> +.read_header= aptx_read_header,
> +.read_packet= aptx_read_packet,
> +.flags  = AVFMT_GENERIC_INDEX,
> +};
> diff --git a/libavformat/rawenc.c b/libavformat/rawenc.c
> index f640121cb4..aa3ef76fbf 100644
> --- a/libavformat/rawenc.c
> +++ b/libavformat/rawenc.c
> @@ -91,6 +91,19 @@ AVOutputFormat ff_adx_muxer = {
>  };
>  #endif
>
> +#if CONFIG_APTX_MUXER
> +AVOutputFormat ff_aptx_muxer = {
> +.name  = "aptx",
> +.long_name = NULL_IF_CONFIG_SMALL("raw aptX (Audio Processing
> Technology for Bluetooth)"),
> +.extensions= "aptx",
> +.audio_codec   = AV_CODEC_ID_APT

Re: [FFmpeg-devel] [PATCH 1/2] aptx: implement the aptX bluetooth codec

[Michael Niedermayer]

On Wed, Nov 08, 2017 at 02:06:09PM +0100, Aurelien Jacobs wrote:
[...]
> +typedef const struct {
> +const int32_t *quantize_intervals;
> +const int32_t *invert_quantize_dither_factors;
> +const int32_t *quantize_dither_factors;

> +const int32_t *quantize_factor_select_offset;

this would fit in int16_t *


> +int tables_size;
> +int32_t quantized_bits;
> +int32_t prediction_order;
> +} ConstTables;
> +
> +static ConstTables tables[NB_SUBBANDS] = {
> +[LF]  = { quantize_intervals_LF,
> +  invert_quantize_dither_factors_LF,
> +  quantize_dither_factors_LF,
> +  quantize_factor_select_offset_LF,
> +  FF_ARRAY_ELEMS(quantize_intervals_LF),
> +  7, 24 },
> +[MLF] = { quantize_intervals_MLF,
> +  invert_quantize_dither_factors_MLF,
> +  quantize_dither_factors_MLF,
> +  quantize_factor_select_offset_MLF,
> +  FF_ARRAY_ELEMS(quantize_intervals_MLF),
> +  4, 12 },
> +[MHF] = { quantize_intervals_MHF,
> +  invert_quantize_dither_factors_MHF,
> +  quantize_dither_factors_MHF,
> +  quantize_factor_select_offset_MHF,
> +  FF_ARRAY_ELEMS(quantize_intervals_MHF),
> +  2, 6 },
> +[HF]  = { quantize_intervals_HF,
> +  invert_quantize_dither_factors_HF,
> +  quantize_dither_factors_HF,
> +  quantize_factor_select_offset_HF,
> +  FF_ARRAY_ELEMS(quantize_intervals_HF),
> +  3, 12 },
> +};
> +

> +static const int32_t quantization_factors[32] = {
> +2048, 2093, 2139, 2186, 2233, 2282, 2332, 2383,
> +2435, 2489, 2543, 2599, 2656, 2714, 2774, 2834,
> +2896, 2960, 3025, 3091, 3158, 3228, 3298, 3371,
> +3444, 3520, 3597, 3676, 3756, 3838, 3922, 4008,
> +};

this too would fir in int16_t

[...]
> +/*
> + * Push one sample into a circular signal buffer.
> + */
> +av_always_inline
> +static void aptx_qmf_filter_signal_push(FilterSignal *signal, int32_t sample)
> +{
> +signal->buffer[signal->pos] = sample;
> +signal->buffer[signal->pos+FILTER_TAPS] = sample;
> +signal->pos = (signal->pos + 1) % FILTER_TAPS;

% could be replaced by &


> +}
> +
> +/*
> + * Compute the convolution of the signal with the coefficients, and reduce
> + * to 24 bits by applying the specified right shifting.
> + */
> +av_always_inline
> +static int32_t aptx_qmf_convolution(FilterSignal *signal,
> +const int32_t coeffs[FILTER_TAPS],
> +int shift)
> +{
> +int32_t *sig = &signal->buffer[signal->pos];
> +int64_t e = 0;
> +

> +for (int i = 0; i < FILTER_TAPS; i++)

"for (int" is something we avoided as some comilers didnt like it,
iam not sure if this is still true but there are none in the codebase

also a fate test for this would be a good idea

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"- "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."


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

Re: [FFmpeg-devel] [PATCH 1/2] aptx: implement the aptX bluetooth codec

[Rostislav Pehlivanov]

On 8 November 2017 at 17:26, Michael Niedermayer 
wrote:

> On Wed, Nov 08, 2017 at 02:06:09PM +0100, Aurelien Jacobs wrote:
> [...]
> > +typedef const struct {
> > +const int32_t *quantize_intervals;
> > +const int32_t *invert_quantize_dither_factors;
> > +const int32_t *quantize_dither_factors;
>
> > +const int32_t *quantize_factor_select_offset;
>
> this would fit in int16_t *
>
>
> > +int tables_size;
> > +int32_t quantized_bits;
> > +int32_t prediction_order;
> > +} ConstTables;
> > +
> > +static ConstTables tables[NB_SUBBANDS] = {
> > +[LF]  = { quantize_intervals_LF,
> > +  invert_quantize_dither_factors_LF,
> > +  quantize_dither_factors_LF,
> > +  quantize_factor_select_offset_LF,
> > +  FF_ARRAY_ELEMS(quantize_intervals_LF),
> > +  7, 24 },
> > +[MLF] = { quantize_intervals_MLF,
> > +  invert_quantize_dither_factors_MLF,
> > +  quantize_dither_factors_MLF,
> > +  quantize_factor_select_offset_MLF,
> > +  FF_ARRAY_ELEMS(quantize_intervals_MLF),
> > +  4, 12 },
> > +[MHF] = { quantize_intervals_MHF,
> > +  invert_quantize_dither_factors_MHF,
> > +  quantize_dither_factors_MHF,
> > +  quantize_factor_select_offset_MHF,
> > +  FF_ARRAY_ELEMS(quantize_intervals_MHF),
> > +  2, 6 },
> > +[HF]  = { quantize_intervals_HF,
> > +  invert_quantize_dither_factors_HF,
> > +  quantize_dither_factors_HF,
> > +  quantize_factor_select_offset_HF,
> > +  FF_ARRAY_ELEMS(quantize_intervals_HF),
> > +  3, 12 },
> > +};
> > +
>
> > +static const int32_t quantization_factors[32] = {
> > +2048, 2093, 2139, 2186, 2233, 2282, 2332, 2383,
> > +2435, 2489, 2543, 2599, 2656, 2714, 2774, 2834,
> > +2896, 2960, 3025, 3091, 3158, 3228, 3298, 3371,
> > +3444, 3520, 3597, 3676, 3756, 3838, 3922, 4008,
> > +};
>
> this too would fir in int16_t
>
> [...]
> > +/*
> > + * Push one sample into a circular signal buffer.
> > + */
> > +av_always_inline
> > +static void aptx_qmf_filter_signal_push(FilterSignal *signal, int32_t
> sample)
> > +{
> > +signal->buffer[signal->pos] = sample;
> > +signal->buffer[signal->pos+FILTER_TAPS] = sample;
> > +signal->pos = (signal->pos + 1) % FILTER_TAPS;
>
> % could be replaced by &
>
>
> > +}
> > +
> > +/*
> > + * Compute the convolution of the signal with the coefficients, and
> reduce
> > + * to 24 bits by applying the specified right shifting.
> > + */
> > +av_always_inline
> > +static int32_t aptx_qmf_convolution(FilterSignal *signal,
> > +const int32_t coeffs[FILTER_TAPS],
> > +int shift)
> > +{
> > +int32_t *sig = &signal->buffer[signal->pos];
> > +int64_t e = 0;
> > +
>
> > +for (int i = 0; i < FILTER_TAPS; i++)
>
> "for (int" is something we avoided as some comilers didnt like it,
> iam not sure if this is still true but there are none in the codebase
>
> also a fate test for this would be a good idea
>
>
No, I completely reject this idea.

I want to use for (int loops and have always wanted to use them.
Show me a comiler which we support which doesn't accept those. Then I'll
suggest we drop support for it.

Its time we bring the codebase to the 21st century and this is my main
issue - being unable to use for (int loops.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavfi/scale2ref: Set output frame rate to main input frame rate

[Michael Niedermayer]

On Wed, Nov 08, 2017 at 09:23:58AM +0100, Carl Eugen Hoyos wrote:
> Hi!
> 
> I believe attached patch fixes ticket #6817.
> 
> Please comment, Carl Eugen

>  vf_scale.c |1 +
>  1 file changed, 1 insertion(+)
> cbc17115125ac2e6ada248ec53f973a81bfc762f  
> 0001-lavfi-scale2ref-Set-output-frame-rate-to-main-input-.patch
> From deee64102ef898b5f99974cf11bb05ec09963e90 Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos 
> Date: Wed, 8 Nov 2017 09:15:29 +0100
> Subject: [PATCH] lavfi/scale2ref: Set output frame rate to main input frame
>  rate.
> 
> Fixes ticket #6817.
> ---
>  libavfilter/vf_scale.c |1 +
>  1 file changed, 1 insertion(+)

LGTM

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.


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

Re: [FFmpeg-devel] [PATCH] avfilter: initial macroblock types export and visualization

[Michael Niedermayer]

On Wed, Nov 08, 2017 at 12:18:24PM +0100, Paul B Mahol wrote:
> On 11/8/17, Michael Niedermayer  wrote:
> > On Thu, Nov 02, 2017 at 12:04:17PM -0400, Ronald S. Bultje wrote:
> >> Hi,
> >>
> >> On Thu, Nov 2, 2017 at 7:52 AM, Paul B Mahol  wrote:
> >>
> >> > On 11/2/17, Michael Niedermayer  wrote:
> >> > > Hi
> >> > >
> >> > > On Sat, Oct 28, 2017 at 07:43:05AM -0400, Ronald S. Bultje wrote:
> >> > >> Hi,
> >> > >>
> >> > >> On Fri, Oct 27, 2017 at 10:14 PM, Michael Niedermayer <
> >> > >> mich...@niedermayer.cc> wrote:
> >> > >>
> >> > >> > On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote:
> >> > >> > > Signed-off-by: Paul B Mahol 
> >> > >> > > ---
> >> > >> > >  libavcodec/mpegvideo.c |  10 +
> >> > >> > >  libavfilter/vf_codecview.c | 105 ++
> >> > >> > +++
> >> > >> > >  libavutil/frame.h  |   4 ++
> >> > >> > >  3 files changed, 119 insertions(+)
> >> > >> >
> >> > >> > First, thanks for working on this.
> >> > >> >
> >> > >> >
> >> > >> > [...]
> >> > >> >
> >> > >> > > diff --git a/libavutil/frame.h b/libavutil/frame.h
> >> > >> > > index fef558ea2f..8481dc080b 100644
> >> > >> > > --- a/libavutil/frame.h
> >> > >> > > +++ b/libavutil/frame.h
> >> > >> > > @@ -141,6 +141,10 @@ enum AVFrameSideDataType {
> >> > >> > >   * metadata key entry "name".
> >> > >> > >   */
> >> > >> > >  AV_FRAME_DATA_ICC_PROFILE,
> >> > >> > > +/**
> >> > >> > > + * Macroblock types exported by some codecs.
> >> > >> > > + */
> >> > >> > > +AV_FRAME_DATA_MACROBLOCK_TYPES,
> >> > >> > >  };
> >> > >> > >
> >> > >> >
> >> > >> > This makes the internal data of the decoder part of the ABI and API
> >> > >> > of
> >> > >> > libavcodec and libavfilter
> >> > >> > and its undocumented
> >> > >> >
> >> > >> > do people prefer to make the internal data part of the ABI,
> >> > >> > document
> >> > >> > it and ensure it does not change till the next bump
> >> > >> >
> >> > >> [..]
> >> > >>
> >> > >> > or is there some other option iam missing ?
> >> > >> >
> >> > >>
> >> > >> I noted this on IRC also. A third option is to not document it and
> >> > >> consider
> >> > >> it codec-specific.
> >> > >>
> >> > >> (Codec-specific implies "ABI/API unstable" and subject to change -
> >> > "don't
> >> > >> mix versions".)
> >> > >
> >> > > If you say the interface is "ABI/API unstable" then we cannot use it
> >> > > from another lib like libavfilter. So this would not work.
> >> >
> >> > I disagree.
> >>
> >>
> >> I'd like to speak in solutions, not problems. We can simply prepend a
> >> version number to the flags and ensure versions match. That makes it easy
> >> to break "ABI" (and if versions don't match, simply display an error
> >> message and tell the user to not mix versions since this is considered an
> >> unstable feature).
> >>
> >> By no means can we set the current ABI of this feature in stone, we're
> >> trying to fix it, not keep it. I appreciate Paul's effort to moving this
> >> forward and think his work pushes it in the right direction.
> >
> > How does this "versioned", "codec specific" API/ABI fit together with the
> > carefully designed strict API used by the same filter to access motion
> > vectors (libavutil/motion_vector.h)
> > ?
> 
> If you do not like it as it is, write specification which I will follow.

I can. Is there some preferrances that people have ?
this can be done in a wide range from simple to quite complex.
from a simple bitmask in a byte or int array to a tree structure
describing subdivisions or a list of structs similar to
libavutil/motion_vector.h.
The simple one will not be able to represent everything


> Otherwise this code gonna be removed from decoder soonTM.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire


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

[FFmpeg-devel] [PATCH 0/9] Fix various issues found by coverity

[Timo Rothenpieler]

Timo Rothenpieler (9):
  lavfi/paletteuse: check get_color return value
  avformat/hlsenc: allocate space for terminating null
  lavc/vaapi_decode: fix uninitialized use of variables
  avfilter/af_haas: validate par_m_source parameter
  avformat/fitsenc: validate input pixel format
  avfilter/vf_premultiply: make sure plane count is less than 4
  avfilter/vf_premultiply: fix memory-leak on failure
  movenc-test: fix potential uninitialized read
  avfilter/signature_lookup: fix potential uninitialized reads

 libavcodec/vaapi_decode.c  | 5 +++--
 libavfilter/af_haas.c  | 1 +
 libavfilter/signature_lookup.c | 2 +-
 libavfilter/vf_paletteuse.c| 5 -
 libavfilter/vf_premultiply.c   | 9 ++---
 libavformat/fitsenc.c  | 2 ++
 libavformat/hlsenc.c   | 2 +-
 libavformat/tests/movenc.c | 2 +-
 8 files changed, 19 insertions(+), 9 deletions(-)

-- 
2.14.2

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

[FFmpeg-devel] [PATCH 1/9] lavfi/paletteuse: check get_color return value

[Timo Rothenpieler]

Fixes CID #1420396
---
 libavfilter/vf_paletteuse.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavfilter/vf_paletteuse.c b/libavfilter/vf_paletteuse.c
index ed80ab04d5..1980907e70 100644
--- a/libavfilter/vf_paletteuse.c
+++ b/libavfilter/vf_paletteuse.c
@@ -380,8 +380,11 @@ static av_always_inline int 
get_dst_color_err(PaletteUseContext *s,
 const uint8_t r = c >> 16 & 0xff;
 const uint8_t g = c >>  8 & 0xff;
 const uint8_t b = c   & 0xff;
+uint32_t dstc;
 const int dstx = color_get(s, c, a, r, g, b, search_method);
-const uint32_t dstc = s->palette[dstx];
+if (dstx < 0)
+return dstx;
+dstc = s->palette[dstx];
 *er = r - (dstc >> 16 & 0xff);
 *eg = g - (dstc >>  8 & 0xff);
 *eb = b - (dstc   & 0xff);
-- 
2.14.2

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

[FFmpeg-devel] [PATCH 3/9] lavc/vaapi_decode: fix uninitialized use of variables

[Timo Rothenpieler]

Fixes CID #1419524 and #1412855
---
 libavcodec/vaapi_decode.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
index 27ef33837c..a782cfd8da 100644
--- a/libavcodec/vaapi_decode.c
+++ b/libavcodec/vaapi_decode.c
@@ -281,8 +281,9 @@ static int vaapi_decode_make_config(AVCodecContext *avctx)
 VAStatus vas;
 int err, i, j;
 const AVCodecDescriptor *codec_desc;
-VAProfile profile, va_profile, *profile_list = NULL;
-int profile_count, exact_match, alt_profile;
+VAProfile va_profile = VAProfileNone;
+VAProfile profile, *profile_list = NULL;
+int profile_count, exact_match, alt_profile = 0;
 const AVPixFmtDescriptor *sw_desc, *desc;
 
 codec_desc = avcodec_descriptor_get(avctx->codec_id);
-- 
2.14.2

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

[FFmpeg-devel] [PATCH 4/9] avfilter/af_haas: validate par_m_source parameter

[Timo Rothenpieler]

Fixes CID #1417663
---
 libavfilter/af_haas.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavfilter/af_haas.c b/libavfilter/af_haas.c
index 691c251f54..fac4b6cf2f 100644
--- a/libavfilter/af_haas.c
+++ b/libavfilter/af_haas.c
@@ -162,6 +162,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 case 1: mid = src[1]; break;
 case 2: mid = (src[0] + src[1]) * 0.5; break;
 case 3: mid = (src[0] - src[1]) * 0.5; break;
+default: return AVERROR(EINVAL);
 }
 
 mid *= level_in;
-- 
2.14.2

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

[FFmpeg-devel] [PATCH 2/9] avformat/hlsenc: allocate space for terminating null

[Timo Rothenpieler]

Fixes CID #1420394
---
 libavformat/hlsenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 5ea9d216a4..b571772f60 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -1455,7 +1455,7 @@ static int hls_write_header(AVFormatContext *s)
 if (basename_size > 0) {
 hls->base_output_dirname = av_malloc(basename_size);
 } else {
-hls->base_output_dirname = 
av_malloc(strlen(hls->fmp4_init_filename));
+hls->base_output_dirname = 
av_malloc(strlen(hls->fmp4_init_filename) + 1);
 }
 if (!hls->base_output_dirname) {
 ret = AVERROR(ENOMEM);
-- 
2.14.2

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

[FFmpeg-devel] [PATCH 7/9] avfilter/vf_premultiply: fix memory-leak on failure

[Timo Rothenpieler]

Fixes CID #1416352
---
 libavfilter/vf_premultiply.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavfilter/vf_premultiply.c b/libavfilter/vf_premultiply.c
index 1af6433bc9..4b6abc6e73 100644
--- a/libavfilter/vf_premultiply.c
+++ b/libavfilter/vf_premultiply.c
@@ -609,9 +609,10 @@ static int activate(AVFilterContext *ctx)
 int64_t pts;
 
 if ((ret = ff_inlink_consume_frame(ctx->inputs[0], &frame)) > 0) {
-if ((ret = filter_frame(ctx, &out, frame, frame)) < 0)
-return ret;
+ret = filter_frame(ctx, &out, frame, frame);
 av_frame_free(&frame);
+if (ret < 0)
+return ret;
 ret = ff_filter_frame(ctx->outputs[0], out);
 }
 if (ret < 0) {
-- 
2.14.2

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

[FFmpeg-devel] [PATCH 9/9] avfilter/signature_lookup: fix potential uninitialized reads

[Timo Rothenpieler]

Fixes CID #1403238 and #1403239
---
 libavfilter/signature_lookup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
index 272c717c77..7de4a88ca2 100644
--- a/libavfilter/signature_lookup.c
+++ b/libavfilter/signature_lookup.c
@@ -421,7 +421,7 @@ static MatchingInfo evaluate_parameters(AVFilterContext 
*ctx, SignatureContext *
 int fcount = 0, goodfcount = 0, gooda = 0, goodb = 0;
 double meandist, minmeandist = bestmatch.meandist;
 int tolerancecount = 0;
-FineSignature *a, *b, *aprev, *bprev;
+FineSignature *a, *b, *aprev = NULL, *bprev = NULL;
 int status = STATUS_NULL;
 
 for (; infos != NULL; infos = infos->next) {
-- 
2.14.2

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

[FFmpeg-devel] [PATCH 6/9] avfilter/vf_premultiply: make sure plane count is less than 4

[Timo Rothenpieler]

Fixes CID #1416350, #1416351, #1416353 and #1416354
---
 libavfilter/vf_premultiply.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavfilter/vf_premultiply.c b/libavfilter/vf_premultiply.c
index 5120adc476..1af6433bc9 100644
--- a/libavfilter/vf_premultiply.c
+++ b/libavfilter/vf_premultiply.c
@@ -476,7 +476,7 @@ static int filter_frame(AVFilterContext *ctx,
 }
 }
 
-for (p = 0; p < s->nb_planes; p++) {
+for (p = 0; p < s->nb_planes && p < 4; p++) {
 if (!((1 << p) & s->planes) || p == 3) {
 av_image_copy_plane((*out)->data[p], (*out)->linesize[p], 
base->data[p], base->linesize[p],
 s->linesize[p], s->height[p]);
@@ -523,6 +523,8 @@ static int config_input(AVFilterLink *inlink)
 int vsub, hsub, ret;
 
 s->nb_planes = av_pix_fmt_count_planes(inlink->format);
+if (s->nb_planes > 4)
+return AVERROR_BUG;
 
 if ((ret = av_image_fill_linesizes(s->linesize, inlink->format, 
inlink->w)) < 0)
 return ret;
-- 
2.14.2

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

[FFmpeg-devel] [PATCH 8/9] movenc-test: fix potential uninitialized read

[Timo Rothenpieler]

Fixes CID #1413023
---
 libavformat/tests/movenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/tests/movenc.c b/libavformat/tests/movenc.c
index 8e59b74259..a210aec7ba 100644
--- a/libavformat/tests/movenc.c
+++ b/libavformat/tests/movenc.c
@@ -108,7 +108,7 @@ static int io_write_data_type(void *opaque, uint8_t *buf, 
int size,
   enum AVIODataMarkerType type, int64_t time)
 {
 char timebuf[30], content[5] = { 0 };
-const char *str;
+const char *str = "unknown";
 switch (type) {
 case AVIO_DATA_MARKER_HEADER: str = "header";   break;
 case AVIO_DATA_MARKER_SYNC_POINT: str = "sync"; break;
-- 
2.14.2

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

[FFmpeg-devel] [PATCH 5/9] avformat/fitsenc: validate input pixel format

[Timo Rothenpieler]

Fixes CID #1416961 and #1416962
---
 libavformat/fitsenc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/fitsenc.c b/libavformat/fitsenc.c
index 7cb171596c..91411f2606 100644
--- a/libavformat/fitsenc.c
+++ b/libavformat/fitsenc.c
@@ -106,6 +106,8 @@ static int write_image_header(AVFormatContext *s)
 }
 bzero = 32768;
 break;
+default:
+return AVERROR(EINVAL);
 }
 
 if (fitsctx->first_image) {
-- 
2.14.2

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

Re: [FFmpeg-devel] Added HW H.264 and HEVC encoding for AMD GPUs based on AMF SDK

[Mironov, Mikhail]

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: November 8, 2017 10:15 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] Added HW H.264 and HEVC encoding for AMD
> GPUs based on AMF SDK
> 
> On 06/11/17 22:46, Michael Niedermayer wrote:
> > ...
> > In file included from src/libavcodec/amfenc.h:24:0,
> >  from src/libavcodec/amfenc.c:27:
> > src/compat/amd/amfsdkenc.h:191:23: error: no previous prototype for
> > ‘AMFConstructRect’ [-Werror=missing-prototypes]  AMF_INLINE struct
> AMFRect AMFConstructRect(amf_int32 left, amf_int32 top, amf_int32 right,
> amf_int32 bottom)
> >^
> > src/compat/amd/amfsdkenc.h:203:23: error: no previous prototype for
> > ‘AMFConstructSize’ [-Werror=missing-prototypes]  AMF_INLINE struct
> AMFSize AMFConstructSize(amf_int32 width, amf_int32 height)
> >^
> > src/compat/amd/amfsdkenc.h:215:24: error: no previous prototype for
> > ‘AMFConstructPoint’ [-Werror=missing-prototypes]  AMF_INLINE struct
> AMFPoint AMFConstructPoint(amf_int32 x, amf_int32 y)
> > ^
> > src/compat/amd/amfsdkenc.h:227:23: error: no previous prototype for
> > ‘AMFConstructRate’ [-Werror=missing-prototypes]  AMF_INLINE struct
> AMFRate AMFConstructRate(amf_uint32 num, amf_uint32 den)
> >^
> > src/compat/amd/amfsdkenc.h:239:24: error: no previous prototype for
> > ‘AMFConstructRatio’ [-Werror=missing-prototypes]  AMF_INLINE struct
> AMFRatio AMFConstructRatio(amf_uint32 num, amf_uint32 den)
> > ^
> > In file included from src/libavcodec/amfenc.h:24:0,
> >  from src/libavcodec/amfenc.c:27:
> > src/compat/amd/amfsdkenc.h:275:24: error: no previous prototype for
> > ‘AMFConstructColor’ [-Werror=missing-prototypes]  AMF_INLINE struct
> AMFColor AMFConstructColor(amf_uint8 r, amf_uint8 g, amf_uint8 b,
> amf_uint8 a)
> > ^
> > In file included from src/libavcodec/amfenc.h:24:0,
> >  from src/libavcodec/amfenc.c:27:
> > src/compat/amd/amfsdkenc.h:293:45: error: no previous prototype for
> ‘amf_variant_alloc’ [-Werror=missing-prototypes]
> >  AMF_INLINE void* AMF_CDECL_CALL amf_variant_alloc(amf_size count)
> >  ^
> > src/compat/amd/amfsdkenc.h:297:44: error: no previous prototype for
> ‘amf_variant_free’ [-Werror=missing-prototypes]
> >  AMF_INLINE void AMF_CDECL_CALL amf_variant_free(void* ptr)
> > ^
> > cc1: some warnings being treated as errors
> > make: *** [libavcodec/amfenc.o] Error 1
> 
> This is showing an error in the AMF C API.
> 
> AMF_INLINE is defined at  LibrariesAndSDKs/AMF/blob/master/amf/public/include/core/Platform.h#L9
> 9> as just "__inline__" (i.e. "inline"), so these functions are all inline 
> without
> a storage class specifier.  But, no external definition actually exists 
> anywhere,
> so if the compiler doesn't inline all instances you will fail to link.
> 
> (The behaviour is not the same in C++, where external versions are
> generated anyway in this case and then the multiple instances collapsed
> together later by invoking the ODR.)
> 
> Trivial test case (gcc defaults to not inlining anything when no optimisation
> options are given):
> 
> $ cat amf-inline.c
> #include "Platform.h"
> 
> int main(void)
> {
> AMFSize size;
> size = AMFConstructSize(123, 456);
> return 0;
> }
> $ gcc -std=c99 amf-inline.c
> In file included from amf-inline.c:1:0:
> Platform.h: In function ‘AMFCompareGUIDs’:
> Platform.h:421:16: warning: implicit declaration of function ‘memcmp’ [-
> Wimplicit-function-declaration]
>  return memcmp(&guid1, &guid2, sizeof(guid1)) == 0;
> ^
> /tmp/ccMCUBTW.o: In function `main':
> amf-inline.c:(.text+0x13): undefined reference to `AMFConstructSize'
> collect2: error: ld returned 1 exit status $ gcc -c -std=c99 amf-inline.c In 
> file
> included from amf-inline.c:1:0:
> Platform.h: In function ‘AMFCompareGUIDs’:
> Platform.h:421:16: warning: implicit declaration of function ‘memcmp’ [-
> Wimplicit-function-declaration]
>  return memcmp(&guid1, &guid2, sizeof(guid1)) == 0;
> ^
> $ nm amf-inline.o
>  U AMFConstructSize
>  T main
> $ gcc -O1 -std=c99 amf-inline.c
> In file included from amf-inline.c:1:0:
> Platform.h: In function ‘AMFCompareGUIDs’:
> Platform.h:421:16: warning: implicit declaration of function ‘memcmp’ [-
> Wimplicit-function-declaration]
>  return memcmp(&guid1, &guid2, sizeof(guid1)) == 0;
> ^
> $
> 
> I think AMF_INLINE should for these functions instead be "static inline"
> (though I didn't look at all uses, so there may be others where this isn't 
> true).


Yes, this is the issue. I found that in most of the functions "static" was 
added 
bu

Re: [FFmpeg-devel] [PATCH 8/9] movenc-test: fix potential uninitialized read

[James Almer]

On 11/8/2017 3:17 PM, Timo Rothenpieler wrote:
> Fixes CID #1413023
> ---
>  libavformat/tests/movenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/tests/movenc.c b/libavformat/tests/movenc.c
> index 8e59b74259..a210aec7ba 100644
> --- a/libavformat/tests/movenc.c
> +++ b/libavformat/tests/movenc.c
> @@ -108,7 +108,7 @@ static int io_write_data_type(void *opaque, uint8_t *buf, 
> int size,
>enum AVIODataMarkerType type, int64_t time)
>  {
>  char timebuf[30], content[5] = { 0 };
> -const char *str;
> +const char *str = "unknown";

Maybe make it a default case inside the switch statement instead?

>  switch (type) {
>  case AVIO_DATA_MARKER_HEADER: str = "header";   break;
>  case AVIO_DATA_MARKER_SYNC_POINT: str = "sync"; break;
> 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 6/9] avfilter/vf_premultiply: make sure plane count is less than 4

[Paul B Mahol]

On 11/8/17, Timo Rothenpieler  wrote:
> Fixes CID #1416350, #1416351, #1416353 and #1416354
> ---
>  libavfilter/vf_premultiply.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libavfilter/vf_premultiply.c b/libavfilter/vf_premultiply.c
> index 5120adc476..1af6433bc9 100644
> --- a/libavfilter/vf_premultiply.c
> +++ b/libavfilter/vf_premultiply.c
> @@ -476,7 +476,7 @@ static int filter_frame(AVFilterContext *ctx,
>  }
>  }
>
> -for (p = 0; p < s->nb_planes; p++) {
> +for (p = 0; p < s->nb_planes && p < 4; p++) {
>  if (!((1 << p) & s->planes) || p == 3) {
>  av_image_copy_plane((*out)->data[p], (*out)->linesize[p],
> base->data[p], base->linesize[p],
>  s->linesize[p], s->height[p]);
> @@ -523,6 +523,8 @@ static int config_input(AVFilterLink *inlink)
>  int vsub, hsub, ret;
>
>  s->nb_planes = av_pix_fmt_count_planes(inlink->format);
> +if (s->nb_planes > 4)
> +return AVERROR_BUG;
>
>  if ((ret = av_image_fill_linesizes(s->linesize, inlink->format,
> inlink->w)) < 0)
>  return ret;
> --
> 2.14.2
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

How could this ever happen?

Also, if you committ this bunch of other filters would need same change.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 5/9] avformat/fitsenc: validate input pixel format

[Paul B Mahol]

On 11/8/17, Timo Rothenpieler  wrote:
> Fixes CID #1416961 and #1416962
> ---
>  libavformat/fitsenc.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavformat/fitsenc.c b/libavformat/fitsenc.c
> index 7cb171596c..91411f2606 100644
> --- a/libavformat/fitsenc.c
> +++ b/libavformat/fitsenc.c
> @@ -106,6 +106,8 @@ static int write_image_header(AVFormatContext *s)
>  }
>  bzero = 32768;
>  break;
> +default:
> +return AVERROR(EINVAL);
>  }
>
>  if (fitsctx->first_image) {
> --
> 2.14.2
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

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

Re: [FFmpeg-devel] [PATCH 7/9] avfilter/vf_premultiply: fix memory-leak on failure

[Paul B Mahol]

On 11/8/17, Timo Rothenpieler  wrote:
> Fixes CID #1416352
> ---
>  libavfilter/vf_premultiply.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libavfilter/vf_premultiply.c b/libavfilter/vf_premultiply.c
> index 1af6433bc9..4b6abc6e73 100644
> --- a/libavfilter/vf_premultiply.c
> +++ b/libavfilter/vf_premultiply.c
> @@ -609,9 +609,10 @@ static int activate(AVFilterContext *ctx)
>  int64_t pts;
>
>  if ((ret = ff_inlink_consume_frame(ctx->inputs[0], &frame)) > 0) {
> -if ((ret = filter_frame(ctx, &out, frame, frame)) < 0)
> -return ret;
> +ret = filter_frame(ctx, &out, frame, frame);
>  av_frame_free(&frame);
> +if (ret < 0)
> +return ret;
>  ret = ff_filter_frame(ctx->outputs[0], out);
>  }
>  if (ret < 0) {
> --
> 2.14.2
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

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

Re: [FFmpeg-devel] [PATCH 4/9] avfilter/af_haas: validate par_m_source parameter

[Paul B Mahol]

On 11/8/17, Timo Rothenpieler  wrote:
> Fixes CID #1417663
> ---
>  libavfilter/af_haas.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/libavfilter/af_haas.c b/libavfilter/af_haas.c
> index 691c251f54..fac4b6cf2f 100644
> --- a/libavfilter/af_haas.c
> +++ b/libavfilter/af_haas.c
> @@ -162,6 +162,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *in)
>  case 1: mid = src[1]; break;
>  case 2: mid = (src[0] + src[1]) * 0.5; break;
>  case 3: mid = (src[0] - src[1]) * 0.5; break;
> +default: return AVERROR(EINVAL);
>  }
>
>  mid *= level_in;
> --
> 2.14.2
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

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

Re: [FFmpeg-devel] [PATCH] avfilter: initial macroblock types export and visualization

[Paul B Mahol]

On 11/8/17, Michael Niedermayer  wrote:
> On Wed, Nov 08, 2017 at 12:18:24PM +0100, Paul B Mahol wrote:
>> On 11/8/17, Michael Niedermayer  wrote:
>> > On Thu, Nov 02, 2017 at 12:04:17PM -0400, Ronald S. Bultje wrote:
>> >> Hi,
>> >>
>> >> On Thu, Nov 2, 2017 at 7:52 AM, Paul B Mahol  wrote:
>> >>
>> >> > On 11/2/17, Michael Niedermayer  wrote:
>> >> > > Hi
>> >> > >
>> >> > > On Sat, Oct 28, 2017 at 07:43:05AM -0400, Ronald S. Bultje wrote:
>> >> > >> Hi,
>> >> > >>
>> >> > >> On Fri, Oct 27, 2017 at 10:14 PM, Michael Niedermayer <
>> >> > >> mich...@niedermayer.cc> wrote:
>> >> > >>
>> >> > >> > On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote:
>> >> > >> > > Signed-off-by: Paul B Mahol 
>> >> > >> > > ---
>> >> > >> > >  libavcodec/mpegvideo.c |  10 +
>> >> > >> > >  libavfilter/vf_codecview.c | 105
>> >> > >> > > ++
>> >> > >> > +++
>> >> > >> > >  libavutil/frame.h  |   4 ++
>> >> > >> > >  3 files changed, 119 insertions(+)
>> >> > >> >
>> >> > >> > First, thanks for working on this.
>> >> > >> >
>> >> > >> >
>> >> > >> > [...]
>> >> > >> >
>> >> > >> > > diff --git a/libavutil/frame.h b/libavutil/frame.h
>> >> > >> > > index fef558ea2f..8481dc080b 100644
>> >> > >> > > --- a/libavutil/frame.h
>> >> > >> > > +++ b/libavutil/frame.h
>> >> > >> > > @@ -141,6 +141,10 @@ enum AVFrameSideDataType {
>> >> > >> > >   * metadata key entry "name".
>> >> > >> > >   */
>> >> > >> > >  AV_FRAME_DATA_ICC_PROFILE,
>> >> > >> > > +/**
>> >> > >> > > + * Macroblock types exported by some codecs.
>> >> > >> > > + */
>> >> > >> > > +AV_FRAME_DATA_MACROBLOCK_TYPES,
>> >> > >> > >  };
>> >> > >> > >
>> >> > >> >
>> >> > >> > This makes the internal data of the decoder part of the ABI and
>> >> > >> > API
>> >> > >> > of
>> >> > >> > libavcodec and libavfilter
>> >> > >> > and its undocumented
>> >> > >> >
>> >> > >> > do people prefer to make the internal data part of the ABI,
>> >> > >> > document
>> >> > >> > it and ensure it does not change till the next bump
>> >> > >> >
>> >> > >> [..]
>> >> > >>
>> >> > >> > or is there some other option iam missing ?
>> >> > >> >
>> >> > >>
>> >> > >> I noted this on IRC also. A third option is to not document it
>> >> > >> and
>> >> > >> consider
>> >> > >> it codec-specific.
>> >> > >>
>> >> > >> (Codec-specific implies "ABI/API unstable" and subject to change
>> >> > >> -
>> >> > "don't
>> >> > >> mix versions".)
>> >> > >
>> >> > > If you say the interface is "ABI/API unstable" then we cannot use
>> >> > > it
>> >> > > from another lib like libavfilter. So this would not work.
>> >> >
>> >> > I disagree.
>> >>
>> >>
>> >> I'd like to speak in solutions, not problems. We can simply prepend a
>> >> version number to the flags and ensure versions match. That makes it
>> >> easy
>> >> to break "ABI" (and if versions don't match, simply display an error
>> >> message and tell the user to not mix versions since this is considered
>> >> an
>> >> unstable feature).
>> >>
>> >> By no means can we set the current ABI of this feature in stone, we're
>> >> trying to fix it, not keep it. I appreciate Paul's effort to moving
>> >> this
>> >> forward and think his work pushes it in the right direction.
>> >
>> > How does this "versioned", "codec specific" API/ABI fit together with
>> > the
>> > carefully designed strict API used by the same filter to access motion
>> > vectors (libavutil/motion_vector.h)
>> > ?
>>
>> If you do not like it as it is, write specification which I will follow.
>
> I can. Is there some preferrances that people have ?
> this can be done in a wide range from simple to quite complex.
> from a simple bitmask in a byte or int array to a tree structure
> describing subdivisions or a list of structs similar to
> libavutil/motion_vector.h.
> The simple one will not be able to represent everything

Just one that gonna be used by single codec.
More complex one can be added when needed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Nicolas George]

L'octidi 18 brumaire, an CCXXVI, Paul B Mahol a écrit :
> Why are you doing this? Your bikesheds do not apply anymore.

I am "doing this" because I care about the quality and design of the
API, no more. In this particular instance, the side data API is now
completely braindead, a huge waste in terms of code complexity for no
actual benefit. It needs to be phased out, not expanded.

> You just like to make your opinions important to block other devs hard work.

I am not the one who introduced a veto in this discussion, you did.
Withdraw it and we will be able to discuss with arguments.

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 5/9] avformat/fitsenc: validate input pixel format

[Michael Niedermayer]

On Wed, Nov 08, 2017 at 07:17:49PM +0100, Timo Rothenpieler wrote:
> Fixes CID #1416961 and #1416962
> ---
>  libavformat/fitsenc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavformat/fitsenc.c b/libavformat/fitsenc.c
> index 7cb171596c..91411f2606 100644
> --- a/libavformat/fitsenc.c
> +++ b/libavformat/fitsenc.c
> @@ -106,6 +106,8 @@ static int write_image_header(AVFormatContext *s)
>  }
>  bzero = 32768;
>  break;
> +default:
> +return AVERROR(EINVAL);
>  }
>  
>  if (fitsctx->first_image) {

this is not sufficient
the return value of write_image_header() is never checked


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

"Nothing to hide" only works if the folks in power share the values of
you and everyone you know entirely and always will -- Tom Scott



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

Re: [FFmpeg-devel] [PATCH 4/9] avfilter/af_haas: validate par_m_source parameter

[Michael Niedermayer]

On Wed, Nov 08, 2017 at 07:17:48PM +0100, Timo Rothenpieler wrote:
> Fixes CID #1417663
> ---
>  libavfilter/af_haas.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavfilter/af_haas.c b/libavfilter/af_haas.c
> index 691c251f54..fac4b6cf2f 100644
> --- a/libavfilter/af_haas.c
> +++ b/libavfilter/af_haas.c
> @@ -162,6 +162,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>  case 1: mid = src[1]; break;
>  case 2: mid = (src[0] + src[1]) * 0.5; break;
>  case 3: mid = (src[0] - src[1]) * 0.5; break;
> +default: return AVERROR(EINVAL);
>  }

This condition is impossible, which makes this a false positive and
it should be marked as such.
you can add a av_assert if you want to ensure future changes to the
code do not add a codepath that can trigger this

the return is unreachable and the next generation of static analyzer
will flag that as dead code

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle


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

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Paul B Mahol]

On 11/8/17, Nicolas George  wrote:
> L'octidi 18 brumaire, an CCXXVI, Paul B Mahol a ecrit :
>> Why are you doing this? Your bikesheds do not apply anymore.
>
> I am "doing this" because I care about the quality and design of the
> API, no more. In this particular instance, the side data API is now
> completely braindead, a huge waste in terms of code complexity for no
> actual benefit. It needs to be phased out, not expanded.

If you want one big 2mb size AVFrame go ahead and fork FFmpeg.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Hendrik Leppkes]

On Wed, Nov 8, 2017 at 8:31 PM, Nicolas George  wrote:
> L'octidi 18 brumaire, an CCXXVI, Paul B Mahol a écrit :
>> Why are you doing this? Your bikesheds do not apply anymore.
>
> I am "doing this" because I care about the quality and design of the
> API, no more. In this particular instance, the side data API is now
> completely braindead, a huge waste in terms of code complexity for no
> actual benefit. It needs to be phased out, not expanded.
>

If you want to discuss and possibly replace the frame sidedata API,
then start such a discussion independent of a small feature patch.
In the meantime, it does no real harm to add one more sidedata type to
the numerous list of types we already have, and lets people carry on
with their work.

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

Re: [FFmpeg-devel] [PATCH] avcodec/cngdec: Fix integer clipping

[Michael Niedermayer]

On Thu, Nov 02, 2017 at 06:34:09PM +0100, Michael Niedermayer wrote:
> Fixes: runtime error: value -36211.7 is outside the range of representable 
> values of type 'short'
> Fixes: 2992/clusterfuzz-testcase-6649611793989632
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/cngdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

applied


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

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle


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

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Nicolas George]

L'octidi 18 brumaire, an CCXXVI, Hendrik Leppkes a écrit :
> In the meantime, it does no real harm to add one more sidedata type to
> the numerous list of types we already have, and lets people carry on
> with their work.

It does harm, look at the other patch: 5 lines of useless clutter out of
10 lines for the feature, using side data for that features has a 100%
overhead.

And that is not only for the code here, it has the same consequences for
the applications that will use that API as well: they have to do the
same errors checks, they have to handle a type-pruned pointer, check the
size of the data, etc. This is a terrible API, there is no doubt that
people would prefer a single field.

By all means let us discuss all this. But if Paul maintains his veto, I
maintain mine. Please persuade Paul to behave like an adult of make good
on his trice-repeated promise of forking.

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 v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Hendrik Leppkes]

On Wed, Nov 8, 2017 at 9:36 PM, Nicolas George  wrote:
> This is a terrible API, there is no doubt that
> people would prefer a single field.
>

I don't. We've done a lot of work to remove fields from generic data
structures which are used by single codecs on semi-obscure cases. The
fact alone that after years of avcodec and a PNG decoder this field
wasn't asked for regularly alone speaks to me that its not generic
enough to warrant inclusion in AVFrame.

Certainly it would make for nicer code if you could access every field
directly, but if we do that for every piece of data provided by all
random single decoders, we have a nightmare of a frame structure, with
hundreds of fields that only ever are applicable in a minority of
cases.

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

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Paul B Mahol]

On 11/8/17, Nicolas George  wrote:
> L'octidi 18 brumaire, an CCXXVI, Hendrik Leppkes a ecrit :
>> In the meantime, it does no real harm to add one more sidedata type to
>> the numerous list of types we already have, and lets people carry on
>> with their work.
>
> It does harm, look at the other patch: 5 lines of useless clutter out of
> 10 lines for the feature, using side data for that features has a 100%
> overhead.
>
> And that is not only for the code here, it has the same consequences for
> the applications that will use that API as well: they have to do the
> same errors checks, they have to handle a type-pruned pointer, check the
> size of the data, etc. This is a terrible API, there is no doubt that
> people would prefer a single field.
>
> By all means let us discuss all this. But if Paul maintains his veto, I
> maintain mine. Please persuade Paul to behave like an adult of make good
> on his trice-repeated promise of forking.

Maybe I promised a fork, but looks like people are already forking FFmpeg
because of people like you, latest one being ffmpeg-mpv.

There should be new call for voting commitee to drop you, Michael and Carl
from veto voting commitee. You guys should not block others work.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Rostislav Pehlivanov]

On 8 November 2017 at 20:52, Paul B Mahol  wrote:

> On 11/8/17, Nicolas George  wrote:
> > L'octidi 18 brumaire, an CCXXVI, Hendrik Leppkes a ecrit :
> >> In the meantime, it does no real harm to add one more sidedata type to
> >> the numerous list of types we already have, and lets people carry on
> >> with their work.
> >
> > It does harm, look at the other patch: 5 lines of useless clutter out of
> > 10 lines for the feature, using side data for that features has a 100%
> > overhead.
> >
> > And that is not only for the code here, it has the same consequences for
> > the applications that will use that API as well: they have to do the
> > same errors checks, they have to handle a type-pruned pointer, check the
> > size of the data, etc. This is a terrible API, there is no doubt that
> > people would prefer a single field.
> >
> > By all means let us discuss all this. But if Paul maintains his veto, I
> > maintain mine. Please persuade Paul to behave like an adult of make good
> > on his trice-repeated promise of forking.
>
> Maybe I promised a fork, but looks like people are already forking FFmpeg
> because of people like you, latest one being ffmpeg-mpv.
>
> There should be new call for voting commitee to drop you, Michael and Carl
> from veto voting commitee. You guys should not block others work.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Chill, different opinions important.
I don't particularly care about the patch really, its only exposed by png
and never used outside of joke images.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[James Almer]

On 11/8/2017 5:52 PM, Paul B Mahol wrote:
> On 11/8/17, Nicolas George  wrote:
>> L'octidi 18 brumaire, an CCXXVI, Hendrik Leppkes a ecrit :
>>> In the meantime, it does no real harm to add one more sidedata type to
>>> the numerous list of types we already have, and lets people carry on
>>> with their work.
>>
>> It does harm, look at the other patch: 5 lines of useless clutter out of
>> 10 lines for the feature, using side data for that features has a 100%
>> overhead.
>>
>> And that is not only for the code here, it has the same consequences for
>> the applications that will use that API as well: they have to do the
>> same errors checks, they have to handle a type-pruned pointer, check the
>> size of the data, etc. This is a terrible API, there is no doubt that
>> people would prefer a single field.
>>
>> By all means let us discuss all this. But if Paul maintains his veto, I
>> maintain mine. Please persuade Paul to behave like an adult of make good
>> on his trice-repeated promise of forking.
> 
> Maybe I promised a fork, but looks like people are already forking FFmpeg
> because of people like you, latest one being ffmpeg-mpv.
> 
> There should be new call for voting commitee to drop you, Michael and Carl
> from veto voting commitee. You guys should not block others work.

Please, everyone, it's a PNG patch. Why are any of you even remotely ok
with turning it into a project flamewar?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Nicolas George]

L'octidi 18 brumaire, an CCXXVI, Hendrik Leppkes a écrit :
> I don't. We've done a lot of work to remove fields from generic data
> structures which are used by single codecs on semi-obscure cases. The
> fact alone that after years of avcodec and a PNG decoder this field
> wasn't asked for regularly alone speaks to me that its not generic
> enough to warrant inclusion in AVFrame.
> 
> Certainly it would make for nicer code if you could access every field
> directly, but if we do that for every piece of data provided by all
> random single decoders, we have a nightmare of a frame structure, with
> hundreds of fields that only ever are applicable in a minority of
> cases.

This is a "slippery slope" kind of argument, and they are rarely valid.
Right now, we have only 16 frame side data types, and the conditions to
accept them were less stringent that they would be for fields in the
frame. Your "hundreds of fields" is quite an exaggeration.

The question we should be asking is: does it make sense, in general, for
any frame (or any video frame, or...)? For this field, I would argue
that the answer is yes: the gamma value is needed to make a few
operations on frames really correct. You can for example observe that
libswscale can do gamma-correct scaling, but it hardcodes 2.2.

Furthermore, I am quite doubtful about another side of the argument: you
complain about "hundreds of fields" that make the AVFrame structure a
"nightmare", but please explain to me how it is worse a nightmare than
having "hundreds of" side data types defined in the big enum just a few
lines above AVFrame. The only difference I can see is the size of the
structure. I grant you that for really hundreds of fields it would
indeed be a concern. But not for sixteen. Apart from that, the problem
is about the number of features that a developers needs to know, and a
side data type counts for that exactly as much as a field in AVFrame.

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] doc/developer: update style guidelines to include for loops with declarations

[Rostislav Pehlivanov]

Signed-off-by: Rostislav Pehlivanov 
---
 doc/developer.texi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/doc/developer.texi b/doc/developer.texi
index a7b4f1d737..de7d887451 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -132,6 +132,9 @@ designated struct initializers (@samp{struct s x = @{ .i = 
17 @};});
 @item
 compound literals (@samp{x = (struct s) @{ 17, 23 @};}).
 
+@item
+for loops with variable definition (@samp{for (int i = 0; i < 8; i++)});
+
 @item
 Implementation defined behavior for signed integers is assumed to match the
 expected behavior for two's complement. Non representable values in integer
-- 
2.15.0.403.gc27cc4dac6

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

Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

[Carl Eugen Hoyos]

2017-11-08 22:26 GMT+01:00 Rostislav Pehlivanov :

> +@item
> +for loops with variable definition (@samp{for (int i = 0; i < 8; i++)});

Don't you think that this makes the code slightly uglier?

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

[FFmpeg-devel] [PATCH]lavf/dashdec: Fix several memleaks

[Carl Eugen Hoyos]

Hi!

Attached patch fixes several memleaks found testing ticket #6820 (I
cannot reproduce the crash).

Please comment, Carl Eugen
From 915eae44e2f9bd7fb5ae78aa697338f348db3e08 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Wed, 8 Nov 2017 22:44:06 +0100
Subject: [PATCH] lavf/dashdec: Fix several memleaks.

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

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index f63f1ff..bc20fb7 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -328,17 +328,17 @@ static void free_representation(struct representation *pls)
 }
 
 av_freep(&pls->url_template);
-av_freep(pls);
+av_freep(&pls);
 }
 
-static void set_httpheader_options(DASHContext *c, AVDictionary *opts)
+static void set_httpheader_options(DASHContext *c, AVDictionary **opts)
 {
 // broker prior HTTP options that should be consistent across requests
-av_dict_set(&opts, "user-agent", c->user_agent, 0);
-av_dict_set(&opts, "cookies", c->cookies, 0);
-av_dict_set(&opts, "headers", c->headers, 0);
+av_dict_set(opts, "user-agent", c->user_agent, 0);
+av_dict_set(opts, "cookies", c->cookies, 0);
+av_dict_set(opts, "headers", c->headers, 0);
 if (c->is_live) {
-av_dict_set(&opts, "seekable", "0", 0);
+av_dict_set(opts, "seekable", "0", 0);
 }
 }
 static void update_options(char **dest, const char *name, void *src)
@@ -885,7 +885,7 @@ static int parse_manifest(AVFormatContext *s, const char *url, AVIOContext *in)
 if (!in) {
 close_in = 1;
 
-set_httpheader_options(c, opts);
+set_httpheader_options(c, &opts);
 ret = avio_open2(&in, url, AVIO_FLAG_READ, c->interrupt_callback, &opts);
 av_dict_free(&opts);
 if (ret < 0)
@@ -1302,7 +1302,7 @@ static int open_input(DASHContext *c, struct representation *pls, struct fragmen
 char url[MAX_URL_SIZE];
 int ret;
 
-set_httpheader_options(c, opts);
+set_httpheader_options(c, &opts);
 if (seg->size >= 0) {
 /* try to restrict the HTTP request to the part we want
  * (if this is in fact a HTTP request) */
@@ -1466,8 +1466,12 @@ static int save_avio_options(AVFormatContext *s)
 if (av_opt_get(s->pb, *opt, AV_OPT_SEARCH_CHILDREN, &buf) >= 0) {
 if (buf[0] != '\0') {
 ret = av_dict_set(&c->avio_opts, *opt, buf, AV_DICT_DONT_STRDUP_VAL);
-if (ret < 0)
+if (ret < 0) {
+av_freep(&buf);
 return ret;
+}
+} else {
+av_freep(&buf);
 }
 }
 opt++;
-- 
1.7.10.4

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

Re: [FFmpeg-devel] [PATCH]lavfi/scale2ref: Set output frame rate to main input frame rate

[Carl Eugen Hoyos]

2017-11-08 18:43 GMT+01:00 Michael Niedermayer :
> On Wed, Nov 08, 2017 at 09:23:58AM +0100, Carl Eugen Hoyos wrote:
>> Hi!
>>
>> I believe attached patch fixes ticket #6817.
>>
>> Please comment, Carl Eugen
>
>>  vf_scale.c |1 +
>>  1 file changed, 1 insertion(+)
>> cbc17115125ac2e6ada248ec53f973a81bfc762f  
>> 0001-lavfi-scale2ref-Set-output-frame-rate-to-main-input-.patch
>> From deee64102ef898b5f99974cf11bb05ec09963e90 Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos 
>> Date: Wed, 8 Nov 2017 09:15:29 +0100
>> Subject: [PATCH] lavfi/scale2ref: Set output frame rate to main input frame
>>  rate.
>>
>> Fixes ticket #6817.
>> ---
>>  libavfilter/vf_scale.c |1 +
>>  1 file changed, 1 insertion(+)
>
> LGTM

Patch applied.

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

Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

[Mark Thompson]

On 08/11/17 21:26, Rostislav Pehlivanov wrote:
> Signed-off-by: Rostislav Pehlivanov 
> ---
>  doc/developer.texi | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/doc/developer.texi b/doc/developer.texi
> index a7b4f1d737..de7d887451 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -132,6 +132,9 @@ designated struct initializers (@samp{struct s x = @{ .i 
> = 17 @};});
>  @item
>  compound literals (@samp{x = (struct s) @{ 17, 23 @};}).
>  
> +@item
> +for loops with variable definition (@samp{for (int i = 0; i < 8; i++)});
> +
>  @item
>  Implementation defined behavior for signed integers is assumed to match the
>  expected behavior for two's complement. Non representable values in integer
> 

IMO if you want this it would be better to just allow mixed statements and 
declarations, with this as a consequence.

Can you comment on what the consequences would be for platform support?  It 
would remove support for at least one platform I know of (the TI ARM compiler). 
 I've no idea whether it or any other platform which would be broken has any 
users, though.

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

Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

[Rostislav Pehlivanov]

On 8 November 2017 at 21:45, Carl Eugen Hoyos  wrote:

> 2017-11-08 22:26 GMT+01:00 Rostislav Pehlivanov :
>
> > +@item
> > +for loops with variable definition (@samp{for (int i = 0; i < 8; i++)});
>
> Don't you think that this makes the code slightly uglier?
>
> Carl Eugen
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>



Definitely not, and here are some reasons why, in order of importance:

1.) Gives a very very clear indication of whether a variable is used after
a for loop. Any loop without its variable defined means that variable is
used for something else so everyone would pay more attention.

2.) It saves a line in case the loop variable isn't used anywhere
2a.) This adds up and if all loops in our codebase were converted many 10s
of thousands of lines would be saved (I'm serious, at least this many
lines).

3.) Simplifies experimenting since you don't have to scroll up to add a
variable and then scroll down to use it in a loop.

4.) Most code editing (and general purpose text editing) software
highlights types, so on a glance there would be no doubt about which
bracketed segment is a loop and which is a condition.

5.) Compilers can better know whether to unroll loops in case the variable
isn't actually used in a small number of cases.

6.) Simple compilers would make less allocation/stack mistakes if a
variable is declared in the first loop in a two loop segment.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Added HW accelerated H.264 and HEVC encoding for AMD

[Carl Eugen Hoyos]

2017-11-08 19:25 GMT+01:00 mmironov :

> Changelog|3 +-

> diff --git a/Changelog b/Changelog
> index 6592d86..f0d22fa 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -6,7 +6,8 @@ version :
>  - Dropped support for OpenJPEG versions 2.0 and below. Using OpenJPEG now
>requires 2.1 (or later) and pkg-config.
>  - VDA dropped (use VideoToolbox instead)
> -
> +- AMF H.264 encoder
> +- AMF HEVC encoder

This patch makes three changes in the file "Changelog":
The first change is a removal of an empty line.
The second and third change are a new line each.
Please:
Do not remove the empty line, leave it unchanged.
Merge the two lines to something like "AMF H.264 and HEVC encoders"
(Feel free to add the word "AMD" to help people understand what AMF is.)

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

Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

[Carl Eugen Hoyos]

2017-11-08 22:49 GMT+01:00 Mark Thompson :
> On 08/11/17 21:26, Rostislav Pehlivanov wrote:

>> +@item
>> +for loops with variable definition (@samp{for (int i = 0; i < 8; i++)});

> Can you comment on what the consequences would be for platform support?

A similar-looking issue was reported for some version of msvc a short time ago.

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

Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

[Rostislav Pehlivanov]

On 8 November 2017 at 21:49, Mark Thompson  wrote:

> On 08/11/17 21:26, Rostislav Pehlivanov wrote:
> > Signed-off-by: Rostislav Pehlivanov 
> > ---
> >  doc/developer.texi | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index a7b4f1d737..de7d887451 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -132,6 +132,9 @@ designated struct initializers (@samp{struct s x =
> @{ .i = 17 @};});
> >  @item
> >  compound literals (@samp{x = (struct s) @{ 17, 23 @};}).
> >
> > +@item
> > +for loops with variable definition (@samp{for (int i = 0; i < 8; i++)});
> > +
> >  @item
> >  Implementation defined behavior for signed integers is assumed to match
> the
> >  expected behavior for two's complement. Non representable values in
> integer
> >
>
> IMO if you want this it would be better to just allow mixed statements and
> declarations, with this as a consequence.
>
> Can you comment on what the consequences would be for platform support?
> It would remove support for at least one platform I know of (the TI ARM
> compiler).  I've no idea whether it or any other platform which would be
> broken has any users, though.
>
> - Mark
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


No, I'm kind of against mixed statements and declarations, as are many
people here. It mostly does make the code look worse and encourages overuse
of variables.

I'm absoultely sure no compiler worth supporting would be broken. If any
+15 year old ones do get broken it would be well worth because it would
still ease maintaining a lot. I'm also quite sure no compiler that would be
broken by this would support compiling ffmpeg at all.
This is still an essential feature of C99 after all and we don't use C89.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Hendrik Leppkes]

On Wed, Nov 8, 2017 at 10:30 PM, Nicolas George  wrote:
>
> The question we should be asking is: does it make sense, in general, for
> any frame (or any video frame, or...)? For this field, I would argue
> that the answer is yes: the gamma value is needed to make a few
> operations on frames really correct. You can for example observe that
> libswscale can do gamma-correct scaling, but it hardcodes 2.2.
>

Gamma might sound useful of a property to have, but there is no actual
technical evidence to really support that. As said in an earlier mail,
we've gone for years without it and no-one missed it, and even now its
only being added for one decoder, and even there its apparently only
used for obscure files ("joke images", as the author called it).
In fact, for videos, the gamme is usually defined through the transfer
function, which has the color_trc field already.

> Furthermore, I am quite doubtful about another side of the argument: you
> complain about "hundreds of fields" that make the AVFrame structure a
> "nightmare", but please explain to me how it is worse a nightmare than
> having "hundreds of" side data types defined in the big enum just a few
> lines above AVFrame. The only difference I can see is the size of the
> structure. I grant you that for really hundreds of fields it would
> indeed be a concern. But not for sixteen. Apart from that, the problem
> is about the number of features that a developers needs to know, and a
> side data type counts for that exactly as much as a field in AVFrame.

I actually think that the separation into "core" data and "side" data
helps clarity. The AVFrame contains fields that most people should
probably look at, so they can read the struct and figure out what
fields may need handling. If that list includes all sorts of
properties with marginal value and rare/special usage, its easy to get
"lost" in there.

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

Re: [FFmpeg-devel] Added HW accelerated H.264 and HEVC encoding for AMD

[Mironov, Mikhail]

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Carl Eugen Hoyos
> Sent: November 8, 2017 5:01 PM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] Added HW accelerated H.264 and HEVC
> encoding for AMD
> 
> 2017-11-08 19:25 GMT+01:00 mmironov :
> 
> > Changelog|3 +-
> 
> > diff --git a/Changelog b/Changelog
> > index 6592d86..f0d22fa 100644
> > --- a/Changelog
> > +++ b/Changelog
> > @@ -6,7 +6,8 @@ version :
> >  - Dropped support for OpenJPEG versions 2.0 and below. Using OpenJPEG
> now
> >requires 2.1 (or later) and pkg-config.
> >  - VDA dropped (use VideoToolbox instead)
> > -
> > +- AMF H.264 encoder
> > +- AMF HEVC encoder
> 
> This patch makes three changes in the file "Changelog":
> The first change is a removal of an empty line.
> The second and third change are a new line each.
> Please:
> Do not remove the empty line, leave it unchanged.
> Merge the two lines to something like "AMF H.264 and HEVC encoders"
> (Feel free to add the word "AMD" to help people understand what AMF is.)

Thanks, got it. Will correct in the next submission.

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

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

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Nicolas George]

L'octidi 18 brumaire, an CCXXVI, Hendrik Leppkes a écrit :
> Gamma might sound useful of a property to have, but there is no actual
> technical evidence to really support that. As said in an earlier mail,
> we've gone for years without it and no-one missed it, and even now its
> only being added for one decoder, and even there its apparently only

We went for years with a channel count and no channel layout too.

> used for obscure files ("joke images", as the author called it).

This would be an argument to not include it at all.

> I actually think that the separation into "core" data and "side" data
> helps clarity. The AVFrame contains fields that most people should
> probably look at, so they can read the struct and figure out what
> fields may need handling. If that list includes all sorts of
> properties with marginal value and rare/special usage, its easy to get
> "lost" in there.

The same result can be achieved with a doxy group and corresponding
documentation.

Note: I maintain my veto as long as Paul maintains his, as a matter of
principle. But feel free to eventually push and disregard them, showing
that the vetoes were worthless in the first place and that Paul's was
just an intimidation tactic to close the discussion. It would make a
good precedent.

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] doc/developer: update style guidelines to include for loops with declarations

[Mark Thompson]

On 08/11/17 22:03, Rostislav Pehlivanov wrote:
> On 8 November 2017 at 21:49, Mark Thompson  wrote:
> 
>> On 08/11/17 21:26, Rostislav Pehlivanov wrote:
>>> Signed-off-by: Rostislav Pehlivanov 
>>> ---
>>>  doc/developer.texi | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/doc/developer.texi b/doc/developer.texi
>>> index a7b4f1d737..de7d887451 100644
>>> --- a/doc/developer.texi
>>> +++ b/doc/developer.texi
>>> @@ -132,6 +132,9 @@ designated struct initializers (@samp{struct s x =
>> @{ .i = 17 @};});
>>>  @item
>>>  compound literals (@samp{x = (struct s) @{ 17, 23 @};}).
>>>
>>> +@item
>>> +for loops with variable definition (@samp{for (int i = 0; i < 8; i++)});
>>> +
>>>  @item
>>>  Implementation defined behavior for signed integers is assumed to match
>> the
>>>  expected behavior for two's complement. Non representable values in
>> integer
>>>
>>
>> IMO if you want this it would be better to just allow mixed statements and
>> declarations, with this as a consequence.
>>
>> Can you comment on what the consequences would be for platform support?
>> It would remove support for at least one platform I know of (the TI ARM
>> compiler).  I've no idea whether it or any other platform which would be
>> broken has any users, though.
>>
> 
> No, I'm kind of against mixed statements and declarations, as are many
> people here. It mostly does make the code look worse and encourages overuse
> of variables.

I think the opposite.  I find declaration inside the loop header ugly and 
weird, while mixed declarations and code do make sense in some cases: e.g. 
pointer chasing through structures with different types - declaring all the 
variables in advance is just annoying.  (Maybe that's not strong enough to 
allow it everywhere if you believe that people will use it inappropriately 
though.)

> I'm absoultely sure no compiler worth supporting would be broken. If any
> +15 year old ones do get broken it would be well worth because it would
> still ease maintaining a lot. I'm also quite sure no compiler that would be
> broken by this would support compiling ffmpeg at all.
> This is still an essential feature of C99 after all and we don't use C89.
TI at least disagrees with you, releases are still made without full C99 
support.  I know it certainly has use on embedded platforms (though likely 
C6000 more so than ARM), but I don't know if anyone builds libavcodec or 
similar with it.  (I don't use it - I only know this because Diego recently 
asked if anyone could test whether it could build libav, and I verified that it 
could after fixing some minor issues.  He couldn't find any users, though, and 
the support was removed anyway.)

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

Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

[Rostislav Pehlivanov]

On 8 November 2017 at 22:20, Mark Thompson  wrote:

> On 08/11/17 22:03, Rostislav Pehlivanov wrote:
> > On 8 November 2017 at 21:49, Mark Thompson  wrote:
> >
> >> On 08/11/17 21:26, Rostislav Pehlivanov wrote:
> >>> Signed-off-by: Rostislav Pehlivanov 
> >>> ---
> >>>  doc/developer.texi | 3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/doc/developer.texi b/doc/developer.texi
> >>> index a7b4f1d737..de7d887451 100644
> >>> --- a/doc/developer.texi
> >>> +++ b/doc/developer.texi
> >>> @@ -132,6 +132,9 @@ designated struct initializers (@samp{struct s x =
> >> @{ .i = 17 @};});
> >>>  @item
> >>>  compound literals (@samp{x = (struct s) @{ 17, 23 @};}).
> >>>
> >>> +@item
> >>> +for loops with variable definition (@samp{for (int i = 0; i < 8;
> i++)});
> >>> +
> >>>  @item
> >>>  Implementation defined behavior for signed integers is assumed to
> match
> >> the
> >>>  expected behavior for two's complement. Non representable values in
> >> integer
> >>>
> >>
> >> IMO if you want this it would be better to just allow mixed statements
> and
> >> declarations, with this as a consequence.
> >>
> >> Can you comment on what the consequences would be for platform support?
> >> It would remove support for at least one platform I know of (the TI ARM
> >> compiler).  I've no idea whether it or any other platform which would be
> >> broken has any users, though.
> >>
> >
> > No, I'm kind of against mixed statements and declarations, as are many
> > people here. It mostly does make the code look worse and encourages
> overuse
> > of variables.
>
> I think the opposite.  I find declaration inside the loop header ugly and
> weird, while mixed declarations and code do make sense in some cases: e.g.
> pointer chasing through structures with different types - declaring all the
> variables in advance is just annoying.  (Maybe that's not strong enough to
> allow it everywhere if you believe that people will use it inappropriately
> though.)
>
>
I'm pretty sure its because you're not used to them yet. I'm not taking
this as a nak.
If you want mixed declaration submit a patch later on and let people
comment on it.




> > I'm absoultely sure no compiler worth supporting would be broken. If any
> > +15 year old ones do get broken it would be well worth because it would
> > still ease maintaining a lot. I'm also quite sure no compiler that would
> be
> > broken by this would support compiling ffmpeg at all.
> > This is still an essential feature of C99 after all and we don't use C89.
> TI at least disagrees with you, releases are still made without full C99
> support.  I know it certainly has use on embedded platforms (though likely
> C6000 more so than ARM), but I don't know if anyone builds libavcodec or
> similar with it.  (I don't use it - I only know this because Diego recently
> asked if anyone could test whether it could build libav, and I verified
> that it could after fixing some minor issues.  He couldn't find any users,
> though, and the support was removed anyway.)
>
>
If you're not aware of anyone compiling ffmpeg with it then don't even
bother mentioning it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] aptx: implement the aptX bluetooth codec

[Aurelien Jacobs]

On Wed, Nov 08, 2017 at 06:26:03PM +0100, Michael Niedermayer wrote:
> On Wed, Nov 08, 2017 at 02:06:09PM +0100, Aurelien Jacobs wrote:
> [...]
> > +typedef const struct {
> > +const int32_t *quantize_intervals;
> > +const int32_t *invert_quantize_dither_factors;
> > +const int32_t *quantize_dither_factors;
> 
> > +const int32_t *quantize_factor_select_offset;
> 
> this would fit in int16_t *

Right.

> [...]
> > +static const int32_t quantization_factors[32] = {
> > +2048, 2093, 2139, 2186, 2233, 2282, 2332, 2383,
> > +2435, 2489, 2543, 2599, 2656, 2714, 2774, 2834,
> > +2896, 2960, 3025, 3091, 3158, 3228, 3298, 3371,
> > +3444, 3520, 3597, 3676, 3756, 3838, 3922, 4008,
> > +};
> 
> this too would fir in int16_t

Indeed, now that I moved the shift inside the code.

> [...]
> > +/*
> > + * Push one sample into a circular signal buffer.
> > + */
> > +av_always_inline
> > +static void aptx_qmf_filter_signal_push(FilterSignal *signal, int32_t 
> > sample)
> > +{
> > +signal->buffer[signal->pos] = sample;
> > +signal->buffer[signal->pos+FILTER_TAPS] = sample;
> > +signal->pos = (signal->pos + 1) % FILTER_TAPS;
> 
> % could be replaced by &

OK. And there was a second one that I changed as well.

> > +}
> > +
> > +/*
> > + * Compute the convolution of the signal with the coefficients, and reduce
> > + * to 24 bits by applying the specified right shifting.
> > + */
> > +av_always_inline
> > +static int32_t aptx_qmf_convolution(FilterSignal *signal,
> > +const int32_t coeffs[FILTER_TAPS],
> > +int shift)
> > +{
> > +int32_t *sig = &signal->buffer[signal->pos];
> > +int64_t e = 0;
> > +
> 
> > +for (int i = 0; i < FILTER_TAPS; i++)
> 
> "for (int" is something we avoided as some comilers didnt like it,
> iam not sure if this is still true but there are none in the codebase

If you insist on this I will of course change it, but hey, we require
a C99 compiler since a long time and we use so many features of C99
that I really don't get why we couldn't use "for (int".
I can't imagine that any relevant C compiler would not support this
nowadays !
What I propose is to get this in as is, and see if anyone encounter
issue with any compiler. If any issue arise, I will of course send a
patch to fix it.

Here is an updated patch.>From d7c728bdb2e78ccfb49a17e7fa75785e771480f6 Mon Sep 17 00:00:00 2001
From: Aurelien Jacobs 
Date: Thu, 31 Aug 2017 20:12:54 +0200
Subject: [PATCH 1/2] aptx: implement the aptX bluetooth codec

The encoder was reverse engineered from binary library and from
EP0398973B1 patent (long expired).
The decoder was simply deduced from the encoder.
---
 doc/general.texi|   2 +
 libavcodec/Makefile |   2 +
 libavcodec/allcodecs.c  |   1 +
 libavcodec/aptx.c   | 826 
 libavcodec/avcodec.h|   1 +
 libavcodec/codec_desc.c |   7 +
 6 files changed, 839 insertions(+)
 create mode 100644 libavcodec/aptx.c

diff --git a/doc/general.texi b/doc/general.texi
index e6ae277d23..de4efee913 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -993,6 +993,8 @@ following image formats are supported:
 @item Amazing Studio PAF Audio @tab @tab  X
 @item Apple lossless audio   @tab  X  @tab  X
 @tab QuickTime fourcc 'alac'
+@item aptX   @tab  X  @tab  X
+@tab Used in Bluetooth A2DP
 @item ATRAC1 @tab @tab  X
 @item ATRAC3 @tab @tab  X
 @item ATRAC3+@tab @tab  X
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 45f4db5939..95c843dee7 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -188,6 +188,8 @@ OBJS-$(CONFIG_AMV_ENCODER) += mjpegenc.o mjpegenc_common.o \
 OBJS-$(CONFIG_ANM_DECODER) += anm.o
 OBJS-$(CONFIG_ANSI_DECODER)+= ansi.o cga_data.o
 OBJS-$(CONFIG_APE_DECODER) += apedec.o
+OBJS-$(CONFIG_APTX_DECODER)+= aptx.o
+OBJS-$(CONFIG_APTX_ENCODER)+= aptx.o
 OBJS-$(CONFIG_APNG_DECODER)+= png.o pngdec.o pngdsp.o
 OBJS-$(CONFIG_APNG_ENCODER)+= png.o pngenc.o
 OBJS-$(CONFIG_SSA_DECODER) += assdec.o ass.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index d96e499ba7..463f7ed64e 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -406,6 +406,7 @@ static void register_all(void)
 REGISTER_DECODER(AMRNB, amrnb);
 REGISTER_DECODER(AMRWB, amrwb);
 REGISTER_DECODER(APE,   ape);
+REGISTER_ENCDEC (APTX,  aptx);
 REGISTER_DECODER(ATRAC1,atrac1);
 REGISTER_DECODER(ATRAC3,atrac3);
 REGISTER_DECODER(ATRAC3AL,  atrac3al);
diff --git a/libavcodec/aptx.c b/libavcodec/aptx.c
new file mode 100644
index 00..392af2eb31
--- /dev/null
+++ b/libavcodec/aptx.c
@@ -0,0 +1,826 @@
+/*
+ * Aud

Re: [FFmpeg-devel] [PATCH 2/2] aptx: add raw muxer and demuxer for aptX

[Aurelien Jacobs]

On Wed, Nov 08, 2017 at 05:24:34PM +, Rostislav Pehlivanov wrote:
> 
> [...]
> 
> Patch doesn't apply

Here is a rebased patch.>From f6d9a7a804bc1c833e7c2e61411ac1b9155cb6ba Mon Sep 17 00:00:00 2001
From: Aurelien Jacobs 
Date: Thu, 31 Aug 2017 20:42:15 +0200
Subject: [PATCH 2/2] aptx: add raw muxer and demuxer for aptX

---
 doc/general.texi |  1 +
 libavformat/Makefile |  2 ++
 libavformat/allformats.c |  1 +
 libavformat/aptxdec.c| 58 
 libavformat/rawenc.c | 13 +++
 libavformat/utils.c  |  1 +
 6 files changed, 76 insertions(+)
 create mode 100644 libavformat/aptxdec.c

diff --git a/doc/general.texi b/doc/general.texi
index de4efee913..efd4a92495 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -441,6 +441,7 @@ library:
 @item raw AC-3  @tab X @tab X
 @item raw AMR-NB@tab   @tab X
 @item raw AMR-WB@tab   @tab X
+@item raw aptX  @tab X @tab X
 @item raw Chinese AVS video @tab X @tab X
 @item raw CRI ADX   @tab X @tab X
 @item raw Dirac @tab X @tab X
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 146a4656f2..b1e7b193f4 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -94,6 +94,8 @@ OBJS-$(CONFIG_APC_DEMUXER)   += apc.o
 OBJS-$(CONFIG_APE_DEMUXER)   += ape.o apetag.o img2.o
 OBJS-$(CONFIG_APNG_DEMUXER)  += apngdec.o
 OBJS-$(CONFIG_APNG_MUXER)+= apngenc.o
+OBJS-$(CONFIG_APTX_DEMUXER)  += aptxdec.o rawdec.o
+OBJS-$(CONFIG_APTX_MUXER)+= rawenc.o
 OBJS-$(CONFIG_AQTITLE_DEMUXER)   += aqtitledec.o subtitles.o
 OBJS-$(CONFIG_ASF_DEMUXER)   += asfdec_f.o asf.o asfcrypt.o \
 avlanguage.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index 1896d50e9e..9213af9301 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -69,6 +69,7 @@ static void register_all(void)
 REGISTER_DEMUXER (APC,  apc);
 REGISTER_DEMUXER (APE,  ape);
 REGISTER_MUXDEMUX(APNG, apng);
+REGISTER_MUXDEMUX(APTX, aptx);
 REGISTER_DEMUXER (AQTITLE,  aqtitle);
 REGISTER_MUXDEMUX(ASF,  asf);
 REGISTER_DEMUXER (ASF_O,asf_o);
diff --git a/libavformat/aptxdec.c b/libavformat/aptxdec.c
new file mode 100644
index 00..90ce789454
--- /dev/null
+++ b/libavformat/aptxdec.c
@@ -0,0 +1,58 @@
+/*
+ * RAW aptX demuxer
+ *
+ * Copyright (C) 2017  Aurelien Jacobs 
+ *
+ * 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 "avformat.h"
+#include "rawdec.h"
+
+#define APTX_BLOCK_SIZE   4
+#define APTX_PACKET_SIZE  (256*APTX_BLOCK_SIZE)
+
+static int aptx_read_header(AVFormatContext *s)
+{
+AVStream *st = avformat_new_stream(s, NULL);
+if (!st)
+return AVERROR(ENOMEM);
+st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
+st->codecpar->codec_id = AV_CODEC_ID_APTX;
+st->codecpar->format = AV_SAMPLE_FMT_S32P;
+st->codecpar->channels = 2;
+st->codecpar->sample_rate = 44100;
+st->codecpar->bits_per_coded_sample = 4;
+st->codecpar->block_align = APTX_BLOCK_SIZE;
+st->codecpar->frame_size = APTX_PACKET_SIZE;
+st->start_time = 0;
+return 0;
+}
+
+static int aptx_read_packet(AVFormatContext *s, AVPacket *pkt)
+{
+return av_get_packet(s->pb, pkt, APTX_PACKET_SIZE);
+}
+
+AVInputFormat ff_aptx_demuxer = {
+.name   = "aptx",
+.long_name  = NULL_IF_CONFIG_SMALL("raw aptX"),
+.extensions = "aptx",
+.read_header= aptx_read_header,
+.read_packet= aptx_read_packet,
+.flags  = AVFMT_GENERIC_INDEX,
+};
diff --git a/libavformat/rawenc.c b/libavformat/rawenc.c
index f640121cb4..aa3ef76fbf 100644
--- a/libavformat/rawenc.c
+++ b/libavformat/rawenc.c
@@ -91,6 +91,19 @@ AVOutputFormat ff_adx_muxer = {
 };
 #endif
 
+#if CONFIG_APTX_MUXER
+AVOutputFormat ff_aptx_muxer = {
+.name  = "aptx",
+.long_name = NULL_IF_CONFIG_SMALL("raw aptX (Audio Processing Technology for Bluetooth)"),
+.extensions= "aptx",
+.audio_codec 

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[James Almer]

On 11/8/2017 7:15 PM, Nicolas George wrote:
> L'octidi 18 brumaire, an CCXXVI, Hendrik Leppkes a écrit :
>> Gamma might sound useful of a property to have, but there is no actual
>> technical evidence to really support that. As said in an earlier mail,
>> we've gone for years without it and no-one missed it, and even now its
>> only being added for one decoder, and even there its apparently only
> 
> We went for years with a channel count and no channel layout too.
> 
>> used for obscure files ("joke images", as the author called it).
> 
> This would be an argument to not include it at all.

Probably a good idea.

Maybe just add the gamma value as a frame AVDictionary metadata entry?
If the idea is that the API user can get a single value from the PNG
image in some form, wouldn't side data and a dict metadata entry work
the same? The latter is much cleaner and easier to remove if needed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avutil/frame: Add private_ref to AVFrame

[Michael Niedermayer]

This gives FFmpeg libs a field that they can freely and safely use.
Avoiding the need of wraping of a users opaque_ref field and its issues.

Signed-off-by: Michael Niedermayer 
---
 libavutil/frame.c |  8 +++-
 libavutil/frame.h | 12 
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index 982fbb5c81..662a7e5ab5 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -383,12 +383,17 @@ FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 
 av_buffer_unref(&dst->opaque_ref);
+av_buffer_unref(&dst->private_ref);
 if (src->opaque_ref) {
 dst->opaque_ref = av_buffer_ref(src->opaque_ref);
 if (!dst->opaque_ref)
 return AVERROR(ENOMEM);
 }
-
+if (src->private_ref) {
+dst->private_ref = av_buffer_ref(src->private_ref);
+if (!dst->private_ref)
+return AVERROR(ENOMEM);
+}
 return 0;
 }
 
@@ -524,6 +529,7 @@ void av_frame_unref(AVFrame *frame)
 av_buffer_unref(&frame->hw_frames_ctx);
 
 av_buffer_unref(&frame->opaque_ref);
+av_buffer_unref(&frame->private_ref);
 
 get_frame_defaults(frame);
 }
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 0c6aab1c02..7b9bec054a 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -563,6 +563,18 @@ typedef struct AVFrame {
 /**
  * @}
  */
+/**
+ * AVBufferRef for free use by libavcodec / libavfilte /  Code outside
+ * the FFmpeg libs will never
+ * check or change the contents of the buffer ref. FFmpeg calls
+ * av_buffer_unref() on it when the frame is unreferenced.
+ * av_frame_copy_props() calls create a new reference with av_buffer_ref()
+ * for the target frame's private_ref field.
+ *
+ * The field should be set to NULL by the FFmpeg libs before passing a 
frame
+ * to the user.
+ */
+AVBufferRef *private_ref;
 } AVFrame;
 
 #if FF_API_FRAME_GET_SET
-- 
2.15.0

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

Re: [FFmpeg-devel] [PATCH] avutil/frame: Add private_ref to AVFrame

[Rostislav Pehlivanov]

On 8 November 2017 at 22:55, Michael Niedermayer 
wrote:

> This gives FFmpeg libs a field that they can freely and safely use.
> Avoiding the need of wraping of a users opaque_ref field and its issues.
>
> Signed-off-by: Michael Niedermayer 
> ---
>  libavutil/frame.c |  8 +++-
>  libavutil/frame.h | 12 
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 982fbb5c81..662a7e5ab5 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -383,12 +383,17 @@ FF_ENABLE_DEPRECATION_WARNINGS
>  #endif
>
>  av_buffer_unref(&dst->opaque_ref);
> +av_buffer_unref(&dst->private_ref);
>  if (src->opaque_ref) {
>  dst->opaque_ref = av_buffer_ref(src->opaque_ref);
>  if (!dst->opaque_ref)
>  return AVERROR(ENOMEM);
>  }
> -
> +if (src->private_ref) {
> +dst->private_ref = av_buffer_ref(src->private_ref);
> +if (!dst->private_ref)
> +return AVERROR(ENOMEM);
> +}
>  return 0;
>  }
>
> @@ -524,6 +529,7 @@ void av_frame_unref(AVFrame *frame)
>  av_buffer_unref(&frame->hw_frames_ctx);
>
>  av_buffer_unref(&frame->opaque_ref);
> +av_buffer_unref(&frame->private_ref);
>
>  get_frame_defaults(frame);
>  }
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 0c6aab1c02..7b9bec054a 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -563,6 +563,18 @@ typedef struct AVFrame {
>  /**
>   * @}
>   */
> +/**
> + * AVBufferRef for free use by libavcodec / libavfilte /  Code
> outside
> + * the FFmpeg libs will never
> + * check or change the contents of the buffer ref. FFmpeg calls
> + * av_buffer_unref() on it when the frame is unreferenced.
> + * av_frame_copy_props() calls create a new reference with
> av_buffer_ref()
> + * for the target frame's private_ref field.
> + *
> + * The field should be set to NULL by the FFmpeg libs before passing
> a frame
> + * to the user.
> + */
> +AVBufferRef *private_ref;
>  } AVFrame;
>
>  #if FF_API_FRAME_GET_SET
> --
> 2.15.0
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


This makes it sound as if it would get used and can't be overwritten (e.g.
feeding frames from a decoder to an encoder and zeroing this field would
make things go bad).

Just say its temporary storage only and that it doesn't get used after
decoding or outputting.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

[James Almer]

On 11/8/2017 7:41 PM, Rostislav Pehlivanov wrote:
> On 8 November 2017 at 22:20, Mark Thompson  wrote:
> 
>> On 08/11/17 22:03, Rostislav Pehlivanov wrote:
>>> On 8 November 2017 at 21:49, Mark Thompson  wrote:
>>>
 On 08/11/17 21:26, Rostislav Pehlivanov wrote:
> Signed-off-by: Rostislav Pehlivanov 
> ---
>  doc/developer.texi | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index a7b4f1d737..de7d887451 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -132,6 +132,9 @@ designated struct initializers (@samp{struct s x =
 @{ .i = 17 @};});
>  @item
>  compound literals (@samp{x = (struct s) @{ 17, 23 @};}).
>
> +@item
> +for loops with variable definition (@samp{for (int i = 0; i < 8;
>> i++)});
> +
>  @item
>  Implementation defined behavior for signed integers is assumed to
>> match
 the
>  expected behavior for two's complement. Non representable values in
 integer
>

 IMO if you want this it would be better to just allow mixed statements
>> and
 declarations, with this as a consequence.

 Can you comment on what the consequences would be for platform support?
 It would remove support for at least one platform I know of (the TI ARM
 compiler).  I've no idea whether it or any other platform which would be
 broken has any users, though.

>>>
>>> No, I'm kind of against mixed statements and declarations, as are many
>>> people here. It mostly does make the code look worse and encourages
>> overuse
>>> of variables.
>>
>> I think the opposite.  I find declaration inside the loop header ugly and
>> weird, while mixed declarations and code do make sense in some cases: e.g.
>> pointer chasing through structures with different types - declaring all the
>> variables in advance is just annoying.  (Maybe that's not strong enough to
>> allow it everywhere if you believe that people will use it inappropriately
>> though.)
>>
>>
> I'm pretty sure its because you're not used to them yet. I'm not taking
> this as a nak.
> If you want mixed declaration submit a patch later on and let people
> comment on it.

It's the other way around. If you want to introduce some change, you're
the one that needs to convince other devs it's a good change, and so
far, two dislike it.
You can't commit this when people are against it saying "send a patch to
undo it later".

Besides, this patch alone is incomplete. Warnings about mixed code and
declaration are currently force enabled in configure.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

[Mark Thompson]

On 08/11/17 22:41, Rostislav Pehlivanov wrote:
> On 8 November 2017 at 22:20, Mark Thompson  wrote:
> 
>> On 08/11/17 22:03, Rostislav Pehlivanov wrote:
>>> On 8 November 2017 at 21:49, Mark Thompson  wrote:
>>>
 On 08/11/17 21:26, Rostislav Pehlivanov wrote:
> Signed-off-by: Rostislav Pehlivanov 
> ---
>  doc/developer.texi | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index a7b4f1d737..de7d887451 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -132,6 +132,9 @@ designated struct initializers (@samp{struct s x =
 @{ .i = 17 @};});
>  @item
>  compound literals (@samp{x = (struct s) @{ 17, 23 @};}).
>
> +@item
> +for loops with variable definition (@samp{for (int i = 0; i < 8;
>> i++)});
> +
>  @item
>  Implementation defined behavior for signed integers is assumed to
>> match
 the
>  expected behavior for two's complement. Non representable values in
 integer
>

 IMO if you want this it would be better to just allow mixed statements
>> and
 declarations, with this as a consequence.

 Can you comment on what the consequences would be for platform support?
 It would remove support for at least one platform I know of (the TI ARM
 compiler).  I've no idea whether it or any other platform which would be
 broken has any users, though.

>>>
>>> No, I'm kind of against mixed statements and declarations, as are many
>>> people here. It mostly does make the code look worse and encourages
>> overuse
>>> of variables.
>>
>> I think the opposite.  I find declaration inside the loop header ugly and
>> weird, while mixed declarations and code do make sense in some cases: e.g.
>> pointer chasing through structures with different types - declaring all the
>> variables in advance is just annoying.  (Maybe that's not strong enough to
>> allow it everywhere if you believe that people will use it inappropriately
>> though.)
>>
>>
> I'm pretty sure its because you're not used to them yet. I'm not taking
> this as a nak.

I have been known to work on other codebases occasionally.  But yes, that's 
just a style opinion and is not a nak (the below issue definitely is without 
further work, though).

> If you want mixed declaration submit a patch later on and let people
> comment on it.

I think I dislike declarations inside loop headers sufficiently that I would 
prefer the current state to having both.

>>> I'm absoultely sure no compiler worth supporting would be broken. If any
>>> +15 year old ones do get broken it would be well worth because it would
>>> still ease maintaining a lot. I'm also quite sure no compiler that would
>> be
>>> broken by this would support compiling ffmpeg at all.
>>> This is still an essential feature of C99 after all and we don't use C89.
>> TI at least disagrees with you, releases are still made without full C99
>> support.  I know it certainly has use on embedded platforms (though likely
>> C6000 more so than ARM), but I don't know if anyone builds libavcodec or
>> similar with it.  (I don't use it - I only know this because Diego recently
>> asked if anyone could test whether it could build libav, and I verified
>> that it could after fixing some minor issues.  He couldn't find any users,
>> though, and the support was removed anyway.)
>>
>>
> If you're not aware of anyone compiling ffmpeg with it then don't even
> bother mentioning it.

I stongly object to your dismissive response to this concern.  Your patch is 
proposing immediately removing (not deprecating with intent to remove later) 
support for an some unclear set of platforms, and it wasn't even mentioned in 
the commit message or the mail.  I have noted one platform which would be so 
removed, Carl noted one as well (I think he was referring to 
), and there might be others.

Before continuing with this patch I think you should at least:
* Have some idea what platforms are affected.
* Investigate whether these platforms have any significant user base (maybe ask 
the user mailing lists, at least).
* Propose a patch to configure which either removes support for them or somehow 
disables them (e.g. it could test-compile a loop including a declaration).

Thanks,

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

Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

[James Almer]

On 11/8/2017 8:05 PM, Mark Thompson wrote:
> On 08/11/17 22:41, Rostislav Pehlivanov wrote:
>> On 8 November 2017 at 22:20, Mark Thompson  wrote:
>>
>>> On 08/11/17 22:03, Rostislav Pehlivanov wrote:
 On 8 November 2017 at 21:49, Mark Thompson  wrote:

> On 08/11/17 21:26, Rostislav Pehlivanov wrote:
>> Signed-off-by: Rostislav Pehlivanov 
>> ---
>>  doc/developer.texi | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/doc/developer.texi b/doc/developer.texi
>> index a7b4f1d737..de7d887451 100644
>> --- a/doc/developer.texi
>> +++ b/doc/developer.texi
>> @@ -132,6 +132,9 @@ designated struct initializers (@samp{struct s x =
> @{ .i = 17 @};});
>>  @item
>>  compound literals (@samp{x = (struct s) @{ 17, 23 @};}).
>>
>> +@item
>> +for loops with variable definition (@samp{for (int i = 0; i < 8;
>>> i++)});
>> +
>>  @item
>>  Implementation defined behavior for signed integers is assumed to
>>> match
> the
>>  expected behavior for two's complement. Non representable values in
> integer
>>
>
> IMO if you want this it would be better to just allow mixed statements
>>> and
> declarations, with this as a consequence.
>
> Can you comment on what the consequences would be for platform support?
> It would remove support for at least one platform I know of (the TI ARM
> compiler).  I've no idea whether it or any other platform which would be
> broken has any users, though.
>

 No, I'm kind of against mixed statements and declarations, as are many
 people here. It mostly does make the code look worse and encourages
>>> overuse
 of variables.
>>>
>>> I think the opposite.  I find declaration inside the loop header ugly and
>>> weird, while mixed declarations and code do make sense in some cases: e.g.
>>> pointer chasing through structures with different types - declaring all the
>>> variables in advance is just annoying.  (Maybe that's not strong enough to
>>> allow it everywhere if you believe that people will use it inappropriately
>>> though.)
>>>
>>>
>> I'm pretty sure its because you're not used to them yet. I'm not taking
>> this as a nak.
> 
> I have been known to work on other codebases occasionally.  But yes, that's 
> just a style opinion and is not a nak (the below issue definitely is without 
> further work, though).
> 
>> If you want mixed declaration submit a patch later on and let people
>> comment on it.
> 
> I think I dislike declarations inside loop headers sufficiently that I would 
> prefer the current state to having both.
> 
 I'm absoultely sure no compiler worth supporting would be broken. If any
 +15 year old ones do get broken it would be well worth because it would
 still ease maintaining a lot. I'm also quite sure no compiler that would
>>> be
 broken by this would support compiling ffmpeg at all.
 This is still an essential feature of C99 after all and we don't use C89.
>>> TI at least disagrees with you, releases are still made without full C99
>>> support.  I know it certainly has use on embedded platforms (though likely
>>> C6000 more so than ARM), but I don't know if anyone builds libavcodec or
>>> similar with it.  (I don't use it - I only know this because Diego recently
>>> asked if anyone could test whether it could build libav, and I verified
>>> that it could after fixing some minor issues.  He couldn't find any users,
>>> though, and the support was removed anyway.)
>>>
>>>
>> If you're not aware of anyone compiling ffmpeg with it then don't even
>> bother mentioning it.
> 
> I stongly object to your dismissive response to this concern.  Your patch is 
> proposing immediately removing (not deprecating with intent to remove later) 
> support for an some unclear set of platforms, and it wasn't even mentioned in 
> the commit message or the mail.  I have noted one platform which would be so 
> removed, Carl noted one as well (I think he was referring to 
> ), and there might be others.
> 
> Before continuing with this patch I think you should at least:
> * Have some idea what platforms are affected.
> * Investigate whether these platforms have any significant user base (maybe 
> ask the user mailing lists, at least).
> * Propose a patch to configure which either removes support for them or 
> somehow disables them (e.g. it could test-compile a loop including a 
> declaration).
> 
> Thanks,

libav directly makes gcc error out with mixed declaration and code,
whereas we only warn about it, so I'd say there are definitely some
systems or compilers out there where that's been an issue. MSVC <= 2012
is most assuredly one.

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

___
ffmpeg-devel mailing list
ffm

Re: [FFmpeg-devel] [PATCH 3/9] lavc/vaapi_decode: fix uninitialized use of variables

[Mark Thompson]

On 08/11/17 18:17, Timo Rothenpieler wrote:
> Fixes CID #1419524 and #1412855
> ---
>  libavcodec/vaapi_decode.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
> index 27ef33837c..a782cfd8da 100644
> --- a/libavcodec/vaapi_decode.c
> +++ b/libavcodec/vaapi_decode.c
> @@ -281,8 +281,9 @@ static int vaapi_decode_make_config(AVCodecContext *avctx)
>  VAStatus vas;
>  int err, i, j;
>  const AVCodecDescriptor *codec_desc;
> -VAProfile profile, va_profile, *profile_list = NULL;
> -int profile_count, exact_match, alt_profile;
> +VAProfile va_profile = VAProfileNone;
> +VAProfile profile, *profile_list = NULL;
> +int profile_count, exact_match, alt_profile = 0;
>  const AVPixFmtDescriptor *sw_desc, *desc;
>  
>  codec_desc = avcodec_descriptor_get(avctx->codec_id);
> 

No - you're just hiding the bug by spuriously initialising the variables 
involved.  I'll rewrite this section to be clearer and not have this bug.

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

[FFmpeg-devel] [PATCH] avformat/dashdec: fix memleak of rep_dest->parent null

[Steven Liu]

fix ticket id: #6820
use the current DASHContext for the rep_dest

Signed-off-by: Steven Liu 
---
 libavformat/dashdec.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index f63f1fffbd..0f5f91c6b5 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -1079,9 +1079,8 @@ static int64_t calc_min_seg_no(AVFormatContext *s, struct 
representation *pls)
 return num;
 }
 
-static int64_t calc_max_seg_no(struct representation *pls)
+static int64_t calc_max_seg_no(struct representation *pls, DASHContext *c)
 {
-DASHContext *c = pls->parent->priv_data;
 int64_t num = 0;
 
 if (pls->n_fragments) {
@@ -1101,21 +1100,21 @@ static int64_t calc_max_seg_no(struct representation 
*pls)
 return num;
 }
 
-static void move_timelines(struct representation *rep_src, struct 
representation *rep_dest)
+static void move_timelines(struct representation *rep_src, struct 
representation *rep_dest, DASHContext *c)
 {
 if (rep_dest && rep_src ) {
 free_timelines_list(rep_dest);
 rep_dest->timelines= rep_src->timelines;
 rep_dest->n_timelines  = rep_src->n_timelines;
 rep_dest->first_seq_no = rep_src->first_seq_no;
-rep_dest->last_seq_no = calc_max_seg_no(rep_dest);
+rep_dest->last_seq_no = calc_max_seg_no(rep_dest, c);
 rep_src->timelines = NULL;
 rep_src->n_timelines = 0;
 rep_dest->cur_seq_no = rep_src->cur_seq_no;
 }
 }
 
-static void move_segments(struct representation *rep_src, struct 
representation *rep_dest)
+static void move_segments(struct representation *rep_src, struct 
representation *rep_dest, DASHContext *c)
 {
 if (rep_dest && rep_src ) {
 free_fragment_list(rep_dest);
@@ -1126,7 +1125,7 @@ static void move_segments(struct representation *rep_src, 
struct representation
 rep_dest->fragments= rep_src->fragments;
 rep_dest->n_fragments  = rep_src->n_fragments;
 rep_dest->parent  = rep_src->parent;
-rep_dest->last_seq_no = calc_max_seg_no(rep_dest);
+rep_dest->last_seq_no = calc_max_seg_no(rep_dest, c);
 rep_src->fragments = NULL;
 rep_src->n_fragments = 0;
 }
@@ -1163,21 +1162,21 @@ static int refresh_manifest(AVFormatContext *s)
 if (cur_video && cur_video->timelines) {
 c->cur_video->cur_seq_no = 
calc_next_seg_no_from_timelines(c->cur_video, currentVideoTime * 
cur_video->fragment_timescale - 1);
 if (c->cur_video->cur_seq_no >= 0) {
-move_timelines(c->cur_video, cur_video);
+move_timelines(c->cur_video, cur_video, c);
 }
 }
 if (cur_audio && cur_audio->timelines) {
 c->cur_audio->cur_seq_no = 
calc_next_seg_no_from_timelines(c->cur_audio, currentAudioTime * 
cur_audio->fragment_timescale - 1);
 if (c->cur_audio->cur_seq_no >= 0) {
-   move_timelines(c->cur_audio, cur_audio);
+   move_timelines(c->cur_audio, cur_audio, c);
 }
 }
 }
 if (cur_video && cur_video->fragments) {
-move_segments(c->cur_video, cur_video);
+move_segments(c->cur_video, cur_video, c);
 }
 if (cur_audio && cur_audio->fragments) {
-move_segments(c->cur_audio, cur_audio);
+move_segments(c->cur_audio, cur_audio, c);
 }
 
 finish:
@@ -1226,7 +1225,7 @@ static struct fragment *get_current_fragment(struct 
representation *pls)
 }
 if (c->is_live) {
 min_seq_no = calc_min_seg_no(pls->parent, pls);
-max_seq_no = calc_max_seg_no(pls);
+max_seq_no = calc_max_seg_no(pls, c);
 
 if (pls->timelines || pls->fragments) {
 refresh_manifest(pls->parent);
@@ -1560,7 +1559,7 @@ 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);
+pls->last_seq_no = calc_max_seg_no(pls, s->priv_data);
 
 ret = reopen_demux_for_component(s, pls);
 if (ret < 0) {
-- 
2.11.0 (Apple Git-81)



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

Re: [FFmpeg-devel] [PATCH v3] lavu: add an AV_FRAME_DATA_GAMMA side data type

[Rostislav Pehlivanov]

On 6 November 2017 at 17:38, Rostislav Pehlivanov 
wrote:

> Signed-off-by: Rostislav Pehlivanov 
> ---
>  doc/APIchanges  | 3 +++
>  libavutil/frame.h   | 6 ++
>  libavutil/version.h | 2 +-
>  3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 1490d67f27..75051deaf8 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil: 2017-10-21
>
>  API changes, most recent first:
>
> +2017-11-06 - xxx - lavu 56.01.100 - frame.h
> +  Add the AV_FRAME_DATA_GAMMA side data type.
> +
>   8< - FFmpeg 3.4 was cut here  8< -
>
>  2017-09-28 - b6cf66ae1c - lavc 57.106.104 - avcodec.h
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 0c6aab1c02..64dcf3a397 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -141,6 +141,12 @@ enum AVFrameSideDataType {
>   * metadata key entry "name".
>   */
>  AV_FRAME_DATA_ICC_PROFILE,
> +
> +/**
> + * The data contains an AVRational which describes the exponent
> needed to
> + * compensate for nonlinearity in the input signal.
> + */
> +AV_FRAME_DATA_GAMMA,
>  };
>
>  enum AVActiveFormatDescription {
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 1bc4b2a6cb..cf8ec498e4 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -80,7 +80,7 @@
>
>
>  #define LIBAVUTIL_VERSION_MAJOR  56
> -#define LIBAVUTIL_VERSION_MINOR   0
> +#define LIBAVUTIL_VERSION_MINOR   1
>  #define LIBAVUTIL_VERSION_MICRO 100
>
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> --
> 2.15.0.403.gc27cc4dac6
>
>

Patch dropped, jamrial suggested a much more reasonable approach by using
the metadata.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Parse upto 2 keyframes after the edit list end in mov_fix_index.

[Sasi Inguva]

Updated the fate test after
http://git.videolan.org/?p=ffmpeg.git;a=commit;h=c2a8f0fcbe57ea9ccaa864130f078af10516c3c1
. Sending the updated patch.

On Thu, Nov 2, 2017 at 4:23 PM, Michael Niedermayer 
wrote:

> On Wed, Nov 01, 2017 at 02:27:50PM -0700, Sasi Inguva wrote:
> > Pls find attached, the FATE sample.
>
> uplaoded
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> There will always be a question for which you do not know the correct
> answer.
>
> ___
> 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] lavf/mov.c: Parse upto 2 keyframes after the edit list end in mov_fix_index.

[Sasi Inguva]

Partially fixes t/6699.
---
 libavformat/mov.c | 32 ---
 tests/fate/mov.mak|  4 
 tests/ref/fate/mov-elst-ends-betn-b-and-i | 31 ++
 3 files changed, 56 insertions(+), 11 deletions(-)
 create mode 100644 tests/ref/fate/mov-elst-ends-betn-b-and-i

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 7954db6e47..436ae42cbb 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3295,6 +3295,7 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
 int packet_skip_samples = 0;
 MOVIndexRange *current_index_range;
 int i;
+int found_keyframe_after_edit = 0;
 
 if (!msc->elst_data || msc->elst_count <= 0 || nb_old <= 0) {
 return;
@@ -3390,6 +3391,7 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
 
 // Iterate over index and arrange it according to edit list
 edit_list_start_encountered = 0;
+found_keyframe_after_edit = 0;
 for (; current < e_old_end; current++, index++) {
 // check  if frame outside edit list mark it for discard
 frame_duration = (current + 1 <  e_old_end) ?
@@ -3502,18 +3504,26 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
 }
 
 // Break when found first key frame after edit entry completion
-if (((curr_cts + frame_duration) >= (edit_list_duration + 
edit_list_media_time)) &&
+if ((curr_cts + frame_duration >= (edit_list_duration + 
edit_list_media_time)) &&
 ((flags & AVINDEX_KEYFRAME) || ((st->codecpar->codec_type == 
AVMEDIA_TYPE_AUDIO {
-
-if (ctts_data_old && ctts_sample_old != 0) {
-if (add_ctts_entry(&msc->ctts_data, &msc->ctts_count,
-   &msc->ctts_allocated_size,
-   ctts_sample_old - 
edit_list_start_ctts_sample,
-   ctts_data_old[ctts_index_old].duration) 
== -1) {
-av_log(mov->fc, AV_LOG_ERROR, "Cannot add CTTS entry 
%"PRId64" - {%"PRId64", %d}\n",
-   ctts_index_old, ctts_sample_old - 
edit_list_start_ctts_sample,
-   ctts_data_old[ctts_index_old].duration);
-break;
+if (ctts_data_old) {
+// If we have CTTS and this is the the first keyframe 
after edit elist,
+// wait for one more, because there might be trailing 
B-frames after this I-frame
+// that do belong to the edit.
+if (st->codecpar->codec_type != AVMEDIA_TYPE_AUDIO && 
found_keyframe_after_edit == 0) {
+found_keyframe_after_edit = 1;
+continue;
+}
+if (ctts_sample_old != 0) {
+if (add_ctts_entry(&msc->ctts_data, &msc->ctts_count,
+   &msc->ctts_allocated_size,
+   ctts_sample_old - 
edit_list_start_ctts_sample,
+   
ctts_data_old[ctts_index_old].duration) == -1) {
+av_log(mov->fc, AV_LOG_ERROR, "Cannot add CTTS 
entry %"PRId64" - {%"PRId64", %d}\n",
+   ctts_index_old, ctts_sample_old - 
edit_list_start_ctts_sample,
+   ctts_data_old[ctts_index_old].duration);
+break;
+}
 }
 }
 break;
diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
index 01893a0767..76f66ff498 100644
--- a/tests/fate/mov.mak
+++ b/tests/fate/mov.mak
@@ -10,6 +10,7 @@ FATE_MOV = fate-mov-3elist \
fate-mov-gpmf-remux \
fate-mov-440hz-10ms \
fate-mov-ibi-elst-starts-b \
+   fate-mov-elst-ends-betn-b-and-i \
 
 FATE_MOV_FFPROBE = fate-mov-aac-2048-priming \
fate-mov-zombie \
@@ -42,6 +43,9 @@ fate-mov-1elist-ends-last-bframe: CMD = framemd5 -i 
$(TARGET_SAMPLES)/mov/mov-1e
 # Makes sure that we handle timestamps of packets in case of multiple edit 
lists with one of them ending on a B-frame correctly.
 fate-mov-2elist-elist1-ends-bframe: CMD = framemd5 -i 
$(TARGET_SAMPLES)/mov/mov-2elist-elist1-ends-bframe.mov
 
+# Makes sure that if edit list ends on a B-frame but before the I-frame, then 
we output the B-frame but discard the I-frame.
+fate-mov-elst-ends-betn-b-and-i: CMD = framemd5 -i 
$(TARGET_SAMPLES)/mov/elst_ends_betn_b_and_i.mp4
+
 # Makes sure that we handle edit lists and start padding correctly.
 fate-mov-440hz-10ms: CMD = framemd5 -i $(TARGET_SAMPLES)/mov/440hz-10ms.m4a
 
diff --git a/tests/ref/fate/mov-elst-ends-betn-b-and-i 
b/tests/ref/fate/mov-elst-ends-betn-b-and-i
new file mode 100644
index 00..ee7c6389ad
--- /de

[FFmpeg-devel] [PATCH v4] pngdec: expose gAMA and cHRM chunks as side/meta data

[Rostislav Pehlivanov]

Signed-off-by: Rostislav Pehlivanov 
---
 libavcodec/pngdec.c | 30 +-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 0d6612ccca..aef0d95819 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -25,6 +25,7 @@
 #include "libavutil/bprint.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/stereo3d.h"
+#include "libavutil/mastering_display_metadata.h"
 
 #include "avcodec.h"
 #include "bytestream.h"
@@ -1165,7 +1166,7 @@ static int decode_frame_common(AVCodecContext *avctx, 
PNGDecContext *s,
 AVDictionary **metadatap = NULL;
 uint32_t tag, length;
 int decode_next_dat = 0;
-int ret;
+int i, ret;
 
 for (;;) {
 length = bytestream2_get_bytes_left(&s->gb);
@@ -1287,6 +1288,33 @@ static int decode_frame_common(AVCodecContext *avctx, 
PNGDecContext *s,
 goto fail;
 break;
 }
+case MKTAG('c', 'H', 'R', 'M'): {
+AVMasteringDisplayMetadata *mdm = 
av_mastering_display_metadata_create_side_data(p);
+if (!mdm) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+
+mdm->white_point[0] = av_make_q(bytestream2_get_be32(&s->gb), 
10);
+mdm->white_point[1] = av_make_q(bytestream2_get_be32(&s->gb), 
10);
+
+/* RGB Primaries */
+for (i = 0; i < 3; i++) {
+mdm->display_primaries[i][0] = 
av_make_q(bytestream2_get_be32(&s->gb), 10);
+mdm->display_primaries[i][1] = 
av_make_q(bytestream2_get_be32(&s->gb), 10);
+}
+
+mdm->has_primaries = 1;
+bytestream2_skip(&s->gb, 4); /* crc */
+break;
+}
+case MKTAG('g', 'A', 'M', 'A'): {
+av_dict_set_int(&p->metadata, "gamma_num", 
bytestream2_get_be32(&s->gb), 0);
+av_dict_set_int(&p->metadata, "gamma_den", 10, 0);
+
+bytestream2_skip(&s->gb, 4); /* crc */
+break;
+}
 case MKTAG('I', 'E', 'N', 'D'):
 if (!(s->pic_state & PNG_ALLIMAGE))
 av_log(avctx, AV_LOG_ERROR, "IEND without all image\n");
-- 
2.15.0.448.gf294e3d99a

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

Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

[Rostislav Pehlivanov]

On 8 November 2017 at 23:09, James Almer  wrote:

> On 11/8/2017 7:41 PM, Rostislav Pehlivanov wrote:
> > On 8 November 2017 at 22:20, Mark Thompson  wrote:
> >
> >> On 08/11/17 22:03, Rostislav Pehlivanov wrote:
> >>> On 8 November 2017 at 21:49, Mark Thompson  wrote:
> >>>
>  On 08/11/17 21:26, Rostislav Pehlivanov wrote:
> > Signed-off-by: Rostislav Pehlivanov 
> > ---
> >  doc/developer.texi | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index a7b4f1d737..de7d887451 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -132,6 +132,9 @@ designated struct initializers (@samp{struct s x
> =
>  @{ .i = 17 @};});
> >  @item
> >  compound literals (@samp{x = (struct s) @{ 17, 23 @};}).
> >
> > +@item
> > +for loops with variable definition (@samp{for (int i = 0; i < 8;
> >> i++)});
> > +
> >  @item
> >  Implementation defined behavior for signed integers is assumed to
> >> match
>  the
> >  expected behavior for two's complement. Non representable values in
>  integer
> >
> 
>  IMO if you want this it would be better to just allow mixed statements
> >> and
>  declarations, with this as a consequence.
> 
>  Can you comment on what the consequences would be for platform
> support?
>  It would remove support for at least one platform I know of (the TI
> ARM
>  compiler).  I've no idea whether it or any other platform which would
> be
>  broken has any users, though.
> 
> >>>
> >>> No, I'm kind of against mixed statements and declarations, as are many
> >>> people here. It mostly does make the code look worse and encourages
> >> overuse
> >>> of variables.
> >>
> >> I think the opposite.  I find declaration inside the loop header ugly
> and
> >> weird, while mixed declarations and code do make sense in some cases:
> e.g.
> >> pointer chasing through structures with different types - declaring all
> the
> >> variables in advance is just annoying.  (Maybe that's not strong enough
> to
> >> allow it everywhere if you believe that people will use it
> inappropriately
> >> though.)
> >>
> >>
> > I'm pretty sure its because you're not used to them yet. I'm not taking
> > this as a nak.
> > If you want mixed declaration submit a patch later on and let people
> > comment on it.
>
> It's the other way around. If you want to introduce some change, you're
> the one that needs to convince other devs it's a good change, and so
> far, two dislike it.
> You can't commit this when people are against it saying "send a patch to
> undo it later".
>


What two people, Carl hasn't said he's against it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 1/2] lavc/libkvazaar: switch to ff_alloc_packet2

[Jun Zhao]

From 190d3cf31345e9a94169278ffe0c195868a6d400 Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Wed, 8 Nov 2017 21:02:23 +0800
Subject: [PATCH 1/2] lavc/libkvazaar: switch to ff_alloc_packet2.

ff_alloc_packet have been deprecated, switch to use the
ff_alloc_packet2.

Signed-off-by: Jun Zhao 
---
 libavcodec/libkvazaar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
index f35b0df61d..25e7b32f5f 100644
--- a/libavcodec/libkvazaar.c
+++ b/libavcodec/libkvazaar.c
@@ -231,7 +231,7 @@ static int libkvazaar_encode(AVCodecContext *avctx,
 kvz_data_chunk *chunk = NULL;
 uint64_t written = 0;
 
-retval = ff_alloc_packet(avpkt, len_out);
+retval = ff_alloc_packet2(avctx, avpkt, len_out, len_out);
 if (retval < 0) {
 av_log(avctx, AV_LOG_ERROR, "Failed to allocate output packet.\n");
 goto done;
-- 
2.14.1

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

Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

[Rostislav Pehlivanov]

On 8 November 2017 at 23:05, Mark Thompson  wrote:

> On 08/11/17 22:41, Rostislav Pehlivanov wrote:
> > On 8 November 2017 at 22:20, Mark Thompson  wrote:
> >
> >> On 08/11/17 22:03, Rostislav Pehlivanov wrote:
> >>> On 8 November 2017 at 21:49, Mark Thompson  wrote:
> >>>
>  On 08/11/17 21:26, Rostislav Pehlivanov wrote:
> > Signed-off-by: Rostislav Pehlivanov 
> > ---
> >  doc/developer.texi | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index a7b4f1d737..de7d887451 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -132,6 +132,9 @@ designated struct initializers (@samp{struct s x
> =
>  @{ .i = 17 @};});
> >  @item
> >  compound literals (@samp{x = (struct s) @{ 17, 23 @};}).
> >
> > +@item
> > +for loops with variable definition (@samp{for (int i = 0; i < 8;
> >> i++)});
> > +
> >  @item
> >  Implementation defined behavior for signed integers is assumed to
> >> match
>  the
> >  expected behavior for two's complement. Non representable values in
>  integer
> >
> 
>  IMO if you want this it would be better to just allow mixed statements
> >> and
>  declarations, with this as a consequence.
> 
>  Can you comment on what the consequences would be for platform
> support?
>  It would remove support for at least one platform I know of (the TI
> ARM
>  compiler).  I've no idea whether it or any other platform which would
> be
>  broken has any users, though.
> 
> >>>
> >>> No, I'm kind of against mixed statements and declarations, as are many
> >>> people here. It mostly does make the code look worse and encourages
> >> overuse
> >>> of variables.
> >>
> >> I think the opposite.  I find declaration inside the loop header ugly
> and
> >> weird, while mixed declarations and code do make sense in some cases:
> e.g.
> >> pointer chasing through structures with different types - declaring all
> the
> >> variables in advance is just annoying.  (Maybe that's not strong enough
> to
> >> allow it everywhere if you believe that people will use it
> inappropriately
> >> though.)
> >>
> >>
> > I'm pretty sure its because you're not used to them yet. I'm not taking
> > this as a nak.
>
> I have been known to work on other codebases occasionally.  But yes,
> that's just a style opinion and is not a nak (the below issue definitely is
> without further work, though).
>
> > If you want mixed declaration submit a patch later on and let people
> > comment on it.
>
> I think I dislike declarations inside loop headers sufficiently that I
> would prefer the current state to having both.
>
> >>> I'm absoultely sure no compiler worth supporting would be broken. If
> any
> >>> +15 year old ones do get broken it would be well worth because it would
> >>> still ease maintaining a lot. I'm also quite sure no compiler that
> would
> >> be
> >>> broken by this would support compiling ffmpeg at all.
> >>> This is still an essential feature of C99 after all and we don't use
> C89.
> >> TI at least disagrees with you, releases are still made without full C99
> >> support.  I know it certainly has use on embedded platforms (though
> likely
> >> C6000 more so than ARM), but I don't know if anyone builds libavcodec or
> >> similar with it.  (I don't use it - I only know this because Diego
> recently
> >> asked if anyone could test whether it could build libav, and I verified
> >> that it could after fixing some minor issues.  He couldn't find any
> users,
> >> though, and the support was removed anyway.)
> >>
> >>
> > If you're not aware of anyone compiling ffmpeg with it then don't even
> > bother mentioning it.
>
> I stongly object to your dismissive response to this concern.  Your patch
> is proposing immediately removing (not deprecating with intent to remove
> later) support for an some unclear set of platforms, and it wasn't even
> mentioned in the commit message or the mail.  I have noted one platform
> which would be so removed, Carl noted one as well (I think he was referring
> to ), and there might be others.
>
> Before continuing with this patch I think you should at least:
> * Have some idea what platforms are affected.
> * Investigate whether these platforms have any significant user base
> (maybe ask the user mailing lists, at least).
> * Propose a patch to configure which either removes support for them or
> somehow disables them (e.g. it could test-compile a loop including a
> declaration).
>
>
You aren't clear on how many platforms are affected either so I object to
your objection to my dismissiveness and making it seem like its a big deal.
I have some idea about which platofrms are affected and I'd be implicitly
dropping support for them, so that's 1 and 3 off.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/m

[FFmpeg-devel] [PATCH 2/2] lavc/libx265: switch to ff_alloc_packet2

[Jun Zhao]

From 5afdf252b3bb6f8c7a276c2a8bde8f4a95d170e4 Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Wed, 8 Nov 2017 21:04:51 +0800
Subject: [PATCH 2/2] lavc/libx265: switch to ff_alloc_packet2

ff_alloc_packet have been deprecated, switch to use
ff_alloc_packet2.

Signed-off-by: Jun Zhao 
---
 libavcodec/libx265.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index 784b51c52d..4456e300f2 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -294,7 +294,7 @@ static int libx265_encode_frame(AVCodecContext *avctx, 
AVPacket *pkt,
 for (i = 0; i < nnal; i++)
 payload += nal[i].sizeBytes;
 
-ret = ff_alloc_packet(pkt, payload);
+ret = ff_alloc_packet2(avctx, pkt, payload, payload);
 if (ret < 0) {
 av_log(avctx, AV_LOG_ERROR, "Error getting output packet.\n");
 return ret;
-- 
2.14.1

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

Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

[James Almer]

On 11/8/2017 9:39 PM, Rostislav Pehlivanov wrote:
> On 8 November 2017 at 23:05, Mark Thompson  wrote:
> 
>> On 08/11/17 22:41, Rostislav Pehlivanov wrote:
>>> On 8 November 2017 at 22:20, Mark Thompson  wrote:
>>>
 On 08/11/17 22:03, Rostislav Pehlivanov wrote:
> On 8 November 2017 at 21:49, Mark Thompson  wrote:
>
>> On 08/11/17 21:26, Rostislav Pehlivanov wrote:
>>> Signed-off-by: Rostislav Pehlivanov 
>>> ---
>>>  doc/developer.texi | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/doc/developer.texi b/doc/developer.texi
>>> index a7b4f1d737..de7d887451 100644
>>> --- a/doc/developer.texi
>>> +++ b/doc/developer.texi
>>> @@ -132,6 +132,9 @@ designated struct initializers (@samp{struct s x
>> =
>> @{ .i = 17 @};});
>>>  @item
>>>  compound literals (@samp{x = (struct s) @{ 17, 23 @};}).
>>>
>>> +@item
>>> +for loops with variable definition (@samp{for (int i = 0; i < 8;
 i++)});
>>> +
>>>  @item
>>>  Implementation defined behavior for signed integers is assumed to
 match
>> the
>>>  expected behavior for two's complement. Non representable values in
>> integer
>>>
>>
>> IMO if you want this it would be better to just allow mixed statements
 and
>> declarations, with this as a consequence.
>>
>> Can you comment on what the consequences would be for platform
>> support?
>> It would remove support for at least one platform I know of (the TI
>> ARM
>> compiler).  I've no idea whether it or any other platform which would
>> be
>> broken has any users, though.
>>
>
> No, I'm kind of against mixed statements and declarations, as are many
> people here. It mostly does make the code look worse and encourages
 overuse
> of variables.

 I think the opposite.  I find declaration inside the loop header ugly
>> and
 weird, while mixed declarations and code do make sense in some cases:
>> e.g.
 pointer chasing through structures with different types - declaring all
>> the
 variables in advance is just annoying.  (Maybe that's not strong enough
>> to
 allow it everywhere if you believe that people will use it
>> inappropriately
 though.)


>>> I'm pretty sure its because you're not used to them yet. I'm not taking
>>> this as a nak.
>>
>> I have been known to work on other codebases occasionally.  But yes,
>> that's just a style opinion and is not a nak (the below issue definitely is
>> without further work, though).
>>
>>> If you want mixed declaration submit a patch later on and let people
>>> comment on it.
>>
>> I think I dislike declarations inside loop headers sufficiently that I
>> would prefer the current state to having both.
>>
> I'm absoultely sure no compiler worth supporting would be broken. If
>> any
> +15 year old ones do get broken it would be well worth because it would
> still ease maintaining a lot. I'm also quite sure no compiler that
>> would
 be
> broken by this would support compiling ffmpeg at all.
> This is still an essential feature of C99 after all and we don't use
>> C89.
 TI at least disagrees with you, releases are still made without full C99
 support.  I know it certainly has use on embedded platforms (though
>> likely
 C6000 more so than ARM), but I don't know if anyone builds libavcodec or
 similar with it.  (I don't use it - I only know this because Diego
>> recently
 asked if anyone could test whether it could build libav, and I verified
 that it could after fixing some minor issues.  He couldn't find any
>> users,
 though, and the support was removed anyway.)


>>> If you're not aware of anyone compiling ffmpeg with it then don't even
>>> bother mentioning it.
>>
>> I stongly object to your dismissive response to this concern.  Your patch
>> is proposing immediately removing (not deprecating with intent to remove
>> later) support for an some unclear set of platforms, and it wasn't even
>> mentioned in the commit message or the mail.  I have noted one platform
>> which would be so removed, Carl noted one as well (I think he was referring
>> to ), and there might be others.
>>
>> Before continuing with this patch I think you should at least:
>> * Have some idea what platforms are affected.
>> * Investigate whether these platforms have any significant user base
>> (maybe ask the user mailing lists, at least).
>> * Propose a patch to configure which either removes support for them or
>> somehow disables them (e.g. it could test-compile a loop including a
>> declaration).
>>
>>
> You aren't clear on how many platforms are affected either so I object to
> your objection to my dismissiveness and making it seem like its a big deal.
> I have some idea about which platofrms are affected and I'd be implicitly
> dropping support for them, so that's 1 and 3 off.

3 requires t

Re: [FFmpeg-devel] [PATCH 1/2] aptx: implement the aptX bluetooth codec

[Rostislav Pehlivanov]

On 8 November 2017 at 22:41, Aurelien Jacobs  wrote:

> On Wed, Nov 08, 2017 at 06:26:03PM +0100, Michael Niedermayer wrote:
> > On Wed, Nov 08, 2017 at 02:06:09PM +0100, Aurelien Jacobs wrote:
> > [...]
> > > +typedef const struct {
> > > +const int32_t *quantize_intervals;
> > > +const int32_t *invert_quantize_dither_factors;
> > > +const int32_t *quantize_dither_factors;
> >
> > > +const int32_t *quantize_factor_select_offset;
> >
> > this would fit in int16_t *
>
> Right.
>
> > [...]
> > > +static const int32_t quantization_factors[32] = {
> > > +2048, 2093, 2139, 2186, 2233, 2282, 2332, 2383,
> > > +2435, 2489, 2543, 2599, 2656, 2714, 2774, 2834,
> > > +2896, 2960, 3025, 3091, 3158, 3228, 3298, 3371,
> > > +3444, 3520, 3597, 3676, 3756, 3838, 3922, 4008,
> > > +};
> >
> > this too would fir in int16_t
>
> Indeed, now that I moved the shift inside the code.
>
> > [...]
> > > +/*
> > > + * Push one sample into a circular signal buffer.
> > > + */
> > > +av_always_inline
> > > +static void aptx_qmf_filter_signal_push(FilterSignal *signal,
> int32_t sample)
> > > +{
> > > +signal->buffer[signal->pos] = sample;
> > > +signal->buffer[signal->pos+FILTER_TAPS] = sample;
> > > +signal->pos = (signal->pos + 1) % FILTER_TAPS;
> >
> > % could be replaced by &
>
> OK. And there was a second one that I changed as well.
>
> > > +}
> > > +
> > > +/*
> > > + * Compute the convolution of the signal with the coefficients, and
> reduce
> > > + * to 24 bits by applying the specified right shifting.
> > > + */
> > > +av_always_inline
> > > +static int32_t aptx_qmf_convolution(FilterSignal *signal,
> > > +const int32_t coeffs[FILTER_TAPS],
> > > +int shift)
> > > +{
> > > +int32_t *sig = &signal->buffer[signal->pos];
> > > +int64_t e = 0;
> > > +
> >
> > > +for (int i = 0; i < FILTER_TAPS; i++)
> >
> > "for (int" is something we avoided as some comilers didnt like it,
> > iam not sure if this is still true but there are none in the codebase
>
> If you insist on this I will of course change it, but hey, we require
> a C99 compiler since a long time and we use so many features of C99
> that I really don't get why we couldn't use "for (int".
> I can't imagine that any relevant C compiler would not support this
> nowadays !
> What I propose is to get this in as is, and see if anyone encounter
> issue with any compiler. If any issue arise, I will of course send a
> patch to fix it.
>
> Here is an updated patch.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Send another patch because some people are beyond convincing and its really
pissing me off. Really. In particular maybe those compiler writers at
Microsoft who seem to think shipping something that doesn't support such a
simple yet important feature is important.
Or maybe users who don't want to update a 6 year old version of msvc.
Or maybe us because we support compiling with msvc at all when its clearly
_not_ a C compiler and this project is written in C.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] example/vaapi_transcode: Add a VA-VAPI transcode example.

[Jun Zhao]

On 2017/11/2 9:49, Jun Zhao wrote:
Ping, any comments?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v4] pngdec: expose gAMA and cHRM chunks as side/meta data

[James Almer]

On 11/8/2017 9:25 PM, Rostislav Pehlivanov wrote:
> Signed-off-by: Rostislav Pehlivanov 
> ---
>  libavcodec/pngdec.c | 30 +-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> index 0d6612ccca..aef0d95819 100644
> --- a/libavcodec/pngdec.c
> +++ b/libavcodec/pngdec.c
> @@ -25,6 +25,7 @@
>  #include "libavutil/bprint.h"
>  #include "libavutil/imgutils.h"
>  #include "libavutil/stereo3d.h"
> +#include "libavutil/mastering_display_metadata.h"
>  
>  #include "avcodec.h"
>  #include "bytestream.h"
> @@ -1165,7 +1166,7 @@ static int decode_frame_common(AVCodecContext *avctx, 
> PNGDecContext *s,
>  AVDictionary **metadatap = NULL;
>  uint32_t tag, length;
>  int decode_next_dat = 0;
> -int ret;
> +int i, ret;
>  
>  for (;;) {
>  length = bytestream2_get_bytes_left(&s->gb);
> @@ -1287,6 +1288,33 @@ static int decode_frame_common(AVCodecContext *avctx, 
> PNGDecContext *s,
>  goto fail;
>  break;
>  }
> +case MKTAG('c', 'H', 'R', 'M'): {
> +AVMasteringDisplayMetadata *mdm = 
> av_mastering_display_metadata_create_side_data(p);
> +if (!mdm) {
> +ret = AVERROR(ENOMEM);
> +goto fail;
> +}
> +
> +mdm->white_point[0] = av_make_q(bytestream2_get_be32(&s->gb), 
> 10);
> +mdm->white_point[1] = av_make_q(bytestream2_get_be32(&s->gb), 
> 10);
> +
> +/* RGB Primaries */
> +for (i = 0; i < 3; i++) {
> +mdm->display_primaries[i][0] = 
> av_make_q(bytestream2_get_be32(&s->gb), 10);
> +mdm->display_primaries[i][1] = 
> av_make_q(bytestream2_get_be32(&s->gb), 10);
> +}
> +
> +mdm->has_primaries = 1;
> +bytestream2_skip(&s->gb, 4); /* crc */
> +break;
> +}
> +case MKTAG('g', 'A', 'M', 'A'): {
> +av_dict_set_int(&p->metadata, "gamma_num", 
> bytestream2_get_be32(&s->gb), 0);
> +av_dict_set_int(&p->metadata, "gamma_den", 10, 0);

You should use snprintf or the AVBPrint API to write a rational value in
the "x/y" or "x:y" form as single string entry to use with av_dict_set()
instead.
Having two metadata entries for a single rational value seems unwieldy.

See ff_tadd_rational_metadata() in tiff_common.c for an example of this
using AVBPrint.

> +
> +bytestream2_skip(&s->gb, 4); /* crc */
> +break;
> +}
>  case MKTAG('I', 'E', 'N', 'D'):
>  if (!(s->pic_state & PNG_ALLIMAGE))
>  av_log(avctx, AV_LOG_ERROR, "IEND without all image\n");
> 

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

Re: [FFmpeg-devel] [PATCH 1/2] aptx: implement the aptX bluetooth codec

[Michael Niedermayer]

On Wed, Nov 08, 2017 at 11:41:16PM +0100, Aurelien Jacobs wrote:
> On Wed, Nov 08, 2017 at 06:26:03PM +0100, Michael Niedermayer wrote:
> > On Wed, Nov 08, 2017 at 02:06:09PM +0100, Aurelien Jacobs wrote:
> > [...]
> > > +typedef const struct {
> > > +const int32_t *quantize_intervals;
> > > +const int32_t *invert_quantize_dither_factors;
> > > +const int32_t *quantize_dither_factors;
> > 
> > > +const int32_t *quantize_factor_select_offset;
> > 
> > this would fit in int16_t *
> 
> Right.
> 
> > [...]
> > > +static const int32_t quantization_factors[32] = {
> > > +2048, 2093, 2139, 2186, 2233, 2282, 2332, 2383,
> > > +2435, 2489, 2543, 2599, 2656, 2714, 2774, 2834,
> > > +2896, 2960, 3025, 3091, 3158, 3228, 3298, 3371,
> > > +3444, 3520, 3597, 3676, 3756, 3838, 3922, 4008,
> > > +};
> > 
> > this too would fir in int16_t
> 
> Indeed, now that I moved the shift inside the code.
> 
> > [...]
> > > +/*
> > > + * Push one sample into a circular signal buffer.
> > > + */
> > > +av_always_inline
> > > +static void aptx_qmf_filter_signal_push(FilterSignal *signal, int32_t 
> > > sample)
> > > +{
> > > +signal->buffer[signal->pos] = sample;
> > > +signal->buffer[signal->pos+FILTER_TAPS] = sample;
> > > +signal->pos = (signal->pos + 1) % FILTER_TAPS;
> > 
> > % could be replaced by &
> 
> OK. And there was a second one that I changed as well.
> 
> > > +}
> > > +
> > > +/*
> > > + * Compute the convolution of the signal with the coefficients, and 
> > > reduce
> > > + * to 24 bits by applying the specified right shifting.
> > > + */
> > > +av_always_inline
> > > +static int32_t aptx_qmf_convolution(FilterSignal *signal,
> > > +const int32_t coeffs[FILTER_TAPS],
> > > +int shift)
> > > +{
> > > +int32_t *sig = &signal->buffer[signal->pos];
> > > +int64_t e = 0;
> > > +
> > 
> > > +for (int i = 0; i < FILTER_TAPS; i++)
> > 
> > "for (int" is something we avoided as some comilers didnt like it,
> > iam not sure if this is still true but there are none in the codebase
> 
> If you insist on this I will of course change it, but hey, we require

me personally, not really, i used "for (int" style as long as
i can remember outside ffmpeg but i also would like to keep supportng
all platforms ...


> a C99 compiler since a long time and we use so many features of C99
> that I really don't get why we couldn't use "for (int".
> I can't imagine that any relevant C compiler would not support this
> nowadays !

Theres a seperate thread about this now


> What I propose is to get this in as is, and see if anyone encounter
> issue with any compiler. If any issue arise, I will of course send a
> patch to fix it.

While that can be done, i think its especially with rarer platforms
not such a good idea. Imagine everyone would do this, for example
for every E* error code that is not supported on all platforms,
FFmpeg would often randomly not build on platforms that are less
mainstream as people re-test what breaks for whom ...

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle


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

[FFmpeg-devel] [PATCH v5] pngdec: expose gAMA and cHRM chunks as side/meta data

[Rostislav Pehlivanov]

Signed-off-by: Rostislav Pehlivanov 
---
 libavcodec/pngdec.c | 39 ++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 0d6612ccca..0a9a248ee7 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -25,6 +25,7 @@
 #include "libavutil/bprint.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/stereo3d.h"
+#include "libavutil/mastering_display_metadata.h"
 
 #include "avcodec.h"
 #include "bytestream.h"
@@ -1165,7 +1166,7 @@ static int decode_frame_common(AVCodecContext *avctx, 
PNGDecContext *s,
 AVDictionary **metadatap = NULL;
 uint32_t tag, length;
 int decode_next_dat = 0;
-int ret;
+int i, ret;
 
 for (;;) {
 length = bytestream2_get_bytes_left(&s->gb);
@@ -1287,6 +1288,42 @@ static int decode_frame_common(AVCodecContext *avctx, 
PNGDecContext *s,
 goto fail;
 break;
 }
+case MKTAG('c', 'H', 'R', 'M'): {
+AVMasteringDisplayMetadata *mdm = 
av_mastering_display_metadata_create_side_data(p);
+if (!mdm) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+
+mdm->white_point[0] = av_make_q(bytestream2_get_be32(&s->gb), 
10);
+mdm->white_point[1] = av_make_q(bytestream2_get_be32(&s->gb), 
10);
+
+/* RGB Primaries */
+for (i = 0; i < 3; i++) {
+mdm->display_primaries[i][0] = 
av_make_q(bytestream2_get_be32(&s->gb), 10);
+mdm->display_primaries[i][1] = 
av_make_q(bytestream2_get_be32(&s->gb), 10);
+}
+
+mdm->has_primaries = 1;
+bytestream2_skip(&s->gb, 4); /* crc */
+break;
+}
+case MKTAG('g', 'A', 'M', 'A'): {
+AVBPrint bp;
+char *gamma_str;
+int num = bytestream2_get_be32(&s->gb);
+
+av_bprint_init(&bp, 0, -1);
+av_bprintf(&bp, "%i/%i", num, 10);
+av_bprint_finalize(&bp, (char **)&gamma_str);
+if (!gamma_str)
+return AVERROR(ENOMEM);
+
+av_dict_set(&p->metadata, "gamma", gamma_str, 0);
+
+bytestream2_skip(&s->gb, 4); /* crc */
+break;
+}
 case MKTAG('I', 'E', 'N', 'D'):
 if (!(s->pic_state & PNG_ALLIMAGE))
 av_log(avctx, AV_LOG_ERROR, "IEND without all image\n");
-- 
2.15.0.448.gf294e3d99a

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

Re: [FFmpeg-devel] [PATCH] avutil/frame: Add private_ref to AVFrame

[Michael Niedermayer]

On Wed, Nov 08, 2017 at 11:05:48PM +, Rostislav Pehlivanov wrote:
> On 8 November 2017 at 22:55, Michael Niedermayer 
> wrote:
> 
> > This gives FFmpeg libs a field that they can freely and safely use.
> > Avoiding the need of wraping of a users opaque_ref field and its issues.
> >
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavutil/frame.c |  8 +++-
> >  libavutil/frame.h | 12 
> >  2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavutil/frame.c b/libavutil/frame.c
> > index 982fbb5c81..662a7e5ab5 100644
> > --- a/libavutil/frame.c
> > +++ b/libavutil/frame.c
> > @@ -383,12 +383,17 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >  #endif
> >
> >  av_buffer_unref(&dst->opaque_ref);
> > +av_buffer_unref(&dst->private_ref);
> >  if (src->opaque_ref) {
> >  dst->opaque_ref = av_buffer_ref(src->opaque_ref);
> >  if (!dst->opaque_ref)
> >  return AVERROR(ENOMEM);
> >  }
> > -
> > +if (src->private_ref) {
> > +dst->private_ref = av_buffer_ref(src->private_ref);
> > +if (!dst->private_ref)
> > +return AVERROR(ENOMEM);
> > +}
> >  return 0;
> >  }
> >
> > @@ -524,6 +529,7 @@ void av_frame_unref(AVFrame *frame)
> >  av_buffer_unref(&frame->hw_frames_ctx);
> >
> >  av_buffer_unref(&frame->opaque_ref);
> > +av_buffer_unref(&frame->private_ref);
> >
> >  get_frame_defaults(frame);
> >  }
> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > index 0c6aab1c02..7b9bec054a 100644
> > --- a/libavutil/frame.h
> > +++ b/libavutil/frame.h
> > @@ -563,6 +563,18 @@ typedef struct AVFrame {
> >  /**
> >   * @}
> >   */
> > +/**
> > + * AVBufferRef for free use by libavcodec / libavfilte /  Code
> > outside
> > + * the FFmpeg libs will never
> > + * check or change the contents of the buffer ref. FFmpeg calls
> > + * av_buffer_unref() on it when the frame is unreferenced.
> > + * av_frame_copy_props() calls create a new reference with
> > av_buffer_ref()
> > + * for the target frame's private_ref field.
> > + *
> > + * The field should be set to NULL by the FFmpeg libs before passing
> > a frame
> > + * to the user.
> > + */
> > +AVBufferRef *private_ref;
> >  } AVFrame;
> >
> >  #if FF_API_FRAME_GET_SET
> > --
> > 2.15.0
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> 
> 
> This makes it sound as if it would get used and can't be overwritten (e.g.
> feeding frames from a decoder to an encoder and zeroing this field would
> make things go bad).

I think i do not understand how you arrive at this from the documentation


> 
> Just say its temporary storage only and that it doesn't get used after
> decoding or outputting.

Thats would not be correct.
this field can be used by libavfilter for example and that also makes it
important to clarify who is respnsible for clearing it

Because if libavcodec doesnt clear it and its passed into libavfilter
which also doesnt clear it that would be bad

with one field per lib thats less an issue as there would be no type
incompatibility

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

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.


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

Re: [FFmpeg-devel] [PATCH v5] pngdec: expose gAMA and cHRM chunks as side/meta data

[James Almer]

On 11/8/2017 11:32 PM, Rostislav Pehlivanov wrote:
> Signed-off-by: Rostislav Pehlivanov 
> ---
>  libavcodec/pngdec.c | 39 ++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> index 0d6612ccca..0a9a248ee7 100644
> --- a/libavcodec/pngdec.c
> +++ b/libavcodec/pngdec.c
> @@ -25,6 +25,7 @@
>  #include "libavutil/bprint.h"
>  #include "libavutil/imgutils.h"
>  #include "libavutil/stereo3d.h"
> +#include "libavutil/mastering_display_metadata.h"
>  
>  #include "avcodec.h"
>  #include "bytestream.h"
> @@ -1165,7 +1166,7 @@ static int decode_frame_common(AVCodecContext *avctx, 
> PNGDecContext *s,
>  AVDictionary **metadatap = NULL;
>  uint32_t tag, length;
>  int decode_next_dat = 0;
> -int ret;
> +int i, ret;
>  
>  for (;;) {
>  length = bytestream2_get_bytes_left(&s->gb);
> @@ -1287,6 +1288,42 @@ static int decode_frame_common(AVCodecContext *avctx, 
> PNGDecContext *s,
>  goto fail;
>  break;
>  }
> +case MKTAG('c', 'H', 'R', 'M'): {
> +AVMasteringDisplayMetadata *mdm = 
> av_mastering_display_metadata_create_side_data(p);
> +if (!mdm) {
> +ret = AVERROR(ENOMEM);
> +goto fail;
> +}
> +
> +mdm->white_point[0] = av_make_q(bytestream2_get_be32(&s->gb), 
> 10);
> +mdm->white_point[1] = av_make_q(bytestream2_get_be32(&s->gb), 
> 10);
> +
> +/* RGB Primaries */
> +for (i = 0; i < 3; i++) {
> +mdm->display_primaries[i][0] = 
> av_make_q(bytestream2_get_be32(&s->gb), 10);
> +mdm->display_primaries[i][1] = 
> av_make_q(bytestream2_get_be32(&s->gb), 10);
> +}
> +
> +mdm->has_primaries = 1;
> +bytestream2_skip(&s->gb, 4); /* crc */
> +break;
> +}
> +case MKTAG('g', 'A', 'M', 'A'): {
> +AVBPrint bp;
> +char *gamma_str;
> +int num = bytestream2_get_be32(&s->gb);
> +
> +av_bprint_init(&bp, 0, -1);
> +av_bprintf(&bp, "%i/%i", num, 10);
> +av_bprint_finalize(&bp, (char **)&gamma_str);

No need for a cast if gamma_str is already char*

> +if (!gamma_str)
> +return AVERROR(ENOMEM);
> +
> +av_dict_set(&p->metadata, "gamma", gamma_str, 0);

Add the AV_DICT_DONT_STRDUP_VAL flag, otherwise gamma_str will leak.

> +
> +bytestream2_skip(&s->gb, 4); /* crc */
> +break;
> +}
>  case MKTAG('I', 'E', 'N', 'D'):
>  if (!(s->pic_state & PNG_ALLIMAGE))
>  av_log(avctx, AV_LOG_ERROR, "IEND without all image\n");
> 

LGTM aside from the above.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel