from:"Aaron"

When parsing a fragmented MP4 with the use_mfra_for option set so that
the mfra box is parsed we currently continue to parse all the top level
boxes in the mp4. We also avoid using the index in mov_seek_fragment
unless the fragment index is marked as complete, which the mfra parsing
code never does.

This updates the mfra parsing code to mark the fragment index as
complete, similar to the way the sidx parsing code does.

With this change in place mov_read_default stops parsing top level boxes
in a fragmented mp4 and uses the mfra data for mov_seek_fragment calls
(assuming a use_mfra_for option is passed). When loading the file over a
network this results in significantly better performance since
avformat_open_input doesn't require a scan of the entire file.

Signed-off-by: Aaron Boushley 
---
 libavformat/mov.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index ec57a05803..1def594626 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -7344,6 +7344,8 @@ static int mov_read_mfra(MOVContext *c, AVIOContext
*f)
 if (ret < 0)
 goto fail;
 } while (!ret);
+
+c->frag_index.complete = 1;
 ret = 0;
 fail:
 seek_ret = avio_seek(f, original_pos, SEEK_SET);
-- 
2.18.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavformat/mov: Speed up fragmented mp4 parsing

On Mon, Nov 5, 2018 at 2:21 PM Michael Niedermayer
 wrote:
>
> On Mon, Nov 05, 2018 at 11:17:25AM -0800, Aaron Boushley wrote:
> > When parsing a fragmented MP4 with the use_mfra_for option set so that
> > the mfra box is parsed we currently continue to parse all the top level
> > boxes in the mp4. We also avoid using the index in mov_seek_fragment
> > unless the fragment index is marked as complete, which the mfra parsing
> > code never does.
> >
> > This updates the mfra parsing code to mark the fragment index as
> > complete, similar to the way the sidx parsing code does.
> >
> > With this change in place mov_read_default stops parsing top level boxes
> > in a fragmented mp4 and uses the mfra data for mov_seek_fragment calls
> > (assuming a use_mfra_for option is passed). When loading the file over a
> > network this results in significantly better performance since
> > avformat_open_input doesn't require a scan of the entire file.
> >
> > Signed-off-by: Aaron Boushley 
> > ---
> >  libavformat/mov.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index ec57a05803..1def594626 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -7344,6 +7344,8 @@ static int mov_read_mfra(MOVContext *c, AVIOContext
> > *f)
> >  if (ret < 0)
>
> patch is corrupted by a newline
What's the best way to address this, submit a new patch email thread?
Or is there a way to fix this thread?
[...]

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

Re: [FFmpeg-devel] [PATCH] libavformat/mov: Speed up fragmented mp4 parsing

On Mon, Nov 5, 2018 at 3:13 PM Carl Eugen Hoyos  wrote:
>
> 2018-11-06 0:03 GMT+01:00, Aaron Boushley :
> > On Mon, Nov 5, 2018 at 2:21 PM Michael Niedermayer
> >  wrote:
>
> >> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> >> > index ec57a05803..1def594626 100644
> >> > --- a/libavformat/mov.c
> >> > +++ b/libavformat/mov.c
> >> > @@ -7344,6 +7344,8 @@ static int mov_read_mfra(MOVContext *c,
> >> > AVIOContext
> >> > *f)
> >> >  if (ret < 0)
> >>
> >> patch is corrupted by a newline
> > What's the best way to address this, submit a new patch email thread?
> > Or is there a way to fix this thread?
>
> Send the patch again as an attachment instead of inlined.

Let me know if there's any problems with this one.

Aaron


libavformat-mov-fragmented-mp4.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavformat/mov: Speed up fragmented mp4 parsing

On Mon, Nov 5, 2018 at 3:25 PM Aaron Boushley  wrote:
>
> On Mon, Nov 5, 2018 at 3:13 PM Carl Eugen Hoyos  wrote:
> >
> > 2018-11-06 0:03 GMT+01:00, Aaron Boushley :
> > > On Mon, Nov 5, 2018 at 2:21 PM Michael Niedermayer
> > >  wrote:
> >
> > >> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > >> > index ec57a05803..1def594626 100644
> > >> > --- a/libavformat/mov.c
> > >> > +++ b/libavformat/mov.c
> > >> > @@ -7344,6 +7344,8 @@ static int mov_read_mfra(MOVContext *c,
> > >> > AVIOContext
> > >> > *f)
> > >> >  if (ret < 0)
> > >>
> > >> patch is corrupted by a newline
> > > What's the best way to address this, submit a new patch email thread?
> > > Or is there a way to fix this thread?
> >
> > Send the patch again as an attachment instead of inlined.
>
> Let me know if there's any problems with this one.

Looks like https://patchwork.ffmpeg.org/patch/10942/ didn't update
with the attached version, so I'm going to try a new thread.

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

[FFmpeg-devel] [PATCH] libavformat/mov: Speed up fragmented mp4 parsing (Take 2)

Sorry for the second thread, first one was corrupted with a newline 
because I attempted an inline patch. Hopefully the mime-type will be set 
correctly on this one.

---
When parsing a fragmented MP4 with the use_mfra_for option set so that
the mfra box is parsed we currently continue to parse all the top level
boxes in the mp4. We also avoid using the index in mov_seek_fragment
unless the fragment index is marked as complete, which the mfra parsing
code never does.

This updates the mfra parsing code to mark the fragment index as
complete, similar to the way the sidx parsing code does.

With this change in place mov_read_default stops parsing top level boxes
in a fragmented mp4 and uses the mfra data for mov_seek_fragment calls
(assuming a use_mfra_for option is passed). When loading the file over a
network this results in significantly better performance since
avformat_open_input doesn't require a scan of the entire file.

Aaron


From 61412c5990037484e13c7daa60d83b8ba548b8a6 Mon Sep 17 00:00:00 2001
From: Aaron Boushley 
Date: Mon, 5 Nov 2018 11:02:00 -0800
Subject: [PATCH] libavformat/mov: Speed up fragmented mp4 parsing

When parsing a fragmented MP4 with the use_mfra_for option set so that
the mfra box is parsed we currently continue to parse all the top level
boxes in the mp4. We also avoid using the index in mov_seek_fragment
unless the fragment index is marked as complete, which the mfra parsing
code never does.

This updates the mfra parsing code to mark the fragment index as
complete, similar to the way the sidx parsing code does.

With this change in place mov_read_default stops parsing top level boxes
in a fragmented mp4 and uses the mfra data for mov_seek_fragment calls
(assuming a use_mfra_for option is passed). When loading the file over a
network this results in significantly better performance since
avformat_open_input doesn't require a scan of the entire file.

Signed-off-by: Aaron Boushley 
---
 libavformat/mov.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index ec57a05803..34ba7ffe61 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -7344,6 +7344,7 @@ static int mov_read_mfra(MOVContext *c, AVIOContext *f)
 if (ret < 0)
 goto fail;
 } while (!ret);
+c->frag_index.complete = 1;
 ret = 0;
 fail:
 seek_ret = avio_seek(f, original_pos, SEEK_SET);
-- 
2.18.0

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

Re: [FFmpeg-devel] [PATCH] libavformat/mov: Speed up fragmented mp4 parsing (Take 2)

2018-11-13 Thread Aaron Boushley

On Mon, Nov 5, 2018 at 3:48 PM Aaron Boushley  wrote:
>
> Sorry for the second thread, first one was corrupted with a newline
> because I attempted an inline patch. Hopefully the mime-type will be set
> correctly on this one.
> ---
> When parsing a fragmented MP4 with the use_mfra_for option set so that
> the mfra box is parsed we currently continue to parse all the top level
> boxes in the mp4. We also avoid using the index in mov_seek_fragment
> unless the fragment index is marked as complete, which the mfra parsing
> code never does.
>
> This updates the mfra parsing code to mark the fragment index as
> complete, similar to the way the sidx parsing code does.
>
> With this change in place mov_read_default stops parsing top level boxes
> in a fragmented mp4 and uses the mfra data for mov_seek_fragment calls
> (assuming a use_mfra_for option is passed). When loading the file over a
> network this results in significantly better performance since
> avformat_open_input doesn't require a scan of the entire file.
>
> Aaron

After further investigation this patch can result in the
AVFormatContext duration field being set incorrectly.

Because the mfra box doesn't contain the duration for the fragment we
have no way of knowing the duration from just reading the mfra. I will
investigate a patch where after reading all of the tfra boxes inside
the mfra we then read the moof for the last fragment. This should
allow us to get the duration without needing to scan every moof.

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

Re: [FFmpeg-devel] [PATCH] libavfilter/vf_drawtext: Avoid undefined behavior from GET_UTF8

2019-09-07 Thread Aaron Boushley

Can I get a review on this. Anywhere we have code calling out and
performing undefined behavior we should clean that up.

Aaron

On Sat, Jul 27, 2019 at 7:58 AM Aaron Boushley  wrote:
>
> The vf_drawtext filter uses the GET_UTF8 macro in multiple locations.
> Each of these use `continue;` as the error handler. However the
> documentation for the GET_UTF8 macro states "ERROR should not contain
> a loop control statement which could interact with the internal while
> loop, and should force an exit from the macro code (e.g. through a
> goto or a return) in order to prevent undefined results."
>
> This patch adjusts vf_drawtext to use goto error handlers similar to
> other locations in ffmpeg.
>
> Aaron
>
> PS Sorry for having to send again, sent from the wrong address last
> time, so patchwork didn't pick it up.
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH] libsrt: add password ACL support using new streamID specification

2019-09-09 Thread Aaron Boxer

Hello!
SRT has added a new stream ID specification allowing extraction of various
fields from the user password. This patch adds support for using an ACL to
check user credentials - base on this Haivision sample code:
https://github.com/Haivision/srt/blob/master/testing/srt-test-live.cpp

Thanks,
Aaron
From 8a11e0d673872049b8260cb08f3c7cb40f9a1297 Mon Sep 17 00:00:00 2001
From: Aaron Boxer 
Date: Wed, 14 Aug 2019 08:26:17 -0400
Subject: [PATCH] libsrt: support streamid spec

---
 libavformat/libsrt.c | 135 +++
 1 file changed, 135 insertions(+)

diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c
index b5568089fa..d597e5fce4 100644
--- a/libavformat/libsrt.c
+++ b/libavformat/libsrt.c
@@ -34,6 +34,9 @@
 #include "os_support.h"
 #include "url.h"
 
+
+static srt_listen_callback_fn libsrt_listen_callback;
+
 /* This is for MPEG-TS and it's a default SRTO_PAYLOADSIZE for SRTT_LIVE (8 TS packets) */
 #ifndef SRT_LIVE_DEFAULT_PAYLOAD_SIZE
 #define SRT_LIVE_DEFAULT_PAYLOAD_SIZE 1316
@@ -62,6 +65,7 @@ typedef struct SRTContext {
 int64_t maxbw;
 int pbkeylen;
 char *passphrase;
+char* user_passphrase_list;
 int mss;
 int ffs;
 int ipttl;
@@ -101,6 +105,7 @@ static const AVOption libsrt_options[] = {
 { "maxbw",  "Maximum bandwidth (bytes per second) that the connection can use", OFFSET(maxbw),AV_OPT_TYPE_INT64,{ .i64 = -1 }, -1, INT64_MAX, .flags = D|E },
 { "pbkeylen",   "Crypto key len in bytes {16,24,32} Default: 16 (128-bit)", OFFSET(pbkeylen), AV_OPT_TYPE_INT,  { .i64 = -1 }, -1, 32,.flags = D|E },
 { "passphrase", "Crypto PBKDF2 Passphrase size[0,10..64] 0:disable crypto", OFFSET(passphrase),   AV_OPT_TYPE_STRING,   { .str = NULL },  .flags = D|E },
+{ "user_passphrase_list", "Comma separated list users and passphrases, of form usrr1=pass1,usr2=pass2,...", OFFSET(user_passphrase_list),   AV_OPT_TYPE_STRING,   { .str = NULL },  .flags = D|E },
 { "mss","The Maximum Segment Size", OFFSET(mss),  AV_OPT_TYPE_INT,  { .i64 = -1 }, -1, 1500,  .flags = D|E },
 { "ffs","Flight flag size (window size) (in bytes)",OFFSET(ffs),  AV_OPT_TYPE_INT,  { .i64 = -1 }, -1, INT_MAX,   .flags = D|E },
 { "ipttl",  "IP Time To Live",  OFFSET(ipttl),AV_OPT_TYPE_INT,  { .i64 = -1 }, -1, 255,   .flags = D|E },
@@ -196,10 +201,134 @@ static int libsrt_network_wait_fd_timeout(URLContext *h, int eid, int fd, int wr
 }
 }
 
+typedef struct _libsrt_parsed_param {
+char *key;
+char *val;
+} libsrt_parsed_param;
+
+
+/**
+ * Parse a key-value string into an array of key/value structs.
+ *
+ * The key-value string should be a null terminated string of parameters separated
+ * by a delimiter. Each parameter are checked for the equal sign character.
+ * If the equal sign character appears in the parameter, it will be used as a null terminator
+ * and the part that comes after it will be the value of the parameter.
+ *
+ *
+ * param: keyvalue: the key-value string to parse. The string will be modified.
+ * param: delimiter:the character that separates the key/value pairs
+ *  from each other.
+ * param: params:   an array of parsed_param structs to hold the result.
+ * param: max_params: maximum number of parameters to parse.
+ *
+ * Return:the number of parsed items. -1 if there was an error.
+ */
+static int libsrt_parse_key_value(char *keyvalue, const char* delimiter,
+libsrt_parsed_param *params, int max_params)
+{
+int i = 0;
+char *token = NULL;
+
+if (!keyvalue || *keyvalue == '\0')
+return -1;
+if (!params || max_params == 0)
+return 0;
+
+token = strtok( keyvalue, delimiter );
+while (token != NULL && i < max_params) {
+params[i].key = token;
+params[i].val = NULL;
+if ((params[i].val = strchr( params[i].key, '=' )) != NULL) {
+size_t val_len = strlen( params[i].val );
+/* make key into a zero-delimited string */
+*(params[i].val) = '\0';
+/* make sure val is not empty */
+if (val_len > 1) {
+params[i].val++;
+/* make sure key is not empty */
+if (params[i].key[0])
+i++;
+};
+}
+token = strtok( NULL, delimiter );
+}
+
+return i;
+}
+
+/* callback to parse streamid */
+static int libsrt_listen_callbac

Re: [FFmpeg-devel] [PATCH] libavfilter/vf_drawtext: Avoid undefined behavior from GET_UTF8

2019-09-09 Thread Aaron Boushley

I'm open to suggestions of alternatives.

I want sure if I could do a goto if there was no code at that location so I
chose to add a log statement figuring that would be helpful if you had
problems.

I chose debug because previously the character was completely ignored, so
didn't want to go from no notification to an error log.

I could also just put a continue call there, or do more research to figure
out if I can go to without having any content on the line.

Suggestions for what I should do differently with that context?

Aaron

On Mon, Sep 9, 2019, 9:13 AM Michael Niedermayer 
wrote:

> On Sat, Jul 27, 2019 at 07:58:48AM -0700, Aaron Boushley wrote:
> > The vf_drawtext filter uses the GET_UTF8 macro in multiple locations.
> > Each of these use `continue;` as the error handler. However the
> > documentation for the GET_UTF8 macro states "ERROR should not contain
> > a loop control statement which could interact with the internal while
> > loop, and should force an exit from the macro code (e.g. through a
> > goto or a return) in order to prevent undefined results."
> >
> > This patch adjusts vf_drawtext to use goto error handlers similar to
> > other locations in ffmpeg.
> >
> > Aaron
> >
> > PS Sorry for having to send again, sent from the wrong address last
> > time, so patchwork didn't pick it up.
> >
>
> >  vf_drawtext.c |   17 ++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> > 04550ed40f98ec78c2719de31da889e92ff42f45
> libavfilter-drawtext-avoid-undefined-behavior.patch
> > From efdc96ace59d676e76434499a399d1d7df7fa093 Mon Sep 17 00:00:00 2001
> > From: Aaron Boushley 
> > Date: Fri, 26 Jul 2019 15:49:36 -0700
> > Subject: [PATCH] libavfilter/drawtext: avoid undefined behavior with
> GET_UTF8
> >
> > Currently the GET_UTF8 usage in drawtext use a continue to skip invalid
> > characters in a string. The macro definition states that using a loop
> > control statement results in undefined behavior since the macro itself
> > uses a loop.
> >
> > This switches drawtext to use a goto statement similar to other usages
> > of GET_UTF8 in other parts of ffmpeg.
> >
> > Signed-off-by: Aaron Boushley 
> > ---
> >  libavfilter/vf_drawtext.c | 17 ++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
> > index 8f4badbdb5..fd2ba84d12 100644
> > --- a/libavfilter/vf_drawtext.c
> > +++ b/libavfilter/vf_drawtext.c
> > @@ -1223,7 +1223,7 @@ static int draw_glyphs(DrawTextContext *s, AVFrame
> *frame,
> >  for (i = 0, p = text; *p; i++) {
> >  FT_Bitmap bitmap;
> >  Glyph dummy = { 0 };
> > -GET_UTF8(code, *p++, continue;);
> > +GET_UTF8(code, *p++, goto invalid_drawing;);
> >
> >  /* skip new line chars, just go to new line */
> >  if (code == '\n' || code == '\r' || code == '\t')
> > @@ -1248,6 +1248,9 @@ static int draw_glyphs(DrawTextContext *s, AVFrame
> *frame,
> >bitmap.width, bitmap.rows,
> >bitmap.pixel_mode == FT_PIXEL_MODE_MONO ? 0 : 3,
> >0, x1, y1);
> > +continue;
> > +invalid_drawing:
> > +av_log(s, AV_LOG_DEBUG, "Invalid UTF8 character while drawing
> glyphs\n");
> >  }
>
> Why does this add av_log() calls ?
> This seems unrelated to the bug fix. Also why are they debug level ?
> if a user draws a string she provided that could be argued to be an error
> the user wants to know about. But maybe iam missing something
> Also they probably should provide more information about where the
> invalid character is. It may be hard to find in a long text for some users
>
> Thanks
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The educated differ from the uneducated as much as the living from the
> dead. -- Aristotle
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH] Fixed bug encountered when decoding interlaced video

2017-03-26 Thread Aaron Levinson

Hopefully I went through the patch process correctly, as this is the first time 
that I've done this.  I didn't submit a ticket regarding this issue, but a 
detailed description of the problem can be found below.  It is possible that 
there is an already existing ticket that corresponds to this issue, but I 
didn't check.  I appreciate any comments.

Thanks,
Aaron Levinson
--- next part 
From 66796ca78e9b307ba0a8a7c33bd89d006c44ddf8 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Sun, 26 Mar 2017 01:57:19 -0700
Subject: [PATCH] Fixed bug encountered when decoding interlaced video.

Purpose: Fixed bug encountered when decoding interlaced video.  In
 some cases, ffmpeg was treating it as progressive.  As a result of
 the change, the issues with interlaced video are fixed.  It is also
 possible that this change fixes other issues besides interlaced
 video.

Detailed description of problem: I generated two mpegts files, each
 with H.264 encoded 1080i59.94 video.  One of the mpegts files had an
 AAC audio stream, while the other mpegts file had an Opus audio
 stream.  When using the following command to play back either file:
 ffmpeg -i  -f decklink -pix_fmt uyvy422 "DeckLink SDI
 4K", I noticed that with the mpegts file with the AAC audio stream,
 it would correctly select an interlaced video mode for the video
 output stream, but with the mpegts file with the Opus audio stream,
 it would use a progressive video mode (1080p29.97) for the video
 output stream.  The reason why it works for AAC (usually, not always)
 and doesn't work for Opus has to do with the order in which
 init_output_stream() is called for each output stream.  When AAC is
 used, it usually calls init_output_stream() for video before it calls
 it for audio.  When Opus is used, it usually calls
 init_output_stream() for audio before it calls it for video.  When
 init_output_stream() is called for audio before it is called for
 video, this results in check_init_output_file() being called
 successfully (as in, all output streams are initialized, and it
 writes the header) before it creates the filtered frame at the end of
 reap_filters().  The process of creating and processing the filtered
 frame is what sets up interlaced video properly, but in this
 particular case, that happens after writing the header in
 check_init_output_file(), and by that point, it is too late.

Testing notes: Ran regression tests, all of which passed

Comments:
ffmpeg.c:
a) Enhanced init_output_stream() to take an additional input indicating
 whether or not check_init_output_file() should be called.  There are
 other places that call init_output_stream(), and it was important to
 make sure that these continued to work properly.
b) Adjusted reap_filters() such that, in the case that
 init_output_stream() is called, it indicates that it should not call
 check_init_output_file() in init_output_stream().  Instead, it makes
 the call to check_init_output_file() directly, but after it does the
 filtered frame setup and processing.

---
 ffmpeg.c | 42 ++
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/ffmpeg.c b/ffmpeg.c
index 62e7d82..a5ac5e0 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -1395,7 +1395,7 @@ static void do_video_stats(OutputStream *ost, int 
frame_size)
 }
 }
 
-static int init_output_stream(OutputStream *ost, char *error, int error_len);
+static int init_output_stream(OutputStream *ost, char *error, int error_len, 
int do_check_output_file);
 
 static void finish_output_stream(OutputStream *ost)
 {
@@ -1410,6 +1410,8 @@ static void finish_output_stream(OutputStream *ost)
 }
 }
 
+static int check_init_output_file(OutputFile *of, int file_index);
+
 /**
  * Get and encode new output from any of the filtergraphs, without causing
  * activity.
@@ -1428,6 +1430,7 @@ static int reap_filters(int flush)
 AVFilterContext *filter;
 AVCodecContext *enc = ost->enc_ctx;
 int ret = 0;
+int did_init_output_stream = 0;
 
 if (!ost->filter || !ost->filter->graph->graph)
 continue;
@@ -1435,12 +1438,14 @@ static int reap_filters(int flush)
 
 if (!ost->initialized) {
 char error[1024] = "";
-ret = init_output_stream(ost, error, sizeof(error));
+ret = init_output_stream(ost, error, sizeof(error), 0);
 if (ret < 0) {
 av_log(NULL, AV_LOG_ERROR, "Error initializing output stream 
%d:%d -- %s\n",
ost->file_index, ost->index, error);
 exit_program(1);
 }
+
+did_init_output_stream = 1;
 }
 
 if (!ost->filtered_frame && !(ost->filtered_frame = av_frame_alloc())) 
{
@@ -1517,6 +1522,25 @@ static int reap_filters(int flush)
 
 av_frame_unref(filtered_fr

Re: [FFmpeg-devel] [PATCH] Fixed bug encountered when decoding interlaced video

2017-03-26 Thread Aaron Levinson


On 3/26/2017 4:41 AM, Matthias Hunstock wrote:

Am 26.03.2017 um 11:50 schrieb Aaron Levinson:

When using the following command to play back either file:
 ffmpeg -i  -f decklink -pix_fmt uyvy422 "DeckLink SDI
 4K", I noticed that with the mpegts file with the AAC audio stream,
 it would correctly select an interlaced video mode for the video
 output stream, but with the mpegts file with the Opus audio stream,
 it would use a progressive video mode (1080p29.97) for the video
 output stream.


Which FFmpeg version did you test this with?

There was a change related to this just short time ago.

Does it happen with current git HEAD?

Matthias


This issue occurs with the current git HEAD.  I'm aware of the 
Blackmagic improvement that was added in February to add support for 
interlaced video modes on output, and actually that's one of the reasons 
why I'm using the latest git sources, as opposed to, say, 3.2.4.  This 
particular issue has nothing to do with Blackmagic, and I only used 
Blackmagic in the example that reproduces the bug because it is 
something that can be reproduced on both Windows and Linux (and 
presumably also on OS/X).  The issue also occurs if I do something like 
-f rawvideo out.avi on Windows, and I'm sure that there are plenty of 
other examples.


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

[FFmpeg-devel] [PATCH] movenc: Add support for writing st3d and sv3d boxes.

2017-03-27 Thread Aaron Colwell


From 8654212c2f2a3ee404020cf5b948d7db3e6270f2 Mon Sep 17 00:00:00 2001
From: Aaron Colwell 
Date: Mon, 27 Mar 2017 08:00:12 -0700
Subject: [PATCH] movenc: Add support for writing st3d and sv3d boxes.

---
 libavformat/movenc.c | 105 +++
 1 file changed, 105 insertions(+)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 3b4e3b519c..4f408b20fa 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1603,6 +1603,101 @@ static int mov_write_subtitle_tag(AVIOContext *pb, MOVTrack *track)
 return update_size(pb, pos);
 }
 
+static int mov_write_st3d_tag(AVIOContext *pb, AVStereo3D *stereo_3d)
+{
+int8_t stereo_mode;
+
+if (stereo_3d->flags != 0) {
+av_log(pb, AV_LOG_WARNING, "Unsupported stereo_3d flags %x. st3d not written.\n", stereo_3d->flags);
+return 0;
+}
+
+switch (stereo_3d->type) {
+case AV_STEREO3D_2D:
+stereo_mode = 0;
+break;
+case AV_STEREO3D_TOPBOTTOM:
+stereo_mode = 1;
+break;
+case AV_STEREO3D_SIDEBYSIDE:
+stereo_mode = 2;
+break;
+default:
+av_log(pb, AV_LOG_WARNING, "Unsupported stereo_3d type %d. st3d not written.\n", stereo_3d->type);
+return 0;
+}
+avio_wb32(pb, 13); /* size */
+ffio_wfourcc(pb, "st3d");
+avio_wb32(pb, 0); /* version = 0 & flags = 0 */
+avio_w8(pb, stereo_mode);
+return 13;
+}
+
+static int mov_write_sv3d_tag(AVIOContext *pb, AVSphericalMapping *spherical_mapping)
+{
+int64_t sv3d_pos, svhd_pos, proj_pos;
+
+if (spherical_mapping->projection != AV_SPHERICAL_EQUIRECTANGULAR &&
+spherical_mapping->projection != AV_SPHERICAL_EQUIRECTANGULAR_TILE &&
+spherical_mapping->projection != AV_SPHERICAL_CUBEMAP) {
+av_log(pb, AV_LOG_WARNING, "Unsupported projection %d. sv3d not written.\n", spherical_mapping->projection);
+return 0;
+}
+
+sv3d_pos = avio_tell(pb);
+avio_wb32(pb, 0);  /* size */
+ffio_wfourcc(pb, "sv3d");
+
+svhd_pos = avio_tell(pb);
+avio_wb32(pb, 0);  /* size */
+ffio_wfourcc(pb, "svhd");
+avio_wb32(pb, 0); /* version = 0 & flags = 0 */
+avio_put_str(pb, LIBAVFORMAT_IDENT); /* metadata_source */
+update_size(pb, svhd_pos);
+
+proj_pos = avio_tell(pb);
+avio_wb32(pb, 0); /* size */
+ffio_wfourcc(pb, "proj");
+
+avio_wb32(pb, 24); /* size */
+ffio_wfourcc(pb, "prhd");
+avio_wb32(pb, 0); /* version = 0 & flags = 0 */
+avio_wb32(pb, spherical_mapping->yaw);
+avio_wb32(pb, spherical_mapping->pitch);
+avio_wb32(pb, spherical_mapping->roll);
+
+switch (spherical_mapping->projection) {
+case AV_SPHERICAL_EQUIRECTANGULAR:
+avio_wb32(pb, 28);/* size */
+ffio_wfourcc(pb, "equi");
+avio_wb32(pb, 0); /* version = 0 & flags = 0 */
+avio_wb32(pb, 0); /* projection_bounds_top */
+avio_wb32(pb, 0); /* projection_bounds_bottom */
+avio_wb32(pb, 0); /* projection_bounds_left */
+avio_wb32(pb, 0); /* projection_bounds_right */
+break;
+case AV_SPHERICAL_EQUIRECTANGULAR_TILE:
+avio_wb32(pb, 28);/* size */
+ffio_wfourcc(pb, "equi");
+avio_wb32(pb, 0); /* version = 0 & flags = 0 */
+avio_wb32(pb, spherical_mapping->bound_top);
+avio_wb32(pb, spherical_mapping->bound_bottom);
+avio_wb32(pb, spherical_mapping->bound_left);
+avio_wb32(pb, spherical_mapping->bound_right);
+break;
+case AV_SPHERICAL_CUBEMAP:
+avio_wb32(pb, 20);/* size */
+ffio_wfourcc(pb, "cbmp");
+avio_wb32(pb, 0); /* version = 0 & flags = 0 */
+avio_wb32(pb, 0); /* layout */
+avio_wb32(pb, spherical_mapping->padding); /* padding */
+break;
+}
+update_size(pb, proj_pos);
+
+return update_size(pb, sv3d_pos);
+}
+
 static int mov_write_pasp_tag(AVIOContext *pb, MOVTrack *track)
 {
 AVRational sar;
@@ -1873,6 +1968,16 @@ static int mov_write_video_tag(AVIOContext *pb, MOVMuxContext *mov, MOVTrack *tr
 av_log(mov->fc, AV_LOG_WARNING, "Not writing 'colr' atom. Format is not MOV or MP4.\n");
 }
 
+if (mov->fc->strict_std_compliance <= FF_COMPLIANCE_EXPERIMENTAL) {
+  AVStereo3D* stereo_3d = (AVStereo3D*) av_stream_get_side_data(track->st, AV_PKT_DATA_STEREO3D, NULL);
+  AVSphericalMapping* spherical_mapping = (AVSphericalMapping*)av_stream_get_side_data(track->st, AV_PKT_DATA_SPHERICAL, NULL);
+
+  if (stereo_3d)
+  mov_write_st3d_tag(pb, stereo_3d);
+  if (spherical_mapping)
+  mov_write_sv3d_tag(pb, spherical_mapping);
+}
+
 if (track->par->sample_aspect_ratio.den && track->par->sample_aspect_ratio.num) {
 mov_write_pasp_tag(pb, track);
 }
-- 
2.12.1.578.ge9c3154ca4-goog

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

Re: [FFmpeg-devel] [PATCH] movenc: Add support for writing st3d and sv3d boxes.

2017-03-27 Thread Aaron Colwell

Made all suggested changes and attached a new patch. Thanks for the quick
review.

Aaron

On Mon, Mar 27, 2017 at 10:00 AM James Almer  wrote:

> On 3/27/2017 1:02 PM, Aaron Colwell wrote:
> >
> > 0001-movenc-Add-support-for-writing-st3d-andsv3d-boxes.patch
> >
> >
> > From 8654212c2f2a3ee404020cf5b948d7db3e6270f2 Mon Sep 17 00:00:00 2001
> > From: Aaron Colwell 
> > Date: Mon, 27 Mar 2017 08:00:12 -0700
> > Subject: [PATCH] movenc: Add support for writing st3d and sv3d boxes.
> >
> > ---
> >  libavformat/movenc.c | 105
> +++
> >  1 file changed, 105 insertions(+)
> >
> > diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> > index 3b4e3b519c..4f408b20fa 100644
> > --- a/libavformat/movenc.c
> > +++ b/libavformat/movenc.c
> > @@ -1603,6 +1603,101 @@ static int mov_write_subtitle_tag(AVIOContext
> *pb, MOVTrack *track)
> >  return update_size(pb, pos);
> >  }
> >
> > +static int mov_write_st3d_tag(AVIOContext *pb, AVStereo3D *stereo_3d)
> > +{
> > +int8_t stereo_mode;
> > +
> > +if (stereo_3d->flags != 0) {
> > +av_log(pb, AV_LOG_WARNING, "Unsupported stereo_3d flags %x.
> st3d not written.\n", stereo_3d->flags);
> > +return 0;
> > +}
> > +
> > +switch (stereo_3d->type) {
> > +case AV_STEREO3D_2D:
> > +stereo_mode = 0;
> > +break;
> > +case AV_STEREO3D_TOPBOTTOM:
> > +stereo_mode = 1;
> > +break;
> > +case AV_STEREO3D_SIDEBYSIDE:
> > +stereo_mode = 2;
> > +break;
> > +default:
> > +av_log(pb, AV_LOG_WARNING, "Unsupported stereo_3d type %d. st3d
> not written.\n", stereo_3d->type);
>
> You could use av_stereo3d_type_name(stereo_3d->type) here.
>

Done.


>
> > +return 0;
> > +}
> > +avio_wb32(pb, 13); /* size */
> > +ffio_wfourcc(pb, "st3d");
> > +avio_wb32(pb, 0); /* version = 0 & flags = 0 */
> > +avio_w8(pb, stereo_mode);
> > +return 13;
> > +}
> > +
> > +static int mov_write_sv3d_tag(AVIOContext *pb, AVSphericalMapping
> *spherical_mapping)
> > +{
> > +int64_t sv3d_pos, svhd_pos, proj_pos;
> > +
> > +if (spherical_mapping->projection != AV_SPHERICAL_EQUIRECTANGULAR &&
> > +spherical_mapping->projection !=
> AV_SPHERICAL_EQUIRECTANGULAR_TILE &&
> > +spherical_mapping->projection != AV_SPHERICAL_CUBEMAP) {
> > +av_log(pb, AV_LOG_WARNING, "Unsupported projection %d. sv3d not
> written.\n", spherical_mapping->projection);
> > +return 0;
> > +}
> > +
> > +sv3d_pos = avio_tell(pb);
> > +avio_wb32(pb, 0);  /* size */
> > +ffio_wfourcc(pb, "sv3d");
> > +
> > +svhd_pos = avio_tell(pb);
> > +avio_wb32(pb, 0);  /* size */
> > +ffio_wfourcc(pb, "svhd");
> > +avio_wb32(pb, 0); /* version = 0 & flags = 0 */
> > +avio_put_str(pb, LIBAVFORMAT_IDENT); /* metadata_source */
>
> You need to use some hardcoded string like "Lavf" or nothing at all if
> AVFMT_FLAG_BITEXACT is set in (AVFormatContext*)->flags.
>

Done.


>
> > +update_size(pb, svhd_pos);
> > +
> > +proj_pos = avio_tell(pb);
> > +avio_wb32(pb, 0); /* size */
> > +ffio_wfourcc(pb, "proj");
> > +
> > +avio_wb32(pb, 24); /* size */
> > +ffio_wfourcc(pb, "prhd");
> > +avio_wb32(pb, 0); /* version = 0 & flags = 0 */
> > +avio_wb32(pb, spherical_mapping->yaw);
> > +avio_wb32(pb, spherical_mapping->pitch);
> > +avio_wb32(pb, spherical_mapping->roll);
> > +
> > +switch (spherical_mapping->projection) {
> > +case AV_SPHERICAL_EQUIRECTANGULAR:
> > +avio_wb32(pb, 28);/* size */
> > +ffio_wfourcc(pb, "equi");
> > +avio_wb32(pb, 0); /* version = 0 & flags = 0 */
> > +avio_wb32(pb, 0); /* projection_bounds_top */
> > +avio_wb32(pb, 0); /* projection_bounds_bottom */
> > +avio_wb32(pb, 0); /* projection_bounds_left */
> > +avio_wb32(pb, 0); /* projection_bounds_right */
> > +break;
> > +case AV_SPHERICAL_EQUIRECTANGULAR_TILE:
> > +avio_wb32(pb, 28);/* size */
> > +ffio_wfourcc(pb, "equi");
> > +avio_wb32(pb, 0); /* version = 0 & flags = 0 */
>

Re: [FFmpeg-devel] [PATCH][RFC] avutil/spherical: add a flag to signal tiled/padded projections

2017-03-30 Thread Aaron Colwell

On Thu, Mar 30, 2017 at 7:17 AM James Almer  wrote:

> On 3/30/2017 5:54 AM, Vittorio Giovara wrote:
> > On Wed, Mar 29, 2017 at 8:01 PM, James Almer  wrote:
> >> A new field is added to AVSphericalMapping for this purpose,
> >> and is used by both Equirectangular and Cubemap projections.
> >> This is a replacement for duplicate projection enums like
> >> AV_SPHERICAL_EQUIRECTANGULAR_TILE, which are therefore
> >> removed.
> >>
> >> Signed-off-by: James Almer 
> >> ---
> >> This patch depends on the av_spherical_projection_name() patchset for
> >> simplicity purposes.
> >>
> >> Ok, this is an RFC mainly because of the API/ABI break it represents.
> >> The AV_SPHERICAL_EQUIRECTANGULAR_TILE projection is a month and a half
> >> old and not present in any release, plus a major bump is queued as part
> >> of the merges, so i personally think this change is acceptable as is for
> >> such an niche and recent feature.
> >> If not then i can deprecate said projection enum value instead and keep
> >> the current muxer functionality for it for a while. It will need a lot
> >> of preprocessor guards, though.
> >>
> >> The reason for this change is that eventually, a new projection enum
> >> for padded cubemap may be suggested with the same arguments as the ones
> >> used to introduce the one for tiled equirectangular. Then if any new
> >> real projections are added, we'd could end up with duplicate enums for
> >> them as well, when setting a single shared flag would be enough.
> >> Stereo3D avoided a lot of duplicate types with the inverted flag, so i
> >> figured the same should be done here.
> >>
> >> Improved doxy and/or flag name is extremely welcome (Read: needed).
> >
> > I'm against this idea because of explanations given in other threads.
> >
> > The stereo3d case is different because it's a property that can be
> > applied to all types, while the cropped/padded feature applies to
> > current existing projections, not the future ones.
>
> But it could apply. It's fairly likely that cropping/padding of unused
> pixels will be present in future projections.
> And this flag will not necessarily be the only one in the long term.
> You could end up having a new property at some point that may have to
> be signaled in some way. And it could even be present in a projection
> alongside cropping/padding.
>
>
It probably isn't a surprise, but I agree with James here. I think it is
highly likely other projections will have tiling/cropping in them and I
think it is better to reflect this as a flag instead of merging it with the
projection enum.

I'm not 100% sure padding should be rolled into the same flag as cropping,
but I think that is a detail that could be ironed out separately. I think
the flag mechanism itself is a good idea.


> > Additionally there
> > is nothing wrong with having more enum values, since it is likely that
> > future cubemaps layouts will have a different enum value, and not
> > another field to check.
>
> Having a single flags field with dozens of potential flags seems like
> a much cleaner solution than several enum values to list the many
> different ways a single projection can be handled to me.
>

I agree. In my view this addresses the "having to check multiple fields"
objection raised in previous threads because now a single flag can be used
to indicate whether you need to pay attention to the bound_xxx fields or
not. It seems like a more reasonable and extendable compromise to me.

I also don't think different cubemap layouts necessarily should have their
own enum values. This feels roughly akin to merging an audio codec_id and
the channel mapping into a single enum space.

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

[FFmpeg-devel] [PATCH] Made appropriate changes to be able to successfully build C++ files using a Visual C++ build on Windows

From 722cbb5f6544323430d883212ac2e38c4eb94e5f Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Wed, 12 Apr 2017 16:33:39 -0700
Subject: [PATCH] Made appropriate changes to be able to successfully build C++ 
files using a Visual C++ build on Windows.

Purpose:  Made appropriate changes to be able to
 successfully build C++ files using a Visual C++ build on Windows.  Note that
 this is a continuation of the "fixes w32pthreads to support C++" aspect of
 Kyle Schwarz's "Remove pthread dependency" patch that is available at
 https://patchwork.ffmpeg.org/patch/2654/ .  This patch wasn't accepted, and
 as far as I can tell, there was no follow-up after it was rejected.

Notes:  Used Visual Studio 2015 (with update 3) for this.

Comments:

-- compat/w32pthreads.h: Made appropriate changes to w32pthreads.h to
   get it to build when it is being included in a C++ file and built
   with Visual C++.  This is mostly a copy of Kyle Schwarz's patch as
   described above.

-- configure: Made minor modifications to MSVC (Microsoft Visual C++)
   toolchain section to make sure that -Fo$@ is used when building
   object files but when the C++ compiler is used and also to use cl
   for the default C++ compiler.  This is currently only relevant for
   building the Blackmagic/Decklink-related files under avdevice.
---
 compat/w32pthreads.h | 24 
 configure|  3 +++
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/compat/w32pthreads.h b/compat/w32pthreads.h
index 0c9a7fa..a6c699b 100644
--- a/compat/w32pthreads.h
+++ b/compat/w32pthreads.h
@@ -77,7 +77,7 @@ typedef struct pthread_cond_t {
 
 static av_unused unsigned __stdcall attribute_align_arg 
win32thread_worker(void *arg)
 {
-pthread_t *h = arg;
+pthread_t *h = (pthread_t*)arg;
 h->ret = h->func(h->arg);
 return 0;
 }
@@ -270,7 +270,7 @@ static av_unused int pthread_cond_init(pthread_cond_t 
*cond, const void *unused_
 }
 
 /* non native condition variables */
-win32_cond = av_mallocz(sizeof(win32_cond_t));
+win32_cond = (win32_cond_t*)av_mallocz(sizeof(win32_cond_t));
 if (!win32_cond)
 return ENOMEM;
 cond->Ptr = win32_cond;
@@ -288,7 +288,7 @@ static av_unused int pthread_cond_init(pthread_cond_t 
*cond, const void *unused_
 
 static av_unused int pthread_cond_destroy(pthread_cond_t *cond)
 {
-win32_cond_t *win32_cond = cond->Ptr;
+win32_cond_t *win32_cond = (win32_cond_t*)cond->Ptr;
 /* native condition variables do not destroy */
 if (cond_init)
 return 0;
@@ -305,7 +305,7 @@ static av_unused int pthread_cond_destroy(pthread_cond_t 
*cond)
 
 static av_unused int pthread_cond_broadcast(pthread_cond_t *cond)
 {
-win32_cond_t *win32_cond = cond->Ptr;
+win32_cond_t *win32_cond = (win32_cond_t*)cond->Ptr;
 int have_waiter;
 
 if (cond_broadcast) {
@@ -337,7 +337,7 @@ static av_unused int pthread_cond_broadcast(pthread_cond_t 
*cond)
 
 static av_unused int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t 
*mutex)
 {
-win32_cond_t *win32_cond = cond->Ptr;
+win32_cond_t *win32_cond = (win32_cond_t*)cond->Ptr;
 int last_waiter;
 if (cond_wait) {
 cond_wait(cond, mutex, INFINITE);
@@ -369,7 +369,7 @@ static av_unused int pthread_cond_wait(pthread_cond_t 
*cond, pthread_mutex_t *mu
 
 static av_unused int pthread_cond_signal(pthread_cond_t *cond)
 {
-win32_cond_t *win32_cond = cond->Ptr;
+win32_cond_t *win32_cond = (win32_cond_t*)cond->Ptr;
 int have_waiter;
 if (cond_signal) {
 cond_signal(cond);
@@ -400,17 +400,17 @@ static av_unused void w32thread_init(void)
 HANDLE kernel_dll = GetModuleHandle(TEXT("kernel32.dll"));
 /* if one is available, then they should all be available */
 cond_init  = (void (WINAPI*)(pthread_cond_t *))
-GetProcAddress(kernel_dll, "InitializeConditionVariable");
+GetProcAddress((HMODULE)kernel_dll, "InitializeConditionVariable");
 cond_broadcast = (void (WINAPI*)(pthread_cond_t *))
-GetProcAddress(kernel_dll, "WakeAllConditionVariable");
+GetProcAddress((HMODULE)kernel_dll, "WakeAllConditionVariable");
 cond_signal= (void (WINAPI*)(pthread_cond_t *))
-GetProcAddress(kernel_dll, "WakeConditionVariable");
+GetProcAddress((HMODULE)kernel_dll, "WakeConditionVariable");
 cond_wait  = (BOOL (WINAPI*)(pthread_cond_t *, pthread_mutex_t *, 
DWORD))
-GetProcAddress(kernel_dll, "SleepConditionVariableCS");
+GetProcAddress((HMODULE)kernel_dll, "SleepConditionVariableCS");
 initonce_begin = (BOOL (WINAPI*)(pthread_once_t *, DWORD, BOOL *, void **))
-GetProcAddress(kernel_dll, "InitOnceBeginInitialize");
+GetProcAddress((HMODULE)kernel_dll, "InitOnceBeginInitialize&qu

[FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency. Made minor changes to get the decklink avdevice code to build using Visual C++.

From bcc0ec4a0bb64e74b9c9369087708f55569bf50e Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Wed, 12 Apr 2017 16:41:53 -0700
Subject: [PATCH] avdevice/decklink: Removed pthread dependency.  Made minor 
changes to get the decklink avdevice code to build using Visual C++.

Purpose: avdevice/decklink: Removed pthread dependency by
 replacing semaphore used in code appropriately.  Doing so makes it easier to
 build ffmpeg using Visual C++ on Windows.  In addition, made other minor
 changes to get the decklink avdevice code to build using Visual C++.  This is
 a contination of Kyle Schwarz's "avdevice/decklink: Remove pthread
 dependency" patch that is available at
 https://patchwork.ffmpeg.org/patch/2654/ .  This patch wasn't accepted, and
 as far as I can tell, there was no follow-up after it was rejected.

Notes: Used Visual Studio 2015 (with update 3) for this.

Comments:

-- configure:

a) Eliminated pthreads dependency for decklink_indev_deps and
   decklink_outdev_deps
b) For MSVC (Microsoft Visual C++) builds, now reinitializing
   decklink_indev_extralibs and decklink_outdev_extralibs to empty and
   then later, following the similar procedure used for mingw by
   adding -lole32 and -loleaut32.  This is necessary in order for it
   to link properly when building with Visual C++.

-- libavdevice/decklink_common.cpp / .h:
a) Eliminated semaphore and replaced with a combination of a mutex,
   condition variable, and a counter (frames_buffer_available_spots).
b) Switched the order of the include of libavformat/internal.h to
   workaround build issues with Visual C++.  See comment in
   decklink_common.cpp for more details.
c) Removed include of pthread.h and semaphore.h and now using
   libavutil/thread.h instead.

-- libavdevice/decklink_dec.cpp:
a) Eliminated include of pthread.h and semaphore.h.
b) Rearranged the include of libavformat/internal.h (for reasons as
   described above).
c) Made slight alteration to an argument for call to av_rescale_q() to
   workaround a compiler error with Visual C++.  This appears to only
   be an issue when building C++ files with Visual C++.  See comments
   in code for more details.

-- libavdevice/decklink_enc.cpp:
a) Eliminated include of pthread.h and semaphore.h.
b) Rearranged the include of libavformat/internal.h (for reasons as
   described above).
c) Replaced use of semaphore with the equivalent using a combination
   of a mutex, condition variable, and a counter
   (frames_buffer_available_spots).  In theory, libavutil/thread.h and
   the associated code could have been modified instead to add
   cross-platform implementations of the sem_ functions, but an
   inspection of the ffmpeg source base indicates that there are only
   two cases in which semaphores are used (including this one that was
   replaced), so it was deemed to not be worth the effort.
---
 configure   |  8 ++--
 libavdevice/decklink_common.cpp | 13 +
 libavdevice/decklink_common.h   |  5 -
 libavdevice/decklink_dec.cpp| 20 +++-
 libavdevice/decklink_enc.cpp| 27 +++
 5 files changed, 53 insertions(+), 20 deletions(-)

diff --git a/configure b/configure
index b0f7b1a..5b76a33 100755
--- a/configure
+++ b/configure
@@ -2992,9 +2992,9 @@ avfoundation_indev_deps="pthreads"
 avfoundation_indev_extralibs="-framework Foundation -framework AVFoundation 
-framework CoreVideo -framework CoreMedia"
 bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h 
dev_video_bktr_ioctl_bt848_h dev_ic_bt8xx_h"
 caca_outdev_deps="libcaca"
-decklink_indev_deps="decklink pthreads"
+decklink_indev_deps="decklink"
 decklink_indev_extralibs="-lstdc++"
-decklink_outdev_deps="decklink pthreads"
+decklink_outdev_deps="decklink"
 decklink_outdev_extralibs="-lstdc++"
 dshow_indev_deps="IBaseFilter"
 dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 -lshlwapi"
@@ -3646,6 +3646,8 @@ case "$toolchain" in
 ld_default="$source_path/compat/windows/mslink"
 nm_default="dumpbin -symbols"
 ar_default="lib"
+decklink_indev_extralibs=""
+decklink_outdev_extralibs=""
 case "$arch" in
 arm*)
 as_default="armasm"
@@ -4902,6 +4904,8 @@ case $target_os in
 fi
 enabled x86_32 && check_ldflags -LARGEADDRESSAWARE
 shlibdir_default="$bindir_default"
+decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 
-loleaut32"
+decklink_indev_extralibs="$decklink_indev_extralibs -lole32 -loleaut32"
 SLIBPREF=""
 SLIBSUF=".dll"
 SLIBNAME_WITH_VERSION='$(SLIBPREF)$(FULLNAME)-$(LIBVERSION)$(SLIBSUF)'
diff --git a/

Re: [FFmpeg-devel] [PATCH] Fixed bug encountered when decoding interlaced video

On 3/26/2017 10:34 AM, Aaron Levinson wrote:
> On 3/26/2017 4:41 AM, Matthias Hunstock wrote:
>> Am 26.03.2017 um 11:50 schrieb Aaron Levinson:
>>> When using the following command to play back either file:
>>>  ffmpeg -i  -f decklink -pix_fmt uyvy422 "DeckLink SDI
>>>  4K", I noticed that with the mpegts file with the AAC audio stream,
>>>  it would correctly select an interlaced video mode for the video
>>>  output stream, but with the mpegts file with the Opus audio stream,
>>>  it would use a progressive video mode (1080p29.97) for the video
>>>  output stream.
>>
>> Which FFmpeg version did you test this with?
>>
>> There was a change related to this just short time ago.
>>
>> Does it happen with current git HEAD?
>>
>> Matthias
> 
> This issue occurs with the current git HEAD.  I'm aware of the
> Blackmagic improvement that was added in February to add support for
> interlaced video modes on output, and actually that's one of the reasons
> why I'm using the latest git sources, as opposed to, say, 3.2.4.  This
> particular issue has nothing to do with Blackmagic, and I only used
> Blackmagic in the example that reproduces the bug because it is
> something that can be reproduced on both Windows and Linux (and
> presumably also on OS/X).  The issue also occurs if I do something like
> -f rawvideo out.avi on Windows, and I'm sure that there are plenty of
> other examples.
> 
> Aaron Levinson

Has anyone had a chance to review this patch yet, which I submitted on March 
26th?  To demonstrate the impact of this patch, here's some output of before 
and after for an .h264 file with interlaced 1080i59.94 video content:

Command-line:  ffmpeg -i test8_1080i.h264 -c:v mpeg2video test8_1080i_mp2.ts

Before patch:

--

Input #0, h264, from 'test8_1080i.h264':
  Duration: N/A, bitrate: N/A
Stream #0:0: Video: h264 (High), yuv420p(top first), 1920x1080 [SAR 1:1 DAR 
16:9], 29.97 fps, 29.97 tbr, 1200k tbn, 59.94 tbc
Stream mapping:
  Stream #0:0 -> #0:0 (h264 (native) -> mpeg2video (native))
Press [q] to stop, [?] for help
Output #0, mpegts, to 'test8_1080i_mp2_2.ts':
  Metadata:
encoder : Lavf57.72.100
Stream #0:0: Video: mpeg2video (Main), yuv420p, 1920x1080 [SAR 1:1 DAR 
16:9], q=2-31, 200 kb/s, 29.97 fps, 90k tbn, 29.97 tbc
Metadata:
  encoder : Lavc57.92.100 mpeg2video
Side data:
  cpb: bitrate max/min/avg: 0/0/20 buffer size: 0 vbv_delay: -1

--

After patch:

--

Input #0, h264, from 'test8_1080i.h264':
  Duration: N/A, bitrate: N/A
Stream #0:0: Video: h264 (High), yuv420p(top first), 1920x1080 [SAR 1:1 DAR 
16:9], 29.97 fps, 29.97 tbr, 1200k tbn, 59.94 tbc
Stream mapping:
  Stream #0:0 -> #0:0 (h264 (native) -> mpeg2video (native))
Press [q] to stop, [?] for help
Output #0, mpegts, to 'test8_1080i_mp2_2.ts':
  Metadata:
encoder : Lavf57.72.100
Stream #0:0: Video: mpeg2video (Main), yuv420p(top coded first (swapped)), 
1920x1080 [SAR 1:1 DAR 16:9], q=2-31, 200 kb/s, 29.97 fps, 90k tbn, 29.97 tbc
Metadata:
  encoder : Lavc57.92.100 mpeg2video
Side data:
  cpb: bitrate max/min/avg: 0/0/20 buffer size: 0 vbv_delay: -1

--

As can be seen, before the patch, after decoding the .h264 file and then 
re-encoding it as mpeg2video in an mpegts container, the interlaced aspect of 
the video has been lost in the output, and it is now effectively 1080p29.97, 
although the video hasn't actually been converted to progressive.  ffmpeg 
simply thinks that the video is progressive when it is not.  With the patch, 
the interlaced aspect is not lost and propagates to the output.  So, this 
conclusively demonstrates that the issue has nothing to do with Blackmagic and 
is a more general issue with interlaced video and decoding.

I can make the input file available if that would be helpful.

Anyway, it would be great if this bug fix could make it into ffmpeg.

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

Re: [FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency. Made minor changes to get the decklink avdevice code to build using Visual C++.


On 4/12/2017 6:51 PM, Marton Balint wrote:



On Wed, 12 Apr 2017, Aaron Levinson wrote:


From bcc0ec4a0bb64e74b9c9369087708f55569bf50e Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Wed, 12 Apr 2017 16:41:53 -0700
Subject: [PATCH] avdevice/decklink: Removed pthread dependency.  Made
minor changes to get the decklink avdevice code to build using Visual
C++.

Purpose: avdevice/decklink: Removed pthread dependency by
replacing semaphore used in code appropriately.  Doing so makes it
easier to
build ffmpeg using Visual C++ on Windows.  In addition, made other minor
changes to get the decklink avdevice code to build using Visual C++.
This is
a contination of Kyle Schwarz's "avdevice/decklink: Remove pthread
dependency" patch that is available at
https://patchwork.ffmpeg.org/patch/2654/ .  This patch wasn't
accepted, and
as far as I can tell, there was no follow-up after it was rejected.

Notes: Used Visual Studio 2015 (with update 3) for this.


Could you split this patch into two separete patches, one that removes
the pthread dependancy, and another which fixes MSVC build issues?


I'll submit two new patches, although, at least with MSVC, I need both 
of them to simply be able to build it successfully.


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

[FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency

From d175e7fc94a2efc4f0bad021c118e4f907832c9c Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Wed, 12 Apr 2017 20:12:11 -0700
Subject: [PATCH] avdevice/decklink: Removed pthread dependency

Purpose: avdevice/decklink: Removed pthread dependency by
 replacing semaphore used in code appropriately.  Doing so makes it easier to
  build ffmpeg using Visual C++ on Windows.  This is a contination of Kyle
 Schwarz's "avdevice/decklink: Remove pthread dependency" patch that is
 available at https://patchwork.ffmpeg.org/patch/2654/ .  This patch wasn't
 accepted, and as far as I can tell, there was no follow-up after it was
 rejected.

Notes: Used Visual Studio 2015 (with update 3) for this.

Comments:

-- configure: Eliminated pthreads dependency for decklink_indev_deps
   and decklink_outdev_deps

-- libavdevice/decklink_common.cpp / .h:
a) Eliminated semaphore and replaced with a combination of a mutex,
   condition variable, and a counter (frames_buffer_available_spots).
b) Removed include of pthread.h and semaphore.h and now using
   libavutil/thread.h instead.

-- libavdevice/decklink_dec.cpp: Eliminated include of pthread.h and
   semaphore.h.

-- libavdevice/decklink_enc.cpp:
a) Eliminated include of pthread.h and semaphore.h.
b) Replaced use of semaphore with the equivalent using a combination
   of a mutex, condition variable, and a counter
   (frames_buffer_available_spots).  In theory, libavutil/thread.h and
   the associated code could have been modified instead to add
   cross-platform implementations of the sem_ functions, but an
   inspection of the ffmpeg source base indicates that there are only
   two cases in which semaphores are used (including this one that was
   replaced), so it was deemed to not be worth the effort.
---
 configure   |  4 ++--
 libavdevice/decklink_common.cpp |  3 ---
 libavdevice/decklink_common.h   |  5 -
 libavdevice/decklink_dec.cpp|  3 ---
 libavdevice/decklink_enc.cpp| 23 +++
 5 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/configure b/configure
index b0f7b1a..adb0060 100755
--- a/configure
+++ b/configure
@@ -2992,9 +2992,9 @@ avfoundation_indev_deps="pthreads"
 avfoundation_indev_extralibs="-framework Foundation -framework AVFoundation 
-framework CoreVideo -framework CoreMedia"
 bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h 
dev_video_bktr_ioctl_bt848_h dev_ic_bt8xx_h"
 caca_outdev_deps="libcaca"
-decklink_indev_deps="decklink pthreads"
+decklink_indev_deps="decklink"
 decklink_indev_extralibs="-lstdc++"
-decklink_outdev_deps="decklink pthreads"
+decklink_outdev_deps="decklink"
 decklink_outdev_extralibs="-lstdc++"
 dshow_indev_deps="IBaseFilter"
 dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 -lshlwapi"
diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
index c9107c0..f01fba9 100644
--- a/libavdevice/decklink_common.cpp
+++ b/libavdevice/decklink_common.cpp
@@ -26,9 +26,6 @@
 #include 
 #endif
 
-#include 
-#include 
-
 extern "C" {
 #include "libavformat/avformat.h"
 #include "libavformat/internal.h"
diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
index 4753287..c12cf18 100644
--- a/libavdevice/decklink_common.h
+++ b/libavdevice/decklink_common.h
@@ -24,6 +24,7 @@
 
 #include 
 
+#include "libavutil/thread.h"
 #include "decklink_common_c.h"
 
 class decklink_output_callback;
@@ -89,7 +90,9 @@ struct decklink_ctx {
 int frames_preroll;
 int frames_buffer;
 
-sem_t semaphore;
+pthread_mutex_t mutex;
+pthread_cond_t cond;
+int frames_buffer_available_spots;
 
 int channels;
 };
diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index 8cc1bdf..67eaf97 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -21,9 +21,6 @@
 
 #include 
 
-#include 
-#include 
-
 extern "C" {
 #include "config.h"
 #include "libavformat/avformat.h"
diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp
index 18ef905..5105967 100644
--- a/libavdevice/decklink_enc.cpp
+++ b/libavdevice/decklink_enc.cpp
@@ -24,9 +24,6 @@ using std::atomic;
 
 #include 
 
-#include 
-#include 
-
 extern "C" {
 #include "libavformat/avformat.h"
 #include "libavformat/internal.h"
@@ -91,7 +88,10 @@ public:
 
 av_frame_unref(avframe);
 
-sem_post(&ctx->semaphore);
+pthread_mutex_lock(&ctx->mutex);
+ctx->frames_buffer_available_spots++;
+pthread_cond_broadcast(&ctx->cond);
+pthread_mutex_unlock(&ctx->mutex);
 
 return S_OK;
 }
@@ -133,7 +133,6 @@ static int decklink_setup_video(AVFormatContext *avctx, 
AVStream *s

[FFmpeg-devel] [PATCH] Made minor changes to get the decklink avdevice code to build using Visual C++

From aa549cf1c463f4a37bda44b934e17811986f11d9 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Wed, 12 Apr 2017 20:21:41 -0700
Subject: [PATCH] Made minor changes to get the decklink avdevice code to build 
using Visual C++

Purpose: Made minor changes to get the decklink avdevice
 code to build using Visual C++.

Notes: Used Visual Studio 2015 (with update 3) for this.

Comments:

-- configure: For MSVC (Microsoft Visual C++) builds, now reinitializing
   decklink_indev_extralibs and decklink_outdev_extralibs to empty and
   then later, following the similar procedure used for mingw by
   adding -lole32 and -loleaut32.  This is necessary in order for it
   to link properly when building with Visual C++.

-- libavdevice/decklink_common.cpp: Switched the order of the include
   of libavformat/internal.h to workaround build issues with Visual
   C++.  See comment in file for more details.

-- libavdevice/decklink_dec.cpp:
a) Rearranged the include of libavformat/internal.h (for reasons as
   described above).
b) Made slight alteration to an argument for call to av_rescale_q() to
   workaround a compiler error with Visual C++.  This appears to only
   be an issue when building C++ files with Visual C++.  See comments
   in code for more details.

-- libavdevice/decklink_enc.cpp: Rearranged the include of
   libavformat/internal.h (for reasons as described above).
---
 configure   |  4 
 libavdevice/decklink_common.cpp | 10 +-
 libavdevice/decklink_dec.cpp| 17 +++--
 libavdevice/decklink_enc.cpp|  5 -
 4 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index adb0060..5b76a33 100755
--- a/configure
+++ b/configure
@@ -3646,6 +3646,8 @@ case "$toolchain" in
 ld_default="$source_path/compat/windows/mslink"
 nm_default="dumpbin -symbols"
 ar_default="lib"
+decklink_indev_extralibs=""
+decklink_outdev_extralibs=""
 case "$arch" in
 arm*)
 as_default="armasm"
@@ -4902,6 +4904,8 @@ case $target_os in
 fi
 enabled x86_32 && check_ldflags -LARGEADDRESSAWARE
 shlibdir_default="$bindir_default"
+decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 
-loleaut32"
+decklink_indev_extralibs="$decklink_indev_extralibs -lole32 -loleaut32"
 SLIBPREF=""
 SLIBSUF=".dll"
 SLIBNAME_WITH_VERSION='$(SLIBPREF)$(FULLNAME)-$(LIBVERSION)$(SLIBSUF)'
diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
index f01fba9..523217c 100644
--- a/libavdevice/decklink_common.cpp
+++ b/libavdevice/decklink_common.cpp
@@ -19,6 +19,15 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+// Moved inclusion of internal.h here in order to get it to build successfully
+// when using Visual C++ to build--otherwise, compilation errors result
+// due to winsock.h (which is included indirectly by DeckLinkAPI.h and
+// DeckLinkAPI_i.c) conflicting with winsock2.h, which is included by
+// internal.h.  If winsock2.h is included first, then the conflict is resolved.
+extern "C" {
+#include "libavformat/internal.h"
+}
+
 #include 
 #ifdef _WIN32
 #include 
@@ -28,7 +37,6 @@
 
 extern "C" {
 #include "libavformat/avformat.h"
-#include "libavformat/internal.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/bswap.h"
diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index 67eaf97..a663029 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -19,12 +19,15 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+extern "C" {
+#include "libavformat/internal.h"
+}
+
 #include 
 
 extern "C" {
 #include "config.h"
 #include "libavformat/avformat.h"
-#include "libavformat/internal.h"
 #include "libavutil/avutil.h"
 #include "libavutil/common.h"
 #include "libavutil/imgutils.h"
@@ -262,8 +265,18 @@ static int64_t get_pkt_pts(IDeckLinkVideoInputFrame 
*videoFrame,
 res = videoFrame->GetHardwareReferenceTimestamp(time_base.den, 
&bmd_pts, &bmd_duration);
 break;
 case PTS_SRC_WALLCLOCK:
-pts = av_rescale_q(wallclock, AV_TIME_BASE_Q, time_base);
+{
+// doing the following rather than using AV_TIME_BASE_Q because
+// AV_TIME_BASE_Q doesn't work when building with Visual C++
+// for C++ files (works for C files).  When building C++ files,
+// it results in compiler error C4576.  At least, this is the case
+// wit

[FFmpeg-devel] Debug builds and if (ARCH_...) linking issues

I wanted to build a debug build of ffmpeg using Visual C++ today, one 
without any optimizations.  This implies the use of the -Od compiler 
option.  Unfortunately, I quickly discovered that the build fails soon 
after it starts because it can't find certain architecture-specific 
references.  For example, in libavutil/cpu.c, there is the following:


if (ARCH_AARCH64)
return ff_get_cpu_flags_aarch64();

The linker couldn't find ff_get_cpu_flags_aarch64 (and other similar 
references) and failed.  This isn't an issue when optimizations are 
turned on because the compiler notices that ARCH_AARCH64 is defined as 0 
and eliminates the relevant code.


Effectively, successful builds of ffmpeg depend on this compiler 
optimization.  This appears to have been the standard practice in the 
ffmpeg code base for at least the last few years, but it is unclear to 
me why this approach is being used, since, in addition to depending on 
specific compiler behavior, it prevents fully debug builds from 
succeeding, at least with Visual C++.


If people like the if (ARCH_...) syntax, while it wouldn't look quite as 
nice, what's wrong with doing the following:


#if ARCH_AARCH64
if (ARCH_AARCH64)
return ff_get_cpu_flags_aarch64();
#endif

Another, much less desirable option is to use #pragma optimize for the 
relevant functions in ffmpeg to turn optimizations on for specific 
functions.


A third option would be to build only the relevant files with 
optimizations turned on, but this will end up making the Makefiles more 
complicated, and the relative simplicity of the Makefiles is appealing.


For now, I'm using both -Od and -Og with Visual C++ (-Og turns on some 
optimizations, but not as much as -O1 or -O2), but this isn't the same 
as a true debug build.


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

Re: [FFmpeg-devel] [PATCH] Made appropriate changes to be able to successfully build C++ files using a Visual C++ build on Windows

On 4/13/2017 12:21 AM, Hendrik Leppkes wrote:
> On Thu, Apr 13, 2017 at 2:16 AM, Aaron Levinson  wrote:
>> diff --git a/configure b/configure
>> index d13d60b..b0f7b1a 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3635,10 +3635,13 @@ case "$toolchain" in
>>  # successfully parses the version number of existing supported
>>  # versions that require the converter (MSVC 2010 and 2012).
>>  cl_major_ver=$(cl 2>&1 | sed -n 's/.*Version 
>> \([[:digit:]]\{1,\}\)\..*/\1/p')
>> +CXX_O='-Fo$@'
>>  if [ -z "$cl_major_ver" ] || [ $cl_major_ver -ge 18 ]; then
>>  cc_default="cl"
>> +cxx_default="cl"
>>  else
>>  cc_default="c99wrap cl"
>> +cxx_default="c99wrap cl"
>>  fi
>>  ld_default="$source_path/compat/windows/mslink"
>>  nm_default="dumpbin -symbols"
> 
> CXX_O is in the wrong spot, it should be set at the same place where
> CC_O is beint set, and not here.
> 
> - Hendrik

I believe that I've addressed your comment, and a new version of the patch can 
be found below.

Thanks,
Aaron Levinson

-

From 764aed11d179fd42d1aa8c1c507d7660386cfde6 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 13 Apr 2017 01:13:07 -0700
Subject: [PATCH] Made appropriate changes to be able to successfully build C++ 
files using a Visual C++ build on Windows

Purpose: Made appropriate changes to be able to successfully
 build C++ files using a Visual C++ build on Windows.  Note that this is a 
 continuation of the "fixes w32pthreads to support C++" aspect of Kyle 
 Schwarz's "Remove pthread dependency" patch that is available at 
 https://patchwork.ffmpeg.org/patch/2654/ .  This patch wasn't accepted, and
 as far as I can tell, there was no follow-up after it was rejected.

Notes: Used Visual Studio 2015 (with update 3) for this.  Altered
approach for specifying -Fo$@ in configure based on code review from
Hendrik Leppkes for first version of patch.

Comments:

-- compat/w32pthreads.h: Made appropriate changes to w32pthreads.h to
   get it to build when it is being included in a C++ file and built
   with Visual C++.  This is mostly a copy of Kyle Schwarz's patch as
   described above.

-- configure: Made minor modifications to MSVC (Microsoft Visual C++)
   toolchain section to make sure that -Fo$@ is used when building
   object files but when the C++ compiler is used and also to use cl
   for the default C++ compiler.  This is currently only relevant for
   building the Blackmagic/Decklink-related files under avdevice.
---
 compat/w32pthreads.h | 24 
 configure|  6 +-
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/compat/w32pthreads.h b/compat/w32pthreads.h
index 0c9a7fa..a6c699b 100644
--- a/compat/w32pthreads.h
+++ b/compat/w32pthreads.h
@@ -77,7 +77,7 @@ typedef struct pthread_cond_t {
 
 static av_unused unsigned __stdcall attribute_align_arg 
win32thread_worker(void *arg)
 {
-pthread_t *h = arg;
+pthread_t *h = (pthread_t*)arg;
 h->ret = h->func(h->arg);
 return 0;
 }
@@ -270,7 +270,7 @@ static av_unused int pthread_cond_init(pthread_cond_t 
*cond, const void *unused_
 }
 
 /* non native condition variables */
-win32_cond = av_mallocz(sizeof(win32_cond_t));
+win32_cond = (win32_cond_t*)av_mallocz(sizeof(win32_cond_t));
 if (!win32_cond)
 return ENOMEM;
 cond->Ptr = win32_cond;
@@ -288,7 +288,7 @@ static av_unused int pthread_cond_init(pthread_cond_t 
*cond, const void *unused_
 
 static av_unused int pthread_cond_destroy(pthread_cond_t *cond)
 {
-win32_cond_t *win32_cond = cond->Ptr;
+win32_cond_t *win32_cond = (win32_cond_t*)cond->Ptr;
 /* native condition variables do not destroy */
 if (cond_init)
 return 0;
@@ -305,7 +305,7 @@ static av_unused int pthread_cond_destroy(pthread_cond_t 
*cond)
 
 static av_unused int pthread_cond_broadcast(pthread_cond_t *cond)
 {
-win32_cond_t *win32_cond = cond->Ptr;
+win32_cond_t *win32_cond = (win32_cond_t*)cond->Ptr;
 int have_waiter;
 
 if (cond_broadcast) {
@@ -337,7 +337,7 @@ static av_unused int pthread_cond_broadcast(pthread_cond_t 
*cond)
 
 static av_unused int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t 
*mutex)
 {
-win32_cond_t *win32_cond = cond->Ptr;
+win32_cond_t *win32_cond = (win32_cond_t*)cond->Ptr;
 int last_waiter;
 if (cond_wait) {
 cond_wait(cond, mutex, INFINITE);
@@ -369,7 +369,7 @@ static av_unused int pthread_cond_wait(pthread_cond_t 
*cond, pthread_mutex_t *m

Re: [FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency


On 4/13/2017 1:21 AM, Hendrik Leppkes wrote:

On Thu, Apr 13, 2017 at 5:32 AM, Aaron Levinson  wrote:

diff --git a/configure b/configure
index b0f7b1a..adb0060 100755
--- a/configure
+++ b/configure
@@ -2992,9 +2992,9 @@ avfoundation_indev_deps="pthreads"
 avfoundation_indev_extralibs="-framework Foundation -framework AVFoundation 
-framework CoreVideo -framework CoreMedia"
 bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h 
dev_video_bktr_ioctl_bt848_h dev_ic_bt8xx_h"
 caca_outdev_deps="libcaca"
-decklink_indev_deps="decklink pthreads"
+decklink_indev_deps="decklink"
 decklink_indev_extralibs="-lstdc++"
-decklink_outdev_deps="decklink pthreads"
+decklink_outdev_deps="decklink"


You should probably still have a dependency on "threads" (a combined
dep for any threading support), or does the device work without any
threading support now?


That makes sense, and I think I follow how the threads dependency is 
generated in configure.  Should I generate an entirely new patch?  The 
only difference would be the following, assuming all is okay with the 
actual code changes:


diff --git a/configure b/configure
index b0f7b1a..adb0060 100755
--- a/configure
+++ b/configure
@@ -2992,9 +2992,9 @@ avfoundation_indev_deps="pthreads"
 avfoundation_indev_extralibs="-framework Foundation -framework 
AVFoundation -framework CoreVideo -framework CoreMedia"
 bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h 
dev_video_bktr_ioctl_bt848_h dev_ic_bt8xx_h"

 caca_outdev_deps="libcaca"
-decklink_indev_deps="decklink pthreads"
+decklink_indev_deps="decklink threads"
 decklink_indev_extralibs="-lstdc++"
-decklink_outdev_deps="decklink pthreads"
+decklink_outdev_deps="decklink threads"
 decklink_outdev_extralibs="-lstdc++"
 dshow_indev_deps="IBaseFilter"
 dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 
-lshlwapi"


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

Re: [FFmpeg-devel] [PATCH] Made appropriate changes to be able to successfully build C++ files using a Visual C++ build on Windows

On 4/13/2017 1:27 AM, Hendrik Leppkes wrote:
> On Thu, Apr 13, 2017 at 10:23 AM, Aaron Levinson  wrote:
>> diff --git a/configure b/configure
>> index b2fc781..6112b9b 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3637,8 +3637,10 @@ case "$toolchain" in
>>  cl_major_ver=$(cl 2>&1 | sed -n 's/.*Version 
>> \([[:digit:]]\{1,\}\)\..*/\1/p')
>>  if [ -z "$cl_major_ver" ] || [ $cl_major_ver -ge 18 ]; then
>>  cc_default="cl"
>> +cxx_default="cl"
>>  else
>>  cc_default="c99wrap cl"
>> +cxx_default="c99wrap cl"
>>  fi
>>  ld_default="$source_path/compat/windows/mslink"
>>  nm_default="dumpbin -symbols"
>> @@ -3983,7 +3985,7 @@ probe_cc(){
>>  _cc=$2
>>  first=$3
>>
>> -unset _type _ident _cc_c _cc_e _cc_o _flags _cflags
>> +unset _type _ident _cc_c _cc_e _cc_o _cxx_o _flags _cflags
>>  unset _ld_o _ldflags _ld_lib _ld_path
>>  unset _depflags _DEPCMD _DEPFLAGS
>>  _flags_filter=echo
>> @@ -4156,6 +4158,7 @@ probe_cc(){
>>  fi
>>  _cc_o='-Fo$@'
>>  _cc_e='-P -Fi$@'
>> +_cxx_o='-Fo$@'
>>  _flags_filter=msvc_flags
>>  _ld_lib='lib%.a'
>>  _ld_path='-libpath:'
>> @@ -4196,6 +4199,7 @@ cflags_noopt=$_cflags_noopt
>>  add_cflags $_flags $_cflags
>>  cc_ldflags=$_ldflags
>>  set_ccvars CC
>> +set_ccvars CXX
>>
>>  probe_cc hostcc "$host_cc"
>>  host_cflags_filter=$_flags_filter
> 
> Technically this just happens to work by accident because CC_O and
> CXX_O get the same value, which is probably fine for all C++ compilers
> we currently support.
> set_ccvars always uses the _cc_o variable to set whatever namespace
> you requested (_cxx_o is never used), so you could remove the _cxx_o
> handling from probe_cc entirely and it would still work.
> 
> Full CXX probing through probe_cc would be the most complete solution,
> but probably overkill?
> 
> - Hendrik

Yes, I realize now that my use of _cxx_o had no effect, and it was simply the 
use of set_ccvars CXX that caused the CXX_ variables to be updated.  I would 
tend to agree that full CXX probing is probably overkill given that there are 
only are few C++ files in ffmpeg, and those files have to do with Decklink, 
which is only supported on a few operating systems as well.

I also realize now that this patch is more general than one that just applies 
to MSVC, although, in practice, the change will only benefit the Intel 
compiler, in addition to MSVC.  Hopefully, I won't be asked to separate out the 
set_ccvars CXX line into a separate patch :-)

Here is hopefully the final version of the patch with updated comments to 
reflect the more general nature of the patch.

Thanks,
Aaron Levinson

--

From 415f1f233eb963318d436fe272c3b80dda9d2d4d Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 13 Apr 2017 01:45:23 -0700
Subject: [PATCH] Made appropriate changes to be able to successfully build C++ 
files using a Visual C++ build on Windows

Purpose: Made appropriate changes to be able to successfully
build C++ files using a Visual C++ build on Windows.  Note that this is a
continuation of the "fixes w32pthreads to support C++" aspect of Kyle
Schwarz's "Remove pthread dependency" patch that is available at
https://patchwork.ffmpeg.org/patch/2654/ .  This patch wasn't accepted, and
as far as I can tell, there was no follow-up after it was rejected.

Notes: Used Visual Studio 2015 (with update 3) for this.  Altered
approach for specifying -Fo$@ in configure based on code review from
Hendrik Leppkes for first version of patch.

Comments:

-- compat/w32pthreads.h: Made appropriate changes to w32pthreads.h to
   get it to build when it is being included in a C++ file and built
   with Visual C++.  This is mostly a copy of Kyle Schwarz's patch as
   described above.

-- configure:
a) Now calling set_ccvars CXX to cause the various CXX_ variables to
   be setup properly.  For example, with MSVC (Microsoft Visual C++),
   this causes CXX_O to be set to -Fo$@ instead of using the default
   value.  The default value does not work with Visual C++.  This
   change will also have the impact of correcting CXX_O (and possibly
   CXX_C) for other compilers, although this is really only relevant
   for the Intel compiler, in addition to MSVC.
b) Now using cl for the C++ compiler for the MSVC toolchain.  This is
   currently only relevant for bui

Re: [FFmpeg-devel] [PATCH] Made appropriate changes to be able to successfully build C++ files using a Visual C++ build on Windows

On 4/13/2017 2:10 AM, Hendrik Leppkes wrote:
> On Thu, Apr 13, 2017 at 11:04 AM, Aaron Levinson  wrote:
>>  static av_unused int pthread_cond_signal(pthread_cond_t *cond)
>>  {
>> -win32_cond_t *win32_cond = cond->Ptr;
>> +win32_cond_t *win32_cond = (win32_cond_t*)cond->Ptr;
>>  int have_waiter;
>>  if (cond_signal) {
>>  cond_signal(cond);
>> @@ -400,17 +400,17 @@ static av_unused void w32thread_init(void)
>>  HANDLE kernel_dll = GetModuleHandle(TEXT("kernel32.dll"));
>>  /* if one is available, then they should all be available */
>>  cond_init  = (void (WINAPI*)(pthread_cond_t *))
>> -GetProcAddress(kernel_dll, "InitializeConditionVariable");
>> +GetProcAddress((HMODULE)kernel_dll, "InitializeConditionVariable");
>>  cond_broadcast = (void (WINAPI*)(pthread_cond_t *))
>> -GetProcAddress(kernel_dll, "WakeAllConditionVariable");
>> +GetProcAddress((HMODULE)kernel_dll, "WakeAllConditionVariable");
>>  cond_signal= (void (WINAPI*)(pthread_cond_t *))
>> -GetProcAddress(kernel_dll, "WakeConditionVariable");
>> +GetProcAddress((HMODULE)kernel_dll, "WakeConditionVariable");
>>  cond_wait  = (BOOL (WINAPI*)(pthread_cond_t *, pthread_mutex_t *, 
>> DWORD))
>> -GetProcAddress(kernel_dll, "SleepConditionVariableCS");
>> +GetProcAddress((HMODULE)kernel_dll, "SleepConditionVariableCS");
>>  initonce_begin = (BOOL (WINAPI*)(pthread_once_t *, DWORD, BOOL *, void 
>> **))
>> -GetProcAddress(kernel_dll, "InitOnceBeginInitialize");
>> +GetProcAddress((HMODULE)kernel_dll, "InitOnceBeginInitialize");
>>  initonce_complete = (BOOL (WINAPI*)(pthread_once_t *, DWORD, void *))
>> -GetProcAddress(kernel_dll, "InitOnceComplete");
>> +GetProcAddress((HMODULE)kernel_dll, "InitOnceComplete");
>>  #endif
> 
> One more comment that I missed earlier (sorry), GetModuleHandle
> returns the type HMODULE, can't you just change the type of kernel_dll
> to match that, and avoid the casts here?
> 
> - Hendrik

Yes, it is simpler and cleaner to properly set the return value of 
GetModuleHandle() to an HMODULE.  Here is what is hopefully the final version 
of the patch, which is also much cleaner and simplified from the original 
version.

Thanks,
Aaron

--

From 56f9a4b6c281a0d9382d2b4ec2e29aff5ab69fbc Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 13 Apr 2017 02:38:02 -0700
Subject: [PATCH] Made appropriate changes to be able to successfully build C++ 
files using a Visual C++ build on Windows

Purpose: Made appropriate changes to be able to successfully
build C++ files using a Visual C++ build on Windows.  Note that this is a
continuation of the "fixes w32pthreads to support C++" aspect of Kyle
Schwarz's "Remove pthread dependency" patch that is available at
https://patchwork.ffmpeg.org/patch/2654/ .  This patch wasn't accepted, and
as far as I can tell, there was no follow-up after it was rejected.

Notes: Used Visual Studio 2015 (with update 3) for this.  Altered
approach for specifying -Fo$@ in configure based on code review from
Hendrik Leppkes for first version of patch.  Also simplified changes
to w32pthreads.h per Hendrik's review.

Comments:

-- compat/w32pthreads.h: Made appropriate changes to w32pthreads.h to
   get it to build when it is being included in a C++ file and built
   with Visual C++.  This is mostly a copy of Kyle Schwarz's patch as
   described above.

-- configure:
a) Now calling set_ccvars CXX to cause the various CXX_ variables to
   be setup properly.  For example, with MSVC (Microsoft Visual C++),
   this causes CXX_O to be set to -Fo$@ instead of using the default
   value.  The default value does not work with Visual C++.  This
   change will also have the impact of correcting CXX_O (and possibly
   CXX_C) for other compilers, although this is really only relevant
   for the Intel compiler, in addition to MSVC.
b) Now using cl for the C++ compiler for the MSVC toolchain.  This is
   currently only relevant for building the
   Blackmagic/Decklink-related files under avdevice.
---
 compat/w32pthreads.h | 14 +++---
 configure|  3 +++
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/compat/w32pthreads.h b/compat/w32pthreads.h
index 0c9a7fa..eeead60 100644
--- a/compat/w32pthreads.h
+++ b/compat/w32pthreads.h
@@ -77,7 +77,7 @@ typedef struct pthread_cond_t {
 
 static av_unused unsigned __stdcall attribute_align_arg 
win32thread_worker(void *arg)
 {
-pthread_t

Re: [FFmpeg-devel] [PATCH] Made minor changes to get the decklink avdevice code to build using Visual C++

On 4/13/2017 12:36 AM, Hendrik Leppkes wrote:
> On Thu, Apr 13, 2017 at 5:34 AM, Aaron Levinson  wrote:
>> diff --git a/configure b/configure
>> index adb0060..5b76a33 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3646,6 +3646,8 @@ case "$toolchain" in
>>  ld_default="$source_path/compat/windows/mslink"
>>  nm_default="dumpbin -symbols"
>>  ar_default="lib"
>> +decklink_indev_extralibs=""
>> +decklink_outdev_extralibs=""
>>  case "$arch" in
>>  arm*)
>>  as_default="armasm"
> 
> As in the other patch before, the toolchain handling section has no
> business setting these.
> 
>> @@ -4902,6 +4904,8 @@ case $target_os in
>>  fi
>>  enabled x86_32 && check_ldflags -LARGEADDRESSAWARE
>>  shlibdir_default="$bindir_default"
>> +decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 
>> -loleaut32"
>> +decklink_indev_extralibs="$decklink_indev_extralibs -lole32 
>> -loleaut32"
>>  SLIBPREF=""
>>  SLIBSUF=".dll"
>>  
>> SLIBNAME_WITH_VERSION='$(SLIBPREF)$(FULLNAME)-$(LIBVERSION)$(SLIBSUF)'
> 
> I really don't like having these decklink specific things in the OS
> sections. I know its already there in one section, but I would greatly
> prefer if this would also be moved into a dedicated decklink section
> somewhere, instead of littering the msvc/mingw sections.
> In fact, its now set for both mingw and msvc, so setting it centrally
> in one place would be even better to avoid duplicating it.

I believe that I've addressed both of these issues with the following new 
version of the patch.

Thanks,
Aaron

--

From 2e87ce15e9fb27b81b11b88a0660581549cfcfaf Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 13 Apr 2017 03:28:40 -0700
Subject: [PATCH] Made minor changes to get the decklink avdevice code to build 
using Visual C++.

Purpose: Made minor changes to get the decklink avdevice code
to build using Visual C++.

Notes: Used Visual Studio 2015 (with update 3) for this.  Also made
changes to configure per Hendrik Leppkes's review of first version of
patch.

Comments:

-- configure: Added if enabled decklink section and setting
   decklink_indev_extralibs and decklink_outdev_extralibs here for
   both mingw and Windows.  In the case of Windows, the new value,
   -lole32 and -loleaut32 overwrites the default value.  In the case
   of mingw, -lole32 and -loleaut32 is added to the default value.
   Also eliminated the setting of these variables in the mingw section
   earlier in the file.

-- libavdevice/decklink_common.cpp: Switched the order of the include
   of libavformat/internal.h to workaround build issues with Visual
   C++.  See comment in file for more details.

-- libavdevice/decklink_dec.cpp:
a) Rearranged the include of libavformat/internal.h (for reasons as
   described above).
b) Made slight alteration to an argument for call to av_rescale_q() to
   workaround a compiler error with Visual C++.  This appears to only
   be an issue when building C++ files with Visual C++.  See comments
   in code for more details.

-- libavdevice/decklink_enc.cpp: Rearranged the include of
   libavformat/internal.h (for reasons as described above).
---
 configure   | 15 +--
 libavdevice/decklink_common.cpp | 10 +-
 libavdevice/decklink_dec.cpp| 17 +++--
 libavdevice/decklink_enc.cpp|  5 -
 4 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/configure b/configure
index a383bf2..9a06437 100755
--- a/configure
+++ b/configure
@@ -4843,8 +4843,6 @@ case $target_os in
 else
 target_os=mingw32
 fi
-decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 
-loleaut32"
-decklink_indev_extralibs="$decklink_indev_extralibs -lole32 -loleaut32"
 LIBTARGET=i386
 if enabled x86_64; then
 LIBTARGET="i386:x86-64"
@@ -5946,6 +5944,19 @@ if ! disabled sdl2; then
 fi
 enabled sdl2 && enable sdl && add_cflags $sdl2_cflags && add_extralibs 
$sdl2_libs
 
+if enabled decklink; then
+case $target_os in
+mingw32*|mingw64*)
+decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 
-loleaut32"
+decklink_indev_extralibs="$decklink_indev_extralibs -lole32 
-loleaut32"
+;;
+win32|win64)
+decklink_outdev_extralibs="-lole32 -loleaut32"
+decklink_ind

Re: [FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency


On 4/13/2017 2:12 AM, Hendrik Leppkes wrote:

On Thu, Apr 13, 2017 at 10:36 AM, Aaron Levinson  wrote:
Give it some time for the other changes to be reviewed by the people
that actually know decklink itself, you can include that in any new
versions of the patch then, no need to send one for that right now.

- Hendrik


Actually, the coding changes made to the decklink source files in this 
patch have pretty much nothing to do with decklink itself, and anyone 
with familiarity with semaphores and pthread could review those changes. 
 In fact, all I really did was use already existing approaches for 
replacing a semaphore with the combination of a condition variable, 
mutex, and counter, and there are plenty of examples of how to do this 
on the Web.


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

Re: [FFmpeg-devel] [PATCH] Made minor changes to get the decklink avdevice code to build using Visual C++


On 4/13/2017 3:55 AM, Hendrik Leppkes wrote:

On Thu, Apr 13, 2017 at 12:39 PM, Aaron Levinson  wrote:

On 4/13/2017 12:36 AM, Hendrik Leppkes wrote:
diff --git a/configure b/configure
index a383bf2..9a06437 100755
--- a/configure
+++ b/configure
@@ -4843,8 +4843,6 @@ case $target_os in
 else
 target_os=mingw32
 fi
-decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 
-loleaut32"
-decklink_indev_extralibs="$decklink_indev_extralibs -lole32 -loleaut32"
 LIBTARGET=i386
 if enabled x86_64; then
 LIBTARGET="i386:x86-64"
@@ -5946,6 +5944,19 @@ if ! disabled sdl2; then
 fi
 enabled sdl2 && enable sdl && add_cflags $sdl2_cflags && add_extralibs 
$sdl2_libs

+if enabled decklink; then
+case $target_os in
+mingw32*|mingw64*)
+decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 
-loleaut32"
+decklink_indev_extralibs="$decklink_indev_extralibs -lole32 
-loleaut32"
+;;
+win32|win64)
+decklink_outdev_extralibs="-lole32 -loleaut32"
+decklink_indev_extralibs="-lole32 -loleaut32"
+;;
+esac
+fi
+


Whats in the extralibs before this block that you need to clear out in
the msvc case?
Maybe that part should just be set conditionally so we can unify this,
and don't set something we need to strip out again after.

- Hendrik


decklink_outdev_extralibs and decklink_indev_extralibs are declared as 
follows earlier in configure:


decklink_indev_extralibs="-lstdc++"
decklink_outdev_extralibs="-lstdc++"

This is done in the standard section where 
__ can be found.


libstdc++ is needed when building the Decklink source files with gcc/g++ 
because of, at a minimum, the use of STL in one of the .cpp files.


For builds targeting Windows, -lole32 and -loleaut32 are also needed 
because of the use of COM.  So, that explains why -lole32 and -loleaut32 
are added for both mingw and Windows.


Finally, when building with the Microsoft compiler (cl.exe), libstdc++ 
is not needed and actually can't be found.  The C/C++ run-time is 
included by default in some form when building with cl.exe unless the 
-nodefaultlib linker option is used.  So, that explains why it replaces 
the extralibs variables with just -lole32 and -loleaut32 for Windows 
builds.  Note that win32 to specified as the target_os when an msvc 
build is done, and it may be the only case when target_os is set to 
win32 unless the user manually sets it as an option when invoking configure.


-lole32 and -loleaut32 is likely relevant for both cygwin and the Intel 
compiler, but it should be a simple matter to add those later if anyone 
uses those for building.  The elimination of -lstdc++ may also be 
relevant for the Intel compiler but probably only when using the Intel 
compiler on Windows.


Anyway, I don't think an approach that moves -lstdc++ into the above if 
block is any better or worse than the patch in its current form, and the 
patch in its current form results in fewer changes to configure than a 
patch that moves -lstdc++ around.


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

Re: [FFmpeg-devel] Debug builds and if (ARCH_...) linking issues


On 4/13/2017 11:04 AM, Matt Oliver wrote:

On 14 April 2017 at 03:31, Hendrik Leppkes  wrote:


On Thu, Apr 13, 2017 at 7:16 PM, Matt Oliver  wrote:

On 14 April 2017 at 02:11, Rostislav Pehlivanov 

wrote:





On 13 April 2017 at 16:51, wm4  wrote:


On Thu, 13 Apr 2017 17:39:57 +1000
Matt Oliver  wrote:


On 13 April 2017 at 17:20, Aaron Levinson 

wrote:



I wanted to build a debug build of ffmpeg using Visual C++ today,

one

without any optimizations.  This implies the use of the -Od

compiler

option.  Unfortunately, I quickly discovered that the build fails

soon

after it starts because it can't find certain architecture-specific
references.  For example, in libavutil/cpu.c, there is the

following:


if (ARCH_AARCH64)
return ff_get_cpu_flags_aarch64();

The linker couldn't find ff_get_cpu_flags_aarch64 (and other

similar

references) and failed.  This isn't an issue when optimizations are

turned

on because the compiler notices that ARCH_AARCH64 is defined as 0

and

eliminates the relevant code.

Effectively, successful builds of ffmpeg depend on this compiler
optimization.  This appears to have been the standard practice in

the

ffmpeg code base for at least the last few years, but it is unclear

to me

why this approach is being used, since, in addition to depending on
specific compiler behavior, it prevents fully debug builds from

succeeding,

at least with Visual C++.

If people like the if (ARCH_...) syntax, while it wouldn't look

quite

as

nice, what's wrong with doing the following:

#if ARCH_AARCH64
if (ARCH_AARCH64)
return ff_get_cpu_flags_aarch64();
#endif

Another, much less desirable option is to use #pragma optimize for

the

relevant functions in ffmpeg to turn optimizations on for specific
functions.

A third option would be to build only the relevant files with
optimizations turned on, but this will end up making the Makefiles

more

complicated, and the relative simplicity of the Makefiles is

appealing.


For now, I'm using both -Od and -Og with Visual C++ (-Og turns on

some

optimizations, but not as much as -O1 or -O2), but this isn't the

same as a

true debug build.



Similar patches have been submitted before. This is an issue with

Dead

Code

Elimination (DCE) within FFmpeg and the fact the MSVC doesn't support
removing it in debug builds.

There have been some discussions on the mailing list in the past

about

resolving this but nothing was ever decided.

As a quick and dirty work around I have a tool that i wrote that

scans

in

the configure/makefile from a ffmpeg distro and generates a native

Visual

Studio project file that can be used to just compile within Visual

Studio

itself. You just pass it the desired configure options that you want

to

use

to build the project and it will make one for you. The main thing is

that

it scans the source and automatically generates the missing DCE

sections

and adds them so that everything will compile correctly with Debug

builds

and you can debug directly in VS. You can find the tool here
http://shiftmediaproject.github.io/1-projects/ (normally i wouldn't

put

external references on the mailing list but this may be of use to

you).


Any chance you could revive your old patches to remove the DCE
requirement? (Not sure if there were any patches.)

Before you do any real work, make a thread on the ML requesting
comments on this. Although I would very much welcome such patches, I'm
not fully sure about others.

This DCE requirement is pretty painful, and affects debugging on Linux
as well.




I put up a general discussion a while ago (
http://ffmpeg.org/pipermail/ffmpeg-devel/2016-December/204530.html) but
there were a few people who opposed a direct removal of DCE and no one
really came up with a consensus as to what the acceptable approach would

be.




I wouldn't like any weird hacks in the source just to work-around the
lack of DCE in debug builds, so we should decide to either keep using
it or get rid of it.



I agree.


It seems that the DCE requirement affects debug builds in general, and 
not just with Windows builds.  And, because of the DCE requirement, it 
is necessary to introduce some level of optimization in debug builds, 
which makes it harder to debug.  Sure, I can use -Zo with MSVC on 
Windows (see https://msdn.microsoft.com/en-us/library/dn785163.aspx for 
more details) to make it easier to debug release builds, but that's not 
the same as debugging a fully debug build, and something similar may not 
exist with other compilers.


I reviewed the thread mentioned by Matt Oliver above, and one of the 
arguments against removing DCE was that it may make some people less 
likely to work on the project.  Nicolas George wrote:


"Someone's personal preferences affect their willingness to work on the
project. Since the project is perpetually short on manpower, this is a
very serious issue."

Re: [FFmpeg-devel] [PATCH] Made minor changes to get the decklink avdevice code to build using Visual C++

On 4/13/2017 1:23 PM, Hendrik Leppkes wrote:
> On Thu, Apr 13, 2017 at 10:09 PM, Aaron Levinson  wrote:
>> On 4/13/2017 3:55 AM, Hendrik Leppkes wrote:
>>>
>>> On Thu, Apr 13, 2017 at 12:39 PM, Aaron Levinson 
>>> wrote:
>>>>
>>>> On 4/13/2017 12:36 AM, Hendrik Leppkes wrote:
>>>> diff --git a/configure b/configure
>>>> index a383bf2..9a06437 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -4843,8 +4843,6 @@ case $target_os in
>>>>  else
>>>>  target_os=mingw32
>>>>  fi
>>>> -decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32
>>>> -loleaut32"
>>>> -decklink_indev_extralibs="$decklink_indev_extralibs -lole32
>>>> -loleaut32"
>>>>  LIBTARGET=i386
>>>>  if enabled x86_64; then
>>>>  LIBTARGET="i386:x86-64"
>>>> @@ -5946,6 +5944,19 @@ if ! disabled sdl2; then
>>>>  fi
>>>>  enabled sdl2 && enable sdl && add_cflags $sdl2_cflags && add_extralibs
>>>> $sdl2_libs
>>>>
>>>> +if enabled decklink; then
>>>> +case $target_os in
>>>> +mingw32*|mingw64*)
>>>> +decklink_outdev_extralibs="$decklink_outdev_extralibs
>>>> -lole32 -loleaut32"
>>>> +decklink_indev_extralibs="$decklink_indev_extralibs -lole32
>>>> -loleaut32"
>>>> +;;
>>>> +win32|win64)
>>>> +decklink_outdev_extralibs="-lole32 -loleaut32"
>>>> +decklink_indev_extralibs="-lole32 -loleaut32"
>>>> +;;
>>>> +esac
>>>> +fi
>>>> +
>>>
>>>
>>> Whats in the extralibs before this block that you need to clear out in
>>> the msvc case?
>>> Maybe that part should just be set conditionally so we can unify this,
>>> and don't set something we need to strip out again after.
>>>
>>> - Hendrik
>>
>>
>> decklink_outdev_extralibs and decklink_indev_extralibs are declared as
>> follows earlier in configure:
>>
>> decklink_indev_extralibs="-lstdc++"
>> decklink_outdev_extralibs="-lstdc++"
>>
>> This is done in the standard section where
>> __ can be found.
>>
>> libstdc++ is needed when building the Decklink source files with gcc/g++
>> because of, at a minimum, the use of STL in one of the .cpp files.
>>
>> For builds targeting Windows, -lole32 and -loleaut32 are also needed because
>> of the use of COM.  So, that explains why -lole32 and -loleaut32 are added
>> for both mingw and Windows.
>>
>> Finally, when building with the Microsoft compiler (cl.exe), libstdc++ is
>> not needed and actually can't be found.  The C/C++ run-time is included by
>> default in some form when building with cl.exe unless the -nodefaultlib
>> linker option is used.  So, that explains why it replaces the extralibs
>> variables with just -lole32 and -loleaut32 for Windows builds.  Note that
>> win32 to specified as the target_os when an msvc build is done, and it may
>> be the only case when target_os is set to win32 unless the user manually
>> sets it as an option when invoking configure.
>>
>> -lole32 and -loleaut32 is likely relevant for both cygwin and the Intel
>> compiler, but it should be a simple matter to add those later if anyone uses
>> those for building.  The elimination of -lstdc++ may also be relevant for
>> the Intel compiler but probably only when using the Intel compiler on
>> Windows.
>>
>> Anyway, I don't think an approach that moves -lstdc++ into the above if
>> block is any better or worse than the patch in its current form, and the
>> patch in its current form results in fewer changes to configure than a patch
>> that moves -lstdc++ around.
>>
> 
> You could add -lstdc++ to the msvc flag filter (ie.
> msvc_common_flags), since its never needed, and may be added by more
> things in the future, that would solve all of those neatly.
> 
> - Hendrik

That's a good idea and is cleaner and more general than the approach I had 
used.  Here's a new version of the patch, although it is my understanding that 
you may push the -lstdc++ change in msvc_common_flags() as part of another 
patch, but regardless, a new patch for this is needed.

---

[FFmpeg-devel] [PATCH] Enhanced configure to improve compiler options associated with debugging with Visual C++ (MSVC)

From 558b957eb85a669899750b2e150eba7cdee8dcd9 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 13 Apr 2017 16:46:59 -0700
Subject: [PATCH] Enhanced configure to improve compiler options associated with 
debugging with Visual C++ (MSVC) 

Purpose: Enhanced configure to improve compiler options
associated with debugging with Visual C++ (MSVC)

Comments:

-- configure:
a) In msvc_common_flags() function, replaced the use of -Z7 with -Zi.
   Effectively, there was no point to using -Z7 anymore given than
   -debug is passed to the linker already (per the line "enabled debug
   && add_ldflags -debug" elsewhere in the file), which causes a .pdb
   to be generated anyway.  -Z7 causes Codeview debug info to be
   placed in .obj files, while -Zi causes debug info to be placed in
   .pdb files.  As a result of switching from -Z7 to -Zi, this may
   result in slightly faster builds with MSVC, since it is apparently
   slower to process Codeview debug info.
b) In probe_cc() function, added _cflags_noopt declaration for MSVC.
   This is set to the following: "-Od -Og -MTd".  See comments for
   more details.  This is exposed when --disable-optimizations is used
   with configure.  In addition, now adding -Zo (or -d2Zi+, depending
   on the compiler version) to all the different _cflags_ variables to
   make it easier to debug optimized builds.  See changes and comments
   for further details.
---
 configure | 33 -
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 684650a..7f2b653 100755
--- a/configure
+++ b/configure
@@ -3841,7 +3841,7 @@ msvc_common_flags(){
 -std=c99) ;;
 # Common flags
 -fomit-frame-pointer) ;;
--g)   echo -Z7 ;;
+-g)   echo -Zi ;;
 -fno-math-errno)  ;;
 -fno-common)  ;;
 -fno-signed-zeros);;
@@ -4150,6 +4150,37 @@ probe_cc(){
 _DEPFLAGS='$(CPPFLAGS) $(CFLAGS) -showIncludes -Zs'
 _cflags_speed="-O2"
 _cflags_size="-O1"
+# Need to use -Og with -Od because otherwise has link issues with
+# "if (ARCH_...)" code.  Unfortunately, this isn't quite the same as
+# a fully debug build.  If the Dead Code Elimination (DCE) compiler
+# optimization requirement is ever removed, this can be replaced
+# with just -Od.  Note that the use of -Og results in a compiler
+# deprecation warning.
+# Also added -MTd to use the multithread, static, debug version of
+# the run-time library.  -MT appears to be the default otherwise.
+# Possible TODO:  revisit and potentially use -MD and -MDd for
+# smaller binaries.
+_cflags_noopt="-Od -Og -MTd"
+cl_major_ver=$(cl 2>&1 | sed -n 's/.*Version 
\([[:digit:]]\{1,\}\)\..*/\1/p')
+
+# Make it easier to debug release builds.
+# See https://msdn.microsoft.com/en-us/library/dn785163.aspx for more
+# details.
+# It may not be necessary to add -Zo to _cflags_noopt due to the use
+# of -Od, but because -Og is also added currently, it is best to
+# add -Zo for now to make sure the best debugging experience is
+# possible.  If the DCE compiler optimization requirement is ever
+# removed, the _cflags_noopt lines can be removed.
+if [ -z "$cl_major_ver" ] || [ $cl_major_ver -ge 18 ]; then
+_cflags_speed="${_cflags_speed} -Zo"
+_cflags_size="${_cflags_size} -Zo"
+_cflags_noopt="${_cflags_noopt} -Zo"
+else
+_cflags_speed="${_cflags_speed} -d2Zi+"
+_cflags_size="${_cflags_size} -d2Zi+"
+_cflags_noopt="${_cflags_noopt} -d2Zi+"
+fi
+
 if $_cc -nologo- 2>&1 | grep -q Linker; then
 _ld_o='-out:$@'
 else
-- 
2.10.1.windows.1

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

[FFmpeg-devel] [PATCH] Enhanced require_pkg_config() in configure to fallback to require() if pkg-config is missing

From 48f7daba16e0fcdb83d9abd254800c7b9f4ab684 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 13 Apr 2017 17:30:47 -0700
Subject: [PATCH] Enhanced require_pkg_config() in configure to fallback to 
require() if pkg-config is missing

Purpose: Enhanced require_pkg_config() in configure to
fallback to require() if pkg-config is missing

Notes: This is likely mainly of relevance when building with MSVC on
Windows.  In my case, I used this approach to get libmfx when invoking
configure with --enable-libmfx, which is used for QuickSync (QSV).

Comments:

-- configure: Enhanced require_pkg_config() function to first check if
   $pkg_config is not false.  If not false, it goes through the
   standard steps of calling use_pkg_config(), but if false, it issues
   a log message and then calls require() with all the inputted
   arguments and an additional argument: -l$1.  So, for something like
   "require_pkg_config libmfx "mfx/mfxvideo.h" MFXInit", this becomes
   "require libmfx "mfx/mfxvideo.h" MFXInit -llibmfx".  This is not a
   perfect solution, but the previous approach didn't work at all
   before when require_pkg_config() is used.
---
 configure | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 7f2b653..ad08b82 100755
--- a/configure
+++ b/configure
@@ -1347,7 +1347,12 @@ use_pkg_config(){
 }
 
 require_pkg_config(){
-use_pkg_config "$@" || die "ERROR: $pkg not found using 
pkg-config$pkg_config_fail_message"
+if test $pkg_config != false; then
+use_pkg_config "$@" || die "ERROR: $pkg not found using 
pkg-config$pkg_config_fail_message"
+else
+log require_pkg_config "No pkg-config, using require for $@"
+require "$@ -l$1"
+fi
 }
 
 require_libfreetype(){
-- 
2.10.1.windows.1

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

Re: [FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency

On 4/13/2017 3:40 PM, Marton Balint wrote:
> 
> On Thu, 13 Apr 2017, Aaron Levinson wrote:
> 
>> On 4/13/2017 2:12 AM, Hendrik Leppkes wrote:
>>> On Thu, Apr 13, 2017 at 10:36 AM, Aaron Levinson  
>> wrote:
>>> Give it some time for the other changes to be reviewed by the people
>>> that actually know decklink itself, you can include that in any new
>>> versions of the patch then, no need to send one for that right now.
>>>
>>> - Hendrik
>>
>> Actually, the coding changes made to the decklink source files in this
>> patch have pretty much nothing to do with decklink itself, and anyone
>> with familiarity with semaphores and pthread could review those changes.
>>  In fact, all I really did was use already existing approaches for
>> replacing a semaphore with the combination of a condition variable,
>> mutex, and counter, and there are plenty of examples of how to do this
>> on the Web.
>>
> 
> Yeah, the changes look fine, please send an updated patch, I will apply
> it after some final testing.
> 
> Thanks,
> Marton

Here's a new version of the patch with the pthreads dependency replaced with 
threads.

Thanks,
Aaron

---


From 7ecc4c280f2185dc1c19131a1dbf9f833ebb42b3 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 13 Apr 2017 18:08:17 -0700
Subject: [PATCH] avdevice/decklink: Removed pthread dependency

Purpose: avdevice/decklink: Removed pthread dependency by replacing
semaphore used in code appropriately.  Doing so makes it easier to
build ffmpeg using Visual C++ on Windows.  This is a contination of
Kyle Schwarz's "avdevice/decklink: Remove pthread dependency" patch
that is available at https://patchwork.ffmpeg.org/patch/2654/ .  This
patch wasn't accepted, and as far as I can tell, there was no
follow-up after it was rejected.

Notes: Used Visual Studio 2015 (with update 3) for this.

Comments:

-- configure: Eliminated pthreads dependency for decklink_indev_deps
   and decklink_outdev_deps and replaced with threads dependency

-- libavdevice/decklink_common.cpp / .h:
a) Eliminated semaphore and replaced with a combination of a mutex,
   condition variable, and a counter (frames_buffer_available_spots).
b) Removed include of pthread.h and semaphore.h and now using
   libavutil/thread.h instead.

-- libavdevice/decklink_dec.cpp: Eliminated include of pthread.h and
   semaphore.h.

-- libavdevice/decklink_enc.cpp:
a) Eliminated include of pthread.h and semaphore.h.
b) Replaced use of semaphore with the equivalent using a combination
   of a mutex, condition variable, and a counter
   (frames_buffer_available_spots).  In theory, libavutil/thread.h and
   the associated code could have been modified instead to add
   cross-platform implementations of the sem_ functions, but an
   inspection of the ffmpeg source base indicates that there are only
   two cases in which semaphores are used (including this one that was
   replaced), so it was deemed to not be worth the effort.
---
 configure   |  4 ++--
 libavdevice/decklink_common.cpp |  3 ---
 libavdevice/decklink_common.h   |  5 -
 libavdevice/decklink_dec.cpp|  3 ---
 libavdevice/decklink_enc.cpp| 23 +++
 5 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/configure b/configure
index 0af4a29..18d79ab 100755
--- a/configure
+++ b/configure
@@ -3000,9 +3000,9 @@ avfoundation_indev_deps="pthreads"
 avfoundation_indev_extralibs="-framework Foundation -framework AVFoundation 
-framework CoreVideo -framework CoreMedia"
 bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h 
dev_video_bktr_ioctl_bt848_h dev_ic_bt8xx_h"
 caca_outdev_deps="libcaca"
-decklink_indev_deps="decklink pthreads"
+decklink_indev_deps="decklink threads"
 decklink_indev_extralibs="-lstdc++"
-decklink_outdev_deps="decklink pthreads"
+decklink_outdev_deps="decklink threads"
 decklink_outdev_extralibs="-lstdc++"
 dshow_indev_deps="IBaseFilter"
 dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 -lshlwapi"
diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
index 587a321..523217c 100644
--- a/libavdevice/decklink_common.cpp
+++ b/libavdevice/decklink_common.cpp
@@ -35,9 +35,6 @@ extern "C" {
 #include 
 #endif
 
-#include 
-#include 
-
 extern "C" {
 #include "libavformat/avformat.h"
 #include "libavutil/imgutils.h"
diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
index 4753287..c12cf18 100644
--- a/libavdevice/decklink_common.h
+++ b/libavdevice/decklink_common.h
@@ -24,6 +24,7 @@
 
 #include 
 
+#include "libavutil/thread.h"

[FFmpeg-devel] [PATCH] Fixed problems with QuickSync (QSV) interlaced video encoding

From da3899b24ad89b4788a3b8191d53b26f5eec328e Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 13 Apr 2017 23:12:30 -0700
Subject: [PATCH] Fixed problems with QuickSync (QSV) interlaced video
 encoding

Purpose: Fixed problems with QuickSync (QSV) interlaced video encoding
that were introduced in revision 1f26a23 on Oct. 31, 2016 (qsv: Merge
libav implementation, at
https://github.com/FFmpeg/FFmpeg/commit/1f26a231bb065276cd80ce02957c759f3197edfa#diff-7d84a34d58597bb7aa4b8239dca1f9f8).
As a result of the qsv libav merge, when attempting to encode
interlaced video, it doesn't work and instead results in a bunch of
incompatible video parameter errors.

Comments:

-- qsvenc.c / .h:
a) Added code back in to set PicStruct appropriately based on whether
   or not interlaced or progressive video is being encoded.  Also
   reintroduced the related code to set the height alignment.  The
   height alignment code was also enhanced slightly (compared to the
   version in 3.2.4) to properly handle progressive video when the
   HEVC encoder is used.  The elimination of this code is the main
   reason why interlaced video encoding stopped working.
b) Reintroduced code to call MFXVideoENCODE_Query() after calling
   init_video_param().  This isn't strictly required to fix the
   interlaced video encoding issue, but it represents a generally good
   practice to make sure that one is working with the right parameter
   values.
---
 libavcodec/qsvenc.c | 33 +
 libavcodec/qsvenc.h |  1 +
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index 9c385a7..24ac390 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -358,6 +358,9 @@ static int init_video_param(AVCodecContext *avctx, 
QSVEncContext *q)
 return AVERROR_BUG;
 q->param.mfx.CodecId = ret;
 
+// TODO:  detect version of MFX--if the minor version is greater than
+// or equal to 19, then can use the same alignment settings as H.264
+// for HEVC
 q->width_align = avctx->codec_id == AV_CODEC_ID_HEVC ? 32 : 16;
 
 if (avctx->level > 0)
@@ -381,20 +384,34 @@ static int init_video_param(AVCodecContext *avctx, 
QSVEncContext *q)
 
 ff_qsv_map_pixfmt(sw_format, &q->param.mfx.FrameInfo.FourCC);
 
-q->param.mfx.FrameInfo.Width  = FFALIGN(avctx->width, 
q->width_align);
-q->param.mfx.FrameInfo.Height = FFALIGN(avctx->height, 32);
 q->param.mfx.FrameInfo.CropX  = 0;
 q->param.mfx.FrameInfo.CropY  = 0;
 q->param.mfx.FrameInfo.CropW  = avctx->width;
 q->param.mfx.FrameInfo.CropH  = avctx->height;
 q->param.mfx.FrameInfo.AspectRatioW   = avctx->sample_aspect_ratio.num;
 q->param.mfx.FrameInfo.AspectRatioH   = avctx->sample_aspect_ratio.den;
-q->param.mfx.FrameInfo.PicStruct  = MFX_PICSTRUCT_PROGRESSIVE;
 q->param.mfx.FrameInfo.ChromaFormat   = MFX_CHROMAFORMAT_YUV420;
 q->param.mfx.FrameInfo.BitDepthLuma   = desc->comp[0].depth;
 q->param.mfx.FrameInfo.BitDepthChroma = desc->comp[0].depth;
 q->param.mfx.FrameInfo.Shift  = desc->comp[0].depth > 8;
 
+q->param.mfx.FrameInfo.Width  = FFALIGN(avctx->width, 
q->width_align);
+if (avctx->flags & AV_CODEC_FLAG_INTERLACED_DCT) {
+// it is important that PicStruct be setup correctly from the
+// start--otherwise, encoding doesn't work and results in a bunch
+// of incompatible video parameter errors
+q->param.mfx.FrameInfo.PicStruct = MFX_PICSTRUCT_FIELD_TFF;
+// height alignment always must be 32 for interlaced video
+q->height_align = 32;
+} else {
+q->param.mfx.FrameInfo.PicStruct = MFX_PICSTRUCT_PROGRESSIVE;
+// for progressive video, the height should be aligned to 16 for
+// H.264.  For HEVC, depending on the version of MFX, it should be
+// either 32 or 16.  The lower number is better if possible.
+q->height_align = avctx->codec_id == AV_CODEC_ID_HEVC ? 32 : 16;
+}
+q->param.mfx.FrameInfo.Height = FFALIGN(avctx->height, q->height_align);
+
 if (avctx->hw_frames_ctx) {
 AVHWFramesContext *frames_ctx = 
(AVHWFramesContext*)avctx->hw_frames_ctx->data;
 AVQSVFramesContext *frames_hwctx = frames_ctx->hwctx;
@@ -740,10 +757,18 @@ int ff_qsv_enc_init(AVCodecContext *avctx, QSVEncContext 
*q)
 if (ret < 0)
 return ret;
 
+ret = MFXVideoENCODE_Query(q->session, &q->param, &q->param);
+if (ret == MFX_WRN_PARTIAL_ACCELERATION) {
+av_log(avctx, AV_LOG_WARNING, "Encoder will work with partial HW 
acceleration\n");
+} else if (ret < 0) {
+return ff_qsv_print_error(avctx, ret,
+  "Error querying encod

Re: [FFmpeg-devel] [PATCH] Enhanced require_pkg_config() in configure to fallback to require() if pkg-config is missing

On 4/14/2017 1:09 AM, Hendrik Leppkes wrote:

On Fri, Apr 14, 2017 at 2:56 AM, Aaron Levinson  wrote:

From 48f7daba16e0fcdb83d9abd254800c7b9f4ab684 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 13 Apr 2017 17:30:47 -0700
Subject: [PATCH] Enhanced require_pkg_config() in configure to fallback to 
require() if pkg-config is missing

Purpose: Enhanced require_pkg_config() in configure to
fallback to require() if pkg-config is missing

Notes: This is likely mainly of relevance when building with MSVC on
Windows.  In my case, I used this approach to get libmfx when invoking
configure with --enable-libmfx, which is used for QuickSync (QSV).

Comments:

-- configure: Enhanced require_pkg_config() function to first check if
   $pkg_config is not false.  If not false, it goes through the
   standard steps of calling use_pkg_config(), but if false, it issues
   a log message and then calls require() with all the inputted
   arguments and an additional argument: -l$1.  So, for something like
   "require_pkg_config libmfx "mfx/mfxvideo.h" MFXInit", this becomes
   "require libmfx "mfx/mfxvideo.h" MFXInit -llibmfx".  This is not a
   perfect solution, but the previous approach didn't work at all
   before when require_pkg_config() is used.
---
 configure | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 7f2b653..ad08b82 100755
--- a/configure
+++ b/configure
@@ -1347,7 +1347,12 @@ use_pkg_config(){
 }

 require_pkg_config(){
-use_pkg_config "$@" || die "ERROR: $pkg not found using 
pkg-config$pkg_config_fail_message"
+if test $pkg_config != false; then
+use_pkg_config "$@" || die "ERROR: $pkg not found using 
pkg-config$pkg_config_fail_message"
+else
+log require_pkg_config "No pkg-config, using require for $@"
+require "$@ -l$1"
+fi
 }

pkg-config typically provides more then just a location of the files,
and while it happens to work for mfx in your case, I don't think this
generic fallback is a good idea.
We already require a linux-esque build environment, even for MSVC
builds (ie. msys or cygwin, etc), having pkg-config on top of the
other required tools isn't that hard of a dependency to fullfill - I
do that all the time.

- Hendrik

OK.  Please disregard this patch, and I will submit a different one that 
does something more specific for libmfx.

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

[FFmpeg-devel] [PATCH] Enhanced configure and Makefile to copy .pdb files to bindir for MSVC builds for make install

From 1059473c449c3079f03461bb42c2d3cc21d1b2c1 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Fri, 14 Apr 2017 18:14:21 -0700
Subject: [PATCH] Enhanced configure and Makefile to copy .pdb files to
 bindir for MSVC builds for make install

Purpose: Enhanced configure and Makefile to copy .pdb files to bindir
for MSVC builds for make install.  Files are also uninstalled
appropriately when make uninstall is exercised.  Placing the PDB files
in the same directory as other binaries can make it easier to debug,
especially if the files are copied to another system.

Note: General idea for how to properly handle the copying of PDB files
associated with programs suggested by Hendrik Leppkes.

Comments:

-- configure:
a) Leveraged already existing SLIB_INSTALL_EXTRA_SHLIB facility (which
   is already pretty specific to Windows) to add .pdb files, in
   addition to .lib files, for shared libraries to bindir during make
   install.
b) Added PROG_INSTALL_EXTRA_BIN variable for MSVC builds and also
   added it to the section that causes it to be added to config.mak.
   This is used in Makefile to copy any additional files associated
   with programs.  For MSVC, it is used to copy the pdb files
   associated with any executables that are built.  Note that such
   executables are build with _g in the file name and are later copied
   to a filename without _g in the file name.  As such, the PDB files
   have _g in the file name.

-- Makefile: Enhanced install-progs and uninstall-progs targets to
   handle PROG_INSTALL_EXTRA_BIN if defined.  It uses a similar
   procedure as already in place for SLIB_INSTALL_EXTRA_SHLIB in
   library.mak.
---
 Makefile  | 2 ++
 configure | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index d5b51de..45c42c6 100644
--- a/Makefile
+++ b/Makefile
@@ -165,6 +165,7 @@ install-progs-$(CONFIG_SHARED): install-libs
 install-progs: install-progs-yes $(AVPROGS)
$(Q)mkdir -p "$(BINDIR)"
$(INSTALL) -c -m 755 $(INSTPROGS) "$(BINDIR)"
+   $(if $(PROG_INSTALL_EXTRA_BIN), $(INSTALL) -m 644 
$(PROG_INSTALL_EXTRA_BIN) "$(BINDIR)")
 
 install-data: $(DATA_FILES) $(EXAMPLES_FILES)
$(Q)mkdir -p "$(DATADIR)/examples"
@@ -175,6 +176,7 @@ uninstall: uninstall-libs uninstall-headers uninstall-progs 
uninstall-data
 
 uninstall-progs:
$(RM) $(addprefix "$(BINDIR)/", $(ALLAVPROGS))
+   $(if $(PROG_INSTALL_EXTRA_BIN), $(RM) $(addprefix "$(BINDIR)/", 
$(PROG_INSTALL_EXTRA_BIN)))
 
 uninstall-data:
$(RM) -r "$(DATADIR)"
diff --git a/configure b/configure
index 18d79ab..88206e3 100755
--- a/configure
+++ b/configure
@@ -4947,9 +4947,10 @@ case $target_os in
 SLIB_CREATE_DEF_CMD='$(SRC_PATH)/compat/windows/makedef 
$(SUBDIR)lib$(NAME).ver $(OBJS) > $$(@:$(SLIBSUF)=.def)'
 SLIB_INSTALL_NAME='$(SLIBNAME_WITH_MAJOR)'
 SLIB_INSTALL_LINKS=
-SLIB_INSTALL_EXTRA_SHLIB='$(SLIBNAME:$(SLIBSUF)=.lib)'
+SLIB_INSTALL_EXTRA_SHLIB='$(SLIBNAME:$(SLIBSUF)=.lib) 
$(SLIBNAME_WITH_MAJOR:$(SLIBSUF)=.pdb)'
 SLIB_INSTALL_EXTRA_LIB='$(SLIBNAME_WITH_MAJOR:$(SLIBSUF)=.def)'
 SHFLAGS='-dll -def:$$(@:$(SLIBSUF)=.def) 
-implib:$(SUBDIR)$(SLIBNAME:$(SLIBSUF)=.lib)'
+PROG_INSTALL_EXTRA_BIN='$(AVPROGS-yes:%=%$(PROGSSUF)_g.pdb)'
 objformat="win32"
 ranlib=:
 enable dos_paths
@@ -6796,6 +6797,7 @@ SLIB_INSTALL_NAME=${SLIB_INSTALL_NAME}
 SLIB_INSTALL_LINKS=${SLIB_INSTALL_LINKS}
 SLIB_INSTALL_EXTRA_LIB=${SLIB_INSTALL_EXTRA_LIB}
 SLIB_INSTALL_EXTRA_SHLIB=${SLIB_INSTALL_EXTRA_SHLIB}
+PROG_INSTALL_EXTRA_BIN=${PROG_INSTALL_EXTRA_BIN}
 VERSION_SCRIPT_POSTPROCESS_CMD=${VERSION_SCRIPT_POSTPROCESS_CMD}
 SAMPLES:=${samples:-\$(FATE_SAMPLES)}
 NOREDZONE_FLAGS=$noredzone_flags
-- 
2.10.1.windows.1

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

[FFmpeg-devel] Added require fallback for libmfx in the case that pkg-config cannot find libmfx

From e0c73c054add0137901d0bf7a7893e42e7e566c8 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Fri, 14 Apr 2017 18:38:37 -0700
Subject: [PATCH] Added require fallback for libmfx in the case that
 pkg-config cannot find libmfx

Purpose: Added require fallback for libmfx in the case that pkg-config
cannot find libmfx.  On Linux, most people likely get libmfx via
https://github.com/lu-zero/mfx_dispatch , but on Windows, the most
well-known way to get libmfx is via the Intel Media SDK, which
provides a static build of libmfx.lib and also provides the source
code for building libmfx yourself.  If built this way, there are no
pkg-config files to be found.  The changes utilize a similar approach
to that already done for libx264 in configure.

Comments:

-- configure: Altered enabled libmfx step to use use_pkg_config()
   instead of require_pkg_config(), and, if use_pkg_config() fails, it
   falls back to require().  Note that the reason that require() is
   passed -llibmfx as the last argument, instead of -lmfx, is the file
   name for the library produced from the Intel Media SDK starts with
   "libmfx".  Apparently, the filename for the library produced via
   https://github.com/lu-zero/mfx_dispatch starts with "mfx".
---
 configure | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 3bea057..b20a0b4 100755
--- a/configure
+++ b/configure
@@ -5819,7 +5819,8 @@ enabled libgsm&& { for gsm_hdr in "gsm.h" 
"gsm/gsm.h"; do
done || die "ERROR: libgsm not found"; }
 enabled libilbc   && require libilbc ilbc.h WebRtcIlbcfix_InitDecode 
-lilbc
 enabled libkvazaar&& require_pkg_config "kvazaar >= 0.8.1" kvazaar.h 
kvz_api_get
-enabled libmfx&& require_pkg_config libmfx "mfx/mfxvideo.h" MFXInit
+enabled libmfx&& { use_pkg_config libmfx "mfx/mfxvideo.h" MFXInit 
||
+   { require libmfx "mfx/mfxvideo.h" MFXInit 
-llibmfx && warn "using libmfx without pkg-config"; } }
 enabled libmodplug&& require_pkg_config libmodplug 
libmodplug/modplug.h ModPlug_Load
 enabled libmp3lame&& require "libmp3lame >= 3.98.3" lame/lame.h 
lame_set_VBR_quality -lmp3lame
 enabled libnut&& require libnut libnut.h nut_demuxer_init -lnut
-- 
2.10.1.windows.1

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

Re: [FFmpeg-devel] [PATCH] Enhanced require_pkg_config() in configure to fallback to require() if pkg-config is missing

On 4/14/2017 6:04 PM, James Almer wrote:

On 4/13/2017 9:56 PM, Aaron Levinson wrote:

From 48f7daba16e0fcdb83d9abd254800c7b9f4ab684 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 13 Apr 2017 17:30:47 -0700
Subject: [PATCH] Enhanced require_pkg_config() in configure to fallback to 
require() if pkg-config is missing

Purpose: Enhanced require_pkg_config() in configure to
fallback to require() if pkg-config is missing

Notes: This is likely mainly of relevance when building with MSVC on
Windows.  In my case, I used this approach to get libmfx when invoking
configure with --enable-libmfx, which is used for QuickSync (QSV).

Comments:

-- configure: Enhanced require_pkg_config() function to first check if
   $pkg_config is not false.  If not false, it goes through the
   standard steps of calling use_pkg_config(), but if false, it issues
   a log message and then calls require() with all the inputted
   arguments and an additional argument: -l$1.  So, for something like
   "require_pkg_config libmfx "mfx/mfxvideo.h" MFXInit", this becomes
   "require libmfx "mfx/mfxvideo.h" MFXInit -llibmfx".  This is not a
   perfect solution, but the previous approach didn't work at all
   before when require_pkg_config() is used.
---
 configure | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 7f2b653..ad08b82 100755
--- a/configure
+++ b/configure
@@ -1347,7 +1347,12 @@ use_pkg_config(){
 }

 require_pkg_config(){
-use_pkg_config "$@" || die "ERROR: $pkg not found using 
pkg-config$pkg_config_fail_message"
+if test $pkg_config != false; then
+use_pkg_config "$@" || die "ERROR: $pkg not found using 
pkg-config$pkg_config_fail_message"
+else
+log require_pkg_config "No pkg-config, using require for $@"
+require "$@ -l$1"
+fi

This will fail in a thousand ways with packages that require other
packages, specific cflags, specific ldflags, or that state specific
versions.
As Hendrik said, pkg-config is not just a fancy way to add -I and -L
options to the compiler and linker.

I responded earlier today to Hendrik's response and indicated that I was 
abandoning the patch.  I just submitted a new patch that does something 
more specific for libmfx.

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

Re: [FFmpeg-devel] [PATCH] Enhanced configure to improve compiler options associated with debugging with Visual C++ (MSVC)

On 4/13/2017 5:03 PM, Aaron Levinson wrote:
> From 558b957eb85a669899750b2e150eba7cdee8dcd9 Mon Sep 17 00:00:00 2001
> From: Aaron Levinson 
> Date: Thu, 13 Apr 2017 16:46:59 -0700
> Subject: [PATCH] Enhanced configure to improve compiler options associated 
> with debugging with Visual C++ (MSVC) 
> 
> Purpose: Enhanced configure to improve compiler options
> associated with debugging with Visual C++ (MSVC)

Please disregard the earlier version of the patch.  I've included a new version 
below that eliminates -MTd from the command-line options, based on a discussion 
that I had on IRC with Hendrik Leppkes.

Thanks,
Aaron Levinson

---

From 4e78a20cf811d87f7a16715f3f9f3553a0d56ce3 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Fri, 14 Apr 2017 18:59:46 -0700
Subject: [PATCH] Enhanced configure to improve compiler options associated
 with debugging with Visual C++ (MSVC)

Purpose: Enhanced configure to improve compiler options associated
with debugging with Visual C++ (MSVC)

Comments:

-- configure:
a) In msvc_common_flags() function, replaced the use of -Z7 with -Zi.
   Effectively, there was no point to using -Z7 anymore given than
   -debug is passed to the linker already (per the line "enabled debug
   && add_ldflags -debug" elsewhere in the file), which causes a .pdb
   to be generated anyway.  -Z7 causes Codeview debug info to be
   placed in .obj files, while -Zi causes debug info to be placed in
   .pdb files.  As a result of switching from -Z7 to -Zi, this may
   result in slightly faster builds with MSVC, since it is apparently
   slower to process Codeview debug info.
b) In probe_cc() function, added _cflags_noopt declaration for MSVC.
   This is set to the following: "-Od -Og".  See comments for more
   details.  This is exposed when --disable-optimizations is used with
   configure.  In addition, now adding -Zo (or -d2Zi+, depending on
   the compiler version) to all the different _cflags_ variables to
   make it easier to debug optimized builds.  See changes and comments
   for further details.
---
 configure | 29 -
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 1703111..c2d8b48 100755
--- a/configure
+++ b/configure
@@ -3844,7 +3844,7 @@ msvc_common_flags(){
 -std=c99) ;;
 # Common flags
 -fomit-frame-pointer) ;;
--g)   echo -Z7 ;;
+-g)   echo -Zi ;;
 -fno-math-errno)  ;;
 -fno-common)  ;;
 -fno-signed-zeros);;
@@ -4153,6 +4153,33 @@ probe_cc(){
 _DEPFLAGS='$(CPPFLAGS) $(CFLAGS) -showIncludes -Zs'
 _cflags_speed="-O2"
 _cflags_size="-O1"
+# Need to use -Og with -Od because otherwise has link issues with
+# "if (ARCH_...)" code.  Unfortunately, this isn't quite the same as
+# a fully debug build.  If the Dead Code Elimination (DCE) compiler
+# optimization requirement is ever removed, this can be replaced
+# with just -Od.  Note that the use of -Og results in a compiler
+# deprecation warning.
+_cflags_noopt="-Od -Og"
+cl_major_ver=$(cl 2>&1 | sed -n 's/.*Version 
\([[:digit:]]\{1,\}\)\..*/\1/p')
+
+# Make it easier to debug release builds.
+# See https://msdn.microsoft.com/en-us/library/dn785163.aspx for more
+# details.
+# It may not be necessary to add -Zo to _cflags_noopt due to the use
+# of -Od, but because -Og is also added currently, it is best to
+# add -Zo for now to make sure the best debugging experience is
+# possible.  If the DCE compiler optimization requirement is ever
+# removed, the _cflags_noopt lines can be removed.
+if [ -z "$cl_major_ver" ] || [ $cl_major_ver -ge 18 ]; then
+_cflags_speed="${_cflags_speed} -Zo"
+_cflags_size="${_cflags_size} -Zo"
+_cflags_noopt="${_cflags_noopt} -Zo"
+else
+_cflags_speed="${_cflags_speed} -d2Zi+"
+_cflags_size="${_cflags_size} -d2Zi+"
+_cflags_noopt="${_cflags_noopt} -d2Zi+"
+fi
+
 if $_cc -nologo- 2>&1 | grep -q Linker; then
 _ld_o='-out:$@'
 else
-- 
2.10.1.windows.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Added require fallback for libmfx in the case that pkg-config cannot find libmfx

2017-04-15 Thread Aaron Levinson

On 4/15/2017 4:25 AM, Ricardo Constantino wrote:
> On 15 April 2017 at 02:51, Aaron Levinson  wrote:
> 
>> From e0c73c054add0137901d0bf7a7893e42e7e566c8 Mon Sep 17 00:00:00 2001
>> From: Aaron Levinson 
>> Date: Fri, 14 Apr 2017 18:38:37 -0700
>> Subject: [PATCH] Added require fallback for libmfx in the case that
>>  pkg-config cannot find libmfx
>>
>> Purpose: Added require fallback for libmfx in the case that pkg-config
>> cannot find libmfx.  On Linux, most people likely get libmfx via
>> https://github.com/lu-zero/mfx_dispatch , but on Windows, the most
>> well-known way to get libmfx is via the Intel Media SDK, which
>> provides a static build of libmfx.lib and also provides the source
>> code for building libmfx yourself.  If built this way, there are no
>> pkg-config files to be found.  The changes utilize a similar approach
>> to that already done for libx264 in configure.
>>
>> Comments:
>>
>> -- configure: Altered enabled libmfx step to use use_pkg_config()
>>instead of require_pkg_config(), and, if use_pkg_config() fails, it
>>falls back to require().  Note that the reason that require() is
>>passed -llibmfx as the last argument, instead of -lmfx, is the file
>>name for the library produced from the Intel Media SDK starts with
>>"libmfx".  Apparently, the filename for the library produced via
>>https://github.com/lu-zero/mfx_dispatch starts with "mfx".
>> ---
>>  configure | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index 3bea057..b20a0b4 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5819,7 +5819,8 @@ enabled libgsm&& { for gsm_hdr in
>> "gsm.h" "gsm/gsm.h"; do
>> done || die "ERROR: libgsm not found"; }
>>  enabled libilbc   && require libilbc ilbc.h
>> WebRtcIlbcfix_InitDecode -lilbc
>>  enabled libkvazaar&& require_pkg_config "kvazaar >= 0.8.1"
>> kvazaar.h kvz_api_get
>> -enabled libmfx&& require_pkg_config libmfx "mfx/mfxvideo.h"
>> MFXInit
>> +enabled libmfx&& { use_pkg_config libmfx "mfx/mfxvideo.h"
>> MFXInit ||
>> +   { require libmfx "mfx/mfxvideo.h" MFXInit
>> -llibmfx && warn "using libmfx without pkg-config"; } }
>>
> Needs a '|| die "ERROR: libmfx not found";' before the last curly bracket
> or it won't complain about not finding libmfx if enabled.

Doesn't require() already do that?  Here is the contents of require():

require(){
log require "$@"
name_version="$1"
headers="$2"
func="$3"
shift 3
check_lib "$headers" $func "$@" || die "ERROR: $name_version not found"
}

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

Re: [FFmpeg-devel] [PATCH] Made minor changes to get the decklink avdevice code to build using Visual C++

2017-04-15 Thread Aaron Levinson


On 4/15/2017 4:19 AM, Marton Balint wrote:


On Thu, 13 Apr 2017, Aaron Levinson wrote:


On 4/13/2017 1:23 PM, Hendrik Leppkes wrote:

[...]




From 00fdc9d15414a92a155eb7d1394bac3736dc9405 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 13 Apr 2017 14:22:19 -0700
Subject: [PATCH] Made minor changes to get the decklink avdevice code
to build using Visual C++.



Maybe it just me, but as I mentioned earlier, I don't like too verbose
comments in the code, see below:


diff --git a/libavdevice/decklink_common.cpp
b/libavdevice/decklink_common.cpp
index f01fba9..523217c 100644
--- a/libavdevice/decklink_common.cpp
+++ b/libavdevice/decklink_common.cpp
@@ -19,6 +19,15 @@
 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
02110-1301 USA
 */

+// Moved inclusion of internal.h here in order to get it to build
successfully
+// when using Visual C++ to build--otherwise, compilation errors result
+// due to winsock.h (which is included indirectly by DeckLinkAPI.h and
+// DeckLinkAPI_i.c) conflicting with winsock2.h, which is included by
+// internal.h.  If winsock2.h is included first, then the conflict is
resolved.


This can be as short as this:

/* Include internal.h first to avoid conflict of winsock.h (used by
 * DeckLink) and winsock2.h (used by libavformat) in MSVC++ builds */

(for multiline comments I think /* */ is preferred)

Although since you do this in multiple headers, maybe it is enough if
you specify the reason in the commit message, and delete the comment
from here entirely.


I think it is a good idea to have a comment in the code, as then it is 
front in center if someone in the future wonders why internal.h precedes 
the other includes and decides to move it because it will look cleaner, 
thereby breaking the MSVC build.  Although, in that case, maybe it would 
be preferable to have the same comment in each of the three places, as 
opposed to just one.


A shorter comment is fine, and your example comment explains things well 
enough, although I tend to think that more information is better than 
less for comments in code.  From my perspective, "used by DeckLink" is a 
bit vague, since technically, DeckLinkAPI.h and DeckLinkAPI_i.c would be 
generated by the user if the actual Blackmagic DeckLink SDK were used 
(these files are not actually supplied with the Blackmagic DeckLink 
SDK), which is how I got DeckLinkAPI.h and DeckLinkAPI_c.h when I built 
ffmpeg.  Well, DeckLinkAPI.h is included in the Linux header files in 
the Blackmagic DeckLink SDK, but the _i.c file is not, and on Windows, 
neither file is provided.


Regarding multi-line comments being wrapped in /* */ vs using // on each 
line, it doesn't so much matter in this case, but I personally find // 
more versatile because then it makes it easier to comment out blocks of 
code.  But, if that's the standard for the ffmpeg code base, then I'll 
make that change.



@@ -262,8 +265,18 @@ static int64_t
get_pkt_pts(IDeckLinkVideoInputFrame *videoFrame,
res =
videoFrame->GetHardwareReferenceTimestamp(time_base.den, &bmd_pts,
&bmd_duration);
break;
case PTS_SRC_WALLCLOCK:
-pts = av_rescale_q(wallclock, AV_TIME_BASE_Q, time_base);
+{
+// doing the following rather than using AV_TIME_BASE_Q
because
+// AV_TIME_BASE_Q doesn't work when building with Visual C++
+// for C++ files (works for C files).  When building C++
files,
+// it results in compiler error C4576.  At least, this is
the case
+// with Visual C++ 2015.


And this is:

// MSVC does not support compound literals like AV_TIME_BASE_Q in C++ code


+AVRational timebase;
+timebase.num = 1;
+timebase.den = AV_TIME_BASE;
+pts = av_rescale_q(wallclock, timebase, time_base);
break;
+}


This whole block needs to be indented 1 column more I think.


I'll double-check later today to make sure that it is indented properly, 
adjust the comment, and submit a new patch then.



Regards,
Marton


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

Re: [FFmpeg-devel] Debug builds and if (ARCH_...) linking issues


On 4/16/2017 8:57 AM, Matt Oliver wrote:

On 14 April 2017 at 08:05, Hendrik Leppkes  wrote:


On Fri, Apr 14, 2017 at 12:00 AM, Ronald S. Bultje 
wrote:

Hi Aaron,

On Thu, Apr 13, 2017 at 4:47 PM, Aaron Levinson 
wrote:


#if ARCH_AARCH64
if (ARCH_AARCH64)
return ff_get_cpu_flags_aarch64();
#endif



If you add #if, at least replace the if with it. #if + if is redundant.



Such a script could be reviewed and checked into the source base, then
applied to the source base, and possibly reused in the future as

necessary.



Why not use the script as a preprocessor during compilation? We did that
for pre-2013 MSVC compilation and that worked """fine""".

Or why not ask Microsoft to add a CLI option to cl.exe for enabling DCE

but

otherwise disabling optimizations? All other compilers seem capable of
this, so it's odd to see that big Microsoft is incompetent whereas a

bunch

of opensource hippies could do the same 10+ yrs ago.



Its not a MSVC-exclusive problem. Many compilers don't perform DCE in
full debug builds, which are as such not possible with ffmpeg, making
debuging sometimes a bit annoying when a bunch of variables are
optimized out and stuff gets inlined.



Unfortunately writing a script is a rather complex task as there are many
DCE blocks that are actually generated by macro expansion and various
pre-processor trickery (swresample is full of this sort of stuff). Combine
that with DCE blocks being nested within each other and it becomes a rather
complex task to find them all. Much more complex than what a simple regex
script can handle.

If people object to changing the current code base then the only solution
would be a program/script that can generate some empty definitions for all
the functions/variables that are used in DCE blocks so as to avoid the
errors about them not existing. These empty definitions would have to be
maintained in a set of external files or generated at compile time.

There are actually a considerable number of missing funcs/vars used in DCE
blocks. As an example i have attached a list of empty funcs/vars that I
have been able to programmatically detect (there are a lot! and i may have
missed some).


I like using this approach, but I think such files should only be added 
to the build if --disable-optimizations is passed into configure.  I 
think that's the sort of approach that most people could get behind 
since it won't affect the regular build.  I think it is okay if these .c 
files aren't perfect from the start, and while it would be nice if 
people that add new functions (or change existing function definitions) 
to arch (or similar) were to make appropriate changes to these files, if 
they don't, that's okay too.  There are likely a limited number of 
people that would choose to employ --disable-optimizations, and it is 
not a big deal to add a function or two to these files as they crop up 
and submit a patch for the changes.


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

Re: [FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency


On 4/15/2017 9:32 PM, James Almer wrote:

On 4/15/2017 7:41 AM, Marton Balint wrote:


On Thu, 13 Apr 2017, Aaron Levinson wrote:


On 4/13/2017 3:40 PM, Marton Balint wrote:


On Thu, 13 Apr 2017, Aaron Levinson wrote:


On 4/13/2017 2:12 AM, Hendrik Leppkes wrote:

On Thu, Apr 13, 2017 at 10:36 AM, Aaron Levinson 

wrote:

Give it some time for the other changes to be reviewed by the people
that actually know decklink itself, you can include that in any new
versions of the patch then, no need to send one for that right now.

- Hendrik


Actually, the coding changes made to the decklink source files in this
patch have pretty much nothing to do with decklink itself, and anyone
with familiarity with semaphores and pthread could review those changes.
 In fact, all I really did was use already existing approaches for
replacing a semaphore with the combination of a condition variable,
mutex, and counter, and there are plenty of examples of how to do this
on the Web.



Yeah, the changes look fine, please send an updated patch, I will apply
it after some final testing.

Thanks,
Marton


Here's a new version of the patch with the pthreads dependency replaced with 
threads.



Thanks, applied.


Wouldn't it be simpler to add posix semaphore emulation to w32threads
and os2threads?
The former should be trivial, and probably even without the need to
use mutexes or conditional variables given there's CreateSemaphore
and ReleaseSemaphore for this purpose. Not sure about the latter.


I addressed this in one of the commit log messages:

-

-- libavdevice/decklink_enc.cpp:
a) Eliminated include of pthread.h and semaphore.h.
b) Replaced use of semaphore with the equivalent using a combination
   of a mutex, condition variable, and a counter
   (frames_buffer_available_spots).  In theory, libavutil/thread.h and
   the associated code could have been modified instead to add
   cross-platform implementations of the sem_ functions, but an
   inspection of the ffmpeg source base indicates that there are only
   two cases in which semaphores are used (including this one that was
   replaced), so it was deemed to not be worth the effort.

--

I considered this approach, but the thought of having to do this in 
os2threads.h pretty much dissuaded me.  I had thought that OS/2 was 
pretty much dead, but it appears that I was wrong.  There is a yearly 
conference, and a new version of OS/2 will soon be released.


However, the two places in ffmpeg that the sem_ functions were/are used 
are likely not relevant to OS/2 anyway:


-- DeckLink:  Blackmagic only provides drivers/kernel modules for 
Windows, Linux, and OS/X
-- avdevice/jack.c:  This pertains to www.jackaudio.org, and at least 
based on what I can see at https://github.com/jackaudio/jack2, there 
doesn't appear to be any build configuration for OS/2.  jack support 
also apparently needs sem_timedwait().


As such, someone could likely get away with just implementing the sem_ 
functions in w32threads.h.


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

Re: [FFmpeg-devel] Debug builds and if (ARCH_...) linking issues


On 4/16/2017 10:08 AM, Ronald S. Bultje wrote:

Hi,

On Sun, Apr 16, 2017 at 12:59 PM, Aaron Levinson 
wrote:


I like using this approach, but I think such files should only be added to
the build if --disable-optimizations is passed into configure.



Or you could detect in configure if the compiler supports DCE?

Ronald


All the compilers that are supported by ffmpeg (including MSVC) 
_already_ support DCE.  They must in order to be able to do any 
successful optimized builds.  So, I think this is really only relevant 
when optimizations are deliberately disabled, and I think it is 
reasonable to expect that DCE will be disabled if optimizations are 
disabled.


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

Re: [FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency


On 4/16/2017 12:15 PM, wm4 wrote:

On Sun, 16 Apr 2017 15:23:15 -0300
James Almer  wrote:


On 4/16/2017 2:17 PM, Aaron Levinson wrote:

On 4/15/2017 9:32 PM, James Almer wrote:

On 4/15/2017 7:41 AM, Marton Balint wrote:


On Thu, 13 Apr 2017, Aaron Levinson wrote:


On 4/13/2017 3:40 PM, Marton Balint wrote:


On Thu, 13 Apr 2017, Aaron Levinson wrote:


On 4/13/2017 2:12 AM, Hendrik Leppkes wrote:

On Thu, Apr 13, 2017 at 10:36 AM, Aaron Levinson 

wrote:

Give it some time for the other changes to be reviewed by the people
that actually know decklink itself, you can include that in any new
versions of the patch then, no need to send one for that right now.

- Hendrik


Actually, the coding changes made to the decklink source files in this
patch have pretty much nothing to do with decklink itself, and anyone
with familiarity with semaphores and pthread could review those changes.
 In fact, all I really did was use already existing approaches for
replacing a semaphore with the combination of a condition variable,
mutex, and counter, and there are plenty of examples of how to do this
on the Web.



Yeah, the changes look fine, please send an updated patch, I will apply
it after some final testing.

Thanks,
Marton


Here's a new version of the patch with the pthreads dependency replaced with 
threads.



Thanks, applied.


Wouldn't it be simpler to add posix semaphore emulation to w32threads
and os2threads?
The former should be trivial, and probably even without the need to
use mutexes or conditional variables given there's CreateSemaphore
and ReleaseSemaphore for this purpose. Not sure about the latter.


I addressed this in one of the commit log messages:


Yes. I made the mistake of reading the commit message after
reading the code and writing a reply, sorry about that :P



-

-- libavdevice/decklink_enc.cpp:
a) Eliminated include of pthread.h and semaphore.h.
b) Replaced use of semaphore with the equivalent using a combination
   of a mutex, condition variable, and a counter
   (frames_buffer_available_spots).  In theory, libavutil/thread.h and
   the associated code could have been modified instead to add
   cross-platform implementations of the sem_ functions, but an
   inspection of the ffmpeg source base indicates that there are only
   two cases in which semaphores are used (including this one that was
   replaced), so it was deemed to not be worth the effort.

--

I considered this approach, but the thought of having to do this in 
os2threads.h pretty much dissuaded me.  I had thought that OS/2 was pretty much 
dead, but it appears that I was wrong.  There is a yearly conference, and a new 
version of OS/2 will soon be released.

However, the two places in ffmpeg that the sem_ functions were/are used are 
likely not relevant to OS/2 anyway:

-- DeckLink:  Blackmagic only provides drivers/kernel modules for Windows, 
Linux, and OS/X
-- avdevice/jack.c:  This pertains to www.jackaudio.org, and at least based on 
what I can see at https://github.com/jackaudio/jack2, there doesn't appear to 
be any build configuration for OS/2.  jack support also apparently needs 
sem_timedwait().

As such, someone could likely get away with just implementing the sem_ 
functions in w32threads.h.

Aaron


We have a developer that keeps os2threads.h up to date, so he
will probably add sem_t compat wrappers to it, if not now when
a module that works on OS/2 needs it.

IMO, if you can and want then add win32 wrappers to w32threads
right now. The pthread_once wrapper was added for one use case
then as soon as it became available it started to see more use
elsewhere in the tree. The same might happen to Semaphores.


Beware that semaphores don't work on OSX.


I believe that semaphores on OS X are handled via dispatch semaphore. 
See instances of "dispatch" in configure and compat/dispatch_semaphore.


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

Re: [FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency


On 4/16/2017 1:33 PM, Timo Rothenpieler wrote:

Thanks, applied.

Regards,
Marton


This seems to have broken the coverity builds:
https://travis-ci.org/FFmpeg/FFmpeg-Coverity/builds/222597943#L1103


It was suggested on IRC by James Almer that "it seems to complain about 
ubitux's 'strict' pthread implementation (for assert levels above 0). 
Maybe it needs to be disabled for C++ sources."


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

Re: [FFmpeg-devel] [PATCH] Made minor changes to get the decklink avdevice code to build using Visual C++

On 4/15/2017 6:13 AM, Aaron Levinson wrote:
> On 4/15/2017 4:19 AM, Marton Balint wrote:
>>
>> On Thu, 13 Apr 2017, Aaron Levinson wrote:
>>
>>> On 4/13/2017 1:23 PM, Hendrik Leppkes wrote:
>> [...]
>>
>>> 
>>>
>>>
>>> From 00fdc9d15414a92a155eb7d1394bac3736dc9405 Mon Sep 17 00:00:00 2001
>>> From: Aaron Levinson 
>>> Date: Thu, 13 Apr 2017 14:22:19 -0700
>>> Subject: [PATCH] Made minor changes to get the decklink avdevice code
>>> to build using Visual C++.
>>>
>>
>> Maybe it just me, but as I mentioned earlier, I don't like too verbose
>> comments in the code, see below:
>>
>>> diff --git a/libavdevice/decklink_common.cpp
>>> b/libavdevice/decklink_common.cpp
>>> index f01fba9..523217c 100644
>>> --- a/libavdevice/decklink_common.cpp
>>> +++ b/libavdevice/decklink_common.cpp
>>> @@ -19,6 +19,15 @@
>>>  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>> 02110-1301 USA
>>>  */
>>>
>>> +// Moved inclusion of internal.h here in order to get it to build
>>> successfully
>>> +// when using Visual C++ to build--otherwise, compilation errors result
>>> +// due to winsock.h (which is included indirectly by DeckLinkAPI.h and
>>> +// DeckLinkAPI_i.c) conflicting with winsock2.h, which is included by
>>> +// internal.h.  If winsock2.h is included first, then the conflict is
>>> resolved.
>>
>> This can be as short as this:
>>
>> /* Include internal.h first to avoid conflict of winsock.h (used by
>>  * DeckLink) and winsock2.h (used by libavformat) in MSVC++ builds */
>>
>> (for multiline comments I think /* */ is preferred)
>>
>> Although since you do this in multiple headers, maybe it is enough if
>> you specify the reason in the commit message, and delete the comment
>> from here entirely.
> 
> I think it is a good idea to have a comment in the code, as then it is
> front in center if someone in the future wonders why internal.h precedes
> the other includes and decides to move it because it will look cleaner,
> thereby breaking the MSVC build.  Although, in that case, maybe it would
> be preferable to have the same comment in each of the three places, as
> opposed to just one.
> 
> A shorter comment is fine, and your example comment explains things well
> enough, although I tend to think that more information is better than
> less for comments in code.  From my perspective, "used by DeckLink" is a
> bit vague, since technically, DeckLinkAPI.h and DeckLinkAPI_i.c would be
> generated by the user if the actual Blackmagic DeckLink SDK were used
> (these files are not actually supplied with the Blackmagic DeckLink
> SDK), which is how I got DeckLinkAPI.h and DeckLinkAPI_c.h when I built
> ffmpeg.  Well, DeckLinkAPI.h is included in the Linux header files in
> the Blackmagic DeckLink SDK, but the _i.c file is not, and on Windows,
> neither file is provided.
> 
> Regarding multi-line comments being wrapped in /* */ vs using // on each
> line, it doesn't so much matter in this case, but I personally find //
> more versatile because then it makes it easier to comment out blocks of
> code.  But, if that's the standard for the ffmpeg code base, then I'll
> make that change.
> 
>>> @@ -262,8 +265,18 @@ static int64_t
>>> get_pkt_pts(IDeckLinkVideoInputFrame *videoFrame,
>>> res =
>>> videoFrame->GetHardwareReferenceTimestamp(time_base.den, &bmd_pts,
>>> &bmd_duration);
>>> break;
>>> case PTS_SRC_WALLCLOCK:
>>> -pts = av_rescale_q(wallclock, AV_TIME_BASE_Q, time_base);
>>> +{
>>> +// doing the following rather than using AV_TIME_BASE_Q
>>> because
>>> +// AV_TIME_BASE_Q doesn't work when building with Visual
>>> C++
>>> +// for C++ files (works for C files).  When building C++
>>> files,
>>> +// it results in compiler error C4576.  At least, this is
>>> the case
>>> +// with Visual C++ 2015.
>>
>> And this is:
>>
>> // MSVC does not support compound literals like AV_TIME_BASE_Q in C++
>> code
>>
>>> +AVRational timebase;
>>> +timebase.num = 1;
>>> +timebase.den = AV_TIME_BASE;
>>> +pts = av_rescale_q(wallclock, timebase, time_base);
>>>

Re: [FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency


On 4/16/2017 3:31 PM, Hendrik Leppkes wrote:

On Mon, Apr 17, 2017 at 12:26 AM, Aaron Levinson  wrote:

On 4/16/2017 1:33 PM, Timo Rothenpieler wrote:


Thanks, applied.

Regards,
Marton



This seems to have broken the coverity builds:
https://travis-ci.org/FFmpeg/FFmpeg-Coverity/builds/222597943#L1103



It was suggested on IRC by James Almer that "it seems to complain about
ubitux's 'strict' pthread implementation (for assert levels above 0). Maybe
it needs to be disabled for C++ sources."



The real problem is in av_err2str, which uses a temporary array, which
is either not valid in C++ (likely), or not supported by MSVC for
reasons (although in contrast to C support, its C++ support is
generally pretty good).
But considering how little C++ code we have, its probably best to
somehow avoid using it instead of trying to fix it.

- Hendrik


I don't think this has much to do with MSVC here, because coverity is 
doing a gcc/g++ build.  I suspect that it is failing with any gcc/g++ 
build because of the patch.  I'll work on this now.


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

Re: [FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency


On 4/16/2017 4:25 PM, Aaron Levinson wrote:

On 4/16/2017 3:31 PM, Hendrik Leppkes wrote:

On Mon, Apr 17, 2017 at 12:26 AM, Aaron Levinson
 wrote:

On 4/16/2017 1:33 PM, Timo Rothenpieler wrote:


Thanks, applied.

Regards,
Marton



This seems to have broken the coverity builds:
https://travis-ci.org/FFmpeg/FFmpeg-Coverity/builds/222597943#L1103



It was suggested on IRC by James Almer that "it seems to complain about
ubitux's 'strict' pthread implementation (for assert levels above 0).
Maybe
it needs to be disabled for C++ sources."



The real problem is in av_err2str, which uses a temporary array, which
is either not valid in C++ (likely), or not supported by MSVC for
reasons (although in contrast to C support, its C++ support is
generally pretty good).
But considering how little C++ code we have, its probably best to
somehow avoid using it instead of trying to fix it.

- Hendrik


I don't think this has much to do with MSVC here, because coverity is
doing a gcc/g++ build.  I suspect that it is failing with any gcc/g++
build because of the patch.  I'll work on this now.

Aaron


The build issue occurs with g++ and an assert level greater than 1, 
although it would also occur with MSVC++ if pthreads is enabled.  I will 
submit a separate patch e-mail that fixes the build error.


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

[FFmpeg-devel] libavutil/thread.h: Fixed g++ build error when ASSERT_LEVEL is greater than 1

From 9e6a9e2b8d58f17c661a3f455e03c95587ec7b18 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Sun, 16 Apr 2017 17:13:31 -0700
Subject: [PATCH] libavutil/thread.h:  Fixed g++ build error when
 ASSERT_LEVEL is greater than 1

Purpose: libavutil/thread.h: Fixed g++ build error when ASSERT_LEVEL
is greater than 1.  This is only relevant when thread.h is included by
C++ files.  In this case, the relevant code is only defined if
HAVE_PTHREADS is defined as 1.  Use configure --assert-level=2 to do
so.

Note: Issue discovered as a result of Coverity build failure.  Cause
of build failure pinpointed by Hendrik Leppkes.

Comments:

-- libavutil/thread.h: Altered ASSERT_PTHREAD_NORET definition such
   that it uses av_make_error_string instead of av_err2str().
   av_err2str() uses a "parenthesized type followed by an initializer
   list", which is apparently not valid C++.  This issue started
   occurring because thread.h is now included by the DeckLink C++
   files.  The alteration does the equivalent of what av_err2str()
   does, but instead declares the character buffer as a local
   variable.
---
 libavutil/thread.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavutil/thread.h b/libavutil/thread.h
index 6e57447..f108e20 100644
--- a/libavutil/thread.h
+++ b/libavutil/thread.h
@@ -36,8 +36,11 @@
 #define ASSERT_PTHREAD_NORET(func, ...) do {\
 int ret = func(__VA_ARGS__);\
 if (ret) {  \
+char errbuf[AV_ERROR_MAX_STRING_SIZE] = ""; \
 av_log(NULL, AV_LOG_FATAL, AV_STRINGIFY(func)   \
-   " failed with error: %s\n", av_err2str(AVERROR(ret)));   \
+   " failed with error: %s\n",  \
+   av_make_error_string(errbuf, AV_ERROR_MAX_STRING_SIZE,   \
+AVERROR(ret))); \
 abort();\
 }   \
 } while (0)
-- 
2.10.1.windows.1

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

Re: [FFmpeg-devel] libavutil/thread.h: Fixed g++ build error when ASSERT_LEVEL is greater than 1


I'll try that again with [PATCH] in the subject line.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] libavutil/thread.h: Fixed g++ build error when ASSERT_LEVEL is greater than 1

From 9e6a9e2b8d58f17c661a3f455e03c95587ec7b18 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Sun, 16 Apr 2017 17:13:31 -0700
Subject: [PATCH] libavutil/thread.h:  Fixed g++ build error when
 ASSERT_LEVEL is greater than 1

Purpose: libavutil/thread.h: Fixed g++ build error when ASSERT_LEVEL
is greater than 1.  This is only relevant when thread.h is included by
C++ files.  In this case, the relevant code is only defined if
HAVE_PTHREADS is defined as 1.  Use configure --assert-level=2 to do
so.

Note: Issue discovered as a result of Coverity build failure.  Cause
of build failure pinpointed by Hendrik Leppkes.

Comments:

-- libavutil/thread.h: Altered ASSERT_PTHREAD_NORET definition such
   that it uses av_make_error_string instead of av_err2str().
   av_err2str() uses a "parenthesized type followed by an initializer
   list", which is apparently not valid C++.  This issue started
   occurring because thread.h is now included by the DeckLink C++
   files.  The alteration does the equivalent of what av_err2str()
   does, but instead declares the character buffer as a local
   variable.
---
 libavutil/thread.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavutil/thread.h b/libavutil/thread.h
index 6e57447..f108e20 100644
--- a/libavutil/thread.h
+++ b/libavutil/thread.h
@@ -36,8 +36,11 @@
 #define ASSERT_PTHREAD_NORET(func, ...) do {\
 int ret = func(__VA_ARGS__);\
 if (ret) {  \
+char errbuf[AV_ERROR_MAX_STRING_SIZE] = ""; \
 av_log(NULL, AV_LOG_FATAL, AV_STRINGIFY(func)   \
-   " failed with error: %s\n", av_err2str(AVERROR(ret)));   \
+   " failed with error: %s\n",  \
+   av_make_error_string(errbuf, AV_ERROR_MAX_STRING_SIZE,   \
+AVERROR(ret))); \
 abort();\
 }   \
 } while (0)
-- 
2.10.1.windows.1

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

Re: [FFmpeg-devel] libavutil/thread.h: Fixed g++ build error when ASSERT_LEVEL is greater than 1

2017-04-17 Thread Aaron Levinson


On 4/17/2017 8:28 AM, wm4 wrote:

On Mon, 17 Apr 2017 12:06:59 -0300
James Almer  wrote:


On 4/17/2017 5:39 AM, Clément Bœsch wrote:

On Sun, Apr 16, 2017 at 05:20:02PM -0700, Aaron Levinson wrote:

From 9e6a9e2b8d58f17c661a3f455e03c95587ec7b18 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Sun, 16 Apr 2017 17:13:31 -0700
Subject: [PATCH] libavutil/thread.h:  Fixed g++ build error when
 ASSERT_LEVEL is greater than 1

Purpose: libavutil/thread.h: Fixed g++ build error when ASSERT_LEVEL
is greater than 1.  This is only relevant when thread.h is included by
C++ files.  In this case, the relevant code is only defined if
HAVE_PTHREADS is defined as 1.  Use configure --assert-level=2 to do
so.

Note: Issue discovered as a result of Coverity build failure.  Cause
of build failure pinpointed by Hendrik Leppkes.

Comments:

-- libavutil/thread.h: Altered ASSERT_PTHREAD_NORET definition such
   that it uses av_make_error_string instead of av_err2str().
   av_err2str() uses a "parenthesized type followed by an initializer
   list", which is apparently not valid C++.  This issue started
   occurring because thread.h is now included by the DeckLink C++
   files.  The alteration does the equivalent of what av_err2str()
   does, but instead declares the character buffer as a local
   variable.
---
 libavutil/thread.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavutil/thread.h b/libavutil/thread.h
index 6e57447..f108e20 100644
--- a/libavutil/thread.h
+++ b/libavutil/thread.h
@@ -36,8 +36,11 @@
 #define ASSERT_PTHREAD_NORET(func, ...) do {\
 int ret = func(__VA_ARGS__);\
 if (ret) {  \
+char errbuf[AV_ERROR_MAX_STRING_SIZE] = ""; \
 av_log(NULL, AV_LOG_FATAL, AV_STRINGIFY(func)   \
-   " failed with error: %s\n", av_err2str(AVERROR(ret)));   \
+   " failed with error: %s\n",  \
+   av_make_error_string(errbuf, AV_ERROR_MAX_STRING_SIZE,   \
+AVERROR(ret))); \
 abort();\
 }   \
 } while (0)


I don't like limiting ourselves in the common C code of the project
because C++ is a bad and limited language. Can't you solve this by bumping
the minimal requirement of C++ version?


We're already using C++11 when available because of atomics on mediacodec.
Also, just tried and it seems to fail even with C++14, so it just doesn't
work with C++.

We could instead just make these strict assert wrappers work only on C
code by for example checking for defined(__cplusplus).


Better solution: move all the code to a .c file.


I've spent some time considering what would be involved in moving the 
relevant code into a .c file.  thread.h is a header file that needs to 
be included to provide implementations of various pthread_ APIs on 
Windows and OS/2 without needing to link to pthread on those OS's.  If 
pthread is available, it just includes pthread.h.  So, it sort of acts 
like a portability layer.  Providing it in the form of a .h file acts as 
a convenience.  If the implementation were moved into a .c file, that 
tends to imply that it will reside in one of the ffmpeg libraries, 
likely libavutil.  And that also implies that functions with the name 
pthread_create, etc, would be exported by libavutil, which is a bad 
idea.  Instead, the right way to go is to provide a true threading 
portability layer with exported functions that start with, say, 
av_thread_.  But, that's a decent project, and while I'm willing to 
undertake it, I would like to see some support for this endeavor first.


However, there also seems to be some resistance to supporting C++ in 
ffmpeg.  The DeckLink C++ files were contributed to ffmpeg in February 
2014, over three years ago.  While there is certainly no issue with 
using C-specific functionality in .c files, there is certainly an issue 
with doing so in header files that are intended to be used by any aspect 
of the project, whether in .c or .cpp files.  thread.h is an example of 
a header file that should be suitable for use in either .c or .cpp 
files.  The patch that I submitted accomplishes exactly that.


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

Re: [FFmpeg-devel] Added require fallback for libmfx in the case that pkg-config cannot find libmfx

2017-04-19 Thread Aaron Levinson


On 4/14/2017 6:51 PM, Aaron Levinson wrote:

From e0c73c054add0137901d0bf7a7893e42e7e566c8 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Fri, 14 Apr 2017 18:38:37 -0700
Subject: [PATCH] Added require fallback for libmfx in the case that
 pkg-config cannot find libmfx

Purpose: Added require fallback for libmfx in the case that pkg-config
cannot find libmfx.  On Linux, most people likely get libmfx via
https://github.com/lu-zero/mfx_dispatch , but on Windows, the most
well-known way to get libmfx is via the Intel Media SDK, which
provides a static build of libmfx.lib and also provides the source
code for building libmfx yourself.  If built this way, there are no
pkg-config files to be found.  The changes utilize a similar approach
to that already done for libx264 in configure.

Comments:

-- configure: Altered enabled libmfx step to use use_pkg_config()
   instead of require_pkg_config(), and, if use_pkg_config() fails, it
   falls back to require().  Note that the reason that require() is
   passed -llibmfx as the last argument, instead of -lmfx, is the file
   name for the library produced from the Intel Media SDK starts with
   "libmfx".  Apparently, the filename for the library produced via
   https://github.com/lu-zero/mfx_dispatch starts with "mfx".
---
 configure | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 3bea057..b20a0b4 100755
--- a/configure
+++ b/configure
@@ -5819,7 +5819,8 @@ enabled libgsm&& { for gsm_hdr in "gsm.h" 
"gsm/gsm.h"; do
done || die "ERROR: libgsm not found"; }
 enabled libilbc   && require libilbc ilbc.h WebRtcIlbcfix_InitDecode 
-lilbc
 enabled libkvazaar&& require_pkg_config "kvazaar >= 0.8.1" kvazaar.h 
kvz_api_get
-enabled libmfx&& require_pkg_config libmfx "mfx/mfxvideo.h" MFXInit
+enabled libmfx&& { use_pkg_config libmfx "mfx/mfxvideo.h" MFXInit 
||
+   { require libmfx "mfx/mfxvideo.h" MFXInit -llibmfx && 
warn "using libmfx without pkg-config"; } }
 enabled libmodplug&& require_pkg_config libmodplug 
libmodplug/modplug.h ModPlug_Load
 enabled libmp3lame&& require "libmp3lame >= 3.98.3" lame/lame.h 
lame_set_VBR_quality -lmp3lame
 enabled libnut&& require libnut libnut.h nut_demuxer_init -lnut



Pinging this patch submission again.

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

[FFmpeg-devel] [PATCH] Fixed memory leaks associated with AVStream objects if FF_API_LAVF_AVCTX is defined

2017-04-20 Thread Aaron Levinson

From 4f27e910aca6dae6642b4d73cf07fa0d6c4b1618 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 20 Apr 2017 23:20:20 -0700
Subject: [PATCH] Fixed memory leaks associated with AVStream objects if
 FF_API_LAVF_AVCTX is defined

Purpose: Fixed memory leaks associated with AVStream objects if
FF_API_LAVF_AVCTX is defined.  If FF_API_LAVF_AVCTX is defined,
AVStream::codec is set to an AVCodecContext object, and this wasn't
being deallocated properly when the AVStream object was freed.  While
FF_API_LAVF_AVCTX has to do with deprecated functionality, in
practice, it will always be defined for typical builds currently,
since it is defined in libavformat\version.h if
LIBAVFORMAT_VERSION_MAJOR is less than 58, and
LIBAVFORMAT_VERSION_MAJOR is currently set to 57.

Comments:

-- libavformat/utils.c: Now using avcodec_free_context() to properly
   deallocate st->codec in free_stream() if FF_API_LAVF_AVCTX is
   defined.
---
 libavformat/utils.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/libavformat/utils.c b/libavformat/utils.c
index ba82a76..fbd8b58 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -4266,9 +4266,7 @@ static void free_stream(AVStream **pst)
 av_freep(&st->index_entries);
 #if FF_API_LAVF_AVCTX
 FF_DISABLE_DEPRECATION_WARNINGS
-av_freep(&st->codec->extradata);
-av_freep(&st->codec->subtitle_header);
-av_freep(&st->codec);
+avcodec_free_context(&st->codec);
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 av_freep(&st->priv_data);
-- 
2.10.1.windows.1

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

Re: [FFmpeg-devel] libavutil/thread.h: Fixed g++ build error when ASSERT_LEVEL is greater than 1

2017-04-20 Thread Aaron Levinson


On 4/19/2017 2:27 PM, Marton Balint wrote:


On Mon, 17 Apr 2017, James Almer wrote:


On 4/17/2017 5:39 AM, Clément Bœsch wrote:

On Sun, Apr 16, 2017 at 05:20:02PM -0700, Aaron Levinson wrote:

From 9e6a9e2b8d58f17c661a3f455e03c95587ec7b18 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Sun, 16 Apr 2017 17:13:31 -0700
Subject: [PATCH] libavutil/thread.h:  Fixed g++ build error when
 ASSERT_LEVEL is greater than 1

Purpose: libavutil/thread.h: Fixed g++ build error when ASSERT_LEVEL
is greater than 1.  This is only relevant when thread.h is included by
C++ files.  In this case, the relevant code is only defined if
HAVE_PTHREADS is defined as 1.  Use configure --assert-level=2 to do
so.

Note: Issue discovered as a result of Coverity build failure.  Cause
of build failure pinpointed by Hendrik Leppkes.

Comments:

-- libavutil/thread.h: Altered ASSERT_PTHREAD_NORET definition such
   that it uses av_make_error_string instead of av_err2str().
   av_err2str() uses a "parenthesized type followed by an initializer
   list", which is apparently not valid C++.  This issue started
   occurring because thread.h is now included by the DeckLink C++
   files.  The alteration does the equivalent of what av_err2str()
   does, but instead declares the character buffer as a local
   variable.
---
 libavutil/thread.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavutil/thread.h b/libavutil/thread.h
index 6e57447..f108e20 100644
--- a/libavutil/thread.h
+++ b/libavutil/thread.h
@@ -36,8 +36,11 @@
 #define ASSERT_PTHREAD_NORET(func, ...) do
{\
 int ret =
func(__VA_ARGS__);\
 if (ret)
{  \
+char errbuf[AV_ERROR_MAX_STRING_SIZE] =
""; \
 av_log(NULL, AV_LOG_FATAL,
AV_STRINGIFY(func)   \
-   " failed with error: %s\n",
av_err2str(AVERROR(ret)));   \
+   " failed with error:
%s\n",  \
+   av_make_error_string(errbuf,
AV_ERROR_MAX_STRING_SIZE,   \
+
AVERROR(ret))); \

abort();\

}   \
 } while (0)


I don't like limiting ourselves in the common C code of the project
because C++ is a bad and limited language. Can't you solve this by
bumping
the minimal requirement of C++ version?


We're already using C++11 when available because of atomics on
mediacodec.
Also, just tried and it seems to fail even with C++14, so it just doesn't
work with C++.

We could instead just make these strict assert wrappers work only on C
code by for example checking for defined(__cplusplus).


I'd say let's apply the patch as is, that is the simplest solution.

Regards,
Marton


I don't think most people care one way or the other--perhaps someone 
with push privileges could apply the patch?  I assume that the Coverity 
builds are continuing to fail as a result of the issue that this patch 
addresses.


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

[FFmpeg-devel] [PATCH] Added additional note to fate.texi to describe the importance of not checking out files from git with core.autocrlf set to true

2017-04-21 Thread Aaron Levinson

From 7ea6455cd52ffe561f94bf03f48c4c2cf61b33c5 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Fri, 21 Apr 2017 15:55:11 -0700
Subject: [PATCH] Added additional note to fate.texi to describe the importance
 of not checking out files from git with core.autocrlf set to true

Purpose: Added additional note to fate.texi to describe the importance
of not checking out files from git with core.autocrlf set to true.
When CRLF is used for line endings, this causes the source FATE test,
which uses check-source.sh, to fail.  The issue has to do with the use
of git grep, which doesn't appear to be able to handle CRLF line
endings.  Older versions of check-source.sh used grep directly, which
seems to do a better job of handling CRLF line endings.

Note: thanks to Hendrik Leppkes for suggestion that the issue with
FATE failing was caused by CRLF line endings.
---
 doc/fate.texi | 9 +
 1 file changed, 9 insertions(+)

diff --git a/doc/fate.texi b/doc/fate.texi
index 7a96c25..80fac0a 100644
--- a/doc/fate.texi
+++ b/doc/fate.texi
@@ -77,6 +77,15 @@ FATE_SAMPLES=fate-suite/ make fate
 @float NOTE
 Do not put a '~' character in the samples path to indicate a home
 directory. Because of shell nuances, this will cause FATE to fail.
+
+In addition, FATE will fail if files are checked out from git such
+that CRLF is used for line endings.  This will occur on Windows when
+git is installed using the default options, because that causes git's
+core.autocrlf setting to be set to 'true'.  Make sure to set
+core.autocrlf to 'input' or 'false', or, in the case that the
+repository has already been cloned, it is possible to get past this by
+executing the following command in the top-level ffmpeg directory:
+@command{find -name '*.h' -type f | xargs dos2unix}.
 @end float
 
 To use a custom wrapper to run the test, pass @option{--target-exec} to
-- 
2.7.4

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

Re: [FFmpeg-devel] [PATCH] Added additional note to fate.texi to describe the importance of not checking out files from git with core.autocrlf set to true

2017-04-21 Thread Aaron Levinson

On 4/21/2017 4:17 PM, Lou Logan wrote:
> On Fri, 21 Apr 2017 16:05:16 -0700
> Aaron Levinson  wrote:
> 
>> From 7ea6455cd52ffe561f94bf03f48c4c2cf61b33c5 Mon Sep 17 00:00:00 2001
>> From: Aaron Levinson 
>> Date: Fri, 21 Apr 2017 15:55:11 -0700
>> Subject: [PATCH] Added additional note to fate.texi to describe the 
>> importance
>>  of not checking out files from git with core.autocrlf set to true
>>
>> Purpose: Added additional note to fate.texi to describe the importance
>> of not checking out files from git with core.autocrlf set to true.
>> When CRLF is used for line endings, this causes the source FATE test,
>> which uses check-source.sh, to fail.  The issue has to do with the use
>> of git grep, which doesn't appear to be able to handle CRLF line
>> endings.  Older versions of check-source.sh used grep directly, which
>> seems to do a better job of handling CRLF line endings.
>>
>> Note: thanks to Hendrik Leppkes for suggestion that the issue with
>> FATE failing was caused by CRLF line endings.
>> ---
>>  doc/fate.texi | 9 +
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/doc/fate.texi b/doc/fate.texi
>> index 7a96c25..80fac0a 100644
>> --- a/doc/fate.texi
>> +++ b/doc/fate.texi
>> @@ -77,6 +77,15 @@ FATE_SAMPLES=fate-suite/ make fate
>>  @float NOTE
>>  Do not put a '~' character in the samples path to indicate a home
>>  directory. Because of shell nuances, this will cause FATE to fail.
>> +
>> +In addition, FATE will fail if files are checked out from git such
>> +that CRLF is used for line endings.  This will occur on Windows when
>> +git is installed using the default options, because that causes git's
>> +core.autocrlf setting to be set to 'true'.  Make sure to set
>> +core.autocrlf to 'input' or 'false', or, in the case that the
>> +repository has already been cloned, it is possible to get past this by
>> +executing the following command in the top-level ffmpeg directory:
>> +@command{find -name '*.h' -type f | xargs dos2unix}.
>>  @end float
> 
> This is a non-blocking minor comment, but it would be nice to add some
> additional markup to core.autocrlf and true/false. For a list see:
> 
> <https://www.gnu.org/software/texinfo/manual/texinfo/html_node/Indicating.html#Indicating>

How about the following?

Thanks,
Aaron Levinson



From 707b61cfcc648eb228a157ce2c9ed0f61a2ba5ca Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Fri, 21 Apr 2017 16:45:28 -0700
Subject: [PATCH] Added additional note to fate.texi to describe the importance
 of not checking out files from git with core.autocrlf set to true

Purpose: Added additional note to fate.texi to describe the importance
of not checking out files from git with core.autocrlf set to true.
When CRLF is used for line endings, this causes the source FATE test,
which uses check-source.sh, to fail.  The issue has to do with the use
of git grep, which doesn't appear to be able to handle CRLF line
endings.  Older versions of check-source.sh used grep directly, which
seems to do a better job of handling CRLF line endings.

Note: thanks to Hendrik Leppkes for suggestion that the issue with
FATE failing was caused by CRLF line endings.
---
 doc/fate.texi | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/doc/fate.texi b/doc/fate.texi
index 7a96c25..f3b8c0c8 100644
--- a/doc/fate.texi
+++ b/doc/fate.texi
@@ -77,6 +77,16 @@ FATE_SAMPLES=fate-suite/ make fate
 @float NOTE
 Do not put a '~' character in the samples path to indicate a home
 directory. Because of shell nuances, this will cause FATE to fail.
+
+In addition, FATE will fail if files are checked out from git such
+that @kbd{@key{CR}@key{LF}} is used for line endings.  This will occur
+on Windows when git is installed using the default options, because
+that causes git's @var{core.autocrlf} setting to be set to
+@option{true}.  Make sure to set @var{core.autocrlf} to @option{input}
+or @option{false}, or, in the case that the repository has already
+been cloned, it is possible to get past this by executing the
+following command in the top-level ffmpeg directory: @command{find
+-name '*.h' -type f | xargs dos2unix}.
 @end float
 
 To use a custom wrapper to run the test, pass @option{--target-exec} to
-- 
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Added additional note to fate.texi to describe the importance of not checking out files from git with core.autocrlf set to true

2017-04-21 Thread Aaron Levinson

Note that in case it wasn't clear, I confirmed that this patch works by 
running make on Linux and examining both the generated text file and the 
generated HTML file in a Web browser.


Aaron

On 4/21/2017 4:48 PM, Aaron Levinson wrote:

On 4/21/2017 4:17 PM, Lou Logan wrote:

On Fri, 21 Apr 2017 16:05:16 -0700
Aaron Levinson  wrote:


From 7ea6455cd52ffe561f94bf03f48c4c2cf61b33c5 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Fri, 21 Apr 2017 15:55:11 -0700
Subject: [PATCH] Added additional note to fate.texi to describe the importance
 of not checking out files from git with core.autocrlf set to true

Purpose: Added additional note to fate.texi to describe the importance
of not checking out files from git with core.autocrlf set to true.
When CRLF is used for line endings, this causes the source FATE test,
which uses check-source.sh, to fail.  The issue has to do with the use
of git grep, which doesn't appear to be able to handle CRLF line
endings.  Older versions of check-source.sh used grep directly, which
seems to do a better job of handling CRLF line endings.

Note: thanks to Hendrik Leppkes for suggestion that the issue with
FATE failing was caused by CRLF line endings.
---
 doc/fate.texi | 9 +
 1 file changed, 9 insertions(+)

diff --git a/doc/fate.texi b/doc/fate.texi
index 7a96c25..80fac0a 100644
--- a/doc/fate.texi
+++ b/doc/fate.texi
@@ -77,6 +77,15 @@ FATE_SAMPLES=fate-suite/ make fate
 @float NOTE
 Do not put a '~' character in the samples path to indicate a home
 directory. Because of shell nuances, this will cause FATE to fail.
+
+In addition, FATE will fail if files are checked out from git such
+that CRLF is used for line endings.  This will occur on Windows when
+git is installed using the default options, because that causes git's
+core.autocrlf setting to be set to 'true'.  Make sure to set
+core.autocrlf to 'input' or 'false', or, in the case that the
+repository has already been cloned, it is possible to get past this by
+executing the following command in the top-level ffmpeg directory:
+@command{find -name '*.h' -type f | xargs dos2unix}.
 @end float


This is a non-blocking minor comment, but it would be nice to add some
additional markup to core.autocrlf and true/false. For a list see:

<https://www.gnu.org/software/texinfo/manual/texinfo/html_node/Indicating.html#Indicating>


How about the following?

Thanks,
Aaron Levinson



From 707b61cfcc648eb228a157ce2c9ed0f61a2ba5ca Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Fri, 21 Apr 2017 16:45:28 -0700
Subject: [PATCH] Added additional note to fate.texi to describe the importance
 of not checking out files from git with core.autocrlf set to true

Purpose: Added additional note to fate.texi to describe the importance
of not checking out files from git with core.autocrlf set to true.
When CRLF is used for line endings, this causes the source FATE test,
which uses check-source.sh, to fail.  The issue has to do with the use
of git grep, which doesn't appear to be able to handle CRLF line
endings.  Older versions of check-source.sh used grep directly, which
seems to do a better job of handling CRLF line endings.

Note: thanks to Hendrik Leppkes for suggestion that the issue with
FATE failing was caused by CRLF line endings.
---
 doc/fate.texi | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/doc/fate.texi b/doc/fate.texi
index 7a96c25..f3b8c0c8 100644
--- a/doc/fate.texi
+++ b/doc/fate.texi
@@ -77,6 +77,16 @@ FATE_SAMPLES=fate-suite/ make fate
 @float NOTE
 Do not put a '~' character in the samples path to indicate a home
 directory. Because of shell nuances, this will cause FATE to fail.
+
+In addition, FATE will fail if files are checked out from git such
+that @kbd{@key{CR}@key{LF}} is used for line endings.  This will occur
+on Windows when git is installed using the default options, because
+that causes git's @var{core.autocrlf} setting to be set to
+@option{true}.  Make sure to set @var{core.autocrlf} to @option{input}
+or @option{false}, or, in the case that the repository has already
+been cloned, it is possible to get past this by executing the
+following command in the top-level ffmpeg directory: @command{find
+-name '*.h' -type f | xargs dos2unix}.
 @end float

 To use a custom wrapper to run the test, pass @option{--target-exec} to


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

Re: [FFmpeg-devel] [PATCH 1/7] avcodec: do not use AVFrame accessor


On 4/22/2017 2:27 AM, wm4 wrote:

On Sat, 22 Apr 2017 16:04:22 +0700
Muhammad Faiz  wrote:


Signed-off-by: Muhammad Faiz 
---


+1 to patches 1-7. As long as the accessors only trivially access
public fields, it's better (and less ugly) not to use accessors at all.


Then why have the accessors at all if the fields are public?  As far as 
I can tell, the benefit to using these accessors is that, if certain 
properties become internal in the future, or variable names change, or 
the structures become opaque, etc, the code will continue to work.


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

Re: [FFmpeg-devel] [PATCH] Added additional note to fate.texi to describe the importance of not checking out files from git with core.autocrlf set to true




On 4/22/2017 2:16 AM, Clément Bœsch wrote:

On Fri, Apr 21, 2017 at 04:48:27PM -0700, Aaron Levinson wrote:
[...]

diff --git a/doc/fate.texi b/doc/fate.texi
index 7a96c25..f3b8c0c8 100644
--- a/doc/fate.texi
+++ b/doc/fate.texi
@@ -77,6 +77,16 @@ FATE_SAMPLES=fate-suite/ make fate
 @float NOTE
 Do not put a '~' character in the samples path to indicate a home
 directory. Because of shell nuances, this will cause FATE to fail.
+
+In addition, FATE will fail if files are checked out from git such
+that @kbd{@key{CR}@key{LF}} is used for line endings.  This will occur
+on Windows when git is installed using the default options, because
+that causes git's @var{core.autocrlf} setting to be set to
+@option{true}.  Make sure to set @var{core.autocrlf} to @option{input}
+or @option{false}, or, in the case that the repository has already
+been cloned, it is possible to get past this by executing the
+following command in the top-level ffmpeg directory: @command{find
+-name '*.h' -type f | xargs dos2unix}.
 @end float



This is documented in doc/git-howto.texi, isn't it?


Yes it is, but just because it can be found elsewhere in the 
documentation doesn't mean there isn't benefit to having a fallback 
elsewhere in the documentation.


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

Re: [FFmpeg-devel] [PATCH] Added additional note to fate.texi to describe the importance of not checking out files from git with core.autocrlf set to true


On 4/22/2017 12:09 PM, Hendrik Leppkes wrote:

On Sat, Apr 22, 2017 at 9:07 PM, Aaron Levinson  wrote:



On 4/22/2017 2:16 AM, Clément Bœsch wrote:


On Fri, Apr 21, 2017 at 04:48:27PM -0700, Aaron Levinson wrote:
[...]


diff --git a/doc/fate.texi b/doc/fate.texi
index 7a96c25..f3b8c0c8 100644
--- a/doc/fate.texi
+++ b/doc/fate.texi
@@ -77,6 +77,16 @@ FATE_SAMPLES=fate-suite/ make fate
 @float NOTE
 Do not put a '~' character in the samples path to indicate a home
 directory. Because of shell nuances, this will cause FATE to fail.
+
+In addition, FATE will fail if files are checked out from git such
+that @kbd{@key{CR}@key{LF}} is used for line endings.  This will occur
+on Windows when git is installed using the default options, because
+that causes git's @var{core.autocrlf} setting to be set to
+@option{true}.  Make sure to set @var{core.autocrlf} to @option{input}
+or @option{false}, or, in the case that the repository has already
+been cloned, it is possible to get past this by executing the
+following command in the top-level ffmpeg directory: @command{find
+-name '*.h' -type f | xargs dos2unix}.
 @end float



This is documented in doc/git-howto.texi, isn't it?



Yes it is, but just because it can be found elsewhere in the documentation
doesn't mean there isn't benefit to having a fallback elsewhere in the
documentation.


Can't we just make it point there, instead of explaining it again?


OK, I'll do that.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/options: factorize avcodec_copy_context cleanup code


On 4/22/2017 10:26 AM, James Almer wrote:

Signed-off-by: James Almer 
---
 libavcodec/options.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/libavcodec/options.c b/libavcodec/options.c
index 7bdb0be5af..b98da9378a 100644
--- a/libavcodec/options.c
+++ b/libavcodec/options.c
@@ -188,6 +188,19 @@ void avcodec_free_context(AVCodecContext **pavctx)
 }

 #if FF_API_COPY_CONTEXT
+static void copy_context_reset(AVCodecContext *avctx)
+{
+av_opt_free(avctx);
+av_freep(&avctx->rc_override);
+av_freep(&avctx->intra_matrix);
+av_freep(&avctx->inter_matrix);
+av_freep(&avctx->extradata);
+av_freep(&avctx->subtitle_header);
+av_buffer_unref(&avctx->hw_frames_ctx);
+avctx->subtitle_header_size = 0;
+avctx->extradata_size = 0;
+}
+
 int avcodec_copy_context(AVCodecContext *dest, const AVCodecContext *src)
 {
 const AVCodec *orig_codec = dest->codec;
@@ -200,12 +213,7 @@ int avcodec_copy_context(AVCodecContext *dest, const 
AVCodecContext *src)
 return AVERROR(EINVAL);
 }

-av_opt_free(dest);
-av_freep(&dest->rc_override);
-av_freep(&dest->intra_matrix);
-av_freep(&dest->inter_matrix);
-av_freep(&dest->extradata);
-av_freep(&dest->subtitle_header);
+copy_context_reset(dest);

 memcpy(dest, src, sizeof(*dest));
 av_opt_copy(dest, src);
@@ -264,15 +272,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
 return 0;

 fail:
-av_freep(&dest->subtitle_header);
-av_freep(&dest->rc_override);
-av_freep(&dest->intra_matrix);
-av_freep(&dest->inter_matrix);
-av_freep(&dest->extradata);
-av_buffer_unref(&dest->hw_frames_ctx);
-dest->subtitle_header_size = 0;
-dest->extradata_size = 0;
-av_opt_free(dest);
+copy_context_reset(dest);
 return AVERROR(ENOMEM);
 }
 #endif



There is one real difference in the behavior--it will call 
av_buffer_unref(&dest->hw_frames_ctx) the first time around, and this 
wasn't done before.  The initialization of subtitle_header_size and 
extradata_size to 0 doesn't really matter the first time around, since 
they will be overwritten anyway as a result of using memcpy(), but the 
use of av_buffer_unref() does technically change the behavior compared 
to what it used to do.  It is likely correct to do that the first time 
through, but I wonder, why not also do 
av_buffer_unref(&avctx->hw_device_ctx) and a bunch of the other things 
that avcodec_close() already does?  The call to memcpy() will basically 
overwrite dest with the values in src, and besides the copies made of 
dest->codec and dest->priv_data earlier in the function, anything else 
that is overwritten could in theory leak.  But, I guess that would have 
been a preexisting issue with this function and not really relevant for 
this patch.  And this function is deprecated anyway.


If there is no issue with the additional call to av_buffer_unref(), as 
mentioned above, this patch looks good.


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

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/options: copy the coded_side_data in avcodec_copy_context()

2017-04-23 Thread Aaron Levinson


On 4/22/2017 10:26 AM, James Almer wrote:

Signed-off-by: James Almer 
---
 libavcodec/options.c | 28 
 1 file changed, 28 insertions(+)

diff --git a/libavcodec/options.c b/libavcodec/options.c
index b98da9378a..c721aa8d43 100644
--- a/libavcodec/options.c
+++ b/libavcodec/options.c
@@ -190,15 +190,21 @@ void avcodec_free_context(AVCodecContext **pavctx)
 #if FF_API_COPY_CONTEXT
 static void copy_context_reset(AVCodecContext *avctx)
 {
+int i;
+
 av_opt_free(avctx);
 av_freep(&avctx->rc_override);
 av_freep(&avctx->intra_matrix);
 av_freep(&avctx->inter_matrix);
 av_freep(&avctx->extradata);
 av_freep(&avctx->subtitle_header);
+for (i = 0; i < avctx->nb_coded_side_data; i++)
+av_freep(&avctx->coded_side_data[i].data);
+av_freep(&avctx->coded_side_data);
 av_buffer_unref(&avctx->hw_frames_ctx);
 avctx->subtitle_header_size = 0;
 avctx->extradata_size = 0;
+avctx->nb_coded_side_data = 0;
 }

 int avcodec_copy_context(AVCodecContext *dest, const AVCodecContext *src)
@@ -262,6 +268,28 @@ FF_ENABLE_DEPRECATION_WARNINGS
 alloc_and_copy_or_fail(subtitle_header, src->subtitle_header_size, 1);
 av_assert0(dest->subtitle_header_size == src->subtitle_header_size);
 #undef alloc_and_copy_or_fail
+if (src->nb_coded_side_data) {
+int i;
+
+dest->nb_coded_side_data = 0;
+dest->coded_side_data = av_realloc_array(NULL, src->nb_coded_side_data,
+ 
sizeof(*dest->coded_side_data));
+if (!dest->coded_side_data)
+goto fail;
+
+for (i = 0; i < src->nb_coded_side_data; i++) {
+const AVPacketSideData *sd_src = &src->coded_side_data[i];
+AVPacketSideData *sd_dst = &dest->coded_side_data[i];
+
+sd_dst->data = av_malloc(sd_src->size);
+if (!sd_dst->data)
+goto fail;
+memcpy(sd_dst->data, sd_src->data, sd_src->size);
+sd_dst->size = sd_src->size;
+sd_dst->type = sd_src->type;
+dest->nb_coded_side_data++;
+}
+}

 if (src->hw_frames_ctx) {
 dest->hw_frames_ctx = av_buffer_ref(src->hw_frames_ctx);



If src has coded_side_data and nb_coded_side_data set to non-zero 
values, as a result of the earlier call to memcpy(), it will copy those 
values into dest.  Then, if it does a goto to fail early, as a result of 
your changes to copy_context_reset(), it will call av_freep() on those 
elements in dest, even though they are owned by src.  After doing the 
memcpy() call, I think you can fix this by setting dest->coded_side_data 
to NULL and dest->nb_coded_side_data to 0.


Everything else looks good.

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

Re: [FFmpeg-devel] [RFC] Reviewing merges

2017-04-23 Thread Aaron Levinson


On 4/23/2017 7:07 PM, Michael Niedermayer wrote:

Hi all

Should changes ported from libav (what we call merges) be reviewed
before being pushed?


I've asked about this on IRC (#ffmpeg-devel).  The overall consensus 
there, at least at the time I asked it, is there is an expectation that 
such changes have already been properly reviewed for libav, so they are 
likely to be okay for ffmpeg, and while some issues have crept into 
ffmpeg as a result of these merges, the merge process has gone smoothly 
overall.


As the code bases diverge more and more, reviewing ports from libav 
would be nice, but based on the rate at which patches posted to 
ffmpeg-devel are reviewed, I think that could overload the code review 
resources on ffmpeg-devel.  Plus, the expertise may not exist to 
properly review some of the ported changes on ffmpeg-devel.  For 
example, I submitted a patch to fix a bug with QuickSync interlaced 
video to ffmpeg-devel on 4/13/2017.  The change mostly reverted some of 
the QSV code in ffmpeg back to an earlier state.  Interlaced video QSV 
encoding used to work in ffmpeg, but this was broken in October 2016 as 
a result of a merge with libav.  However, I was asked to submit the 
patch to libav instead because QSV development is more active on 
libav-devel.  I did that, and the patch finally made it into libav 
today.  And, my hope is that it will be merged into ffmpeg--that is, it 
seemed like the most readily available path to getting this code into 
ffmpeg was to contribute it to libav first.  But, if this patch has to 
pass yet another round of code reviews, code reviews that weren't 
forthcoming in the first place on ffmpeg-devel when I originally 
submitted the patch, then that could be problematic.


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

Re: [FFmpeg-devel] [rfc] coc violation tribunal

2017-04-24 Thread Aaron Levinson


On 4/24/2017 5:05 PM, Compn wrote:

as a few developers have wondered...

how is our project to judge, report and punish coc violations?

since we had a vote to approve of the COC, we will probably need
another vote to approve of the COC rules.

do you want group consensus?
how big of a group? whos in the group?
who wants to do a bunch of crapwork telling devs to be nice in the
sandbox and stop throwing sand at each other or they'll get a timeout?

do you want irc/ml admins to handle it? e.g. lou and me set moderation
flags on developers and delete mails that are not development related?

who judges the judges?

what if we scare off devs who get frustrated that the COC was not fully
fairly applied to everyone? e.g. one dev gets moderated while another
skates ? thats where the strife gains momentum.

thoughts?


I think it would be best to set this aside for now.  I think the reason 
things flared up again is nothing was ever resolved the last time around 
("Re: [FFmpeg-devel] [PATCH 1/2] opus_pvq: add resynth support and band 
encoding cost function"), and instead, it just sort of died down.  There 
was such animosity displayed on that thread from specific individuals 
towards other specific individuals.  No one benefits from that, least of 
all the people writing it or the people being targeted.  I know what it 
is like to be in an environment in which co-workers basically hate each 
other, and it isn't pleasant, and I think it is reasonable to consider 
other people on this e-mail list as sort of co-workers.


Perhaps what would be helpful would be to get these individuals together 
on a private IRC channel with someone who perhaps knows both of them to 
neutrally moderate and have them air their differences and ideally come 
to some sort of resolution and move forward.  Its one thing to fire off 
an e-mail--its another to say things to someone's "face", so to speak.


I'd also suggest, when passions are up, to step aside and then maybe 
come back to it later.


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

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/options: do a more thorough clean up in avcodec_copy_context()

2017-04-25 Thread Aaron Levinson


On 4/24/2017 3:47 PM, James Almer wrote:

Free coded_frame, coded_side_data and unref hw_device_ctx to prevent
potential leaks.

Signed-off-by: James Almer 
---
 libavcodec/options.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/libavcodec/options.c b/libavcodec/options.c
index b98da9378a..82e12179a6 100644
--- a/libavcodec/options.c
+++ b/libavcodec/options.c
@@ -190,14 +190,26 @@ void avcodec_free_context(AVCodecContext **pavctx)
 #if FF_API_COPY_CONTEXT
 static void copy_context_reset(AVCodecContext *avctx)
 {
+int i;
+
 av_opt_free(avctx);
+#if FF_API_CODED_FRAME
+FF_DISABLE_DEPRECATION_WARNINGS
+av_frame_free(&avctx->coded_frame);
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
 av_freep(&avctx->rc_override);
 av_freep(&avctx->intra_matrix);
 av_freep(&avctx->inter_matrix);
 av_freep(&avctx->extradata);
 av_freep(&avctx->subtitle_header);
 av_buffer_unref(&avctx->hw_frames_ctx);
+av_buffer_unref(&avctx->hw_device_ctx);
+for (i = 0; i < avctx->nb_coded_side_data; i++)
+av_freep(&avctx->coded_side_data[i].data);
+av_freep(&avctx->coded_side_data);
 avctx->subtitle_header_size = 0;
+avctx->nb_coded_side_data = 0;
 avctx->extradata_size = 0;
 }

@@ -238,11 +250,14 @@ FF_ENABLE_DEPRECATION_WARNINGS

 /* reallocate values that should be allocated separately */
 dest->extradata   = NULL;
+dest->coded_side_data = NULL;
 dest->intra_matrix= NULL;
 dest->inter_matrix= NULL;
 dest->rc_override = NULL;
 dest->subtitle_header = NULL;
 dest->hw_frames_ctx   = NULL;
+dest->hw_device_ctx   = NULL;
+dest->nb_coded_side_data = 0;

 #define alloc_and_copy_or_fail(obj, size, pad) \
 if (src->obj && size > 0) { \



I'm not sure if this patch is intended to be a replacement for your last 
"2/2" patch, but this version is missing the the coded_side_data 
population code that was in the first version of your "2/2" patch.


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

Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: fix CID 1405135

2017-04-25 Thread Aaron Levinson


On 4/25/2017 4:47 PM, Steven Liu wrote:

CID: 1405135
Signed-off-by: Steven Liu 
---
 libavformat/hlsenc.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 3ec0f330fb..b7aafb73da 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -394,11 +394,11 @@ static int do_encrypt(AVFormatContext *s)
 av_strlcat(hls->key_basename, ".key", len);

 if (hls->key_url) {
-strncpy(hls->key_file, hls->key_url, sizeof(hls->key_file));
-strncpy(hls->key_uri, hls->key_url, sizeof(hls->key_uri));
+av_strlcpy(hls->key_file, hls->key_url, strlen(hls->key_url));
+av_strlcpy(hls->key_uri, hls->key_url, strlen(hls->key_url));
 } else {
-strncpy(hls->key_file, hls->key_basename, sizeof(hls->key_file));
-strncpy(hls->key_uri, hls->key_basename, sizeof(hls->key_uri));
+av_strlcpy(hls->key_file, hls->key_basename, 
strlen(hls->key_basename));
+av_strlcpy(hls->key_uri, hls->key_basename, strlen(hls->key_basename));


If either hls->key_url or hls->key_basename, depending on which path the 
code takes, is longer in length than key_file or key_uri (both of size 
LINE_BUFFER_SIZE + 1), then the calls to av_strlcpy() will write past 
the end of the buffers.  As key_url corresponds to a user-passed option, 
and I think the same may be true for key_basename based on the way it is 
formed, this in theory could be a problem, although it is unlikely in 
practice since LINE_BUFFER_SIZE is 1024.  While ideally there would be a 
sanity check somewhere in the code to make sure those URLs aren't too 
long, an alternative is to copy up to max LINE_BUFFER_SIZE characters 
and make sure the string is null-terminated in that case.


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

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/options: do a more thorough clean up in avcodec_copy_context()

2017-04-30 Thread Aaron Levinson


On 4/26/2017 12:47 PM, James Almer wrote:

On 4/26/2017 2:46 AM, Aaron Levinson wrote:

On 4/24/2017 3:47 PM, James Almer wrote:

Free coded_frame, coded_side_data and unref hw_device_ctx to prevent
potential leaks.

Signed-off-by: James Almer 
---
 libavcodec/options.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/libavcodec/options.c b/libavcodec/options.c
index b98da9378a..82e12179a6 100644
--- a/libavcodec/options.c
+++ b/libavcodec/options.c
@@ -190,14 +190,26 @@ void avcodec_free_context(AVCodecContext **pavctx)
 #if FF_API_COPY_CONTEXT
 static void copy_context_reset(AVCodecContext *avctx)
 {
+int i;
+
 av_opt_free(avctx);
+#if FF_API_CODED_FRAME
+FF_DISABLE_DEPRECATION_WARNINGS
+av_frame_free(&avctx->coded_frame);
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
 av_freep(&avctx->rc_override);
 av_freep(&avctx->intra_matrix);
 av_freep(&avctx->inter_matrix);
 av_freep(&avctx->extradata);
 av_freep(&avctx->subtitle_header);
 av_buffer_unref(&avctx->hw_frames_ctx);
+av_buffer_unref(&avctx->hw_device_ctx);
+for (i = 0; i < avctx->nb_coded_side_data; i++)
+av_freep(&avctx->coded_side_data[i].data);
+av_freep(&avctx->coded_side_data);
 avctx->subtitle_header_size = 0;
+avctx->nb_coded_side_data = 0;
 avctx->extradata_size = 0;
 }

@@ -238,11 +250,14 @@ FF_ENABLE_DEPRECATION_WARNINGS

 /* reallocate values that should be allocated separately */
 dest->extradata   = NULL;
+dest->coded_side_data = NULL;
 dest->intra_matrix= NULL;
 dest->inter_matrix= NULL;
 dest->rc_override = NULL;
 dest->subtitle_header = NULL;
 dest->hw_frames_ctx   = NULL;
+dest->hw_device_ctx   = NULL;
+dest->nb_coded_side_data = 0;

 #define alloc_and_copy_or_fail(obj, size, pad) \
 if (src->obj && size > 0) { \



I'm not sure if this patch is intended to be a replacement for your last
"2/2" patch, but this version is missing the the coded_side_data
population code that was in the first version of your "2/2" patch.

Aaron Levinson


It is. I'm keeping the functionality of the function as is, and only
making sure cleanup is complete to prevent leaks.
This function is deprecated and shouldn't be used, so i'm not going to
make it copy even more stuff.


I've reviewed both patches 1 and 2, and both are good to commit.

Technically, the behavior of the code is changed by zeroing out 
coded_side_data and nb_coded_side_data after doing the memcpy() call, 
even though, by not doing a copy of coded_side_data() previously, this 
resulted in the same coded_side_data pointer being owned by two 
AVCodecContext objects (assuming that there is any coded side data in 
the source AVCodecContext object).  This was a bug, but if there is any 
code that depends on there being valid coded side data in the copied 
context, then this could result in problems.  However, I've surveyed the 
code in ffmpeg proper, and I actually can find no uses of 
coded_side_data in AVCodecContext objects other than the code in 
ffmpeg.c to copy coded side data into the stream, and it is the copy of 
the coded side data in the stream that is actually used.  This entire 
procedure of populating coded_side_data in AVCodecContext objects for it 
to be later copied into an AVStream object where it is actually used is 
a bit kludgy and could use some work.


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

Re: [FFmpeg-devel] [PATCH] This fixes ISO date formatissue when manifest created by this muxer is not playable in most players. This ensures compatibility with dash standard. Tested on many players (d

2017-04-30 Thread Aaron Levinson


On 4/26/2017 4:27 AM, mfojtak wrote:

---
 libavformat/dashenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 6232c70..fe1d6c2 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -433,7 +433,7 @@ static void format_date_now(char *buf, int size)
 struct tm *ptm, tmbuf;
 ptm = gmtime_r(&t, &tmbuf);
 if (ptm) {
-if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%S", ptm))
+if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%SZ", ptm))
 buf[0] = '\0';
 }
 }



This change appears to be correct.  I wasn't previously knowledgeable 
about the 'Z' suffix, but I looked into it and it is documented in ISO 
8601 (https://en.wikipedia.org/wiki/ISO_8601 and also 
http://www.ecma-international.org/ecma-262/5.1/#sec-15.9.1.15 ).


On a separate note, the actual format is:  -MM-DDTHH:mm:ss.sssZ . 
The ".sss" part is missing from this implementation, which represents 
milliseconds.  According to the specification, ".sss" may be absent, but 
maybe it would work with even more players if it were included. 
Technically, the specification states that an absent time-zone offset 
should be treated as 'Z', which indicates that the code was already 
compliant without the 'Z', even if it didn't work with most players. 
strftime() doesn't handle milliseconds, but perhaps it ought to populate 
milliseconds anyway as follows:


if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%S.000Z", ptm))

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

Re: [FFmpeg-devel] [PATCH]compat/strtod: Add missing const qualifiers


On 5/1/2017 1:51 AM, Carl Eugen Hoyos wrote:

Hi!

Even without the casts, the patch reduces the number of warnings shown when
compiling compat/strtod from seven to three.

Please comment, Carl Eugen


LGTM

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

Re: [FFmpeg-devel] [PATCH] This fixes ISO date formatissue when manifest created by this muxer is not playable in most players. This ensures compatibility with dash standard. Tested on many players (d


On 5/1/2017 11:06 PM, MFojtak wrote:

Currently this muxer does not work at all. I don't know if 000Z would make
it compatible with more player as I don't know any. However, adding Z makes
it compatible with most popular ones like dash.js and shaka. Having this
muxer working properly would bring more attention to it and maybe in the
future somebody tests it with more players. But for now I suggest to roll
out the change and "unblock" this muxer for some real wold use. Also, it
would be great to make this muxer codec and container agnostic as it works
with h264 and mp4 only. But again, nobody would bother if the muxer doesn't
work at all with browsers.


I think your original patch is fine, and I only offered the possibility 
that adding ".000Z" might be even better than just "Z".  I don't have 
push privileges, so I can't commit your patch, but hopefully someone 
else will do so.


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

Re: [FFmpeg-devel] [PATCH]configure: Fix libopus detection


On 4/25/2017 1:19 AM, Carl Eugen Hoyos wrote:

2017-04-13 1:08 GMT+02:00 Carl Eugen Hoyos :

2017-03-30 1:52 GMT+02:00 James Almer :

On 3/29/2017 7:47 PM, Carl Eugen Hoyos wrote:

Hi!

Attached patch fixes a compilation error here.

Please test for success, Carl Eugen


0001-configure-Fix-libopus-detection.patch


From 600b568651c60f8de609f211c814b5cd0640e584 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Thu, 30 Mar 2017 00:45:06 +0200
Subject: [PATCH] configure: Fix libopus detection.

Avoids a compilation error for old libopus.
Regression since 37941878
---
 configure |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index a84b126..76a287e 100755
--- a/configure
+++ b/configure
@@ -5797,7 +5797,7 @@ enabled libopenjpeg   && { { check_lib 
openjpeg-2.1/openjpeg.h opj_version -
{ check_lib openjpeg.h opj_version -lopenjpeg 
-DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } ||
die "ERROR: libopenjpeg not found"; }
 enabled libopenmpt&& require_pkg_config "libopenmpt >= 0.2.6557" 
libopenmpt/libopenmpt.h openmpt_module_create
-enabled libopus   && require_pkg_config opus opus_multistream.h 
opus_multistream_decoder_create
+enabled libopus   && require_pkg_config opus opus_multistream.h 
opus_multistream_surround_encoder_create


Should be ok,



but strictly speaking, this function is needed by the
encoder and not the decoder. Something like

enabled libopus   && {
enabled libopus_decoder && {
require_pkg_config opus opus_multistream.h 
opus_multistream_decoder_create
}
enabled libopus_encoder && {
use_pkg_config "opus >= 1.1" opus_multistream.h 
opus_multistream_surround_encoder_create ||
disable libopus_encoder;
}
}


Please commit this if you prefer it.


Ping.


Perhaps you could submit a new patch that modifies configure in the 
fashion suggested by James Almer.  Technically, it is possible to enable 
the libopus decoder independently of the libopus encoder, and if that is 
done, it won't build libopusenc.c.  But, with your original patch, it 
won't even permit configure to succeed even though such a build would be 
possible.  It is true that the approach currently taken in configure 
doesn't distinguish between encoding and decoding, but the patch as 
originally proposed just trades decoding API requirements for encoding 
API requirements.


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

Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: hold old key info when append list


On 4/27/2017 7:21 PM, Steven Liu wrote:

2017-04-26 7:30 GMT+08:00 Steven Liu :


fix ticket id: #6353

Signed-off-by: Steven Liu 
---
 libavformat/hlsenc.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 27c8e3355d..3ec0f330fb 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -810,6 +810,7 @@ static int parse_playlist(AVFormatContext *s, const
char *url)
 int64_t new_start_pos;
 char line[1024];
 const char *ptr;
+const char *end;

 if ((ret = ffio_open_whitelist(&in, url, AVIO_FLAG_READ,
&s->interrupt_callback, NULL,
@@ -842,6 +843,29 @@ static int parse_playlist(AVFormatContext *s, const
char *url)
 } else if (av_strstart(line, "#EXTINF:", &ptr)) {
 is_segment = 1;
 hls->duration = atof(ptr);
+} else if (av_stristart(line, "#EXT-X-KEY:", &ptr)) {
+ptr = av_stristr(line, "URI=\"");
+if (ptr) {
+ptr += strlen("URI=\"");
+end = av_stristr(ptr, ",");
+if (end) {
+av_strlcpy(hls->key_uri, ptr, end - ptr);
+} else {
+av_strlcpy(hls->key_uri, ptr, sizeof(hls->key_uri));


I know that this patch has already been applied (although it didn't get 
any reviews on the ML), but I think that there are some issues with it. 
First, it seems that both av_strlcpy() cases above will copy the 
terminating '\"' character into hls->key_uri.  Perhaps that won't cause 
an issue in practice, but it is likely not the intended result.  Second, 
with both av_strlcpy() calls that use a length of end - ptr, this could 
in theory result in a buffer overrun depending on how long the URI is, 
since the destination buffers have a fixed size.  This issue is 
prevented in the second call to av_strlcpy(), since it uses 
sizeof(hls->key_uri), but it is a potential issue with the first calls 
(note that this comment applies to the IV=0x code below).  If you want 
to use av_strlcpy(), either make sure that end - ptr is less than or 
equal to sizeof(hls->key_uri) or do something like *end = 0 first and 
then use av_strlcpy(hls->key_uri, ptr, sizeof(hls->key_uri)).


In addition, based on the EXT-X-KEY example at 
https://developer.apple.com/library/content/technotes/tn2288/_index.html 
, it would appear that an EXT-X-KEY declaration may span multiple lines. 
 Your solution will not work properly in this case.


Aaron Levinson


+}
+}
+
+ptr = av_stristr(line, "IV=0x");
+if (ptr) {
+ptr += strlen("IV=0x");
+end = av_stristr(ptr, ",");
+if (end) {
+av_strlcpy(hls->iv_string, ptr, end - ptr);
+} else {
+av_strlcpy(hls->iv_string, ptr,
sizeof(hls->iv_string));
+}
+}
+
 } else if (av_strstart(line, "#", NULL)) {
 continue;
 } else if (line[0]) {
--
2.11.0 (Apple Git-81)



Applied!


Thanks

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

Re: [FFmpeg-devel] [PATCH] avformat/concatdec: port to the new bitstream filter API

I don't seem to have the original e-mail in my inbox anymore, so I'll 
respond to wm4's response.  Overall, the new code is a lot cleaner and 
easier to understand than the existing code.


See below for more.

Aaron Levinson

On 5/2/2017 5:04 PM, wm4 wrote:

On Fri, 28 Apr 2017 21:27:56 -0300
James Almer  wrote:


Signed-off-by: James Almer 
---
 libavformat/concatdec.c | 94 -
 1 file changed, 30 insertions(+), 64 deletions(-)

diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
index dd52e4d366..fa9443ff78 100644
--- a/libavformat/concatdec.c
+++ b/libavformat/concatdec.c
@@ -34,8 +34,7 @@ typedef enum ConcatMatchMode {
 } ConcatMatchMode;

 typedef struct ConcatStream {
-AVBitStreamFilterContext *bsf;
-AVCodecContext *avctx;
+AVBSFContext *bsf;
 int out_stream_index;
 } ConcatStream;

@@ -196,7 +195,8 @@ static int detect_stream_specific(AVFormatContext *avf, int 
idx)
 ConcatContext *cat = avf->priv_data;
 AVStream *st = cat->avf->streams[idx];
 ConcatStream *cs = &cat->cur_file->streams[idx];
-AVBitStreamFilterContext *bsf;
+const AVBitStreamFilter *filter;
+AVBSFContext *bsf;
 int ret;

 if (cat->auto_convert && st->codecpar->codec_id == AV_CODEC_ID_H264) {
@@ -206,29 +206,28 @@ static int detect_stream_specific(AVFormatContext *avf, 
int idx)
 return 0;
 av_log(cat->avf, AV_LOG_INFO,
"Auto-inserting h264_mp4toannexb bitstream filter\n");
-if (!(bsf = av_bitstream_filter_init("h264_mp4toannexb"))) {
+filter = av_bsf_get_by_name("h264_mp4toannexb");
+if (!filter) {
 av_log(avf, AV_LOG_ERROR, "h264_mp4toannexb bitstream filter "
"required for H.264 streams\n");
 return AVERROR_BSF_NOT_FOUND;
 }
+ret = av_bsf_alloc(filter, &bsf);
+if (ret < 0)
+return ret;
 cs->bsf = bsf;

-cs->avctx = avcodec_alloc_context3(NULL);
-if (!cs->avctx)
-return AVERROR(ENOMEM);
-
-/* This really should be part of the bsf work.
-   Note: input bitstream filtering will not work with bsf that
-   create extradata from the first packet. */
-av_freep(&st->codecpar->extradata);
-st->codecpar->extradata_size = 0;
+ret = avcodec_parameters_copy(bsf->par_in, st->codecpar);
+if (ret < 0)
+   return ret;

-ret = avcodec_parameters_to_context(cs->avctx, st->codecpar);
-if (ret < 0) {
-avcodec_free_context(&cs->avctx);
+ret = av_bsf_init(bsf);
+if (ret < 0)
 return ret;
-}

+ret = avcodec_parameters_copy(st->codecpar, bsf->par_out);
+if (ret < 0)
+return ret;
 }
 return 0;
 }



@@ -291,8 +290,11 @@ static int match_streams(AVFormatContext *avf)
 memset(map + cat->cur_file->nb_streams, 0,
(cat->avf->nb_streams - cat->cur_file->nb_streams) * sizeof(*map));

-for (i = cat->cur_file->nb_streams; i < cat->avf->nb_streams; i++)
+for (i = cat->cur_file->nb_streams; i < cat->avf->nb_streams; i++) {
 map[i].out_stream_index = -1;
+if ((ret = detect_stream_specific(avf, i)) < 0)
+return ret;
+}
 switch (cat->stream_match_mode) {
 case MATCH_ONE_TO_ONE:
 ret = match_streams_one_to_one(avf);
@@ -305,9 +307,6 @@ static int match_streams(AVFormatContext *avf)
 }
 if (ret < 0)
 return ret;
-for (i = cat->cur_file->nb_streams; i < cat->avf->nb_streams; i++)
-if ((ret = detect_stream_specific(avf, i)) < 0)
-return ret;
 cat->cur_file->nb_streams = cat->avf->nb_streams;
 return 0;
 }


This just moves already existing code around.  While it is unclear to me 
why this is being done (a comment and/or log message would help), I 
would suspect that this particular change is unrelated to the purpose of 
this patch:  "port to the new bitstream filter API".



@@ -370,10 +369,8 @@ static int concat_read_close(AVFormatContext *avf)
 for (i = 0; i < cat->nb_files; i++) {
 av_freep(&cat->files[i].url);
 for (j = 0; j < cat->files[i].nb_streams; j++) {
-if (cat->files[i].streams[j].avctx)
-avcodec_free_context(&cat->files[i].streams[j].avctx);
 if (cat->files[i].streams[j].bsf)
-av_bitstream_filter_close(cat->files[i].streams[j].bsf);
+av_bsf_free(&cat->files[i].streams[j].bsf);
 }
 av_freep(&cat->files[i].streams);
 av_dict_free(&cat->files[i].metadata);
@@ -524,56 +521,25

Re: [FFmpeg-devel] [PATCH] Added additional note to fate.texi to describe the importance of not checking out files from git with core.autocrlf set to true


On 4/22/2017 12:22 PM, Aaron Levinson wrote:

On 4/22/2017 12:09 PM, Hendrik Leppkes wrote:

On Sat, Apr 22, 2017 at 9:07 PM, Aaron Levinson 
wrote:



On 4/22/2017 2:16 AM, Clément Bœsch wrote:


On Fri, Apr 21, 2017 at 04:48:27PM -0700, Aaron Levinson wrote:

[...]


This is documented in doc/git-howto.texi, isn't it?



Yes it is, but just because it can be found elsewhere in the
documentation
doesn't mean there isn't benefit to having a fallback elsewhere in the
documentation.


Can't we just make it point there, instead of explaining it again?


OK, I'll do that.


I'm abandoning this patch and will submit a new one shortly with a new 
title.


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

[FFmpeg-devel] [PATCH] doc: Enhanced fate.texi to reference Windows line endings section from git-howto.texi

From 345a98d31c6c758f0f82edab1b502cd8aeb3354b Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Wed, 3 May 2017 12:38:39 -0700
Subject: [PATCH] doc: Enhanced fate.texi to reference Windows line endings
 section from git-howto.texi

Purpose: Enhanced fate.texi to indicate that FATE will fail if files
are checked out with Windows line endings (CRLF) and added reference
to Windows line endings anchor in git-howto.texi for how to address.
Also added Windows line endings anchor in git-howto.texi.

The exact problem caused by CRLF endings occurs with check-source.sh,
which uses git grep.  git grep doesn't appear to be able to handle
CRLF line endings.

Note: thanks to Hendrik Leppkes for suggestion that the issue with
FATE failing was caused by CRLF line endings.

Signed-off-by: Aaron Levinson 
---
 doc/fate.texi  | 5 +
 doc/git-howto.texi | 1 +
 2 files changed, 6 insertions(+)

diff --git a/doc/fate.texi b/doc/fate.texi
index 7a96c25..c498336 100644
--- a/doc/fate.texi
+++ b/doc/fate.texi
@@ -77,6 +77,11 @@ FATE_SAMPLES=fate-suite/ make fate
 @float NOTE
 Do not put a '~' character in the samples path to indicate a home
 directory. Because of shell nuances, this will cause FATE to fail.
+
+In addition, FATE will fail if files are checked out from git such
+that @kbd{@key{CR}@key{LF}} is used for line endings.  Please refer to
+@ref{Windows line endings,,Windows line endings,git-howto} in the
+FFmpeg documentation for more information.
 @end float
 
 To use a custom wrapper to run the test, pass @option{--target-exec} to
diff --git a/doc/git-howto.texi b/doc/git-howto.texi
index 2b4fb80..f2b6182 100644
--- a/doc/git-howto.texi
+++ b/doc/git-howto.texi
@@ -80,6 +80,7 @@ create patches after making a read-only ffmpeg-web clone:
 git clone git://ffmpeg.org/ffmpeg-web 
 @end example
 
+@anchor{Windows line endings}
 Make sure that you do not have Windows line endings in your checkouts,
 otherwise you may experience spurious compilation failures. One way to
 achieve this is to run
-- 
2.7.4

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

Re: [FFmpeg-devel] [PATCH] Fixed memory leaks associated with AVStream objects if FF_API_LAVF_AVCTX is defined

On 4/21/2017 2:03 PM, James Almer wrote:

On 4/21/2017 12:09 PM, Michael Niedermayer wrote:

On Thu, Apr 20, 2017 at 11:30:13PM -0700, Aaron Levinson wrote:

 From 4f27e910aca6dae6642b4d73cf07fa0d6c4b1618 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 20 Apr 2017 23:20:20 -0700
Subject: [PATCH] Fixed memory leaks associated with AVStream objects if
  FF_API_LAVF_AVCTX is defined

Purpose: Fixed memory leaks associated with AVStream objects if
FF_API_LAVF_AVCTX is defined.  If FF_API_LAVF_AVCTX is defined,
AVStream::codec is set to an AVCodecContext object, and this wasn't
being deallocated properly when the AVStream object was freed.  While
FF_API_LAVF_AVCTX has to do with deprecated functionality, in
practice, it will always be defined for typical builds currently,
since it is defined in libavformat\version.h if
LIBAVFORMAT_VERSION_MAJOR is less than 58, and
LIBAVFORMAT_VERSION_MAJOR is currently set to 57.

Comments:

-- libavformat/utils.c: Now using avcodec_free_context() to properly
deallocate st->codec in free_stream() if FF_API_LAVF_AVCTX is
defined.
---
  libavformat/utils.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

please use "make fate" to test your changes

This one causes many all? tests to segfault

The issue is coded_side_data in AVStream->codec.

ffmpeg.c calls avcodec_copy_context() to copy the encoder AVCodecContext
to the output's AVStream->codec because the latter is still needed by
some internal code in lavf/utils.c and such.
avcodec_copy_context() copies the coded_side_data pointer as part of its
memcpy call but not the buffers, and by the time avcodec_free_context()
is called for AVStream->codec the buffers said pointer points to was
already freed by the avcodec_free_context() call for the encoder
AVCodecContext.

The proper solution: Remove the avcodec_copy_context() call in ffmpeg.c
and make libavformat stop needing AVStream->codec internally. It should
use AVStream->internal->avctx instead. Even if this is not done now, it
will anyway when the deprecation period ends and it's time to remove
AVStream->codec.
The easy but ugly solution until the above is done: Copy the
coded_side_data buffers in avcodec_copy_context().

One could argue that by definition avcodec_copy_context() should copy
said side data, but the function is clearly marked as "do not use, its
behavior is ill defined" and the user is asked instead to copy using an
intermediary AVCodecParameters context instead.

Now that James Almer's patches pertaining to avcodec_copy_context() have 
been applied, perhaps the patch I submitted a couple of weeks ago can be 
considered anew.  I've confirmed that make fate SAMPLES=fate-suite/ 
(64-bit, Linux) completes successfully with the patch that I submitted.

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

Re: [FFmpeg-devel] [PATCH v2 0/9] Removing HEVCContext depencencies


James,

Can you document which patches depend on other patches in this patch set 
to make it easier to review this patch set in chunks?


Thanks,
Aaron Levinson

On 5/2/2017 2:55 PM, James Almer wrote:

Some changes and fixes suggested on IRC. Some patches are v3 as they have
been sent before outside the previous set.

James Almer (9):
  hevcdec: remove HEVCContext usage from hevc_sei
  hevcdec: move SEI message parsing into a separate header
  hevcdec: remove HEVCContext usage from ff_hevc_compute_poc()
  hevcdec: move SliceHeader struct definition to hevc_ps
  hevc_parser: use ff_h2645_packet_split() to parse NAL units
  hevc_parser: remove HEVCContext usage
  hevc_parser: move slice header parsing to its own function
  hevc_parse: decode SEI message NALUs in extradata
  doc/libav_merge: remove line about ADVANCED_PARSER

 doc/libav-merge.txt|   1 -
 libavcodec/Makefile|   4 +-
 libavcodec/hevc_parse.c|  21 ++-
 libavcodec/hevc_parse.h|   7 +-
 libavcodec/hevc_parser.c   | 415 -
 libavcodec/hevc_ps.c   |  23 +++
 libavcodec/hevc_ps.h   |  88 ++
 libavcodec/hevc_refs.c |  27 +--
 libavcodec/hevc_sei.c  | 193 +
 libavcodec/hevc_sei.h  | 125 ++
 libavcodec/hevcdec.c   |  94 +-
 libavcodec/hevcdec.h   | 137 +--
 libavcodec/mediacodecdec.c |   4 +-
 13 files changed, 532 insertions(+), 607 deletions(-)
 create mode 100644 libavcodec/hevc_sei.h


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

Re: [FFmpeg-devel] [RFC]lavu/opt: Use && instead of * in boolean expression

On 5/4/2017 4:32 PM, Carl Eugen Hoyos wrote:
> Hi!
> 
> It may be better to disable the warning.
> 
> Carl Eugen
>
> -num = den ? num * intnum / den : (num * intnum ? INFINITY : NAN);
> +num = den ? num * intnum / den : (num && intnum ? INFINITY : NAN);

In order to preserve the original logic, why not do the following:

+num = den ? num * intnum / den : (((num * intnum) != 0) ? INFINITY : 
NAN);

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

Re: [FFmpeg-devel] [RFC]lavu/timecode: Increase AV_TIMECODE_STR_SIZE.


On 5/4/2017 4:27 PM, Carl Eugen Hoyos wrote:

Hi!

Attached patch is one possibility to fix the following warning with gcc 7:
libavutil/timecode.c: In function ‘av_timecode_make_string’:
libavutil/timecode.c:103:60: warning: ‘%02d’ directive output may be truncated
writing between 2 and 10 bytes into a region of size between 0 and 7
[-Wformat-truncation=]
 snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
^~~~
libavutil/timecode.c:103:41: note: directive argument in the range [0,
2147483647]
 snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
 ^~~~
libavutil/timecode.c:103:5: note: ‘snprintf’ output between 12 and 32 bytes
into a destination of size 16
 snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
 ^
  neg ? "-" : "",
  ~~~
  hh, mm, ss, drop ? ';' : ':', ff);
  ~

Several similar warnings are printed, an alternative is to disable the
warning.

The warning seemed wrong on first sight but may actually be correct, we
accept invalid fps for timecode.


Regarding the change in your patch:

-#define AV_TIMECODE_STR_SIZE 16
+#define AV_TIMECODE_STR_SIZE 23

It seems like 23 characters wouldn't be sufficient based on the warning: 
 "output between 12 and 32 bytes into a destination of size 16".  I 
would guess that you would need at least 32 characters and perhaps one 
more (?) for the terminating null character to avoid that warning.  Even 
if in practice, it will never use 32 characters, its not a big deal to 
have a little waste here.  The buffer size should match the format 
specification.  An alternative would be to use "%02hd" or "%02hhd", but 
doing so would require casts or using different types for hh, mm, etc.


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

Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: hold old key info when append list


On 5/4/2017 9:15 PM, Steven Liu wrote:

2017-05-03 9:49 GMT+08:00 Aaron Levinson :


On 4/27/2017 7:21 PM, Steven Liu wrote:


2017-04-26 7:30 GMT+08:00 Steven Liu :

fix ticket id: #6353


Signed-off-by: Steven Liu 
---
 libavformat/hlsenc.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 27c8e3355d..3ec0f330fb 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -810,6 +810,7 @@ static int parse_playlist(AVFormatContext *s, const
char *url)
 int64_t new_start_pos;
 char line[1024];
 const char *ptr;
+const char *end;

 if ((ret = ffio_open_whitelist(&in, url, AVIO_FLAG_READ,
&s->interrupt_callback, NULL,
@@ -842,6 +843,29 @@ static int parse_playlist(AVFormatContext *s, const
char *url)
 } else if (av_strstart(line, "#EXTINF:", &ptr)) {
 is_segment = 1;
 hls->duration = atof(ptr);
+} else if (av_stristart(line, "#EXT-X-KEY:", &ptr)) {
+ptr = av_stristr(line, "URI=\"");
+if (ptr) {
+ptr += strlen("URI=\"");
+end = av_stristr(ptr, ",");
+if (end) {
+av_strlcpy(hls->key_uri, ptr, end - ptr);
+} else {
+av_strlcpy(hls->key_uri, ptr, sizeof(hls->key_uri));




I know that this patch has already been applied (although it didn't get
any reviews on the ML), but I think that there are some issues with it.
First, it seems that both av_strlcpy() cases above will copy the
terminating '\"' character into hls->key_uri.  Perhaps that won't cause an
issue in practice, but it is likely not the intended result.  Second, with
both av_strlcpy() calls that use a length of end - ptr, this could in
theory result in a buffer overrun depending on how long the URI is, since
the destination buffers have a fixed size.  This issue is prevented in the
second call to av_strlcpy(), since it uses sizeof(hls->key_uri), but it is
a potential issue with the first calls (note that this comment applies to
the IV=0x code below).  If you want to use av_strlcpy(), either make sure
that end - ptr is less than or equal to sizeof(hls->key_uri) or do
something like *end = 0 first and then use av_strlcpy(hls->key_uri, ptr,
sizeof(hls->key_uri)).

In addition, based on the EXT-X-KEY example at
https://developer.apple.com/library/content/technotes/tn2288/_index.html
, it would appear that an EXT-X-KEY declaration may span multiple lines.
Your solution will not work properly in this case.


Hi Aaron,
 I think the libavformat/hls.c maybe have the same problem, because
both of hlsenc and hls use  read_chomp_line to read one line,
 that need check the '\' tail a line, and read the next line into a
buffer, that maybe better, is that right?


Yes, I was thinking the same thing, that read_chomp_line() needs to be 
enhanced to deal with declarations that span multiple lines.  That 
probably belongs in a separate patch though, even if it is only relevant 
for EXT-X-KEY.


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

Re: [FFmpeg-devel] [PATCH] This fixes ISO date formatissue when manifest created by this muxer is not playable in most players. This ensures compatibility with dash standard. Tested on many players (d


On 5/2/2017 2:29 PM, wm4 wrote:

On Tue, 2 May 2017 14:17:33 -0700
Aaron Levinson  wrote:


On 5/1/2017 11:06 PM, MFojtak wrote:

Currently this muxer does not work at all. I don't know if 000Z would make
it compatible with more player as I don't know any. However, adding Z makes
it compatible with most popular ones like dash.js and shaka. Having this
muxer working properly would bring more attention to it and maybe in the
future somebody tests it with more players. But for now I suggest to roll
out the change and "unblock" this muxer for some real wold use. Also, it
would be great to make this muxer codec and container agnostic as it works
with h264 and mp4 only. But again, nobody would bother if the muxer doesn't
work at all with browsers.


I think your original patch is fine, and I only offered the possibility
that adding ".000Z" might be even better than just "Z".  I don't have
push privileges, so I can't commit your patch, but hopefully someone
else will do so.


Before someone pushes it, please fix the commit message.


MFojtak:  you can make it more likely that this patch will be pushed 
sooner by altering the commit message (shortening it--plenty of examples 
on the e-mail list) and then putting any additional information later. 
Something like:


"
avformat/dashenc:  Adjusts ISO date formatting for improved 
compatibility with most players


Adjusts ISO date formatting
"

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

Re: [FFmpeg-devel] [PATCH] Fixed bug encountered when decoding interlaced video

On 4/12/2017 6:08 PM, Aaron Levinson wrote:
> On 3/26/2017 10:34 AM, Aaron Levinson wrote:
>> On 3/26/2017 4:41 AM, Matthias Hunstock wrote:
>>> Am 26.03.2017 um 11:50 schrieb Aaron Levinson:
>>>> When using the following command to play back either file:
>>>>  ffmpeg -i  -f decklink -pix_fmt uyvy422 "DeckLink SDI
>>>>  4K", I noticed that with the mpegts file with the AAC audio stream,
>>>>  it would correctly select an interlaced video mode for the video
>>>>  output stream, but with the mpegts file with the Opus audio stream,
>>>>  it would use a progressive video mode (1080p29.97) for the video
>>>>  output stream.
>>>
>>> Which FFmpeg version did you test this with?
>>>
>>> There was a change related to this just short time ago.
>>>
>>> Does it happen with current git HEAD?
>>>
>>> Matthias
>>
>> This issue occurs with the current git HEAD.  I'm aware of the
>> Blackmagic improvement that was added in February to add support for
>> interlaced video modes on output, and actually that's one of the reasons
>> why I'm using the latest git sources, as opposed to, say, 3.2.4.  This
>> particular issue has nothing to do with Blackmagic, and I only used
>> Blackmagic in the example that reproduces the bug because it is
>> something that can be reproduced on both Windows and Linux (and
>> presumably also on OS/X).  The issue also occurs if I do something like
>> -f rawvideo out.avi on Windows, and I'm sure that there are plenty of
>> other examples.
>>
>> Aaron Levinson
> 
> Has anyone had a chance to review this patch yet, which I submitted on March 
> 26th?  To demonstrate the impact of this patch, here's some output of before 
> and after for an .h264 file with interlaced 1080i59.94 video content:
> 
> Command-line:  ffmpeg -i test8_1080i.h264 -c:v mpeg2video test8_1080i_mp2.ts
> 
> Before patch:
> 
> --
> 
> Input #0, h264, from 'test8_1080i.h264':
>   Duration: N/A, bitrate: N/A
> Stream #0:0: Video: h264 (High), yuv420p(top first), 1920x1080 [SAR 1:1 
> DAR 16:9], 29.97 fps, 29.97 tbr, 1200k tbn, 59.94 tbc
> Stream mapping:
>   Stream #0:0 -> #0:0 (h264 (native) -> mpeg2video (native))
> Press [q] to stop, [?] for help
> Output #0, mpegts, to 'test8_1080i_mp2_2.ts':
>   Metadata:
> encoder : Lavf57.72.100
> Stream #0:0: Video: mpeg2video (Main), yuv420p, 1920x1080 [SAR 1:1 DAR 
> 16:9], q=2-31, 200 kb/s, 29.97 fps, 90k tbn, 29.97 tbc
> Metadata:
>   encoder : Lavc57.92.100 mpeg2video
> Side data:
>   cpb: bitrate max/min/avg: 0/0/20 buffer size: 0 vbv_delay: -1
> 
> --
> 
> After patch:
> 
> --
> 
> Input #0, h264, from 'test8_1080i.h264':
>   Duration: N/A, bitrate: N/A
> Stream #0:0: Video: h264 (High), yuv420p(top first), 1920x1080 [SAR 1:1 
> DAR 16:9], 29.97 fps, 29.97 tbr, 1200k tbn, 59.94 tbc
> Stream mapping:
>   Stream #0:0 -> #0:0 (h264 (native) -> mpeg2video (native))
> Press [q] to stop, [?] for help
> Output #0, mpegts, to 'test8_1080i_mp2_2.ts':
>   Metadata:
> encoder : Lavf57.72.100
> Stream #0:0: Video: mpeg2video (Main), yuv420p(top coded first 
> (swapped)), 1920x1080 [SAR 1:1 DAR 16:9], q=2-31, 200 kb/s, 29.97 fps, 90k 
> tbn, 29.97 tbc
> Metadata:
>   encoder : Lavc57.92.100 mpeg2video
> Side data:
>   cpb: bitrate max/min/avg: 0/0/20 buffer size: 0 vbv_delay: -1
> 
> --
> 
> As can be seen, before the patch, after decoding the .h264 file and then 
> re-encoding it as mpeg2video in an mpegts container, the interlaced aspect of 
> the video has been lost in the output, and it is now effectively 1080p29.97, 
> although the video hasn't actually been converted to progressive.  ffmpeg 
> simply thinks that the video is progressive when it is not.  With the patch, 
> the interlaced aspect is not lost and propagates to the output.  So, this 
> conclusively demonstrates that the issue has nothing to do with Blackmagic 
> and is a more general issue with interlaced video and decoding.
> 
> I can make the input file available if that would be helpful.
> 
> Anyway, it would be great if this bug fix could make it into ffmpeg.
> 
> Thanks,
> Aaron Levinson

I've provided a new version of the patch.  When I created the first version of 
the patch on March 26th, this was the first pat

Re: [FFmpeg-devel] [PATCH] Enhanced configure and Makefile to copy .pdb files to bindir for MSVC builds for make install


On 4/14/2017 6:27 PM, Aaron Levinson wrote:

From 1059473c449c3079f03461bb42c2d3cc21d1b2c1 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Fri, 14 Apr 2017 18:14:21 -0700
Subject: [PATCH] Enhanced configure and Makefile to copy .pdb files to
 bindir for MSVC builds for make install

Purpose: Enhanced configure and Makefile to copy .pdb files to bindir
for MSVC builds for make install.  Files are also uninstalled
appropriately when make uninstall is exercised.  Placing the PDB files
in the same directory as other binaries can make it easier to debug,
especially if the files are copied to another system.

Note: General idea for how to properly handle the copying of PDB files
associated with programs suggested by Hendrik Leppkes.

Comments:

-- configure:
a) Leveraged already existing SLIB_INSTALL_EXTRA_SHLIB facility (which
   is already pretty specific to Windows) to add .pdb files, in
   addition to .lib files, for shared libraries to bindir during make
   install.
b) Added PROG_INSTALL_EXTRA_BIN variable for MSVC builds and also
   added it to the section that causes it to be added to config.mak.
   This is used in Makefile to copy any additional files associated
   with programs.  For MSVC, it is used to copy the pdb files
   associated with any executables that are built.  Note that such
   executables are build with _g in the file name and are later copied
   to a filename without _g in the file name.  As such, the PDB files
   have _g in the file name.

-- Makefile: Enhanced install-progs and uninstall-progs targets to
   handle PROG_INSTALL_EXTRA_BIN if defined.  It uses a similar
   procedure as already in place for SLIB_INSTALL_EXTRA_SHLIB in
   library.mak.
---
 Makefile  | 2 ++
 configure | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index d5b51de..45c42c6 100644
--- a/Makefile
+++ b/Makefile
@@ -165,6 +165,7 @@ install-progs-$(CONFIG_SHARED): install-libs
 install-progs: install-progs-yes $(AVPROGS)
$(Q)mkdir -p "$(BINDIR)"
$(INSTALL) -c -m 755 $(INSTPROGS) "$(BINDIR)"
+   $(if $(PROG_INSTALL_EXTRA_BIN), $(INSTALL) -m 644 $(PROG_INSTALL_EXTRA_BIN) 
"$(BINDIR)")

 install-data: $(DATA_FILES) $(EXAMPLES_FILES)
$(Q)mkdir -p "$(DATADIR)/examples"
@@ -175,6 +176,7 @@ uninstall: uninstall-libs uninstall-headers uninstall-progs 
uninstall-data

 uninstall-progs:
$(RM) $(addprefix "$(BINDIR)/", $(ALLAVPROGS))
+   $(if $(PROG_INSTALL_EXTRA_BIN), $(RM) $(addprefix "$(BINDIR)/", 
$(PROG_INSTALL_EXTRA_BIN)))

 uninstall-data:
$(RM) -r "$(DATADIR)"
diff --git a/configure b/configure
index 18d79ab..88206e3 100755
--- a/configure
+++ b/configure
@@ -4947,9 +4947,10 @@ case $target_os in
 SLIB_CREATE_DEF_CMD='$(SRC_PATH)/compat/windows/makedef 
$(SUBDIR)lib$(NAME).ver $(OBJS) > $$(@:$(SLIBSUF)=.def)'
 SLIB_INSTALL_NAME='$(SLIBNAME_WITH_MAJOR)'
 SLIB_INSTALL_LINKS=
-SLIB_INSTALL_EXTRA_SHLIB='$(SLIBNAME:$(SLIBSUF)=.lib)'
+SLIB_INSTALL_EXTRA_SHLIB='$(SLIBNAME:$(SLIBSUF)=.lib) 
$(SLIBNAME_WITH_MAJOR:$(SLIBSUF)=.pdb)'
 SLIB_INSTALL_EXTRA_LIB='$(SLIBNAME_WITH_MAJOR:$(SLIBSUF)=.def)'
 SHFLAGS='-dll -def:$$(@:$(SLIBSUF)=.def) 
-implib:$(SUBDIR)$(SLIBNAME:$(SLIBSUF)=.lib)'
+PROG_INSTALL_EXTRA_BIN='$(AVPROGS-yes:%=%$(PROGSSUF)_g.pdb)'
 objformat="win32"
 ranlib=:
 enable dos_paths
@@ -6796,6 +6797,7 @@ SLIB_INSTALL_NAME=${SLIB_INSTALL_NAME}
 SLIB_INSTALL_LINKS=${SLIB_INSTALL_LINKS}
 SLIB_INSTALL_EXTRA_LIB=${SLIB_INSTALL_EXTRA_LIB}
 SLIB_INSTALL_EXTRA_SHLIB=${SLIB_INSTALL_EXTRA_SHLIB}
+PROG_INSTALL_EXTRA_BIN=${PROG_INSTALL_EXTRA_BIN}
 VERSION_SCRIPT_POSTPROCESS_CMD=${VERSION_SCRIPT_POSTPROCESS_CMD}
 SAMPLES:=${samples:-\$(FATE_SAMPLES)}
 NOREDZONE_FLAGS=$noredzone_flags



Ping.

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

Re: [FFmpeg-devel] Added require fallback for libmfx in the case that pkg-config cannot find libmfx

On 4/19/2017 10:43 AM, Aaron Levinson wrote:
> On 4/14/2017 6:51 PM, Aaron Levinson wrote:
>> From e0c73c054add0137901d0bf7a7893e42e7e566c8 Mon Sep 17 00:00:00 2001
>> From: Aaron Levinson 
>> Date: Fri, 14 Apr 2017 18:38:37 -0700
>> Subject: [PATCH] Added require fallback for libmfx in the case that
>>  pkg-config cannot find libmfx
>>
>> Purpose: Added require fallback for libmfx in the case that pkg-config
>> cannot find libmfx.  On Linux, most people likely get libmfx via
>> https://github.com/lu-zero/mfx_dispatch , but on Windows, the most
>> well-known way to get libmfx is via the Intel Media SDK, which
>> provides a static build of libmfx.lib and also provides the source
>> code for building libmfx yourself.  If built this way, there are no
>> pkg-config files to be found.  The changes utilize a similar approach
>> to that already done for libx264 in configure.
>>
>> Comments:
>>
>> -- configure: Altered enabled libmfx step to use use_pkg_config()
>>instead of require_pkg_config(), and, if use_pkg_config() fails, it
>>falls back to require().  Note that the reason that require() is
>>passed -llibmfx as the last argument, instead of -lmfx, is the file
>>name for the library produced from the Intel Media SDK starts with
>>"libmfx".  Apparently, the filename for the library produced via
>>https://github.com/lu-zero/mfx_dispatch starts with "mfx".
>> ---
>>  configure | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index 3bea057..b20a0b4 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5819,7 +5819,8 @@ enabled libgsm&& { for gsm_hdr in
>> "gsm.h" "gsm/gsm.h"; do
>> done || die "ERROR: libgsm not found"; }
>>  enabled libilbc   && require libilbc ilbc.h
>> WebRtcIlbcfix_InitDecode -lilbc
>>  enabled libkvazaar&& require_pkg_config "kvazaar >= 0.8.1"
>> kvazaar.h kvz_api_get
>> -enabled libmfx&& require_pkg_config libmfx
>> "mfx/mfxvideo.h" MFXInit
>> +enabled libmfx&& { use_pkg_config libmfx "mfx/mfxvideo.h"
>> MFXInit ||
>> +   { require libmfx "mfx/mfxvideo.h"
>> MFXInit -llibmfx && warn "using libmfx without pkg-config"; } }
>>  enabled libmodplug&& require_pkg_config libmodplug
>> libmodplug/modplug.h ModPlug_Load
>>  enabled libmp3lame&& require "libmp3lame >= 3.98.3"
>> lame/lame.h lame_set_VBR_quality -lmp3lame
>>  enabled libnut&& require libnut libnut.h nut_demuxer_init
>> -lnut
>>
> 
> Pinging this patch submission again.
> 
> Thanks,
> Aaron Levinson

And again.  This patch is pretty straightforward, and considering that this 
approach was deemed suitable for libx264, I don't see why there would be any 
issue with it being applied to libmfx as well.  Here's a new version of this 
patch with some of the text altered slightly.

Thanks,
Aaron Levinson

--

From aa427b09435eb99de6b308d32f066fbca71e4b18 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Fri, 5 May 2017 00:06:42 -0700
Subject: [PATCH] configure:  Added require fallback for libmfx

Purpose: Added require fallback for libmfx in the case that pkg-config
cannot find libmfx.  On Linux, most people likely get libmfx via
https://github.com/lu-zero/mfx_dispatch , but on Windows, the most
well-known way to get libmfx is via the Intel Media SDK, which
provides a static build of libmfx.lib and also provides the source
code for building libmfx yourself.  If built this way, there are no
pkg-config files to be found.  The changes utilize a similar approach
to that already done for libx264 in configure.

Comments:

-- configure: Altered enabled libmfx step to use use_pkg_config()
   instead of require_pkg_config(), and, if use_pkg_config() fails, it
   falls back to require().  Note that the reason that require() is
   passed -llibmfx as the last argument, instead of -lmfx, is the file
   name for the library produced from the Intel Media SDK starts with
   "libmfx".  Apparently, the filename for the library produced via
   https://github.com/lu-zero/mfx_dispatch starts with "mfx".

Signed-off-by: Aaron Levinson 
---
 configure | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index c3fa9d858f..883ec390d6 100755
--- a/configure
+++ b/configure
@@ -5787,7 +5787,8 @@ enabled li

Re: [FFmpeg-devel] [PATCH] Made minor changes to get the decklink avdevice code to build using Visual C++

On 4/16/2017 4:11 PM, Aaron Levinson wrote:
> On 4/15/2017 6:13 AM, Aaron Levinson wrote:
>> On 4/15/2017 4:19 AM, Marton Balint wrote:
>>>
>>> On Thu, 13 Apr 2017, Aaron Levinson wrote:
>>>
>>>> On 4/13/2017 1:23 PM, Hendrik Leppkes wrote:
>>> [...]

Ping, and in addition, I've provided a new patch below since the original won't 
apply anymore.  I've also adjusted the patch text a little bit.  The original 
patch went through a few different reviews.  These changes are necessary to get 
the decklink code to build using MSVC on Windows.

Thanks,
Aaron Levinson



From 90d20d87401584138f6bdea5567b92e7dfe07f1f Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Fri, 5 May 2017 00:20:56 -0700
Subject: [PATCH] decklink:  Fixed MSVC build issues

Purpose: Made minor changes to get the decklink avdevice code to build
using Visual C++.

Notes: Made changes to configure per Hendrik Leppkes's review of first
and second versions of patch.

Comments:

-- configure: Added if enabled decklink section and setting
   decklink_indev_extralibs and decklink_outdev_extralibs here for
   both mingw and Windows.  Also eliminated the setting of these
   variables in the mingw section earlier in the file.

-- libavdevice/decklink_common.cpp: Switched the order of the include
   of libavformat/internal.h to workaround build issues with Visual
   C++.  See comment in file for more details.

-- libavdevice/decklink_dec.cpp:
a) Rearranged the include of libavformat/internal.h (for reasons as
   described above).
b) Made slight alteration to an argument for call to av_rescale_q() to
   workaround a compiler error with Visual C++.  This appears to only
   be an issue when building C++ files with Visual C++.  See comment
   in code for more details.

-- libavdevice/decklink_enc.cpp: Rearranged the include of
   libavformat/internal.h (for reasons as described above).

Signed-off-by: Aaron Levinson 
---
 configure   | 11 +--
 libavdevice/decklink_common.cpp |  7 ++-
 libavdevice/decklink_dec.cpp| 17 +++--
 libavdevice/decklink_enc.cpp|  7 ++-
 4 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/configure b/configure
index 883ec390d6..bbb9fab738 100755
--- a/configure
+++ b/configure
@@ -4847,8 +4847,6 @@ case $target_os in
 else
 target_os=mingw32
 fi
-decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 
-loleaut32"
-decklink_indev_extralibs="$decklink_indev_extralibs -lole32 -loleaut32"
 LIBTARGET=i386
 if enabled x86_64; then
 LIBTARGET="i386:x86-64"
@@ -5947,6 +5945,15 @@ if ! disabled sdl2; then
 fi
 enabled sdl2 && enable sdl && add_cflags $sdl2_cflags && add_extralibs 
$sdl2_extralibs
 
+if enabled decklink; then
+case $target_os in
+mingw32*|mingw64*|win32|win64)
+decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 
-loleaut32"
+decklink_indev_extralibs="$decklink_indev_extralibs -lole32 
-loleaut32"
+;;
+esac
+fi
+
 disabled securetransport || { check_func SecIdentityCreate 
"-Wl,-framework,CoreFoundation -Wl,-framework,Security" &&
 check_lib securetransport "Security/SecureTransport.h Security/Security.h" 
"SSLCreateContext SecItemImport" "-Wl,-framework,CoreFoundation 
-Wl,-framework,Security"; }
 
diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
index f01fba953e..cbb591ce64 100644
--- a/libavdevice/decklink_common.cpp
+++ b/libavdevice/decklink_common.cpp
@@ -19,6 +19,12 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+/* Include internal.h first to avoid conflict between winsock.h (used by
+ * DeckLink headers) and winsock2.h (used by libavformat) in MSVC++ builds */
+extern "C" {
+#include "libavformat/internal.h"
+}
+
 #include 
 #ifdef _WIN32
 #include 
@@ -28,7 +34,6 @@
 
 extern "C" {
 #include "libavformat/avformat.h"
-#include "libavformat/internal.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/bswap.h"
diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index 67eaf97e89..69f790d606 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -19,12 +19,17 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+/* Include internal.h first to avoid conflict between winsock.h (used by
+ * DeckLink headers) and winsock2.h (used by libavformat) in MSVC++ builds */
+extern "C" {
+#include "libavformat/internal.h"
+}
+
 #include 
 
 extern

Re: [FFmpeg-devel] [RFC]lavu/timecode: Increase AV_TIMECODE_STR_SIZE.


On 5/5/2017 12:20 AM, Carl Eugen Hoyos wrote:

2017-05-05 3:29 GMT+02:00 Aaron Levinson :

On 5/4/2017 4:27 PM, Carl Eugen Hoyos wrote:


Hi!

Attached patch is one possibility to fix the following warning with gcc 7:
libavutil/timecode.c: In function ‘av_timecode_make_string’:
libavutil/timecode.c:103:60: warning: ‘%02d’ directive output may be
truncated
writing between 2 and 10 bytes into a region of size between 0 and 7
[-Wformat-truncation=]
 snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
^~~~
libavutil/timecode.c:103:41: note: directive argument in the range [0,
2147483647]
 snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
 ^~~~
libavutil/timecode.c:103:5: note: ‘snprintf’ output between 12 and 32
bytes
into a destination of size 16
 snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
 ^
  neg ? "-" : "",
  ~~~
  hh, mm, ss, drop ? ';' : ':', ff);
  ~

Several similar warnings are printed, an alternative is to disable the
warning.

The warning seemed wrong on first sight but may actually be correct, we
accept invalid fps for timecode.



Regarding the change in your patch:

-#define AV_TIMECODE_STR_SIZE 16
+#define AV_TIMECODE_STR_SIZE 23

It seems like 23 characters wouldn't be sufficient based on the warning:
"output between 12 and 32 bytes into a destination of size 16".  I would
guess that you would need at least 32 characters and perhaps one more (?)
for the terminating null character to avoid that warning.


How can I reproduce / test your findings?


#define AV_TIMECODE_STR_SIZE 32 or 33 and confirm that gcc 7 warnings go 
away.


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

Re: [FFmpeg-devel] Added require fallback for libmfx in the case that pkg-config cannot find libmfx


On 5/5/2017 12:42 AM, Clément Bœsch wrote:

On Fri, May 05, 2017 at 12:11:27AM -0700, Aaron Levinson wrote:

On 4/19/2017 10:43 AM, Aaron Levinson wrote:

On 4/14/2017 6:51 PM, Aaron Levinson wrote:

From e0c73c054add0137901d0bf7a7893e42e7e566c8 Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Fri, 14 Apr 2017 18:38:37 -0700
Subject: [PATCH] Added require fallback for libmfx in the case that
 pkg-config cannot find libmfx

Purpose: Added require fallback for libmfx in the case that pkg-config
cannot find libmfx.  On Linux, most people likely get libmfx via
https://github.com/lu-zero/mfx_dispatch , but on Windows, the most
well-known way to get libmfx is via the Intel Media SDK, which
provides a static build of libmfx.lib and also provides the source
code for building libmfx yourself.  If built this way, there are no
pkg-config files to be found.  The changes utilize a similar approach
to that already done for libx264 in configure.

Comments:

-- configure: Altered enabled libmfx step to use use_pkg_config()
   instead of require_pkg_config(), and, if use_pkg_config() fails, it
   falls back to require().  Note that the reason that require() is
   passed -llibmfx as the last argument, instead of -lmfx, is the file
   name for the library produced from the Intel Media SDK starts with
   "libmfx".  Apparently, the filename for the library produced via
   https://github.com/lu-zero/mfx_dispatch starts with "mfx".
---
 configure | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 3bea057..b20a0b4 100755
--- a/configure
+++ b/configure
@@ -5819,7 +5819,8 @@ enabled libgsm&& { for gsm_hdr in
"gsm.h" "gsm/gsm.h"; do
done || die "ERROR: libgsm not found"; }
 enabled libilbc   && require libilbc ilbc.h
WebRtcIlbcfix_InitDecode -lilbc
 enabled libkvazaar&& require_pkg_config "kvazaar >= 0.8.1"
kvazaar.h kvz_api_get
-enabled libmfx&& require_pkg_config libmfx
"mfx/mfxvideo.h" MFXInit
+enabled libmfx&& { use_pkg_config libmfx "mfx/mfxvideo.h"
MFXInit ||
+   { require libmfx "mfx/mfxvideo.h"
MFXInit -llibmfx && warn "using libmfx without pkg-config"; } }
 enabled libmodplug&& require_pkg_config libmodplug
libmodplug/modplug.h ModPlug_Load
 enabled libmp3lame&& require "libmp3lame >= 3.98.3"
lame/lame.h lame_set_VBR_quality -lmp3lame
 enabled libnut&& require libnut libnut.h nut_demuxer_init
-lnut



Pinging this patch submission again.

Thanks,
Aaron Levinson


And again.  This patch is pretty straightforward, and considering that this 
approach was deemed suitable for libx264


It wasn't, Carl Eugen insisted in doing this against everyone opinion, but
we tolerated his whim because he didn't want to use or install a 80kB
package installed on virtually every system that supports x264.

Anyway, here is a nasty side effect of doing this: you use PKG_CONFIG_PATH
to make sure it's linking against a specific local build of your lib, but
you did it wrong and it ends up linking silently against the system one
instead of failing like you would like to.

We predicted that allowing this hack for x264 was going to bring up
patches like this and we were right.

So maybe we should actually drop the hack for libx264.

Back to your issue: you should fix the .pc in the upstream project, this
is the correct fix.


The "upstream project" in this case is the Intel Media SDK.  This is not 
an open source project, and developers get it through Intel in the form 
of an installation package on Windows.  So, there is no opportunity to 
"fix the .pc"--there isn't one, nor is there any way to contribute one. 
The https://github.com/lu-zero/mfx_dispatch project is not maintained by 
Intel, and I would argue that it is preferable to obtain the relevant 
files via the official Intel release.  This is not the same situation as 
exists with libx264, which is an open source project.  This is not much 
different in concept than the approach someone needs to take to get the 
Blackmagic header files, which are available via the Blackmagic SDK, and 
I don't see it using pkg-config for Blackmagic.


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

Re: [FFmpeg-devel] [RFC]lavu/timecode: Increase AV_TIMECODE_STR_SIZE.


On 5/5/2017 12:44 AM, Carl Eugen Hoyos wrote:

2017-05-05 9:42 GMT+02:00 Aaron Levinson :

On 5/5/2017 12:20 AM, Carl Eugen Hoyos wrote:


2017-05-05 3:29 GMT+02:00 Aaron Levinson :


On 5/4/2017 4:27 PM, Carl Eugen Hoyos wrote:



Hi!

Attached patch is one possibility to fix the following warning with gcc
7:
libavutil/timecode.c: In function ‘av_timecode_make_string’:
libavutil/timecode.c:103:60: warning: ‘%02d’ directive output may be
truncated
writing between 2 and 10 bytes into a region of size between 0 and 7
[-Wformat-truncation=]
 snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
^~~~
libavutil/timecode.c:103:41: note: directive argument in the range [0,
2147483647]
 snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
 ^~~~
libavutil/timecode.c:103:5: note: ‘snprintf’ output between 12 and 32
bytes
into a destination of size 16
 snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
 ^
  neg ? "-" : "",
  ~~~
  hh, mm, ss, drop ? ';' : ':', ff);
  ~

Several similar warnings are printed, an alternative is to disable the
warning.

The warning seemed wrong on first sight but may actually be correct, we
accept invalid fps for timecode.




Regarding the change in your patch:

-#define AV_TIMECODE_STR_SIZE 16
+#define AV_TIMECODE_STR_SIZE 23

It seems like 23 characters wouldn't be sufficient based on the warning:
"output between 12 and 32 bytes into a destination of size 16".  I would
guess that you would need at least 32 characters and perhaps one more (?)
for the terminating null character to avoid that warning.



How can I reproduce / test your findings?



#define AV_TIMECODE_STR_SIZE 32 or 33 and confirm that gcc 7
warnings go away.


Of course!
Since the warnings go away with 23, they also go away with 32.
I just wonder why 23 isn't enough in your opinion?


You hadn't previously indicated that the warnings disappeared when 23 is 
used, so I was operating based on the warning text, which said that the 
output text could consume as much as 32 bytes.  It seems odd that the 
warning text would disappear in this case, but if that's sufficient, it 
seems okay to apply.


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

Re: [FFmpeg-devel] [PATCH] Fixed bug encountered when decoding interlaced video

On 5/5/2017 12:55 AM, wm4 wrote:

On Thu, 4 May 2017 23:46:30 -0700
Aaron Levinson  wrote:

On 4/12/2017 6:08 PM, Aaron Levinson wrote:

On 3/26/2017 10:34 AM, Aaron Levinson wrote:

On 3/26/2017 4:41 AM, Matthias Hunstock wrote:

Am 26.03.2017 um 11:50 schrieb Aaron Levinson:

>>>>> [...]
>>> [...]

I've provided a new version of the patch.  When I created the first version of the patch on March 
26th, this was the first patch that I submitted to ffmpeg, and some aspects were rough.  I had 
indicated that the patch passed regression tests, but all I did was run "make fate", 
instead of "make fate SAMPLES=fate-suite/", and once I understood that I should use 
fate-suite, I discovered that some of the FATE tests failed with this patch.  I fixed the issues 
with the patch, adjusted some comments, and adjusted the patch description text.  I've confirmed 
that this patch passes all fate-suite tests for 64-bit MSVC on Windows and 64-bit gcc on Linux.

Thanks,
Aaron Levinson

From 85aa383f5753795652bae015f4fe91b016f7387e Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Thu, 4 May 2017 22:54:31 -0700
Subject: [PATCH] ffmpeg:  Fixed bug with decoding interlaced video

Purpose: Fixed bug in ffmpeg encountered when decoding interlaced
video.  In some cases, ffmpeg would treat it as progressive.  As a
result of the change, the issues with interlaced video are fixed.

Detailed description of problem: Run the following command: "ffmpeg -i
test8_1080i.h264 -c:v mpeg2video test8_1080i_mp2.ts".  In this case,
test8_1080i.h264 is an H.264-encoded file with 1080i59.94
(interlaced).  Prior to the patch, the following output is generated:

Input #0, h264, from 'test8_1080i.h264':
  Duration: N/A, bitrate: N/A
Stream #0:0: Video: h264 (High), yuv420p(top first), 1920x1080 [SAR 1:1 DAR 
16:9], 29.97 fps, 29.97 tbr, 1200k tbn, 59.94 tbc
Stream mapping:
  Stream #0:0 -> #0:0 (h264 (native) -> mpeg2video (native))
Press [q] to stop, [?] for help
Output #0, mpegts, to 'test8_1080i_mp2_2.ts':
  Metadata:
encoder : Lavf57.72.100
Stream #0:0: Video: mpeg2video (Main), yuv420p, 1920x1080 [SAR 1:1 DAR 
16:9], q=2-31, 200 kb/s, 29.97 fps, 90k tbn, 29.97 tbc
Metadata:
  encoder : Lavc57.92.100 mpeg2video

which demonstrates the bug.  The output should instead look like:

Stream #0:0: Video: mpeg2video (Main), yuv420p(top coded first (swapped)), 
1920x1080 [SAR 1:1 DAR 16:9], q=2-31, 200 kb/s, 29.97 fps, 90k tbn, 29.97 tbc

The bug is caused by the fact that reap_filters() calls
init_output_stream(), which in turn calls check_init_output_file(),
and this is all done prior to the first call to do_video_out().  An
initial call to do_video_out() is necessary to populate the interlaced
video information to the output stream's codecpar
(mux_par->field_order in do_video_out()).  However,
check_init_output_file() calls avformat_write_header() prior to the
initial call to do_video_out(), so field_order is populated too late,
and the header is written with the default field_order value,
progressive.

Comments:

-- ffmpeg.c:
a) Enhanced init_output_stream() to take an additional input
   indicating whether or not check_init_output_file() should be
   called.  There are other places that call init_output_stream(), and
   it was important to make sure that these continued to work
   properly.
b) Adjusted reap_filters() such that, in the case that
   init_output_stream() is called, it indicates that it should not
   call check_init_output_file() in init_output_stream().  Instead, it
   makes the call to check_init_output_file() directly, but after it
   does the filtered frame setup and processing.

Signed-off-by: Aaron Levinson 
---
 ffmpeg.c | 61 +
 1 file changed, 53 insertions(+), 8 deletions(-)

diff --git a/ffmpeg.c b/ffmpeg.c
index e798d92277..45dbfafc04 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -1400,7 +1400,7 @@ static void do_video_stats(OutputStream *ost, int 
frame_size)
 }
 }

-static int init_output_stream(OutputStream *ost, char *error, int error_len);
+static int init_output_stream(OutputStream *ost, char *error, int error_len, 
int do_check_output_file);

 static void finish_output_stream(OutputStream *ost)
 {
@@ -1415,6 +1415,8 @@ static void finish_output_stream(OutputStream *ost)
 }
 }

+static int check_init_output_file(OutputFile *of, int file_index);
+
 /**
  * Get and encode new output from any of the filtergraphs, without causing
  * activity.
@@ -1433,6 +1435,7 @@ static int reap_filters(int flush)
 AVFilterContext *filter;
 AVCodecContext *enc = ost->enc_ctx;
 int ret = 0;
+int did_init_output_stream = 0;

 if (!ost->filter || !ost->filter->graph->graph)
 continue;
@@ -1440,

Re: [FFmpeg-devel] Added require fallback for libmfx in the case that pkg-config cannot find libmfx

On 5/5/2017 12:57 AM, Clément Bœsch wrote:

On Fri, May 05, 2017 at 12:54:12AM -0700, Aaron Levinson wrote:
[...]

Back to your issue: you should fix the .pc in the upstream project, this
is the correct fix.

The "upstream project" in this case is the Intel Media SDK. This is not an
open source project, and developers get it through Intel in the form of an
installation package on Windows. So, there is no opportunity to "fix the
.pc"--there isn't one, nor is there any way to contribute one.

OK so we have a common configure flag for supporting two different
projects with different authors, licensing, etc?

Maybe we should have 2 configure flags? Add --enable-libmfxintel or
whatever?

The licensing is the same--lu_zero's (Luca Barbato's) libmfx project
just packages the files from the Intel Media SDK (or more likely Intel
Media Server Studio). And this means that there is effectively one
author as well. While the licensing for libmfx (really, mfx_dispatch)
is suitable for open source projects, I don't think the same can be
claimed for the Intel Media SDK or the Intel Media Server Studio as a
whole, and it is technically necessary to install one of those to get
the mfx_dispatch sources. I really didn't want this to turn into
another non-free discussion ala Blackmagic. I'm fairly certain that
Intel's intention is that the mfx_dispatch sources are suitable for use
in open source projects, but it would be helpful if Intel offered the
mfx_dispatch sources independent of either the Intel Media SDK or Intel
Media Server Studio.

Nevertheless, it does seem reasonable to have another configure flag to
enable the use of libmfx via Intel's official installers, which don't
provide anything for pkg-config. Even Intel Media Server Studio, which
is available for Linux, doesn't provide anything for pkg-config, and the
instructions at
http://www.intel.com/content/dam/www/public/us/en/documents/white-papers/quicksync-video-ffmpeg-install-valid.pdf
describe a process for manually creating a libmfx.pc file in order to be
able to build ffmpeg with Intel Media Server Studio. I'll scrap this
patch and work on a different patch that uses this approach.

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

Re: [FFmpeg-devel] [PATCH v2 0/9] Removing HEVCContext depencencies


The entire patch set LGTM.

Aaron Levinson

On 5/3/2017 6:41 PM, James Almer wrote:

On 5/3/2017 9:39 PM, Aaron Levinson wrote:

James,

Can you document which patches depend on other patches in this patch set
to make it easier to review this patch set in chunks?


Patch 2 depends on 1. Patches 3, 4 and 5 are functionally standalone but
need previous patches to apply cleanly. Patches 7 and 6 depend on every
patch before themselves. Patch 8 depends on patches 1 and 2 (and maybe
others to apply cleanly).



Thanks,
Aaron Levinson

On 5/2/2017 2:55 PM, James Almer wrote:

Some changes and fixes suggested on IRC. Some patches are v3 as they have
been sent before outside the previous set.

James Almer (9):
  hevcdec: remove HEVCContext usage from hevc_sei
  hevcdec: move SEI message parsing into a separate header
  hevcdec: remove HEVCContext usage from ff_hevc_compute_poc()
  hevcdec: move SliceHeader struct definition to hevc_ps
  hevc_parser: use ff_h2645_packet_split() to parse NAL units
  hevc_parser: remove HEVCContext usage
  hevc_parser: move slice header parsing to its own function
  hevc_parse: decode SEI message NALUs in extradata
  doc/libav_merge: remove line about ADVANCED_PARSER

 doc/libav-merge.txt|   1 -
 libavcodec/Makefile|   4 +-
 libavcodec/hevc_parse.c|  21 ++-
 libavcodec/hevc_parse.h|   7 +-
 libavcodec/hevc_parser.c   | 415
-
 libavcodec/hevc_ps.c   |  23 +++
 libavcodec/hevc_ps.h   |  88 ++
 libavcodec/hevc_refs.c |  27 +--
 libavcodec/hevc_sei.c  | 193 +
 libavcodec/hevc_sei.h  | 125 ++
 libavcodec/hevcdec.c   |  94 +-
 libavcodec/hevcdec.h   | 137 +--
 libavcodec/mediacodecdec.c |   4 +-
 13 files changed, 532 insertions(+), 607 deletions(-)
 create mode 100644 libavcodec/hevc_sei.h


___
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 mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Made minor changes to get the decklink avdevice code to build using Visual C++

On 5/5/2017 4:57 PM, Marton Balint wrote:
> 
> 
> On Fri, 5 May 2017, Aaron Levinson wrote:
> 
>> On 4/16/2017 4:11 PM, Aaron Levinson wrote:
>>> On 4/15/2017 6:13 AM, Aaron Levinson wrote:
>>>> On 4/15/2017 4:19 AM, Marton Balint wrote:
>>>>>
>>>>> On Thu, 13 Apr 2017, Aaron Levinson wrote:
>>>>>
>>>>>> On 4/13/2017 1:23 PM, Hendrik Leppkes wrote:
>>>>> [...]
>>
>> Ping, and in addition, I've provided a new patch below since the
>> original won't apply anymore.  I've also adjusted the patch text a
>> little bit.  The original patch went through a few different reviews.
>> These changes are necessary to get the decklink code to build using
>> MSVC on Windows.
>>
>> Thanks,
>> Aaron Levinson
>>
>> 
>> [...]
>> case PTS_SRC_WALLCLOCK:
>> -pts = av_rescale_q(wallclock, AV_TIME_BASE_Q, time_base);
>> +{
>> +/* MSVC does not support compound literals like
>> AV_TIME_BASE_Q
>> + * in C++ code (compiler error C4576) */
>> +// pts = av_rescale_q(wallclock, AV_TIME_BASE_Q, time_base);
> 
> I'd rather remove the old code, i see no point in keeping it as a comment.

Done--new patch with this change follows at the end.

>> [...]
> 
> The decklink part seems fine otherwise, maybe you should get an ACK from
> Hendrik for the configure part, which I have no opinion about.

Hendrik already reviewed the configure changes last month, and the configure 
part of the patch went through three iterations till it was determined that it 
was adequate, although it wouldn't hurt to have another look since then.

New patch with the slight alteration follows.

Thanks,
Aaron Levinson

--

From 269f836b50f8949dbc45a2578bd16a53952b Mon Sep 17 00:00:00 2001
From: Aaron Levinson 
Date: Fri, 5 May 2017 17:59:21 -0700
Subject: [PATCH] decklink:  Fixed MSVC build issues

Purpose: Made minor changes to get the decklink avdevice code to build
using Visual C++.

Notes: Made changes to configure per Hendrik Leppkes's review of first
and second versions of patch.  Also made slight alterations per Marton
Balint's reviews.

Comments:

-- configure: Added if enabled decklink section and setting
   decklink_indev_extralibs and decklink_outdev_extralibs here for
   both mingw and Windows.  Also eliminated the setting of these
   variables in the mingw section earlier in the file.

-- libavdevice/decklink_common.cpp: Switched the order of the include
   of libavformat/internal.h to workaround build issues with Visual
   C++.  See comment in file for more details.

-- libavdevice/decklink_dec.cpp:
a) Rearranged the include of libavformat/internal.h (for reasons as
   described above).
b) Made slight alteration to an argument for call to av_rescale_q() to
   workaround a compiler error with Visual C++.  This appears to only
   be an issue when building C++ files with Visual C++.  See comment
   in code for more details.

-- libavdevice/decklink_enc.cpp: Rearranged the include of
   libavformat/internal.h (for reasons as described above).

Signed-off-by: Aaron Levinson 
---
 configure   | 11 +--
 libavdevice/decklink_common.cpp |  7 ++-
 libavdevice/decklink_dec.cpp| 16 ++--
 libavdevice/decklink_enc.cpp|  7 ++-
 4 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/configure b/configure
index 2e77a1034e..6f2c900a7b 100755
--- a/configure
+++ b/configure
@@ -4850,8 +4850,6 @@ case $target_os in
 else
 target_os=mingw32
 fi
-decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 
-loleaut32"
-decklink_indev_extralibs="$decklink_indev_extralibs -lole32 -loleaut32"
 LIBTARGET=i386
 if enabled x86_64; then
 LIBTARGET="i386:x86-64"
@@ -5957,6 +5955,15 @@ if ! disabled sdl2; then
 fi
 enabled sdl2 && enable sdl && add_cflags $sdl2_cflags && add_extralibs 
$sdl2_extralibs
 
+if enabled decklink; then
+case $target_os in
+mingw32*|mingw64*|win32|win64)
+decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 
-loleaut32"
+decklink_indev_extralibs="$decklink_indev_extralibs -lole32 
-loleaut32"
+;;
+esac
+fi
+
 disabled securetransport || { check_func SecIdentityCreate 
"-Wl,-framework,CoreFoundation -Wl,-framework,Security" &&
 check_lib securetransport "Security/SecureTransport.h Security/Security.h" 
"SSLCreateContext SecItemImport"

Re: [FFmpeg-devel] Added require fallback for libmfx in the case that pkg-config cannot find libmfx